Force arguments object allocation on dynamic name accesses

Issue #1547

14 files changed, 6 insertions, 212 deletions

diff --git a/js/src/frontend/BytecodeCompiler.cpp b/js/src/frontend/BytecodeCompiler.cpp
index dcd310f52..501390944 100644
--- a/js/src/frontend/BytecodeCompiler.cpp
+++ b/js/src/frontend/BytecodeCompiler.cpp
@@ -72,15 +72,9 @@ CheckArgumentsWithinEval(JSContext* cx, Parser<FullParseHandler>& parser, Handle
         return false;
     }
 
-    // Force construction of arguments objects for functions that use
-    // |arguments| within an eval.
     RootedScript script(cx, fun->getOrCreateScript(cx));
     if (!script)
         return false;
-    if (script->argumentsHasVarBinding()) {
-        if (!JSScript::argumentsOptimizationFailed(cx, script))
-            return false;
-    }
 
     // It's an error to use |arguments| in a legacy generator expression.
     if (script->isGeneratorExp() && script->isLegacyGenerator()) {
diff --git a/js/src/frontend/Parser.cpp b/js/src/frontend/Parser.cpp
index 6753da52c..962bc4d73 100644
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -950,14 +950,8 @@ Parser<FullParseHandler>::checkFunctionArguments()
         FunctionBox* funbox = pc->sc->asFunctionBox();
         funbox->setArgumentsHasLocalBinding();
 
-        /*
-         * If a script has both explicit mentions of 'arguments' and dynamic
-         * name lookups which could access the arguments, an arguments object
-         * must be created eagerly. The SSA analysis used for lazy arguments
-         * cannot cope with dynamic name accesses, so any 'arguments' accessed
-         * via a NAME opcode must force construction of the arguments object.
-         */
-        if (pc->sc->bindingsAccessedDynamically() && maybeArgDef)
+        /* Dynamic scope access destroys all hope of optimization. */
+        if (pc->sc->bindingsAccessedDynamically())
             funbox->setDefinitelyNeedsArgsObj();
 
         /*
@@ -985,9 +979,6 @@ Parser<FullParseHandler>::checkFunctionArguments()
                         funbox->setDefinitelyNeedsArgsObj();
                 }
             }
-            /* Watch for mutation of arguments through e.g. eval(). */
-            if (pc->sc->bindingsAccessedDynamically())
-                funbox->setDefinitelyNeedsArgsObj();
         }
     }
 
diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp
index aec5913b3..a4ae40eb4 100644
--- a/js/src/jit/CodeGenerator.cpp
+++ b/js/src/jit/CodeGenerator.cpp
@@ -3388,45 +3388,6 @@ CodeGenerator::visitGetDynamicName(LGetDynamicName* lir)
     bailoutFrom(&undefined, lir->snapshot());
 }
 
-void
-CodeGenerator::emitFilterArgumentsOrEval(LInstruction* lir, Register string,
-                                         Register temp1, Register temp2)
-{
-    masm.loadJSContext(temp2);
-
-    masm.setupUnalignedABICall(2, temp1);
-    masm.passABIArg(temp2);
-    masm.passABIArg(string);
-    masm.callWithABI(JS_FUNC_TO_DATA_PTR(void*, FilterArgumentsOrEval));
-
-    Label bail;
-    masm.branchIfFalseBool(ReturnReg, &bail);
-    bailoutFrom(&bail, lir->snapshot());
-}
-
-void
-CodeGenerator::visitFilterArgumentsOrEvalS(LFilterArgumentsOrEvalS* lir)
-{
-    emitFilterArgumentsOrEval(lir, ToRegister(lir->getString()),
-                              ToRegister(lir->temp1()),
-                              ToRegister(lir->temp2()));
-}
-
-void
-CodeGenerator::visitFilterArgumentsOrEvalV(LFilterArgumentsOrEvalV* lir)
-{
-    ValueOperand input = ToValue(lir, LFilterArgumentsOrEvalV::Input);
-
-    // Act as nop on non-strings.
-    Label done;
-    masm.branchTestString(Assembler::NotEqual, input, &done);
-
-    emitFilterArgumentsOrEval(lir, masm.extractString(input, ToRegister(lir->temp3())),
-                              ToRegister(lir->temp1()), ToRegister(lir->temp2()));
-
-    masm.bind(&done);
-}
-
 typedef bool (*DirectEvalSFn)(JSContext*, HandleObject, HandleScript, HandleValue, HandleString,
                               jsbytecode*, MutableHandleValue);
 static const VMFunction DirectEvalStringInfo = FunctionInfo<DirectEvalSFn>(DirectEvalStringFromIon);
