Changeset 172867 in webkit for trunk/Source/JavaScriptCore/interpreter

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.

Location:

trunk/Source/JavaScriptCore/interpreter

Files:

• Interpreter.cpp (modified) (5 diffs)
• Interpreter.h (modified) (2 diffs)
• StackVisitor.cpp (modified) (2 diffs)
• StackVisitor.h (modified) (2 diffs)

trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp

@@ -646 +646 @@
 class UnwindFunctor {
 public:
-    UnwindFunctor(CallFrame*& callFrame, bool isTermination, CodeBlock*& codeBlock, HandlerInfo*& handler)
-        : m_callFrame(callFrame)
+    UnwindFunctor(VMEntryFrame*& vmEntryFrame, CallFrame*& callFrame, bool isTermination, CodeBlock*& codeBlock, HandlerInfo*& handler)
+        : m_vmEntryFrame(vmEntryFrame)
+        , m_callFrame(callFrame)
         , m_isTermination(isTermination)
         , m_codeBlock(codeBlock)
@@ -657 +658 @@
     {
         VM& vm = m_callFrame->vm();
+        m_vmEntryFrame = visitor->vmEntryFrame();
         m_callFrame = visitor->callFrame();
         m_codeBlock = visitor->codeBlock();
@@ -674 +676 @@
 
 private:
+    VMEntryFrame*& m_vmEntryFrame;
     CallFrame*& m_callFrame;
     bool m_isTermination;
@@ -680 +683 @@
 };
 
-NEVER_INLINE HandlerInfo* Interpreter::unwind(CallFrame*& callFrame, JSValue& exceptionValue)
+NEVER_INLINE HandlerInfo* Interpreter::unwind(VMEntryFrame*& vmEntryFrame, CallFrame*& callFrame, JSValue& exceptionValue)
 {
     CodeBlock* codeBlock = callFrame->codeBlock();
@@ -725 +728 @@
     VM& vm = callFrame->vm();
     ASSERT(callFrame == vm.topCallFrame);
-    UnwindFunctor functor(callFrame, isTermination, codeBlock, handler);
+    UnwindFunctor functor(vmEntryFrame, callFrame, isTermination, codeBlock, handler);
     callFrame->iterate(functor);
     if (!handler)

trunk/Source/JavaScriptCore/interpreter/Interpreter.h

@@ -176 +176 @@
             ASSERT(vm);
             ASSERT(callFrame);
-            ASSERT(callFrame < vm->topVMEntryFrame);
             vm->topCallFrame = callFrame;
         }
-
-        ALWAYS_INLINE NativeCallFrameTracer(VM* vm, VMEntryFrame* vmEntryFrame, CallFrame* callFrame)
+    };
+
+    class NativeCallFrameTracerWithRestore {
+    public:
+        ALWAYS_INLINE NativeCallFrameTracerWithRestore(VM* vm, VMEntryFrame* vmEntryFrame, CallFrame* callFrame)
+            : m_vm(vm)
         {
             ASSERT(vm);
             ASSERT(callFrame);
-            ASSERT(callFrame < vmEntryFrame);
+            m_savedTopVMEntryFrame = vm->topVMEntryFrame;
+            m_savedTopCallFrame = vm->topCallFrame;
             vm->topVMEntryFrame = vmEntryFrame;
             vm->topCallFrame = callFrame;
         }
+
+        ALWAYS_INLINE ~NativeCallFrameTracerWithRestore()
+        {
+            m_vm->topVMEntryFrame = m_savedTopVMEntryFrame;
+            m_vm->topCallFrame = m_savedTopCallFrame;
+        }
+
+    private:
+        VM* m_vm;
+        VMEntryFrame* m_savedTopVMEntryFrame;
+        CallFrame* m_savedTopCallFrame;
     };
 
@@ -237 +252 @@
         SamplingTool* sampler() { return m_sampler.get(); }
 
-        NEVER_INLINE HandlerInfo* unwind(CallFrame*&, JSValue&);
+        NEVER_INLINE HandlerInfo* unwind(VMEntryFrame*&, CallFrame*&, JSValue&);
         NEVER_INLINE void debug(CallFrame*, DebugHookID);
         JSString* stackTraceAsString(ExecState*, Vector<StackFrame>);

trunk/Source/JavaScriptCore/interpreter/StackVisitor.cpp

@@ -62 +62 @@
         CodeOrigin* callerCodeOrigin = &inlineCallFrame->caller;
         readInlinedFrame(m_frame.callFrame(), callerCodeOrigin);
-
-    } else
+        return;
+    }
 #endif // ENABLE(DFG_JIT)
-        readFrame(m_frame.callerFrame());
+    m_frame.m_VMEntryFrame = m_frame.m_CallerVMEntryFrame;
+    readFrame(m_frame.callerFrame());
 }
 
@@ -117 +118 @@
     m_frame.m_callFrame = callFrame;
     m_frame.m_argumentCountIncludingThis = callFrame->argumentCountIncludingThis();
-    VMEntryFrame* currentVMEntryFrame = m_frame.m_VMEntryFrame;
-    m_frame.m_callerFrame = callFrame->callerFrame(m_frame.m_VMEntryFrame);
-    m_frame.m_callerIsVMEntryFrame = currentVMEntryFrame != m_frame.m_VMEntryFrame;
+    m_frame.m_CallerVMEntryFrame = m_frame.m_VMEntryFrame;
+    m_frame.m_callerFrame = callFrame->callerFrame(m_frame.m_CallerVMEntryFrame);
+    m_frame.m_callerIsVMEntryFrame = m_frame.m_CallerVMEntryFrame != m_frame.m_VMEntryFrame;
     m_frame.m_callee = callFrame->callee();
     m_frame.m_scope = callFrame->scope();

trunk/Source/JavaScriptCore/interpreter/StackVisitor.h

@@ -82 +82 @@
         Arguments* createArguments();
         Arguments* existingArguments();
+        VMEntryFrame* vmEntryFrame() const { return m_VMEntryFrame; }
         CallFrame* callFrame() const { return m_callFrame; }
         
@@ -98 +99 @@
         size_t m_argumentCountIncludingThis;
         VMEntryFrame* m_VMEntryFrame;
+        VMEntryFrame* m_CallerVMEntryFrame;
         CallFrame* m_callerFrame;
         JSObject* m_callee;