Bug 844048 - Track uses of 'var arguments' within eval scripts, always make arguments objects for generators, r=luke.

js/src/frontend/BytecodeCompiler.cpp

@@ -46,6 +46,27 @@ SetSourceMap(JSContext *cx, TokenStream &tokenStream, ScriptSource *ss, Unrooted
     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,
@@ -199,23 +220,20 @@ frontend::CompileScript(JSContext *cx, HandleObject scopeChain,
         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);
+            }
+        }
+        for (AtomDefnListMap::Range r = pc.decls().all(); !r.empty(); r.popFront()) {
+            if (r.front().key() == arguments) {
+                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);
-                }
             }
         }
 

js/src/jit-test/tests/arguments/bug843985.js

@@ -0,0 +1,5 @@
+
+for (x in (function() {
+    eval("arguments[0]");
+    yield;
+})())(function() {})

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);
+}

js/src/jsanalyze.cpp

@@ -1908,6 +1908,13 @@ 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
@@ -1923,10 +1930,9 @@ ScriptAnalysis::needsArgsObj(JSContext *cx)
 
     /*
      * 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;
 
     /*