Changeset 260805 in webkit

Timestamp:

Apr 27, 2020 6:33:22 PM (4 years ago)

Author:

ysuzuki@apple.com

Message:

[JSC] >>> should call ToNumeric
​https://bugs.webkit.org/show_bug.cgi?id=211065

Reviewed by Ross Kirsling.

JSTests:

• stress/bigint-urshift.js: Added.

(shouldBe):
(shouldThrow):

• test262/expectations.yaml:

Source/JavaScriptCore:

While BigInt does not support >>> operator, >>> operator should call ToNumeric (in this case, toBigIntOrInt32) for both before throwing an error.
We call toBigIntOrInt32 for both operands, and throw an error. And after that, casting int32_t to uint32_t to perform >>> operator. This is correct
since the only difference between toUint32 and toInt32 is casting int32_t result to uint32_t.

• dfg/DFGOperations.cpp:
• runtime/CommonSlowPaths.cpp:

(JSC::SLOW_PATH_DECL):

• runtime/Operations.h:

(JSC::shift):
(JSC::jsURShift):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/bigint-urshift.js (added)
• JSTests/test262/expectations.yaml (modified) (1 diff)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/dfg/DFGOperations.cpp (modified) (1 diff)
• Source/JavaScriptCore/runtime/CommonSlowPaths.cpp (modified) (1 diff)
• Source/JavaScriptCore/runtime/JSCJSValue.h (modified) (1 diff)
• Source/JavaScriptCore/runtime/JSCJSValueInlines.h (modified) (2 diffs)
• Source/JavaScriptCore/runtime/Operations.h (modified) (2 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+2020-04-27  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] >>> should call ToNumeric
+        https://bugs.webkit.org/show_bug.cgi?id=211065
+
+        Reviewed by Ross Kirsling.
+
+        * stress/bigint-urshift.js: Added.
+        (shouldBe):
+        (shouldThrow):
+        * test262/expectations.yaml:
+
 2020-04-27  Yusuke Suzuki  <ysuzuki@apple.com>
 

trunk/JSTests/test262/expectations.yaml

@@ -3093 +3093 @@
   default: 'Test262Error: should not be called'
   strict mode: 'Test262Error: should not be called'
-test/language/expressions/unsigned-right-shift/bigint-toprimitive.js:
-  default: 'Test262Error: 0n >>> {[Symbol.toPrimitive]: function() {throw new MyError();}} throws MyError Expected a MyError but got a TypeError'
-  strict mode: 'Test262Error: 0n >>> {[Symbol.toPrimitive]: function() {throw new MyError();}} throws MyError Expected a MyError but got a TypeError'
 test/language/expressions/yield/star-iterable.js:
   default: 'Test262Error: First result `done` flag Expected SameValue(«false», «undefined») to be true'

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2020-04-27  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] >>> should call ToNumeric
+        https://bugs.webkit.org/show_bug.cgi?id=211065
+
+        Reviewed by Ross Kirsling.
+
+        While BigInt does not support >>> operator, >>> operator should call ToNumeric (in this case, toBigIntOrInt32) for both before throwing an error.
+        We call toBigIntOrInt32 for both operands, and throw an error. And after that, casting int32_t to uint32_t to perform >>> operator. This is correct
+        since the only difference between toUint32 and toInt32 is casting int32_t result to uint32_t.
+
+        * dfg/DFGOperations.cpp:
+        * runtime/CommonSlowPaths.cpp:
+        (JSC::SLOW_PATH_DECL):
+        * runtime/Operations.h:
+        (JSC::shift):
+        (JSC::jsURShift):
+
 2020-04-27  Keith Miller  <keith_miller@apple.com>
 

trunk/Source/JavaScriptCore/dfg/DFGOperations.cpp

@@ -507 +507 @@
     CallFrame* callFrame = DECLARE_CALL_FRAME(vm);
     JITOperationPrologueCallFrameTracer tracer(vm, callFrame);
-    auto scope = DECLARE_THROW_SCOPE(vm);
-
-    JSValue op1 = JSValue::decode(encodedOp1);
-    JSValue op2 = JSValue::decode(encodedOp2);
-
-    uint32_t a = op1.toUInt32(globalObject);
-    RETURN_IF_EXCEPTION(scope, encodedJSValue());
-    scope.release();
-    uint32_t b = op2.toUInt32(globalObject);
-    return JSValue::encode(jsNumber(static_cast<int32_t>(a >> (b & 0x1f))));
+
+    return JSValue::encode(jsURShift(globalObject, JSValue::decode(encodedOp1), JSValue::decode(encodedOp2)));
 }
 

