ES5-conformance fix to make a new RegExp for each regexp literal evaluation (98409, r=igor/jwalden).

js/src/jsemit.cpp

@@ -6707,16 +6707,24 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn)
         ok = EmitNumberOp(cx, pn->pn_dval, cg);
         break;
 
-      case TOK_REGEXP:
+      case TOK_REGEXP: {
         /*
-         * If the regexp's script is one-shot, we can avoid the extra
-         * fork-on-exec costs of JSOP_REGEXP by selecting JSOP_OBJECT.
-         * Otherwise, to avoid incorrect proto, parent, and lastIndex
-         * sharing among threads and sequentially across re-execution,
-         * select JSOP_REGEXP.
+         * If the regexp's script is one-shot and the regexp is not used in a
+         * loop, we can avoid the extra fork-on-exec costs of JSOP_REGEXP by
+         * selecting JSOP_OBJECT. Otherwise, to avoid incorrect proto, parent,
+         * and lastIndex sharing, select JSOP_REGEXP.
          */
         JS_ASSERT(pn->pn_op == JSOP_REGEXP);
-        if (cg->flags & TCF_COMPILE_N_GO) {
+        bool singleton = !cg->fun && (cg->flags & TCF_COMPILE_N_GO);
+        if (singleton) {
+            for (JSStmtInfo *stmt = cg->topStmt; stmt; stmt = stmt->down) {
+                if (STMT_IS_LOOP(stmt)) {
+                    singleton = false;
+                    break;
+                }
+            }
+        }
+        if (singleton) {
             ok = EmitObjectOp(cx, pn->pn_objbox, JSOP_OBJECT, cg);
         } else {
             ok = EmitIndexOp(cx, JSOP_REGEXP,
@@ -6724,6 +6732,7 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn)
                              cg);
         }
         break;
+      }
 
 #if JS_HAS_XML_SUPPORT
       case TOK_ANYNAME:

js/src/jsfun.cpp

@@ -1785,12 +1785,7 @@ JSFunction::countInterpretedReservedSlots() const
 {
     JS_ASSERT(FUN_INTERPRETED(this));
 
-    uint32 nslots = (u.i.nupvars == 0)
-                    ? 0
-                    : u.i.script->upvars()->length;
-    if (u.i.script->regexpsOffset != 0)
-        nslots += u.i.script->regexps()->length;
-    return nslots;
+    return (u.i.nupvars == 0) ? 0 : u.i.script->upvars()->length;
 }
 
 static uint32

js/src/jsinterp.h

