Title: [244211] trunk
Revision
244211
Author
ysuz...@apple.com
Date
2019-04-11 23:35:17 -0700 (Thu, 11 Apr 2019)

Log Message

[JSC] op_has_indexed_property should not assume subscript part is Uint32
https://bugs.webkit.org/show_bug.cgi?id=196850

Reviewed by Saam Barati.

JSTests:

* stress/has-indexed-property-should-accept-non-int32.js: Added.
(foo):

Source/_javascript_Core:

op_has_indexed_property assumed that subscript part is always Uint32. However, this is just a load from non-constant RegisterID,
DFG can store it in double format and can perform OSR exit. op_has_indexed_property should not assume that.
In this patch, instead, we check it with isAnyInt and get uint32_t from AnyInt.

* jit/JITOpcodes.cpp:
(JSC::JIT::emit_op_has_indexed_property):
* jit/JITOpcodes32_64.cpp:
(JSC::JIT::emit_op_has_indexed_property):
* jit/JITOperations.cpp:
* runtime/CommonSlowPaths.cpp:
(JSC::SLOW_PATH_DECL):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (244210 => 244211)


--- trunk/JSTests/ChangeLog	2019-04-12 06:02:50 UTC (rev 244210)
+++ trunk/JSTests/ChangeLog	2019-04-12 06:35:17 UTC (rev 244211)
@@ -1,3 +1,13 @@
+2019-04-11  Yusuke Suzuki  <ysuz...@apple.com>
+
+        [JSC] op_has_indexed_property should not assume subscript part is Uint32
+        https://bugs.webkit.org/show_bug.cgi?id=196850
+
+        Reviewed by Saam Barati.
+
+        * stress/has-indexed-property-should-accept-non-int32.js: Added.
+        (foo):
+
 2019-04-11  Saam barati  <sbar...@apple.com>
 
         Remove invalid assertion in operationInstanceOfCustom

Added: trunk/JSTests/stress/has-indexed-property-should-accept-non-int32.js (0 => 244211)


--- trunk/JSTests/stress/has-indexed-property-should-accept-non-int32.js	                        (rev 0)
+++ trunk/JSTests/stress/has-indexed-property-should-accept-non-int32.js	2019-04-12 06:35:17 UTC (rev 244211)
@@ -0,0 +1,13 @@
+//@ runDefault("--useRandomizingFuzzerAgent=1", "--jitPolicyScale=0", "--useMaximalFlushInsertionPhase=1", "--useConcurrentJIT=0")
+function foo(obj) {
+  for (var x in obj) {
+    if (0 > 0) {
+      break;
+    }
+  }
+  0 && Object.prototype.hasOwnProperty
+}
+
+foo([]);
+foo([]);
+foo([0, 0]);

Modified: trunk/Source/_javascript_Core/ChangeLog (244210 => 244211)


--- trunk/Source/_javascript_Core/ChangeLog	2019-04-12 06:02:50 UTC (rev 244210)
+++ trunk/Source/_javascript_Core/ChangeLog	2019-04-12 06:35:17 UTC (rev 244211)
@@ -1,3 +1,22 @@
+2019-04-11  Yusuke Suzuki  <ysuz...@apple.com>
+
+        [JSC] op_has_indexed_property should not assume subscript part is Uint32
+        https://bugs.webkit.org/show_bug.cgi?id=196850
+
+        Reviewed by Saam Barati.
+
+        op_has_indexed_property assumed that subscript part is always Uint32. However, this is just a load from non-constant RegisterID,
+        DFG can store it in double format and can perform OSR exit. op_has_indexed_property should not assume that.
+        In this patch, instead, we check it with isAnyInt and get uint32_t from AnyInt.
+
+        * jit/JITOpcodes.cpp:
+        (JSC::JIT::emit_op_has_indexed_property):
+        * jit/JITOpcodes32_64.cpp:
+        (JSC::JIT::emit_op_has_indexed_property):
+        * jit/JITOperations.cpp:
+        * runtime/CommonSlowPaths.cpp:
+        (JSC::SLOW_PATH_DECL):
+
 2019-04-11  Saam barati  <sbar...@apple.com>
 
         Remove invalid assertion in operationInstanceOfCustom

Modified: trunk/Source/_javascript_Core/jit/JITOpcodes.cpp (244210 => 244211)


--- trunk/Source/_javascript_Core/jit/JITOpcodes.cpp	2019-04-12 06:02:50 UTC (rev 244210)
+++ trunk/Source/_javascript_Core/jit/JITOpcodes.cpp	2019-04-12 06:35:17 UTC (rev 244211)
@@ -1249,6 +1249,8 @@
     
     emitGetVirtualRegisters(base, regT0, property, regT1);
 
