Bug 848665 - Don't use ScriptFrameIter in onEnterFrame/onLeaveFrame hooks. r=jimb

js/src/jsdbgapi.cpp

@@ -95,7 +95,7 @@ js::ScriptDebugPrologue(JSContext *cx, AbstractFramePtr frame)
     }
 
     RootedValue rval(cx);
-    JSTrapStatus status = Debugger::onEnterFrame(cx, &rval);
+    JSTrapStatus status = Debugger::onEnterFrame(cx, frame, &rval);
     switch (status) {
       case JSTRAP_CONTINUE:
         break;
@@ -130,7 +130,7 @@ js::ScriptDebugEpilogue(JSContext *cx, AbstractFramePtr frame, bool okArg)
         }
     }
 
-    return Debugger::onLeaveFrame(cx, ok);
+    return Debugger::onLeaveFrame(cx, frame, ok);
 }
 
 JSTrapStatus

js/src/vm/Debugger.cpp

@@ -487,19 +487,21 @@ Debugger::hasAnyLiveHooks() const
 }
 
 JSTrapStatus
-Debugger::slowPathOnEnterFrame(JSContext *cx, MutableHandleValue vp)
+Debugger::slowPathOnEnterFrame(JSContext *cx, AbstractFramePtr frame, MutableHandleValue vp)
 {
     /* Build the list of recipients. */
     AutoValueVector triggered(cx);
     Handle<GlobalObject*> global = cx->global();
 
+#ifdef DEBUG
     ScriptFrameIter iter(cx);
-    JS_ASSERT(!iter.done());
+    JS_ASSERT(iter.abstractFramePtr() == frame);
+#endif
 
     if (GlobalObject::DebuggerVector *debuggers = global->getDebuggers()) {
         for (Debugger **p = debuggers->begin(); p != debuggers->end(); p++) {
             Debugger *dbg = *p;
-            JS_ASSERT(dbg->observesFrame(iter.abstractFramePtr()));
+            JS_ASSERT(dbg->observesFrame(frame));
             if (dbg->observesEnterFrame() && !triggered.append(ObjectValue(*dbg->toJSObject())))
                 return JSTRAP_ERROR;
         }
@@ -527,21 +529,23 @@ DebuggerFrame_freeStackIterData(FreeOp *fop, RawObject obj);
  * |cx->fp()|'s return value, and return a new success value.
  */
 bool
-Debugger::slowPathOnLeaveFrame(JSContext *cx, bool frameOk)
+Debugger::slowPathOnLeaveFrame(JSContext *cx, AbstractFramePtr frame, bool frameOk)
 {
+#ifdef DEBUG
     ScriptFrameIter iter(cx);
-    JS_ASSERT(!iter.done());
+    JS_ASSERT(iter.abstractFramePtr() == frame);
+#endif
 
     Handle<GlobalObject*> global = cx->global();
 
     /* Save the frame's completion value. */
     JSTrapStatus status;
     RootedValue value(cx);
-    Debugger::resultToCompletion(cx, frameOk, iter.returnValue(), &status, &value);
+    Debugger::resultToCompletion(cx, frameOk, frame.returnValue(), &status, &value);
 
     /* Build a list of the recipients. */
     AutoObjectVector frames(cx);
-    for (FrameRange r(iter.abstractFramePtr(), global); !r.empty(); r.popFront()) {
+    for (FrameRange r(frame, global); !r.empty(); r.popFront()) {
         if (!frames.append(r.frontFrame())) {
             cx->clearPendingException();
             return false;
@@ -593,7 +597,7 @@ Debugger::slowPathOnLeaveFrame(JSContext *cx, bool frameOk)
      * debugger's onPop handler could have caused another debugger to create its
      * own Debugger.Frame instance.
      */
-    for (FrameRange r(iter.abstractFramePtr(), global); !r.empty(); r.popFront()) {
+    for (FrameRange r(frame, global); !r.empty(); r.popFront()) {
         RootedObject frameobj(cx, r.frontFrame());
         Debugger *dbg = r.frontDebugger();
         JS_ASSERT(dbg == Debugger::fromChildJSObject(frameobj));
@@ -602,28 +606,28 @@ Debugger::slowPathOnLeaveFrame(JSContext *cx, bool frameOk)
 
         /* If this frame had an onStep handler, adjust the script's count. */
         if (!frameobj->getReservedSlot(JSSLOT_DEBUGFRAME_ONSTEP_HANDLER).isUndefined() &&
-            !iter.script()->changeStepModeCount(cx, -1))
+            !frame.script()->changeStepModeCount(cx, -1))
         {
             status = JSTRAP_ERROR;
             /* Don't exit the loop; we must mark all frames as dead. */
         }
 
-        dbg->frames.remove(iter.abstractFramePtr());
+        dbg->frames.remove(frame);
     }
 
     /*
      * If this is an eval frame, then from the debugger's perspective the
      * script is about to be destroyed. Remove any breakpoints in it.
      */
-    if (iter.isEvalFrame()) {
-        RootedScript script(cx, iter.script());
+    if (frame.isEvalFrame()) {
+        RootedScript script(cx, frame.script());
         script->clearBreakpointsIn(cx->runtime->defaultFreeOp(), NULL, NULL);
     }
 
     /* Establish (status, value) as our resumption value. */
     switch (status) {
       case JSTRAP_RETURN:
-        iter.setReturnValue(value);
+        frame.setReturnValue(value);
         return true;
 
       case JSTRAP_THROW:

js/src/vm/Debugger.h

@@ -330,8 +330,9 @@ class Debugger : private mozilla::LinkedListElement<Debugger>
     JSObject *getHook(Hook hook) const;
     bool hasAnyLiveHooks() const;
 
-    static JSTrapStatus slowPathOnEnterFrame(JSContext *cx, MutableHandleValue vp);
-    static bool slowPathOnLeaveFrame(JSContext *cx, bool ok);
+    static JSTrapStatus slowPathOnEnterFrame(JSContext *cx, AbstractFramePtr frame,
+                                             MutableHandleValue vp);
+    static bool slowPathOnLeaveFrame(JSContext *cx, AbstractFramePtr frame, bool ok);
     static void slowPathOnNewScript(JSContext *cx, HandleScript script,
                                     GlobalObject *compileAndGoGlobal);
     static bool slowPathOnNewGlobalObject(JSContext *cx, Handle<GlobalObject *> global);
@@ -393,8 +394,9 @@ class Debugger : private mozilla::LinkedListElement<Debugger>
     static bool isDebugWrapper(RawObject o);
     static void findCompartmentEdges(JS::Zone *v, gc::ComponentFinder<JS::Zone> &finder);
 
-    static inline JSTrapStatus onEnterFrame(JSContext *cx, MutableHandleValue vp);
-    static inline bool onLeaveFrame(JSContext *cx, bool ok);
+    static inline JSTrapStatus onEnterFrame(JSContext *cx, AbstractFramePtr frame,
+                                            MutableHandleValue vp);
+    static inline bool onLeaveFrame(JSContext *cx, AbstractFramePtr frame, bool ok);
     static inline JSTrapStatus onDebuggerStatement(JSContext *cx, MutableHandleValue vp);
     static inline JSTrapStatus onExceptionUnwind(JSContext *cx, MutableHandleValue vp);
     static inline void onNewScript(JSContext *cx, HandleScript script,
@@ -645,21 +647,21 @@ Debugger::observesFrame(AbstractFramePtr frame) const
 }
 
 JSTrapStatus
-Debugger::onEnterFrame(JSContext *cx, MutableHandleValue vp)
+Debugger::onEnterFrame(JSContext *cx, AbstractFramePtr frame, MutableHandleValue vp)
 {
     if (cx->compartment->getDebuggees().empty())
         return JSTRAP_CONTINUE;
-    return slowPathOnEnterFrame(cx, vp);
+    return slowPathOnEnterFrame(cx, frame, vp);
 }
 
 bool
-Debugger::onLeaveFrame(JSContext *cx, bool ok)
+Debugger::onLeaveFrame(JSContext *cx, AbstractFramePtr frame, bool ok)
 {
     /* Traps must be cleared from eval frames, see slowPathOnLeaveFrame. */
-    bool evalTraps = cx->fp()->isEvalFrame() &&
-                     cx->fp()->script()->hasAnyBreakpointsOrStepMode();
+    bool evalTraps = frame.isEvalFrame() &&
+                     frame.script()->hasAnyBreakpointsOrStepMode();
     if (!cx->compartment->getDebuggees().empty() || evalTraps)
-        ok = slowPathOnLeaveFrame(cx, ok);
+        ok = slowPathOnLeaveFrame(cx, frame, ok);
     return ok;
 }
 

js/src/vm/Stack-inl.h

@@ -600,6 +600,14 @@ AbstractFramePtr::setHookData(void *data) const
     JS_NOT_REACHED("Invalid frame");
 }
 
+inline Value
+AbstractFramePtr::returnValue() const
+{
+    if (isStackFrame())
+        return asStackFrame()->returnValue();
+    JS_NOT_REACHED("Invalid frame");
+}
+
 inline void
 AbstractFramePtr::setReturnValue(const Value &rval) const
 {

js/src/vm/Stack.h

@@ -313,6 +313,7 @@ class AbstractFramePtr
 
     inline void *maybeHookData() const;
     inline void setHookData(void *data) const;
+    inline Value returnValue() const;
     inline void setReturnValue(const Value &rval) const;
 
     inline void popBlock(JSContext *cx) const;