Title: [173282] trunk/Source/_javascript_Core
Revision
173282
Author
msab...@apple.com
Date
2014-09-04 14:23:38 -0700 (Thu, 04 Sep 2014)

Log Message

REGRESSION(r173031): crashes during run-layout-jsc on x86/Linux
https://bugs.webkit.org/show_bug.cgi?id=136436

Reviewed by Geoffrey Garen.

Instead of trying to calculate a stack pointer that allows for possible
stacked argument space, just use the "home" stack pointer location.
That stack pointer provides space for the worst case number of stacked
arguments on architectures that use stacked arguments.  It also provides
stack space so that the return PC and caller frame pointer that are stored
as part of making the call to operationCallEval will not override any part
of the callee frame created on the stack.

Changed compileCallEval() to use the stackPointer value of the calling
function.  That stack pointer is calculated to have enough space for
outgoing stacked arguments.  By moving the stack pointer to its "home"
position, the caller frame and return PC are not set as part of making
the call to operationCallEval().  Moved the explicit setting of the
callerFrame field of the callee CallFrame from operationCallEval() to
compileCallEval() since it has been the artifact of making a call for
most architectures.  Simplified the exception logic in compileCallEval()
as a result of the change.  To be compliant with the stack state
expected by virtualCallThunkGenerator(), moved the stack pointer to
point above the CallerFrameAndPC of the callee CallFrame.

* jit/JIT.h: Changed callOperationNoExceptionCheck(J_JITOperation_EE, ...)
to callOperation(J_JITOperation_EE, ...) as it now can do a typical exception
check.
* jit/JITCall.cpp & jit/JITCall32_64.cpp:
(JSC::JIT::compileCallEval): Use the home stack pointer when making the call
to operationCallEval.  Since the stack pointer adjustment no longer needs
to be done after making the call to operationCallEval(), the exception check
logic can be simplified.
(JSC::JIT::compileCallEvalSlowCase): Restored the stack pointer to point
to above the calleeFrame as this is what the generated thunk expects.
* jit/JITInlines.h:
(JSC::JIT::callOperation): Refactor of callOperationNoExceptionCheck
with the addition of a standard exception check.
(JSC::JIT::callOperationNoExceptionCheck): Deleted.
* jit/JITOperations.cpp:
(JSC::operationCallEval): Eliminated the explicit setting of caller frame
as that is now done in the code generated by compileCallEval().

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (173281 => 173282)


--- trunk/Source/_javascript_Core/ChangeLog	2014-09-04 21:20:12 UTC (rev 173281)
+++ trunk/Source/_javascript_Core/ChangeLog	2014-09-04 21:23:38 UTC (rev 173282)
@@ -1,3 +1,48 @@
+2014-09-04  Michael Saboff  <msab...@apple.com>
+
+        REGRESSION(r173031): crashes during run-layout-jsc on x86/Linux
+        https://bugs.webkit.org/show_bug.cgi?id=136436
+
+        Reviewed by Geoffrey Garen.
+
+        Instead of trying to calculate a stack pointer that allows for possible
+        stacked argument space, just use the "home" stack pointer location.
+        That stack pointer provides space for the worst case number of stacked
+        arguments on architectures that use stacked arguments.  It also provides
+        stack space so that the return PC and caller frame pointer that are stored
+        as part of making the call to operationCallEval will not override any part
+        of the callee frame created on the stack.
+
+        Changed compileCallEval() to use the stackPointer value of the calling
+        function.  That stack pointer is calculated to have enough space for
+        outgoing stacked arguments.  By moving the stack pointer to its "home"
+        position, the caller frame and return PC are not set as part of making
+        the call to operationCallEval().  Moved the explicit setting of the
+        callerFrame field of the callee CallFrame from operationCallEval() to
+        compileCallEval() since it has been the artifact of making a call for
+        most architectures.  Simplified the exception logic in compileCallEval()
+        as a result of the change.  To be compliant with the stack state
+        expected by virtualCallThunkGenerator(), moved the stack pointer to
+        point above the CallerFrameAndPC of the callee CallFrame.
+
+        * jit/JIT.h: Changed callOperationNoExceptionCheck(J_JITOperation_EE, ...)
+        to callOperation(J_JITOperation_EE, ...) as it now can do a typical exception
+        check.
+        * jit/JITCall.cpp & jit/JITCall32_64.cpp:
+        (JSC::JIT::compileCallEval): Use the home stack pointer when making the call
+        to operationCallEval.  Since the stack pointer adjustment no longer needs
+        to be done after making the call to operationCallEval(), the exception check
+        logic can be simplified.
+        (JSC::JIT::compileCallEvalSlowCase): Restored the stack pointer to point
+        to above the calleeFrame as this is what the generated thunk expects.
+        * jit/JITInlines.h:
+        (JSC::JIT::callOperation): Refactor of callOperationNoExceptionCheck
+        with the addition of a standard exception check.
+        (JSC::JIT::callOperationNoExceptionCheck): Deleted.
+        * jit/JITOperations.cpp:
+        (JSC::operationCallEval): Eliminated the explicit setting of caller frame
+        as that is now done in the code generated by compileCallEval().
+
 2014-09-03  Filip Pizlo  <fpi...@apple.com>
 
         Beef up the DFG's CFG analyses to include iterated dominance frontiers and more user-friendly BlockSets

