Bug 687683: Collect resumption values from onEnterFrame handlers, and respect them. r=jorendorff
authorJim Blandy <jimb@mozilla.com>
Tue, 06 Dec 2011 11:40:28 -0800
changeset 82083 bffaef968b0885d525470bb47a34c9ead50d2600
parent 82082 cca7e56a13d91604ff2326832b0e676027d0cdce
child 82084 319c74e59fd4161a78b8a0f87bf4a7289e35f7d0
push id21582
push userbmo@edmorley.co.uk
push dateWed, 07 Dec 2011 09:30:09 +0000
treeherdermozilla-central@489f2d51b011 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs687683
milestone11.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 687683: Collect resumption values from onEnterFrame handlers, and respect them. r=jorendorff This patch makes SpiderMonkey respect resumption values returned by Debugger onEnterFrame handler functions, as documented. In Debugger, we change fireEnterFrame to collect a resumption value from the hook, and change onEnterFrame and slowPathOnEnterFrame to propagate them out. These now need an 'rval' argument, so that they can return forced return values and exceptions. ScriptDebugPrologue now accepts a JSTrapStatus from Debugger::onEnterFrame, takes care of placing the return value or exception where it belongs, and returns a JSTrapStatus. Calls to ScriptDebugPrologue now handle the JSTrapStatus: - at the head of js::Interpret; - in the JSOP_NEW/JSOP_CALL/JSOP_FUNCALL/JSOP_FUNAPPLY case; - in stubs::ScriptDebugPrologue, which JM epilogues call; and - in the REJOIN_THIS_PROTOTYPE case in js_InternalInterpret (you must add a JS_GC call to ic::GetPropNoCache for Debugger-onEnterFrame-resumption-05.js to hit this reliably). We also rearrange the js_InternalThrow JSTrapStatus-handling switch statement to have an explicit default case that raises a JS_NOT_REACHED assertion, instead of just omitting JSTRAP_CONTINUE.
js/src/jit-test/tests/debug/Debugger-onEnterFrame-resumption-01.js
js/src/jit-test/tests/debug/Debugger-onEnterFrame-resumption-02.js
js/src/jit-test/tests/debug/Debugger-onEnterFrame-resumption-03.js
js/src/jit-test/tests/debug/Debugger-onEnterFrame-resumption-04.js
js/src/jit-test/tests/debug/Debugger-onEnterFrame-resumption-05.js
js/src/jit-test/tests/debug/onEnterFrame-04.js
js/src/jsdbgapi.cpp
js/src/jsinterp.cpp
js/src/jsinterp.h
js/src/methodjit/InvokeHelpers.cpp
js/src/vm/Debugger.cpp
js/src/vm/Debugger.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/Debugger-onEnterFrame-resumption-01.js
@@ -0,0 +1,45 @@
+// If debugger.onEnterFrame returns {return:val}, the frame returns.
+
+var g = newGlobal('new-compartment');
+g.set = false;
+g.eval("function f() {\n" +
+       "    set = true;\n" +
+       "    return 'fail';\n" +
+       "}\n");
+g.eval("function g() { return 'g ' + f(); }");
+g.eval("function h() { return 'h ' + g(); }");
+
+var dbg = Debugger(g);
+var savedFrame;
+dbg.onEnterFrame = function (frame) {
+    savedFrame = frame;
+    return {return: "pass"};
+};
+
+// Call g.f as a function.
+savedFrame = undefined;
+assertEq(g.f(), "pass");
+assertEq(savedFrame.live, false);
+assertEq(g.set, false);
+
+// Call g.f as a constructor.
+savedFrame = undefined;
+var r = new g.f;
+assertEq(typeof r, "object");
+assertEq(savedFrame.live, false);
+assertEq(g.set, false);
+
+var count = 0;
+dbg.onEnterFrame = function (frame) {
+    count++;
+    if (count == 3) {
+        savedFrame = frame;
+        return {return: "pass"};
+    }
+    return undefined;
+};
+g.set = false;
+savedFrame = undefined;
+assertEq(g.h(), "h g pass");
+assertEq(savedFrame.live, false);
+assertEq(g.set, false);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/Debugger-onEnterFrame-resumption-02.js
@@ -0,0 +1,28 @@
+// If debugger.onEnterFrame returns {throw:val}, an exception is thrown in the debuggee.
+
+load(libdir + "asserts.js");
+
+var g = newGlobal('new-compartment');
+g.set = false;
+g.eval("function f() {\n" +
+       "    set = true;\n" +
+       "    return 'fail';\n" +
+       "}\n");
+
+var dbg = Debugger(g);
+var savedFrame;
+dbg.onEnterFrame = function (frame) {
+    savedFrame = frame;
+    return {throw: "pass"};
+};
+
+savedFrame = undefined;
+assertThrowsValue(g.f, "pass");
+assertEq(savedFrame.live, false);
+assertEq(g.set, false);
+
+savedFrame = undefined;
+assertThrowsValue(function () { new g.f; }, "pass");
+assertEq(savedFrame.live, false);
+assertEq(g.set, false);
+
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/Debugger-onEnterFrame-resumption-03.js
@@ -0,0 +1,26 @@
+// If debugger.onEnterFrame returns {return:val}, the frame returns immediately.
+
+load(libdir + "asserts.js");
+
+var g = newGlobal('new-compartment');
+g.set = false;
+
+var dbg = Debugger(g);
+var savedFrame;
+dbg.onDebuggerStatement = function (frame) {
+    var innerSavedFrame;
+    dbg.onEnterFrame = function (frame) {
+        innerSavedFrame = frame;
+        return null;
+    };
+    // Using frame.eval lets us catch termination.  
+    assertEq(frame.eval("set = true;"), null);
+    assertEq(innerSavedFrame.live, false);
+    savedFrame = frame;
+    return { return: "pass" };
+};
+
+savedFrame = undefined;
+assertEq(g.eval("debugger;"), "pass");
+assertEq(savedFrame.live, false);
+assertEq(g.set, false);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/Debugger-onEnterFrame-resumption-04.js
@@ -0,0 +1,16 @@
+// If debugger.onEnterFrame returns undefined, the frame should continue execution.
+
+var g = newGlobal('new-compartment');
+var dbg = Debugger(g);
+var hits = 0;
+var savedFrame;
+dbg.onEnterFrame = function (frame) {
+    hits++;
+    savedFrame = frame;
+    return undefined;
+};
+
+savedFrame = undefined;
+assertEq(g.eval("'pass';"), "pass");
+assertEq(savedFrame.live, false);
+assertEq(hits, 1);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/Debugger-onEnterFrame-resumption-05.js
@@ -0,0 +1,98 @@
+// Exercise the call to ScriptDebugPrologue in js_InternalInterpret.
+
+// This may change, but as of this writing, inline caches (ICs) are
+// disabled in debug mode, and those are the only users of the out-of-line entry
+// points for JIT code (arityCheckEntry, argsCheckEntry, fastEntry); debug
+// mode uses only invokeEntry. This means most of the bytecode tails in
+// js_InternalInterpret that might call ScriptPrologue or ScriptEpilogue are
+// unreachable in debug mode: they're only called from the out-of-line entry
+// points.
+//
+// The exception is REJOIN_THIS_PROTOTYPE, which can be reached reliably if you
+// add a JS_GC call to stubs::GetPropNoCache. JIT code calls that stub to
+// retrieve the 'prototype' property of a function called as a constructor, if
+// TI can't establish the exact identity of that prototype's value at compile
+// time. Thus the preoccupation with constructors here.
+
+load(libdir + "asserts.js");
+
+var debuggee = newGlobal('new-compartment');
+var dbg = Debugger(debuggee);
+var hits, savedFrame;
+
+// Allow the constructor to return normally.
+dbg.onEnterFrame = function (frame) {
+    hits++;
+    if (frame.constructing) {
+        savedFrame = frame;
+        assertEq(savedFrame.live, true);
+        return undefined;
+    }
+    return undefined;
+};
+hits = 0;
+debuggee.hits = 0;
+savedFrame = undefined;
+assertEq(typeof debuggee.eval("function f(){ hits++; } f.prototype = {}; new f;"), "object");
+assertEq(hits, 2);
+assertEq(savedFrame.live, false);
+assertEq(debuggee.hits, 1);
+
+// Force an early return from the constructor.
+dbg.onEnterFrame = function (frame) {
+    hits++;
+    if (frame.constructing) {
+        savedFrame = frame;
+        assertEq(savedFrame.live, true);
+        return { return: "pass" };
+    }
+    return undefined;
+};
+hits = 0;
+debuggee.hits = 0;
+savedFrame = undefined;
+assertEq(typeof debuggee.eval("function f(){ hits++; } f.prototype = {}; new f;"), "object");
+assertEq(hits, 2);
+assertEq(savedFrame.live, false);
+assertEq(debuggee.hits, 0);
+
+// Force the constructor to throw an exception.
+dbg.onEnterFrame = function (frame) {
+    hits++;
+    if (frame.constructing) {
+        savedFrame = frame;
+        assertEq(savedFrame.live, true);
+        return { throw: "pass" };
+    }
+    return undefined;
+};
+hits = 0;
+debuggee.hits = 0;
+savedFrame = undefined;
+assertThrowsValue(function () {
+                      debuggee.eval("function f(){ hits++ } f.prototype = {}; new f;");
+                  }, "pass");
+assertEq(hits, 2);
+assertEq(savedFrame.live, false);
+assertEq(debuggee.hits, 0);
+
+// Ensure that forcing an early return only returns from one JS call.
+debuggee.eval("function g() { var result = new f; g_hits++; return result; }");
+dbg.onEnterFrame = function (frame) {
+    hits++;
+    if (frame.constructing) {
+        savedFrame = frame;
+        assertEq(savedFrame.live, true);
+        return { return: "pass" };
+    }
+    return undefined;
+};
+hits = 0;
+debuggee.hits = 0;
+debuggee.g_hits = 0;
+savedFrame = undefined;
+assertEq(typeof debuggee.eval("g();"), "object");
+assertEq(hits, 3);
+assertEq(savedFrame.live, false);
+assertEq(debuggee.hits, 0);
+assertEq(debuggee.g_hits, 1);
--- a/js/src/jit-test/tests/debug/onEnterFrame-04.js
+++ b/js/src/jit-test/tests/debug/onEnterFrame-04.js
@@ -1,17 +1,49 @@
-// We detect and stop the runaway recursion caused by making onEnterFrame a wrapper of a debuggee function.
+// We detect and stop the runaway recursion caused by making onEnterFrame a
+// wrapper of a debuggee function.
+
+// This is all a bit silly. In any reasonable design, both debugger re-entry
+// (the second onEnterFrame invocation) and debuggee re-entry (the call to g.f
+// from within the debugger, not via a Debugger invocation function) would raise
+// errors immediately. We have plans to do so, but in the mean time, we settle
+// for at least detecting the recursion.
+
+load(libdir + 'asserts.js');
 
 var g = newGlobal('new-compartment');
