AccessCase::generateImpl() should exclude the result register when restoring register...
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 Nov 2017 19:33:22 +0000 (19:33 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 Nov 2017 19:33:22 +0000 (19:33 +0000)
https://bugs.webkit.org/show_bug.cgi?id=179355
<rdar://problem/35263053>

Reviewed by Saam Barati.

JSTests:

* stress/regress-179355.js: Added.

Source/JavaScriptCore:

In the Transition case in AccessCase::generateImpl(), we were restoring registers
using restoreLiveRegistersFromStackForCall() without excluding the scratchGPR
where we previously stashed the reallocated butterfly.  If the generated code is
under heavy register pressure, scratchGPR could have been from the set of preserved
registers, and hence, would be restored by restoreLiveRegistersFromStackForCall().
As a result, the restoration would trash the butterfly result we stored there.
This patch fixes the issue by excluding the scratchGPR in the restoration.

* bytecode/AccessCase.cpp:
(JSC::AccessCase::generateImpl):

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

JSTests/ChangeLog
JSTests/stress/regress-179355.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/AccessCase.cpp

index c921790..81ad95a 100644 (file)
@@ -1,3 +1,13 @@
+2017-11-07  Mark Lam  <mark.lam@apple.com>
+
+        AccessCase::generateImpl() should exclude the result register when restoring registers after a call.
+        https://bugs.webkit.org/show_bug.cgi?id=179355
+        <rdar://problem/35263053>
+
+        Reviewed by Saam Barati.
+
+        * stress/regress-179355.js: Added.
+
 2017-11-05  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         JIT call inline caches should cache calls to objects with getCallData/getConstructData traps
diff --git a/JSTests/stress/regress-179355.js b/JSTests/stress/regress-179355.js
new file mode 100644 (file)
index 0000000..bde91a0
--- /dev/null
@@ -0,0 +1,25 @@
+var arr0 = [1,2,3,4];
+var arr1 = new Array(1000);
+
+Array.prototype.__defineGetter__(1, function() {
+    [].concat(arr1); //generate to invalid JIT code here?
+});
+
+Array.prototype.__defineGetter__(Symbol.isConcatSpreadable, (function() {
+    for(var i=0;i<10000;i++) {
+        if(i==0)
+            arr1[i];
+        this.x = 1.1;
+        arr1.legnth = 1;
+    }
+}));
+
+var exception;
+try {
+    arr1[1].toString();
+} catch (e) {
+    exception = e;
+}
+
+if (exception != "RangeError: Maximum call stack size exceeded.")
+    throw "FAILED";
index 3a35f6e..ebd3d21 100644 (file)
@@ -1,3 +1,22 @@
+2017-11-07  Mark Lam  <mark.lam@apple.com>
+
+        AccessCase::generateImpl() should exclude the result register when restoring registers after a call.
+        https://bugs.webkit.org/show_bug.cgi?id=179355
+        <rdar://problem/35263053>
+
+        Reviewed by Saam Barati.
+
+        In the Transition case in AccessCase::generateImpl(), we were restoring registers
+        using restoreLiveRegistersFromStackForCall() without excluding the scratchGPR
+        where we previously stashed the reallocated butterfly.  If the generated code is
+        under heavy register pressure, scratchGPR could have been from the set of preserved
+        registers, and hence, would be restored by restoreLiveRegistersFromStackForCall().
+        As a result, the restoration would trash the butterfly result we stored there.
+        This patch fixes the issue by excluding the scratchGPR in the restoration.
+
+        * bytecode/AccessCase.cpp:
+        (JSC::AccessCase::generateImpl):
+
 2017-11-06  Robin Morisset  <rmorisset@apple.com>
 
         CodeBlock::usesOpcode() is dead code
index ca3ecf2..67f965b 100644 (file)
@@ -1042,7 +1042,9 @@ void AccessCase::generateImpl(AccessGenerationState& state)
                 state.emitExplicitExceptionHandler();
                 
                 noException.link(&jit);
-                state.restoreLiveRegistersFromStackForCall(spillState);
+                RegisterSet resultRegisterToExclude;
+                resultRegisterToExclude.set(scratchGPR);
+                state.restoreLiveRegistersFromStackForCall(spillState, resultRegisterToExclude);
             }
         }