Make sure that we clean up block objects that were cloned into the scope chain for environment capture, so they don't reference dead frame pointers or dead stack slots, and instead have property-based copies of their locals. bug 343765, r=brendan

js/src/jsfun.c

@@ -579,8 +579,6 @@ js_PutCallObject(JSContext *cx, JSStackFrame *fp)
 {
     JSObject *callobj;
     JSBool ok;
-    JSObject *obj;
-    JSScopeProperty *sprop;
     jsid argsid;
     jsval aval;
 
@@ -593,17 +591,6 @@ js_PutCallObject(JSContext *cx, JSStackFrame *fp)
         return JS_TRUE;
     ok = call_enumerate(cx, callobj);
 
-    /*
-     * Walk the scope chain looking for block scopes whose locals need to be
-     * copied from stack slots into object slots before fp goes away.
-     */
-    for (obj = fp->scopeChain; obj; obj = OBJ_GET_PARENT(cx, obj)) {
-        if (OBJ_GET_CLASS(cx, obj) == &js_BlockClass) {
-            for (sprop = OBJ_SCOPE(obj)->lastProp; sprop; sprop = sprop->parent)
-                ok &= OBJ_GET_PROPERTY(cx, obj, sprop->id, &aval);
-        }
-    }
-
     /*
      * Get the arguments object to snapshot fp's actual argument values.
      */

js/src/jsinterp.c

@@ -486,6 +486,24 @@ CloneBlockChain(JSContext *cx, JSStackFrame *fp, JSObject *obj)
     return js_CloneBlockObject(cx, obj, parent, fp);
 }
 
+static JSBool
+PutBlockObjects(JSContext *cx, JSStackFrame *fp)
+{
+    JSBool ok;
+    JSObject *obj;
+
+    /*
+     * Walk the scope chain looking for block scopes whose locals need to be
+     * copied from stack slots into object slots before fp goes away.
+     */
+    ok = JS_TRUE;
+    for (obj = fp->scopeChain; obj; obj = OBJ_GET_PARENT(cx, obj)) {
+        if (OBJ_GET_CLASS(cx, obj) == &js_BlockClass)
+            ok &= js_PutBlockObject(cx, obj);
+    }
+    return ok;
+}
+
 JSObject *
 js_GetScopeChain(JSContext *cx, JSStackFrame *fp)
 {
@@ -496,6 +514,7 @@ js_GetScopeChain(JSContext *cx, JSStackFrame *fp)
         obj = CloneBlockChain(cx, fp, obj);
         if (!obj)
             return NULL;
+        fp->flags |= JSFRAME_POP_BLOCKS;
         fp->scopeChain = obj;
         fp->blockChain = NULL;
         return obj;
@@ -1360,6 +1379,10 @@ out:
             hook(cx, &frame, JS_FALSE, &ok, hookData);
     }
 
+    /* If frame has block objects on its scope chain, cut them loose. */
+    if (frame.flags & JSFRAME_POP_BLOCKS)
+        ok &= PutBlockObjects(cx, &frame);
+
     /* If frame has a call object, sync values and clear back-pointer. */
     if (frame.callobj)
         ok &= js_PutCallObject(cx, &frame);
@@ -2342,8 +2365,14 @@ interrupt:
                     }
                 }
 
