REGRESSION(174226): Captured arguments in a using function compiled by the DFG have...
authormsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 19 Dec 2014 18:48:25 +0000 (18:48 +0000)
committermsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 19 Dec 2014 18:48:25 +0000 (18:48 +0000)
https://bugs.webkit.org/show_bug.cgi?id=139808

Reviewed by Oliver Hunt.

Source/JavaScriptCore:

There are three changes here.
1) Create a VariableWatchpointSet for captured arguments variables.
2) Properly use the VariableWatchpointSet* found in op_put_to_scope in the 64 bit LLInt code.
3) Add the same putLocalClosureVar path to the 32 bit LLInt code that exists in the 64 bit version.

* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::BytecodeGenerator):
* llint/LowLevelInterpreter32_64.asm:
* llint/LowLevelInterpreter64.asm:

LayoutTests:

New regression test.

* js/regress-139808-expected.txt: Added.
* js/regress-139808.html: Added.
* js/script-tests/regress-139808.js: Added.
(theClosureFunction.rot13):
(theClosureFunction):

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

LayoutTests/ChangeLog
LayoutTests/js/regress-139808-expected.txt [new file with mode: 0644]
LayoutTests/js/regress-139808.html [new file with mode: 0644]
LayoutTests/js/script-tests/regress-139808.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm
Source/JavaScriptCore/llint/LowLevelInterpreter64.asm

index cc6162d..44aad24 100644 (file)
@@ -1,3 +1,18 @@
+2014-12-19  Michael Saboff  <msaboff@apple.com>
+
+        REGRESSION(174226): Captured arguments in a using function compiled by the DFG have the initial value when the closure was invoked
+        https://bugs.webkit.org/show_bug.cgi?id=139808
+
+        Reviewed by Oliver Hunt.
+
+        New regression test.
+
+        * js/regress-139808-expected.txt: Added.
+        * js/regress-139808.html: Added.
+        * js/script-tests/regress-139808.js: Added.
+        (theClosureFunction.rot13):
+        (theClosureFunction):
+
 2014-12-19  Alexey Proskuryakov  <ap@apple.com>
 
         Updte WebKit2 test expectations based on what bots see now.
diff --git a/LayoutTests/js/regress-139808-expected.txt b/LayoutTests/js/regress-139808-expected.txt
new file mode 100644 (file)
index 0000000..08f5ce2
--- /dev/null
@@ -0,0 +1,9 @@
+Regression test for https://webkit.org/b/139808. This test should run without any exceptions.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/js/regress-139808.html b/LayoutTests/js/regress-139808.html
new file mode 100644 (file)
index 0000000..7d33bbd
--- /dev/null
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../resources/js-test-pre.js"></script>
+</head>
+<body>
+<script src="script-tests/regress-139808.js"></script>
+<script src="../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/js/script-tests/regress-139808.js b/LayoutTests/js/script-tests/regress-139808.js
new file mode 100644 (file)
index 0000000..026b52c
--- /dev/null
@@ -0,0 +1,47 @@
+description(
+"Regression test for https://webkit.org/b/139808. This test should run without any exceptions."
+);
+
+function theClosureFunction(a)
+{
+    var o = {
+        1: "Gur dhvpx oebja sbk whzcrq bire gur ynml qbt\'f onpx.",
+        2: "Abj vf gur gvzr sbe nyy zra gb pbzr gb gur nvq bs gurve cnegl.",
+        3: "N zna n cyna n pnany, Cnanzn."
+    };
+
+    var expect = {
+        1: "The quick brown fox jumped over the lazy dog\'s back.",
+        2: "Now is the time for all men to come to the aid of their party.",
+        3: "A man a plan a canal, Panama."
+    };
+
+    e = expect[a]
+    a = o[a];
+
+    var rot13 = function(startIndex) {
+        result = "";
+
+        for (var i = startIndex; i < a.length; i++) {
+            c = a.charAt(i);
+            if (c >= 'a' && c <= 'z')
+                c = String.fromCharCode((a.charCodeAt(i) - 84) % 26 + 97);
+            else if (c >= 'A' && c <= 'Z')
+                c = String.fromCharCode((a.charCodeAt(i) - 52) % 26 + 65);
+
+           result += c;
+       }
+
+       return result;
+    }
+
+    // Call in a loop to tier up to DFG
+    for (var i = 0; i < 1000; i++)
+        s = rot13(0);
+
+    return s == e;
+}
+
+for (var i = 1; i <= 3; i++)
+    if (!theClosureFunction(i))
+        throw "Incorrect result calling theClosureFunction";
index bacb408..a875592 100644 (file)
@@ -1,3 +1,20 @@
+2014-12-19  Michael Saboff  <msaboff@apple.com>
+
+        REGRESSION(174226): Captured arguments in a using function compiled by the DFG have the initial value when the closure was invoked
+        https://bugs.webkit.org/show_bug.cgi?id=139808
+
+        Reviewed by Oliver Hunt.
+
+        There are three changes here.
+        1) Create a VariableWatchpointSet for captured arguments variables.
+        2) Properly use the VariableWatchpointSet* found in op_put_to_scope in the 64 bit LLInt code.
+        3) Add the same putLocalClosureVar path to the 32 bit LLInt code that exists in the 64 bit version.
+
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::BytecodeGenerator::BytecodeGenerator):
+        * llint/LowLevelInterpreter32_64.asm:
+        * llint/LowLevelInterpreter64.asm:
+
 2014-12-19  David Kilzer  <ddkilzer@apple.com>
 
         Switch from using PLATFORM_NAME to SDK selectors in WebCore, WebInspectorUI, WebKit, WebKit2
