Spread's effects are modeled incorrectly both in AI and in Clobberize
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 19 Jan 2018 21:15:55 +0000 (21:15 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 19 Jan 2018 21:15:55 +0000 (21:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=181867
<rdar://problem/36290415>

Reviewed by Michael Saboff.

JSTests:

* stress/ai-needs-to-model-spreads-effects.js: Added.
(try.p.Symbol.iterator):
(try.go):
(catch):
* stress/clobberize-needs-to-model-spread-effects.js: Added.
(assert):
(foo):
(a.Symbol.iterator):

Source/JavaScriptCore:

* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):

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

JSTests/ChangeLog
JSTests/stress/ai-needs-to-model-spreads-effects.js [new file with mode: 0644]
JSTests/stress/clobberize-needs-to-model-spread-effects.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
Source/JavaScriptCore/dfg/DFGClobberize.h

index ce9adc746a6bcf008495de3bda778cdd0f46a773..743bc050cb0806daac609818de19b7b5bc5323c3 100644 (file)
@@ -1,3 +1,20 @@
+2018-01-19  Saam Barati  <sbarati@apple.com>
+
+        Spread's effects are modeled incorrectly both in AI and in Clobberize
+        https://bugs.webkit.org/show_bug.cgi?id=181867
+        <rdar://problem/36290415>
+
+        Reviewed by Michael Saboff.
+
+        * stress/ai-needs-to-model-spreads-effects.js: Added.
+        (try.p.Symbol.iterator):
+        (try.go):
+        (catch):
+        * stress/clobberize-needs-to-model-spread-effects.js: Added.
+        (assert):
+        (foo):
+        (a.Symbol.iterator):
+
 2018-01-19  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         Unreviewed, reduce count of iteration to fix timing out debug JSC test
diff --git a/JSTests/stress/ai-needs-to-model-spreads-effects.js b/JSTests/stress/ai-needs-to-model-spreads-effects.js
new file mode 100644 (file)
index 0000000..71e3c70
--- /dev/null
@@ -0,0 +1,33 @@
+try {
+   var ary_1 = [1.1,2.2,3.3]
+   var ary_2 = [1.1,2.2,3.3]
+   var ary_3 = [1.1,2.2,3.3]
+   ary_3['www'] = 1
+   var f64_1 = new Float64Array(0x10)
+   f64_1['0x7a'] = 0xffffffff
+
+   var flag = 0;
+   var p = {"a":{}};
+   p[Symbol.iterator] = function* () {
+       if (flag == 1) {
+           ary_2[0] = {}
+       }
+       yield 1;
+       yield 2;
+   };
+   var go = function(a,b,c){
+       a[0] = 1.1;
+       a[1] = 2.2;
+       [...c];
+       b[0] = a[0];
+       a[2] = 2.3023e-320
+   }
+
+   for (var i = 0; i < 0x100000; i++) {
+       go(ary_1, f64_1, p)
+   }
+
+   flag = 1;
+
+   go(ary_2, f64_1, p);
+} catch(e) { }
diff --git a/JSTests/stress/clobberize-needs-to-model-spread-effects.js b/JSTests/stress/clobberize-needs-to-model-spread-effects.js
new file mode 100644 (file)
index 0000000..76218dd
--- /dev/null
@@ -0,0 +1,23 @@
+function assert(b) {
+    if (!b)
+        throw new Error;
+}
+noInline(assert);
+
+function foo(a, b) {
+    let r1 = b[0];
+    let x = [...a];
+    let r2 = b[0];
+    assert(r1 + r2 === 43);
+}
+noInline(foo);
+
+let b = [42];
+let a = [];
+a[Symbol.iterator] = function* () {
+    b[0] = 1;
+};
+for (let i = 0; i < 10000; ++i) {
+    b[0] = 42;
+    foo(a, b);
+}
index 48bf955a6e2badcbcbaaf7a232cd6bd0c34c2503..542b09141f9fa5f36ccdb41394810a9bae88072a 100644 (file)
@@ -1,3 +1,16 @@
+2018-01-19  Saam Barati  <sbarati@apple.com>
+
+        Spread's effects are modeled incorrectly both in AI and in Clobberize
+        https://bugs.webkit.org/show_bug.cgi?id=181867
+        <rdar://problem/36290415>
+
+        Reviewed by Michael Saboff.
+
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * dfg/DFGClobberize.h:
+        (JSC::DFG::clobberize):
+
 2018-01-19  Keith Miller  <keith_miller@apple.com>
 
         HaveInternalSDK includes should be "#include?"
index f4313697bcc6fc250fb992c9bafde38e5951e827..64f3db9094387b5cfeca3d74824eef834f7938f7 100644 (file)
@@ -2149,6 +2149,9 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
         break;
 
     case Spread:
+        if (!m_graph.canDoFastSpread(node, forNode(node->child1())))
+            clobberWorld(node->origin.semantic, clobberLimit);
+
         forNode(node).set(
             m_graph, m_vm.fixedArrayStructure.get());
         break;
index 4d666e9a0ebeb84ef52ad549bb6a9fb1b04a7a0b..4a7a35f495298d59082e6aa1bb424e2977f8e50e 100644 (file)
@@ -1321,21 +1321,13 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
     }
 
     case Spread: {
-        if (node->child1()->op() == PhantomCreateRest)
-            read(Stack);
-
-        if (node->child1().useKind() == ArrayUse) {
-            // FIXME: We can probably CSE these together, but we need to construct the right rules
-            // to prove that nobody writes to child1() in between two Spreads: https://bugs.webkit.org/show_bug.cgi?id=164531
-            read(HeapObjectCount); 
-            read(JSCell_indexingType);
-            read(JSObject_butterfly);
-            read(Butterfly_publicLength);
-            read(IndexedDoubleProperties);
-            read(IndexedInt32Properties);
-            read(IndexedContiguousProperties);
-            read(IndexedArrayStorageProperties);
+        if (node->child1()->op() == PhantomNewArrayBuffer) {
+            read(MiscFields);
+            return;
+        }
 
+        if (node->child1()->op() == PhantomCreateRest) {
+            read(Stack);
             write(HeapObjectCount);
             return;
         }