Title: [160656] branches/jsCStack/Source/_javascript_Core
Revision
160656
Author
[email protected]
Date
2013-12-16 12:46:39 -0800 (Mon, 16 Dec 2013)

Log Message

Fix exception handling for the baseline JIT.
https://bugs.webkit.org/show_bug.cgi?id=125736.

Reviewed by Geoffrey Garen.

* interpreter/Interpreter.cpp:
(JSC::unwindCallFrame):
Removed some unneeded code.
1. Removed use of the oldCodeBlock variable which is only a copy of
   the codeBlock variable. There is no longer any reason to use a copy.
   Just use codeBlock directly instead.
2. There's no need to set VM::topCallFrame. The UnwindFunctor
   automatically updates the incoming callFrame pointer reference to
   the frame that should be catching / handling the exception, and that
   is adequate for what we need.

* jit/JITOpcodes.cpp:
(JSC::JIT::emit_op_catch):
- Restored sp.

* jit/JITOpcodes32_64.cpp:
(JSC::JIT::privateCompileCTINativeCall):
(JSC::JIT::emit_op_catch):
* jit/ThunkGenerators.cpp:
(JSC::nativeForGenerator):
Fixed 2 issues:
1. After returning from a native / host function, the thunk did not pop
   the native / host frame before executing exception handling code.
   This resulted in the VM trying to "unwind" the native / host frame
   which is not possible, and crashes ensue.
2. After returning from a native / host function and discovering the
   need to handle an exception, the thunk was wrongly popping a
   non-existant return address off the stack. This caused the
   callerFrame pointer of the current frame to be popped off the stack,
   and havoc ensues.

* llint/LowLevelInterpreter32_64.asm:
- Updated to match exception handling code in LowLevelInterpreter64.asm.

* runtime/VM.h:
- Removed dead code.

Modified Paths

Diff

Modified: branches/jsCStack/Source/_javascript_Core/ChangeLog (160655 => 160656)


--- branches/jsCStack/Source/_javascript_Core/ChangeLog	2013-12-16 20:44:01 UTC (rev 160655)
+++ branches/jsCStack/Source/_javascript_Core/ChangeLog	2013-12-16 20:46:39 UTC (rev 160656)
@@ -1,3 +1,47 @@
+2013-12-14  Mark Lam  <[email protected]>
+
+        Fix exception handling for the baseline JIT.
+        https://bugs.webkit.org/show_bug.cgi?id=125736.
+
+        Reviewed by Geoffrey Garen.
+
+        * interpreter/Interpreter.cpp:
+        (JSC::unwindCallFrame):
+        Removed some unneeded code.
+        1. Removed use of the oldCodeBlock variable which is only a copy of
+           the codeBlock variable. There is no longer any reason to use a copy.
+           Just use codeBlock directly instead.
+        2. There's no need to set VM::topCallFrame. The UnwindFunctor
+           automatically updates the incoming callFrame pointer reference to
+           the frame that should be catching / handling the exception, and that
+           is adequate for what we need.
+
+        * jit/JITOpcodes.cpp:
+        (JSC::JIT::emit_op_catch):
+        - Restored sp.
+
+        * jit/JITOpcodes32_64.cpp:
+        (JSC::JIT::privateCompileCTINativeCall):
+        (JSC::JIT::emit_op_catch):
+        * jit/ThunkGenerators.cpp:
+        (JSC::nativeForGenerator):
+        Fixed 2 issues:
+        1. After returning from a native / host function, the thunk did not pop
+           the native / host frame before executing exception handling code.
+           This resulted in the VM trying to "unwind" the native / host frame
+           which is not possible, and crashes ensue.
+        2. After returning from a native / host function and discovering the
+           need to handle an exception, the thunk was wrongly popping a
+           non-existant return address off the stack. This caused the
+           callerFrame pointer of the current frame to be popped off the stack,
+           and havoc ensues.
+
+        * llint/LowLevelInterpreter32_64.asm:
+        - Updated to match exception handling code in LowLevelInterpreter64.asm.
+
+        * runtime/VM.h:
+        - Removed dead code.
+
 2013-12-14  Filip Pizlo  <[email protected]>
 
         FTL should *really* know when things are flushed

Modified: branches/jsCStack/Source/_javascript_Core/interpreter/Interpreter.cpp (160655 => 160656)


