From 035fd11885ebd3c51fa0ecb72bb351f486b13b2e Mon Sep 17 00:00:00 2001 From: Mark Lam Date: Wed, 24 Jan 2018 09:37:09 +0000 Subject: [PATCH] Merge r224539 - AccessCase::generateImpl() should exclude the result register when restoring registers after a call. https://bugs.webkit.org/show_bug.cgi?id=179355 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): --- JSTests/ChangeLog | 10 ++++++++ JSTests/stress/regress-179355.js | 25 +++++++++++++++++++ Source/JavaScriptCore/ChangeLog | 19 ++++++++++++++ Source/JavaScriptCore/bytecode/AccessCase.cpp | 4 ++- 4 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 JSTests/stress/regress-179355.js diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog index e9eccb25b9db..f4666450cf11 100644 --- a/JSTests/ChangeLog +++ b/JSTests/ChangeLog @@ -1,3 +1,13 @@ +2017-11-07 Mark Lam + + AccessCase::generateImpl() should exclude the result register when restoring registers after a call. + https://bugs.webkit.org/show_bug.cgi?id=179355 + + + Reviewed by Saam Barati. + + * stress/regress-179355.js: Added. + 2017-11-02 Filip Pizlo AI does not correctly model the clobber case of ArithClz32 diff --git a/JSTests/stress/regress-179355.js b/JSTests/stress/regress-179355.js new file mode 100644 index 000000000000..bde91a030d70 --- /dev/null +++ b/JSTests/stress/regress-179355.js @@ -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"; diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog index ca3bcc8b94e0..33e86746aa53 100644 --- a/Source/JavaScriptCore/ChangeLog +++ b/Source/JavaScriptCore/ChangeLog @@ -1,3 +1,22 @@ +2017-11-07 Mark Lam + + AccessCase::generateImpl() should exclude the result register when restoring registers after a call. + https://bugs.webkit.org/show_bug.cgi?id=179355 + + + 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-03 Michael Saboff The Abstract Interpreter needs to change similar to clobberize() in r224366 diff --git a/Source/JavaScriptCore/bytecode/AccessCase.cpp b/Source/JavaScriptCore/bytecode/AccessCase.cpp index 35ffbf1abab8..4e84b2565e96 100644 --- a/Source/JavaScriptCore/bytecode/AccessCase.cpp +++ b/Source/JavaScriptCore/bytecode/AccessCase.cpp @@ -945,7 +945,9 @@ void AccessCase::generateImpl(AccessGenerationState& state) state.emitExplicitExceptionHandler(); noException.link(&jit); - state.restoreLiveRegistersFromStackForCall(spillState); + RegisterSet resultRegisterToExclude; + resultRegisterToExclude.set(scratchGPR); + state.restoreLiveRegistersFromStackForCall(spillState, resultRegisterToExclude); } }