Bug 659577 - Remove ScopeObject::maybeStackFrame use in the debugger, part 1 (r=jimb)
authorLuke Wagner <luke@mozilla.com>
Wed, 02 May 2012 09:56:02 -0700
changeset 100586 d657e8cb0201ffe090bb78234fab19147dd4373b
parent 100585 577a88fc97b6247c92c1301d5eea16f81702ab7d
child 100587 2ff46668b15607f08d7ed8db975729eacbb0d8a2
push idunknown
push userunknown
push dateunknown
reviewersjimb
bugs659577
milestone15.0a1
Bug 659577 - Remove ScopeObject::maybeStackFrame use in the debugger, part 1 (r=jimb)
js/src/jsinterp.cpp
js/src/vm/ScopeObject.cpp
js/src/vm/ScopeObject.h
js/src/vm/Stack.cpp
js/src/vm/Stack.h
--- a/js/src/jsinterp.cpp
+++ b/js/src/jsinterp.cpp
@@ -460,18 +460,21 @@ js::ExecuteKernel(JSContext *cx, JSScrip
         return false;
 
     Probes::startExecution(cx, script);
 
     TypeScript::SetThis(cx, script, fp->thisValue());
 
     bool ok = RunScript(cx, script, fp);
 
-    if (fp->isStrictEvalFrame())
+    if (fp->isStrictEvalFrame()) {
+        if (cx->compartment->debugMode())
+            cx->runtime->debugScopes->onPopStrictEvalScope(fp);
         js_PutCallObject(fp, fp->callObj());
+    }
 
     Probes::stopExecution(cx, script);
 
     /* Propgate the return value out. */
     if (result)
         *result = fp->returnValue();
     return ok;
 }
--- a/js/src/vm/ScopeObject.cpp
+++ b/js/src/vm/ScopeObject.cpp
@@ -1463,28 +1463,30 @@ js_IsDebugScopeSlow(const JSObject *obj)
     return obj->getClass() == &ObjectProxyClass &&
            GetProxyHandler(obj) == &DebugScopeProxy::singleton;
 }
 
 /*****************************************************************************/
 
 DebugScopes::DebugScopes(JSRuntime *rt)
  : proxiedScopes(rt),