Modified: trunk/Source/_javascript_Core/jit/JIT.h (173281 => 173282)


--- trunk/Source/_javascript_Core/jit/JIT.h	2014-09-04 21:20:12 UTC (rev 173281)
+++ trunk/Source/_javascript_Core/jit/JIT.h	2014-09-04 21:23:38 UTC (rev 173282)
@@ -718,6 +718,7 @@
         MacroAssembler::Call callOperation(V_JITOperation_EC, RegisterID);
         MacroAssembler::Call callOperation(V_JITOperation_ECC, RegisterID, RegisterID);
         MacroAssembler::Call callOperation(V_JITOperation_ECICC, RegisterID, const Identifier*, RegisterID, RegisterID);
+        MacroAssembler::Call callOperation(J_JITOperation_EE, RegisterID);
         MacroAssembler::Call callOperation(V_JITOperation_EIdJZ, const Identifier*, RegisterID, int32_t);
         MacroAssembler::Call callOperation(V_JITOperation_EJ, RegisterID);
 #if USE(JSVALUE64)
@@ -738,7 +739,6 @@
         MacroAssembler::Call callOperation(V_JITOperation_EPc, Instruction*);
         MacroAssembler::Call callOperation(V_JITOperation_EZ, int32_t);
         MacroAssembler::Call callOperationWithCallFrameRollbackOnException(J_JITOperation_E);
-        MacroAssembler::Call callOperationNoExceptionCheck(J_JITOperation_EE, RegisterID);
         MacroAssembler::Call callOperationWithCallFrameRollbackOnException(V_JITOperation_ECb, CodeBlock*);
         MacroAssembler::Call callOperationWithCallFrameRollbackOnException(Z_JITOperation_E);
 #if USE(JSVALUE32_64)

Modified: trunk/Source/_javascript_Core/jit/JITCall.cpp (173281 => 173282)