+                /* If fp has blocks on its scope chain, cut them loose. */
+                if (fp->flags & JSFRAME_POP_BLOCKS) {
+                    SAVE_SP_AND_PC(fp);
+                    ok &= PutBlockObjects(cx, fp);
+                }
+
                 /*
-                 * If frame has a call object, sync values and clear the back-
+                 * If fp has a call object, sync values and clear the back-
                  * pointer. This can happen for a lightweight function if it
                  * calls eval unexpectedly (in a way that is hidden from the
                  * compiler). See bug 325540.
@@ -5428,23 +5457,33 @@ interrupt:
           BEGIN_CASE(JSOP_SETSP)
             i = (jsint) GET_ATOM_INDEX(pc);
             JS_ASSERT(i >= 0);
-            sp = fp->spbase + i;
 
             for (obj = fp->blockChain; obj; obj = OBJ_GET_PARENT(cx, obj)) {
                 JS_ASSERT(OBJ_GET_CLASS(cx, obj) == &js_BlockClass);
-                if (OBJ_BLOCK_DEPTH(cx, obj) <= i)
+                if (OBJ_BLOCK_DEPTH(cx, obj) < i)
                     break;
             }
             fp->blockChain = obj;
 
+            JS_ASSERT(ok);
             for (obj = fp->scopeChain;
                  (clasp = OBJ_GET_CLASS(cx, obj)) == &js_WithClass ||
                  clasp == &js_BlockClass;
                  obj = OBJ_GET_PARENT(cx, obj)) {
-                if (OBJ_BLOCK_DEPTH(cx, obj) <= i)
+                if (OBJ_BLOCK_DEPTH(cx, obj) < i)
                     break;
+                if (clasp == &js_BlockClass)
+                    ok &= js_PutBlockObject(cx, obj);
             }
+
             fp->scopeChain = obj;
+
+            /* Set sp after js_PutBlockObject to avoid potential GC hazards. */
+            sp = fp->spbase + i;
+
+            /* Don't fail until after we've updated all stacks. */
+            if (!ok)
+                goto out;
           END_CASE(JSOP_SETSP)
 
           BEGIN_CASE(JSOP_GOSUB)
@@ -5939,6 +5978,14 @@ interrupt:
             if (!obj) {
                 chainp = &fp->scopeChain;
                 obj = *chainp;
+
+                /*
+                 * This block was cloned, so clear its private data and sync
+                 * its locals to their property slots.
+                 */
+                ok = js_PutBlockObject(cx, obj);
+                if (!ok)
+                    goto out;
             }
             JS_ASSERT(OBJ_GET_CLASS(cx, obj) == &js_BlockClass);
             JS_ASSERT(op == JSOP_LEAVEBLOCKEXPR

js/src/jsinterp.h

@@ -108,6 +108,7 @@ typedef struct JSInlineFrame {
 #define JSFRAME_YIELDING      0x200 /* js_Interpret dispatched JSOP_YIELD */
 #define JSFRAME_FILTERING     0x400 /* XML filtering predicate expression */
 #define JSFRAME_ITERATOR      0x800 /* trying to get an iterator for for-in */
+#define JSFRAME_POP_BLOCKS   0x1000 /* scope chain contains blocks to pop */
 
 #define JSFRAME_OVERRIDE_SHIFT 24   /* override bit-set params; see jsfun.c */
 #define JSFRAME_OVERRIDE_BITS  8

js/src/jsobj.c

@@ -1901,6 +1901,22 @@ js_CloneBlockObject(JSContext *cx, JSObject *proto, JSObject *parent,
     return clone;
 }
 
+JSBool
+js_PutBlockObject(JSContext *cx, JSObject *obj)
+{
+    JSScopeProperty *sprop;
+    jsval v;
+
+    for (sprop = OBJ_SCOPE(obj)->lastProp; sprop; sprop = sprop->parent) {
+        if (!OBJ_GET_PROPERTY(cx, obj, sprop->id, &v)) {
+            JS_SetPrivate(cx, obj, NULL);
+            return JS_FALSE;
+        }
+    }
+
+    return JS_SetPrivate(cx, obj, NULL);
+}
+
 static JSBool
 block_getProperty(JSContext *cx, JSObject *obj, jsval id, jsval *vp)
 {

js/src/jsobj.h

@@ -282,6 +282,9 @@ extern JSObject *
 js_CloneBlockObject(JSContext *cx, JSObject *proto, JSObject *parent,
                     JSStackFrame *fp);
 
+extern JSBool
+js_PutBlockObject(JSContext *cx, JSObject *obj);
+
 struct JSSharpObjectMap {
     jsrefcount  depth;
     jsatomid    sharpgen;