diff --git a/js/src/jit/CodeGenerator.h b/js/src/jit/CodeGenerator.h
index f0c86eafb..21a465d53 100644
--- a/js/src/jit/CodeGenerator.h
+++ b/js/src/jit/CodeGenerator.h
@@ -142,8 +142,6 @@ class CodeGenerator : public CodeGeneratorSpecific
     void visitBail(LBail* lir);
     void visitUnreachable(LUnreachable* unreachable);
     void visitGetDynamicName(LGetDynamicName* lir);
-    void visitFilterArgumentsOrEvalS(LFilterArgumentsOrEvalS* lir);
-    void visitFilterArgumentsOrEvalV(LFilterArgumentsOrEvalV* lir);
     void visitCallDirectEvalS(LCallDirectEvalS* lir);
     void visitCallDirectEvalV(LCallDirectEvalV* lir);
     void visitDoubleToInt32(LDoubleToInt32* lir);
diff --git a/js/src/jit/IonAnalysis.cpp b/js/src/jit/IonAnalysis.cpp
index a539df504..dad0f5f76 100644
--- a/js/src/jit/IonAnalysis.cpp
+++ b/js/src/jit/IonAnalysis.cpp
@@ -3458,26 +3458,14 @@ jit::AnalyzeArgumentsUsage(JSContext* cx, JSScript* scriptArg)
     // 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).
+    // Always construct arguments objects when in debug mode, for generator
+    // scripts (generators can be suspended when speculation fails) or when
+    // direct eval is present.
     //
     // FIXME: Don't build arguments for ES6 generator expressions.
-    if (scriptArg->isDebuggee() || script->isGenerator())
+    if (scriptArg->isDebuggee() || script->isGenerator() || script->bindingsAccessedDynamically())
         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))
         return true;
 
diff --git a/js/src/jit/IonBuilder.cpp b/js/src/jit/IonBuilder.cpp
index 5c89f8620..0897cb95d 100644
--- a/js/src/jit/IonBuilder.cpp
+++ b/js/src/jit/IonBuilder.cpp
@@ -6194,9 +6194,6 @@ IonBuilder::jsop_eval(uint32_t argc)
             }
         }
 
-        MInstruction* filterArguments = MFilterArgumentsOrEval::New(alloc(), string);
-        current->add(filterArguments);
-
         MInstruction* ins = MCallDirectEval::New(alloc(), scopeChain, string, thisValue, pc);
         current->add(ins);
         current->push(ins);
diff --git a/js/src/jit/LIR-Common.h b/js/src/jit/LIR-Common.h
index 65f951682..68f5f5b6b 100644
--- a/js/src/jit/LIR-Common.h
+++ b/js/src/jit/LIR-Common.h
@@ -1847,64 +1847,6 @@ class LGetDynamicName : public LCallInstructionHelper<BOX_PIECES, 2, 3>
     }
 };
 
