Bug 1145491 part 7. Stop checking compileAndGo before emitting GNAME ops. r=luke
☠☠ backed out by 3b7a4d9da546 ☠ ☠
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 20 Mar 2015 21:34:19 -0400
changeset 263681 8d14c6661f003cbe0aabf38268bddf2b20abba04
parent 263680 7be39afdf528795236b3d2bd82ff796947e195a7
child 263682 1b0a6e932bfb339d83c722fe39986cb21c7b3c67
push id4718
push userraliiev@mozilla.com
push dateMon, 11 May 2015 18:39:53 +0000
treeherdermozilla-beta@c20c4ef55f08 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs1145491
milestone39.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 1145491 part 7. Stop checking compileAndGo before emitting GNAME ops. r=luke
js/src/builtin/TestingFunctions.cpp
js/src/frontend/BytecodeCompiler.cpp
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/BytecodeEmitter.h
js/src/jit-test/tests/basic/bug1106982-2.js
js/src/jit-test/tests/basic/bug1106982.js
js/src/jit-test/tests/basic/eval-scopes.js
js/src/vm/Xdr.h
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -2427,16 +2427,33 @@ DumpStringRepresentation(JSContext *cx, 
 
     str->dumpRepresentation(stderr, 0);
 
     args.rval().setUndefined();
     return true;
 }
 #endif
 
+static bool
+SetLazyParsingEnabled(JSContext *cx, unsigned argc, Value *vp)
+{
+    CallArgs args = CallArgsFromVp(argc, vp);
+
+    if (argc < 1) {
+        JS_ReportError(cx, "setLazyParsingEnabled: need an argument");
+        return false;
+    }
+
+    bool arg = ToBoolean(args.get(0));
+    JS::CompartmentOptionsRef(cx->compartment()).setDiscardSource(!arg);
+
+    args.rval().setUndefined();
+    return true;
+}
+
 static const JSFunctionSpecWithHelp TestingFunctions[] = {
     JS_FN_HELP("gc", ::GC, 0, 0,
 "gc([obj] | 'compartment' [, 'shrinking'])",
 "  Run the garbage collector. When obj is given, GC only its compartment.\n"
 "  If 'compartment' is given, GC any compartments that were scheduled for\n"
 "  GC via schedulegc.\n"
 "  If 'shrinking' is passed as the optional second argument, perform a\n"
 "  shrinking GC rather than a normal GC."),
@@ -2809,16 +2826,20 @@ gc::ZealModeHelpText),
 "  because the object is a revoked proxy)."),
 
 #ifdef DEBUG
     JS_FN_HELP("dumpStringRepresentation", DumpStringRepresentation, 1, 0,
 "dumpStringRepresentation(str)",
 "  Print a human-readable description of how the string |str| is represented.\n"),
 #endif
 
+    JS_FN_HELP("setLazyParsingEnabled", SetLazyParsingEnabled, 1, 0,
+"setLazyParsingEnabled(bool)",
+"  Enable or disable lazy parsing in the current compartment.  The default is enabled."),
+
     JS_FS_HELP_END
 };
 
 static const JSPropertySpec TestingProperties[] = {
     JS_PSG("timesAccessed", TimesAccessed, 0),
     JS_PS_END
 };
 
--- a/js/src/frontend/BytecodeCompiler.cpp
+++ b/js/src/frontend/BytecodeCompiler.cpp
@@ -287,27 +287,24 @@ frontend::CompileScript(ExclusiveContext
 
     bool savedCallerFun = evalCaller && evalCaller->functionOrCallerFunction();
     Rooted<JSScript*> script(cx, JSScript::Create(cx, evalStaticScope, savedCallerFun,
                                                   options, staticLevel, sourceObject, 0,
                                                   srcBuf.length()));
     if (!script)
         return nullptr;
 
-    // We can specialize a bit for the given scope chain if that scope chain is the global object.
-    JSObject *globalScope =
-        scopeChain && scopeChain == &scopeChain->global() ? (JSObject*) scopeChain : nullptr;
-    MOZ_ASSERT_IF(globalScope, globalScope->isNative());
-    MOZ_ASSERT_IF(globalScope, JSCLASS_HAS_GLOBAL_FLAG_AND_SLOTS(globalScope->getClass()));
-
+    bool insideNonGlobalEval =
+        evalStaticScope && evalStaticScope->enclosingScopeForStaticScopeIter();
     BytecodeEmitter::EmitterMode emitterMode =
         options.selfHostingMode ? BytecodeEmitter::SelfHosting : BytecodeEmitter::Normal;
     BytecodeEmitter bce(/* parent = */ nullptr, &parser, &globalsc, script,
                         /* lazyScript = */ js::NullPtr(), options.forEval,
-                        evalCaller, evalStaticScope, !!globalScope, options.lineno, emitterMode);
+                        evalCaller, evalStaticScope, insideNonGlobalEval,
+                        options.lineno, emitterMode);
     if (!bce.init())
         return nullptr;
 
     // Syntax parsing may cause us to restart processing of top level
     // statements in the script. Use Maybe<> so that the parse context can be
     // reset when this occurs.
     Maybe<ParseContext<FullParseHandler> > pc;
 
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -112,17 +112,18 @@ struct LoopStmtInfo : public StmtInfoBCE
 
 } // anonymous namespace
 
 BytecodeEmitter::BytecodeEmitter(BytecodeEmitter *parent,
                                  Parser<FullParseHandler> *parser, SharedContext *sc,
                                  HandleScript script, Handle<LazyScript *> lazyScript,
                                  bool insideEval, HandleScript evalCaller,
                                  Handle<StaticEvalObject *> staticEvalScope,
-                                 bool hasGlobalScope, uint32_t lineNum, EmitterMode emitterMode)
+                                 bool insideNonGlobalEval, uint32_t lineNum,
+                                 EmitterMode emitterMode)
   : sc(sc),
     parent(parent),
     script(sc->context, script),
     lazyScript(sc->context, lazyScript),
     prolog(sc->context, lineNum),
     main(sc->context, lineNum),
     current(&main),
     parser(parser),
