Changeset 160656 in webkit

Timestamp:

Dec 16, 2013 12:46:39 PM (10 years ago)

Author:

mark.lam@apple.com

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.

Location:

branches/jsCStack/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• interpreter/Interpreter.cpp (modified) (3 diffs)
• jit/JITOpcodes.cpp (modified) (2 diffs)
• jit/JITOpcodes32_64.cpp (modified) (3 diffs)
• jit/ThunkGenerators.cpp (modified) (2 diffs)
• llint/LowLevelInterpreter32_64.asm (modified) (3 diffs)
• runtime/VM.h (modified) (1 diff)

branches/jsCStack/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2013-12-14  Mark Lam  <mark.lam@apple.com>
+
+        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  <fpizlo@apple.com>
 

branches/jsCStack/Source/JavaScriptCore/interpreter/Interpreter.cpp

@@ -422 +422 @@
     CallFrame* callFrame = visitor->callFrame();
     CodeBlock* codeBlock = visitor->codeBlock();
-    CodeBlock* oldCodeBlock = codeBlock;
     JSScope* scope = callFrame->scope();
 
@@ -433 +432 @@
 
     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)
@@ -456 +455 @@
 
     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();
 }
 

branches/jsCStack/Source/JavaScriptCore/jit/JITOpcodes.cpp

@@ -39 +39 @@
 #include "JSPropertyNameIterator.h"
 #include "LinkBuffer.h"
+#include "MaxFrameExtentForSlowPathCall.h"
 #include "SlowPathCall.h"
 #include "VirtualRegister.h"
@@ -636 +637 @@
     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()));

branches/jsCStack/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp

@@ -40 +40 @@
 #include "JSVariableObject.h"
 #include "LinkBuffer.h"
+#include "MaxFrameExtentForSlowPathCall.h"
 #include "SlowPathCall.h"
 #include "VirtualRegister.h"
@@ -120 +121 @@
     sawException.link(this);
 
-    // Grab the return address.
-    preserveReturnAddressAfterCall(regT1);
-
-    move(TrustedImmPtr(&vm->exceptionLocation), regT2);
-    storePtr(regT1, regT2);
     storePtr(callFrameRegister, &m_vm->topCallFrame);
 
@@ -928 +924 @@
     // 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);

branches/jsCStack/Source/JavaScriptCore/jit/ThunkGenerators.cpp

@@ -368 +368 @@
 #endif
 
+    jit.emitFunctionEpilogue();
+
     // Check for an exception
 #if USE(JSVALUE64)
@@ -380 +382 @@
 
     // 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);

branches/jsCStack/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm

@@ -345 +345 @@
 
 _handleUncaughtException:
-    loadp ScopeChain[cfr], t3
+    loadp ScopeChain + PayloadOffset[cfr], t3
     andp MarkedBlockMask, t3
     loadp MarkedBlock::m_weakSet + WeakSet::m_vm[t3], t3
@@ -353 +353 @@
     # 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 Callee[cfr], t3 # VM.topCallFrame
-    loadp ScopeChain[cfr], t6
+    loadp CallerFrame + PayloadOffset[cfr], cfr
+
+    loadp Callee + PayloadOffset[cfr], t3 # VM.topCallFrame
+    loadp ScopeChain + PayloadOffset[cfr], t6
     storep t6, [t3]
 
@@ -2003 +2003 @@
     # 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

branches/jsCStack/Source/JavaScriptCore/runtime/VM.h

@@ -382 +382 @@
         const ClassInfo* const jsFinalObjectClassInfo;
 
-        ReturnAddressPtr exceptionLocation;
         JSValue hostCallReturnValue;
         ExecState* newCallFrameReturnValue;