-class LFilterArgumentsOrEvalS : public LCallInstructionHelper<0, 1, 2>
-{
-  public:
-    LIR_HEADER(FilterArgumentsOrEvalS)
-
-    LFilterArgumentsOrEvalS(const LAllocation& string, const LDefinition& temp1,
-                            const LDefinition& temp2)
-    {
-        setOperand(0, string);
-        setTemp(0, temp1);
-        setTemp(1, temp2);
-    }
-
-    MFilterArgumentsOrEval* mir() const {
-        return mir_->toFilterArgumentsOrEval();
-    }
-
-    const LAllocation* getString() {
-        return getOperand(0);
-    }
-    const LDefinition* temp1() {
-        return getTemp(0);
-    }
-    const LDefinition* temp2() {
-        return getTemp(1);
-    }
-};
-
-class LFilterArgumentsOrEvalV : public LCallInstructionHelper<0, BOX_PIECES, 3>
-{
-  public:
-    LIR_HEADER(FilterArgumentsOrEvalV)
-
-    LFilterArgumentsOrEvalV(const LDefinition& temp1, const LDefinition& temp2,
-                            const LDefinition& temp3)
-    {
-        setTemp(0, temp1);
-        setTemp(1, temp2);
-        setTemp(2, temp3);
-    }
-
-    static const size_t Input = 0;
-
-    MFilterArgumentsOrEval* mir() const {
-        return mir_->toFilterArgumentsOrEval();
-    }
-
-    const LDefinition* temp1() {
-        return getTemp(0);
-    }
-    const LDefinition* temp2() {
-        return getTemp(1);
-    }
-    const LDefinition* temp3() {
-        return getTemp(2);
-    }
-};
-
 class LCallDirectEvalS : public LCallInstructionHelper<BOX_PIECES, 2 + BOX_PIECES, 0>
 {
   public:
diff --git a/js/src/jit/LOpcodes.h b/js/src/jit/LOpcodes.h
index aad0a5ea1..1a5faff91 100644
--- a/js/src/jit/LOpcodes.h
+++ b/js/src/jit/LOpcodes.h
@@ -73,8 +73,6 @@
     _(Bail)                         \
     _(Unreachable)                  \
     _(GetDynamicName)               \
-    _(FilterArgumentsOrEvalS)       \
-    _(FilterArgumentsOrEvalV)       \
     _(CallDirectEvalS)              \
     _(CallDirectEvalV)              \
     _(StackArgT)                    \
diff --git a/js/src/jit/Lowering.cpp b/js/src/jit/Lowering.cpp
index 57a75d22a..b23cddf6b 100644
--- a/js/src/jit/Lowering.cpp
+++ b/js/src/jit/Lowering.cpp
@@ -557,30 +557,6 @@ LIRGenerator::visitGetDynamicName(MGetDynamicName* ins)
 }
 
 void
-LIRGenerator::visitFilterArgumentsOrEval(MFilterArgumentsOrEval* ins)
-{
-    MDefinition* string = ins->getString();
-    MOZ_ASSERT(string->type() == MIRType_String || string->type() == MIRType_Value);
-
-    LInstruction* lir;
-    if (string->type() == MIRType_String) {
-        lir = new(alloc()) LFilterArgumentsOrEvalS(useFixed(string, CallTempReg0),
-                                                   tempFixed(CallTempReg1),
-                                                   tempFixed(CallTempReg2));
-    } else {
-        lir = new(alloc()) LFilterArgumentsOrEvalV(tempFixed(CallTempReg0),
-                                                   tempFixed(CallTempReg1),
-                                                   tempFixed(CallTempReg2));
-        useBoxFixed(lir, LFilterArgumentsOrEvalV::Input, string,
-                    CallTempReg3, CallTempReg4);
-    }
-
-    assignSnapshot(lir, Bailout_StringArgumentsEval);
-    add(lir, ins);
-    assignSafepoint(lir, ins);
-}
-
-void
 LIRGenerator::visitCallDirectEval(MCallDirectEval* ins)
 {
     MDefinition* scopeChain = ins->getScopeChain();
diff --git a/js/src/jit/Lowering.h b/js/src/jit/Lowering.h
index b04f9c89f..5ac0b543f 100644
--- a/js/src/jit/Lowering.h
+++ b/js/src/jit/Lowering.h
@@ -104,7 +104,6 @@ class LIRGenerator : public LIRGeneratorSpecific
     void visitUnreachable(MUnreachable* unreachable);
     void visitAssertFloat32(MAssertFloat32* ins);
     void visitGetDynamicName(MGetDynamicName* ins);
-    void visitFilterArgumentsOrEval(MFilterArgumentsOrEval* ins);
     void visitCallDirectEval(MCallDirectEval* ins);
     void visitTest(MTest* test);
     void visitGotoWithFake(MGotoWithFake* ins);
diff --git a/js/src/jit/MIR.h b/js/src/jit/MIR.h
index 51d2a0a6d..b07fcc72e 100644
--- a/js/src/jit/MIR.h
+++ b/js/src/jit/MIR.h
@@ -3732,34 +3732,6 @@ class MGetDynamicName
     }
 };
 
