From 284853060485feeb26b5c996d62ae4bb0f7f5f18 Mon Sep 17 00:00:00 2001 From: Shu-yu Guo Date: Tue, 6 Oct 2015 14:00:30 -0700 Subject: [PATCH] Bug 589199 - Implement all-or-nothing redeclaration checks for global and eval scripts. (r=efaust) --- js/src/frontend/BytecodeCompiler.cpp | 51 +++++-- js/src/frontend/BytecodeEmitter.h | 6 +- js/src/frontend/Parser.cpp | 100 ++++++++++--- js/src/frontend/Parser.h | 19 ++- js/src/jit-test/tests/basic/testLet.js | 16 +- .../tests/debug/Environment-variables.js | 1 - js/src/jit/BaselineCompiler.cpp | 33 ++--- js/src/jit/CodeGenerator.cpp | 11 ++ js/src/jit/CodeGenerator.h | 1 + js/src/jit/IonBuilder.cpp | 10 ++ js/src/jit/Lowering.cpp | 8 + js/src/jit/Lowering.h | 1 + js/src/jit/MIR.h | 15 ++ js/src/jit/MOpcodes.h | 1 + js/src/jit/VMFunctions.cpp | 36 ++++- js/src/jit/VMFunctions.h | 3 +- js/src/jit/shared/LIR-shared.h | 10 ++ js/src/jit/shared/LOpcodes-shared.h | 1 + js/src/jsscript.cpp | 13 +- js/src/jsscript.h | 5 +- js/src/vm/Interpreter-inl.h | 138 ++++++++++++++++-- js/src/vm/Stack.cpp | 20 ++- 22 files changed, 425 insertions(+), 74 deletions(-) diff --git a/js/src/frontend/BytecodeCompiler.cpp b/js/src/frontend/BytecodeCompiler.cpp index aebaf8e4dee..6e638783072 100644 --- a/js/src/frontend/BytecodeCompiler.cpp +++ b/js/src/frontend/BytecodeCompiler.cpp @@ -92,7 +92,9 @@ class MOZ_STACK_CLASS BytecodeCompiler bool maybeSetSourceMap(TokenStream& tokenStream); bool maybeSetSourceMapFromOptions(); bool emitFinalReturn(); - bool initGlobalBindings(ParseContext& pc); + bool initGlobalOrEvalBindings(ParseContext& pc, + Handle> vars, + Handle> lexicals); bool maybeCompleteCompressSource(); AutoCompilationTraceLogger traceLogger; @@ -522,19 +524,35 @@ BytecodeCompiler::emitFinalReturn() } bool -BytecodeCompiler::initGlobalBindings(ParseContext& pc) +BytecodeCompiler::initGlobalOrEvalBindings(ParseContext& pc, + Handle> vars, + Handle> lexicals) { - // Global/eval script bindings are always empty (all names are added to the - // scope dynamically via JSOP_DEFFUN/VAR). They may have block-scoped - // locals, however, which are allocated to the fixed part of the stack - // frame. Rooted bindings(cx, script->bindings); - if (!Bindings::initWithTemporaryStorage(cx, &bindings, 0, 0, 0, - pc.blockScopeDepth, 0, 0, nullptr)) - { + Binding* packedBindings = alloc->newArrayUninitialized(vars.length() + + lexicals.length()); + if (!packedBindings) { + ReportOutOfMemory(cx); return false; } + // Bindings for global and eval scripts are used solely for redeclaration + // checks in the prologue. Neither 'true' nor 'false' accurately describe + // their aliased-ness. These bindings don't live in CallObjects or the + // frame, but either on the global object and the global lexical + // scope. Force aliased to be false to avoid confusing other analyses in + // the engine that assumes the frame has a call object if there are + // aliased bindings. + Binding* packedIter = packedBindings; + for (const Binding& b: vars) + *packedIter++ = Binding(b.name(), b.kind(), false); + for (const Binding& b: lexicals) + *packedIter++ = Binding(b.name(), b.kind(), false); + + if (!Bindings::initWithTemporaryStorage(cx, &bindings, 0, vars.length(), lexicals.length(), + pc.blockScopeDepth, 0, 0, packedBindings)) + return false; + script->bindings = bindings; return true; } @@ -559,9 +577,16 @@ BytecodeCompiler::compileScript(HandleObject scopeChain, HandleScript evalCaller if (!createEmitter(&globalsc, evalCaller, isNonGlobalEvalCompilationUnit())) return nullptr; + Rooted> vars(cx, TraceableVector(cx)); + Rooted> lexicals(cx, TraceableVector(cx)); + // Syntax parsing may cause us to restart processing of top level // statements in the script. Use Maybe<> so that the parse context can be // reset when this occurs. + // + // WARNING: ParseContext contains instances of Rooted and may be + // reset(). Do not make any new Rooted instances below this point to avoid + // violating the Rooted LIFO invariant. Maybe> pc; if (!createParseContext(pc, globalsc)) return nullptr; @@ -584,6 +609,9 @@ BytecodeCompiler::compileScript(HandleObject scopeChain, HandleScript evalCaller if (!prepareAndEmitTree(&pn, *pc)) return nullptr; + if (!pc->drainGlobalOrEvalBindings(cx, &vars, &lexicals)) + return nullptr; + parser->handler.freeTree(pn); } else { bool canHaveDirectives = true; @@ -616,6 +644,9 @@ BytecodeCompiler::compileScript(HandleObject scopeChain, HandleScript evalCaller if (!prepareAndEmitTree(&pn, *pc)) return nullptr; + if (!pc->drainGlobalOrEvalBindings(cx, &vars, &lexicals)) + return nullptr; + parser->handler.freeTree(pn); } } @@ -625,7 +656,7 @@ BytecodeCompiler::compileScript(HandleObject scopeChain, HandleScript evalCaller !maybeSetSourceMap(parser->tokenStream) || !maybeSetSourceMapFromOptions() || !emitFinalReturn() || - !initGlobalBindings(pc.ref()) || + !initGlobalOrEvalBindings(pc.ref(), vars, lexicals) || !JSScript::fullyInitFromEmitter(cx, script, emitter.ptr())) { return nullptr; diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h index 44fe9cb0646..0d1f6c69738 100644 --- a/js/src/frontend/BytecodeEmitter.h +++ b/js/src/frontend/BytecodeEmitter.h @@ -237,10 +237,8 @@ struct BytecodeEmitter StmtInfoBCE* innermostStmt() const { return stmtStack.innermost(); } StmtInfoBCE* innermostScopeStmt() const { return stmtStack.innermostScopeStmt(); } JSObject* innermostStaticScope() const; - JSObject* blockScopeOfDef(ParseNode* pn) const { - MOZ_ASSERT(pn->resolve()); - unsigned blockid = pn->resolve()->pn_blockid; - return parser->blockScopes[blockid]; + JSObject* blockScopeOfDef(Definition* dn) const { + return parser->blockScopes[dn->pn_blockid]; } bool atBodyLevel() const; diff --git a/js/src/frontend/Parser.cpp b/js/src/frontend/Parser.cpp index 8fc5957f113..a8b77025ddd 100644 --- a/js/src/frontend/Parser.cpp +++ b/js/src/frontend/Parser.cpp @@ -242,13 +242,22 @@ ParseContext::define(TokenStream& ts, break; case Definition::VAR: - if (!sc->isGlobalContext()) { + // Unlike args, var bindings keep the blockid of where the statement + // was found until ParseContext::generateBindings. In practice, this + // means when we emit bytecode for function scripts, var Definition + // nodes will have their static scopes correctly set to the static + // scope of the body. For global scripts, vars are dynamically defined + // on the global object and their static scope is never consulted. + if (!vars_.append(dn)) + return false; + + // We always track vars for redeclaration checks, but only non-global + // and non-deoptimized (e.g., inside a with scope) vars live in frame + // or CallObject slots. + if (!sc->isGlobalContext() && !dn->isDeoptimized()) { dn->setOp((js_CodeSpec[dn->getOp()].format & JOF_SET) ? JSOP_SETLOCAL : JSOP_GETLOCAL); - dn->pn_blockid = bodyid; dn->pn_dflags |= PND_BOUND; - if (!dn->pn_scopecoord.setSlot(ts, vars_.length())) - return false; - if (!vars_.append(dn)) + if (!dn->pn_scopecoord.setSlot(ts, vars_.length() - 1)) return false; if (!checkLocalsOverflow(ts)) return false; @@ -336,7 +345,7 @@ ParseContext::prepareToAddDuplicateArg(HandlePropertyName name, De template void -ParseContext::updateDecl(JSAtom* atom, Node pn) +ParseContext::updateDecl(TokenStream& ts, JSAtom* atom, Node pn) { Definition* oldDecl = decls_.lookupFirst(atom); @@ -344,8 +353,24 @@ ParseContext::updateDecl(JSAtom* atom, Node pn) Definition* newDecl = (Definition*)pn; decls_.updateFirst(atom, newDecl); - if (sc->isGlobalContext()) { + if (sc->isGlobalContext() || oldDecl->isDeoptimized()) { MOZ_ASSERT(newDecl->isFreeVar()); + // Global 'var' bindings have no slots, but are still tracked for + // redeclaration checks. + for (uint32_t i = 0; i < vars_.length(); i++) { + if (vars_[i] == oldDecl) { + // Terribly, deoptimized bindings may be updated with + // optimized bindings due to hoisted function statements, so + // give the new declaration a slot. + if (oldDecl->isDeoptimized() && !newDecl->isDeoptimized()) { + newDecl->pn_dflags |= PND_BOUND; + newDecl->pn_scopecoord.setSlot(ts, i); + newDecl->setOp(JSOP_GETLOCAL); + } + vars_[i] = newDecl; + break; + } + } return; } @@ -438,6 +463,12 @@ ParseContext::generateBindings(ExclusiveContext* cx, TokenStream& if (UINT32_MAX - args_.length() <= vars_.length() + bodyLevelLexicals_.length()) return ts.reportError(JSMSG_TOO_MANY_LOCALS); + // Fix up the blockids of vars, whose static scope is always at the body + // level. This could not be done up front in ParseContext::Define, as + // the original blockids are used for redeclaration checks. + for (size_t i = 0; i < vars_.length(); i++) + vars_[i]->pn_blockid = bodyid; + // Fix up the slots of body-level lets to come after the vars now that we // know how many vars there are. for (size_t i = 0; i < bodyLevelLexicals_.length(); i++) { @@ -467,6 +498,32 @@ ParseContext::generateBindings(ExclusiveContext* cx, TokenStream& packedBindings, sc->isModuleBox()); } +template <> +bool +ParseContext::drainGlobalOrEvalBindings(ExclusiveContext* cx, + MutableHandle> vars, + MutableHandle> lexicals) +{ + MOZ_ASSERT(sc->isGlobalContext()); + + uint32_t newVarsPos = vars.length(); + uint32_t newLexicalsPos = lexicals.length(); + + if (!vars.growBy(vars_.length())) + return false; + AppendPackedBindings(this, vars_, vars.begin() + newVarsPos); + vars_.clear(); + + if (!sc->staticScope()->is()) { + if (!lexicals.growBy(bodyLevelLexicals_.length())) + return false; + AppendPackedBindings(this, bodyLevelLexicals_, lexicals.begin() + newLexicalsPos); + } + bodyLevelLexicals_.clear(); + + return true; +} + template bool Parser::reportHelper(ParseReportKind kind, bool strict, uint32_t offset, @@ -1224,10 +1281,10 @@ Parser::functionBody(InHandling inHandling, YieldHandling yieldHan /* See comment for use in Parser::functionDef. */ template <> bool -Parser::makeDefIntoUse(Definition* dn, ParseNode* pn, JSAtom* atom) +Parser::makeDefIntoUse(Definition* dn, ParseNode* pn, HandleAtom atom) { /* Turn pn into a definition. */ - pc->updateDecl(atom, pn); + pc->updateDecl(tokenStream, atom, pn); /* Change all uses of dn to be uses of pn. */ for (ParseNode* pnu = dn->dn_uses; pnu; pnu = pnu->pn_link) { @@ -3263,7 +3320,7 @@ Parser::bindLexical(BindData* data, ExclusiveContext* cx = parser->context; Rooted blockObj(cx, data->letData().blockObj); - uint32_t index; + uint32_t index = StaticBlockObject::LOCAL_INDEX_LIMIT; if (blockObj) { // Leave the scope coordinate free on global lexicals. // @@ -3310,7 +3367,10 @@ Parser::bindLexical(BindData* data, * define() right now. Otherwise, delay define until pushLetScope. */ if (data->letData().varContext == HoistVars) { - if (dn && dn->pn_blockid == pc->blockid()) + // The reason we compare using >= instead of == on the block id is to + // detect redeclarations where a 'var' binding first appeared in a + // nested block: |{ var x; } let x;| + if (dn && dn->pn_blockid >= pc->blockid()) return parser->reportRedeclaration(pn, dn->kind(), name); if (!pc->define(parser->tokenStream, name, pn, bindingKind)) return false; @@ -3518,21 +3578,27 @@ Parser::bindVar(BindData* data, } /* - * This definition isn't being added to the parse context's - * declarations, so make sure to indicate the need to deoptimize - * the script's arguments object. Mark the function as if it - * contained a debugger statement, which will deoptimize arguments - * as much as possible. + * Make sure to indicate the need to deoptimize the script's arguments + * object. Mark the function as if it contained a debugger statement, + * which will deoptimize arguments as much as possible. */ if (name == cx->names().arguments) pc->sc->setHasDebuggerStatement(); - return true; + // Find the nearest enclosing non-with scope that defined name, if + // any, for redeclaration checks below. + while (stmt && stmt->type == StmtType::WITH) { + if (stmt->enclosingScope) + stmt = LexicalLookup(pc, name, stmt->enclosingScope); + else + stmt = nullptr; + } } DefinitionList::Range defs = pc->decls().lookupMulti(name); MOZ_ASSERT_IF(stmt, !defs.empty()); + // TODOshu: ES6 Annex B.3.5 is not implemented. if (defs.empty()) return pc->define(parser->tokenStream, name, pn, Definition::VAR); diff --git a/js/src/frontend/Parser.h b/js/src/frontend/Parser.h index a8d3bebab5c..596a23f1be7 100644 --- a/js/src/frontend/Parser.h +++ b/js/src/frontend/Parser.h @@ -195,7 +195,7 @@ struct MOZ_STACK_CLASS ParseContext : public GenericParseContext void prepareToAddDuplicateArg(HandlePropertyName name, DefinitionNode prevDecl); /* See the sad story in MakeDefIntoUse. */ - void updateDecl(JSAtom* atom, Node newDecl); + void updateDecl(TokenStream& ts, JSAtom* atom, Node newDecl); /* * After a function body or module has been parsed, the parser generates the @@ -214,6 +214,21 @@ struct MOZ_STACK_CLASS ParseContext : public GenericParseContext bool generateBindings(ExclusiveContext* cx, TokenStream& ts, LifoAlloc& alloc, MutableHandle bindings) const; + // All global names in global scripts are added to the scope dynamically + // via JSOP_DEF{FUN,VAR,LET,CONST}, but ES6 15.1.8 specifies that if there + // are name conflicts in the script, *no* bindings from the script are + // instantiated. So, record the vars and lexical bindings to check for + // redeclarations in the prologue. + // + // Eval scripts do not need this mechanism as they always have a + // non-extensible lexical scope. + // + // Global and eval scripts may have block-scoped locals, however, which + // are allocated to the fixed part of the stack frame. + bool drainGlobalOrEvalBindings(ExclusiveContext* cx, + MutableHandle> vars, + MutableHandle> lexicals); + private: ParseContext** parserPC; /* this points to the Parser's active pc and holds either |this| or one of @@ -774,7 +789,7 @@ class Parser : private JS::AutoGCRooter, public StrictModeGetter bool matchInOrOf(bool* isForInp, bool* isForOfp); bool checkFunctionArguments(); - bool makeDefIntoUse(Definition* dn, Node pn, JSAtom* atom); + bool makeDefIntoUse(Definition* dn, Node pn, HandleAtom atom); bool checkFunctionDefinition(HandlePropertyName funName, Node* pn, FunctionSyntaxKind kind, bool* pbodyProcessed); bool finishFunctionDefinition(Node pn, FunctionBox* funbox, Node body); diff --git a/js/src/jit-test/tests/basic/testLet.js b/js/src/jit-test/tests/basic/testLet.js index 7f42fd6c064..aaa78af6397 100644 --- a/js/src/jit-test/tests/basic/testLet.js +++ b/js/src/jit-test/tests/basic/testLet.js @@ -44,6 +44,18 @@ function isParseError(str) assertEq(caught, true); } +function isRuntimeParseError(str, arg) +{ + var caught = false; + try { + (new Function("x", str))(arg); + } catch(e) { + assertEq(e instanceof TypeError || e instanceof SyntaxError, true); + caught = true; + } + assertEq(caught, true); +} + function isReferenceError(str) { var caught = false; @@ -141,7 +153,7 @@ test('let (y = x[1]) {let (x = x[0]) {try {let (y = "unicorns") {throw y;}} catc isParseError('let (x = x) {try {let (x = "unicorns") eval("throw x");} catch (e) {return x;}}'); test('let (y = x) {return function () {return eval("y");}();}'); test('return eval("let (y = x) {y;}");'); -test('let (y = x) {eval("var y = 2");return y;}', 'ponies', 2); +isRuntimeParseError('let (y = x) {eval("var y = 2");return y;}', 'ponies'); test('"use strict";let (y = x) {eval("var y = 2");return y;}'); test('this.y = x;let (y = 1) {return this.eval("y");}'); isParseError('let (x = 1, x = 2) {x}'); @@ -215,7 +227,7 @@ test('x.foo;{let y = x;return y;}'); test('x.foo;if (x) {x.bar;let y = x;return y;}'); test('if (x) {let y = x;return function () {return eval("y");}();}'); test('return eval("let y = x; y");'); -test('if (x) {let y = x;eval("var y = 2");return y;}', 'ponies', 2); +isRuntimeParseError('if (x) {let y = x;eval("var y = 2");return y;}', 'ponies'); test('"use strict";if (x) {let y = x;eval("var y = 2");return y;}'); test('"use strict";if (x) {let y = x;eval("let y = 2");return y;}'); test('"use strict";if (x) {let y = 1;return eval("let y = x;y;");}'); diff --git a/js/src/jit-test/tests/debug/Environment-variables.js b/js/src/jit-test/tests/debug/Environment-variables.js index d20548d8d5f..22895200a9e 100644 --- a/js/src/jit-test/tests/debug/Environment-variables.js +++ b/js/src/jit-test/tests/debug/Environment-variables.js @@ -34,7 +34,6 @@ var cases = [ // dynamic bindings "function f(s) { eval(s); @@ } f('var x = VAL');", - "function f(s) { let (x = 'fail') { eval(s); } x = VAL; @@ } f('var x;');", "var x = VAL; function f(s) { eval('var x = 0;'); eval(s); @@ } f('delete x;');", "function f(obj) { with (obj) { @@ } } f({x: VAL});", "function f(obj) { with (obj) { @@ } } f(Object.create({x: VAL}));", diff --git a/js/src/jit/BaselineCompiler.cpp b/js/src/jit/BaselineCompiler.cpp index 166e6c97839..51c9df08346 100644 --- a/js/src/jit/BaselineCompiler.cpp +++ b/js/src/jit/BaselineCompiler.cpp @@ -428,15 +428,15 @@ BaselineCompiler::emitPrologue() // the scope chain is initialized. prologueOffset_ = CodeOffsetLabel(masm.currentOffset()); + // When compiling with Debugger instrumentation, set the debuggeeness of + // the frame before any operation that can call into the VM. + emitIsDebuggeeCheck(); + // Initialize the scope chain before any operation that may // call into the VM and trigger a GC. if (!initScopeChain()) return false; - // When compiling with Debugger instrumentation, set the debuggeeness of - // the frame before any operation that can call into the VM. - emitIsDebuggeeCheck(); - if (!emitStackCheck()) return false; @@ -639,9 +639,9 @@ BaselineCompiler::emitDebugPrologue() return true; } -typedef bool (*InitStrictEvalScopeObjectsFn)(JSContext*, BaselineFrame*); -static const VMFunction InitStrictEvalScopeObjectsInfo = - FunctionInfo(jit::InitStrictEvalScopeObjects); +typedef bool (*InitGlobalOrEvalScopeObjectsFn)(JSContext*, BaselineFrame*); +static const VMFunction InitGlobalOrEvalScopeObjectsInfo = + FunctionInfo(jit::InitGlobalOrEvalScopeObjects); typedef bool (*InitFunctionScopeObjectsFn)(JSContext*, BaselineFrame*); static const VMFunction InitFunctionScopeObjectsInfo = @@ -682,18 +682,17 @@ BaselineCompiler::initScopeChain() masm.storePtr(scope, frame.addressOfScopeChain()); } else { // ScopeChain pointer in BaselineFrame has already been initialized - // in prologue. + // in prologue, but we need to do two more things: + // + // 1. Check for redeclaration errors + // 2. Possibly create a new call object for strict eval. - if (script->isForEval() && script->strict()) { - // Strict eval needs its own call object. - prepareVMCall(); + prepareVMCall(); + masm.loadBaselineFramePtr(BaselineFrameReg, R0.scratchReg()); + pushArg(R0.scratchReg()); - masm.loadBaselineFramePtr(BaselineFrameReg, R0.scratchReg()); - pushArg(R0.scratchReg()); - - if (!callVMNonOp(InitStrictEvalScopeObjectsInfo, phase)) - return false; - } + if (!callVMNonOp(InitGlobalOrEvalScopeObjectsInfo, phase)) + return false; } return true; diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index 0795880d7b8..39daaaddc44 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -10263,6 +10263,17 @@ CodeGenerator::visitThrowUninitializedLexical(LThrowUninitializedLexical* ins) callVM(ThrowUninitializedLexicalInfo, ins); } +typedef bool (*GlobalNameConflictsCheckFromIonFn)(JSContext*, HandleScript); +static const VMFunction GlobalNameConflictsCheckFromIonInfo = + FunctionInfo(GlobalNameConflictsCheckFromIon); + +void +CodeGenerator::visitGlobalNameConflictsCheck(LGlobalNameConflictsCheck* ins) +{ + pushArg(ImmGCPtr(ins->mirRaw()->block()->info().script())); + callVM(GlobalNameConflictsCheckFromIonInfo, ins); +} + void CodeGenerator::visitDebugger(LDebugger* ins) { diff --git a/js/src/jit/CodeGenerator.h b/js/src/jit/CodeGenerator.h index 0ba843cca38..7dc9e70add1 100644 --- a/js/src/jit/CodeGenerator.h +++ b/js/src/jit/CodeGenerator.h @@ -331,6 +331,7 @@ class CodeGenerator : public CodeGeneratorSpecific void visitAsmJSVoidReturn(LAsmJSVoidReturn* ret); void visitLexicalCheck(LLexicalCheck* ins); void visitThrowUninitializedLexical(LThrowUninitializedLexical* ins); + void visitGlobalNameConflictsCheck(LGlobalNameConflictsCheck* ins); void visitDebugger(LDebugger* ins); void visitNewTarget(LNewTarget* ins); void visitArrowNewTarget(LArrowNewTarget* ins); diff --git a/js/src/jit/IonBuilder.cpp b/js/src/jit/IonBuilder.cpp index 4b18fc0b94a..3ed0845ce5f 100644 --- a/js/src/jit/IonBuilder.cpp +++ b/js/src/jit/IonBuilder.cpp @@ -1239,6 +1239,16 @@ IonBuilder::initScopeChain(MDefinition* callee) MOZ_ASSERT(!script()->isForEval()); MOZ_ASSERT(!script()->hasNonSyntacticScope()); scope = constant(ObjectValue(script()->global().lexicalScope())); + + // Check for redeclaration errors for global scripts. + if (script()->bindings.numBodyLevelLocals() > 0) { + MGlobalNameConflictsCheck* redeclCheck = MGlobalNameConflictsCheck::New(alloc()); + current->add(redeclCheck); + MResumePoint* entryRpCopy = MResumePoint::Copy(alloc(), current->entryResumePoint()); + if (!entryRpCopy) + return false; + redeclCheck->setResumePoint(entryRpCopy); + } } current->setScopeChain(scope); diff --git a/js/src/jit/Lowering.cpp b/js/src/jit/Lowering.cpp index f65595faa45..0397572b5df 100644 --- a/js/src/jit/Lowering.cpp +++ b/js/src/jit/Lowering.cpp @@ -4271,6 +4271,14 @@ LIRGenerator::visitThrowUninitializedLexical(MThrowUninitializedLexical* ins) assignSafepoint(lir, ins); } +void +LIRGenerator::visitGlobalNameConflictsCheck(MGlobalNameConflictsCheck* ins) +{ + LGlobalNameConflictsCheck* lir = new(alloc()) LGlobalNameConflictsCheck(); + add(lir, ins); + assignSafepoint(lir, ins); +} + void LIRGenerator::visitDebugger(MDebugger* ins) { diff --git a/js/src/jit/Lowering.h b/js/src/jit/Lowering.h index 4e03c5ef56d..addf1536b98 100644 --- a/js/src/jit/Lowering.h +++ b/js/src/jit/Lowering.h @@ -301,6 +301,7 @@ class LIRGenerator : public LIRGeneratorSpecific void visitUnknownValue(MUnknownValue* ins); void visitLexicalCheck(MLexicalCheck* ins); void visitThrowUninitializedLexical(MThrowUninitializedLexical* ins); + void visitGlobalNameConflictsCheck(MGlobalNameConflictsCheck* ins); void visitDebugger(MDebugger* ins); void visitNewTarget(MNewTarget* ins); void visitArrowNewTarget(MArrowNewTarget* ins); diff --git a/js/src/jit/MIR.h b/js/src/jit/MIR.h index 89c2191eb13..eba00f6211b 100644 --- a/js/src/jit/MIR.h +++ b/js/src/jit/MIR.h @@ -7279,6 +7279,21 @@ class MThrowUninitializedLexical : public MNullaryInstruction } }; +// In the prologues of global and eval scripts, check for redeclarations. +class MGlobalNameConflictsCheck : public MNullaryInstruction +{ + MGlobalNameConflictsCheck() { + setGuard(); + } + + public: + INSTRUCTION_HEADER(GlobalNameConflictsCheck) + + static MGlobalNameConflictsCheck* New(TempAllocator& alloc) { + return new(alloc) MGlobalNameConflictsCheck(); + } +}; + // If not defined, set a global variable to |undefined|. class MDefVar : public MUnaryInstruction, diff --git a/js/src/jit/MOpcodes.h b/js/src/jit/MOpcodes.h index 530d70ee42f..6eaef4158c7 100644 --- a/js/src/jit/MOpcodes.h +++ b/js/src/jit/MOpcodes.h @@ -274,6 +274,7 @@ namespace jit { _(UnknownValue) \ _(LexicalCheck) \ _(ThrowUninitializedLexical) \ + _(GlobalNameConflictsCheck) \ _(Debugger) \ _(NewTarget) \ _(ArrowNewTarget) diff --git a/js/src/jit/VMFunctions.cpp b/js/src/jit/VMFunctions.cpp index 8b05380bb4d..dcee1193075 100644 --- a/js/src/jit/VMFunctions.cpp +++ b/js/src/jit/VMFunctions.cpp @@ -845,9 +845,41 @@ GeneratorThrowOrClose(JSContext* cx, BaselineFrame* frame, HandleinitStrictEvalScopeObjects(cx); + RootedScript script(cx, frame->script()); + RootedObject varObj(cx, frame->scopeChain()); + while (!varObj->isQualifiedVarObj()) + varObj = varObj->enclosingScope(); + + if (script->isForEval()) { + // Strict eval needs its own call object. + // + // Non-strict eval may introduce 'var' bindings that conflict with + // lexical bindings in an enclosing lexical scope. + if (script->strict()) { + if (!frame->initStrictEvalScopeObjects(cx)) + return false; + } else { + RootedObject scopeChain(cx, frame->scopeChain()); + if (!CheckEvalDeclarationConflicts(cx, script, scopeChain, varObj)) + return false; + } + } else { + Rooted lexicalScope(cx, + &NearestEnclosingExtensibleLexicalScope(frame->scopeChain())); + if (!CheckGlobalDeclarationConflicts(cx, script, lexicalScope, varObj)) + return false; + } + + return true; +} + +bool +GlobalNameConflictsCheckFromIon(JSContext* cx, HandleScript script) +{ + Rooted lexicalScope(cx, &cx->global()->lexicalScope()); + return CheckGlobalDeclarationConflicts(cx, script, lexicalScope, cx->global()); } bool diff --git a/js/src/jit/VMFunctions.h b/js/src/jit/VMFunctions.h index 1af58e0ad82..944ded9d158 100644 --- a/js/src/jit/VMFunctions.h +++ b/js/src/jit/VMFunctions.h @@ -658,7 +658,8 @@ bool DebugAfterYield(JSContext* cx, BaselineFrame* frame); bool GeneratorThrowOrClose(JSContext* cx, BaselineFrame* frame, Handle genObj, HandleValue arg, uint32_t resumeKind); -bool InitStrictEvalScopeObjects(JSContext* cx, BaselineFrame* frame); +bool GlobalNameConflictsCheckFromIon(JSContext* cx, HandleScript script); +bool InitGlobalOrEvalScopeObjects(JSContext* cx, BaselineFrame* frame); bool InitFunctionScopeObjects(JSContext* cx, BaselineFrame* frame); bool NewArgumentsObject(JSContext* cx, BaselineFrame* frame, MutableHandleValue res); diff --git a/js/src/jit/shared/LIR-shared.h b/js/src/jit/shared/LIR-shared.h index c69e3160ba6..178bc8d2430 100644 --- a/js/src/jit/shared/LIR-shared.h +++ b/js/src/jit/shared/LIR-shared.h @@ -7131,6 +7131,16 @@ class LThrowUninitializedLexical : public LCallInstructionHelper<0, 0, 0> } }; +class LGlobalNameConflictsCheck : public LInstructionHelper<0, 0, 0> +{ + public: + LIR_HEADER(GlobalNameConflictsCheck) + + MGlobalNameConflictsCheck* mir() { + return mir_->toGlobalNameConflictsCheck(); + } +}; + class LMemoryBarrier : public LInstructionHelper<0, 0, 0> { private: diff --git a/js/src/jit/shared/LOpcodes-shared.h b/js/src/jit/shared/LOpcodes-shared.h index a3ceea33e71..6393fa00f31 100644 --- a/js/src/jit/shared/LOpcodes-shared.h +++ b/js/src/jit/shared/LOpcodes-shared.h @@ -365,6 +365,7 @@ _(AssertResultT) \ _(LexicalCheck) \ _(ThrowUninitializedLexical) \ + _(GlobalNameConflictsCheck) \ _(Debugger) \ _(NewTarget) \ _(ArrowNewTarget) diff --git a/js/src/jsscript.cpp b/js/src/jsscript.cpp index 70f64b45da1..280eb3f3cc0 100644 --- a/js/src/jsscript.cpp +++ b/js/src/jsscript.cpp @@ -320,6 +320,13 @@ Bindings::bindingIsAliased(uint32_t bindingIndex) return bindingArray()[bindingIndex].aliased(); } +void +Binding::trace(JSTracer* trc) +{ + PropertyName* name = this->name(); + TraceManuallyBarrieredEdge(trc, &name, "binding"); +} + void Bindings::trace(JSTracer* trc) { @@ -334,10 +341,8 @@ Bindings::trace(JSTracer* trc) if (bindingArrayUsingTemporaryStorage()) return; - for (const Binding& b : *this) { - PropertyName* name = b.name(); - TraceManuallyBarrieredEdge(trc, &name, "bindingArray"); - } + for (Binding& b : *this) + b.trace(trc); } template diff --git a/js/src/jsscript.h b/js/src/jsscript.h index 244070fc663..7acd5b27d94 100644 --- a/js/src/jsscript.h +++ b/js/src/jsscript.h @@ -164,7 +164,7 @@ class YieldOffsetArray { } }; -class Binding +class Binding : public JS::Traceable { // One JSScript stores one Binding per formal/variable so we use a // packed-word representation. @@ -200,6 +200,9 @@ class Binding bool aliased() const { return bool(bits_ & ALIASED_BIT); } + + static void trace(Binding* self, JSTracer* trc) { self->trace(trc); } + void trace(JSTracer* trc); }; JS_STATIC_ASSERT(sizeof(Binding) == sizeof(uintptr_t)); diff --git a/js/src/vm/Interpreter-inl.h b/js/src/vm/Interpreter-inl.h index 433bf79e10c..90a43585bce 100644 --- a/js/src/vm/Interpreter-inl.h +++ b/js/src/vm/Interpreter-inl.h @@ -300,11 +300,9 @@ SetNameOperation(JSContext* cx, JSScript* script, jsbytecode* pc, HandleObject s } inline bool -DefLexicalOperation(JSContext* cx, Handle lexicalScope, - HandleObject varObj, HandlePropertyName name, unsigned attrs) +CheckLexicalNameConflict(JSContext* cx, Handle lexicalScope, + HandleObject varObj, HandlePropertyName name) { - // Due to the extensibility of the global lexical scope, we must check for - // redeclaring a binding. mozilla::Maybe redeclKind; RootedId id(cx, NameToId(name)); RootedShape shape(cx); @@ -321,11 +319,127 @@ DefLexicalOperation(JSContext* cx, Handle lexicalScope, if (desc.object() && desc.hasConfigurable() && !desc.configurable()) redeclKind = mozilla::Some(frontend::Definition::VAR); } + if (redeclKind.isSome()) { ReportRuntimeRedeclaration(cx, name, *redeclKind); return false; } + return true; +} + +inline bool +CheckVarNameConflict(JSContext* cx, Handle lexicalScope, + HandlePropertyName name) +{ + if (Shape* shape = lexicalScope->lookup(cx, name)) { + ReportRuntimeRedeclaration(cx, name, shape->writable() ? frontend::Definition::LET + : frontend::Definition::CONSTANT); + return false; + } + return true; +} + +inline bool +CheckVarNameConflict(JSContext* cx, Handle callObj, HandlePropertyName name) +{ + RootedFunction fun(cx, &callObj->callee()); + RootedScript script(cx, fun->nonLazyScript()); + uint32_t bodyLevelLexicalsStart = script->bindings.numVars(); + + for (BindingIter bi(script); !bi.done(); bi++) { + if (name == bi->name() && + bi.isBodyLevelLexical() && + bi.localIndex() >= bodyLevelLexicalsStart) + { + ReportRuntimeRedeclaration(cx, name, + bi->kind() == Binding::CONSTANT + ? frontend::Definition::CONSTANT + : frontend::Definition::LET); + return false; + } + } + + return true; +} + +inline bool +CheckGlobalDeclarationConflicts(JSContext* cx, HandleScript script, + Handle lexicalScope, + HandleObject varObj) +{ + // Due to the extensibility of the global lexical scope, we must check for + // redeclaring a binding. + // + // In the case of non-syntactic scope chains, we are checking + // redeclarations against the non-syntactic lexical scope and the + // variables object that the lexical scope corresponds to. + RootedPropertyName name(cx); + BindingIter bi(script); + + for (uint32_t i = 0; i < script->bindings.numVars(); i++, bi++) { + name = bi->name(); + if (!CheckVarNameConflict(cx, lexicalScope, name)) + return false; + } + + for (uint32_t i = 0; i < script->bindings.numBodyLevelLexicals(); i++, bi++) { + name = bi->name(); + if (!CheckLexicalNameConflict(cx, lexicalScope, varObj, name)) + return false; + } + + return true; +} + +inline bool +CheckEvalDeclarationConflicts(JSContext* cx, HandleScript script, + HandleObject scopeChain, HandleObject varObj) +{ + MOZ_ASSERT(script->bindings.numBodyLevelLexicals() == 0); + + if (script->bindings.numVars() == 0) + return true; + + RootedPropertyName name(cx); + RootedObject obj(cx, scopeChain); + Rooted lexicalScope(cx); + + // ES6 18.2.1.2 step d + // + // Check that a direct eval will not hoist 'var' bindings over lexical + // bindings with the same name. + while (obj != varObj) { + if (obj->is()) { + lexicalScope = &obj->as(); + for (BindingIter bi(script); !bi.done(); bi++) { + name = bi->name(); + if (!CheckVarNameConflict(cx, lexicalScope, name)) + return false; + } + } + obj = obj->enclosingScope(); + } + + if (varObj->is()) { + Rooted callObj(cx, &varObj->as()); + for (BindingIter bi(script); !bi.done(); bi++) { + name = bi->name(); + if (!CheckVarNameConflict(cx, callObj, name)) + return false; + } + } + + return true; +} + +inline bool +DefLexicalOperation(JSContext* cx, Handle lexicalScope, + HandleObject varObj, HandlePropertyName name, unsigned attrs) +{ + // Redeclaration checks should have already been done. + MOZ_ASSERT(CheckLexicalNameConflict(cx, lexicalScope, varObj, name)); + RootedId id(cx, NameToId(name)); RootedValue uninitialized(cx, MagicValue(JS_UNINITIALIZED_LEXICAL)); return NativeDefineProperty(cx, lexicalScope, id, uninitialized, nullptr, nullptr, attrs); } @@ -380,15 +494,15 @@ DefVarOperation(JSContext* cx, HandleObject varobj, HandlePropertyName dn, unsig { MOZ_ASSERT(varobj->isQualifiedVarObj()); - // Due to the extensibility of the global lexical scope, we must check for - // redeclaring a lexical binding. - if (varobj == cx->global()) { - if (Shape* shape = cx->global()->lexicalScope().lookup(cx, dn)) { - ReportRuntimeRedeclaration(cx, dn, shape->writable() ? frontend::Definition::LET - : frontend::Definition::CONSTANT); - return false; - } +#ifdef DEBUG + // Per spec, it is an error to redeclare a lexical binding. This should + // have already been checked. + if (JS_HasExtensibleLexicalScope(varobj)) { + Rooted lexicalScope(cx); + lexicalScope = &JS_ExtensibleLexicalScope(varobj)->as(); + MOZ_ASSERT(CheckVarNameConflict(cx, lexicalScope, dn)); } +#endif RootedShape prop(cx); RootedObject obj2(cx); diff --git a/js/src/vm/Stack.cpp b/js/src/vm/Stack.cpp index 08a8809522f..6de93ec6eb6 100644 --- a/js/src/vm/Stack.cpp +++ b/js/src/vm/Stack.cpp @@ -220,12 +220,30 @@ InterpreterFrame::prologue(JSContext* cx) return false; pushOnScopeChain(*callobj); flags_ |= HAS_CALL_OBJ; + } else { + // Non-strict eval may introduce var bindings that conflict with + // lexical bindings in an enclosing lexical scope. + RootedObject varObjRoot(cx, &varObj()); + if (!CheckEvalDeclarationConflicts(cx, script, scopeChain(), varObjRoot)) + return false; } return probes::EnterScript(cx, script, nullptr, this); } - if (isGlobalFrame()) + if (isGlobalFrame()) { + Rooted lexicalScope(cx); + RootedObject varObjRoot(cx); + if (script->hasNonSyntacticScope()) { + lexicalScope = &extensibleLexicalScope(); + varObjRoot = &varObj(); + } else { + lexicalScope = &cx->global()->lexicalScope(); + varObjRoot = cx->global(); + } + if (!CheckGlobalDeclarationConflicts(cx, script, lexicalScope, varObjRoot)) + return false; return probes::EnterScript(cx, script, nullptr, this); + } AssertDynamicScopeMatchesStaticScope(cx, script, scopeChain());