+    emitJumpSlowCaseIfNotInt(regT1);
+
     // This is technically incorrect - we're zero-extending an int32. On the hot path this doesn't matter.
     // We check the value as if it was a uint32 against the m_vectorLength - which will always fail if
     // number was signed since m_vectorLength is always less than intmax (since the total allocation

Modified: trunk/Source/_javascript_Core/jit/JITOpcodes32_64.cpp (244210 => 244211)


--- trunk/Source/_javascript_Core/jit/JITOpcodes32_64.cpp	2019-04-12 06:02:50 UTC (rev 244210)
+++ trunk/Source/_javascript_Core/jit/JITOpcodes32_64.cpp	2019-04-12 06:35:17 UTC (rev 244211)
@@ -1121,7 +1121,8 @@
     emitLoadPayload(base, regT0);
     emitJumpSlowCaseIfNotJSCell(base);
 
-    emitLoadPayload(property, regT1);
+    emitLoad(property, regT3, regT1);
+    addSlowCase(branchIfNotInt32(regT3));
 
     // This is technically incorrect - we're zero-extending an int32. On the hot path this doesn't matter.
     // We check the value as if it was a uint32 against the m_vectorLength - which will always fail if

Modified: trunk/Source/_javascript_Core/jit/JITOperations.cpp (244210 => 244211)


--- trunk/Source/_javascript_Core/jit/JITOperations.cpp	2019-04-12 06:02:50 UTC (rev 244210)
+++ trunk/Source/_javascript_Core/jit/JITOperations.cpp	2019-04-12 06:35:17 UTC (rev 244211)
@@ -2013,7 +2013,7 @@
     JSValue subscript = JSValue::decode(encodedSubscript);
     
     ASSERT(baseValue.isObject());
-    ASSERT(subscript.isUInt32());
+    ASSERT(subscript.isUInt32AsAnyInt());
 
     JSObject* object = asObject(baseValue);
     bool didOptimize = false;
@@ -2043,7 +2043,7 @@
         }
     }
 
-    uint32_t index = subscript.asUInt32();
+    uint32_t index = subscript.asUInt32AsAnyInt();
     if (object->canGetIndexQuickly(index))
         return JSValue::encode(JSValue(JSValue::JSTrue));
 
@@ -2064,10 +2064,10 @@
     JSValue subscript = JSValue::decode(encodedSubscript);
     
     ASSERT(baseValue.isObject());
-    ASSERT(subscript.isUInt32());
+    ASSERT(subscript.isUInt32AsAnyInt());
 
     JSObject* object = asObject(baseValue);
-    uint32_t index = subscript.asUInt32();
+    uint32_t index = subscript.asUInt32AsAnyInt();
     if (object->canGetIndexQuickly(index))
         return JSValue::encode(JSValue(JSValue::JSTrue));
 
@@ -2077,7 +2077,7 @@
         // https://bugs.webkit.org/show_bug.cgi?id=149886
         byValInfo->arrayProfile->setOutOfBounds();
     }
-    return JSValue::encode(jsBoolean(object->hasPropertyGeneric(exec, subscript.asUInt32(), PropertySlot::InternalMethodType::GetOwnProperty)));
+    return JSValue::encode(jsBoolean(object->hasPropertyGeneric(exec, index, PropertySlot::InternalMethodType::GetOwnProperty)));
 }
     
 EncodedJSValue JIT_OPERATION operationGetByValString(ExecState* exec, EncodedJSValue encodedBase, EncodedJSValue encodedSubscript, ByValInfo* byValInfo)

Modified: trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp (244210 => 244211)


--- trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp	2019-04-12 06:02:50 UTC (rev 244210)
+++ trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp	2019-04-12 06:35:17 UTC (rev 244211)
@@ -1530,8 +1530,8 @@
     JSArray* resultArray = jsCast<JSArray*>(exec->uncheckedArgument(0));
     JSArray* otherArray = jsCast<JSArray*>(exec->uncheckedArgument(1));
     JSValue startValue = exec->uncheckedArgument(2);
-    ASSERT(startValue.isAnyInt() && startValue.asAnyInt() >= 0 && startValue.asAnyInt() <= std::numeric_limits<unsigned>::max());
-    unsigned startIndex = static_cast<unsigned>(startValue.asAnyInt());
+    ASSERT(startValue.isUInt32AsAnyInt());
+    unsigned startIndex = startValue.asUInt32AsAnyInt();
     bool success = resultArray->appendMemcpy(exec, vm, startIndex, otherArray);
     EXCEPTION_ASSERT(!scope.exception() || !success);
     if (success)

Modified: trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp (244210 => 244211)


--- trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp	2019-04-12 06:02:50 UTC (rev 244210)
+++ trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp	2019-04-12 06:35:17 UTC (rev 244211)
@@ -903,8 +903,8 @@
     CHECK_EXCEPTION();
     JSValue property = GET(bytecode.m_property).jsValue();
     metadata.m_arrayProfile.observeStructure(base->structure(vm));
