Fix FuncNameInferrer usage in ParseAssignmentExpression

Without this fix, AssignmentExpressions that happen to be arrow functions
would lead to unbalanced Enter/Leave calls on the fni_, causing thrashing
while trying to infer function names. Symptoms include slow parsing
or OOM (when we create too many AstConsStrings).

To try to keep this from happening in the future, added an RAII helper
class to handle Entering/Leaving FNI state.

The included regression test crashes on my workstation without the patch.
Note that it's too slow in debug mode (as well as under TurboFan),
so I've skipped it there.

BUG=v8:4595
LOG=y

Review URL: https://codereview.chromium.org/1507283003

Cr-Commit-Position: refs/heads/master@{#32768}

src/parsing/func-name-inferrer.h

@@ -30,17 +30,29 @@ class FuncNameInferrer : public ZoneObject {
  public:
   FuncNameInferrer(AstValueFactory* ast_value_factory, Zone* zone);
 
+  // To enter function name inference state, put a FuncNameInferrer::State
+  // on the stack.
+  class State {
+   public:
+    explicit State(FuncNameInferrer* fni) : fni_(fni) {
+      if (fni_ != nullptr) fni_->Enter();
+    }
+    ~State() {
+      if (fni_ != nullptr) fni_->Leave();
+    }
+
+   private:
+    FuncNameInferrer* fni_;
+
+    DISALLOW_COPY_AND_ASSIGN(State);
+  };
+
   // Returns whether we have entered name collection state.
   bool IsOpen() const { return !entries_stack_.is_empty(); }
 
   // Pushes an enclosing the name of enclosing function onto names stack.
   void PushEnclosingName(const AstRawString* name);
 
-  // Enters name collection state.
-  void Enter() {
-    entries_stack_.Add(names_stack_.length(), zone());
-  }
-
   // Pushes an encountered name onto names stack when in collection state.
   void PushLiteralName(const AstRawString* name);
 
@@ -67,14 +79,6 @@ class FuncNameInferrer : public ZoneObject {
     }
   }
 
-  // Leaves names collection state.
-  void Leave() {
-    DCHECK(IsOpen());
-    names_stack_.Rewind(entries_stack_.RemoveLast());
-    if (entries_stack_.is_empty())
-      funcs_to_infer_.Clear();
-  }
-
  private:
   enum NameType {
     kEnclosingConstructorName,
@@ -87,6 +91,14 @@ class FuncNameInferrer : public ZoneObject {
     NameType type;
   };
 
+  void Enter() { entries_stack_.Add(names_stack_.length(), zone()); }
+
+  void Leave() {
+    DCHECK(IsOpen());
+    names_stack_.Rewind(entries_stack_.RemoveLast());
+    if (entries_stack_.is_empty()) funcs_to_infer_.Clear();
+  }
+
   Zone* zone() const { return zone_; }
 
   // Constructs a full name in dotted notation from gathered names.

src/parsing/parser-base.h

@@ -1847,7 +1847,7 @@ typename ParserBase<Traits>::ExpressionT ParserBase<Traits>::ParseObjectLiteral(
   Expect(Token::LBRACE, CHECK_OK);
 
   while (peek() != Token::RBRACE) {
-    if (fni_ != nullptr) fni_->Enter();
+    FuncNameInferrer::State fni_state(fni_);
 
     const bool in_class = false;
     const bool is_static = false;
@@ -1878,10 +1878,7 @@ typename ParserBase<Traits>::ExpressionT ParserBase<Traits>::ParseObjectLiteral(
       Expect(Token::COMMA, CHECK_OK);
     }
 
-    if (fni_ != nullptr) {
-      fni_->Infer();
-      fni_->Leave();
-    }
+    if (fni_ != nullptr) fni_->Infer();
   }
   Expect(Token::RBRACE, CHECK_OK);
 
@@ -1984,7 +1981,7 @@ ParserBase<Traits>::ParseAssignmentExpression(bool accept_IN, int flags,
     return this->ParseYieldExpression(classifier, ok);
   }
 
-  if (fni_ != NULL) fni_->Enter();
+  FuncNameInferrer::State fni_state(fni_);
   ParserBase<Traits>::Checkpoint checkpoint(this);
   ExpressionClassifier arrow_formals_classifier(classifier->duplicate_finder());
   bool parenthesized_formals = peek() == Token::LPAREN;
@@ -2028,6 +2025,9 @@ ParserBase<Traits>::ParseAssignmentExpression(bool accept_IN, int flags,
           Scanner::Location(lhs_beg_pos, scanner()->location().end_pos),
           MessageTemplate::kInvalidDestructuringTarget);
     }
+
+    if (fni_ != nullptr) fni_->Infer();
+
     return expression;
   }
 
@@ -2049,7 +2049,6 @@ ParserBase<Traits>::ParseAssignmentExpression(bool accept_IN, int flags,
   //    allow_harmony_destructuring_bind() && maybe_pattern && !is_rhs;
 
   if (!Token::IsAssignmentOp(peek())) {
-    if (fni_ != NULL) fni_->Leave();
     // Parsed conditional expression only (no assignment).
     if (is_pattern_element && !this->IsValidReferenceExpression(expression) &&
         !maybe_pattern) {
@@ -2125,7 +2124,6 @@ ParserBase<Traits>::ParseAssignmentExpression(bool accept_IN, int flags,
     } else {
       fni_->RemoveLastFunction();
     }
-    fni_->Leave();
   }
 
   ExpressionT result = factory()->NewAssignment(op, expression, right, pos);
@@ -2604,7 +2602,7 @@ ParserBase<Traits>::ParseStrongInitializationExpression(
   //  'this' '.' IdentifierName '=' AssignmentExpression
   //  'this' '[' Expression ']' '=' AssignmentExpression
 
-  if (fni_ != NULL) fni_->Enter();
+  FuncNameInferrer::State fni_state(fni_);
 
   Consume(Token::THIS);
   int pos = position();
@@ -2663,7 +2661,6 @@ ParserBase<Traits>::ParseStrongInitializationExpression(
     } else {
       fni_->RemoveLastFunction();
     }
-    fni_->Leave();
   }
 
   if (function_state_->return_location().IsValid()) {

src/parsing/parser.cc

@@ -2253,10 +2253,8 @@ Statement* Parser::ParseFunctionDeclaration(
   const AstRawString* name = ParseIdentifierOrStrictReservedWord(
       &is_strict_reserved, CHECK_OK);
 
-  if (fni_ != NULL) {
-    fni_->Enter();
-    fni_->PushEnclosingName(name);
-  }
+  FuncNameInferrer::State fni_state(fni_);
+  if (fni_ != NULL) fni_->PushEnclosingName(name);
   FunctionLiteral* fun = ParseFunctionLiteral(
       name, scanner()->location(),
       is_strict_reserved ? kFunctionNameIsStrictReserved
@@ -2265,7 +2263,6 @@ Statement* Parser::ParseFunctionDeclaration(
                    : FunctionKind::kNormalFunction,
       pos, FunctionLiteral::DECLARATION, FunctionLiteral::NORMAL_ARITY,
       language_mode(), CHECK_OK);
-  if (fni_ != NULL) fni_->Leave();
 
   // Even if we're not at the top-level of the global or a function
   // scope, we treat it as such and introduce the function with its
@@ -2495,7 +2492,7 @@ void Parser::ParseVariableDeclarations(VariableDeclarationContext var_context,
   int bindings_start = peek_position();
   bool is_for_iteration_variable;
   do {
-    if (fni_ != NULL) fni_->Enter();
+    FuncNameInferrer::State fni_state(fni_);
 
     // Parse name.
     if (!first_declaration) Consume(Token::COMMA);
@@ -2585,7 +2582,6 @@ void Parser::ParseVariableDeclarations(VariableDeclarationContext var_context,
       value = GetLiteralUndefined(position());
     }
 
-    if (single_name && fni_ != NULL) fni_->Leave();
     parsing_result->declarations.Add(DeclarationParsingResult::Declaration(
         pattern, initializer_position, value));
     first_declaration = false;
@@ -4943,7 +4939,7 @@ ClassLiteral* Parser::ParseClassLiteral(const AstRawString* name,
   const bool has_extends = extends != nullptr;
   while (peek() != Token::RBRACE) {
     if (Check(Token::SEMICOLON)) continue;
-    if (fni_ != NULL) fni_->Enter();
+    FuncNameInferrer::State fni_state(fni_);
     const bool in_class = true;
     const bool is_static = false;
     bool is_computed_name = false;  // Classes do not care about computed
@@ -4961,10 +4957,7 @@ ClassLiteral* Parser::ParseClassLiteral(const AstRawString* name,
       properties->Add(property, zone());
     }
 
-    if (fni_ != NULL) {
-      fni_->Infer();
-      fni_->Leave();
-    }
+    if (fni_ != NULL) fni_->Infer();
   }
 
   Expect(Token::RBRACE, CHECK_OK);

test/mjsunit/mjsunit.status

@@ -199,6 +199,9 @@
   # Too slow in debug mode for GC stress mode.
   'regress/regress-crbug-217858': [PASS, ['mode == debug', SKIP]],
 
+  # Too slow in debug mode and under turbofan.
+  'regress/regress-4595': [PASS, NO_VARIANTS, ['mode == debug', SKIP]],
+
   ##############################################################################
   # Only RegExp stuff tested, no need for extensive optimizing compiler tests.
   'regexp-global': [PASS, NO_VARIANTS],