Bug 927782 - Part 4: Generators allocate all locals on the scope chain. r=luke
☠☠ backed out by 94cdaced90bf ☠ ☠
authorAndy Wingo <wingo@igalia.com>
Fri, 06 Dec 2013 18:22:06 +0100
changeset 159189 cbdd50c96b858458ea7b3bfd6b5335ddde65b4c9
parent 159188 fc7a979712fc7b52f35125e8c15a85ed33f18c6c
child 159190 d6946bd743b38c99c82eb45cf9203423cda9467c
push id37219
push userryanvm@gmail.com
push dateFri, 06 Dec 2013 17:52:33 +0000
treeherdermozilla-inbound@f86d2d4cfadf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs927782
milestone28.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 927782 - Part 4: Generators allocate all locals on the scope chain. r=luke
dom/indexedDB/test/unit/test_cursor_cycle.js
dom/indexedDB/test/unit/test_cursor_mutation.js
dom/indexedDB/test/unit/test_index_object_cursors.js
dom/indexedDB/test/unit/test_index_update_delete.js
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/Parser.cpp
js/src/frontend/SharedContext.h
js/src/jsfun.h
js/src/vm/ScopeObject.cpp
js/src/vm/ScopeObject.h
js/src/vm/Stack-inl.h
js/src/vm/Stack.cpp
js/src/vm/Stack.h
--- a/dom/indexedDB/test/unit/test_cursor_cycle.js
+++ b/dom/indexedDB/test/unit/test_cursor_cycle.js
@@ -17,21 +17,16 @@ function testSteps()
   let db = event.target.result;
   event.target.onsuccess = continueToNextStep;
 
   let objectStore = db.createObjectStore("foo", { keyPath: "ss" });
   objectStore.createIndex("name", "name", { unique: true });
   objectStore.add(Bob);
   yield undefined;
 
