[parsing] Fix detection of invalid continue targets.

In order to know which labels are valid continue targets, we must
track the labels that immediately prefix an iteration statement.

Also document some things that I had to figure out.

Bug: v8:8033
Change-Id: Ia8288fd0e553a547aa0f9d1b4381bb103325bc3a
Reviewed-on: https://chromium-review.googlesource.com/1172292Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55110}

src/ast/ast.h

@@ -249,7 +249,23 @@ class Expression : public AstNode {
   static const uint8_t kNextBitFieldIndex = AstNode::kNextBitFieldIndex;
 };
 
-
+// V8's notion of BreakableStatement does not correspond to the notion of
+// BreakableStatement in ECMAScript. In V8, the idea is that a
+// BreakableStatement is a statement that can be the target of a break
+// statement.  The BreakableStatement AST node carries a list of labels, any of
+// which can be used as an argument to the break statement in order to target
+// it.
+//
+// Since we don't want to attach a list of labels to all kinds of statements, we
+// only declare switchs, loops, and blocks as BreakableStatements.  This means
+// that we implement breaks targeting other statement forms as breaks targeting
+// a substatement thereof. For instance, in "foo: if (b) { f(); break foo; }" we
+// pretend that foo is the label of the inner block. That's okay because one
+// can't observe the difference.
+//
+// This optimization makes it harder to detect invalid continue labels, see the
+// need for own_labels in IterationStatement.
+//
 class BreakableStatement : public Statement {
  public:
   enum BreakableType {
@@ -257,6 +273,13 @@ class BreakableStatement : public Statement {
     TARGET_FOR_NAMED_ONLY
   };
 
+  // A list of all labels declared on the path up to the previous
+  // BreakableStatement (if any).
+  //
+  // Example: "l1: for (;;) l2: l3: { l4: if (b) l5: { s } }"
+  // labels() of the ForStatement will be l1.
+  // labels() of the Block { l4: ... } will be l2, l3.
+  // labels() of the Block { s } will be l4, l5.
   ZonePtrList<const AstRawString>* labels() const;
 
   // Testers.
@@ -441,11 +464,23 @@ class IterationStatement : public BreakableStatement {
 
   ZonePtrList<const AstRawString>* labels() const { return labels_; }
 
+  // A list of all labels that the iteration statement is directly prefixed
+  // with, i.e.  all the labels that a continue statement in the body can use to
+  // continue this iteration statement. This is always a subset of {labels}.
+  //
+  // Example: "l1: { l2: if (b) l3: l4: for (;;) s }"
+  // labels() of the Block will be l1.
+  // labels() of the ForStatement will be l2, l3, l4.
+  // own_labels() of the ForStatement will be l3, l4.
+  ZonePtrList<const AstRawString>* own_labels() const { return own_labels_; }
+
  protected:
-  IterationStatement(ZonePtrList<const AstRawString>* labels, int pos,
+  IterationStatement(ZonePtrList<const AstRawString>* labels,
+                     ZonePtrList<const AstRawString>* own_labels, int pos,
                      NodeType type)
       : BreakableStatement(TARGET_FOR_ANONYMOUS, pos, type),
         labels_(labels),
+        own_labels_(own_labels),
         body_(nullptr) {}
   void Initialize(Statement* body) { body_ = body; }
 
@@ -454,6 +489,7 @@ class IterationStatement : public BreakableStatement {
 
  private:
   ZonePtrList<const AstRawString>* labels_;
+  ZonePtrList<const AstRawString>* own_labels_;
   Statement* body_;
 };
 
@@ -470,8 +506,10 @@ class DoWhileStatement final : public IterationStatement {
  private:
   friend class AstNodeFactory;
 
-  DoWhileStatement(ZonePtrList<const AstRawString>* labels, int pos)
-      : IterationStatement(labels, pos, kDoWhileStatement), cond_(nullptr) {}
+  DoWhileStatement(ZonePtrList<const AstRawString>* labels,
+                   ZonePtrList<const AstRawString>* own_labels, int pos)
+      : IterationStatement(labels, own_labels, pos, kDoWhileStatement),
+        cond_(nullptr) {}
 
   Expression* cond_;
 };
@@ -489,8 +527,10 @@ class WhileStatement final : public IterationStatement {
  private:
   friend class AstNodeFactory;
 
-  WhileStatement(ZonePtrList<const AstRawString>* labels, int pos)
-      : IterationStatement(labels, pos, kWhileStatement), cond_(nullptr) {}
+  WhileStatement(ZonePtrList<const AstRawString>* labels,
+                 ZonePtrList<const AstRawString>* own_labels, int pos)
+      : IterationStatement(labels, own_labels, pos, kWhileStatement),
+        cond_(nullptr) {}
 
   Expression* cond_;
 };
@@ -513,8 +553,9 @@ class ForStatement final : public IterationStatement {
  private:
   friend class AstNodeFactory;
 
-  ForStatement(ZonePtrList<const AstRawString>* labels, int pos)
-      : IterationStatement(labels, pos, kForStatement),
+  ForStatement(ZonePtrList<const AstRawString>* labels,
+               ZonePtrList<const AstRawString>* own_labels, int pos)
+      : IterationStatement(labels, own_labels, pos, kForStatement),
         init_(nullptr),
         cond_(nullptr),
         next_(nullptr) {}
@@ -539,9 +580,10 @@ class ForEachStatement : public IterationStatement {
   }
 
  protected:
-  ForEachStatement(ZonePtrList<const AstRawString>* labels, int pos,
+  ForEachStatement(ZonePtrList<const AstRawString>* labels,
+                   ZonePtrList<const AstRawString>* own_labels, int pos,
                    NodeType type)
-      : IterationStatement(labels, pos, type) {}
+      : IterationStatement(labels, own_labels, pos, type) {}
 };
 
 
@@ -566,8 +608,9 @@ class ForInStatement final : public ForEachStatement {
  private:
   friend class AstNodeFactory;
 
-  ForInStatement(ZonePtrList<const AstRawString>* labels, int pos)
-      : ForEachStatement(labels, pos, kForInStatement),
+  ForInStatement(ZonePtrList<const AstRawString>* labels,
+                 ZonePtrList<const AstRawString>* own_labels, int pos)
+      : ForEachStatement(labels, own_labels, pos, kForInStatement),
         each_(nullptr),
         subject_(nullptr) {
     bit_field_ = ForInTypeField::update(bit_field_, SLOW_FOR_IN);
@@ -632,8 +675,9 @@ class ForOfStatement final : public ForEachStatement {
  private:
   friend class AstNodeFactory;
 
-  ForOfStatement(ZonePtrList<const AstRawString>* labels, int pos)
-      : ForEachStatement(labels, pos, kForOfStatement),
+  ForOfStatement(ZonePtrList<const AstRawString>* labels,
+                 ZonePtrList<const AstRawString>* own_labels, int pos)
+      : ForEachStatement(labels, own_labels, pos, kForOfStatement),
         iterator_(nullptr),
         assign_iterator_(nullptr),
         next_result_(nullptr),
@@ -2810,9 +2854,11 @@ class AstNodeFactory final BASE_EMBEDDED {
                      Block(zone_, labels, capacity, ignore_completion_value);
   }
 
-#define STATEMENT_WITH_LABELS(NodeType)                                       \
-  NodeType* New##NodeType(ZonePtrList<const AstRawString>* labels, int pos) { \
-    return new (zone_) NodeType(labels, pos);                                 \
+#define STATEMENT_WITH_LABELS(NodeType)                                \
+  NodeType* New##NodeType(ZonePtrList<const AstRawString>* labels,     \
+                          ZonePtrList<const AstRawString>* own_labels, \
+                          int pos) {                                   \
+    return new (zone_) NodeType(labels, own_labels, pos);              \
   }
   STATEMENT_WITH_LABELS(DoWhileStatement)
   STATEMENT_WITH_LABELS(WhileStatement)
@@ -2824,23 +2870,25 @@ class AstNodeFactory final BASE_EMBEDDED {
     return new (zone_) SwitchStatement(zone_, labels, tag, pos);
   }
 
-  ForEachStatement* NewForEachStatement(ForEachStatement::VisitMode visit_mode,
-                                        ZonePtrList<const AstRawString>* labels,
-                                        int pos) {
+  ForEachStatement* NewForEachStatement(
+      ForEachStatement::VisitMode visit_mode,
+      ZonePtrList<const AstRawString>* labels,
+      ZonePtrList<const AstRawString>* own_labels, int pos) {
     switch (visit_mode) {
       case ForEachStatement::ENUMERATE: {
-        return new (zone_) ForInStatement(labels, pos);
+        return new (zone_) ForInStatement(labels, own_labels, pos);
       }
       case ForEachStatement::ITERATE: {
-        return new (zone_) ForOfStatement(labels, pos);
+        return new (zone_) ForOfStatement(labels, own_labels, pos);
       }
     }
     UNREACHABLE();
   }
 
   ForOfStatement* NewForOfStatement(ZonePtrList<const AstRawString>* labels,
+                                    ZonePtrList<const AstRawString>* own_labels,
                                     int pos) {
-    return new (zone_) ForOfStatement(labels, pos);
+    return new (zone_) ForOfStatement(labels, own_labels, pos);
   }
 
   ExpressionStatement* NewExpressionStatement(Expression* expression, int pos) {

src/ast/prettyprinter.cc

@@ -749,9 +749,11 @@ void AstPrinter::PrintLiteralWithModeIndented(const char* info, Variable* var,
   }
 }
 
-void AstPrinter::PrintLabelsIndented(ZonePtrList<const AstRawString>* labels) {
+void AstPrinter::PrintLabelsIndented(ZonePtrList<const AstRawString>* labels,
+                                     const char* prefix) {
   if (labels == nullptr || labels->length() == 0) return;
-  PrintIndented("LABELS ");
+  PrintIndented(prefix);
+  Print("LABELS ");
   PrintLabels(labels);
   Print("\n");
 }
@@ -922,6 +924,7 @@ void AstPrinter::VisitSwitchStatement(SwitchStatement* node) {
 void AstPrinter::VisitDoWhileStatement(DoWhileStatement* node) {
   IndentedScope indent(this, "DO", node->position());
   PrintLabelsIndented(node->labels());
+  PrintLabelsIndented(node->own_labels(), "OWN ");
   PrintIndentedVisit("BODY", node->body());
   PrintIndentedVisit("COND", node->cond());
 }
@@ -930,6 +933,7 @@ void AstPrinter::VisitDoWhileStatement(DoWhileStatement* node) {
 void AstPrinter::VisitWhileStatement(WhileStatement* node) {
   IndentedScope indent(this, "WHILE", node->position());
   PrintLabelsIndented(node->labels());
+  PrintLabelsIndented(node->own_labels(), "OWN ");
   PrintIndentedVisit("COND", node->cond());
   PrintIndentedVisit("BODY", node->body());
 }
@@ -938,6 +942,7 @@ void AstPrinter::VisitWhileStatement(WhileStatement* node) {
 void AstPrinter::VisitForStatement(ForStatement* node) {
   IndentedScope indent(this, "FOR", node->position());
   PrintLabelsIndented(node->labels());
+  PrintLabelsIndented(node->own_labels(), "OWN ");
   if (node->init()) PrintIndentedVisit("INIT", node->init());
   if (node->cond()) PrintIndentedVisit("COND", node->cond());
   PrintIndentedVisit("BODY", node->body());
@@ -947,6 +952,8 @@ void AstPrinter::VisitForStatement(ForStatement* node) {
 
 void AstPrinter::VisitForInStatement(ForInStatement* node) {
   IndentedScope indent(this, "FOR IN", node->position());
+  PrintLabelsIndented(node->labels());
+  PrintLabelsIndented(node->own_labels(), "OWN ");
   PrintIndentedVisit("FOR", node->each());
   PrintIndentedVisit("IN", node->enumerable());
   PrintIndentedVisit("BODY", node->body());
@@ -955,6 +962,8 @@ void AstPrinter::VisitForInStatement(ForInStatement* node) {
 
 void AstPrinter::VisitForOfStatement(ForOfStatement* node) {
   IndentedScope indent(this, "FOR OF", node->position());
+  PrintLabelsIndented(node->labels());
+  PrintLabelsIndented(node->own_labels(), "OWN ");
   PrintIndentedVisit("INIT", node->assign_iterator());
   PrintIndentedVisit("NEXT", node->next_result());
   PrintIndentedVisit("DONE", node->result_done());

src/ast/prettyprinter.h

@@ -110,7 +110,8 @@ class AstPrinter final : public AstVisitor<AstPrinter> {
                             bool quote);
   void PrintLiteralWithModeIndented(const char* info, Variable* var,
                                     const AstRawString* value);
-  void PrintLabelsIndented(ZonePtrList<const AstRawString>* labels);
+  void PrintLabelsIndented(ZonePtrList<const AstRawString>* labels,
+                           const char* prefix = "");
   void PrintObjectProperties(ZonePtrList<ObjectLiteral::Property>* properties);
   void PrintClassProperties(ZonePtrList<ClassLiteral::Property>* properties);
 

src/parsing/parser.cc

@@ -1470,29 +1470,40 @@ Statement* Parser::DeclareNative(const AstRawString* name, int pos, bool* ok) {
       pos);
 }
 
-ZonePtrList<const AstRawString>* Parser::DeclareLabel(
-    ZonePtrList<const AstRawString>* labels, VariableProxy* var, bool* ok) {
+void Parser::DeclareLabel(ZonePtrList<const AstRawString>** labels,
+                          ZonePtrList<const AstRawString>** own_labels,
+                          VariableProxy* var, bool* ok) {
   DCHECK(IsIdentifier(var));
   const AstRawString* label = var->raw_name();
+
   // TODO(1240780): We don't check for redeclaration of labels
   // during preparsing since keeping track of the set of active
   // labels requires nontrivial changes to the way scopes are
   // structured.  However, these are probably changes we want to
   // make later anyway so we should go back and fix this then.
-  if (ContainsLabel(labels, label) || TargetStackContainsLabel(label)) {
+  if (ContainsLabel(*labels, label) || TargetStackContainsLabel(label)) {
     ReportMessage(MessageTemplate::kLabelRedeclaration, label);
     *ok = false;
-    return nullptr;
+    return;
   }
-  if (labels == nullptr) {
-    labels = new (zone()) ZonePtrList<const AstRawString>(1, zone());
+
+  // Add {label} to both {labels} and {own_labels}.
+  if (*labels == nullptr) {
+    DCHECK_NULL(*own_labels);
+    *labels = new (zone()) ZonePtrList<const AstRawString>(1, zone());
+    *own_labels = new (zone()) ZonePtrList<const AstRawString>(1, zone());
+  } else {
+    if (*own_labels == nullptr) {
+      *own_labels = new (zone()) ZonePtrList<const AstRawString>(1, zone());
+    }
   }
-  labels->Add(label, zone());
+  (*labels)->Add(label, zone());
+  (*own_labels)->Add(label, zone());
+
   // Remove the "ghost" variable that turned out to be a label
   // from the top scope. This way, we don't try to resolve it
   // during the scope processing.
   scope()->RemoveUnresolved(var);
-  return labels;
 }
 
 bool Parser::ContainsLabel(ZonePtrList<const AstRawString>* labels,
@@ -2191,7 +2202,7 @@ Statement* Parser::DesugarLexicalBindingsInForStatement(
   // need to know about it. This should be safe because we don't run any code
   // in this function that looks up break targets.
   ForStatement* outer_loop =
-      factory()->NewForStatement(nullptr, kNoSourcePosition);
+      factory()->NewForStatement(nullptr, nullptr, kNoSourcePosition);
   outer_block->statements()->Add(outer_loop, zone());
   outer_block->set_scope(scope());
 
@@ -3393,9 +3404,10 @@ IterationStatement* Parser::LookupContinueTarget(const AstRawString* label,
     if (stat == nullptr) continue;
 
     DCHECK(stat->is_target_for_anonymous());
-    if (anonymous || ContainsLabel(stat->labels(), label)) {
+    if (anonymous || ContainsLabel(stat->own_labels(), label)) {
       return stat;
     }
+    if (ContainsLabel(stat->labels(), label)) break;
   }
   return nullptr;
 }

src/parsing/parser.h

@@ -284,8 +284,9 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   Block* BuildInitializationBlock(DeclarationParsingResult* parsing_result,
                                   ZonePtrList<const AstRawString>* names,
                                   bool* ok);
-  ZonePtrList<const AstRawString>* DeclareLabel(
-      ZonePtrList<const AstRawString>* labels, VariableProxy* expr, bool* ok);
+  void DeclareLabel(ZonePtrList<const AstRawString>** labels,
+                    ZonePtrList<const AstRawString>** own_labels,
+                    VariableProxy* expr, bool* ok);
   bool ContainsLabel(ZonePtrList<const AstRawString>* labels,
                      const AstRawString* label);
   Expression* RewriteReturn(Expression* return_value, int pos);

src/parsing/pattern-rewriter.cc

@@ -671,7 +671,8 @@ void PatternRewriter::VisitArrayLiteral(ArrayLiteral* node,
     //   #maybe_store_and_unset_done;
     //   #increment_index;
     // }
-    WhileStatement* loop = factory()->NewWhileStatement(nullptr, nopos);
+    WhileStatement* loop =
+        factory()->NewWhileStatement(nullptr, nullptr, nopos);
     {
       Expression* condition = factory()->NewUnaryOperation(
           Token::NOT, factory()->NewVariableProxy(done), nopos);

src/parsing/preparser.h

@@ -788,12 +788,14 @@ class PreParserFactory {
   }
 
   PreParserStatement NewDoWhileStatement(
-      ZonePtrList<const AstRawString>* labels, int pos) {
+      ZonePtrList<const AstRawString>* labels,
+      ZonePtrList<const AstRawString>* own_labels, int pos) {
     return PreParserStatement::Default();
   }
 
-  PreParserStatement NewWhileStatement(ZonePtrList<const AstRawString>* labels,
-                                       int pos) {
+  PreParserStatement NewWhileStatement(
+      ZonePtrList<const AstRawString>* labels,
+      ZonePtrList<const AstRawString>* own_labels, int pos) {
     return PreParserStatement::Default();
   }
 
@@ -808,19 +810,22 @@ class PreParserFactory {
     return PreParserStatement::Default();
   }
 
-  PreParserStatement NewForStatement(ZonePtrList<const AstRawString>* labels,
-                                     int pos) {
+  PreParserStatement NewForStatement(
+      ZonePtrList<const AstRawString>* labels,
+      ZonePtrList<const AstRawString>* own_labels, int pos) {
     return PreParserStatement::Default();
   }
 
   PreParserStatement NewForEachStatement(
       ForEachStatement::VisitMode visit_mode,
-      ZonePtrList<const AstRawString>* labels, int pos) {
+      ZonePtrList<const AstRawString>* labels,
+      ZonePtrList<const AstRawString>* own_labels, int pos) {
     return PreParserStatement::Default();
   }
 
-  PreParserStatement NewForOfStatement(ZonePtrList<const AstRawString>* labels,
-                                       int pos) {
+  PreParserStatement NewForOfStatement(
+      ZonePtrList<const AstRawString>* labels,
+      ZonePtrList<const AstRawString>* own_labels, int pos) {
     return PreParserStatement::Default();
   }
 
@@ -1071,12 +1076,11 @@ class PreParser : public ParserBase<PreParser> {
       const DeclarationParsingResult::Declaration* declaration,
       ZonePtrList<const AstRawString>* names, bool* ok);
 
-  V8_INLINE ZonePtrList<const AstRawString>* DeclareLabel(
-      ZonePtrList<const AstRawString>* labels, const PreParserExpression& expr,
-      bool* ok) {
+  V8_INLINE void DeclareLabel(ZonePtrList<const AstRawString>** labels,
+                              ZonePtrList<const AstRawString>** own_labels,
+                              const PreParserExpression& expr, bool* ok) {
     DCHECK(!parsing_module_ || !expr.AsIdentifier().IsAwait());
     DCHECK(IsIdentifier(expr));
-    return labels;
   }
 
   // TODO(nikolaos): The preparser currently does not keep track of labels.

test/mjsunit/regress/regress-8033.js

+// Copyright 2018 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+assertThrows("foo: if (true) do { continue foo } while (false)", SyntaxError);
+assertThrows("foo: if (true) while (false) { continue foo }", SyntaxError);
+assertThrows("foo: if (true) for (; false; ) { continue foo }", SyntaxError);
+assertThrows("foo: if (true) for (let x of []) { continue foo }", SyntaxError);
+assertThrows("foo: if (true) for (let x in []) { continue foo }", SyntaxError);
+
+assertThrows("foo: if (true) { do { continue foo } while (false) }", SyntaxError);
+assertThrows("foo: if (true) { while (false) { continue foo } }", SyntaxError);
+assertThrows("foo: if (true) { for (; false; ) { continue foo } }", SyntaxError);
+assertThrows("foo: if (true) { for (let x of []) { continue foo } }", SyntaxError);
+assertThrows("foo: if (true) { for (let x in []) { continue foo } }", SyntaxError);
+
+assertThrows("foo: goo: if (true) do { continue foo } while (false)", SyntaxError);
+assertThrows("foo: goo: if (true) while (false) { continue foo }", SyntaxError);
+assertThrows("foo: goo: if (true) for (; false; ) { continue foo }", SyntaxError);
+assertThrows("foo: goo: if (true) for (let x of []) { continue foo }", SyntaxError);
+assertThrows("foo: goo: if (true) for (let x in []) { continue foo }", SyntaxError);
+
+assertThrows("foo: goo: if (true) { do { continue foo } while (false) }", SyntaxError);
+assertThrows("foo: goo: if (true) { while (false) { continue foo } }", SyntaxError);
+assertThrows("foo: goo: if (true) { for (; false; ) { continue foo } }", SyntaxError);
+assertThrows("foo: goo: if (true) { for (let x of []) { continue foo } }", SyntaxError);
+assertThrows("foo: goo: if (true) { for (let x in []) { continue foo } }", SyntaxError);
+
+assertDoesNotThrow("if (true) foo: goo: do { continue foo } while (false)");
+assertDoesNotThrow("if (true) foo: goo: while (false) { continue foo }");
+assertDoesNotThrow("if (true) foo: goo: for (; false; ) { continue foo }");
+assertDoesNotThrow("if (true) foo: goo: for (let x of []) { continue foo }");
+assertDoesNotThrow("if (true) foo: goo: for (let x in []) { continue foo }");
+
+assertThrows("if (true) foo: goo: { do { continue foo } while (false) }", SyntaxError);
+assertThrows("if (true) foo: goo: { while (false) { continue foo } }", SyntaxError);
+assertThrows("if (true) foo: goo: { for (; false; ) { continue foo } }", SyntaxError);
+assertThrows("if (true) foo: goo: { for (let x of []) { continue foo } }", SyntaxError);
+assertThrows("if (true) foo: goo: { for (let x in []) { continue foo } }", SyntaxError);
+
+assertDoesNotThrow("if (true) { foo: goo: do { continue foo } while (false) }");
+assertDoesNotThrow("if (true) { foo: goo: while (false) { continue foo } }");
+assertDoesNotThrow("if (true) { foo: goo: for (; false; ) { continue foo } }");
+assertDoesNotThrow("if (true) { foo: goo: for (let x of []) { continue foo } }");
+assertDoesNotThrow("if (true) { foo: goo: for (let x in []) { continue foo } }");