Refactor common code between GetCatchHandlerFunctor and UnwindFunctor
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 19 Sep 2015 01:26:32 +0000 (01:26 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 19 Sep 2015 01:26:32 +0000 (01:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=149276

Reviewed by Mark Lam.

There is currently code copy-pasted between these
two functors. Lets not do that. It's better to write
a function, even if the function is small.

I also did a bit of renaming to make the intent of the
unwindCallFrame function clear. The name of the function
didn't really indicate what it did. It decided if it was
okay to unwind further, and it also notified the debugger.
I've renamed the function to notifyDebuggerOfUnwinding.
And I've inlined the logic of deciding if it's okay
to unwind further into UnwindFunctor itself.

* interpreter/Interpreter.cpp:
(JSC::Interpreter::isOpcode):
(JSC::getStackFrameCodeType):
(JSC::Interpreter::stackTraceAsString):
(JSC::findExceptionHandler):
(JSC::GetCatchHandlerFunctor::GetCatchHandlerFunctor):
(JSC::GetCatchHandlerFunctor::operator()):
(JSC::notifyDebuggerOfUnwinding):
(JSC::UnwindFunctor::UnwindFunctor):
(JSC::UnwindFunctor::operator()):
(JSC::Interpreter::notifyDebuggerOfExceptionToBeThrown):
(JSC::unwindCallFrame): Deleted.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@190004 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/interpreter/Interpreter.cpp

index 6ec9328..4a90d9c 100644 (file)
@@ -1,3 +1,35 @@
+2015-09-18  Saam barati  <sbarati@apple.com>
+
+        Refactor common code between GetCatchHandlerFunctor and UnwindFunctor
+        https://bugs.webkit.org/show_bug.cgi?id=149276
+
+        Reviewed by Mark Lam.
+
+        There is currently code copy-pasted between these
+        two functors. Lets not do that. It's better to write
+        a function, even if the function is small.
+
+        I also did a bit of renaming to make the intent of the
+        unwindCallFrame function clear. The name of the function
+        didn't really indicate what it did. It decided if it was
+        okay to unwind further, and it also notified the debugger.
+        I've renamed the function to notifyDebuggerOfUnwinding.
+        And I've inlined the logic of deciding if it's okay
+        to unwind further into UnwindFunctor itself.
+
+        * interpreter/Interpreter.cpp:
+        (JSC::Interpreter::isOpcode):
+        (JSC::getStackFrameCodeType):
+        (JSC::Interpreter::stackTraceAsString):
+        (JSC::findExceptionHandler):
+        (JSC::GetCatchHandlerFunctor::GetCatchHandlerFunctor):
+        (JSC::GetCatchHandlerFunctor::operator()):
+        (JSC::notifyDebuggerOfUnwinding):
+        (JSC::UnwindFunctor::UnwindFunctor):
+        (JSC::UnwindFunctor::operator()):
+        (JSC::Interpreter::notifyDebuggerOfExceptionToBeThrown):
+        (JSC::unwindCallFrame): Deleted.
+
 2015-09-18  Sukolsak Sakshuwong  <sukolsak@gmail.com>
 
         Implement the arithmetic instructions for doubles in WebAssembly
index 64732a2..04be8c8 100644 (file)
@@ -436,22 +436,6 @@ bool Interpreter::isOpcode(Opcode opcode)
 #endif
 }
 
-static bool unwindCallFrame(StackVisitor& visitor)
-{
-    CallFrame* callFrame = visitor->callFrame();
-    if (Debugger* debugger = callFrame->vmEntryGlobalObject()->debugger()) {
-        SuspendExceptionScope scope(&callFrame->vm());
-        if (jsDynamicCast<JSFunction*>(callFrame->callee()))
-            debugger->returnEvent(callFrame);
-        else
-            debugger->didExecuteProgram(callFrame);
-        ASSERT(!callFrame->hadException());
-    }
-
-    bool shouldContinueUnwinding = !visitor->callerIsVMEntryFrame();
-    return shouldContinueUnwinding;
-}
-
 static StackFrameCodeType getStackFrameCodeType(StackVisitor& visitor)
 {
     switch (visitor->codeType()) {
@@ -603,6 +587,23 @@ JSString* Interpreter::stackTraceAsString(ExecState* exec, Vector<StackFrame> st
     return jsString(&exec->vm(), builder.toString());
 }
 
+ALWAYS_INLINE static HandlerInfo* findExceptionHandler(StackVisitor& visitor, CodeBlock* codeBlock, CodeBlock::RequiredHandler requiredHandler)
+{
+    ASSERT(codeBlock);
+#if ENABLE(DFG_JIT)
+    ASSERT(!visitor->isInlinedFrame());
+#endif
+
+    CallFrame* callFrame = visitor->callFrame();
+    unsigned exceptionHandlerIndex;
+    if (codeBlock->jitType() != JITCode::DFGJIT)
+        exceptionHandlerIndex = callFrame->bytecodeOffset();
+    else
+        exceptionHandlerIndex = callFrame->callSiteIndex().bits();
+
+    return codeBlock->handlerForIndex(exceptionHandlerIndex, requiredHandler);
+}
+
 class GetCatchHandlerFunctor {
 public:
     GetCatchHandlerFunctor()
@@ -620,13 +621,7 @@ public:
         if (!codeBlock)
             return StackVisitor::Continue;
 
-        unsigned exceptionHandlerIndex;
-        if (codeBlock->jitType() != JITCode::DFGJIT)
-            exceptionHandlerIndex = visitor->callFrame()->bytecodeOffset();
-        else
-            exceptionHandlerIndex = visitor->callFrame()->callSiteIndex().bits();
-
-        m_handler = codeBlock->handlerForIndex(exceptionHandlerIndex, CodeBlock::RequiredHandler::CatchHandler);
+        m_handler = findExceptionHandler(visitor, codeBlock, CodeBlock::RequiredHandler::CatchHandler);
         if (m_handler)
             return StackVisitor::Done;
 
@@ -637,6 +632,18 @@ private:
     HandlerInfo* m_handler;
 };
 
+ALWAYS_INLINE static void notifyDebuggerOfUnwinding(CallFrame* callFrame)
+{
+    if (Debugger* debugger = callFrame->vmEntryGlobalObject()->debugger()) {
+        SuspendExceptionScope scope(&callFrame->vm());
+        if (jsDynamicCast<JSFunction*>(callFrame->callee()))
+            debugger->returnEvent(callFrame);
+        else
+            debugger->didExecuteProgram(callFrame);
+        ASSERT(!callFrame->hadException());
+    }
+}
+
 class UnwindFunctor {
 public:
     UnwindFunctor(CallFrame*& callFrame, bool isTermination, CodeBlock*& codeBlock, HandlerInfo*& handler)
@@ -654,22 +661,19 @@ public:
         m_callFrame = visitor->callFrame();
         m_codeBlock = visitor->codeBlock();
 
-        unsigned exceptionHandlerIndex;
-        if (m_codeBlock->jitType() != JITCode::DFGJIT)
-            exceptionHandlerIndex = m_callFrame->bytecodeOffset();
-        else
-            exceptionHandlerIndex = m_callFrame->callSiteIndex().bits();
         m_handler = nullptr;
         if (!m_isTermination) {
             if (m_codeBlock && !isWebAssemblyExecutable(m_codeBlock->ownerExecutable()))
-                m_handler = m_codeBlock->handlerForIndex(exceptionHandlerIndex);
+                m_handler = findExceptionHandler(visitor, m_codeBlock, CodeBlock::RequiredHandler::AnyHandler);
         }
 
         if (m_handler)
             return StackVisitor::Done;
 
-        bool shouldContinueUnwinding = unwindCallFrame(visitor);
-        if (!shouldContinueUnwinding) {
+        notifyDebuggerOfUnwinding(m_callFrame);
+
+        bool shouldStopUnwinding = visitor->callerIsVMEntryFrame();
+        if (shouldStopUnwinding) {
             if (LegacyProfiler* profiler = vm.enabledProfiler())
                 profiler->exceptionUnwind(m_callFrame);
 
@@ -759,7 +763,6 @@ NEVER_INLINE HandlerInfo* Interpreter::unwind(VM& vm, CallFrame*& callFrame, Exc
 
 void Interpreter::notifyDebuggerOfExceptionToBeThrown(CallFrame* callFrame, Exception* exception)
 {
-    bool isTermination = isTerminatedExecutionException(exception);
     Debugger* debugger = callFrame->vmEntryGlobalObject()->debugger();
     if (debugger && debugger->needsExceptionCallbacks() && !exception->didNotifyInspectorOfThrow()) {
         // This code assumes that if the debugger is enabled then there is no inlining.
@@ -768,6 +771,7 @@ void Interpreter::notifyDebuggerOfExceptionToBeThrown(CallFrame* callFrame, Exce
         // https://bugs.webkit.org/show_bug.cgi?id=121754
 
         bool hasCatchHandler;
+        bool isTermination = isTerminatedExecutionException(exception);
         if (isTermination)
             hasCatchHandler = false;
         else {