--- trunk/Source/_javascript_Core/jit/JITCall.cpp	2014-09-04 21:20:12 UTC (rev 173281)
+++ trunk/Source/_javascript_Core/jit/JITCall.cpp	2014-09-04 21:23:38 UTC (rev 173282)
@@ -136,18 +136,15 @@
 void JIT::compileCallEval(Instruction* instruction)
 {
     addPtr(TrustedImm32(-static_cast<ptrdiff_t>(sizeof(CallerFrameAndPC))), stackPointerRegister, regT1);
-    callOperationNoExceptionCheck(operationCallEval, regT1);
+    storePtr(callFrameRegister, Address(regT1, CallFrame::callerFrameOffset()));
 
-    Jump noException = emitExceptionCheck(InvertedExceptionCheck);
-    addPtr(TrustedImm32(stackPointerOffsetFor(m_codeBlock) * sizeof(Register)), callFrameRegister, stackPointerRegister);    
-    exceptionCheck(jump());
+    addPtr(TrustedImm32(stackPointerOffsetFor(m_codeBlock) * sizeof(Register)), callFrameRegister, stackPointerRegister);
+    checkStackPointerAlignment();
 
-    noException.link(this);
+    callOperation(operationCallEval, regT1);
+
     addSlowCase(branch64(Equal, regT0, TrustedImm64(JSValue::encode(JSValue()))));
 
-    addPtr(TrustedImm32(stackPointerOffsetFor(m_codeBlock) * sizeof(Register)), callFrameRegister, stackPointerRegister);
-    checkStackPointerAlignment();
-
     sampleCodeBlock(m_codeBlock);
     
     emitPutCallResult(instruction);
@@ -156,7 +153,10 @@
 void JIT::compileCallEvalSlowCase(Instruction* instruction, Vector<SlowCaseEntry>::iterator& iter)
 {
     linkSlowCase(iter);
+    int registerOffset = -instruction[4].u.operand;
 
+    addPtr(TrustedImm32(registerOffset * sizeof(Register) + sizeof(CallerFrameAndPC)), callFrameRegister, stackPointerRegister);
+
     load64(Address(stackPointerRegister, sizeof(Register) * JSStack::Callee - sizeof(CallerFrameAndPC)), regT0);
     move(TrustedImmPtr(&CallLinkInfo::dummy()), regT2);
     emitNakedCall(m_vm->getCTIStub(virtualCallThunkGenerator).code());

Modified: trunk/Source/_javascript_Core/jit/JITCall32_64.cpp (173281 => 173282)


--- trunk/Source/_javascript_Core/jit/JITCall32_64.cpp	2014-09-04 21:20:12 UTC (rev 173281)
+++ trunk/Source/_javascript_Core/jit/JITCall32_64.cpp	2014-09-04 21:23:38 UTC (rev 173282)
@@ -220,19 +220,14 @@
 void JIT::compileCallEval(Instruction* instruction)
 {
     addPtr(TrustedImm32(-static_cast<ptrdiff_t>(sizeof(CallerFrameAndPC))), stackPointerRegister, regT1);
+    storePtr(callFrameRegister, Address(regT1, CallFrame::callerFrameOffset()));
 
-    callOperationNoExceptionCheck(operationCallEval, regT1);
-
-    Jump noException = emitExceptionCheck(InvertedExceptionCheck);
     addPtr(TrustedImm32(stackPointerOffsetFor(m_codeBlock) * sizeof(Register)), callFrameRegister, stackPointerRegister);
-    exceptionCheck(jump());
 
-    noException.link(this);
+    callOperation(operationCallEval, regT1);
+
     addSlowCase(branch32(Equal, regT1, TrustedImm32(JSValue::EmptyValueTag)));
 
-    addPtr(TrustedImm32(stackPointerOffsetFor(m_codeBlock) * sizeof(Register)), callFrameRegister, stackPointerRegister);
-    checkStackPointerAlignment();
-
     sampleCodeBlock(m_codeBlock);
     
     emitPutCallResult(instruction);
@@ -242,6 +237,10 @@
 {
     linkSlowCase(iter);
 
+    int registerOffset = -instruction[4].u.operand;
+
+    addPtr(TrustedImm32(registerOffset * sizeof(Register) + sizeof(CallerFrameAndPC)), callFrameRegister, stackPointerRegister);
+
     loadPtr(Address(stackPointerRegister, sizeof(Register) * JSStack::Callee - sizeof(CallerFrameAndPC)), regT0);
     loadPtr(Address(stackPointerRegister, sizeof(Register) * JSStack::Callee - sizeof(CallerFrameAndPC)), regT1);
     move(TrustedImmPtr(&CallLinkInfo::dummy()), regT2);

Modified: trunk/Source/_javascript_Core/jit/JITInlines.h (173281 => 173282)


--- trunk/Source/_javascript_Core/jit/JITInlines.h	2014-09-04 21:20:12 UTC (rev 173281)
+++ trunk/Source/_javascript_Core/jit/JITInlines.h	2014-09-04 21:23:38 UTC (rev 173282)
@@ -317,6 +317,13 @@
     return appendCallWithExceptionCheck(operation);
 }
 
+ALWAYS_INLINE MacroAssembler::Call JIT::callOperation(J_JITOperation_EE operation, RegisterID regOp)
+{
+    setupArgumentsWithExecState(regOp);
+    updateTopCallFrame();
+    return appendCallWithExceptionCheck(operation);
+}
+
 ALWAYS_INLINE MacroAssembler::Call JIT::callOperation(V_JITOperation_EPc operation, Instruction* bytecodePC)
 {
     setupArgumentsWithExecState(TrustedImmPtr(bytecodePC));
@@ -335,13 +342,6 @@
     return appendCallWithCallFrameRollbackOnException(operation);
 }
 
-ALWAYS_INLINE MacroAssembler::Call JIT::callOperationNoExceptionCheck(J_JITOperation_EE operation, RegisterID regOp)
-{
-    setupArgumentsWithExecState(regOp);
-    updateTopCallFrame();
-    return appendCall(operation);
-}
-
 ALWAYS_INLINE MacroAssembler::Call JIT::callOperationWithCallFrameRollbackOnException(V_JITOperation_ECb operation, CodeBlock* pointer)
 {
     setupArgumentsWithExecState(TrustedImmPtr(pointer));

Modified: trunk/Source/_javascript_Core/jit/JITOperations.cpp (173281 => 173282)


--- trunk/Source/_javascript_Core/jit/JITOperations.cpp	2014-09-04 21:20:12 UTC (rev 173281)
+++ trunk/Source/_javascript_Core/jit/JITOperations.cpp	2014-09-04 21:23:38 UTC (rev 173282)
@@ -612,7 +612,6 @@
 
     execCallee->setScope(exec->scope());
     execCallee->setCodeBlock(0);
-    execCallee->setCallerFrame(exec);
 
     if (!isHostFunction(execCallee->calleeAsValue(), globalFuncEval))
         return JSValue::encode(JSValue());
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to