-// Bailout if the input string contains 'arguments' or 'eval'.
-class MFilterArgumentsOrEval
-  : public MAryInstruction<1>,
-    public BoxExceptPolicy<0, MIRType_String>::Data
-{
-  protected:
-    explicit MFilterArgumentsOrEval(MDefinition* string)
-    {
-        initOperand(0, string);
-        setGuard();
-        setResultType(MIRType_None);
-    }
-
-  public:
-    INSTRUCTION_HEADER(FilterArgumentsOrEval)
-
-    static MFilterArgumentsOrEval* New(TempAllocator& alloc, MDefinition* string) {
-        return new(alloc) MFilterArgumentsOrEval(string);
-    }
-
-    MDefinition* getString() const {
-        return getOperand(0);
-    }
-    bool possiblyCalls() const override {
-        return true;
-    }
-};
-
 class MCallDirectEval
   : public MAryInstruction<3>,
     public Mix3Policy<ObjectPolicy<0>,
diff --git a/js/src/jit/MOpcodes.h b/js/src/jit/MOpcodes.h
index 4daf215db..9deb7b8f2 100644
--- a/js/src/jit/MOpcodes.h
+++ b/js/src/jit/MOpcodes.h
@@ -67,7 +67,6 @@ namespace jit {
     _(Unreachable)                                                          \
     _(AssertFloat32)                                                        \
     _(GetDynamicName)                                                       \
-    _(FilterArgumentsOrEval)                                                \
     _(CallDirectEval)                                                       \
     _(BitNot)                                                               \
     _(TypeOf)                                                               \
diff --git a/js/src/jit/VMFunctions.cpp b/js/src/jit/VMFunctions.cpp
index 9b8c1f816..5ca42f33d 100644
--- a/js/src/jit/VMFunctions.cpp
+++ b/js/src/jit/VMFunctions.cpp
@@ -622,25 +622,6 @@ GetDynamicName(JSContext* cx, JSObject* scopeChain, JSString* str, Value* vp)
     vp->setUndefined();
 }
 
-bool
-FilterArgumentsOrEval(JSContext* cx, JSString* str)
-{
-    // ensureLinear() is fallible, but cannot GC: it can only allocate a
-    // character buffer for the flattened string. If this call fails then the
-    // calling Ion code will bailout, resume in Baseline and likely fail again
-    // when trying to flatten the string and unwind the stack.
-    JS::AutoCheckCannotGC nogc;
-    JSLinearString* linear = str->ensureLinear(cx);
-    if (!linear)
-        return false;
-
-    static const char16_t arguments[] = {'a', 'r', 'g', 'u', 'm', 'e', 'n', 't', 's'};
-    static const char16_t eval[] = {'e', 'v', 'a', 'l'};
-
-    return !StringHasPattern(linear, arguments, mozilla::ArrayLength(arguments)) &&
-        !StringHasPattern(linear, eval, mozilla::ArrayLength(eval));
-}
-
 void
 PostWriteBarrier(JSRuntime* rt, JSObject* obj)
 {
diff --git a/js/src/jit/VMFunctions.h b/js/src/jit/VMFunctions.h
index 2a7ad43be..e957164e0 100644
--- a/js/src/jit/VMFunctions.h
+++ b/js/src/jit/VMFunctions.h
@@ -694,8 +694,6 @@ bool CreateThis(JSContext* cx, HandleObject callee, MutableHandleValue rval);
 
 void GetDynamicName(JSContext* cx, JSObject* scopeChain, JSString* str, Value* vp);
 
-bool FilterArgumentsOrEval(JSContext* cx, JSString* str);
-
 void PostWriteBarrier(JSRuntime* rt, JSObject* obj);
 void PostGlobalWriteBarrier(JSRuntime* rt, JSObject* obj);