-   missingScopes(rt)
+   missingScopes(rt),
+   liveScopes(rt)
 {}
 
 DebugScopes::~DebugScopes()
 {
     JS_ASSERT(missingScopes.empty());
 }
 
 bool
 DebugScopes::init()
 {
-    if (!proxiedScopes.init() ||
+    if (!liveScopes.init() ||
+        !proxiedScopes.init() ||
         !missingScopes.init())
     {
         return false;
     }
     return true;
 }
 
 void
@@ -1500,16 +1502,28 @@ DebugScopes::sweep()
      * Note: missingScopes points to debug scopes weakly not just so that debug
      * scopes can be released more eagerly, but, more importantly, to avoid
      * creating an uncollectable cycle with suspended generator frames.
      */
     for (MissingScopeMap::Enum e(missingScopes); !e.empty(); e.popFront()) {
         if (!IsObjectMarked(&e.front().value))
             e.removeFront();
     }
+
+    /*
+     * Since liveScopes includes entries for suspended generator frames which
+     * may be collected when the generator becomes unreachable we must sweep
+     * liveScopes for dead generator frames.
+     */
+    for (LiveScopeMap::Enum e(liveScopes); !e.empty(); e.popFront()) {
+        if (JS_IsAboutToBeFinalized(e.front().key)) {
+            JS_ASSERT(e.front().value->isGeneratorFrame());
+            e.removeFront();
+        }
+    }
 }
 
 /*
  * Unfortunately, GetDebugScopeForFrame needs to work even outside debug mode
  * (in particular, JS_GetFrameScopeChain does not require debug mode). Since
  * DebugScopes::onPop* are only called in debug mode, this means we cannot
  * use any of the maps in DebugScopes. This will produce debug scope chains
  * that do not obey the debugger invariants but that is just fine.
@@ -1569,44 +1583,79 @@ DebugScopes::addDebugScope(JSContext *cx
 }
 
 void
 DebugScopes::onPopCall(StackFrame *fp)
 {
     if (fp->isYielding())
         return;
 
-    if (!fp->fun()->isHeavyweight()) {
+    if (fp->fun()->isHeavyweight()) {
+        /*
+         * When a frame finishes executing in mjit code, the epilogue is called
+         * once from the return and once when the frame is popped.
+         * TODO: bug 659577 will remove this (with HAS_CALL_OBJ).
+         */
+        if (fp->hasCallObj())
+            liveScopes.remove(&fp->scopeChain()->asCall());
+    } else {
         JS_ASSERT(!fp->hasCallObj());
         if (MissingScopeMap::Ptr p = missingScopes.lookup(ScopeIter(fp))) {
             js_PutCallObject(fp, p->value->scope().asCall());
             missingScopes.remove(p);
         }
     }
 }
 
 void
 DebugScopes::onPopBlock(JSContext *cx, StackFrame *fp)
 {
     StaticBlockObject &block = *fp->maybeBlockChain();
-    if (!block.needsClone()) {
+    if (block.needsClone()) {
+        liveScopes.remove(&fp->scopeChain()->asClonedBlock());
+    } else {
         JS_ASSERT(!fp->scopeChain()->isBlock() ||
                   fp->scopeChain()->asClonedBlock().staticBlock() != block);
         if (MissingScopeMap::Ptr p = missingScopes.lookup(ScopeIter(fp))) {
             p->value->scope().asClonedBlock().put(fp);
             missingScopes.remove(p);
         }
     }
 }
 
 void
+DebugScopes::onPopWith(StackFrame *fp)
+{
+    liveScopes.remove(&fp->scopeChain()->asWith());
+}
+
+void
+DebugScopes::onPopStrictEvalScope(StackFrame *fp)
+{
+    liveScopes.remove(&fp->scopeChain()->asCall());
+}
+
+void
 DebugScopes::onGeneratorFrameChange(StackFrame *from, StackFrame *to)
 {
     for (ScopeIter toIter(to); !toIter.done(); toIter = toIter.enclosing()) {
-        if (!toIter.hasScopeObject()) {
+        if (toIter.hasScopeObject()) {
+            /*
+             * Not only must we correctly replace mappings [scope -> from] with
+             * mappings [scope -> to], but we must add [scope -> to] if it
+             * doesn't already exist so that if we need to proxy a generator's
+             * scope while it is suspended, we can find its frame (which would
+             * otherwise not be found by AllFramesIter).
+             */
+            LiveScopeMap::AddPtr livePtr = liveScopes.lookupForAdd(&toIter.scope());
+            if (livePtr)
+                livePtr->value = to;
+            else
+                liveScopes.add(livePtr, &toIter.scope(), to);
+        } else {
             if (MissingScopeMap::Ptr p = missingScopes.lookup(ScopeIter(toIter, from))) {
                 DebugScopeObject &debugScope = *p->value;
                 ScopeObject &scope = debugScope.scope();
                 if (scope.isCall()) {
                     JS_ASSERT(scope.maybeStackFrame() == from);
                     scope.setStackFrame(to);
                     if (scope.enclosingScope().isDeclEnv()) {
                         JS_ASSERT(scope.enclosingScope().asDeclEnv().maybeStackFrame() == from);
@@ -1622,16 +1671,69 @@ DebugScopes::onGeneratorFrameChange(Stac
 
 void
 DebugScopes::onCompartmentLeaveDebugMode(JSCompartment *c)
 {
     for (MissingScopeMap::Enum e(missingScopes); !e.empty(); e.popFront()) {
         if (e.front().key.fp()->compartment() == c)
             e.removeFront();
     }
+    for (LiveScopeMap::Enum e(liveScopes); !e.empty(); e.popFront()) {
+        if (e.front().key->compartment() == c)
+            e.removeFront();
+    }
+}
+
+bool
+DebugScopes::updateLiveScopes(JSContext *cx)
+{
+    JS_CHECK_RECURSION(cx, return false);
+
+    /*
+     * Note that we must always update the top frame's scope objects' entries
+     * in liveScopes because we can't be sure code hasn't run in that frame to
+     * change the scope chain since we were last called. The fp->prevUpToDate()
+     * flag indicates whether the scopes of frames older than fp are already
+     * included in liveScopes. It might seem simpler to have fp instead carry a
+     * flag indicating whether fp itself is accurately described, but then we
+     * would need to clear that flag whenever fp ran code. By storing the 'up
+     * to date' bit for fp->prev() in fp, simply popping fp effectively clears
+     * the flag for us, at exactly the time when execution resumes fp->prev().
+     */
+    for (AllFramesIter i(cx->runtime->stackSpace); !i.done(); ++i) {
+        StackFrame *fp = i.fp();
+        if (fp->isDummyFrame() || fp->scopeChain()->compartment() != cx->compartment)
+            continue;
+
+        for (ScopeIter si(fp); !si.done(); si = si.enclosing()) {
+            if (si.hasScopeObject() && !liveScopes.put(&si.scope(), fp))
+                return false;
+        }
+
+        if (fp->prevUpToDate())
+            return true;
+        JS_ASSERT(fp->compartment()->debugMode());
+        fp->setPrevUpToDate();
+    }
+
+    return true;
+}
+
+StackFrame *
+DebugScopes::hasLiveFrame(ScopeObject &scope)
+{
+    if (LiveScopeMap::Ptr p = liveScopes.lookup(&scope)) {
+        JS_ASSERT_IF(scope.isClonedBlock(),
+                     p->value == js_LiveFrameIfGenerator(scope.maybeStackFrame()));
+        JS_ASSERT_IF(scope.isCall(),
+                     p->value == scope.maybeStackFrame());
+        return p->value;
+    }
+    JS_ASSERT_IF(!scope.isWith(), !scope.maybeStackFrame());
+    return NULL;
 }
 
 /*****************************************************************************/
 
 static JSObject *
 GetDebugScope(JSContext *cx, ScopeIter si);
 
 static DebugScopeObject *
@@ -1741,30 +1843,19 @@ GetDebugScope(JSContext *cx, JSObject &o
 #ifdef DEBUG
         JSObject *o = &obj;
         while ((o = o->enclosingScope()))
             JS_ASSERT(!o->isScope());
 #endif
         return &obj;
     }
 
-    /*
-     * If 'scope' is a 'with' block, then the chain is fully reified from that
-     * point outwards, and there's no point in bothering with a ScopeIter. If
-     * |scope| has an associated stack frame, we can get more detailed scope
-     * chain information from that.
-     * Note: all this frame hackery will be removed by bug 659577.
-     */
     ScopeObject &scope = obj.asScope();
-    if (!scope.isWith() && scope.maybeStackFrame()) {
-        StackFrame *fp = scope.maybeStackFrame();
-        if (scope.isClonedBlock())
-            fp = js_LiveFrameIfGenerator(fp);
+    if (StackFrame *fp = cx->runtime->debugScopes->hasLiveFrame(scope))
         return GetDebugScope(cx, ScopeIter(fp, scope));
-    }
     return GetDebugScopeForScope(cx, scope, ScopeIter(scope.enclosingScope()));
 }
 
 static JSObject *
 GetDebugScope(JSContext *cx, ScopeIter si)
 {
     JS_CHECK_RECURSION(cx, return NULL);
 
@@ -1777,18 +1868,21 @@ GetDebugScope(JSContext *cx, ScopeIter s
     return GetDebugScopeForScope(cx, si.scope(), si.enclosing());
 }
 
 JSObject *
 js::GetDebugScopeForFunction(JSContext *cx, JSFunction *fun)
 {
     assertSameCompartment(cx, fun);
     JS_ASSERT(cx->compartment->debugMode());
+    if (!cx->runtime->debugScopes->updateLiveScopes(cx))
+        return NULL;
     return GetDebugScope(cx, *fun->environment());
 }
 
 JSObject *
 js::GetDebugScopeForFrame(JSContext *cx, StackFrame *fp)
 {
     assertSameCompartment(cx, fp);
-    /* Unfortunately, we cannot JS_ASSERT(debugMode); see CanUseDebugScopeMaps. */
+    if (CanUseDebugScopeMaps(cx) && !cx->runtime->debugScopes->updateLiveScopes(cx))
+        return NULL;
     return GetDebugScope(cx, ScopeIter(fp));
 }
--- a/js/src/vm/ScopeObject.h
+++ b/js/src/vm/ScopeObject.h
@@ -417,37 +417,58 @@ class DebugScopes
     /* The map from (non-debug) scopes to debug scopes. */
     typedef WeakMap<HeapPtrObject, HeapPtrObject> ObjectWeakMap;
     ObjectWeakMap proxiedScopes;
 
     /*
      * The map from live frames which have optimized-away scopes to the
      * corresponding debug scopes.
      */
-    typedef HashMap<ScopeIter, DebugScopeObject *, ScopeIter, RuntimeAllocPolicy> MissingScopeMap;
+    typedef HashMap<ScopeIter,
+                    DebugScopeObject *,
+                    ScopeIter,
+                    RuntimeAllocPolicy> MissingScopeMap;
     MissingScopeMap missingScopes;
 
+    /*
+     * The map from scope objects of live frames to the live frame. This map
+     * updated lazily whenever the debugger needs the information. In between
+     * two lazy updates, liveScopes becomes incomplete (but not invalid, onPop*
+     * removes scopes as they are popped). Thus, two consecutive debugger lazy
+     * updates of liveScopes need only fill in the new scopes.
+     */
+    typedef HashMap<ScopeObject *,
+                    StackFrame *,
+                    DefaultHasher<ScopeObject *>,
+                    RuntimeAllocPolicy> LiveScopeMap;
+    LiveScopeMap liveScopes;
+
   public:
     DebugScopes(JSRuntime *rt);
     ~DebugScopes();
     bool init();
 
     void mark(JSTracer *trc);
     void sweep();
 
     DebugScopeObject *hasDebugScope(JSContext *cx, ScopeObject &scope) const;
     bool addDebugScope(JSContext *cx, ScopeObject &scope, DebugScopeObject &debugScope);
 
     DebugScopeObject *hasDebugScope(JSContext *cx, ScopeIter si) const;
     bool addDebugScope(JSContext *cx, ScopeIter si, DebugScopeObject &debugScope);
 
+    bool updateLiveScopes(JSContext *cx);
+    StackFrame *hasLiveFrame(ScopeObject &scope);
+
     /*
      * In debug-mode, these must be called whenever exiting a call/block or
      * when activating/yielding a generator.
      */
     void onPopCall(StackFrame *fp);
     void onPopBlock(JSContext *cx, StackFrame *fp);
+    void onPopWith(StackFrame *fp);
+    void onPopStrictEvalScope(StackFrame *fp);
     void onGeneratorFrameChange(StackFrame *from, StackFrame *to);
     void onCompartmentLeaveDebugMode(JSCompartment *c);
 };
 
 }  /* namespace js */
 #endif /* ScopeObject_h___ */
--- a/js/src/vm/Stack.cpp
+++ b/js/src/vm/Stack.cpp
@@ -254,16 +254,19 @@ StackFrame::popBlock(JSContext *cx)
     }
 
     blockChain_ = blockChain_->enclosingBlock();
 }
 
 void
 StackFrame::popWith(JSContext *cx)
 {
+    if (cx->compartment->debugMode())
+        cx->runtime->debugScopes->onPopWith(this);
+
     setScopeChain(scopeChain()->asWith().enclosingScope());
 }
 
 void
 StackFrame::mark(JSTracer *trc)
 {
     /*
      * Normally we would use MarkRoot here, except that generators also take
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -340,17 +340,20 @@ class StackFrame
         HAS_ANNOTATION     =     0x8000,  /* frame has annotation_ set */
         HAS_RVAL           =    0x10000,  /* frame has rval_ set */
         HAS_SCOPECHAIN     =    0x20000,  /* frame has scopeChain_ set */
         HAS_PREVPC         =    0x40000,  /* frame has prevpc_ and prevInline_ set */
         HAS_BLOCKCHAIN     =    0x80000,  /* frame has blockChain_ set */
 
         /* Method JIT state */
         DOWN_FRAMES_EXPANDED = 0x100000,  /* inlining in down frames has been expanded */
-        LOWERED_CALL_APPLY   = 0x200000   /* Pushed by a lowered call/apply */
+        LOWERED_CALL_APPLY   = 0x200000,  /* Pushed by a lowered call/apply */
+
+        /* Debugger state */
+        PREV_UP_TO_DATE    =   0x400000   /* see DebugScopes::updateLiveScopes */
     };
 
   private:
     mutable uint32_t    flags_;         /* bits described by Flags */
     union {                             /* describes what code is executing in a */
         JSScript        *script;        /*   global frame */
         JSFunction      *fun;           /*   function frame, pre GetScopeChain */
     } exec;
@@ -1084,16 +1087,24 @@ class StackFrame
     bool loweredCallOrApply() const {
         return !!(flags_ & LOWERED_CALL_APPLY);
     }
 
     bool isDebuggerFrame() const {
         return !!(flags_ & DEBUGGER);
     }
 
+    bool prevUpToDate() const {
+        return !!(flags_ & PREV_UP_TO_DATE);
+    }
+
+    void setPrevUpToDate() {
+        flags_ |= PREV_UP_TO_DATE;
+    }
+
     bool hasOverflowArgs() const {
         return !!(flags_ & OVERFLOW_ARGS);
     }
 
     bool isYielding() {
         return !!(flags_ & YIELDING);
     }