Bug 844048 - Track uses of 'var arguments' within eval scripts, always make arguments objects for generators, r=luke.
authorBrian Hackett <bhackett1024@gmail.com>
Fri, 22 Feb 2013 09:29:28 -0700
changeset 122670 8177cabc4e22a9f4f9bdbd241101e020b6bbd62c
parent 122669 4d301b2bcad047442fa34470893aff52b4df73ee
child 122671 dee88fe417fe364ebea9164c1f284908fba3b5f9
push id24356
push usergszorc@mozilla.com
push dateSun, 24 Feb 2013 01:00:12 +0000
treeherdermozilla-central@195e706140d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs844048
milestone22.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 844048 - Track uses of 'var arguments' within eval scripts, always make arguments objects for generators, r=luke.
js/src/frontend/BytecodeCompiler.cpp
js/src/jit-test/tests/arguments/bug843985.js
js/src/jit-test/tests/arguments/bug844048.js
js/src/jsanalyze.cpp
--- a/js/src/frontend/BytecodeCompiler.cpp
+++ b/js/src/frontend/BytecodeCompiler.cpp
@@ -41,16 +41,37 @@ SetSourceMap(JSContext *cx, TokenStream 
 {
     if (tokenStream.hasSourceMap()) {
         if (!ss->setSourceMap(cx, tokenStream.releaseSourceMap(), script->filename))
             return false;
     }
     return true;
 }
 
+static bool
+CheckArgumentsWithinEval(JSContext *cx, Parser &parser, HandleFunction fun)
+{
+    if (fun->hasRest()) {
+        // It's an error to use |arguments| in a function that has a rest
+        // parameter.
+        parser.reportError(NULL, JSMSG_ARGUMENTS_AND_REST);
+        return false;
+    }
+
+    // Force construction of arguments objects for functions that use
+    // |arguments| within an eval.
+    RootedScript script(cx, fun->nonLazyScript());
+    if (script->argumentsHasVarBinding()) {
+        if (!JSScript::argumentsOptimizationFailed(cx, script))
+            return false;
+    }
+
+    return true;
+}
+
 UnrootedScript
 frontend::CompileScript(JSContext *cx, HandleObject scopeChain,
                         HandleScript evalCaller,
                         const CompileOptions &options,
                         const jschar *chars, size_t length,
                         JSString *source_ /* = NULL */,
                         unsigned staticLevel /* = 0 */,
                         SourceCompressionToken *extraSct /* = NULL */)
@@ -194,33 +215,30 @@ frontend::CompileScript(JSContext *cx, H
 
         parser.freeTree(pn);
     }
 
     if (!SetSourceMap(cx, tokenStream, ss, script))
         return UnrootedScript(NULL);
 
     if (evalCaller && evalCaller->functionOrCallerFunction()) {
-        JSFunction *fun = evalCaller->functionOrCallerFunction();
+        // Watch for uses of 'arguments' within the evaluated script, both as
+        // free variables and as variables redeclared with 'var'.
+        RootedFunction fun(cx, evalCaller->functionOrCallerFunction());
         HandlePropertyName arguments = cx->names().arguments;
         for (AtomDefnRange r = pc.lexdeps->all(); !r.empty(); r.popFront()) {
             if (r.front().key() == arguments) {
-                if (fun->hasRest()) {
-                    // It's an error to use |arguments| in a function that has
-                    // a rest parameter.
-                    parser.reportError(NULL, JSMSG_ARGUMENTS_AND_REST);
+                if (!CheckArgumentsWithinEval(cx, parser, fun))
                     return UnrootedScript(NULL);
-                }
-                // Force construction of arguments objects for functions that
-                // use 'arguments' within an eval.
-                RootedScript script(cx, fun->nonLazyScript());
-                if (script->argumentsHasVarBinding()) {
-                    if (!JSScript::argumentsOptimizationFailed(cx, script))
-                        return UnrootedScript(NULL);
-                }
+            }
+        }
+        for (AtomDefnListMap::Range r = pc.decls().all(); !r.empty(); r.popFront()) {
+            if (r.front().key() == arguments) {
+                if (!CheckArgumentsWithinEval(cx, parser, fun))
+                    return UnrootedScript(NULL);
             }
         }
 
         // If the eval'ed script contains any debugger statement, force construction
         // of arguments objects for the caller script and any other scripts it is
         // transitively nested inside.
         if (pc.sc->hasDebuggerStatement()) {
             RootedObject scope(cx, scopeChain);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/arguments/bug843985.js
@@ -0,0 +1,5 @@
+
+for (x in (function() {
+    eval("arguments[0]");
+    yield;
+})())(function() {})
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/arguments/bug844048.js
@@ -0,0 +1,24 @@
+
+function foo() {
+  eval("\
+    for (var arguments in arguments)\
+      assertEq(f(i, 1), i+1);\
+  ");
+}
+foo();
+
+function bar() {
+  eval("\
+    var arguments;\
+    for each(e in [arguments, arguments]) {}\
+  ");
+}
+bar();
+
+(function(){assertEq(typeof eval("var arguments; arguments"), "object")})();
+try {
+  (function(... rest){assertEq(typeof eval("var arguments; arguments"), "object")})();
+  assertEq(false, true);
+} catch (e) {
+  assertEq(/SyntaxError/.test(e), true);
+}
--- a/js/src/jsanalyze.cpp
+++ b/js/src/jsanalyze.cpp
@@ -1904,34 +1904,40 @@ ScriptAnalysis::needsArgsObj(JSContext *
 }
 
 bool
 ScriptAnalysis::needsArgsObj(JSContext *cx)
 {
     JS_ASSERT(script_->argumentsHasVarBinding());
 
     /*
+     * Always construct arguments objects when in debug mode and for generator
+     * scripts (generators can be suspended when speculation fails).
+     */
+    if (cx->compartment->debugMode() || script_->isGenerator)
+        return true;
+
+    /*
      * If the script has dynamic name accesses which could reach 'arguments',
      * the parser will already have checked to ensure there are no explicit
      * uses of 'arguments' in the function. If there are such uses, the script
      * will be marked as definitely needing an arguments object.
      *
      * New accesses on 'arguments' can occur through 'eval' or the debugger
      * statement. In the former case, we will dynamically detect the use and
      * mark the arguments optimization as having failed.
      */
     if (script_->bindingsAccessedDynamically)
         return false;
 
     /*
      * Since let variables and are not tracked, we cannot soundly perform this
-     * analysis in their presence. Generators can be suspended when the
-     * speculation fails, so disallow it also.
+     * analysis in their presence.
      */
-    if (localsAliasStack() || cx->compartment->debugMode() || script_->isGenerator)
+    if (localsAliasStack())
         return true;
 
     /*
      * If a script has explicit mentions of 'arguments' and formals which may
      * be stored as part of a call object, don't use lazy arguments. The
      * compiler can then assume that accesses through arguments[i] will be on
      * unaliased variables.
      */