deps: v8, cherry-pick 9365d09, aac2f8c, 47d34a3

Original commit message 9365d09: [coverage] Rework continuation counter handling This changes a few bits about how continuation counters are handled. It introduces a new mechanism that allows removal of a continuation range after it has been created. If coverage is enabled, we run a first post-processing pass on the AST immediately after parsing, which removes problematic continuation ranges in two situations: 1. nested continuation counters - only the outermost stays alive. 2. trailing continuation counters within a block-like structure are removed if the containing structure itself has a continuation. R=bmeurer@chromium.org, jgruber@chromium.org, yangguo@chromium.org Bug: v8:8381, v8:8539 Change-Id: I6bcaea5060d8c481d7bae099f6db9f993cc30ee3 Reviewed-on: https://chromium-review.googlesource.com/c/1339119 Reviewed-by: Yang Guo <yangguo@chromium.org> Reviewed-by: Leszek Swirski <leszeks@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Commit-Queue: Jakob Gruber <jgruber@chromium.org> Cr-Commit-Position: refs/heads/master@{#58443} Refs: v8/v8@9365d09 Original commit message aac2f8c: [coverage] Filter out singleton ranges that alias full ranges Block coverage is based on a system of ranges that can either have both a start and end position, or only a start position (so-called singleton ranges). When formatting coverage information, singletons are expanded until the end of the immediate full parent range. E.g. in: {0, 10} // Full range. {5, -1} // Singleton range. the singleton range is expanded to {5, 10}. Singletons are produced mostly for continuation counters that track whether we execute past a specific language construct. Unfortunately, continuation counters can turn up in spots that confuse our post-processing. For example: if (true) { ... block1 ... } else { ... block2 ... } If block1 produces a continuation counter, it could end up with the same start position as the else-branch counter. Since we merge identical blocks, the else-branch could incorrectly end up with an execution count of one. We need to avoid merging such cases. A full range should always take precedence over a singleton range; a singleton range should never expand to completely fill a full range. An additional post-processing pass ensures this. Bug: v8:8237 Change-Id: Idb3ec7b2feddc0585313810b9c8be1e9f4ec64bf Reviewed-on: https://chromium-review.googlesource.com/c/1273095 Reviewed-by: Georg Neis <neis@chromium.org> Reviewed-by: Yang Guo <yangguo@chromium.org> Commit-Queue: Jakob Gruber <jgruber@chromium.org> Cr-Commit-Position: refs/heads/master@{#56531} Refs: v8/v8@aac2f8c deps: V8: backport 47d34a3 Original commit message: Revert "[coverage] change block range to avoid ambiguity." This reverts commit 471fef0469d04d7c487f3a08e81f3d77566a2f50. Reason for revert: A more general fix incoming at https://crrev.com/c/1273095. Original change's description: > [coverage] change block range to avoid ambiguity. > > By moving the block range end to left of closing bracket, > we can avoid ambiguity where an open-ended singleton range > could be both interpreted as inside the parent range, or > next to it. > > R=<U+200B>verwaest@chromium.org > > Bug: v8:8237 > Change-Id: Ibc9412b31efe900b6d8bff0d8fa8c52ddfbf460a > Reviewed-on: https://chromium-review.googlesource.com/1254127 > Reviewed-by: Georg Neis <neis@chromium.org> > Commit-Queue: Yang Guo <yangguo@chromium.org> > Cr-Commit-Position: refs/heads/master@{#56347} TBR=yangguo@chromium.org,neis@chromium.org,verwaest@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: v8:8237 Change-Id: I39310cf3c2f06a0d98ff314740aaeefbfffc0834 Reviewed-on: https://chromium-review.googlesource.com/c/1273096 Reviewed-by: Jakob Gruber <jgruber@chromium.org> Reviewed-by: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Yang Guo <yangguo@chromium.org> Commit-Queue: Jakob Gruber <jgruber@chromium.org> Cr-Commit-Position: refs/heads/master@{#56513} Refs: v8/v8@47d34a3 PR-URL: #25429 Backport-PR-URL: #25728 Reviewed-By: Yang Guo <yangguo@chromium.org> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
nodejs · bcoe · Jan 26, 2019 · Jan 26, 2019 · c92fd82a58e3ab22dc597990ee051ed378d55573 · hashseed
commit c92fd82a58e3ab22dc597990ee051ed378d55573
diff --git a/deps/v8/BUILD.gn b/deps/v8/BUILD.gn
@@ -1564,6 +1564,8 @@ v8_source_set("v8_base") {
     "src/ast/prettyprinter.h",
     "src/ast/scopes.cc",
     "src/ast/scopes.h",
+    "src/ast/source-range-ast-visitor.cc",
+    "src/ast/source-range-ast-visitor.h",
     "src/ast/variables.cc",
     "src/ast/variables.h",
     "src/bailout-reason.cc",

diff --git a/deps/v8/gypfiles/v8.gyp b/deps/v8/gypfiles/v8.gyp
@@ -548,6 +548,8 @@
         '../src/ast/prettyprinter.h',
         '../src/ast/scopes.cc',
         '../src/ast/scopes.h',
+        '../src/ast/source-range-ast-visitor.cc',
+        '../src/ast/source-range-ast-visitor.h',
         '../src/ast/variables.cc',
         '../src/ast/variables.h',
         '../src/bailout-reason.cc',

diff --git a/deps/v8/src/ast/ast-source-ranges.h b/deps/v8/src/ast/ast-source-ranges.h
@@ -58,18 +58,24 @@ class AstNodeSourceRanges : public ZoneObject {
  public:
   virtual ~AstNodeSourceRanges() {}
   virtual SourceRange GetRange(SourceRangeKind kind) = 0;
+  virtual bool HasRange(SourceRangeKind kind) = 0;
+  virtual void RemoveContinuationRange() { UNREACHABLE(); }
 };
 
 class BinaryOperationSourceRanges final : public AstNodeSourceRanges {
  public:
   explicit BinaryOperationSourceRanges(const SourceRange& right_range)
       : right_range_(right_range) {}
 
-  SourceRange GetRange(SourceRangeKind kind) {
+  SourceRange GetRange(SourceRangeKind kind) override {
     DCHECK_EQ(kind, SourceRangeKind::kRight);
     return right_range_;
   }
 
+  bool HasRange(SourceRangeKind kind) override {
+    return kind == SourceRangeKind::kRight;
+  }
+
  private:
   SourceRange right_range_;
 };
@@ -79,11 +85,20 @@ class ContinuationSourceRanges : public AstNodeSourceRanges {
   explicit ContinuationSourceRanges(int32_t continuation_position)
       : continuation_position_(continuation_position) {}
 
-  SourceRange GetRange(SourceRangeKind kind) {
+  SourceRange GetRange(SourceRangeKind kind) override {
     DCHECK_EQ(kind, SourceRangeKind::kContinuation);
     return SourceRange::OpenEnded(continuation_position_);
   }
 
+  bool HasRange(SourceRangeKind kind) override {
+    return kind == SourceRangeKind::kContinuation;
+  }
+
+  void RemoveContinuationRange() override {
+    DCHECK(HasRange(SourceRangeKind::kContinuation));
+    continuation_position_ = kNoSourcePosition;
+  }
+
  private:
   int32_t continuation_position_;
 };
@@ -99,11 +114,15 @@ class CaseClauseSourceRanges final : public AstNodeSourceRanges {
   explicit CaseClauseSourceRanges(const SourceRange& body_range)
       : body_range_(body_range) {}
 
-  SourceRange GetRange(SourceRangeKind kind) {
+  SourceRange GetRange(SourceRangeKind kind) override {
     DCHECK_EQ(kind, SourceRangeKind::kBody);
     return body_range_;
   }
 
+  bool HasRange(SourceRangeKind kind) override {
+    return kind == SourceRangeKind::kBody;
+  }
+
  private:
   SourceRange body_range_;
 };
@@ -114,7 +133,7 @@ class ConditionalSourceRanges final : public AstNodeSourceRanges {
                                    const SourceRange& else_range)
       : then_range_(then_range), else_range_(else_range) {}
 
-  SourceRange GetRange(SourceRangeKind kind) {
+  SourceRange GetRange(SourceRangeKind kind) override {
     switch (kind) {
       case SourceRangeKind::kThen:
         return then_range_;
@@ -125,6 +144,10 @@ class ConditionalSourceRanges final : public AstNodeSourceRanges {
     }
   }
 
+  bool HasRange(SourceRangeKind kind) override {
+    return kind == SourceRangeKind::kThen || kind == SourceRangeKind::kElse;
+  }
+
  private:
   SourceRange then_range_;
   SourceRange else_range_;
@@ -136,13 +159,14 @@ class IfStatementSourceRanges final : public AstNodeSourceRanges {
                                    const SourceRange& else_range)
       : then_range_(then_range), else_range_(else_range) {}
 
-  SourceRange GetRange(SourceRangeKind kind) {
+  SourceRange GetRange(SourceRangeKind kind) override {
     switch (kind) {
       case SourceRangeKind::kElse:
         return else_range_;
       case SourceRangeKind::kThen:
         return then_range_;
       case SourceRangeKind::kContinuation: {
+        if (!has_continuation_) return SourceRange::Empty();
         const SourceRange& trailing_range =
             else_range_.IsEmpty() ? then_range_ : else_range_;
         return SourceRange::ContinuationOf(trailing_range);
@@ -152,29 +176,52 @@ class IfStatementSourceRanges final : public AstNodeSourceRanges {
     }
   }
 
+  bool HasRange(SourceRangeKind kind) override {
+    return kind == SourceRangeKind::kThen || kind == SourceRangeKind::kElse ||
+           kind == SourceRangeKind::kContinuation;
+  }
+
+  void RemoveContinuationRange() override {
+    DCHECK(HasRange(SourceRangeKind::kContinuation));
+    has_continuation_ = false;
+  }
+
  private:
   SourceRange then_range_;
   SourceRange else_range_;
+  bool has_continuation_ = true;
 };
 
 class IterationStatementSourceRanges final : public AstNodeSourceRanges {
  public:
   explicit IterationStatementSourceRanges(const SourceRange& body_range)
       : body_range_(body_range) {}
 
-  SourceRange GetRange(SourceRangeKind kind) {
+  SourceRange GetRange(SourceRangeKind kind) override {
     switch (kind) {
       case SourceRangeKind::kBody:
         return body_range_;
       case SourceRangeKind::kContinuation:
+        if (!has_continuation_) return SourceRange::Empty();
         return SourceRange::ContinuationOf(body_range_);
       default:
         UNREACHABLE();
     }
   }
 
+  bool HasRange(SourceRangeKind kind) override {
+    return kind == SourceRangeKind::kBody ||
+           kind == SourceRangeKind::kContinuation;
+  }
+
+  void RemoveContinuationRange() override {
+    DCHECK(HasRange(SourceRangeKind::kContinuation));
+    has_continuation_ = false;
+  }
+
  private:
   SourceRange body_range_;
+  bool has_continuation_ = true;
 };
 
 class JumpStatementSourceRanges final : public ContinuationSourceRanges {
@@ -199,6 +246,7 @@ class NaryOperationSourceRanges final : public AstNodeSourceRanges {
   size_t RangeCount() const { return ranges_.size(); }
 
   SourceRange GetRange(SourceRangeKind kind) { UNREACHABLE(); }
+  bool HasRange(SourceRangeKind kind) { return false; }
 
  private:
   ZoneVector<SourceRange> ranges_;
@@ -227,39 +275,63 @@ class TryCatchStatementSourceRanges final : public AstNodeSourceRanges {
   explicit TryCatchStatementSourceRanges(const SourceRange& catch_range)
       : catch_range_(catch_range) {}
 
-  SourceRange GetRange(SourceRangeKind kind) {
+  SourceRange GetRange(SourceRangeKind kind) override {
     switch (kind) {
       case SourceRangeKind::kCatch:
         return catch_range_;
       case SourceRangeKind::kContinuation:
+        if (!has_continuation_) return SourceRange::Empty();
         return SourceRange::ContinuationOf(catch_range_);
       default:
         UNREACHABLE();
     }
   }
 
+  bool HasRange(SourceRangeKind kind) override {
+    return kind == SourceRangeKind::kCatch ||
+           kind == SourceRangeKind::kContinuation;
+  }
+
+  void RemoveContinuationRange() override {
+    DCHECK(HasRange(SourceRangeKind::kContinuation));
+    has_continuation_ = false;
+  }
+
  private:
   SourceRange catch_range_;
+  bool has_continuation_ = true;
 };
 
 class TryFinallyStatementSourceRanges final : public AstNodeSourceRanges {
  public:
   explicit TryFinallyStatementSourceRanges(const SourceRange& finally_range)
       : finally_range_(finally_range) {}
 
-  SourceRange GetRange(SourceRangeKind kind) {
+  SourceRange GetRange(SourceRangeKind kind) override {
     switch (kind) {
       case SourceRangeKind::kFinally:
         return finally_range_;
       case SourceRangeKind::kContinuation:
+        if (!has_continuation_) return SourceRange::Empty();
         return SourceRange::ContinuationOf(finally_range_);
       default:
         UNREACHABLE();
     }
   }
 
+  bool HasRange(SourceRangeKind kind) override {
+    return kind == SourceRangeKind::kFinally ||
+           kind == SourceRangeKind::kContinuation;
+  }
+
+  void RemoveContinuationRange() override {
+    DCHECK(HasRange(SourceRangeKind::kContinuation));
+    has_continuation_ = false;
+  }
+
  private:
   SourceRange finally_range_;
+  bool has_continuation_ = true;
 };
 
 // Maps ast node pointers to associated source ranges. The parser creates these

diff --git a/deps/v8/src/ast/source-range-ast-visitor.cc b/deps/v8/src/ast/source-range-ast-visitor.cc
@@ -0,0 +1,68 @@
+// 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.
+
+#include "src/ast/source-range-ast-visitor.h"
+
+#include "src/ast/ast-source-ranges.h"
+
+namespace v8 {
+namespace internal {
+
+SourceRangeAstVisitor::SourceRangeAstVisitor(uintptr_t stack_limit,
+                                             Expression* root,
+                                             SourceRangeMap* source_range_map)
+    : AstTraversalVisitor(stack_limit, root),
+      source_range_map_(source_range_map) {}
+
+void SourceRangeAstVisitor::VisitBlock(Block* stmt) {
+  AstTraversalVisitor::VisitBlock(stmt);
+  ZonePtrList<Statement>* stmts = stmt->statements();
+  AstNodeSourceRanges* enclosingSourceRanges = source_range_map_->Find(stmt);
+  if (enclosingSourceRanges != nullptr) {
+    CHECK(enclosingSourceRanges->HasRange(SourceRangeKind::kContinuation));
+    MaybeRemoveLastContinuationRange(stmts);
+  }
+}
+
+void SourceRangeAstVisitor::VisitFunctionLiteral(FunctionLiteral* expr) {
+  AstTraversalVisitor::VisitFunctionLiteral(expr);
+  ZonePtrList<Statement>* stmts = expr->body();
+  MaybeRemoveLastContinuationRange(stmts);
+}
+
+bool SourceRangeAstVisitor::VisitNode(AstNode* node) {
+  AstNodeSourceRanges* range = source_range_map_->Find(node);
+
+  if (range == nullptr) return true;
+  if (!range->HasRange(SourceRangeKind::kContinuation)) return true;
+
+  // Called in pre-order. In case of conflicting continuation ranges, only the
+  // outermost range may survive.
+
+  SourceRange continuation = range->GetRange(SourceRangeKind::kContinuation);
+  if (continuation_positions_.find(continuation.start) !=
+      continuation_positions_.end()) {
+    range->RemoveContinuationRange();
+  } else {
+    continuation_positions_.emplace(continuation.start);
+  }
+
+  return true;
+}
+
+void SourceRangeAstVisitor::MaybeRemoveLastContinuationRange(
+    ZonePtrList<Statement>* statements) {
+  if (statements == nullptr || statements->is_empty()) return;
+
+  Statement* last_statement = statements->last();
+  AstNodeSourceRanges* last_range = source_range_map_->Find(last_statement);
+  if (last_range == nullptr) return;
+
+  if (last_range->HasRange(SourceRangeKind::kContinuation)) {
+    last_range->RemoveContinuationRange();
+  }
+}
+
+}  // namespace internal
+}  // namespace v8
diff --git a/deps/v8/src/ast/source-range-ast-visitor.h b/deps/v8/src/ast/source-range-ast-visitor.h
@@ -0,0 +1,49 @@
+// 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.
+
+#ifndef V8_AST_SOURCE_RANGE_AST_VISITOR_H_
+#define V8_AST_SOURCE_RANGE_AST_VISITOR_H_
+
+#include <unordered_set>
+
+#include "src/ast/ast-traversal-visitor.h"
+
+namespace v8 {
+namespace internal {
+
+class SourceRangeMap;
+
+// Post-processes generated source ranges while the AST structure still exists.
+//
+// In particular, SourceRangeAstVisitor
+//
+// 1. deduplicates continuation source ranges, only keeping the outermost one.
+// See also: https://crbug.com/v8/8539.
+//
+// 2. removes the source range associated with the final statement in a block
+// or function body if the parent itself has a source range associated with it.
+// See also: https://crbug.com/v8/8381.
+class SourceRangeAstVisitor final
+    : public AstTraversalVisitor<SourceRangeAstVisitor> {
+ public:
+  SourceRangeAstVisitor(uintptr_t stack_limit, Expression* root,
+                        SourceRangeMap* source_range_map);
+
+ private:
+  friend class AstTraversalVisitor<SourceRangeAstVisitor>;
+
+  void VisitBlock(Block* stmt);
+  void VisitFunctionLiteral(FunctionLiteral* expr);
+  bool VisitNode(AstNode* node);
+
+  void MaybeRemoveLastContinuationRange(ZonePtrList<Statement>* stmts);
+
+  SourceRangeMap* source_range_map_ = nullptr;
+  std::unordered_set<int> continuation_positions_;
+};
+
+}  // namespace internal
+}  // namespace v8
+
+#endif  // V8_AST_SOURCE_RANGE_AST_VISITOR_H_