Bug 1003161 - Don't optimize arguments usage in scripts with aliased arguments, r=jandem.
authorBrian Hackett <bhackett1024@gmail.com>
Thu, 01 May 2014 06:13:10 -0700
changeset 181561 6173eafc6e42260aeb9ae8dc1bdec1d1f5158efc
parent 181560 720a660bac20c8d11822ca99606e6f33eda87380
child 181562 e829940609270d5a05ece49c43a543a4a2c19fd0
push id272
push userpvanderbeken@mozilla.com
push dateMon, 05 May 2014 16:31:18 +0000
reviewersjandem
bugs1003161
milestone32.0a1
Bug 1003161 - Don't optimize arguments usage in scripts with aliased arguments, r=jandem.
js/src/jit-test/tests/basic/bug1003161.js
js/src/jit/IonAnalysis.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/bug1003161.js
@@ -0,0 +1,8 @@
+
+function foo(a, b) {
+    function bar() {
+	return b;
+    }
+    return arguments[0] + arguments[1] + bar();
+}
+foo(1, 2);
--- a/js/src/jit/IonAnalysis.cpp
+++ b/js/src/jit/IonAnalysis.cpp
@@ -2366,31 +2366,35 @@ jit::AnalyzeNewScriptProperties(JSContex
         if (!handled)
             return true;
     }
 
     return true;
 }
 
 static bool
-ArgumentsUseCanBeLazy(JSContext *cx, JSScript *script, MInstruction *ins, size_t index)
+ArgumentsUseCanBeLazy(JSContext *cx, JSScript *script, MInstruction *ins, size_t index,
+                      bool *argumentsContentsObserved)
 {
     // We can read the frame's arguments directly for f.apply(x, arguments).
     if (ins->isCall()) {
         if (*ins->toCall()->resumePoint()->pc() == JSOP_FUNAPPLY &&
             ins->toCall()->numActualArgs() == 2 &&
             index == MCall::IndexOfArgument(1))
         {
+            *argumentsContentsObserved = true;
             return true;
         }
     }
 
     // arguments[i] can read fp->canonicalActualArg(i) directly.
-    if (ins->isCallGetElement() && index == 0)
+    if (ins->isCallGetElement() && index == 0) {
+        *argumentsContentsObserved = true;
         return true;
+    }
 
     // arguments.length length can read fp->numActualArgs() directly.
     if (ins->isCallGetProperty() && index == 0 && ins->toCallGetProperty()->name() == cx->names().length)
         return true;
 
     return false;
 }
 
@@ -2403,16 +2407,36 @@ jit::AnalyzeArgumentsUsage(JSContext *cx
     JS_ASSERT(!script->analyzedArgsUsage());
 
     // Treat the script as needing an arguments object until we determine it
     // does not need one. This both allows us to easily see where the arguments
     // object can escape through assignments to the function's named arguments,
     // and also simplifies handling of early returns.
     script->setNeedsArgsObj(true);
 
+    // Always construct arguments objects when in debug mode and for generator
+    // scripts (generators can be suspended when speculation fails).
+    //
+    // FIXME: Don't build arguments for ES6 generator expressions.
+    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()) {
+        script->setNeedsArgsObj(false);
+        return true;
+    }
+
     if (!jit::IsIonEnabled(cx) || !script->compileAndGo())
         return true;
 
     static const uint32_t MAX_SCRIPT_SIZE = 10000;
     if (script->length() > MAX_SCRIPT_SIZE)
         return true;
 
     if (!script->ensureHasTypes(cx))
@@ -2465,22 +2489,34 @@ jit::AnalyzeArgumentsUsage(JSContext *cx
     if (!BuildDominatorTree(graph))
         return false;
 
     if (!EliminatePhis(&builder, graph, AggressiveObservability))
         return false;
 
     MDefinition *argumentsValue = graph.begin()->getSlot(info.argsObjSlot());
 
+    bool argumentsContentsObserved = false;
+
     for (MUseDefIterator uses(argumentsValue); uses; uses++) {
         MDefinition *use = uses.def();
 
         // Don't track |arguments| through assignments to phis.
         if (!use->isInstruction())
             return true;
 
-        if (!ArgumentsUseCanBeLazy(cx, script, use->toInstruction(), uses.index()))
+        if (!ArgumentsUseCanBeLazy(cx, script, use->toInstruction(), uses.index(),
+                                   &argumentsContentsObserved))
+        {
             return true;
+        }
     }
 
+    // If a script explicitly accesses the contents of 'arguments', and has
+    // 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.
+    if (script->funHasAnyAliasedFormal() && argumentsContentsObserved)
+        return true;
+
     script->setNeedsArgsObj(false);
     return true;
 }