Bug 488034 - Crash [@ js_GetUpvar] or "Assertion failure: (script)->upvarsOffset != 0, at ../jsinterp.cpp" (r=mrbkap).
authorBrendan Eich <brendan@mozilla.org>
Mon, 13 Apr 2009 14:16:15 -0700
changeset 24910 2f83aa29331b24bf661a07e2bfd42c377fb65924
parent 24909 ea7424828fb1e3c00512710c6bda836544c2a7e8
child 24911 6fd196ae4f20e08fac113d3de838518c1fe0c69f
push id1267
push userrsayre@mozilla.com
push dateSun, 19 Apr 2009 02:47:24 +0000
reviewersmrbkap
bugs488034
milestone1.9.1b4pre
Bug 488034 - Crash [@ js_GetUpvar] or "Assertion failure: (script)->upvarsOffset != 0, at ../jsinterp.cpp" (r=mrbkap).
js/src/jsemit.cpp
--- a/js/src/jsemit.cpp
+++ b/js/src/jsemit.cpp
@@ -1822,16 +1822,86 @@ EmitEnterBlock(JSContext *cx, JSParseNod
     }
 
     blockObj->map->freeslot = JSSLOT_FREE(&js_BlockClass);
     js_ReallocSlots(cx, blockObj, JSSLOT_FREE(&js_BlockClass), JS_TRUE);
     return true;
 }
 
 /*
+ * When eval is called from a function, the eval code or function code it
+ * compiles may reference upvars that live in the eval-calling function. The
+ * eval-invoked compiler does not have explicit definitions for these upvars
+ * and we do not attempt to create them a-priori (by inspecting the function's
+ * args and vars) -- we could, but we'd take an avoidable penalty for each
+ * function local not referenced by any upvar. Instead, we map such upvars
+ * lazily, growing upvarMap.vector by powers of two.
+ *
+ * This function knows that it is called with pn pointing to a PN_NAME-arity
+ * node, and cg->compiler->callerFrame having a non-null fun member, and the
+ * static level of cg at least one greater than the eval-calling function's
+ * static level.
+ */
+static bool
+MakeUpvarForEval(JSParseNode *pn, JSCodeGenerator *cg)
+{
+    JSContext *cx = cg->compiler->context;
+    JSFunction *fun = cg->compiler->callerFrame->fun;
+    JSAtom *atom = pn->pn_atom;
+
+    uintN index;
+    JSLocalKind localKind = js_LookupLocal(cx, fun, atom, &index);
+    if (localKind == JSLOCAL_NONE)
+        return true;
+
+    uintN upvarLevel = fun->u.i.script->staticLevel;
+    JS_ASSERT(cg->staticLevel > upvarLevel);
+    if (upvarLevel >= JS_DISPLAY_SIZE)
+        return true;
+
+    JSAtomListElement *ale = cg->upvarList.lookup(atom);
+    if (!ale) {
+        if ((cg->flags & TCF_IN_FUNCTION) &&
+            !js_AddLocal(cx, cg->fun, atom, JSLOCAL_UPVAR)) {
+            return false;
+        }
+
+        ale = cg->upvarList.add(cg->compiler, atom);
+        if (!ale)
+            return false;
+        JS_ASSERT(ALE_INDEX(ale) == cg->upvarList.count - 1);
+
+        uint32 *vector = cg->upvarMap.vector;
+        uint32 length = cg->upvarMap.length;
+
+        JS_ASSERT(ALE_INDEX(ale) <= length);
+        if (ALE_INDEX(ale) == length) {
+            length = 2 * JS_MAX(2, length);
+            vector = (uint32 *) JS_realloc(cx, vector, length * sizeof *vector);
+            if (!vector)
+                return false;
+            cg->upvarMap.vector = vector;
+            cg->upvarMap.length = length;
+        }
+
+        if (localKind != JSLOCAL_ARG)
+            index += fun->nargs;
+        JS_ASSERT(index < JS_BIT(16));
+
+        uintN skip = cg->staticLevel - upvarLevel;
+        vector[ALE_INDEX(ale)] = MAKE_UPVAR_COOKIE(skip, index);
+    }
+
+    pn->pn_op = JSOP_GETUPVAR;
+    pn->pn_cookie = MAKE_UPVAR_COOKIE(cg->staticLevel, ALE_INDEX(ale));
+    pn->pn_dflags |= PND_BOUND;
+    return true;
+}
+
+/*
  * BindNameToSlot attempts to optimize name gets and sets to stack slot loads
  * and stores, given the compile-time information in cg and a TOK_NAME node pn.
  * It returns false on error, true on success.
  *
  * The caller can inspect pn->pn_cookie for FREE_UPVAR_COOKIE to tell whether
  * optimization occurred, in which case BindNameToSlot also updated pn->pn_op.
  * If pn->pn_cookie is still FREE_UPVAR_COOKIE on return, pn->pn_op still may
  * have been optimized, e.g., from JSOP_NAME to JSOP_CALLEE.  Whether or not
@@ -1901,92 +1971,64 @@ BindNameToSlot(JSContext *cx, JSCodeGene
       case JSOP_DELNAME:
         break;
       default:
         if (pn->isConst())
             pn->pn_op = op = JSOP_NAME;
     }
 
     if (cookie == FREE_UPVAR_COOKIE) {
-        /* If free variable reference, leave pn_op as JSOP_NAME, etc. */
-        if (cg->flags & TCF_IN_FUNCTION)
-            return JS_TRUE;
-
         JSStackFrame *caller = cg->compiler->callerFrame;
         if (caller) {
             JS_ASSERT(cg->flags & TCF_COMPILE_N_GO);
 
             /*
              * Don't generate upvars on the left side of a for loop. See
              * bug 470758.
              */
             if (cg->flags & TCF_IN_FOR_INIT)
                 return JS_TRUE;
 
             JS_ASSERT(caller->script);
-            if (!caller->fun || caller->varobj != cg->scopeChain)
+            if (!caller->fun)
+                return JS_TRUE;
+
+            /*
+             * Make sure the variable object used by the compiler to initialize
+             * parent links matches the caller's varobj. Compile-n-go compiler-
+             * created function objects have the top-level cg's scopeChain set
+             * as their parent by JSCompiler::newFunction.
+             */
+            JSObject *scopeobj = (cg->flags & TCF_IN_FUNCTION)
+                                 ? STOBJ_GET_PARENT(FUN_OBJECT(cg->fun))
+                                 : cg->scopeChain;
+            if (scopeobj != caller->varobj)
                 return JS_TRUE;
 
             /*
              * We are compiling eval or debug script inside a function frame
              * and the scope chain matches the function's variable object.
              * Optimize access to function's arguments and variable and the
              * arguments object.
              */
             if (op != JSOP_NAME)
                 return JS_TRUE;
 
-            JSLocalKind localKind = js_LookupLocal(cx, caller->fun, atom, &index);
-            if (localKind == JSLOCAL_NONE)
-                return JS_TRUE;
-
-            uintN upvarLevel = caller->fun->u.i.script->staticLevel;
-            JS_ASSERT(cg->staticLevel > upvarLevel);
-            if (upvarLevel >= JS_DISPLAY_SIZE)
-                return JS_TRUE;
-
-            ale = cg->upvarList.lookup(atom);
-            if (!ale) {
-                uint32 length, *vector;
-
-                ale = cg->upvarList.add(cg->compiler, atom);
-                if (!ale)
-                    return JS_FALSE;
-                JS_ASSERT(ALE_INDEX(ale) == cg->upvarList.count - 1);
-
-                length = cg->upvarMap.length;
-                JS_ASSERT(ALE_INDEX(ale) <= length);
-                if (ALE_INDEX(ale) == length) {
-                    length = 2 * JS_MAX(2, length);
-                    vector = (uint32 *)
-                             JS_realloc(cx, cg->upvarMap.vector,
-                                        length * sizeof *vector);
-                    if (!vector)
-                        return JS_FALSE;
-                    cg->upvarMap.vector = vector;
-                    cg->upvarMap.length = length;
-                }
-
-                if (localKind != JSLOCAL_ARG)
-                    index += caller->fun->nargs;
-                JS_ASSERT(index < JS_BIT(16));
-
-                uintN skip = cg->staticLevel - upvarLevel;
-                cg->upvarMap.vector[ALE_INDEX(ale)] = MAKE_UPVAR_COOKIE(skip, index);
-            }
-
-            pn->pn_op = JSOP_GETUPVAR;
-            pn->pn_cookie = ALE_INDEX(ale);
-            pn->pn_dflags |= PND_BOUND;
+            return MakeUpvarForEval(pn, cg);
         }
         return JS_TRUE;
     }
 
     if (dn->pn_dflags & PND_GVAR) {
-        /* If free variable reference, leave pn_op as JSOP_NAME, etc. */
+        /*
+         * If this is a global reference from within a function, leave pn_op as
+         * JSOP_NAME, etc. We could emit JSOP_*GVAR ops within function code if
+         * only we could depend on the global frame's slots being valid for all
+         * calls to the function.
+         */
         if (cg->flags & TCF_IN_FUNCTION)
             return JS_TRUE;
 
         /*
          * We are optimizing global variables and there may be no pre-existing
          * global property named atom when this global script runs. If atom was
          * declared via const or var, optimize pn to access fp->vars using the
          * appropriate JSOP_*GVAR op.
@@ -2008,36 +2050,58 @@ BindNameToSlot(JSContext *cx, JSCodeGene
           default: JS_NOT_REACHED("gvar");
         }
         pn->pn_op = op;
         pn->pn_cookie = cookie;
         pn->pn_dflags |= PND_BOUND;
         return JS_TRUE;
     }
 
+    uintN level = UPVAR_FRAME_SKIP(cookie);
+    JS_ASSERT(cg->staticLevel >= level);
+
     /*
      * A JSDefinition witnessed as a declaration by the parser cannot be an
-     * upvar, unless it is the degenerate kind of upvar selected immediately
-     * below for the special case of compile-and-go code generated from eval
-     * called from a function, where the eval code uses function locals. Before
-     * misinterpreting the frame-skip half of dn->pn_cookie in such a bound
-     * upvar pseudo-definition we detect this case by checking dn->pn_op.
+     * upvar, unless it is the degenerate kind of upvar selected above (in the
+     * code before the PND_GVAR test) for the special case of compile-and-go
+     * code generated from eval called from a function, where the eval code
+     * uses local vars defined in the function. We detect this upvar-for-eval
+     * case by checking dn's op.
      */
     if (PN_OP(dn) == JSOP_GETUPVAR) {
-        if (op == JSOP_NAME) {
+        JS_ASSERT(cg->staticLevel >= level);
+        if (op != JSOP_NAME)
+            return JS_TRUE;
+
+#ifdef DEBUG
+        JSStackFrame *caller = cg->compiler->callerFrame;
+        JS_ASSERT(caller);
+
+        JSTreeContext *tc = cg;
+        while (tc->staticLevel != level)
+            tc = tc->parent;
+        JS_ASSERT(tc->flags & TCF_COMPILING);
+
+        JSCodeGenerator *evalcg = (JSCodeGenerator *) tc;
+        JS_ASSERT(evalcg->flags & TCF_COMPILE_N_GO);
+        JS_ASSERT(!(evalcg->flags & TCF_IN_FOR_INIT));
+        JS_ASSERT(caller->script);
+        JS_ASSERT(caller->fun && caller->varobj == evalcg->scopeChain);
+#endif
+
+        if (cg->staticLevel == level) {
             pn->pn_op = JSOP_GETUPVAR;
-            pn->pn_cookie = dn->pn_cookie;
+            pn->pn_cookie = cookie;
             pn->pn_dflags |= PND_BOUND;
-        }
-        return JS_TRUE;
+            return JS_TRUE;
+        }
+
+        return MakeUpvarForEval(pn, cg);
     }
 
-    uintN level = UPVAR_FRAME_SKIP(cookie);
-    JS_ASSERT(cg->staticLevel >= level);
-
     uintN skip = cg->staticLevel - level;
     if (skip != 0) {
         JS_ASSERT(cg->flags & TCF_IN_FUNCTION);
         JS_ASSERT(cg->upvars.lookup(atom));
         JS_ASSERT(JOF_OPTYPE(op) == JOF_ATOM);
 
         /*
          * If op is a mutating opcode, this upvar's static level is too big to