+g.eval("function f(frame) { n++; return 42; }");
 g.n = 0;
-g.eval("function f(frame) { n++; return 42; }");
-print('ok');
+
 var dbg = Debugger(g);
+
+// Register the debuggee function as the onEnterFrame handler. When we first
+// call or eval in the debuggee:
+//
+// - The onEnterFrame call reporting that frame's creation is itself an event
+//   that must be reported, so we call onEnterFrame again.
+// 
+// - SpiderMonkey detects the out-of-control recursion, and generates a "too
+//   much recursion" InternalError in the youngest onEnterFrame call.
+//
+// - We don't catch it, so the onEnterFrame handler call itself throws.
+//
+// - Since the Debugger doesn't have an uncaughtExceptionHook (it can't; such a
+//   hook would itself raise a "too much recursion" exception), Spidermonkey
+//   reports the exception immediately and terminates the debuggee --- which is
+//   the next-older onEnterFrame call.
+//
+// - This termination propagates all the way out to the initial attempt to
+//   create a frame in the debuggee.
 dbg.onEnterFrame = g.f;
 
-// Since enterFrame cannot throw, the InternalError is reported and execution proceeds.
-var x = g.f();
-assertEq(x, 42);
-assertEq(g.n > 20, true);
+// Get a Debugger.Object instance referring to f.
+var debuggeeF = dbg.addDebuggee(g.f);
+
+// Using f.call allows us to catch the termination.
+assertEq(debuggeeF.call(), null);
 