@@ -181,13 +181,8 @@ JSStackFrame::assertValidStackDepth(uintN depth)
 static JS_INLINE uintN
 GlobalVarCount(JSStackFrame *fp)
 {
-    uintN n;
-
     JS_ASSERT(!fp->fun);
-    n = fp->script->nfixed;
-    if (fp->script->regexpsOffset != 0)
-        n -= fp->script->regexps()->length;
-    return n;
+    return fp->script->nfixed;
 }
 
 typedef struct JSInlineFrame {

js/src/jsops.cpp

@@ -2438,117 +2438,21 @@ BEGIN_CASE(JSOP_OBJECT)
 END_CASE(JSOP_OBJECT)
 
 BEGIN_CASE(JSOP_REGEXP)
-{
-    JSObject *funobj;
-
     /*
-     * Push a regexp object for the atom mapped by the bytecode at pc, cloning
-     * the literal's regexp object if necessary, to simulate in the
-     * pre-compile/execute-later case what ECMA specifies for the
-     * compile-and-go case: that scanning each regexp literal creates a single
-     * corresponding RegExp object.
-     *
-     * To support pre-compilation transparently, we must handle the case where
-     * a regexp object literal is used in a different global at execution time
-     * from the global with which it was scanned at compile time.  We do this
-     * by re-wrapping the JSRegExp private data struct with a cloned object
-     * having the right prototype and parent, and having its own lastIndex
-     * property value storage.
-     *
-     * Unlike JSOP_DEFFUN and other prolog bytecodes that may clone literal
-     * objects, we don't want to pay a script prolog execution price for all
-     * regexp literals in a script (many may not be used by a particular
-     * execution of that script, depending on control flow), so we initialize
-     * lazily here.
+     * Push a regexp object cloned from the regexp literal object mapped by the
+     * bytecode at pc. ES5 finally fixed this bad old ES3 design flaw which was
+     * flouted by many browser-based implementations.
      *
      * XXX This code is specific to regular expression objects.  If we need a
      * similar op for other kinds of object literals, we should push cloning
      * down under JSObjectOps and reuse code here.
      */
     index = GET_FULL_INDEX(0);
-    JS_ASSERT(index < script->regexps()->length);
-
-    slot = index;
-    if (fp->fun) {
-        /*
-         * We're in function code, not global or eval code (in eval code,
-         * JSOP_REGEXP is never emitted). The cloned funobj contains
-         * script->regexps()->length reserved slots for the cloned regexps; see
-         * fun_reserveSlots, jsfun.c.
-         */
-        funobj = JSVAL_TO_OBJECT(fp->argv[-2]);
-        slot += JSCLASS_RESERVED_SLOTS(&js_FunctionClass);
-        if (script->upvarsOffset != 0)
-            slot += script->upvars()->length;
-        if (!JS_GetReservedSlot(cx, funobj, slot, &rval))
-            goto error;
-        if (JSVAL_IS_VOID(rval))
-            rval = JSVAL_NULL;
-    } else {
-        /*
-         * We're in global code. The code generator reserved a slot for the
-         * regexp among script->nfixed slots. All such slots are initialized to
-         * null, not void, for faster testing in JSOP_*GVAR cases. To simplify
-         * index calculations we count regexps in the reverse order down from
-         * script->nslots - 1.
-         */
-        JS_ASSERT(slot < script->nfixed);
-        slot = script->nfixed - slot - 1;
-        rval = fp->slots[slot];
-#ifdef __GNUC__
-        funobj = NULL;  /* suppress bogus gcc warnings */
-#endif
-    }
-
-    if (JSVAL_IS_NULL(rval)) {
-        /* Compute the current global object in obj2. */
-        obj2 = fp->scopeChain;
-        while ((parent = OBJ_GET_PARENT(cx, obj2)) != NULL)
-            obj2 = parent;
-
-        /*
-         * If obj's parent is not obj2, we must clone obj so that it has the
-         * right parent, and therefore, the right prototype.
-         *
-         * Yes, this means we assume that the correct RegExp.prototype to which
-         * regexp instances (including literals) delegate can be distinguished
-         * solely by the instance's parent, which was set to the parent of the
-         * RegExp constructor function object when the instance was created.
-         * In other words,
-         *
-         *   (/x/.__parent__ == RegExp.__parent__) implies
-         *   (/x/.__proto__ == RegExp.prototype)
-         *
-         * (unless you assign a different object to RegExp.prototype at
-         * runtime, in which case, ECMA doesn't specify operation, and you get
-         * what you deserve).
-         *
-         * This same coupling between instance parent and constructor parent
-         * turns up everywhere (see jsobj.c's FindClassObject,
-         * js_ConstructObject, and js_NewObject).  It's fundamental to the
-         * design of the language when you consider multiple global objects and
-         * separate compilation and execution, even though it is not specified
-         * fully in ECMA.
-         */
-        obj = script->getRegExp(index);
-        if (OBJ_GET_PARENT(cx, obj) != obj2) {
-            obj = js_CloneRegExpObject(cx, obj, obj2);
+    obj = js_CloneRegExpObject(cx, script->getRegExp(index), NULL);
     if (!obj)
         goto error;
-        }
     rval = OBJECT_TO_JSVAL(obj);
-
-        /* Store the regexp object value in its cloneIndex slot. */
-        if (fp->fun) {
-            if (!JS_SetReservedSlot(cx, funobj, slot, rval))
-                goto error;
-        } else {
-            fp->slots[slot] = rval;
-        }
-    }
-
     PUSH_OPND(rval);
-}
 END_CASE(JSOP_REGEXP)
 
 BEGIN_CASE(JSOP_ZERO)

js/src/jsparse.cpp

@@ -951,11 +951,11 @@ JSCompiler::compileScript(JSContext *cx, JSObject *scopeChain, JSStackFrame *cal
 #endif
 
     /*
-     * Global variables and regexps share the index space with locals. Due to
+     * Global variables (gvars) share the atom index space with locals. Due to
      * incremental code generation we need to patch the bytecode to adjust the
      * local references to skip the globals.
      */
-    scriptGlobals = cg.ngvars + cg.regexpList.length;
+    scriptGlobals = cg.ngvars;
     if (scriptGlobals != 0 || cg.hasSharps()) {
         jsbytecode *code, *end;
         JSOp op;

js/src/jsscript.cpp

@@ -1596,7 +1596,7 @@ js_NewScriptFromCG(JSContext *cx, JSCodeGenerator *cg)
     memcpy(script->main, CG_BASE(cg), mainLength * sizeof(jsbytecode));
     nfixed = (cg->flags & TCF_IN_FUNCTION)
              ? cg->fun->u.i.nvars
-             : cg->ngvars + cg->regexpList.length + cg->sharpSlots();
+             : cg->ngvars + cg->sharpSlots();
     JS_ASSERT(nfixed < SLOTNO_LIMIT);
     script->nfixed = (uint16) nfixed;
     js_InitAtomMap(cx, &script->atomMap, &cg->atomList);

js/src/tests/js1_5/GC/regress-324278.js

@@ -51,15 +51,20 @@ printStatus (summary);
 var N = 100*1000;
 
 function build(N) {
-  // Explore the fact that regexp literals are shared between
-  // function invocations. Thus we build the following chain:
+  // We used to exploit the fact that regexp literals were shared between
+  // function invocations, but ES5 fixes this design flaw, so we have to
+  // make a regexp for each new function f, and store it as a property of f.
+  // Thus we build the following chain:
+  //
   //   chainTop: function->regexp->function->regexp....->null
+  //
   // to check how GC would deal with this chain.
 
   var chainTop = null;
   for (var i = 0; i != N; ++i) {
-    var f = Function('some_arg'+i, ' return /test/;');
-    var re = f();
+    var f = Function('some_arg'+i, ' return some_arg'+i+'.re;');
+    var re = /test/;
+    f.re = re;
     re.previous = chainTop;
     chainTop = f;
   }
@@ -68,7 +73,7 @@ function build(N) {
 
 function check(chainTop, N) {
   for (var i = 0; i != N; ++i) {
-    var re = chainTop();
+    var re = chainTop(chainTop);
     chainTop = re.previous;
   }
   if (chainTop !== null)