From 0e7b5200f4fe647b33b8c96a8a6729a2365869a5 Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Wed, 30 Dec 2015 13:09:37 -0600 Subject: [PATCH] Bug 1235640 - Correctly perform assignment-target detection and marking on a name (arguments, eval, or some other name) used as the target of a for-in/of loop. r=shu --HG-- extra : rebase_source : 3071b43b19d4a35de0f8e898fa237ab3f5f8fbb3 --- js/src/frontend/FullParseHandler.h | 32 ++++++++----- js/src/frontend/Parser.cpp | 37 +++++++-------- js/src/frontend/SyntaxParseHandler.h | 47 ++++++++++--------- ...ration-expression-contains-index-string.js | 43 +++++++++++++++++ 4 files changed, 105 insertions(+), 54 deletions(-) create mode 100644 js/src/tests/ecma_6/Statements/for-inof-name-iteration-expression-contains-index-string.js diff --git a/js/src/frontend/FullParseHandler.h b/js/src/frontend/FullParseHandler.h index 4af4cd894013..8b83edfdeded 100644 --- a/js/src/frontend/FullParseHandler.h +++ b/js/src/frontend/FullParseHandler.h @@ -881,22 +881,30 @@ class FullParseHandler return pn->isConstant(); } - PropertyName* maybeUnparenthesizedName(ParseNode* pn) { - if (!pn->isInParens() && pn->isKind(PNK_NAME)) - return pn->pn_atom->asPropertyName(); - return nullptr; + bool isUnparenthesizedName(ParseNode* node) { + return node->isKind(PNK_NAME) && !node->isInParens(); } - PropertyName* maybeParenthesizedName(ParseNode* pn) { - if (pn->isInParens() && pn->isKind(PNK_NAME)) - return pn->pn_atom->asPropertyName(); - return nullptr; + bool isNameAnyParentheses(ParseNode* node) { + return node->isKind(PNK_NAME); } - PropertyName* maybeNameAnyParentheses(ParseNode* node) { - if (PropertyName* name = maybeUnparenthesizedName(node)) - return name; - return maybeParenthesizedName(node); + bool nameIsEvalAnyParentheses(ParseNode* node, ExclusiveContext* cx) { + MOZ_ASSERT(isNameAnyParentheses(node), + "must only call this function on known names"); + + return node->pn_atom == cx->names().eval; + } + + const char* nameIsArgumentsEvalAnyParentheses(ParseNode* node, ExclusiveContext* cx) { + MOZ_ASSERT(isNameAnyParentheses(node), + "must only call this function on known names"); + + if (nameIsEvalAnyParentheses(node, cx)) + return js_eval_str; + if (node->pn_atom == cx->names().arguments) + return js_arguments_str; + return nullptr; } bool isCall(ParseNode* pn) { diff --git a/js/src/frontend/Parser.cpp b/js/src/frontend/Parser.cpp index 361bb1aaf029..4a7fd7a41862 100644 --- a/js/src/frontend/Parser.cpp +++ b/js/src/frontend/Parser.cpp @@ -4079,7 +4079,7 @@ Parser::checkDestructuringName(BindData* dat if (data) { // Destructuring patterns in declarations must only contain // unparenthesized names. - if (!handler.maybeUnparenthesizedName(expr)) { + if (!handler.isUnparenthesizedName(expr)) { report(ParseError, false, expr, JSMSG_NO_VARIABLE_NAME); return false; } @@ -4095,7 +4095,7 @@ Parser::checkDestructuringName(BindData* dat "function calls shouldn't be considered valid targets in " "destructuring patterns"); - if (handler.maybeNameAnyParentheses(expr)) { + if (handler.isNameAnyParentheses(expr)) { // The arguments/eval identifiers are simple in non-strict mode code. // Warn to discourage their use nonetheless. if (!reportIfArgumentsEvalTarget(expr)) @@ -5666,7 +5666,7 @@ Parser::validateForInOrOfLHSExpression(Node target) if (handler.isPropertyAccess(target)) return true; - if (handler.maybeNameAnyParentheses(target)) { + if (handler.isNameAnyParentheses(target)) { // The arguments/eval identifiers are simple in non-strict mode code, // but warn to discourage use nonetheless. if (!reportIfArgumentsEvalTarget(target)) @@ -5954,7 +5954,7 @@ Parser::forStatement(YieldHandling yieldHandling) pn3 = iteratedExpr; - if (handler.maybeNameAnyParentheses(pn2)) { + if (handler.isNameAnyParentheses(pn2)) { // Beware 'for (arguments in ...)' with or without a 'var'. handler.markAsAssigned(pn2); } @@ -7520,7 +7520,7 @@ Parser::checkAndMarkAsAssignmentLhs(Node target, AssignmentFlavor if (handler.isPropertyAccess(target)) return true; - if (handler.maybeNameAnyParentheses(target)) { + if (handler.isNameAnyParentheses(target)) { // The arguments/eval identifiers are simple in non-strict mode code, // but warn to discourage use nonetheless. if (!reportIfArgumentsEvalTarget(target)) @@ -7701,11 +7701,11 @@ Parser::isValidSimpleAssignmentTarget(Node node, // error for the various syntaxes that fail this, and warning for the // various syntaxes that "pass" this but should not, occurs elsewhere. - if (PropertyName* name = handler.maybeNameAnyParentheses(node)) { + if (handler.isNameAnyParentheses(node)) { if (!pc->sc->strict()) return true; - return name != context->names().arguments && name != context->names().eval; + return !handler.nameIsArgumentsEvalAnyParentheses(node, context); } if (handler.isPropertyAccess(node)) @@ -7723,21 +7723,16 @@ template bool Parser::reportIfArgumentsEvalTarget(Node nameNode) { - PropertyName* name = handler.maybeNameAnyParentheses(nameNode); - MOZ_ASSERT(name, "must only call this function on known names"); - - const char* chars = (name == context->names().arguments) - ? js_arguments_str - : (name == context->names().eval) - ? js_eval_str - : nullptr; + const char* chars = handler.nameIsArgumentsEvalAnyParentheses(nameNode, context); if (!chars) return true; if (!report(ParseStrictError, pc->sc->strict(), nameNode, JSMSG_BAD_STRICT_ASSIGN, chars)) return false; - MOZ_ASSERT(!pc->sc->strict(), "in strict mode an error should have been reported"); + MOZ_ASSERT(!pc->sc->strict(), + "an error should have been reported if this was strict mode " + "code"); return true; } @@ -7751,7 +7746,7 @@ Parser::reportIfNotValidSimpleAssignmentTarget(Node target, Assign if (isValidSimpleAssignmentTarget(target, behavior)) return true; - if (handler.maybeNameAnyParentheses(target)) { + if (handler.isNameAnyParentheses(target)) { // Use a special error if the target is arguments/eval. This ensures // targeting these names is consistently a SyntaxError (which error numbers // below don't guarantee) while giving us a nicer error message. @@ -7802,7 +7797,7 @@ Parser::checkAndMarkAsIncOperand(Node target, AssignmentFlavor fla return false; // Mark. - if (handler.maybeNameAnyParentheses(target)) { + if (handler.isNameAnyParentheses(target)) { // Assignment to arguments/eval is allowed outside strict mode code, // but it's dodgy. Report a strict warning (error, if werror was set). if (!reportIfArgumentsEvalTarget(target)) @@ -7894,7 +7889,7 @@ Parser::unaryExpr(YieldHandling yieldHandling, TripledotHandling t // Per spec, deleting any unary expression is valid -- it simply // returns true -- except for one case that is illegal in strict mode. - if (handler.maybeNameAnyParentheses(expr)) { + if (handler.isNameAnyParentheses(expr)) { if (!report(ParseStrictError, pc->sc->strict(), expr, JSMSG_DEPRECATED_DELETE_OPERAND)) return null(); pc->sc->setBindingsAccessedDynamically(); @@ -9090,8 +9085,8 @@ Parser::memberExpr(YieldHandling yieldHandling, TripledotHandling return null(); JSOp op = JSOP_CALL; - if (PropertyName* name = handler.maybeNameAnyParentheses(lhs)) { - if (tt == TOK_LP && name == context->names().eval) { + if (handler.isNameAnyParentheses(lhs)) { + if (tt == TOK_LP && handler.nameIsEvalAnyParentheses(lhs, context)) { /* Select JSOP_EVAL and flag pc as needsCallObject. */ op = pc->sc->strict() ? JSOP_STRICTEVAL : JSOP_EVAL; pc->sc->setBindingsAccessedDynamically(); diff --git a/js/src/frontend/SyntaxParseHandler.h b/js/src/frontend/SyntaxParseHandler.h index a9ca91521075..f023ee56f5c6 100644 --- a/js/src/frontend/SyntaxParseHandler.h +++ b/js/src/frontend/SyntaxParseHandler.h @@ -354,7 +354,7 @@ class SyntaxParseHandler // Careful: we're asking this well after the name was parsed, so the // value returned may not correspond to |kid|'s actual name. But it // *will* be truthy iff |kid| was a name, so we're safe. - MOZ_ASSERT(maybeUnparenthesizedName(kid)); + MOZ_ASSERT(isUnparenthesizedName(kid)); return NodeGeneric; } @@ -551,30 +551,35 @@ class SyntaxParseHandler bool isConstant(Node pn) { return false; } - PropertyName* maybeUnparenthesizedName(Node node) { - if (node == NodeUnparenthesizedName || - node == NodeUnparenthesizedArgumentsName || - node == NodeUnparenthesizedEvalName) - { - return lastAtom->asPropertyName(); - } - return nullptr; + bool isUnparenthesizedName(Node node) { + return node == NodeUnparenthesizedArgumentsName || + node == NodeUnparenthesizedEvalName || + node == NodeUnparenthesizedName; } - PropertyName* maybeParenthesizedName(Node node) { - if (node == NodeParenthesizedName || - node == NodeParenthesizedArgumentsName || - node == NodeParenthesizedEvalName) - { - return lastAtom->asPropertyName(); - } - return nullptr; + bool isNameAnyParentheses(Node node) { + if (isUnparenthesizedName(node)) + return true; + return node == NodeParenthesizedArgumentsName || + node == NodeParenthesizedEvalName || + node == NodeParenthesizedName; } - PropertyName* maybeNameAnyParentheses(Node node) { - if (PropertyName* name = maybeUnparenthesizedName(node)) - return name; - return maybeParenthesizedName(node); + bool nameIsEvalAnyParentheses(Node node, ExclusiveContext* cx) { + MOZ_ASSERT(isNameAnyParentheses(node), + "must only call this function on known names"); + return node == NodeUnparenthesizedEvalName || node == NodeParenthesizedEvalName; + } + + const char* nameIsArgumentsEvalAnyParentheses(Node node, ExclusiveContext* cx) { + MOZ_ASSERT(isNameAnyParentheses(node), + "must only call this method on known names"); + + if (nameIsEvalAnyParentheses(node, cx)) + return js_eval_str; + if (node == NodeUnparenthesizedArgumentsName || node == NodeParenthesizedArgumentsName) + return js_arguments_str; + return nullptr; } PropertyName* maybeDottedProperty(Node node) { diff --git a/js/src/tests/ecma_6/Statements/for-inof-name-iteration-expression-contains-index-string.js b/js/src/tests/ecma_6/Statements/for-inof-name-iteration-expression-contains-index-string.js new file mode 100644 index 000000000000..212b34db97a6 --- /dev/null +++ b/js/src/tests/ecma_6/Statements/for-inof-name-iteration-expression-contains-index-string.js @@ -0,0 +1,43 @@ +// Any copyright is dedicated to the Public Domain. +// http://creativecommons.org/licenses/publicdomain/ + +//----------------------------------------------------------------------------- +var gTestfile = "for-inof-name-iteration-expression-contains-index-string.js"; +var BUGNUMBER = 1235640; +var summary = + "Don't assert parsing a for-in/of loop whose target is a name, where the " + + "expression being iterated over contains a string containing an index"; + +print(BUGNUMBER + ": " + summary); + +/************** + * BEGIN TEST * + **************/ + +function f() +{ + var x; + for (x in "9") + continue; + assertEq(x, "0"); +} + +f(); + +function g() +{ + "use strict"; + var x = "unset"; + for (x in arguments) + continue; + assertEq(x, "unset"); +} + +g(); + +/******************************************************************************/ + +if (typeof reportCompare === "function") + reportCompare(true, true); + +print("Tests complete");