Title: [189325] trunk/Source/_javascript_Core
Revision
189325
Author
[email protected]
Date
2015-09-03 17:25:19 -0700 (Thu, 03 Sep 2015)

Log Message

_javascript_ functions should restore the stack pointer after a call
https://bugs.webkit.org/show_bug.cgi?id=148659

Reviewed by Michael Saboff.

This patch makes it so that the various places where we are making a
JS-to-JS call restore the stack pointer afterwards. This allows us to
no longer rely on the stack pointer still being valid after a call, and
is a prerequisite for getting rid of the arity fixup return thunk.

* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::emitCall):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::emitCall):
* ftl/FTLCompile.cpp:
(JSC::FTL::mmAllocateDataSection):
* ftl/FTLInlineCacheSize.cpp:
(JSC::FTL::sizeOfCall):
* ftl/FTLJSCall.cpp:
(JSC::FTL::JSCall::emit):
* ftl/FTLJSCall.h:
* ftl/FTLStackMaps.h:
(JSC::FTL::StackMaps::stackSizeForLocals):
* jit/Repatch.cpp:
(JSC::generateByIdStub):
* tests/stress/tail-call-in-inline-cache.js: Added.
(tail):
(obj.get x):

Modified Paths

Added Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (189324 => 189325)


--- trunk/Source/_javascript_Core/ChangeLog	2015-09-04 00:09:00 UTC (rev 189324)
+++ trunk/Source/_javascript_Core/ChangeLog	2015-09-04 00:25:19 UTC (rev 189325)
@@ -1,3 +1,34 @@
+2015-09-03  Basile Clement  <[email protected]>
+
+        _javascript_ functions should restore the stack pointer after a call
+        https://bugs.webkit.org/show_bug.cgi?id=148659
+
+        Reviewed by Michael Saboff.
+
+        This patch makes it so that the various places where we are making a
+        JS-to-JS call restore the stack pointer afterwards. This allows us to
+        no longer rely on the stack pointer still being valid after a call, and
+        is a prerequisite for getting rid of the arity fixup return thunk.
+
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::emitCall):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::emitCall):
+        * ftl/FTLCompile.cpp:
+        (JSC::FTL::mmAllocateDataSection):
+        * ftl/FTLInlineCacheSize.cpp:
+        (JSC::FTL::sizeOfCall):
+        * ftl/FTLJSCall.cpp:
+        (JSC::FTL::JSCall::emit):
+        * ftl/FTLJSCall.h:
+        * ftl/FTLStackMaps.h:
+        (JSC::FTL::StackMaps::stackSizeForLocals):
+        * jit/Repatch.cpp:
+        (JSC::generateByIdStub):
+        * tests/stress/tail-call-in-inline-cache.js: Added.
+        (tail):
+        (obj.get x):
+
 2015-09-03  Filip Pizlo  <[email protected]>
 
         StructureStubInfo should be able to reset itself without going through CodeBlock

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (189324 => 189325)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp	2015-09-04 00:09:00 UTC (rev 189324)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp	2015-09-04 00:25:19 UTC (rev 189325)
@@ -821,9 +821,10 @@
     info->setUpCall(callType, node->origin.semantic, calleePayloadGPR);
     m_jit.addJSCall(fastCall, slowCall, targetToCheck, info);
     
-    // If we were varargs, then after the calls are done, we need to reestablish our stack pointer.
-    if (isVarargs || isForwardVarargs)
-        m_jit.addPtr(TrustedImm32(m_jit.graph().stackPointerOffset() * sizeof(Register)), GPRInfo::callFrameRegister, JITCompiler::stackPointerRegister);
+    // After the calls are done, we need to reestablish our stack
+    // pointer. We rely on this for varargs calls, calls with arity
+    // mismatch (the callframe is slided) and tail calls.
+    m_jit.addPtr(TrustedImm32(m_jit.graph().stackPointerOffset() * sizeof(Register)), GPRInfo::callFrameRegister, JITCompiler::stackPointerRegister);
 }
 
 template<bool strict>

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (189324 => 189325)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp	2015-09-04 00:09:00 UTC (rev 189324)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp	2015-09-04 00:25:19 UTC (rev 189325)
@@ -776,10 +776,11 @@
     
     callLinkInfo->setUpCall(callType, m_currentNode->origin.semantic,  calleeGPR);    
     m_jit.addJSCall(fastCall, slowCall, targetToCheck, callLinkInfo);
-    
-    // If we were varargs, then after the calls are done, we need to reestablish our stack pointer.
-    if (isVarargs || isForwardVarargs)
-        m_jit.addPtr(TrustedImm32(m_jit.graph().stackPointerOffset() * sizeof(Register)), GPRInfo::callFrameRegister, JITCompiler::stackPointerRegister);
+
+    // After the calls are done, we need to reestablish our stack
+    // pointer. We rely on this for varargs calls, calls with arity
+    // mismatch (the callframe is slided) and tail calls.
+    m_jit.addPtr(TrustedImm32(m_jit.graph().stackPointerOffset() * sizeof(Register)), GPRInfo::callFrameRegister, JITCompiler::stackPointerRegister);
 }
 
 // Clang should allow unreachable [[clang::fallthrough]] in template functions if any template expansion uses it

Modified: trunk/Source/_javascript_Core/ftl/FTLCompile.cpp (189324 => 189325)


--- trunk/Source/_javascript_Core/ftl/FTLCompile.cpp	2015-09-04 00:09:00 UTC (rev 189324)
+++ trunk/Source/_javascript_Core/ftl/FTLCompile.cpp	2015-09-04 00:25:19 UTC (rev 189325)
@@ -593,7 +593,7 @@
         JSCall& call = state.jsCalls[i];
 
         CCallHelpers fastPathJIT(&vm, codeBlock);
