Skip to content

Commit

Permalink
deps: v8, backport coverage fixes
Browse files Browse the repository at this point in the history
Pulls in patches from V8 (52e2b5, 512175, aac2f8, 9365d09, 2d08967)
which address a variety of coverage bugs, bringing Node v10 coverage
to parity with Node v11.

  Original commit message:

    [coverage] Extend SourceRangeAstVisitor for throw statements

    The SourceRangeAstVisitor has custom logic for blocks ending with a
    statement that has a continuation range. In these cases, the trailing
    continuation is removed which makes the reported coverage ranges a bit
    nicer.

    throw Error('foo') consists of an ExpressionStatement, with a
    Throw expression stored within the statement. The source range itself
    is stored with the Throw, not the statement.

    We now properly extract the correct AST node for trailing throw
    statements.

  R=jgruber@chromium.org, neis@chromium.org, yangguo@chromium.org

  Bug: v8:8691
  Change-Id: Ibcbab79fbe54719a8993045040349c863b139011
  Reviewed-on: https://chromium-review.googlesource.com/c/1480632
  Commit-Queue: Georg Neis <neis@chromium.org>
  Reviewed-by: Georg Neis <neis@chromium.org>
  Reviewed-by: Jakob Gruber <jgruber@chromium.org>
  Cr-Commit-Position: refs/heads/master@{#59936}

  Refs: v8/v8@2d08967

  Original commit message:

    [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:

    [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

  Original commit message:

    [ast] Introduce ZonePtrList<T> typedef for ZoneList<T*>.

    This is a preliminary step before changing the way we store zone pointers
    in the zones.

    Bug: v8:7903, v8:7754
    Change-Id: I1e1af1823766c888ee0f8fe190f205f5b7e21973
    Reviewed-on: https://chromium-review.googlesource.com/1118887
    Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
    Reviewed-by: Marja Hölttä <marja@chromium.org>
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Commit-Queue: Igor Sheludko <ishell@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#54193}

  Refs: v8/v8@512175a

  Original commit message:

    [explicit isolates] Replace every Handle(T*) in parsing/

    Replace all but one Handle<T*>(T*) calls with ones that explicitly pass
    in an Isolate.

    Requires plumbing Isolate* through several Parser functions which
    previously avoided it because of worries about accessing the heap off
    the main thread. In all off-main-thread cases, isolate will be nullptr
    and every such function asserts with:
    DCHECK_EQ(parsing_on_main_thread_, isolate != nullptr);

    Also deletes unused function ParseInfo::ReopenHandlesInNewHandleScope.

    Bug: v8:7786
    Change-Id: I3dd9c49dcde49fdbcb684ba73f47a30d00fc495e
    Reviewed-on: https://chromium-review.googlesource.com/1087272
    Commit-Queue: Dan Elphick <delphick@chromium.org>
    Reviewed-by: Toon Verwaest <verwaest@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#53820}

  Refs: v8/v8@52e2b5a

PR-URL: #26579
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
  • Loading branch information
bcoe authored and BethGriggs committed Apr 8, 2019
1 parent 2643801 commit 23ea7ee
Show file tree
Hide file tree
Showing 35 changed files with 964 additions and 605 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.51',
'v8_embedder_string': '-node.52',

# Enable disassembler for `--print-code` v8 options
'v8_enable_disassembler': 1,
Expand Down
2 changes: 1 addition & 1 deletion deps/v8/AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ Andrew Paprocki <andrew@ishiboo.com>
Andrei Kashcha <anvaka@gmail.com>
Anna Henningsen <anna@addaleax.net>
Bangfu Tao <bangfu.tao@samsung.com>
Ben Coe <ben@npmjs.com>
Ben Coe <bencoe@gmail.com>
Ben Newman <ben@meteor.com>
Ben Noordhuis <info@bnoordhuis.nl>
Benjamin Tan <demoneaux@gmail.com>
Expand Down
2 changes: 2 additions & 0 deletions deps/v8/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1496,6 +1496,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",
Expand Down
2 changes: 2 additions & 0 deletions deps/v8/gypfiles/v8.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,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',
Expand Down
101 changes: 89 additions & 12 deletions deps/v8/src/ast/ast-source-ranges.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
DCHECK_EQ(kind, SourceRangeKind::kRight);
SourceRange GetRange(SourceRangeKind kind) override {
DCHECK(HasRange(kind));
return right_range_;
}

bool HasRange(SourceRangeKind kind) override {
return kind == SourceRangeKind::kRight;
}

private:
SourceRange right_range_;
};
Expand All @@ -79,11 +85,20 @@ class ContinuationSourceRanges : public AstNodeSourceRanges {
explicit ContinuationSourceRanges(int32_t continuation_position)
: continuation_position_(continuation_position) {}

SourceRange GetRange(SourceRangeKind kind) {
DCHECK_EQ(kind, SourceRangeKind::kContinuation);
SourceRange GetRange(SourceRangeKind kind) override {
DCHECK(HasRange(kind));
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_;
};
Expand All @@ -99,11 +114,15 @@ class CaseClauseSourceRanges final : public AstNodeSourceRanges {
explicit CaseClauseSourceRanges(const SourceRange& body_range)
: body_range_(body_range) {}

SourceRange GetRange(SourceRangeKind kind) {
DCHECK_EQ(kind, SourceRangeKind::kBody);
SourceRange GetRange(SourceRangeKind kind) override {
DCHECK(HasRange(kind));
return body_range_;
}

bool HasRange(SourceRangeKind kind) override {
return kind == SourceRangeKind::kBody;
}

private:
SourceRange body_range_;
};
Expand All @@ -114,7 +133,8 @@ 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 {
DCHECK(HasRange(kind));
switch (kind) {
case SourceRangeKind::kThen:
return then_range_;
Expand All @@ -125,6 +145,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_;
Expand All @@ -136,13 +160,15 @@ 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 {
DCHECK(HasRange(kind));
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);
Expand All @@ -152,29 +178,53 @@ 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 {
DCHECK(HasRange(kind));
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 {
Expand All @@ -198,7 +248,8 @@ class NaryOperationSourceRanges final : public AstNodeSourceRanges {
void AddRange(const SourceRange& range) { ranges_.push_back(range); }
size_t RangeCount() const { return ranges_.size(); }

SourceRange GetRange(SourceRangeKind kind) { UNREACHABLE(); }
SourceRange GetRange(SourceRangeKind kind) override { UNREACHABLE(); }
bool HasRange(SourceRangeKind kind) override { return false; }

private:
ZoneVector<SourceRange> ranges_;
Expand Down Expand Up @@ -227,39 +278,65 @@ class TryCatchStatementSourceRanges final : public AstNodeSourceRanges {
explicit TryCatchStatementSourceRanges(const SourceRange& catch_range)
: catch_range_(catch_range) {}

SourceRange GetRange(SourceRangeKind kind) {
SourceRange GetRange(SourceRangeKind kind) override {
DCHECK(HasRange(kind));
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 {
DCHECK(HasRange(kind));
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
Expand Down
22 changes: 11 additions & 11 deletions deps/v8/src/ast/ast-traversal-visitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class AstTraversalVisitor : public AstVisitor<Subclass> {

// Iteration left-to-right.
void VisitDeclarations(Declaration::List* declarations);
void VisitStatements(ZoneList<Statement*>* statements);
void VisitStatements(ZonePtrList<Statement>* statements);

// Individual nodes
#define DECLARE_VISIT(type) void Visit##type(type* node);
Expand Down Expand Up @@ -112,7 +112,7 @@ void AstTraversalVisitor<Subclass>::VisitDeclarations(

template <class Subclass>
void AstTraversalVisitor<Subclass>::VisitStatements(
ZoneList<Statement*>* stmts) {
ZonePtrList<Statement>* stmts) {
for (int i = 0; i < stmts->length(); ++i) {
Statement* stmt = stmts->at(i);
RECURSE(Visit(stmt));
Expand Down Expand Up @@ -198,14 +198,14 @@ void AstTraversalVisitor<Subclass>::VisitSwitchStatement(
PROCESS_NODE(stmt);
RECURSE(Visit(stmt->tag()));

ZoneList<CaseClause*>* clauses = stmt->cases();
ZonePtrList<CaseClause>* clauses = stmt->cases();
for (int i = 0; i < clauses->length(); ++i) {
CaseClause* clause = clauses->at(i);
if (!clause->is_default()) {
Expression* label = clause->label();
RECURSE(Visit(label));
}
ZoneList<Statement*>* stmts = clause->statements();
ZonePtrList<Statement>* stmts = clause->statements();
RECURSE(VisitStatements(stmts));
}
}
Expand Down Expand Up @@ -330,7 +330,7 @@ void AstTraversalVisitor<Subclass>::VisitRegExpLiteral(RegExpLiteral* expr) {
template <class Subclass>
void AstTraversalVisitor<Subclass>::VisitObjectLiteral(ObjectLiteral* expr) {
PROCESS_EXPRESSION(expr);
ZoneList<ObjectLiteralProperty*>* props = expr->properties();
ZonePtrList<ObjectLiteralProperty>* props = expr->properties();
for (int i = 0; i < props->length(); ++i) {
ObjectLiteralProperty* prop = props->at(i);
RECURSE_EXPRESSION(Visit(prop->key()));
Expand All @@ -341,7 +341,7 @@ void AstTraversalVisitor<Subclass>::VisitObjectLiteral(ObjectLiteral* expr) {
template <class Subclass>
void AstTraversalVisitor<Subclass>::VisitArrayLiteral(ArrayLiteral* expr) {
PROCESS_EXPRESSION(expr);
ZoneList<Expression*>* values = expr->values();
ZonePtrList<Expression>* values = expr->values();
for (int i = 0; i < values->length(); ++i) {
Expression* value = values->at(i);
RECURSE_EXPRESSION(Visit(value));
Expand Down Expand Up @@ -404,7 +404,7 @@ template <class Subclass>
void AstTraversalVisitor<Subclass>::VisitCall(Call* expr) {
PROCESS_EXPRESSION(expr);
RECURSE_EXPRESSION(Visit(expr->expression()));
ZoneList<Expression*>* args = expr->arguments();
ZonePtrList<Expression>* args = expr->arguments();
for (int i = 0; i < args->length(); ++i) {
Expression* arg = args->at(i);
RECURSE_EXPRESSION(Visit(arg));
Expand All @@ -415,7 +415,7 @@ template <class Subclass>
void AstTraversalVisitor<Subclass>::VisitCallNew(CallNew* expr) {
PROCESS_EXPRESSION(expr);
RECURSE_EXPRESSION(Visit(expr->expression()));
ZoneList<Expression*>* args = expr->arguments();
ZonePtrList<Expression>* args = expr->arguments();
for (int i = 0; i < args->length(); ++i) {
Expression* arg = args->at(i);
RECURSE_EXPRESSION(Visit(arg));
Expand All @@ -425,7 +425,7 @@ void AstTraversalVisitor<Subclass>::VisitCallNew(CallNew* expr) {
template <class Subclass>
void AstTraversalVisitor<Subclass>::VisitCallRuntime(CallRuntime* expr) {
PROCESS_EXPRESSION(expr);
ZoneList<Expression*>* args = expr->arguments();
ZonePtrList<Expression>* args = expr->arguments();
for (int i = 0; i < args->length(); ++i) {
Expression* arg = args->at(i);
RECURSE_EXPRESSION(Visit(arg));
Expand Down Expand Up @@ -487,7 +487,7 @@ void AstTraversalVisitor<Subclass>::VisitClassLiteral(ClassLiteral* expr) {
if (expr->instance_fields_initializer_function() != nullptr) {
RECURSE_EXPRESSION(Visit(expr->instance_fields_initializer_function()));
}
ZoneList<ClassLiteralProperty*>* props = expr->properties();
ZonePtrList<ClassLiteral::Property>* props = expr->properties();
for (int i = 0; i < props->length(); ++i) {
ClassLiteralProperty* prop = props->at(i);
if (!prop->key()->IsLiteral()) {
Expand All @@ -501,7 +501,7 @@ template <class Subclass>
void AstTraversalVisitor<Subclass>::VisitInitializeClassFieldsStatement(
InitializeClassFieldsStatement* stmt) {
PROCESS_NODE(stmt);
ZoneList<ClassLiteralProperty*>* props = stmt->fields();
ZonePtrList<ClassLiteral::Property>* props = stmt->fields();
for (int i = 0; i < props->length(); ++i) {
ClassLiteralProperty* prop = props->at(i);
if (!prop->key()->IsLiteral()) {
Expand Down
4 changes: 2 additions & 2 deletions deps/v8/src/ast/ast.cc
Original file line number Diff line number Diff line change
Expand Up @@ -831,7 +831,7 @@ Call::CallType Call::GetCallType() const {
return OTHER_CALL;
}

CaseClause::CaseClause(Expression* label, ZoneList<Statement*>* statements)
CaseClause::CaseClause(Expression* label, ZonePtrList<Statement>* statements)
: label_(label), statements_(statements) {}

bool Literal::IsPropertyName() const {
Expand Down Expand Up @@ -954,7 +954,7 @@ const char* CallRuntime::debug_name() {
case k##NodeType: \
return static_cast<const NodeType*>(this)->labels();

ZoneList<const AstRawString*>* BreakableStatement::labels() const {
ZonePtrList<const AstRawString>* BreakableStatement::labels() const {
switch (node_type()) {
BREAKABLE_NODE_LIST(RETURN_LABELS)
ITERATION_NODE_LIST(RETURN_LABELS)
Expand Down
Loading

0 comments on commit 23ea7ee

Please sign in to comment.