@@ -142,17 +143,17 @@ BytecodeEmitter::BytecodeEmitter(Bytecod
     blockScopeList(sc->context),
     yieldOffsetList(sc->context),
     typesetCount(0),
     hasSingletons(false),
     hasTryFinally(false),
     emittingForInit(false),
     emittingRunOnceLambda(false),
     insideEval(insideEval),
-    hasGlobalScope(hasGlobalScope),
+    insideNonGlobalEval(insideNonGlobalEval),
     emitterMode(emitterMode)
 {
     MOZ_ASSERT_IF(evalCaller, insideEval);
     MOZ_ASSERT_IF(emitterMode == LazyFunction, lazyScript);
     // Function scripts are never eval scripts.
     MOZ_ASSERT_IF(evalStaticScope, !sc->isFunctionBox());
 }
 
@@ -1629,18 +1630,23 @@ TryConvertFreeName(BytecodeEmitter *bce,
             }
 
             if (script->funHasExtensibleScope() || script->directlyInsideEval())
                 return false;
         }
     }
 
     // Unbound names aren't recognizable global-property references if the
-    // script isn't running against its global object.
-    if (!bce->script->compileAndGo() || !bce->hasGlobalScope)
+    // script is inside a non-global eval call.
+    if (bce->insideNonGlobalEval)
+        return false;
+
+    // Skip trying to use GNAME ops if we know our script has a polluted
+    // global scope, since they'll just get treated as NAME ops anyway.
+    if (bce->script->hasPollutedGlobalScope())
         return false;
 
     // Deoptimized names also aren't necessarily globals.
     if (pn->isDeoptimized())
         return false;
 
     if (bce->sc->isFunctionBox()) {
         // Unbound names in function code may not be globals if new locals can
@@ -5407,17 +5413,17 @@ EmitFunc(ExclusiveContext *cx, BytecodeE
                 return false;
 
             script->bindings = funbox->bindings;
 
             uint32_t lineNum = bce->parser->tokenStream.srcCoords.lineNum(pn->pn_pos.begin);
             BytecodeEmitter bce2(bce, bce->parser, funbox, script, /* lazyScript = */ js::NullPtr(),
                                  bce->insideEval, bce->evalCaller,
                                  /* evalStaticScope = */ js::NullPtr(),
-                                 bce->hasGlobalScope, lineNum, bce->emitterMode);
+                                 bce->insideNonGlobalEval, lineNum, bce->emitterMode);
             if (!bce2.init())
                 return false;
 
             /* We measured the max scope depth when we parsed the function. */
             if (!EmitFunctionScript(cx, &bce2, pn->pn_body))
                 return false;
 
             if (funbox->usesArguments && funbox->usesApply && funbox->usesThis)
--- a/js/src/frontend/BytecodeEmitter.h
+++ b/js/src/frontend/BytecodeEmitter.h
@@ -169,18 +169,18 @@ struct BytecodeEmitter
     bool            emittingRunOnceLambda:1; /* true while emitting a lambda which is only
                                                 expected to run once. */
 
     bool isRunOnceLambda();
 
     bool            insideEval:1;       /* True if compiling an eval-expression or a function
                                            nested inside an eval. */
 
-    const bool      hasGlobalScope:1;   /* frontend::CompileScript's scope chain is the
-                                           global object */
+    const bool      insideNonGlobalEval:1;  /* True if this is a direct eval
+                                               call in some non-global scope. */
 
     enum EmitterMode {
         Normal,
 
         /*
          * Emit JSOP_GETINTRINSIC instead of JSOP_GETNAME and assert that
          * JSOP_GETNAME and JSOP_*GNAME don't ever get emitted. See the comment
          * for the field |selfHostingMode| in Parser.h for details.
@@ -200,17 +200,17 @@ struct BytecodeEmitter
      * Note that BytecodeEmitters are magic: they own the arena "top-of-stack"
      * space above their tempMark points. This means that you cannot alloc from
      * tempLifoAlloc and save the pointer beyond the next BytecodeEmitter
      * destruction.
      */
     BytecodeEmitter(BytecodeEmitter *parent, Parser<FullParseHandler> *parser, SharedContext *sc,
                     HandleScript script, Handle<LazyScript *> lazyScript,
                     bool insideEval, HandleScript evalCaller,
-                    Handle<StaticEvalObject *> evalStaticScope, bool hasGlobalScope,
+                    Handle<StaticEvalObject *> evalStaticScope, bool insideNonGlobalEval,
                     uint32_t lineNum, EmitterMode emitterMode = Normal);
     bool init();
     bool updateLocalsToFrameSlots();
 
     bool isAliasedName(ParseNode *pn);
 
     MOZ_ALWAYS_INLINE
     bool makeAtomIndex(JSAtom *atom, jsatomid *indexp) {
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/bug1106982-2.js
@@ -0,0 +1,20 @@
+var x = "wrong";
+var t = {x: "x"};
+var hits = 0;
+var p = new Proxy(t, {
+    has(t, id) {
+        var found = id in t;
+        if (++hits == 2)
+            delete t[id];
+        return found;
+    },
+    get(t, id) { return t[id]; }
+});
+evaluate(`function testFunc() {
+    x += " x";
+}`, { compileAndGo: false });
+
+var cloneFunc = clone(testFunc, p);
+cloneFunc();
+assertEq(hits, 2);
+assertEq(t.x, "undefined x");
--- a/js/src/jit-test/tests/basic/bug1106982.js
+++ b/js/src/jit-test/tests/basic/bug1106982.js
@@ -7,10 +7,13 @@ var p = new Proxy(t, {
         if (++hits == 2)
             delete t[id];
         return found;
     },
     get(t, id) { return t[id]; }
 });
 with (p)
     x += " x";
+// If you change this testcase (e.g. because we fix the number of
+// has() calls we end up making to match spec better), don't forget to
+// update bug1106982-2.js too.  See also bug 1145641.
 assertEq(hits, 2);
 assertEq(t.x, "undefined x");
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/eval-scopes.js
@@ -0,0 +1,37 @@
+var x = "outer";
+
+setLazyParsingEnabled(false);
+{
+    let x = "inner";
+    eval("function g() { assertEq(x, 'inner');} g()");
+}
+setLazyParsingEnabled(true);
+
+{
+    let x = "inner";
+    eval("function h() { assertEq(x, 'inner');} h()");
+}
+
+setLazyParsingEnabled(false);
+with ({}) {
+    let x = "inner";
+    eval("function i() { assertEq(x, 'inner');} i()");
+}
+setLazyParsingEnabled(true);
+
+with ({}) {
+    let x = "inner";
+    eval("function j() { assertEq(x, 'inner');} j()");
+}
+
+setLazyParsingEnabled(false);
+(function () {
+    var x = "inner";
+    eval("function k() { assertEq(x, 'inner');} k()");
+})();
+setLazyParsingEnabled(true);
+
+(function () {
+    let x = "inner";
+    eval("function l() { assertEq(x, 'inner');} l()");
+})();
--- a/js/src/vm/Xdr.h
+++ b/js/src/vm/Xdr.h
@@ -24,17 +24,17 @@ namespace js {
  * versions.  If deserialization fails, the data should be invalidated if
  * possible.
  *
  * When you change this, run make_opcode_doc.py and copy the new output into
  * this wiki page:
  *
  *  https://developer.mozilla.org/en-US/docs/SpiderMonkey/Internals/Bytecode
  */
-static const uint32_t XDR_BYTECODE_VERSION_SUBTRAHEND = 266;
+static const uint32_t XDR_BYTECODE_VERSION_SUBTRAHEND = 267;
 static const uint32_t XDR_BYTECODE_VERSION =
     uint32_t(0xb973c0de - XDR_BYTECODE_VERSION_SUBTRAHEND);
 
 static_assert(JSErr_Limit == 388,
               "GREETINGS, POTENTIAL SUBTRAHEND INCREMENTER! If you added or "
               "removed MSG_DEFs from js.msg, you should increment "
               "XDR_BYTECODE_VERSION_SUBTRAHEND and update this assertion's "
               "expected JSErr_Limit value.");