-// When an error is reported, the shell usually exits with a nonzero exit
-// code. If we get here, the test passed, so override that behavior.
+// We should never actually begin execution of the function.
+assertEq(g.n, 0);
+
+// When an error is reported, the shell usually exits with a nonzero exit code.
+// If we get here, the test passed, so override that behavior.
 quit(0);
--- a/js/src/jsdbgapi.cpp
+++ b/js/src/jsdbgapi.cpp
@@ -103,29 +103,47 @@ JS_SetDebugMode(JSContext *cx, JSBool de
 JS_PUBLIC_API(void)
 JS_SetRuntimeDebugMode(JSRuntime *rt, JSBool debug)
 {
     rt->debugMode = !!debug;
 }
 
 namespace js {
 
-void
+JSTrapStatus
 ScriptDebugPrologue(JSContext *cx, StackFrame *fp)
 {
     JS_ASSERT(fp == cx->fp());
 
     if (fp->isFramePushedByExecute()) {
         if (JSInterpreterHook hook = cx->debugHooks->executeHook)
             fp->setHookData(hook(cx, Jsvalify(fp), true, 0, cx->debugHooks->executeHookData));
     } else {
         if (JSInterpreterHook hook = cx->debugHooks->callHook)
             fp->setHookData(hook(cx, Jsvalify(fp), true, 0, cx->debugHooks->callHookData));
     }
-    Debugger::onEnterFrame(cx);
+
+    Value rval;
+    JSTrapStatus status = Debugger::onEnterFrame(cx, &rval);
+    switch (status) {
+      case JSTRAP_CONTINUE:
+        break;
+      case JSTRAP_THROW:
+        cx->setPendingException(rval);
+        break;
+      case JSTRAP_ERROR:
+        cx->clearPendingException();
+        break;
+      case JSTRAP_RETURN:
+        fp->setReturnValue(rval);
+        break;
+      default:
+        JS_NOT_REACHED("bad Debugger::onEnterFrame JSTrapStatus value");
+    }
+    return status;
 }
 
 bool
 ScriptDebugEpilogue(JSContext *cx, StackFrame *fp, bool okArg)
 {
     JS_ASSERT(fp == cx->fp());
     JSBool ok = okArg;
 
--- a/js/src/jsinterp.cpp
+++ b/js/src/jsinterp.cpp
@@ -1779,40 +1779,53 @@ js::Interpret(JSContext *cx, StackFrame 
          * To support generator_throw and to catch ignored exceptions,
          * fail if cx->isExceptionPending() is true.
          */
         if (cx->isExceptionPending())
             goto error;
     }
 #endif
 
+    /* State communicated between non-local jumps: */
+    JSBool interpReturnOK;
+    JSAtom *atomNotDefined;
+
     /* Don't call the script prologue if executing between Method and Trace JIT. */
     if (interpMode == JSINTERP_NORMAL) {
         StackFrame *fp = regs.fp();
         JS_ASSERT_IF(!fp->isGeneratorFrame(), regs.pc == script->code);
         if (!ScriptPrologueOrGeneratorResume(cx, fp, UseNewTypeAtEntry(cx, fp)))
             goto error;
-        if (cx->compartment->debugMode())
-            ScriptDebugPrologue(cx, fp);
+        if (cx->compartment->debugMode()) {
+            JSTrapStatus status = ScriptDebugPrologue(cx, fp);
+            switch (status) {
+              case JSTRAP_CONTINUE:
+                break;
+              case JSTRAP_RETURN:
+                interpReturnOK = JS_TRUE;
+                goto forced_return;
+              case JSTRAP_THROW:
+              case JSTRAP_ERROR:
+                goto error;
+              default:
+                JS_NOT_REACHED("bad ScriptDebugPrologue status");
+            }
+        }
     }
 
     /* The REJOIN mode acts like the normal mode, except the prologue is skipped. */
     if (interpMode == JSINTERP_REJOIN)
         interpMode = JSINTERP_NORMAL;
 
     JS_ASSERT_IF(interpMode == JSINTERP_SKIP_TRAP, JSOp(*regs.pc) == JSOP_TRAP);
 
     CHECK_INTERRUPT_HANDLER();
 
     RESET_USE_METHODJIT();
 
-    /* State communicated between non-local jumps: */
-    JSBool interpReturnOK;
-    JSAtom *atomNotDefined;
-
     /*
      * It is important that "op" be initialized before calling DO_OP because
      * it is possible for "op" to be specially assigned during the normal
      * processing of an opcode while looping. We rely on DO_NEXT_OP to manage
      * "op" correctly in all other cases.
      */
     JSOp op;
     jsint len;
@@ -3508,18 +3521,30 @@ BEGIN_CASE(JSOP_FUNAPPLY)
             goto jit_return;
         }
     }
 #endif
 
     if (!ScriptPrologue(cx, regs.fp(), newType))
         goto error;
 
-    if (cx->compartment->debugMode())
-        ScriptDebugPrologue(cx, regs.fp());
+    if (cx->compartment->debugMode()) {
+        switch (ScriptDebugPrologue(cx, regs.fp())) {
+          case JSTRAP_CONTINUE:
+            break;
+          case JSTRAP_RETURN:
+            interpReturnOK = JS_TRUE;
+            goto forced_return;
+          case JSTRAP_THROW:
+          case JSTRAP_ERROR:
+            goto error;
+          default:
+            JS_NOT_REACHED("bad ScriptDebugPrologue status");
+        }
+    }
 
     CHECK_INTERRUPT_HANDLER();
 
     /* Load first op and dispatch it (safe since JSOP_STOP). */
     op = (JSOp) *regs.pc;
     DO_OP();
 }
 
