From ee60c0ba2b75fbcaa6d036356623a4bacbf6f3d8 Mon Sep 17 00:00:00 2001 From: janekptacijarabaci Date: Mon, 18 Dec 2017 13:11:06 +0100 Subject: [PATCH] Force arguments object allocation on dynamic name accesses Issue #1547 --- js/src/frontend/BytecodeCompiler.cpp | 6 --- js/src/frontend/Parser.cpp | 13 +------ js/src/jit/CodeGenerator.cpp | 39 ------------------- js/src/jit/CodeGenerator.h | 2 - js/src/jit/IonAnalysis.cpp | 20 ++-------- js/src/jit/IonBuilder.cpp | 3 -- js/src/jit/LIR-Common.h | 58 ---------------------------- js/src/jit/LOpcodes.h | 2 - js/src/jit/Lowering.cpp | 24 ------------ js/src/jit/Lowering.h | 1 - js/src/jit/MIR.h | 28 -------------- js/src/jit/MOpcodes.h | 1 - js/src/jit/VMFunctions.cpp | 19 --------- js/src/jit/VMFunctions.h | 2 - 14 files changed, 6 insertions(+), 212 deletions(-) diff --git a/js/src/frontend/BytecodeCompiler.cpp b/js/src/frontend/BytecodeCompiler.cpp index dcd310f52d..501390944b 100644 --- a/js/src/frontend/BytecodeCompiler.cpp +++ b/js/src/frontend/BytecodeCompiler.cpp @@ -72,15 +72,9 @@ CheckArgumentsWithinEval(JSContext* cx, Parser& 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 6753da52c7..962bc4d73e 100644 --- a/js/src/frontend/Parser.cpp +++ b/js/src/frontend/Parser.cpp @@ -950,14 +950,8 @@ Parser::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::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 aec5913b35..a4ae40eb4a 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(DirectEvalStringFromIon); diff --git a/js/src/jit/CodeGenerator.h b/js/src/jit/CodeGenerator.h index f0c86eafba..21a465d539 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 a539df5040..dad0f5f762 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 5c89f86206..0897cb95d7 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 65f9516823..68f5f5b6b8 100644 --- a/js/src/jit/LIR-Common.h +++ b/js/src/jit/LIR-Common.h @@ -1847,64 +1847,6 @@ class LGetDynamicName : public LCallInstructionHelper } }; -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 { public: diff --git a/js/src/jit/LOpcodes.h b/js/src/jit/LOpcodes.h index aad0a5ea10..1a5faff91f 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 57a75d22a3..b23cddf6b6 100644 --- a/js/src/jit/Lowering.cpp +++ b/js/src/jit/Lowering.cpp @@ -556,30 +556,6 @@ LIRGenerator::visitGetDynamicName(MGetDynamicName* ins) defineReturn(lir, 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) { diff --git a/js/src/jit/Lowering.h b/js/src/jit/Lowering.h index b04f9c89f3..5ac0b543f0 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 51d2a0a6d5..b07fcc72e6 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, diff --git a/js/src/jit/MOpcodes.h b/js/src/jit/MOpcodes.h index 4daf215db0..9deb7b8f29 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 9b8c1f8167..5ca42f33d5 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 2a7ad43be3..e957164e09 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);