Changeset 172867 in webkit for trunk/Source/JavaScriptCore/jit/JIT.cpp

Timestamp:

Aug 22, 2014, 12:54:30 PM (11 years ago)

Author:

[email protected]

Message:

REGRESSION(r163179): Sporadic crash in js/dom/line-column-numbers.html test
​https://bugs.webkit.org/show_bug.cgi?id=136111

Reviewed by Filip Pizlo.

The problem was that we weren't properly handling VM::topVMEntryFrame in two ways.

First in the case where we get an exception of a stack overflow during setup of the direct
callee frame of a VM entry frame, we need to throw the exception in the caller's frame.
This requires unrolling topVMEntryFrame while creating the exception object. This is
accomplished with the renamed NativeCallFrameTracerWithRestore object. As part of this,
split the JIT rollback exception handling to call a new helper,
callLookupExceptionHandlerFromCallerFrame, which will unroll the callFrame and VMEntryFrame.

Second, when we unwind to find a handler, we also need to unwind topVMCallFrame for the
case where we end up (re)throwing another exception after entering the catch block, but
before another vmEntry call. Added VM::vmEntryFrameForThrow as a way similar to
VM::callFrameForThrow to pass the appropriate VMENtryFrame to the catch block.

• dfg/DFGJITCompiler.cpp:

(JSC::DFG::JITCompiler::compileExceptionHandlers):

• ftl/FTLCompile.cpp:

(JSC::FTL::fixFunctionBasedOnStackMaps):

• jit/JIT.cpp:

(JSC::JIT::privateCompileExceptionHandlers):
Split out the unroll cases to use the new helper callLookupExceptionHandlerFromCallerFrame()
to unwind both the callFrame and topVMEntryFrame.

• interpreter/Interpreter.cpp:

(JSC::UnwindFunctor::UnwindFunctor):
(JSC::UnwindFunctor::operator()):
(JSC::Interpreter::unwind):

• jit/JITExceptions.cpp:

(JSC::genericUnwind):
Added VMEntryFrame as another component to unwind.

• interpreter/Interpreter.h:

(JSC::NativeCallFrameTracer::NativeCallFrameTracer):
(JSC::NativeCallFrameTracerWithRestore::NativeCallFrameTracerWithRestore):
(JSC::NativeCallFrameTracerWithRestore::~NativeCallFrameTracerWithRestore):
Renamed and changed to save and restore topCallFrame and topVMEntryFrame around the setting of
both values.

• interpreter/StackVisitor.cpp:

(JSC::StackVisitor::gotoNextFrame):
(JSC::StackVisitor::readNonInlinedFrame):

• interpreter/StackVisitor.h:

(JSC::StackVisitor::Frame::vmEntryFrame):
Added code to unwind the VMEntryFrame.

• jit/CCallHelpers.h:

(JSC::CCallHelpers::jumpToExceptionHandler): Updated comment to indicate that the value
the handler should use for VM::topEntryFrame is in VM::vmEntryFrameForThrow.

• jit/JITOpcodes.cpp:

(JSC::JIT::emit_op_catch):

• jit/JITOpcodes32_64.cpp:

(JSC::JIT::emit_op_catch):

• llint/LowLevelInterpreter32_64.asm:
• llint/LowLevelInterpreter64.asm:

Added code to update VM::topVMEntryFrame from VM::vmEntryFrameForThrowOffset.

• jit/JITOperations.cpp:
• jit/JITOperations.h:

(JSC::operationThrowStackOverflowError):
(JSC::operationCallArityCheck):
(JSC::operationConstructArityCheck):

• runtime/VM.h:

(JSC::VM::vmEntryFrameForThrowOffset):
(JSC::VM::topVMEntryFrameOffset):
Added as the side channel to return the topVMEntryFrame that the handler should use.

File:

• trunk/Source/JavaScriptCore/jit/JIT.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/jit/JIT.cpp

@@ -723 +723 @@
 void JIT::privateCompileExceptionHandlers()
 {
-    if (m_exceptionChecks.empty() && m_exceptionChecksWithCallFrameRollback.empty())
-        return;
-
-    Jump doLookup;
-
     if (!m_exceptionChecksWithCallFrameRollback.empty()) {
         m_exceptionChecksWithCallFrameRollback.link(this);
-        emitGetCallerFrameFromCallFrameHeaderPtr(GPRInfo::argumentGPR1);
-        doLookup = jump();
-    }
-
-    if (!m_exceptionChecks.empty())
+
+        // lookupExceptionHandlerFromCallerFrame is passed two arguments, the VM and the exec (the CallFrame*).
+
+        move(TrustedImmPtr(vm()), GPRInfo::argumentGPR0);
+        move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR1);
+
+#if CPU(X86)
+        // FIXME: should use the call abstraction, but this is currently in the SpeculativeJIT layer!
+        poke(GPRInfo::argumentGPR0);
+        poke(GPRInfo::argumentGPR1, 1);
+#endif
+        m_calls.append(CallRecord(call(), (unsigned)-1, FunctionPtr(lookupExceptionHandlerFromCallerFrame).value()));
+        jumpToExceptionHandler();
+    }
+
+    if (!m_exceptionChecks.empty()) {
         m_exceptionChecks.link(this);
-    
-    // lookupExceptionHandler is passed two arguments, the VM and the exec (the CallFrame*).
-    move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR1);
-
-    if (doLookup.isSet())
-        doLookup.link(this);
-
-    move(TrustedImmPtr(vm()), GPRInfo::argumentGPR0);
+
+        // lookupExceptionHandler is passed two arguments, the VM and the exec (the CallFrame*).
+        move(TrustedImmPtr(vm()), GPRInfo::argumentGPR0);
+        move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR1);
 
 #if CPU(X86)
-    // FIXME: should use the call abstraction, but this is currently in the SpeculativeJIT layer!
-    poke(GPRInfo::argumentGPR0);
-    poke(GPRInfo::argumentGPR1, 1);
-#endif
-    m_calls.append(CallRecord(call(), (unsigned)-1, FunctionPtr(lookupExceptionHandler).value()));
-    jumpToExceptionHandler();
+        // FIXME: should use the call abstraction, but this is currently in the SpeculativeJIT layer!
+        poke(GPRInfo::argumentGPR0);
+        poke(GPRInfo::argumentGPR1, 1);
+#endif
+        m_calls.append(CallRecord(call(), (unsigned)-1, FunctionPtr(lookupExceptionHandler).value()));
+        jumpToExceptionHandler();
+    }
 }