index 0b15658..a1ff17b 100644 (file)
@@ -316,7 +316,7 @@ BytecodeGenerator::BytecodeGenerator(VM& vm, FunctionNode* functionNode, Unlinke
             if (!functionNode->captures(ident) && !shouldCaptureAllTheThings)
                 continue;
             capturesAnyArgumentByName = true;
-            capturedArguments[i] = addVar();
+            capturedArguments[i] = addVar(ident, IsVariable, IsWatchable);
         }
     }
 
index 4632353..88ddef8 100644 (file)
@@ -2339,6 +2339,19 @@ macro putClosureVar()
     storei t3, PayloadOffset[t0, t1, 8]
 end
 
+macro putLocalClosureVar()
+    loadisFromInstruction(3, t1)
+    loadConstantOrVariable(t1, t2, t3)
+    loadpFromInstruction(5, t4)
+    btpz t4, .noVariableWatchpointSet
+    notifyWrite(t4, t2, t3, t1, .pDynamic)
+.noVariableWatchpointSet:
+    loadp JSEnvironmentRecord::m_registers[t0], t0
+    loadisFromInstruction(6, t1)
+    storei t2, TagOffset[t0, t1, 8]
+    storei t3, PayloadOffset[t0, t1, 8]
+end
+
 
 _llint_op_put_to_scope:
     traceExecution()
@@ -2349,7 +2362,7 @@ _llint_op_put_to_scope:
     bineq t0, LocalClosureVar, .pGlobalProperty
     writeBarrierOnOperands(1, 3)
     loadVariable(1, t2, t1, t0)
-    putClosureVar()
+    putLocalClosureVar()
     dispatch(7)
 
 .pGlobalProperty:
index 5db9dfa..ed641bc 100644 (file)
@@ -2165,7 +2165,10 @@ end
 macro putLocalClosureVar()
     loadisFromInstruction(3, t1)
     loadConstantOrVariable(t1, t2)
-    notifyWrite(t0, t2, t1, .pDynamic)
+    loadpFromInstruction(5, t3)
+    btpz t3, .noVariableWatchpointSet
+    notifyWrite(t3, t2, t1, .pDynamic)
+.noVariableWatchpointSet:
     loadp JSEnvironmentRecord::m_registers[t0], t0
     loadisFromInstruction(6, t1)
     storeq t2, [t0, t1, 8]