From 17eb84f5a3b632d3e4e5e5abfb1d1ae83be48237 Mon Sep 17 00:00:00 2001 From: Brian Hackett Date: Fri, 17 Jul 2015 09:26:51 -0700 Subject: [PATCH] Bug 1172895 - Handle chains of if/else statements without overrecursing in the parser and bytecode emitter, r=jorendorff. --- js/src/frontend/BytecodeEmitter.cpp | 21 ++------ js/src/frontend/FoldConstants.cpp | 51 ++++++++++++++----- js/src/frontend/Parser.cpp | 76 ++++++++++++++++++----------- 3 files changed, 92 insertions(+), 56 deletions(-) diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 7fed912a216f..35fc03faf7ee 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -1904,6 +1904,8 @@ BytecodeEmitter::checkSideEffects(ParseNode* pn, bool* answer) { JS_CHECK_RECURSION(cx, return false); + restart: + switch (pn->getKind()) { // Trivial cases with no side effects. case PNK_NEWTARGET: @@ -2151,6 +2153,7 @@ BytecodeEmitter::checkSideEffects(ParseNode* pn, bool* answer) *answer = true; return true; + case PNK_IF: case PNK_CONDITIONAL: MOZ_ASSERT(pn->isArity(PN_TERNARY)); if (!checkSideEffects(pn->pn_kid1, answer)) @@ -2161,22 +2164,8 @@ BytecodeEmitter::checkSideEffects(ParseNode* pn, bool* answer) return false; if (*answer) return true; - return checkSideEffects(pn->pn_kid3, answer); - - case PNK_IF: - MOZ_ASSERT(pn->isArity(PN_TERNARY)); - if (!checkSideEffects(pn->pn_kid1, answer)) - return false; - if (*answer) - return true; - if (!checkSideEffects(pn->pn_kid2, answer)) - return false; - if (*answer) - return true; - if (ParseNode* elseNode = pn->pn_kid3) { - if (!checkSideEffects(elseNode, answer)) - return false; - } + if ((pn = pn->pn_kid3)) + goto restart; return true; // Function calls can invoke non-local code. diff --git a/js/src/frontend/FoldConstants.cpp b/js/src/frontend/FoldConstants.cpp index bee049ff2c4c..7875c714e00d 100644 --- a/js/src/frontend/FoldConstants.cpp +++ b/js/src/frontend/FoldConstants.cpp @@ -58,6 +58,8 @@ ContainsHoistedDeclaration(ExclusiveContext* cx, ParseNode* node, bool* result) { JS_CHECK_RECURSION(cx, return false); + restart: + // With a better-typed AST, we would have distinct parse node classes for // expressions and for statements and would characterize expressions with // ExpressionKind and statements with StatementKind. Perhaps someday. In @@ -164,8 +166,8 @@ ContainsHoistedDeclaration(ExclusiveContext* cx, ParseNode* node, bool* result) if (*result) return true; - if (ParseNode* alternative = node->pn_kid3) - return ContainsHoistedDeclaration(cx, alternative, result); + if ((node = node->pn_kid3)) + goto restart; *result = false; return true; @@ -606,13 +608,27 @@ Fold(ExclusiveContext* cx, ParseNode** pnp, FullParseHandler& handler, const ReadOnlyCompileOptions& options, bool inGenexpLambda, SyntacticContext sc) { + JS_CHECK_RECURSION(cx, return false); + + ParseNode** restartNode = nullptr; + SyntacticContext restartContext; + + bool mightHaveHoistedDeclarations = true; + + if (false) { + restart: + if (!restartNode) + return true; + pnp = restartNode; + sc = restartContext; + restartNode = nullptr; + } + ParseNode* pn = *pnp; ParseNode* pn1 = nullptr; ParseNode* pn2 = nullptr; ParseNode* pn3 = nullptr; - JS_CHECK_RECURSION(cx, return false); - // First, recursively fold constants on the children of this node. switch (pn->getArity()) { case PN_CODE: @@ -673,8 +689,13 @@ Fold(ExclusiveContext* cx, ParseNode** pnp, pn2 = pn->pn_kid2; if (pn->pn_kid3) { - if (!Fold(cx, &pn->pn_kid3, handler, options, inGenexpLambda, SyntacticContext::Other)) - return false; + if (pn->isKind(PNK_IF) || pn->isKind(PNK_CONDITIONAL)) { + restartNode = &pn->pn_kid3; + restartContext = SyntacticContext::Other; + } else { + if (!Fold(cx, &pn->pn_kid3, handler, options, inGenexpLambda, SyntacticContext::Other)) + return false; + } } pn3 = pn->pn_kid3; break; @@ -748,11 +769,11 @@ Fold(ExclusiveContext* cx, ParseNode** pnp, // pn is the immediate child in question. Its descendants were already // constant-folded above, so we're done. if (sc == SyntacticContext::Delete) - return true; + goto restart; switch (pn->getKind()) { case PNK_IF: - { + if (mightHaveHoistedDeclarations) { bool result; if (ParseNode* consequent = pn2) { if (!ContainsHoistedDeclaration(cx, consequent, &result)) @@ -767,6 +788,7 @@ Fold(ExclusiveContext* cx, ParseNode** pnp, break; } } + mightHaveHoistedDeclarations = false; /* FALL THROUGH */ case PNK_CONDITIONAL: @@ -788,7 +810,7 @@ Fold(ExclusiveContext* cx, ParseNode** pnp, break; default: /* Early return to dodge common code that copies pn2 to pn. */ - return true; + goto restart; } #if JS_HAS_GENERATOR_EXPRS @@ -798,6 +820,8 @@ Fold(ExclusiveContext* cx, ParseNode** pnp, #endif if (pn2 && !pn2->isDefn()) { + if (restartNode && *restartNode == pn2) + restartNode = pnp; ReplaceNode(pnp, pn2); pn = pn2; } @@ -814,8 +838,11 @@ Fold(ExclusiveContext* cx, ParseNode** pnp, pn->setArity(PN_LIST); pn->makeEmpty(); } - if (pn3 && pn3 != pn2) + if (pn3 && pn3 != pn2) { + if (restartNode && *restartNode == pn3) + restartNode = nullptr; handler.freeTree(pn3); + } break; case PNK_OR: @@ -1028,7 +1055,7 @@ Fold(ExclusiveContext* cx, ParseNode** pnp, default: /* Return early to dodge the common PNK_NUMBER code. */ - return true; + goto restart; } pn->setKind(PNK_NUMBER); pn->setOp(JSOP_DOUBLE); @@ -1131,7 +1158,7 @@ Fold(ExclusiveContext* cx, ParseNode** pnp, } } - return true; + goto restart; } bool diff --git a/js/src/frontend/Parser.cpp b/js/src/frontend/Parser.cpp index 981475141aff..7d38c4f21e59 100644 --- a/js/src/frontend/Parser.cpp +++ b/js/src/frontend/Parser.cpp @@ -4631,42 +4631,62 @@ template typename ParseHandler::Node Parser::ifStatement(YieldHandling yieldHandling) { - uint32_t begin = pos().begin; - - /* An IF node has three kids: condition, then, and optional else. */ - Node cond = condition(InAllowed, yieldHandling); - if (!cond) - return null(); - - TokenKind tt; - if (!tokenStream.peekToken(&tt, TokenStream::Operand)) - return null(); - if (tt == TOK_SEMI) { - if (!report(ParseExtraWarning, false, null(), JSMSG_EMPTY_CONSEQUENT)) - return null(); - } + Vector condList(context), thenList(context); + Vector posList(context); + Node elseBranch; StmtInfoPC stmtInfo(context); PushStatementPC(pc, &stmtInfo, STMT_IF); - Node thenBranch = statement(yieldHandling); - if (!thenBranch) - return null(); - Node elseBranch; - bool matched; - if (!tokenStream.matchToken(&matched, TOK_ELSE, TokenStream::Operand)) - return null(); - if (matched) { - stmtInfo.type = STMT_ELSE; - elseBranch = statement(yieldHandling); - if (!elseBranch) + while (true) { + uint32_t begin = pos().begin; + + /* An IF node has three kids: condition, then, and optional else. */ + Node cond = condition(InAllowed, yieldHandling); + if (!cond) return null(); - } else { - elseBranch = null(); + + TokenKind tt; + if (!tokenStream.peekToken(&tt, TokenStream::Operand)) + return null(); + if (tt == TOK_SEMI) { + if (!report(ParseExtraWarning, false, null(), JSMSG_EMPTY_CONSEQUENT)) + return null(); + } + + Node thenBranch = statement(yieldHandling); + if (!thenBranch) + return null(); + + if (!condList.append(cond) || !thenList.append(thenBranch) || !posList.append(begin)) + return null(); + + bool matched; + if (!tokenStream.matchToken(&matched, TOK_ELSE, TokenStream::Operand)) + return null(); + if (matched) { + if (!tokenStream.matchToken(&matched, TOK_IF, TokenStream::Operand)) + return null(); + if (matched) + continue; + elseBranch = statement(yieldHandling); + if (!elseBranch) + return null(); + } else { + elseBranch = null(); + } + break; } PopStatementPC(tokenStream, pc); - return handler.newIfStatement(begin, cond, thenBranch, elseBranch); + + for (int i = condList.length() - 1; i >= 0; i--) { + elseBranch = handler.newIfStatement(posList[i], condList[i], thenList[i], elseBranch); + if (!elseBranch) + return null(); + } + + return elseBranch; } template