Remove invalid assertion in operationInstanceOfCustom
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Apr 2019 01:04:32 +0000 (01:04 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Apr 2019 01:04:32 +0000 (01:04 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196842
<rdar://problem/49725493>

Reviewed by Michael Saboff.

JSTests:

* stress/operationInstanceOfCustom-bad-assertion.js: Added.

Source/JavaScriptCore:

In the generated JIT code, we go to the slow path when the incoming function
isn't the Node's CodeOrigin's functionProtoHasInstanceSymbolFunction. However,
in the JIT operation, we were asserting against exec->lexicalGlobalObject()'s
functionProtoHasInstanceSymbolFunction. That assertion might be wrong when
inlining across global objects as exec->lexicalGlobalObject() uses the machine
frame for procuring the global object. There is no harm when this assertion fails
as we just execute the slow path. This patch removes the assertion. (However, this
does shed light on the deficiency in our exec->lexicalGlobalObject() function with
respect to inlining. However, this isn't new -- we've known about this for a while.)

* jit/JITOperations.cpp:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@244206 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/operationInstanceOfCustom-bad-assertion.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/jit/JITOperations.cpp

index 3b1cf82..7bdded8 100644 (file)
@@ -1,3 +1,13 @@
+2019-04-11  Saam barati  <sbarati@apple.com>
+
+        Remove invalid assertion in operationInstanceOfCustom
+        https://bugs.webkit.org/show_bug.cgi?id=196842
+        <rdar://problem/49725493>
+
+        Reviewed by Michael Saboff.
+
+        * stress/operationInstanceOfCustom-bad-assertion.js: Added.
+
 2019-04-10  Saam Barati  <sbarati@apple.com>
 
         AbstractValue::validateOSREntryValue is wrong for Int52 constants
diff --git a/JSTests/stress/operationInstanceOfCustom-bad-assertion.js b/JSTests/stress/operationInstanceOfCustom-bad-assertion.js
new file mode 100644 (file)
index 0000000..919ffae
--- /dev/null
@@ -0,0 +1,11 @@
+const x = createGlobalObject();
+
+function foo(resolve) {
+    0 instanceof resolve;
+}
+
+noInline(x.Promise);
+
+for (let i = 0; i < 1000; i++) {
+    new x.Promise(foo);
+}
index b6f41e4..e7b3f98 100644 (file)
@@ -1,3 +1,23 @@
+2019-04-11  Saam barati  <sbarati@apple.com>
+
+        Remove invalid assertion in operationInstanceOfCustom
+        https://bugs.webkit.org/show_bug.cgi?id=196842
+        <rdar://problem/49725493>
+
+        Reviewed by Michael Saboff.
+
+        In the generated JIT code, we go to the slow path when the incoming function
+        isn't the Node's CodeOrigin's functionProtoHasInstanceSymbolFunction. However,
+        in the JIT operation, we were asserting against exec->lexicalGlobalObject()'s
+        functionProtoHasInstanceSymbolFunction. That assertion might be wrong when
+        inlining across global objects as exec->lexicalGlobalObject() uses the machine
+        frame for procuring the global object. There is no harm when this assertion fails
+        as we just execute the slow path. This patch removes the assertion. (However, this
+        does shed light on the deficiency in our exec->lexicalGlobalObject() function with
+        respect to inlining. However, this isn't new -- we've known about this for a while.)
+
+        * jit/JITOperations.cpp:
+
 2019-04-11  Michael Saboff  <msaboff@apple.com>
 
         Improve the Inline Cache Stats code
index b54c92a..cedd5bf 100644 (file)
@@ -1822,8 +1822,6 @@ int32_t JIT_OPERATION operationInstanceOfCustom(ExecState* exec, EncodedJSValue
     JSValue value = JSValue::decode(encodedValue);
     JSValue hasInstanceValue = JSValue::decode(encodedHasInstance);
 
-    ASSERT(hasInstanceValue != exec->lexicalGlobalObject()->functionProtoHasInstanceSymbolFunction() || !constructor->structure(vm)->typeInfo().implementsDefaultHasInstance());
-
     if (constructor->hasInstance(exec, value, hasInstanceValue))
         return 1;
     return 0;