--- a/js/src/jsinterp.h
+++ b/js/src/jsinterp.h
@@ -95,17 +95,34 @@ ScriptEpilogue(JSContext *cx, StackFrame
 inline bool
 ScriptPrologueOrGeneratorResume(JSContext *cx, StackFrame *fp);
 
 inline bool
 ScriptEpilogueOrGeneratorYield(JSContext *cx, StackFrame *fp, bool ok);
 
 /* Implemented in jsdbgapi: */
 
-extern void
+/*
+ * Announce to the debugger that the thread has entered a new JavaScript frame,
+ * |fp|. Call whatever hooks have been registered to observe new frames, and
+ * return a JSTrapStatus code indication how execution should proceed:
+ *
+ * - JSTRAP_CONTINUE: Continue execution normally.
+ * 
+ * - JSTRAP_THROW: Throw an exception. ScriptDebugPrologue has set |cx|'s
+ *   pending exception to the value to be thrown.
+ *
+ * - JSTRAP_ERROR: Terminate execution (as is done when a script is terminated
+ *   for running too long). ScriptDebugPrologue has cleared |cx|'s pending
+ *   exception.
+ *
+ * - JSTRAP_RETURN: Return from the new frame immediately. ScriptDebugPrologue
+ *   has set |cx->fp()|'s return value appropriately.
+ */
+extern JSTrapStatus
 ScriptDebugPrologue(JSContext *cx, StackFrame *fp);
 
 extern bool
 ScriptDebugEpilogue(JSContext *cx, StackFrame *fp, bool ok);
 
 /*
  * For a given |call|, convert null/undefined |this| into the global object for
  * the callee and replace other primitives with boxed versions. This assumes
--- a/js/src/methodjit/InvokeHelpers.cpp
+++ b/js/src/methodjit/InvokeHelpers.cpp
@@ -545,27 +545,30 @@ js_InternalThrow(VMFrame &f)
                                  cx->debugHooks->throwHookData);
                 }
 
                 switch (st) {
                 case JSTRAP_ERROR:
                     cx->clearPendingException();
                     return NULL;
 
+                case JSTRAP_CONTINUE:
+                  break;
+
                 case JSTRAP_RETURN:
                     cx->clearPendingException();
                     cx->fp()->setReturnValue(rval);
                     return cx->jaegerCompartment()->forceReturnFromExternC();
 
                 case JSTRAP_THROW:
                     cx->setPendingException(rval);
                     break;
 
                 default:
-                    break;
+                    JS_NOT_REACHED("bad onExceptionUnwind status");
                 }
             }
         }
 
         pc = FindExceptionHandler(cx);
         if (pc)
             break;
 
@@ -653,17 +656,29 @@ stubs::CreateThis(VMFrame &f, JSObject *
         THROW();
     fp->formalArgs()[-1].setObject(*obj);
 }
 
 void JS_FASTCALL
 stubs::ScriptDebugPrologue(VMFrame &f)
 {
     Probes::enterJSFun(f.cx, f.fp()->maybeFun(), f.fp()->script());
-    js::ScriptDebugPrologue(f.cx, f.fp());
+    JSTrapStatus status = js::ScriptDebugPrologue(f.cx, f.fp());
+    switch (status) {
+      case JSTRAP_CONTINUE:
+        break;
+      case JSTRAP_RETURN:
+        *f.returnAddressLocation() = f.cx->jaegerCompartment()->forceReturnFromFastCall();
+        return;
+      case JSTRAP_ERROR:
+      case JSTRAP_THROW:
+        THROW();
+      default:
+        JS_NOT_REACHED("bad ScriptDebugPrologue status");
+    }
 }
 
 void JS_FASTCALL
 stubs::ScriptDebugEpilogue(VMFrame &f)
 {
     Probes::exitJSFun(f.cx, f.fp()->maybeFun(), f.fp()->script());
     if (!js::ScriptDebugEpilogue(f.cx, f.fp(), JS_TRUE))
         THROW();
@@ -888,18 +903,35 @@ js_InternalInterpret(void *returnData, v
       case REJOIN_THIS_PROTOTYPE: {
         JSObject *callee = &fp->callee();
         JSObject *proto = f.regs.sp[0].isObject() ? &f.regs.sp[0].toObject() : NULL;
         JSObject *obj = js_CreateThisForFunctionWithProto(cx, callee, proto);
         if (!obj)
             return js_InternalThrow(f);
         fp->formalArgs()[-1].setObject(*obj);
 
-        if (script->debugMode || Probes::callTrackingActive(cx))
-            js::ScriptDebugPrologue(cx, fp);
+        if (Probes::callTrackingActive(cx))
+            Probes::enterJSFun(f.cx, f.fp()->maybeFun(), f.fp()->script());
+
+        if (script->debugMode) {
+            JSTrapStatus status = js::ScriptDebugPrologue(f.cx, f.fp());
+            switch (status) {
+              case JSTRAP_CONTINUE:
+                break;
+              case JSTRAP_RETURN:
+                *f.returnAddressLocation() = f.cx->jaegerCompartment()->forceReturnFromExternC();
+                return NULL;
+              case JSTRAP_THROW:
+              case JSTRAP_ERROR:
+                return js_InternalThrow(f);
+              default:
+                JS_NOT_REACHED("bad ScriptDebugPrologue status");
+            }
+        }
+
         break;
       }
 
       case REJOIN_CHECK_ARGUMENTS:
         /*
          * Do all the work needed in arity check JIT prologues after the
          * arguments check occurs (FixupArity has been called if needed, but
          * the stack check and late prologue have not been performed.
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -412,37 +412,42 @@ Debugger::hasAnyLiveHooks(JSContext *cx)
     for (FrameMap::Range r = frames.all(); !r.empty(); r.popFront()) {
         if (!r.front().value->getReservedSlot(JSSLOT_DEBUGFRAME_ONSTEP_HANDLER).isUndefined())
             return true;
     }
 
     return false;
 }
 
-void
-Debugger::slowPathOnEnterFrame(JSContext *cx)
+JSTrapStatus
+Debugger::slowPathOnEnterFrame(JSContext *cx, Value *vp)
 {
     /* Build the list of recipients. */
     AutoValueVector triggered(cx);
     GlobalObject *global = cx->fp()->scopeChain().getGlobal();
     if (GlobalObject::DebuggerVector *debuggers = global->getDebuggers()) {
         for (Debugger **p = debuggers->begin(); p != debuggers->end(); p++) {
             Debugger *dbg = *p;
             JS_ASSERT(dbg->observesFrame(cx->fp()));
             if (dbg->observesEnterFrame() && !triggered.append(ObjectValue(*dbg->toJSObject())))
-                return;
+                return JSTRAP_ERROR;
         }
     }
 
     /* Deliver the event, checking again as in dispatchHook. */
     for (Value *p = triggered.begin(); p != triggered.end(); p++) {
         Debugger *dbg = Debugger::fromJSObject(&p->toObject());
-        if (dbg->debuggees.has(global) && dbg->observesEnterFrame())
-            dbg->fireEnterFrame(cx);
+        if (dbg->debuggees.has(global) && dbg->observesEnterFrame()) {
+            JSTrapStatus status = dbg->fireEnterFrame(cx, vp);
+            if (status != JSTRAP_CONTINUE)
+                return status;
+        }
     }
+
+    return JSTRAP_CONTINUE;
 }
 
 void
 Debugger::slowPathOnLeaveFrame(JSContext *cx)
 {
     StackFrame *fp = cx->fp();
     GlobalObject *global = fp->scopeChain().getGlobal();
 
@@ -704,36 +709,35 @@ Debugger::fireExceptionUnwind(JSContext 
     Value rv;
     bool ok = Invoke(cx, ObjectValue(*object), ObjectValue(*hook), 2, argv, &rv);
     JSTrapStatus st = parseResumptionValue(ac, ok, rv, vp);
     if (st == JSTRAP_CONTINUE)
         cx->setPendingException(exc);
     return st;
 }
 
-void
-Debugger::fireEnterFrame(JSContext *cx)
+JSTrapStatus
+Debugger::fireEnterFrame(JSContext *cx, Value *vp)
 {
     JSObject *hook = getHook(OnEnterFrame);
     JS_ASSERT(hook);
     JS_ASSERT(hook->isCallable());
 
     StackFrame *fp = cx->fp();
     AutoCompartment ac(cx, object);
     if (!ac.enter())
-        return;
+        return JSTRAP_ERROR;
 
     Value argv[1];
-    if (!getScriptFrame(cx, fp, &argv[0])) {
-        handleUncaughtException(ac, NULL, false);
-        return;
-    }
+    if (!getScriptFrame(cx, fp, &argv[0]))
+        return handleUncaughtException(ac, vp, false);
+
     Value rv;
-    if (!Invoke(cx, ObjectValue(*object), ObjectValue(*hook), 1, argv, &rv))
-        handleUncaughtException(ac, NULL, true);
+    bool ok = Invoke(cx, ObjectValue(*object), ObjectValue(*hook), 1, argv, &rv);
+    return parseResumptionValue(ac, ok, rv, vp);
 }
 
 void
 Debugger::fireNewScript(JSContext *cx, JSScript *script)
 {
     JSObject *hook = getHook(OnNewScript);
     JS_ASSERT(hook);
     JS_ASSERT(hook->isCallable());
--- a/js/src/vm/Debugger.h
+++ b/js/src/vm/Debugger.h
@@ -194,25 +194,25 @@ class Debugger {
     static JSBool clearAllBreakpoints(JSContext *cx, uintN argc, Value *vp);
     static JSBool construct(JSContext *cx, uintN argc, Value *vp);
     static JSPropertySpec properties[];
     static JSFunctionSpec methods[];
 
     JSObject *getHook(Hook hook) const;
     bool hasAnyLiveHooks(JSContext *cx) const;
 
-    static void slowPathOnEnterFrame(JSContext *cx);
+    static JSTrapStatus slowPathOnEnterFrame(JSContext *cx, Value *vp);
     static void slowPathOnLeaveFrame(JSContext *cx);
     static void slowPathOnNewScript(JSContext *cx, JSScript *script,
                                     GlobalObject *compileAndGoGlobal);
     static JSTrapStatus dispatchHook(JSContext *cx, Value *vp, Hook which);
 
     JSTrapStatus fireDebuggerStatement(JSContext *cx, Value *vp);
     JSTrapStatus fireExceptionUnwind(JSContext *cx, Value *vp);
-    void fireEnterFrame(JSContext *cx);
+    JSTrapStatus fireEnterFrame(JSContext *cx, Value *vp);
 
     /*
      * Allocate and initialize a Debugger.Script instance whose referent is
      * |script|.
      */
     JSObject *newDebuggerScript(JSContext *cx, JSScript *script);
 
     /*
@@ -251,17 +251,17 @@ class Debugger {
      * them and returns true. If not, it returns false.
      */
     static void markCrossCompartmentDebuggerObjectReferents(JSTracer *tracer);
     static bool markAllIteratively(GCMarker *trc);
     static void sweepAll(JSContext *cx);
     static void detachAllDebuggersFromGlobal(JSContext *cx, GlobalObject *global,
                                              GlobalObjectSet::Enum *compartmentEnum);
 
-    static inline void onEnterFrame(JSContext *cx);
+    static inline JSTrapStatus onEnterFrame(JSContext *cx, Value *vp);
     static inline void onLeaveFrame(JSContext *cx);
     static inline JSTrapStatus onDebuggerStatement(JSContext *cx, Value *vp);
     static inline JSTrapStatus onExceptionUnwind(JSContext *cx, Value *vp);
     static inline void onNewScript(JSContext *cx, JSScript *script,
                                    GlobalObject *compileAndGoGlobal);
     static JSTrapStatus onTrap(JSContext *cx, Value *vp);
     static JSTrapStatus onSingleStep(JSContext *cx, Value *vp);
 
@@ -470,21 +470,22 @@ Debugger::observesGlobal(GlobalObject *g
 }
 
 bool
 Debugger::observesFrame(StackFrame *fp) const
 {
     return observesGlobal(fp->scopeChain().getGlobal());
 }
 
-void
-Debugger::onEnterFrame(JSContext *cx)
+JSTrapStatus
+Debugger::onEnterFrame(JSContext *cx, Value *vp)
 {
-    if (!cx->compartment->getDebuggees().empty())
-        slowPathOnEnterFrame(cx);
+    if (cx->compartment->getDebuggees().empty())
+        return JSTRAP_CONTINUE;
+    return slowPathOnEnterFrame(cx, vp);
 }
 
 void
 Debugger::onLeaveFrame(JSContext *cx)
 {
     if (!cx->compartment->getDebuggees().empty() || !cx->compartment->breakpointSites.empty())
         slowPathOnLeaveFrame(cx);
 }