Bug 1100320 - Don't call onExceptionUnwind and onPop debugger hooks on over-recursion. (r=jimb)

js/src/jit-test/tests/debug/onExceptionUnwind-12.js

@@ -0,0 +1,14 @@
+var g = newGlobal();
+g.parent = this;
+g.hits = 0;
+g.eval("new Debugger(parent).onExceptionUnwind = function () { hits++; };");
+function f() {
+    var x = f();
+}
+try {
+  f();
+} catch (e) {
+  assertEq(e instanceof InternalError, true);
+} finally {
+  assertEq(g.hits, 0);
+}

js/src/jsapi.cpp

@@ -6099,6 +6099,7 @@ JS_ReportPendingException(JSContext *cx)
 JS::AutoSaveExceptionState::AutoSaveExceptionState(JSContext *cx)
   : context(cx),
     wasPropagatingForcedReturn(cx->propagatingForcedReturn_),
+    wasOverRecursed(cx->overRecursed_),
     wasThrowing(cx->throwing),
     exceptionValue(cx)
 {
@@ -6106,6 +6107,8 @@ JS::AutoSaveExceptionState::AutoSaveExceptionState(JSContext *cx)
     CHECK_REQUEST(cx);
     if (wasPropagatingForcedReturn)
         cx->clearPropagatingForcedReturn();
+    if (wasOverRecursed)
+        cx->overRecursed_ = false;
     if (wasThrowing) {
         exceptionValue = cx->unwrappedException_;
         cx->clearPendingException();
@@ -6116,6 +6119,7 @@ void
 JS::AutoSaveExceptionState::restore()
 {
     context->propagatingForcedReturn_ = wasPropagatingForcedReturn;
+    context->overRecursed_ = wasOverRecursed;
     context->throwing = wasThrowing;
     context->unwrappedException_ = exceptionValue;
     drop();
@@ -6127,6 +6131,7 @@ JS::AutoSaveExceptionState::~AutoSaveExceptionState()
         if (wasPropagatingForcedReturn)
             context->setPropagatingForcedReturn();
         if (wasThrowing) {
+            context->overRecursed_ = wasOverRecursed;
             context->throwing = true;
             context->unwrappedException_ = exceptionValue;
         }

js/src/jsapi.h

@@ -4989,6 +4989,7 @@ class JS_PUBLIC_API(AutoSaveExceptionState)
   private:
     JSContext *context;
     bool wasPropagatingForcedReturn;
+    bool wasOverRecursed;
     bool wasThrowing;
     RootedValue exceptionValue;
 
@@ -5011,6 +5012,7 @@ class JS_PUBLIC_API(AutoSaveExceptionState)
      */
     void drop() {
         wasPropagatingForcedReturn = false;
+        wasOverRecursed = false;
         wasThrowing = false;
         exceptionValue.setUndefined();
     }

js/src/jscntxt.cpp

@@ -416,8 +416,10 @@ js_ReportOverRecursed(JSContext *maybecx)
      */
     fprintf(stderr, "js_ReportOverRecursed called\n");
 #endif
-    if (maybecx)
+    if (maybecx) {
         JS_ReportErrorNumber(maybecx, js_GetErrorMessage, nullptr, JSMSG_OVER_RECURSED);
+        maybecx->overRecursed_ = true;
+    }
 }
 
 void
@@ -1010,6 +1012,7 @@ JSContext::JSContext(JSRuntime *rt)
     throwing(false),
     unwrappedException_(UndefinedValue()),
     options_(),
+    overRecursed_(false),
     propagatingForcedReturn_(false),
     liveVolatileJitFrameIterators_(nullptr),
     reportGranularity(JS_DEFAULT_JITREPORT_GRANULARITY),
@@ -1042,11 +1045,13 @@ JSContext::getPendingException(MutableHandleValue rval)
     rval.set(unwrappedException_);
     if (IsAtomsCompartment(compartment()))
         return true;
+    bool wasOverRecursed = overRecursed_;
     clearPendingException();
     if (!compartment()->wrap(this, rval))
         return false;
     assertSameCompartment(this, rval);
     setPendingException(rval);
+    overRecursed_ = wasOverRecursed;
     return true;
 }
 

js/src/jscntxt.h

@@ -423,6 +423,7 @@ struct JSContext : public js::ExclusiveContext,
     friend class js::ExclusiveContext;
     friend class JS::AutoSaveExceptionState;
     friend class js::jit::DebugModeOSRVolatileJitFrameIterator;
+    friend void js_ReportOverRecursed(JSContext *);
 
   private:
     /* Exception state -- the exception member is a GC root by definition. */
@@ -432,6 +433,10 @@ struct JSContext : public js::ExclusiveContext,
     /* Per-context options. */
     JS::ContextOptions  options_;
 
+    // True if the exception currently being thrown is by result of
+    // js_ReportOverRecursed. See Debugger::slowPathOnExceptionUnwind.
+    bool                overRecursed_;
+
     // True if propagating a forced return from an interrupt handler during
     // debug mode.
     bool                propagatingForcedReturn_;
@@ -571,9 +576,11 @@ struct JSContext : public js::ExclusiveContext,
 
     void clearPendingException() {
         throwing = false;
+        overRecursed_ = false;
         unwrappedException_.setUndefined();
     }
 
+    bool isThrowingOverRecursed() const { return throwing && overRecursed_; }
     bool isPropagatingForcedReturn() const { return propagatingForcedReturn_; }
     void setPropagatingForcedReturn() { propagatingForcedReturn_ = true; }
     void clearPropagatingForcedReturn() { propagatingForcedReturn_ = false; }

js/src/jscntxtinlines.h

@@ -367,6 +367,8 @@ inline void
 JSContext::setPendingException(js::Value v)
 {
     MOZ_ASSERT(!IsPoisonedValue(v));
+    // overRecursed_ is set after the fact by js_ReportOverRecursed.
+    this->overRecursed_ = false;
     this->throwing = true;
     this->unwrappedException_ = v;
     // We don't use assertSameCompartment here to allow

js/src/vm/Debugger.cpp

@@ -592,51 +592,57 @@ Debugger::slowPathOnLeaveFrame(JSContext *cx, AbstractFramePtr frame, bool frame
     RootedValue value(cx);
     Debugger::resultToCompletion(cx, frameOk, frame.returnValue(), &status, &value);
 
-    /* Build a list of the recipients. */
-    AutoObjectVector frames(cx);
-    for (FrameRange r(frame, global); !r.empty(); r.popFront()) {
-        if (!frames.append(r.frontFrame())) {
-            cx->clearPendingException();
-            return false;
-        }
-    }
-
-    /* For each Debugger.Frame, fire its onPop handler, if any. */
-    for (JSObject **p = frames.begin(); p != frames.end(); p++) {
-        RootedNativeObject frameobj(cx, &(*p)->as<NativeObject>());
-        Debugger *dbg = Debugger::fromChildJSObject(frameobj);
-
-        if (dbg->enabled &&
-            !frameobj->getReservedSlot(JSSLOT_DEBUGFRAME_ONPOP_HANDLER).isUndefined()) {
-            RootedValue handler(cx, frameobj->getReservedSlot(JSSLOT_DEBUGFRAME_ONPOP_HANDLER));
-
-            Maybe<AutoCompartment> ac;
-            ac.emplace(cx, dbg->object);
-
-            RootedValue completion(cx);
-            if (!dbg->newCompletionValue(cx, status, value, &completion)) {
-                status = dbg->handleUncaughtException(ac, false);
-                break;
+    // This path can be hit via unwinding the stack due to
+    // over-recursion. Only fire the frames' onPop handlers if we haven't
+    // over-recursed, because invoking more JS will only result in more
+    // over-recursion errors. See slowPathOnExceptionUnwind.
+    if (!cx->isThrowingOverRecursed()) {
+        /* Build a list of the recipients. */
+        AutoObjectVector frames(cx);
+        for (FrameRange r(frame, global); !r.empty(); r.popFront()) {
+            if (!frames.append(r.frontFrame())) {
+                cx->clearPendingException();
+                return false;
             }
+        }
 
-            /* Call the onPop handler. */
-            RootedValue rval(cx);
-            bool hookOk = Invoke(cx, ObjectValue(*frameobj), handler, 1, completion.address(),
-                                 &rval);
-            RootedValue nextValue(cx);
-            JSTrapStatus nextStatus = dbg->parseResumptionValue(ac, hookOk, rval, &nextValue);
+        /* For each Debugger.Frame, fire its onPop handler, if any. */
+        for (JSObject **p = frames.begin(); p != frames.end(); p++) {
+            RootedNativeObject frameobj(cx, &(*p)->as<NativeObject>());
+            Debugger *dbg = Debugger::fromChildJSObject(frameobj);
 
-            /*
-             * At this point, we are back in the debuggee compartment, and any error has
-             * been wrapped up as a completion value.
-             */
-            MOZ_ASSERT(cx->compartment() == global->compartment());
-            MOZ_ASSERT(!cx->isExceptionPending());
+            if (dbg->enabled &&
+                !frameobj->getReservedSlot(JSSLOT_DEBUGFRAME_ONPOP_HANDLER).isUndefined()) {
+                RootedValue handler(cx, frameobj->getReservedSlot(JSSLOT_DEBUGFRAME_ONPOP_HANDLER));
 
-            /* JSTRAP_CONTINUE means "make no change". */
-            if (nextStatus != JSTRAP_CONTINUE) {
-                status = nextStatus;
-                value = nextValue;
+                Maybe<AutoCompartment> ac;
+                ac.emplace(cx, dbg->object);
+
+                RootedValue completion(cx);
+                if (!dbg->newCompletionValue(cx, status, value, &completion)) {
+                    status = dbg->handleUncaughtException(ac, false);
+                    break;
+                }
+
+                /* Call the onPop handler. */
+                RootedValue rval(cx);
+                bool hookOk = Invoke(cx, ObjectValue(*frameobj), handler, 1, completion.address(),
+                                     &rval);
+                RootedValue nextValue(cx);
+                JSTrapStatus nextStatus = dbg->parseResumptionValue(ac, hookOk, rval, &nextValue);
+
+                /*
+                 * At this point, we are back in the debuggee compartment, and any error has
+                 * been wrapped up as a completion value.
+                 */
+                MOZ_ASSERT(cx->compartment() == global->compartment());
+                MOZ_ASSERT(!cx->isExceptionPending());
+
+                /* JSTRAP_CONTINUE means "make no change". */
+                if (nextStatus != JSTRAP_CONTINUE) {
+                    status = nextStatus;
+                    value = nextValue;
+                }
             }
         }
     }
@@ -715,6 +721,11 @@ Debugger::slowPathOnDebuggerStatement(JSContext *cx, AbstractFramePtr frame)
 /* static */ JSTrapStatus
 Debugger::slowPathOnExceptionUnwind(JSContext *cx, AbstractFramePtr frame)
 {
+    // Invoking more JS on an over-recursed stack is only going to result in
+    // more over-recursion errors.
+    if (cx->isThrowingOverRecursed())
+        return JSTRAP_CONTINUE;
+
     RootedValue rval(cx);
     JSTrapStatus status = dispatchHook(cx, &rval, OnExceptionUnwind, NullPtr());