trunk/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp

@@ -756 +756 @@
     BEGIN();
     auto bytecode = pc->as<OpUrshift>();
-    uint32_t a = GET_C(bytecode.m_lhs).jsValue().toUInt32(globalObject);
-    if (UNLIKELY(throwScope.exception()))
-        RETURN(JSValue());
-    uint32_t b = GET_C(bytecode.m_rhs).jsValue().toUInt32(globalObject);
-    RETURN(jsNumber(static_cast<int32_t>(a >> (b & 31))));
+    JSValue left = GET_C(bytecode.m_lhs).jsValue();
+    JSValue right = GET_C(bytecode.m_rhs).jsValue();
+
+    JSValue result = jsURShift(globalObject, left, right);
+    CHECK_EXCEPTION();
+    RETURN(result);
 }
 

trunk/Source/JavaScriptCore/runtime/JSCJSValue.h

@@ -295 +295 @@
     double toLength(JSGlobalObject*) const;
 
+    Optional<uint32_t> toUInt32AfterToNumeric(JSGlobalObject*) const;
+
     // Floating point conversions (this is a convenience function for WebCore;
     // single precision float is not a representation used in JS or JSC).

trunk/Source/JavaScriptCore/runtime/JSCJSValueInlines.h

@@ -54 +54 @@
 inline uint32_t JSValue::toUInt32(JSGlobalObject* globalObject) const
 {
-    // See comment on JSC::toUInt32, in JSCJSValue.h.
+    // The only difference between toInt32 and toUint32 is that toUint32 reinterprets resulted int32_t value as uint32_t.
+    // https://tc39.es/ecma262/#sec-touint32
     return toInt32(globalObject);
 }
@@ -848 +849 @@
 
     return jsNumber(value);
+}
+
+ALWAYS_INLINE Optional<uint32_t> JSValue::toUInt32AfterToNumeric(JSGlobalObject* globalObject) const
+{
+    VM& vm = getVM(globalObject);
+    auto scope = DECLARE_THROW_SCOPE(vm);
+    JSValue result = toBigIntOrInt32(globalObject);
+    RETURN_IF_EXCEPTION(scope, { });
+    if (LIKELY(result.isInt32()))
+        return static_cast<uint32_t>(result.asInt32());
+    return WTF::nullopt;
 }
 

trunk/Source/JavaScriptCore/runtime/Operations.h

@@ -793 +793 @@
 #endif
 
-    if (!(leftNumeric.isBigInt() && rightNumeric.isBigInt())) {
+    if (UNLIKELY(!(leftNumeric.isBigInt() && rightNumeric.isBigInt()))) {
         auto errorMessage = isLeft ? "Invalid mix of BigInt and other type in left shift operation." : "Invalid mix of BigInt and other type in signed right shift operation.";
         return throwTypeError(globalObject, scope, errorMessage);
@@ -824 +824 @@
 }
 
+ALWAYS_INLINE JSValue jsURShift(JSGlobalObject* globalObject, JSValue left, JSValue right)
+{
+    VM& vm = globalObject->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
+    Optional<uint32_t> leftUint32 = left.toUInt32AfterToNumeric(globalObject);
+    RETURN_IF_EXCEPTION(scope, { });
+    Optional<uint32_t> rightUint32 = right.toUInt32AfterToNumeric(globalObject);
+    RETURN_IF_EXCEPTION(scope, { });
+
+    if (UNLIKELY(!leftUint32 || !rightUint32)) {
+        throwTypeError(globalObject, scope, "BigInt does not support >>> operator"_s);
+        return { };
+    }
+
+    return jsNumber(static_cast<int32_t>(leftUint32.value() >> (rightUint32.value() & 31)));
+}
+
 template<typename HeapBigIntOperation, typename Int32Operation>
 ALWAYS_INLINE JSValue bitwiseBinaryOp(JSGlobalObject* globalObject, JSValue v1, JSValue v2, HeapBigIntOperation&& bigIntOp, Int32Operation&& int32Op, const char* errorMessage)