-        call.emit(fastPathJIT);
+        call.emit(fastPathJIT, state.jitCode->stackmaps.stackSizeForLocals());
 
         char* startOfIC = bitwise_cast<char*>(generatedFunction) + call.m_instructionOffset;
 

Modified: trunk/Source/_javascript_Core/ftl/FTLInlineCacheSize.cpp (189324 => 189325)


--- trunk/Source/_javascript_Core/ftl/FTLInlineCacheSize.cpp	2015-09-04 00:09:00 UTC (rev 189324)
+++ trunk/Source/_javascript_Core/ftl/FTLInlineCacheSize.cpp	2015-09-04 00:25:19 UTC (rev 189325)
@@ -67,9 +67,9 @@
 size_t sizeOfCall()
 {
 #if CPU(ARM64)
-    return 56;
+    return 60;
 #else
-    return 53;
+    return 60;
 #endif
 }
 

Modified: trunk/Source/_javascript_Core/ftl/FTLJSCall.cpp (189324 => 189325)


--- trunk/Source/_javascript_Core/ftl/FTLJSCall.cpp	2015-09-04 00:09:00 UTC (rev 189324)
+++ trunk/Source/_javascript_Core/ftl/FTLJSCall.cpp	2015-09-04 00:25:19 UTC (rev 189325)
@@ -51,6 +51,13 @@
     ASSERT(node->op() == Call || node->op() == Construct);
 }
 
+void JSCall::emit(CCallHelpers& jit, unsigned stackSizeForLocals)
+{
+    JSCallBase::emit(jit);
+
+    jit.addPtr(CCallHelpers::TrustedImm32(- static_cast<int64_t>(stackSizeForLocals)), CCallHelpers::framePointerRegister, CCallHelpers::stackPointerRegister);
+}
+
 } } // namespace JSC::FTL
 
 #endif // ENABLE(FTL_JIT)

Modified: trunk/Source/_javascript_Core/ftl/FTLJSCall.h (189324 => 189325)


--- trunk/Source/_javascript_Core/ftl/FTLJSCall.h	2015-09-04 00:09:00 UTC (rev 189324)
+++ trunk/Source/_javascript_Core/ftl/FTLJSCall.h	2015-09-04 00:25:19 UTC (rev 189325)
@@ -42,6 +42,8 @@
 public:
     JSCall();
     JSCall(unsigned stackmapID, DFG::Node*);
+
+    void emit(CCallHelpers&, unsigned stackSizeForLocals);
     
     unsigned stackmapID() const { return m_stackmapID; }
     

Modified: trunk/Source/_javascript_Core/ftl/FTLStackMaps.h (189324 => 189325)


--- trunk/Source/_javascript_Core/ftl/FTLStackMaps.h	2015-09-04 00:09:00 UTC (rev 189324)
+++ trunk/Source/_javascript_Core/ftl/FTLStackMaps.h	2015-09-04 00:25:19 UTC (rev 189325)
@@ -127,6 +127,19 @@
     RecordMap computeRecordMap() const;
 
     unsigned stackSize() const;
+
+    unsigned stackSizeForLocals() const
+    {
+#if CPU(X86_64)
+        // LLVM will store fp in the call frame
+        return stackSize() - sizeof(void*);
+#elif CPU(ARM64)
+        // LLVM will store fp & lr in the call frame
+        return stackSize() - 2 * sizeof(void*);
+#else
+        UNREACHABLE_FOR_PLATFORM();
+#endif
+    }
 };
 
 } } // namespace JSC::FTL

Modified: trunk/Source/_javascript_Core/jit/Repatch.cpp (189324 => 189325)


--- trunk/Source/_javascript_Core/jit/Repatch.cpp	2015-09-04 00:09:00 UTC (rev 189324)
+++ trunk/Source/_javascript_Core/jit/Repatch.cpp	2015-09-04 00:25:19 UTC (rev 189325)
@@ -492,9 +492,10 @@
                 MacroAssembler::TrustedImmPtr(0));
             
             fastPathCall = stubJit.nearCall();
-            
+
             stubJit.addPtr(
-                MacroAssembler::TrustedImm32(alignedNumberOfBytesForCall),
+                MacroAssembler::TrustedImm32(codeBlock->stackPointerOffset() * sizeof(Register)),
+                GPRInfo::callFrameRegister,
                 MacroAssembler::stackPointerRegister);
             if (kind == CallGetter)
                 stubJit.setupResults(valueRegs);
@@ -510,7 +511,8 @@
             slowPathCall = stubJit.nearCall();
             
             stubJit.addPtr(
-                MacroAssembler::TrustedImm32(alignedNumberOfBytesForCall),
+                MacroAssembler::TrustedImm32(codeBlock->stackPointerOffset() * sizeof(Register)),
+                GPRInfo::callFrameRegister,
                 MacroAssembler::stackPointerRegister);
             if (kind == CallGetter)
                 stubJit.setupResults(valueRegs);

Added: trunk/Source/_javascript_Core/tests/stress/tail-call-in-inline-cache.js (0 => 189325)


--- trunk/Source/_javascript_Core/tests/stress/tail-call-in-inline-cache.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/tail-call-in-inline-cache.js	2015-09-04 00:25:19 UTC (rev 189325)
@@ -0,0 +1,10 @@
+"use strict";
+
+function tail() { }
+
+var obj = {
+    get x() { return tail(0); }
+};
+
+for (var i = 0; i < 10; ++i)
+    obj.x;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to