Bug 1032869 - Part 3: Don't consider onExceptionUnwind an all-execution-observing hook. (r=jandem)
authorShu-yu Guo <shu@rfrn.org>
Thu, 13 Nov 2014 14:39:40 -0800
changeset 215679 96a2f59f6ce4
parent 215678 f8e316fa65bb
child 215680 06d07689a043
push id27823
push usercbook@mozilla.com
push date2014-11-14 11:59 +0000
treeherdermozilla-central@bbb68df450c2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1032869
milestone36.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1032869 - Part 3: Don't consider onExceptionUnwind an all-execution-observing hook. (r=jandem)
js/src/jit/BaselineDebugModeOSR.cpp
js/src/jit/IonFrames.cpp
js/src/vm/Debugger-inl.h
js/src/vm/Debugger.cpp
js/src/vm/Debugger.h
--- a/js/src/jit/BaselineDebugModeOSR.cpp
+++ b/js/src/jit/BaselineDebugModeOSR.cpp
@@ -314,17 +314,18 @@ PatchBaselineFramesForDebugMode(JSContex
     // Recompile Patching Overview
     //
     // When toggling debug mode with live baseline scripts on the stack, we
     // could have entered the VM via the following ways from the baseline
     // script.
     //
     // Off to On:
     //  A. From a "can call" stub.
-    //  B. From a VM call (interrupt handler, debugger statement handler).
+    //  B. From a VM call (interrupt handler, debugger statement handler,
+    //                     throw).
     //
     // On to Off:
     //  - All the ways above.
     //  C. From the debug trap handler.
     //  D. From the debug prologue.
     //  E. From the debug epilogue.
     //
     // Off to On to Off:
@@ -415,17 +416,29 @@ PatchBaselineFramesForDebugMode(JSContex
             bool popFrameReg;
             switch (kind) {
               case ICEntry::Kind_CallVM:
                 // Case B above.
                 //
                 // Patching returns from an interrupt handler or the debugger
                 // statement handler is similar in that we can resume at the
                 // next op.
-                pc += GetBytecodeLength(pc);
+                //
+                // Throws are treated differently, as patching a throw means
+                // we are recompiling on-stack scripts from inside an
+                // onExceptionUnwind invocation. The Baseline compiler
+                // considers all bytecode after the throw to be unreachable
+                // and does not compile them, so we cannot patch the resume
+                // address to be the next pc. Since execution cannot continue
+                // after the throw anyways, it doesn't matter what we patch
+                // the resume address with. So that frame iterators may
+                // continue working, we patch the resume address to be right
+                // at the throw.
+                if (JSOp(*pc) != JSOP_THROW)
+                    pc += GetBytecodeLength(pc);
                 recompInfo->resumeAddr = bl->nativeCodeForPC(script, pc, &recompInfo->slotInfo);
                 popFrameReg = true;
                 break;
 
               case ICEntry::Kind_DebugTrap:
                 // Case C above.
                 //
                 // Debug traps are emitted before each op, so we resume at the
--- a/js/src/jit/IonFrames.cpp
+++ b/js/src/jit/IonFrames.cpp
@@ -418,19 +418,20 @@ HandleExceptionIon(JSContext *cx, const 
                    bool *overrecursed)
 {
     RootedScript script(cx, frame.script());
     jsbytecode *pc = frame.pc();
 
     if (cx->compartment()->isDebuggee()) {
         // We need to bail when debug mode is active to observe the Debugger's
         // exception unwinding handler if either a Debugger is observing all
-        // execution in the compartment, or it has observed this frame, i.e.,
-        // by there being a debuggee RematerializedFrame.
-        bool shouldBail = cx->compartment()->debugObservesAllExecution();
+        // execution in the compartment, or it has a live onExceptionUnwind
+        // hook, or it has observed this frame (e.g., for onPop).
+        bool shouldBail = cx->compartment()->debugObservesAllExecution() ||
+                          Debugger::hasLiveOnExceptionUnwind(cx->global());
         if (!shouldBail) {
             JitActivation *act = cx->mainThread().activation()->asJit();
             RematerializedFrame *rematFrame =
                 act->lookupRematerializedFrame(frame.frame().fp(), frame.frameNo());
             shouldBail = rematFrame && rematFrame->isDebuggee();
         }
 
         if (shouldBail) {
@@ -538,17 +539,17 @@ HandleClosingGeneratorReturn(JSContext *
     if (!cx->getPendingException(&exception))
         return;
     if (!exception.isMagic(JS_GENERATOR_CLOSING))
         return;
 
     cx->clearPendingException();
     frame.baselineFrame()->setReturnValue(UndefinedValue());
 
-    if (cx->compartment()->debugMode() && unwoundScopeToPc)
+    if (frame.baselineFrame()->isDebuggee() && unwoundScopeToPc)
         frame.baselineFrame()->setUnwoundScopeOverridePc(unwoundScopeToPc);
 
     ForcedReturn(cx, frame, pc, rfe, calledDebugEpilogue);
 }
 
 static void
 HandleExceptionBaseline(JSContext *cx, const JitFrameIterator &frame, ResumeFromException *rfe,
                         jsbytecode **unwoundScopeToPc, bool *calledDebugEpilogue)
@@ -564,21 +565,20 @@ HandleExceptionBaseline(JSContext *cx, c
     // callback, which cannot easily force a return.
     if (cx->isPropagatingForcedReturn()) {
         cx->clearPropagatingForcedReturn();
         ForcedReturn(cx, frame, pc, rfe, calledDebugEpilogue);
         return;
     }
 
     RootedValue exception(cx);
-    BaselineFrame *baselineFrame = frame.baselineFrame();
-    if (cx->isExceptionPending() && baselineFrame->isDebuggee() &&
+    if (cx->isExceptionPending() && cx->compartment()->isDebuggee() &&
         cx->getPendingException(&exception) && !exception.isMagic(JS_GENERATOR_CLOSING))
     {
-        switch (Debugger::onExceptionUnwind(cx, baselineFrame)) {
+        switch (Debugger::onExceptionUnwind(cx, frame.baselineFrame())) {
           case JSTRAP_ERROR:
             // Uncatchable exception.
             MOZ_ASSERT(!cx->isExceptionPending());
             break;
 
           case JSTRAP_CONTINUE:
           case JSTRAP_THROW:
             MOZ_ASSERT(cx->isExceptionPending());
--- a/js/src/vm/Debugger-inl.h
+++ b/js/src/vm/Debugger-inl.h
@@ -48,15 +48,14 @@ js::Debugger::onDebuggerStatement(JSCont
     return frame.isDebuggee()
            ? dispatchHook(cx, vp, OnDebuggerStatement)
            : JSTRAP_CONTINUE;
 }
 
 /* static */ JSTrapStatus
 js::Debugger::onExceptionUnwind(JSContext *cx, AbstractFramePtr frame)
 {
-    MOZ_ASSERT_IF(frame.script()->isDebuggee(), frame.isDebuggee());
-    if (!frame.isDebuggee())
+    if (!cx->compartment()->isDebuggee())
         return JSTRAP_CONTINUE;
     return slowPathOnExceptionUnwind(cx, frame);
 }
 
 #endif /* vm_Debugger_inl_h */
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -461,16 +461,29 @@ Debugger::getScriptFrameWithIter(JSConte
 
         if (!ensureExecutionObservabilityOfFrame(cx, frame))
             return false;
     }
     vp.setObject(*p->value());
     return true;
 }
 
+/* static */ bool
+Debugger::hasLiveOnExceptionUnwind(GlobalObject *global)
+{
+    if (GlobalObject::DebuggerVector *debuggers = global->getDebuggers()) {
+        for (Debugger **p = debuggers->begin(); p != debuggers->end(); p++) {
+            Debugger *dbg = *p;
+            if (dbg->enabled && dbg->getHook(OnExceptionUnwind))
+                return true;
+        }
+    }
+    return false;
+}
+
 JSObject *
 Debugger::getHook(Hook hook) const
 {
     MOZ_ASSERT(hook >= 0 && hook < HookCount);
     const Value &v = object->getReservedSlot(JSSLOT_DEBUG_HOOK_START + hook);
     return v.isUndefined() ? nullptr : &v.toObject();
 }
 
@@ -1884,17 +1897,16 @@ Debugger::ensureExecutionObservabilityOf
     ExecutionObservableCompartments obs(cx);
     if (!obs.init() || !obs.add(comp))
         return false;
     comp->setDebugObservesAllExecution();
     return updateExecutionObservability(cx, obs, Observing);
 }
 
 static const Debugger::Hook ObserveAllExecutionHooks[] = { Debugger::OnDebuggerStatement,
-                                                           Debugger::OnExceptionUnwind,
                                                            Debugger::OnEnterFrame,
                                                            Debugger::HookCount };
 
 /* static */ bool
 Debugger::hookObservesAllExecution(Hook which)
 {
     for (const Hook *hook = ObserveAllExecutionHooks; *hook != HookCount; hook++) {
         if (*hook == which)
--- a/js/src/vm/Debugger.h
+++ b/js/src/vm/Debugger.h
@@ -554,16 +554,17 @@ class Debugger : private mozilla::Linked
     static inline void onNewScript(JSContext *cx, HandleScript script, GlobalObject *compileAndGoGlobal);
     static inline void onNewGlobalObject(JSContext *cx, Handle<GlobalObject *> global);
     static inline bool onLogAllocationSite(JSContext *cx, HandleSavedFrame frame, int64_t when);
     static JSTrapStatus onTrap(JSContext *cx, MutableHandleValue vp);
     static JSTrapStatus onSingleStep(JSContext *cx, MutableHandleValue vp);
     static bool handleBaselineOsr(JSContext *cx, InterpreterFrame *from, jit::BaselineFrame *to);
     static bool handleIonBailout(JSContext *cx, jit::RematerializedFrame *from, jit::BaselineFrame *to);
     static void propagateForcedReturn(JSContext *cx, AbstractFramePtr frame, HandleValue rval);
+    static bool hasLiveOnExceptionUnwind(GlobalObject *global);
 
     /************************************* Functions for use by Debugger.cpp. */
 
     inline bool observesEnterFrame() const;
     inline bool observesNewScript() const;
     inline bool observesNewGlobalObject() const;
     inline bool observesGlobal(GlobalObject *global) const;
     bool observesFrame(AbstractFramePtr frame) const;