-    ASSERT(property.isUInt32());
-    RETURN(jsBoolean(base->hasPropertyGeneric(exec, property.asUInt32(), PropertySlot::InternalMethodType::GetOwnProperty)));
+    ASSERT(property.isUInt32AsAnyInt());
+    RETURN(jsBoolean(base->hasPropertyGeneric(exec, property.asUInt32AsAnyInt(), PropertySlot::InternalMethodType::GetOwnProperty)));
 }
 
 SLOW_PATH_DECL(slow_path_has_structure_property)
@@ -996,11 +996,8 @@
     BEGIN();
     auto bytecode = pc->as<OpToIndexString>();
     JSValue indexValue = GET(bytecode.m_index).jsValue();
-    ASSERT(indexValue.isAnyInt());
-    ASSERT(indexValue.asAnyInt() <= UINT32_MAX);
-    ASSERT(indexValue.asAnyInt() >= 0);
-    uint32_t index = static_cast<uint32_t>(indexValue.asAnyInt());
-    RETURN(jsString(exec, Identifier::from(exec, index).string()));
+    ASSERT(indexValue.isUInt32AsAnyInt());
+    RETURN(jsString(exec, Identifier::from(exec, indexValue.asUInt32AsAnyInt()).string()));
 }
 
 SLOW_PATH_DECL(slow_path_profile_type_clear_log)

Modified: trunk/Source/_javascript_Core/runtime/JSCJSValue.h (244210 => 244211)


--- trunk/Source/_javascript_Core/runtime/JSCJSValue.h	2019-04-12 06:02:50 UTC (rev 244210)
+++ trunk/Source/_javascript_Core/runtime/JSCJSValue.h	2019-04-12 06:35:17 UTC (rev 244211)
@@ -211,6 +211,8 @@
     int32_t asInt32() const;
     uint32_t asUInt32() const;
     int64_t asAnyInt() const;
+    uint32_t asUInt32AsAnyInt() const;
+    int32_t asInt32AsAnyInt() const;
     double asDouble() const;
     bool asBoolean() const;
     double asNumber() const;
@@ -228,6 +230,8 @@
     bool isUndefinedOrNull() const;
     bool isBoolean() const;
     bool isAnyInt() const;
+    bool isUInt32AsAnyInt() const;
+    bool isInt32AsAnyInt() const;
     bool isNumber() const;
     bool isString() const;
     bool isBigInt() const;

Modified: trunk/Source/_javascript_Core/runtime/JSCJSValueInlines.h (244210 => 244211)


--- trunk/Source/_javascript_Core/runtime/JSCJSValueInlines.h	2019-04-12 06:02:50 UTC (rev 244210)
+++ trunk/Source/_javascript_Core/runtime/JSCJSValueInlines.h	2019-04-12 06:35:17 UTC (rev 244211)
@@ -571,6 +571,38 @@
     return static_cast<int64_t>(asDouble());
 }
 
+inline bool JSValue::isInt32AsAnyInt() const
+{
+    if (!isAnyInt())
+        return false;
+    int64_t value = asAnyInt();
+    return value >= INT32_MIN && value <= INT32_MAX;
+}
+
+inline int32_t JSValue::asInt32AsAnyInt() const
+{
+    ASSERT(isInt32AsAnyInt());
+    if (isInt32())
+        return asInt32();
+    return static_cast<int32_t>(asDouble());
+}
+
+inline bool JSValue::isUInt32AsAnyInt() const
+{
+    if (!isAnyInt())
+        return false;
+    int64_t value = asAnyInt();
+    return value >= 0 && value <= UINT32_MAX;
+}
+
+inline uint32_t JSValue::asUInt32AsAnyInt() const
+{
+    ASSERT(isUInt32AsAnyInt());
+    if (isUInt32())
+        return asUInt32();
+    return static_cast<uint32_t>(asDouble());
+}
+
 inline bool JSValue::isString() const
 {
     return isCell() && asCell()->isString();

Modified: trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp (244210 => 244211)


--- trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp	2019-04-12 06:02:50 UTC (rev 244210)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp	2019-04-12 06:35:17 UTC (rev 244211)
@@ -248,11 +248,8 @@
     JSValue lengthValue = exec->uncheckedArgument(3);
     JSString* nameString = asString(exec->uncheckedArgument(4));
 
-    ASSERT(lengthValue.isAnyInt());
-    ASSERT(lengthValue.asAnyInt() <= INT32_MAX);
-    ASSERT(lengthValue.asAnyInt() >= INT32_MIN);
-    int32_t length = lengthValue.toInt32(exec);
-    scope.assertNoException();
+    ASSERT(lengthValue.isInt32AsAnyInt());
+    int32_t length = lengthValue.asInt32AsAnyInt();
 
     String name = nameString->value(exec);
     RETURN_IF_EXCEPTION(scope, { });
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to