DFG needs to handle code motion of code in for..in loop bodies
authormsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 3 Nov 2017 00:23:00 +0000 (00:23 +0000)
committermsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 3 Nov 2017 00:23:00 +0000 (00:23 +0000)
https://bugs.webkit.org/show_bug.cgi?id=179212

Reviewed by Keith Miller.

JSTests:

New regression test.

* stress/for-in-side-effects.js: Added.
(getPrototypeOf):
(reset):
(testWithoutFTL.f):
(testWithoutFTL):
(testWithFTL.f):
(testWithFTL):

Source/JavaScriptCore:

The processing of the DFG nodes HasGenericProperty, HasStructureProperty & GetPropertyEnumerator
make calls with side effects.  Updated clobberize() for those nodes to take that into account.

* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):

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

JSTests/ChangeLog
JSTests/stress/for-in-side-effects.js [new file with mode: 0755]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGClobberize.h

index 8ad85c1..3e9e288 100644 (file)
@@ -1,3 +1,20 @@
+2017-11-02  Michael Saboff  <msaboff@apple.com>
+
+        DFG needs to handle code motion of code in for..in loop bodies
+        https://bugs.webkit.org/show_bug.cgi?id=179212
+
+        Reviewed by Keith Miller.
+
+        New regression test.
+
+        * stress/for-in-side-effects.js: Added.
+        (getPrototypeOf):
+        (reset):
+        (testWithoutFTL.f):
+        (testWithoutFTL):
+        (testWithFTL.f):
+        (testWithFTL):
+
 2017-11-02  Filip Pizlo  <fpizlo@apple.com>
 
         AI does not correctly model the clobber case of ArithClz32
diff --git a/JSTests/stress/for-in-side-effects.js b/JSTests/stress/for-in-side-effects.js
new file mode 100755 (executable)
index 0000000..f822eba
--- /dev/null
@@ -0,0 +1,79 @@
+// Regression test for bug 179212
+
+var p = { "a": {} };
+
+var flag = 0;
+var data = [];
+var copy = [];
+
+var z = new Proxy({}, {
+    getPrototypeOf: function() {
+        if (flag == 2) {
+            data[0] = { "x": "I changed" };
+        }
+
+        if (flag == 1) {
+            flag = 2;
+        }
+
+        return {"a": 1, "b": 2}
+    }
+});
+
+p.__proto__ = z;
+
+function reset()
+{
+    flag = 0;
+    data = [1.1, 2.2, 3.3];
+    copy = [];
+}
+
+function runTest(func)
+{
+    reset();
+
+    for (var i = 0; i < 0x10000; i++)
+        func();
+
+    flag = 1;
+    func();
+
+    if (copy[0].x != "I changed")
+        throw "Expected updated value for copy[0]";
+}
+
+function testWithoutFTL()
+{
+    function f()
+    {
+        data[0] = 2.2;
+        for(var d in p) {
+            copy[0] = data[0];
+            copy[1] = data[1];
+            copy[2] = data[2];
+        }
+    }
+
+    noFTL(f);
+
+    runTest(f);
+}
+
+function testWithFTL()
+{
+    function f()
+    {
+        data[0] = 2.2;
+        for(var d in p) {
+            copy[0] = data[0];
+            copy[1] = data[1];
+            copy[2] = data[2];
+        }
+    }
+
+    runTest(f);
+}
+
+testWithoutFTL();
+testWithFTL();
index 968f5c2..eefd256 100644 (file)
@@ -1,3 +1,16 @@
+2017-11-02  Michael Saboff  <msaboff@apple.com>
+
+        DFG needs to handle code motion of code in for..in loop bodies
+        https://bugs.webkit.org/show_bug.cgi?id=179212
+
+        Reviewed by Keith Miller.
+
+        The processing of the DFG nodes HasGenericProperty, HasStructureProperty & GetPropertyEnumerator
+        make calls with side effects.  Updated clobberize() for those nodes to take that into account.
+
+        * dfg/DFGClobberize.h:
+        (JSC::DFG::clobberize):
+
 2017-11-02  Joseph Pecoraro  <pecoraro@apple.com>
 
         Inspector should display service worker served responses properly
index b4940d9..bbd450a 100644 (file)
@@ -270,8 +270,13 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
 
     case HasGenericProperty:
     case HasStructureProperty:
-    case GetEnumerableLength:
     case GetPropertyEnumerator: {
+        read(World);
+        write(Heap);
+        return;
+    }
+
+    case GetEnumerableLength: {
         read(Heap);
         write(SideState);
         return;