Bug 480132: Clone lexical blocks only when needed. r=igor

Terminology:

A "script block" is an object of class Block allocated by the byte
compiler and associated with a script.  Script blocks are never
modified, and may be used as a prototype for a "closure block":

A "closure block" is an object of class Block that holds variables
that have been closed over (although we actually leave the variables
on the stack until we leave their dynamic scope).  A closure block is
a clone of a script block (its prototype is a script block).

Adjust the meanings of fp->blockChain and fp->scopeChain:

  fp->blockChain is always the innermost script block in whose static
  scope we're executing.

  fp->scopeChain is the current scope chain, including 'call' objects
  and closure blocks for those function calls and blocks in whose
  static scope we are currently executing, and 'with' objects for with
  statements; the chain is typically terminated by a global object.
  However, as an optimization, the young end of the chain omits block
  objects we have not yet needed to clone.

Closures need fully reified scope chains, so have js_GetScopeChain
reify any closure blocks missing from the young end of fp->scopeChain
by cloning script blocks as needed from fp->blockChain.  Thus, if we
never actually close over a particular block, we never place a closure
block for it on fp->scopeChain.

Have JSOP_ENTERBLOCK and JSOP_LEAVEBLOCK always keep fp->blockChain
current.  When JSOP_LEAVEBLOCK pops a block from fp->blockChain that
has been cloned on fp->scopeChain, pop fp->scopeChain as well.

Remove the JSFRAME_POP_BLOCKS flag, as it is no longer needed.

Ensure that the JIT won't have to create closure blocks or call
js_PutBlockObject; it can't handle those things yet.  Note our current
script block when we begin recording.  Abort recording if we leave
that block; we can't tell in advance whether it will need to be "put"
in future trace invocations.  Leave trace if we call js_GetScopeChain
while in the static scope of lexical blocks.  Remove JIT tests based
on JSFRAME_POP_BLOCKS.

Verify that generators capture the correct value for blockChain.

Add a constructor to JSAutoTempValueRooter for rooting JSObject
pointers.

js/src/jscntxt.h

@@ -1038,6 +1038,10 @@ class JSAutoTempValueRooter
         : mContext(cx) {
         JS_PUSH_TEMP_ROOT_STRING(mContext, str, &mTvr);
     }