-  // This direct eval causes locals to be aliased, and thus allocated on
-  // the scope chain.  Comment it out (and the workarounds below) and
-  // the test passes.  Bug 943409.
-  eval('');
-
   db.transaction("foo", "readwrite").objectStore("foo")
     .index("name").openCursor().onsuccess = function(event) {
     event.target.transaction.oncomplete = continueToNextStep;
     let cursor = event.target.result;
     if (cursor) {
       let objectStore = event.target.transaction.objectStore("foo");
       objectStore.delete(Bob.ss)
                  .onsuccess = function(event) { cursor.continue(); };
--- a/dom/indexedDB/test/unit/test_cursor_mutation.js
+++ b/dom/indexedDB/test/unit/test_cursor_mutation.js
@@ -30,21 +30,16 @@ function testSteps()
   let event = yield undefined;
 
   let db = event.target.result;
   event.target.onsuccess = continueToNextStep;
 
   let objectStore = db.createObjectStore("foo", { keyPath: "ss" });
   objectStore.createIndex("name", "name", { unique: true });
 
-  // This direct eval causes locals to be aliased, and thus allocated on
-  // the scope chain.  Comment it out (and the workarounds below) and
-  // the test passes.  Bug 943409.
-  eval('');
-
   for (let i = 0; i < objectStoreData.length - 1; i++) {
     objectStore.add(objectStoreData[i]);
   }
   yield undefined;
 
   let count = 0;
 
   let sawAdded = false;
--- a/dom/indexedDB/test/unit/test_index_object_cursors.js
+++ b/dom/indexedDB/test/unit/test_index_object_cursors.js
@@ -29,19 +29,16 @@ function testSteps()
   request.onupgradeneeded = grabEventAndContinueHandler;
   let event = yield undefined;
 
   let db = event.target.result;
   db.onerror = errorHandler;
 
   event.target.onsuccess = continueToNextStep;
 
-  // Bug 943409.
-  eval('');
-
   for (let objectStoreIndex in objectStoreData) {
     const objectStoreInfo = objectStoreData[objectStoreIndex];
     let objectStore = db.createObjectStore(objectStoreInfo.name,
                                            objectStoreInfo.options);
     for (let indexIndex in indexData) {
       const indexInfo = indexData[indexIndex];
       let index = objectStore.createIndex(indexInfo.name,
                                           indexInfo.keyPath,
--- a/dom/indexedDB/test/unit/test_index_update_delete.js
+++ b/dom/indexedDB/test/unit/test_index_update_delete.js
@@ -13,19 +13,16 @@ function testSteps()
   request.onupgradeneeded = grabEventAndContinueHandler;
   request.onsuccess = grabEventAndContinueHandler;
 
   let event = yield undefined;
 
   let db = event.target.result;
   db.onerror = errorHandler;
 
-  // Bug 943409.
-  eval('');
-
   for each (let autoIncrement in [false, true]) {
     let objectStore =
       db.createObjectStore(autoIncrement, { keyPath: "id",
                                             autoIncrement: autoIncrement });
 
     for (let i = 0; i < 10; i++) {
       objectStore.add({ id: i, index: i });
     }
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -1017,33 +1017,39 @@ BytecodeEmitter::isAliasedName(ParseNode
     if (dn->pn_cookie.level() != script->staticLevel)
         return true;
 
     switch (dn->kind()) {
       case Definition::LET:
         /*
          * There are two ways to alias a let variable: nested functions and
          * dynamic scope operations. (This is overly conservative since the
-         * bindingsAccessedDynamically flag is function-wide.)
+         * bindingsAccessedDynamically flag, checked by allLocalsAliased, is
+         * function-wide.)
+         *
+         * In addition all locals in generators are marked as aliased, to ensure
+         * that they are allocated on scope chains instead of on the stack.  See
+         * the definition of SharedContext::allLocalsAliased.
          */
-        return dn->isClosed() || sc->bindingsAccessedDynamically();
+        return dn->isClosed() || sc->allLocalsAliased();
       case Definition::ARG:
         /*
          * Consult the bindings, since they already record aliasing. We might
          * be tempted to use the same definition as VAR/CONST/LET, but there is
          * a problem caused by duplicate arguments: only the last argument with
          * a given name is aliased. This is necessary to avoid generating a
          * shape for the call object with with more than one name for a given
          * slot (which violates internal engine invariants). All this means that
-         * the '|| sc->bindingsAccessedDynamically' disjunct is incorrect since
-         * it will mark both parameters in function(x,x) as aliased.
+         * the '|| sc->allLocalsAliased()' disjunct is incorrect since it will
+         * mark both parameters in function(x,x) as aliased.
          */
         return script->formalIsAliased(pn->pn_cookie.slot());
       case Definition::VAR:
       case Definition::CONST:
+        JS_ASSERT_IF(sc->allLocalsAliased(), script->varIsAliased(pn->pn_cookie.slot()));
         return script->varIsAliased(pn->pn_cookie.slot());
       case Definition::PLACEHOLDER:
       case Definition::NAMED_LAMBDA:
       case Definition::MISSING:
         MOZ_ASSUME_UNREACHABLE("unexpected dn->kind");
     }
     return false;
 }
@@ -1065,16 +1071,27 @@ AdjustBlockSlot(ExclusiveContext *cx, By
         if ((unsigned) slot >= SLOTNO_LIMIT) {
             bce->reportError(nullptr, JSMSG_TOO_MANY_LOCALS);
             slot = -1;
         }
     }
     return slot;
 }
 
+#ifdef DEBUG
+static bool
+AllLocalsAliased(StaticBlockObject &obj)
+{
+    for (unsigned i = 0; i < obj.slotCount(); i++)
+        if (!obj.isAliased(i))
+            return false;
+    return true;
+}
+#endif
+
 static bool
 EmitEnterBlock(ExclusiveContext *cx, BytecodeEmitter *bce, ParseNode *pn, JSOp op)
 {
     JS_ASSERT(pn->isKind(PNK_LEXICALSCOPE));
     StmtInfoBCE *stmt = bce->topStmt;
     JS_ASSERT(stmt->type == STMT_BLOCK || stmt->type == STMT_SWITCH);
     JS_ASSERT(stmt->isBlockScope);
     JS_ASSERT(stmt->blockScopeIndex == bce->blockScopeList.length() - 1);
@@ -1101,17 +1118,17 @@ EmitEnterBlock(ExclusiveContext *cx, Byt
     if (depthPlusFixed < 0)
         return false;
 
     for (unsigned i = 0; i < blockObj->slotCount(); i++) {
         Definition *dn = blockObj->maybeDefinitionParseNode(i);
 
         /* Beware the empty destructuring dummy. */
         if (!dn) {
-            blockObj->setAliased(i, bce->sc->bindingsAccessedDynamically());
+            blockObj->setAliased(i, bce->sc->allLocalsAliased());
             continue;
         }
 
         JS_ASSERT(dn->isDefn());
         JS_ASSERT(unsigned(dn->frameSlot() + depthPlusFixed) < JS_BIT(16));
         if (!dn->pn_cookie.set(bce->parser->tokenStream, dn->pn_cookie.level(),
                                uint16_t(dn->frameSlot() + depthPlusFixed)))
             return false;
@@ -1122,16 +1139,18 @@ EmitEnterBlock(ExclusiveContext *cx, Byt
             JS_ASSERT(!(pnu->pn_dflags & PND_BOUND));
             JS_ASSERT(pnu->pn_cookie.isFree());
         }
 #endif
 
         blockObj->setAliased(i, bce->isAliasedName(dn));
     }
 
+    JS_ASSERT_IF(bce->sc->allLocalsAliased(), AllLocalsAliased(*blockObj));
+
     return true;
 }
 
 /*
  * Try to convert a *NAME op with a free name to a more specialized GNAME,
  * INTRINSIC or ALIASEDVAR op, which optimize accesses on that name.
  * Return true if a conversion was made.
  */
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -273,17 +273,17 @@ AppendPackedBindings(const ParseContext<
 
         /*
          * Bindings::init does not check for duplicates so we must ensure that
          * only one binding with a given name is marked aliased. pc->decls
          * maintains the canonical definition for each name, so use that.
          */
         JS_ASSERT_IF(dn->isClosed(), pc->decls().lookupFirst(name) == dn);
         bool aliased = dn->isClosed() ||
-                       (pc->sc->bindingsAccessedDynamically() &&
+                       (pc->sc->allLocalsAliased() &&
                         pc->decls().lookupFirst(name) == dn);
 
         *dst = Binding(name, kind, aliased);
     }
 }
 
 template <typename ParseHandler>
 bool
--- a/js/src/frontend/SharedContext.h
+++ b/js/src/frontend/SharedContext.h
@@ -195,16 +195,18 @@ class SharedContext
     bool hasExplicitUseStrict()        const { return anyCxFlags.hasExplicitUseStrict; }
     bool bindingsAccessedDynamically() const { return anyCxFlags.bindingsAccessedDynamically; }
     bool hasDebuggerStatement()        const { return anyCxFlags.hasDebuggerStatement; }
 
     void setExplicitUseStrict()           { anyCxFlags.hasExplicitUseStrict        = true; }
     void setBindingsAccessedDynamically() { anyCxFlags.bindingsAccessedDynamically = true; }
     void setHasDebuggerStatement()        { anyCxFlags.hasDebuggerStatement        = true; }
 
+    inline bool allLocalsAliased();
+
     // JSOPTION_EXTRA_WARNINGS warnings or strict mode errors.
     bool needStrictChecks() {
         return strict || extraWarnings;
     }
 };
 
 class GlobalSharedContext : public SharedContext
 {
@@ -302,27 +304,40 @@ class FunctionBox : public ObjectBox, pu
         startColumn = tokenStream.getColumn();
     }
 
     bool isHeavyweight()
     {
         // Note: this should be kept in sync with JSFunction::isHeavyweight().
         return bindings.hasAnyAliasedBindings() ||
                hasExtensibleScope() ||
-               needsDeclEnvObject();
+               needsDeclEnvObject() ||
+               isGenerator();
     }
 };
 
 inline FunctionBox *
 SharedContext::asFunctionBox()
 {
     JS_ASSERT(isFunctionBox());
     return static_cast<FunctionBox*>(this);
 }
 
+// In generators, we treat all locals as aliased so that they get stored on the
+// heap.  This way there is less information to copy off the stack when
+// suspending, and back on when resuming.  It also avoids the need to create and
+// invalidate DebugScope proxies for unaliased locals in a generator frame, as
+// the generator frame will be copied out to the heap and released only by GC.
+inline bool
+SharedContext::allLocalsAliased()
+{
+    return bindingsAccessedDynamically() || (isFunctionBox() && asFunctionBox()->isGenerator());
+}
+
+
 /*
  * NB: If you add a new type of statement that is a scope, add it between
  * STMT_WITH and STMT_CATCH, or you will break StmtInfoBase::linksScope. If you
  * add a non-looping statement type, add it before STMT_DO_LOOP or you will
  * break StmtInfoBase::isLoop().
  *
  * Also remember to keep the statementName array in BytecodeEmitter.cpp in
  * sync.
--- a/js/src/jsfun.h
+++ b/js/src/jsfun.h
@@ -95,17 +95,18 @@ class JSFunction : public JSObject
         JS_ASSERT(!isInterpretedLazy());
 
         if (isNative())
             return false;
 
         // Note: this should be kept in sync with FunctionBox::isHeavyweight().
         return nonLazyScript()->bindings.hasAnyAliasedBindings() ||
                nonLazyScript()->funHasExtensibleScope ||
-               nonLazyScript()->funNeedsDeclEnvObject;
+               nonLazyScript()->funNeedsDeclEnvObject ||
+               isGenerator();
     }
 
     /* A function can be classified as either native (C++) or interpreted (JS): */
     bool isInterpreted()            const { return flags & (INTERPRETED | INTERPRETED_LAZY); }
     bool isNative()                 const { return !isInterpreted(); }
 
     /* Possible attributes of a native function: */
     bool isNativeConstructor()      const { return flags & NATIVE_CTOR; }
--- a/js/src/vm/ScopeObject.cpp
+++ b/js/src/vm/ScopeObject.cpp
@@ -881,28 +881,16 @@ ScopeIter::ScopeIter(AbstractFramePtr fr
     cur_(cx, frame.scopeChain()),
     block_(cx, frame.maybeBlockChain())
 {
     assertSameCompartment(cx, frame);
     settle();
     MOZ_GUARD_OBJECT_NOTIFIER_INIT;
 }
 
-ScopeIter::ScopeIter(const ScopeIter &si, AbstractFramePtr frame, JSContext *cx
-                     MOZ_GUARD_OBJECT_NOTIFIER_PARAM_IN_IMPL)
-  : cx(si.cx),
-    frame_(frame),
-    cur_(cx, si.cur_),
-    block_(cx, si.block_),
-    type_(si.type_),
-    hasScopeObject_(si.hasScopeObject_)
-{
-    MOZ_GUARD_OBJECT_NOTIFIER_INIT;
-}
-
 ScopeIter::ScopeIter(AbstractFramePtr frame, ScopeObject &scope, JSContext *cx
                      MOZ_GUARD_OBJECT_NOTIFIER_PARAM_IN_IMPL)
   : cx(cx),
     frame_(frame),
     cur_(cx, &scope),
     block_(cx)
 {
     /*
@@ -1091,18 +1079,17 @@ class DebugScopeProxy : public BaseProxy
     enum Action { SET, GET };
 
     /*
      * This function handles access to unaliased locals/formals. Since they are
      * unaliased, the values of these variables are not stored in the slots of
      * the normal Call/BlockObject scope objects and thus must be recovered
      * from somewhere else:
      *  + if the invocation for which the scope was created is still executing,
-     *    there is a StackFrame (either live on the stack or floating in a
-     *    generator object) holding the values;
+     *    there is a StackFrame live on the stack holding the values;
      *  + if the invocation for which the scope was created finished executing:
      *     - and there was a DebugScopeObject associated with scope, then the
      *       DebugScopes::onPop(Call|Block) handler copied out the unaliased
      *       variables:
      *        . for block scopes, the unaliased values were copied directly
      *          into the block object, since there is a slot allocated for every
      *          block binding, regardless of whether it is aliased;
      *        . for function scopes, a dense array is created in onPopCall to hold
@@ -1582,50 +1569,35 @@ DebugScopes::mark(JSTracer *trc)
 {
     proxiedScopes.trace(trc);
 }
 
 void
 DebugScopes::sweep(JSRuntime *rt)
 {
     /*
-     * 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.
+     * missingScopes points to debug scopes weakly so that debug scopes can be
+     * released more eagerly.
      */
     for (MissingScopeMap::Enum e(missingScopes); !e.empty(); e.popFront()) {
         if (IsObjectAboutToBeFinalized(e.front().value().unsafeGet()))
             e.removeFront();
     }
 
     for (LiveScopeMap::Enum e(liveScopes); !e.empty(); e.popFront()) {
         ScopeObject *scope = e.front().key();
-        AbstractFramePtr frame = e.front().value();
 
         /*
          * Scopes can be finalized when a debugger-synthesized ScopeObject is
          * no longer reachable via its DebugScopeObject.
          */
         if (IsObjectAboutToBeFinalized(&scope)) {
             e.removeFront();
             continue;
         }
-
-        /*
-         * As explained in onGeneratorFrameChange, liveScopes includes
-         * suspended generator frames. Since a generator can be finalized while
-         * its scope is live, we must explicitly detect finalized generators.
-         */
-        if (JSGenerator *gen = frame.maybeSuspendedGenerator(rt)) {
-            JS_ASSERT(gen->state == JSGEN_NEWBORN || gen->state == JSGEN_OPEN);
-            if (IsObjectAboutToBeFinalized(&gen->obj)) {
-                e.removeFront();
-                continue;
-            }
-        }
     }
 }
 
 /*
  * 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
@@ -1706,16 +1678,17 @@ DebugScopes::hasDebugScope(JSContext *cx
     return nullptr;
 }
 
 bool
 DebugScopes::addDebugScope(JSContext *cx, const ScopeIter &si, DebugScopeObject &debugScope)
 {
     JS_ASSERT(!si.hasScopeObject());
     JS_ASSERT(cx->compartment() == debugScope.compartment());
+    JS_ASSERT_IF(si.frame().isFunctionFrame(), !si.frame().callee()->isGenerator());
 
     if (!CanUseDebugScopeMaps(cx))
         return true;
 
     DebugScopes *scopes = ensureCompartmentData(cx);
     if (!scopes)
         return false;
 
@@ -1858,53 +1831,16 @@ DebugScopes::onPopStrictEvalScope(Abstra
      * The StackFrame may be observed before the prologue has created the
      * CallObject. See ScopeIter::settle.
      */
     if (frame.hasCallObj())
         scopes->liveScopes.remove(&frame.scopeChain()->as<CallObject>());
 }
 
 void
-DebugScopes::onGeneratorFrameChange(AbstractFramePtr from, AbstractFramePtr to, JSContext *cx)
-{
-    for (ScopeIter toIter(to, cx); !toIter.done(); ++toIter) {
-        DebugScopes *scopes = ensureCompartmentData(cx);
-        if (!scopes)
-            return;
-
-        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).
-             */
-            JS_ASSERT(toIter.scope().compartment() == cx->compartment());
-            LiveScopeMap::AddPtr livePtr = scopes->liveScopes.lookupForAdd(&toIter.scope());
-            if (livePtr) {
-                livePtr->value() = to;
-            } else {
-                scopes->liveScopes.add(livePtr, &toIter.scope(), to);  // OOM here?
-                liveScopesPostWriteBarrier(cx->runtime(), &scopes->liveScopes, &toIter.scope());
-            }
-        } else {
-            ScopeIter si(toIter, from, cx);
-            JS_ASSERT(si.frame().scopeChain()->compartment() == cx->compartment());
-            if (MissingScopeMap::Ptr p = scopes->missingScopes.lookup(si)) {
-                DebugScopeObject &debugScope = *p->value();
-                scopes->liveScopes.lookup(&debugScope.scope())->value() = to;
-                scopes->missingScopes.remove(p);
-                scopes->missingScopes.put(toIter, &debugScope);  // OOM here?
-            }
-        }
-    }
-}
-
-void
 DebugScopes::onCompartmentLeaveDebugMode(JSCompartment *c)
 {
     DebugScopes *scopes = c->debugScopes;
     if (scopes) {
         scopes->proxiedScopes.clear();
         scopes->missingScopes.clear();
         scopes->liveScopes.clear();
     }
@@ -1933,16 +1869,19 @@ DebugScopes::updateLiveScopes(JSContext 
          */
         if (i.isIon())
             continue;
 
         AbstractFramePtr frame = i.abstractFramePtr();
         if (frame.scopeChain()->compartment() != cx->compartment())
             continue;
 
+        if (frame.isFunctionFrame() && frame.callee()->isGenerator())
+            continue;
+
         for (ScopeIter si(frame, cx); !si.done(); ++si) {
             if (si.hasScopeObject()) {
                 JS_ASSERT(si.scope().compartment() == cx->compartment());
                 DebugScopes *scopes = ensureCompartmentData(cx);
                 if (!scopes)
                     return false;
                 if (!scopes->liveScopes.put(&si.scope(), frame))
                     return false;
@@ -1961,35 +1900,19 @@ DebugScopes::updateLiveScopes(JSContext 
 
 AbstractFramePtr
 DebugScopes::hasLiveFrame(ScopeObject &scope)
 {
     DebugScopes *scopes = scope.compartment()->debugScopes;
     if (!scopes)
         return NullFramePtr();
 
-    if (LiveScopeMap::Ptr p = scopes->liveScopes.lookup(&scope)) {
-        AbstractFramePtr frame = p->value();
+    if (LiveScopeMap::Ptr p = scopes->liveScopes.lookup(&scope))
+        return p->value();
 
-        /*
-         * Since liveScopes is effectively a weak pointer, we need a read
-         * barrier. The scenario where this is necessary is:
-         *  1. GC starts, a suspended generator is not live
-         *  2. hasLiveFrame returns a StackFrame* to the (soon to be dead)
-         *     suspended generator
-         *  3. stack frame values (which will neve be marked) are read from the
-         *     StackFrame
-         *  4. GC completes, live objects may now point to values that weren't
-         *     marked and thus may point to swept GC things
-         */
-        if (JSGenerator *gen = frame.maybeSuspendedGenerator(scope.compartment()->runtimeFromMainThread()))
-            JSObject::readBarrier(gen->obj);
-
-        return frame;
-    }
     return NullFramePtr();
 }
 
 /*****************************************************************************/
 
 static JSObject *
 GetDebugScope(JSContext *cx, const ScopeIter &si);
 
@@ -2041,32 +1964,36 @@ GetDebugScopeForMissing(JSContext *cx, c
      *
      * Note: to preserve scopeChain depth invariants, these lazily-reified
      * scopes must not be put on the frame's scope chain; instead, they are
      * maintained via DebugScopes hooks.
      */
     DebugScopeObject *debugScope = nullptr;
     switch (si.type()) {
       case ScopeIter::Call: {
+        // Generators should always reify their scopes.
+        JS_ASSERT(!si.frame().callee()->isGenerator());
         Rooted<CallObject*> callobj(cx, CallObject::createForFunction(cx, si.frame()));
         if (!callobj)
             return nullptr;
 
         if (callobj->enclosingScope().is<DeclEnvObject>()) {
             JS_ASSERT(CallObjectLambdaName(callobj->callee()));
             DeclEnvObject &declenv = callobj->enclosingScope().as<DeclEnvObject>();
             enclosingDebug = DebugScopeObject::create(cx, declenv, enclosingDebug);
             if (!enclosingDebug)
                 return nullptr;
         }
 
         debugScope = DebugScopeObject::create(cx, *callobj, enclosingDebug);
         break;
       }
       case ScopeIter::Block: {
+        // Generators should always reify their scopes.
+        JS_ASSERT_IF(si.frame().isFunctionFrame(), !si.frame().callee()->isGenerator());
         Rooted<StaticBlockObject *> staticBlock(cx, &si.staticBlock());
         ClonedBlockObject *block = ClonedBlockObject::create(cx, staticBlock, si.frame());
         if (!block)
             return nullptr;
 
         debugScope = DebugScopeObject::create(cx, *block, enclosingDebug);
         break;
       }
--- a/js/src/vm/ScopeObject.h
+++ b/js/src/vm/ScopeObject.h
@@ -543,23 +543,16 @@ class ScopeIter
 
     /*
      * Without a StackFrame, the resulting ScopeIter is done() with
      * enclosingScope() as given.
      */
     explicit ScopeIter(JSObject &enclosingScope, JSContext *cx
                        MOZ_GUARD_OBJECT_NOTIFIER_PARAM);
 
-    /*
-     * For the special case of generators, copy the given ScopeIter, with 'fp'
-     * as the StackFrame instead of si.fp(). Not for general use.
-     */
-    ScopeIter(const ScopeIter &si, AbstractFramePtr frame, JSContext *cx
-              MOZ_GUARD_OBJECT_NOTIFIER_PARAM);
-
     /* Like ScopeIter(StackFrame *) except start at 'scope'. */
     ScopeIter(AbstractFramePtr frame, ScopeObject &scope, JSContext *cx
               MOZ_GUARD_OBJECT_NOTIFIER_PARAM);
 
     bool done() const { return !frame_; }
 
     /* If done(): */
 
@@ -714,25 +707,22 @@ class DebugScopes
     static bool addDebugScope(JSContext *cx, ScopeObject &scope, DebugScopeObject &debugScope);
 
     static DebugScopeObject *hasDebugScope(JSContext *cx, const ScopeIter &si);
     static bool addDebugScope(JSContext *cx, const ScopeIter &si, DebugScopeObject &debugScope);
 
     static bool updateLiveScopes(JSContext *cx);
     static AbstractFramePtr hasLiveFrame(ScopeObject &scope);
 
-    /*
-     * In debug-mode, these must be called whenever exiting a call/block or
-     * when activating/yielding a generator.
-     */
+    // In debug-mode, these must be called whenever exiting a scope that might
+    // have stack-allocated locals.
     static void onPopCall(AbstractFramePtr frame, JSContext *cx);
     static void onPopBlock(JSContext *cx, AbstractFramePtr frame);
     static void onPopWith(AbstractFramePtr frame);
     static void onPopStrictEvalScope(AbstractFramePtr frame);
-    static void onGeneratorFrameChange(AbstractFramePtr from, AbstractFramePtr to, JSContext *cx);
     static void onCompartmentLeaveDebugMode(JSCompartment *c);
 };
 
 }  /* namespace js */
 
 template<>
 inline bool
 JSObject::is<js::NestedScopeObject>() const
--- a/js/src/vm/Stack-inl.h
+++ b/js/src/vm/Stack-inl.h
@@ -511,24 +511,16 @@ AbstractFramePtr::unaliasedActual(unsign
         return asStackFrame()->unaliasedActual(i, checkAliasing);
 #ifdef JS_ION
     return asBaselineFrame()->unaliasedActual(i, checkAliasing);
 #else
     MOZ_ASSUME_UNREACHABLE("Invalid frame");
 #endif
 }
 
-inline JSGenerator *
-AbstractFramePtr::maybeSuspendedGenerator(JSRuntime *rt) const
-{
-    if (isStackFrame())
-        return asStackFrame()->maybeSuspendedGenerator(rt);
-    return nullptr;
-}
-
 inline StaticBlockObject *
 AbstractFramePtr::maybeBlockChain() const
 {
     if (isStackFrame())
         return asStackFrame()->maybeBlockChain();
 #ifdef JS_ION
     return asBaselineFrame()->maybeBlockChain();
 #else
--- a/js/src/vm/Stack.cpp
+++ b/js/src/vm/Stack.cpp
@@ -118,19 +118,16 @@ StackFrame::copyFrameAndValues(JSContext
 
     srcend = othersp;
     dst = slots();
     for (const Value *src = otherfp->slots(); src < srcend; src++, dst++) {
         *dst = *src;
         if (doPostBarrier)
             HeapValue::writeBarrierPost(*dst, dst);
     }
-
-    if (JS_UNLIKELY(cx->compartment()->debugMode()))
-        DebugScopes::onGeneratorFrameChange(otherfp, this, cx);
 }
 
 /* Note: explicit instantiation for js_NewGenerator located in jsiter.cpp. */
 template
 void StackFrame::copyFrameAndValues<StackFrame::NoPostBarrier>(
                                     JSContext *, Value *, StackFrame *, const Value *, Value *);
 template
 void StackFrame::copyFrameAndValues<StackFrame::DoPostBarrier>(
@@ -150,37 +147,16 @@ StackFrame::writeBarrierPost()
             JSScript::writeBarrierPost(u.evalScript, (void *)&u.evalScript);
     } else {
         JSScript::writeBarrierPost(exec.script, (void *)&exec.script);
     }
     if (hasReturnValue())
         HeapValue::writeBarrierPost(rval_, &rval_);
 }
 
-JSGenerator *
-StackFrame::maybeSuspendedGenerator(JSRuntime *rt)
-{
-    /*
-     * A suspended generator's frame is embedded inside the JSGenerator object
-     * and is not currently running.
-     */
-    if (!isGeneratorFrame() || !isSuspended())
-        return nullptr;
-
-    /*
-     * Once we know we have a suspended generator frame, there is a static
-     * offset from the frame's snapshot to beginning of the JSGenerator.
-     */
-    char *vp = reinterpret_cast<char *>(generatorArgsSnapshotBegin());
-    char *p = vp - offsetof(JSGenerator, stackSnapshot);
-    JSGenerator *gen = reinterpret_cast<JSGenerator *>(p);
-    JS_ASSERT(gen->fp == this);
-    return gen;
-}
-
 bool
 StackFrame::copyRawFrameSlots(AutoValueVector *vec)
 {
     if (!vec->resize(numFormalArgs() + script()->nfixed))
         return false;
     PodCopy(vec->begin(), argv(), numFormalArgs());
     PodCopy(vec->begin() + numFormalArgs(), slots(), script()->nfixed);
     return true;
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -167,18 +167,16 @@ class AbstractFramePtr
 
     void *raw() const { return reinterpret_cast<void *>(ptr_); }
 
     bool operator ==(const AbstractFramePtr &other) const { return ptr_ == other.ptr_; }
     bool operator !=(const AbstractFramePtr &other) const { return ptr_ != other.ptr_; }
 
     operator bool() const { return !!ptr_; }
 
-    inline JSGenerator *maybeSuspendedGenerator(JSRuntime *rt) const;
-
     inline JSObject *scopeChain() const;
     inline CallObject &callObj() const;
     inline bool initFunctionScopeObjects(JSContext *cx);
     inline void pushOnScopeChain(ScopeObject &scope);
 
     inline JSCompartment *compartment() const;
 
     inline StaticBlockObject *maybeBlockChain() const;
@@ -858,18 +856,16 @@ class StackFrame
     enum TriggerPostBarriers {
         DoPostBarrier = true,
         NoPostBarrier = false
     };
     template <TriggerPostBarriers doPostBarrier>
     void copyFrameAndValues(JSContext *cx, Value *vp, StackFrame *otherfp,
                             const Value *othervp, Value *othersp);
 
-    JSGenerator *maybeSuspendedGenerator(JSRuntime *rt);
-
     /*
      * js::Execute pushes both global and function frames (since eval() in a
      * function pushes a frame with isFunctionFrame() && isEvalFrame()). Most
      * code should not care where a frame was pushed, but if it is necessary to
      * pick out frames pushed by js::Execute, this is the right query:
      */
 
     bool isFramePushedByExecute() const {