--- branches/jsCStack/Source/_javascript_Core/interpreter/Interpreter.cpp	2013-12-16 20:44:01 UTC (rev 160655)
+++ branches/jsCStack/Source/_javascript_Core/interpreter/Interpreter.cpp	2013-12-16 20:46:39 UTC (rev 160656)
@@ -421,7 +421,6 @@
 {
     CallFrame* callFrame = visitor->callFrame();
     CodeBlock* codeBlock = visitor->codeBlock();
-    CodeBlock* oldCodeBlock = codeBlock;
     JSScope* scope = callFrame->scope();
 
     if (Debugger* debugger = callFrame->vmEntryGlobalObject()->debugger()) {
@@ -432,16 +431,16 @@
     }
 
     JSValue activation;
-    if (oldCodeBlock->codeType() == FunctionCode && oldCodeBlock->needsActivation()) {
+    if (codeBlock->codeType() == FunctionCode && codeBlock->needsActivation()) {
 #if ENABLE(DFG_JIT)
         RELEASE_ASSERT(!visitor->isInlinedFrame());
 #endif
-        activation = callFrame->uncheckedR(oldCodeBlock->activationRegister().offset()).jsValue();
+        activation = callFrame->uncheckedR(codeBlock->activationRegister().offset()).jsValue();
         if (activation)
             jsCast<JSActivation*>(activation)->tearOff(*scope->vm());
     }
 
-    if (oldCodeBlock->codeType() == FunctionCode && oldCodeBlock->usesArguments()) {
+    if (codeBlock->codeType() == FunctionCode && codeBlock->usesArguments()) {
         if (Arguments* arguments = visitor->existingArguments()) {
             if (activation)
                 arguments->didTearOffActivation(callFrame, jsCast<JSActivation*>(activation));
@@ -455,14 +454,7 @@
     }
 
     CallFrame* callerFrame = callFrame->callerFrame();
-    if (callerFrame->isVMEntrySentinel()) {
-        if (callerFrame->vmEntrySentinelCallerFrame())
-            callFrame->vm().topCallFrame = callerFrame->vmEntrySentinelCallerFrame();
-        else
-            callFrame->vm().topCallFrame = callFrame; // _handleUncaughtException will pop the frame off.
-        return false;
-    }
-    return true;
+    return !callerFrame->isVMEntrySentinel();
 }
 
 static StackFrameCodeType getStackFrameCodeType(StackVisitor& visitor)

Modified: branches/jsCStack/Source/_javascript_Core/jit/JITOpcodes.cpp (160655 => 160656)


--- branches/jsCStack/Source/_javascript_Core/jit/JITOpcodes.cpp	2013-12-16 20:44:01 UTC (rev 160655)
+++ branches/jsCStack/Source/_javascript_Core/jit/JITOpcodes.cpp	2013-12-16 20:46:39 UTC (rev 160656)
@@ -38,6 +38,7 @@
 #include "JSFunction.h"
 #include "JSPropertyNameIterator.h"
 #include "LinkBuffer.h"
+#include "MaxFrameExtentForSlowPathCall.h"
 #include "SlowPathCall.h"
 #include "VirtualRegister.h"
 
@@ -635,6 +636,11 @@
 {
     move(TrustedImmPtr(m_vm), regT3);
     load64(Address(regT3, VM::callFrameForThrowOffset()), callFrameRegister);
+
+    size_t frameExtent = JIT::frameRegisterCountFor(codeBlock()) * sizeof(Register) + maxFrameExtentForSlowPathCall;
+    ASSERT(frameExtent == WTF::roundUpToMultipleOf(stackAlignmentBytes(), frameExtent));
+    addPtr(TrustedImm32(-frameExtent), callFrameRegister, stackPointerRegister);
+
     load64(Address(regT3, VM::exceptionOffset()), regT0);
     store64(TrustedImm64(JSValue::encode(JSValue())), Address(regT3, VM::exceptionOffset()));
     emitPutVirtualRegister(currentInstruction[1].u.operand);

Modified: branches/jsCStack/Source/_javascript_Core/jit/JITOpcodes32_64.cpp (160655 => 160656)


--- branches/jsCStack/Source/_javascript_Core/jit/JITOpcodes32_64.cpp	2013-12-16 20:44:01 UTC (rev 160655)
+++ branches/jsCStack/Source/_javascript_Core/jit/JITOpcodes32_64.cpp	2013-12-16 20:46:39 UTC (rev 160656)
@@ -39,6 +39,7 @@
 #include "JSPropertyNameIterator.h"
 #include "JSVariableObject.h"
 #include "LinkBuffer.h"
+#include "MaxFrameExtentForSlowPathCall.h"
 #include "SlowPathCall.h"
 #include "VirtualRegister.h"
 
@@ -119,11 +120,6 @@
     // Handle an exception
     sawException.link(this);
 
-    // Grab the return address.
-    preserveReturnAddressAfterCall(regT1);
-
-    move(TrustedImmPtr(&vm->exceptionLocation), regT2);
-    storePtr(regT1, regT2);
     storePtr(callFrameRegister, &m_vm->topCallFrame);
 
 #if CPU(X86)
@@ -927,6 +923,11 @@
     move(TrustedImmPtr(m_vm), regT3);
     // operationThrow returns the callFrame for the handler.
     load32(Address(regT3, VM::callFrameForThrowOffset()), callFrameRegister);
+
+    size_t frameExtent = JIT::frameRegisterCountFor(codeBlock()) * sizeof(Register) + maxFrameExtentForSlowPathCall;
+    ASSERT(frameExtent == WTF::roundUpToMultipleOf(stackAlignmentBytes(), frameExtent));
+    addPtr(TrustedImm32(-frameExtent), callFrameRegister, stackPointerRegister);
+
     // Now store the exception returned by operationThrow.
     load32(Address(regT3, VM::exceptionOffset() + OBJECT_OFFSETOF(JSValue, u.asBits.payload)), regT0);
     load32(Address(regT3, VM::exceptionOffset() + OBJECT_OFFSETOF(JSValue, u.asBits.tag)), regT1);

Modified: branches/jsCStack/Source/_javascript_Core/jit/ThunkGenerators.cpp (160655 => 160656)


--- branches/jsCStack/Source/_javascript_Core/jit/ThunkGenerators.cpp	2013-12-16 20:44:01 UTC (rev 160655)
+++ branches/jsCStack/Source/_javascript_Core/jit/ThunkGenerators.cpp	2013-12-16 20:46:39 UTC (rev 160656)
@@ -367,6 +367,8 @@
     breakpoint();
 #endif
 
+    jit.emitFunctionEpilogue();
+
     // Check for an exception
 #if USE(JSVALUE64)
     jit.load64(vm->addressOfException(), JSInterfaceJIT::regT2);
@@ -379,18 +381,11 @@
 #endif
 
     // Return.
-    jit.emitFunctionEpilogue();
     jit.ret();
 
     // Handle an exception
     exceptionHandler.link(&jit);
 
-    // Grab the return address.
-    jit.preserveReturnAddressAfterCall(JSInterfaceJIT::regT1);
-    
-    jit.move(JSInterfaceJIT::TrustedImmPtr(&vm->exceptionLocation), JSInterfaceJIT::regT2);
-    jit.storePtr(JSInterfaceJIT::regT1, JSInterfaceJIT::regT2);
-
     jit.storePtr(JSInterfaceJIT::callFrameRegister, &vm->topCallFrame);
 
 #if CPU(X86) && USE(JSVALUE32_64)

Modified: branches/jsCStack/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm (160655 => 160656)


--- branches/jsCStack/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm	2013-12-16 20:44:01 UTC (rev 160655)
+++ branches/jsCStack/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm	2013-12-16 20:46:39 UTC (rev 160656)
@@ -344,7 +344,7 @@
 end
 
 _handleUncaughtException:
-    loadp ScopeChain[cfr], t3
+    loadp ScopeChain + PayloadOffset[cfr], t3
     andp MarkedBlockMask, t3
     loadp MarkedBlock::m_weakSet + WeakSet::m_vm[t3], t3
     loadp VM::callFrameForThrow[t3], cfr
@@ -352,10 +352,10 @@
     # So far, we've unwound the stack to the frame just below the sentinel frame.
     # We need to pop to the sentinel frame and do the necessary clean up for
     # returning to the caller C frame.
-    loadp CallerFrame[cfr], cfr
+    loadp CallerFrame + PayloadOffset[cfr], cfr
 
-    loadp Callee[cfr], t3 # VM.topCallFrame
-    loadp ScopeChain[cfr], t6
+    loadp Callee + PayloadOffset[cfr], t3 # VM.topCallFrame
+    loadp ScopeChain + PayloadOffset[cfr], t6
     storep t6, [t3]
 
     callToJavaScriptEpilogue()
@@ -2002,10 +2002,12 @@
     # the interpreter's throw trampoline (see _llint_throw_trampoline).
     # The throwing code must have known that we were throwing to the interpreter,
     # and have set VM::targetInterpreterPCForThrow.
-    loadp ScopeChain[cfr], t3
+    loadp ScopeChain + PayloadOffset[cfr], t3
     andp MarkedBlockMask, t3
     loadp MarkedBlock::m_weakSet + WeakSet::m_vm[t3], t3
     loadp VM::callFrameForThrow[t3], cfr
+    restoreStackPointerAfterCall()
+
     loadi VM::targetInterpreterPCForThrow[t3], PC
     loadi VM::m_exception + PayloadOffset[t3], t0
     loadi VM::m_exception + TagOffset[t3], t1

Modified: branches/jsCStack/Source/_javascript_Core/runtime/VM.h (160655 => 160656)


--- branches/jsCStack/Source/_javascript_Core/runtime/VM.h	2013-12-16 20:44:01 UTC (rev 160655)
+++ branches/jsCStack/Source/_javascript_Core/runtime/VM.h	2013-12-16 20:46:39 UTC (rev 160656)
@@ -381,7 +381,6 @@
         const ClassInfo* const jsArrayClassInfo;
         const ClassInfo* const jsFinalObjectClassInfo;
 
-        ReturnAddressPtr exceptionLocation;
         JSValue hostCallReturnValue;
         ExecState* newCallFrameReturnValue;
         ExecState* callFrameForThrow;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to