+    JSAutoTempValueRooter(JSContext *cx, JSObject *obj)
+        : mContext(cx) {
+        JS_PUSH_TEMP_ROOT_OBJECT(mContext, obj, &mTvr);
+    }
 
     ~JSAutoTempValueRooter() {
         JS_POP_TEMP_ROOT(mContext, &mTvr);

js/src/jsinterp.cpp

@@ -663,11 +663,9 @@ js_FreeStack(JSContext *cx, void *mark)
 JSObject *
 js_GetScopeChain(JSContext *cx, JSStackFrame *fp)
 {
-    JSObject *obj, *cursor, *clonedChild, *parent;
+    JSObject *sharedBlock = fp->blockChain;
-    JSTempValueRooter tvr;
 
-    obj = fp->blockChain;
+    if (!sharedBlock) {
-    if (!obj) {
         /*
          * Don't force a call object for a lightweight function call, but do
          * insist that there is a call object for a heavyweight function call.
@@ -679,70 +677,115 @@ js_GetScopeChain(JSContext *cx, JSStackFrame *fp)
         return fp->scopeChain;
     }
 
+    /* We don't handle cloning blocks on trace.  */
+    js_LeaveTrace(cx);
+
     /*
      * We have one or more lexical scopes to reflect into fp->scopeChain, so
      * make sure there's a call object at the current head of the scope chain,
      * if this frame is a call frame.
+     *
+     * Also, identify the innermost compiler-allocated block we needn't clone.
      */
+    JSObject *limitBlock, *limitClone;
     if (fp->fun && !fp->callobj) {
         JS_ASSERT(OBJ_GET_CLASS(cx, fp->scopeChain) != &js_BlockClass ||
                   OBJ_GET_PRIVATE(cx, fp->scopeChain) != fp);
         if (!js_GetCallObject(cx, fp))
             return NULL;
+
+        /* We know we must clone everything on blockChain.  */ 
+        limitBlock = limitClone = NULL;
+    } else {
+        /*
+         * scopeChain includes all blocks whose static scope we're within that
+         * have already been cloned.  Find the innermost such block.  Its
+         * prototype should appear on blockChain; we'll clone blockChain up
+         * to, but not including, that prototype.
+         */
+        limitClone = fp->scopeChain;
+        while (OBJ_GET_CLASS(cx, limitClone) == &js_WithClass)
+            limitClone = OBJ_GET_PARENT(cx, limitClone);
+        JS_ASSERT(limitClone);
+
+        /* 
+         * It may seem like we don't know enough about limitClone to be able
+         * to just grab its prototype as we do here, but it's actually okay.
+         *
+         * If limitClone is a block object belonging to this frame, then its
+         * prototype is the innermost entry in blockChain that we have already
+         * cloned, and is thus the place to stop when we clone below.
+         *
+         * Otherwise, there are no blocks for this frame on scopeChain, and we
+         * need to clone the whole blockChain.  In this case, limitBlock can
+         * point to any object known not to be on blockChain, since we simply
+         * loop until we hit limitBlock or NULL.  If limitClone is a block, it
+         * isn't a block from this function, since blocks can't be nested
+         * within themselves on scopeChain (recursion is dynamic nesting, not
+         * static nesting).  If limitClone isn't a block, its prototype won't
+         * be a block either.  So we can just grab limitClone's prototype here
+         * regardless of its type or which frame it belongs to.
+         */
+        limitBlock = OBJ_GET_PROTO(cx, limitClone);
+
+        /* If the innermost block has already been cloned, we are done. */
+        if (limitBlock == sharedBlock)
+            return fp->scopeChain;
     }
 
     /*
-     * Clone the block chain. To avoid recursive cloning we set the parent of
+     * Special-case cloning the innermost block; this doesn't have enough in
-     * the cloned child after we clone the parent. In the following loop when
+     * common with subsequent steps to include in the loop.
-     * clonedChild is null it indicates the first iteration when no special GC
+     * 
-     * rooting is necessary. On the second and the following iterations we
+     * We pass fp->scopeChain and not null even if we override the parent slot
-     * have to protect cloned so far chain against the GC during cloning of
+     * later as null triggers useless calculations of slot's value in
-     * the cursor object.
+     * js_NewObject that js_CloneBlockObject calls.
      */
-    cursor = obj;
+    JSObject *innermostNewChild
-    clonedChild = NULL;
+        = js_CloneBlockObject(cx, sharedBlock, fp->scopeChain, fp);
+    if (!innermostNewChild)
+        return NULL;
+    JSAutoTempValueRooter tvr(cx, innermostNewChild);
+
+    /*
+     * Clone our way towards outer scopes until we reach the innermost
+     * enclosing function, or the innermost block we've already cloned.
+     */
+    JSObject *newChild = innermostNewChild;
     for (;;) {
-        parent = OBJ_GET_PARENT(cx, cursor);
+        JS_ASSERT(OBJ_GET_PROTO(cx, newChild) == sharedBlock);
+        sharedBlock = OBJ_GET_PARENT(cx, sharedBlock);
+
+        /* Sometimes limitBlock will be NULL, so check that first.  */
+        if (sharedBlock == limitBlock || !sharedBlock)
+            break;
+
+        /* As in the call above, we don't know the real parent yet.  */
+        JSObject *clone
+            = js_CloneBlockObject(cx, sharedBlock, fp->scopeChain, fp);
+        if (!clone)
+            return NULL;
 
         /*
-         * We pass fp->scopeChain and not null even if we override the parent
+         * Avoid OBJ_SET_PARENT overhead as newChild cannot escape to
-         * slot later as null triggers useless calculations of slot's value in
+         * other threads.
-         * js_NewObject that js_CloneBlockObject calls.
          */
-        cursor = js_CloneBlockObject(cx, cursor, fp->scopeChain, fp);
+        STOBJ_SET_PARENT(newChild, clone);
-        if (!cursor) {
+        newChild = clone;
-            if (clonedChild)
-                JS_POP_TEMP_ROOT(cx, &tvr);
-            return NULL;
-        }
-        if (!clonedChild) {
-            /*
-             * The first iteration. Check if other follow and root obj if so
-             * to protect the whole cloned chain against GC.
-             */
-            obj = cursor;
-            if (!parent)
-                break;
-            JS_PUSH_TEMP_ROOT_OBJECT(cx, obj, &tvr);
-        } else {
-            /*
-             * Avoid OBJ_SET_PARENT overhead as clonedChild cannot escape to
-             * other threads.
-             */
-            STOBJ_SET_PARENT(clonedChild, cursor);
-            if (!parent) {
-                JS_ASSERT(tvr.u.value == OBJECT_TO_JSVAL(obj));
-                JS_POP_TEMP_ROOT(cx, &tvr);
-                break;
-            }
-        }
-        clonedChild = cursor;
-        cursor = parent;
     }
-    fp->flags |= JSFRAME_POP_BLOCKS;
+
-    fp->scopeChain = obj;
+    /*
-    fp->blockChain = NULL;
+     * If we found a limit block belonging to this frame, then we should have
-    return obj;
+     * found it in blockChain.
+     */
+    JS_ASSERT_IF(limitBlock && 
+                 OBJ_GET_CLASS(cx, limitBlock) == &js_BlockClass && 
+                 OBJ_GET_PRIVATE(cx, limitClone) == fp,
+                 sharedBlock);
+
+    /* Place our newly cloned blocks at the head of the scope chain.  */
+    fp->scopeChain = innermostNewChild;
+    return fp->scopeChain;
 }
 
 JSBool
@@ -6741,51 +6784,58 @@ js_Interpret(JSContext *cx)
                 regs.sp++;
             }
 
+#ifdef DEBUG
+            JS_ASSERT(fp->blockChain == OBJ_GET_PARENT(cx, obj));
+
             /*
-             * If this frame had to reflect the compile-time block chain into
+             * The young end of fp->scopeChain may omit blocks if we
-             * the runtime scope chain, we can't optimize block scopes out of
+             * haven't closed over them, but if there are any closure
-             * runtime any longer, because an outer block that parents obj has
+             * blocks on fp->scopeChain, they'd better be (clones of)
-             * been cloned onto the scope chain.  To avoid re-cloning such a
+             * ancestors of the block we're entering now; anything
-             * parent and accumulating redundant clones via js_GetScopeChain,
+             * else we should have popped off fp->scopeChain when we
-             * we must clone each block eagerly on entry, and push it on the
+             * left its static scope.
-             * scope chain, until this frame pops.
              */
-            if (fp->flags & JSFRAME_POP_BLOCKS) {
+            obj2 = fp->scopeChain;
-                JS_ASSERT(!fp->blockChain);
+            while ((clasp = OBJ_GET_CLASS(cx, obj2)) == &js_WithClass)
-                obj = js_CloneBlockObject(cx, obj, fp->scopeChain, fp);
+                obj2 = OBJ_GET_PARENT(cx, obj2);
-                if (!obj)
+            if (clasp == &js_BlockClass &&
-                    goto error;
+                OBJ_GET_PRIVATE(cx, obj2) == fp) {
-                fp->scopeChain = obj;
+                JSObject *youngestProto = OBJ_GET_PROTO(cx, obj2);
-            } else {
+                JS_ASSERT(!OBJ_IS_CLONED_BLOCK(youngestProto));
-                JS_ASSERT(!fp->blockChain ||
+                parent = obj;
-                          OBJ_GET_PARENT(cx, obj) == fp->blockChain);
+                while ((parent = OBJ_GET_PARENT(cx, parent)) != youngestProto)
-                fp->blockChain = obj;
+                    JS_ASSERT(parent);
             }
+#endif
+
+            fp->blockChain = obj;
           END_CASE(JSOP_ENTERBLOCK)
 
           BEGIN_CASE(JSOP_LEAVEBLOCKEXPR)
           BEGIN_CASE(JSOP_LEAVEBLOCK)
           {
 #ifdef DEBUG
-             uintN blockDepth = OBJ_BLOCK_DEPTH(cx,
+            JS_ASSERT(OBJ_GET_CLASS(cx, fp->blockChain) == &js_BlockClass);
-                                                fp->blockChain
+            uintN blockDepth = OBJ_BLOCK_DEPTH(cx, fp->blockChain);
-                                                ? fp->blockChain
-                                                : fp->scopeChain);
              
-             JS_ASSERT(blockDepth <= StackDepth(script));
+            JS_ASSERT(blockDepth <= StackDepth(script));
 #endif
-            if (fp->blockChain) {
+            /*
-                JS_ASSERT(OBJ_GET_CLASS(cx, fp->blockChain) == &js_BlockClass);
+             * If we're about to leave the dynamic scope of a block that has
-                fp->blockChain = OBJ_GET_PARENT(cx, fp->blockChain);
+             * been cloned onto fp->scopeChain, clear its private data, move
-            } else {
+             * its locals from the stack into the clone, and pop it off the
-                /*
+             * chain.
-                 * This block was cloned into fp->scopeChain, so clear its
+             */
-                 * private data and sync its locals to their property slots.
+            obj = fp->scopeChain;
-                 */
+            if (OBJ_GET_PROTO(cx, obj) == fp->blockChain) {
+                JS_ASSERT (OBJ_GET_CLASS(cx, obj) == &js_BlockClass);
                 if (!js_PutBlockObject(cx, JS_TRUE))
                     goto error;
             }
 
+            /* Pop the block chain, too.  */
+            fp->blockChain = OBJ_GET_PARENT(cx, fp->blockChain);
+
             /*
              * We will move the result of the expression to the new topmost
              * stack slot.

js/src/jsinterp.h

@@ -82,13 +82,51 @@ struct JSStackFrame {
     jsval           rval;           /* function return value */
     JSStackFrame    *down;          /* previous frame */
     void            *annotation;    /* used by Java security */
-    JSObject        *scopeChain;    /* scope chain */
+
+    /*
+     * We can't determine in advance which local variables can live on
+     * the stack and be freed when their dynamic scope ends, and which
+     * will be closed over and need to live in the heap.  So we place
+     * variables on the stack initially, note when they are closed
+     * over, and copy those that are out to the heap when we leave
+     * their dynamic scope.
+     * 
+     * The bytecode compiler produces a tree of block objects
+     * accompanying each JSScript representing those lexical blocks in
+     * the script that have let-bound variables associated with them.
+     * These block objects are never modified, and never become part
+     * of any function's scope chain.  Their parent slots point to the
+     * innermost block that encloses them, or are NULL in the
+     * outermost blocks within a function or in eval or global code.
+     *
+     * When we are in the static scope of such a block, blockChain
+     * points to its compiler-allocated block object; otherwise, it is
+     * NULL.
+     * 
+     * scopeChain is the current scope chain, including 'call' and
+     * 'block' objects for those function calls and lexical blocks
+     * whose static scope we are currently executing in, and 'with'
+     * objects for with statements; the chain is typically terminated
+     * by a global object.  However, as an optimization, the young end
+     * of the chain omits block objects we have not yet cloned.  To
+     * create a closure, we clone the missing blocks from blockChain
+     * (which is always current), place them at the head of
+     * scopeChain, and use that for the closure's scope chain.  If we
+     * never close over a lexical block, we never place a mutable
+     * clone of it on scopeChain.
+     *
+     * This lazy cloning is implemented in js_GetScopeChain, which is
+     * also used in some other cases --- entering 'with' blocks, for
+     * example.
+     */
+    JSObject        *scopeChain;
+    JSObject        *blockChain;
+
     uintN           sharpDepth;     /* array/object initializer depth */
     JSObject        *sharpArray;    /* scope for #n= initializer vars */
     uint32          flags;          /* frame flags -- see below */
     JSStackFrame    *dormantNext;   /* next dormant frame chain */
     JSObject        *xmlNamespace;  /* null or default xml namespace in E4X */
-    JSObject        *blockChain;    /* active compile-time block scopes */
     JSStackFrame    *displaySave;   /* previous value of display entry for
                                        script->staticDepth */
 #ifdef DEBUG
@@ -140,7 +178,6 @@ typedef struct JSInlineFrame {
 #define JSFRAME_ROOTED_ARGV    0x20 /* frame.argv is rooted by the caller */
 #define JSFRAME_YIELDING       0x40 /* js_Interpret dispatched JSOP_YIELD */
 #define JSFRAME_ITERATOR       0x80 /* trying to get an iterator for for-in */
-#define JSFRAME_POP_BLOCKS    0x100 /* scope chain contains blocks to pop */
 #define JSFRAME_GENERATOR     0x200 /* frame belongs to generator-iterator */
 #define JSFRAME_IMACRO_START  0x400 /* imacro starting -- see jstracer.h */
 

js/src/jsiter.cpp

@@ -786,6 +786,8 @@ js_NewGenerator(JSContext *cx, JSStackFrame *fp)
     gen->frame.flags = (fp->flags & ~JSFRAME_ROOTED_ARGV) | JSFRAME_GENERATOR;
     gen->frame.dormantNext = NULL;
     gen->frame.xmlNamespace = NULL;
+    /* JSOP_GENERATOR appears in the prologue, outside all blocks.  */
+    JS_ASSERT(!fp->blockChain);
     gen->frame.blockChain = NULL;
 
     /* Note that gen is newborn. */

js/src/jstracer.cpp

@@ -1206,6 +1206,7 @@ TraceRecorder::TraceRecorder(JSContext* cx, VMSideExit* _anchor, Fragment* _frag
     this->cx = cx;
     this->traceMonitor = &JS_TRACE_MONITOR(cx);
     this->globalObj = JS_GetGlobalForObject(cx, cx->fp->scopeChain);
+    this->lexicalBlock = cx->fp->blockChain;
     this->anchor = _anchor;
     this->fragment = _fragment;
     this->lirbuf = _fragment->lirbuf;
@@ -3208,7 +3209,6 @@ js_SynthesizeFrame(JSContext* cx, const FrameInfo& fi)
     newifp->callerRegs.sp = fp->slots + fi.s.spdist;
     fp->imacpc = fi.imacpc;
 
-    JS_ASSERT(!(fp->flags & JSFRAME_POP_BLOCKS));
 #ifdef DEBUG
     if (fi.block != fp->blockChain) {
         for (JSObject* obj = fi.block; obj != fp->blockChain; obj = STOBJ_GET_PARENT(obj))
@@ -3948,7 +3948,6 @@ js_ExecuteTree(JSContext* cx, Fragment* f, uintN& inlineCallCount,
         return NULL;
 
 #ifdef DEBUG
-    state.jsframe_pop_blocks_set_on_entry = ((cx->fp->flags & JSFRAME_POP_BLOCKS) != 0);
     memset(stack_buffer, 0xCD, sizeof(stack_buffer));
     memset(state.global, 0xCD, (globalFrameSize+1)*sizeof(double));
 #endif
@@ -4134,8 +4133,6 @@ LeaveTree(InterpState& state, VMSideExit* lr)
        the side exit struct. */
     JSStackFrame* fp = cx->fp;
 
-    JS_ASSERT_IF(fp->flags & JSFRAME_POP_BLOCKS,
-                 calldepth == 0 && state.jsframe_pop_blocks_set_on_entry);
     fp->blockChain = innermost->block;
 
     /* If we are not exiting from an inlined frame the state->sp is spbase, otherwise spbase
@@ -9133,9 +9130,6 @@ TraceRecorder::record_JSOP_TYPEOFEXPR()
 JS_REQUIRES_STACK bool
 TraceRecorder::record_JSOP_ENTERBLOCK()
 {
-    if (cx->fp->flags & JSFRAME_POP_BLOCKS)
-        ABORT_TRACE("can't trace after js_GetScopeChain");
-
     JSScript* script = cx->fp->script;
     JSFrameRegs& regs = *cx->fp->regs;
     JSObject* obj;
@@ -9150,7 +9144,8 @@ TraceRecorder::record_JSOP_ENTERBLOCK()
 JS_REQUIRES_STACK bool
 TraceRecorder::record_JSOP_LEAVEBLOCK()
 {
-    return true;
+    /* We mustn't exit the lexical block we began recording in.  */
+    return cx->fp->blockChain != lexicalBlock;
 }
 
 JS_REQUIRES_STACK bool

js/src/jstracer.h

@@ -382,6 +382,7 @@ class TraceRecorder : public avmplus::GCObject {
     JSContext*              cx;
     JSTraceMonitor*         traceMonitor;
     JSObject*               globalObj;
+    JSObject*               lexicalBlock;
     Tracker                 tracker;
     Tracker                 nativeFrameTracker;
     char*                   entryTypeMap;