Skip to content

Commit

Permalink
deps: V8: cherry-pick 6be2f6e26e8d
Browse files Browse the repository at this point in the history
Original commit message:

    [coverage] IncBlockCounter should not be side-effect

    Incrementing coverage counter was triggering EvalError for
    evaluateOnCallFrame when throwOnSideEffect is true.

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

    Bug: v8:10856
    Change-Id: I0552e19a3a14ff61a9cb626494fb4a21979d535e
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2384011
    Commit-Queue: Benjamin Coe <bencoe@google.com>
    Reviewed-by: Jakob Gruber <jgruber@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#69628}

Refs: v8/v8@6be2f6e

PR-URL: #35055
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
bcoe authored and richardlau committed Sep 7, 2020
1 parent 1a9ca52 commit 791a85b
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 1 deletion.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,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.15',
'v8_embedder_string': '-node.16',

##### V8 defaults for Node.js #####

Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/debug/debug-evaluate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,7 @@ bool BytecodeHasNoSideEffect(interpreter::Bytecode bytecode) {
case Bytecode::kToNumeric:
case Bytecode::kToString:
// Misc.
case Bytecode::kIncBlockCounter: // Coverage counters.
case Bytecode::kForInEnumerate:
case Bytecode::kForInPrepare:
case Bytecode::kForInContinue:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Tests side-effect-free evaluation with coverage enabled
Paused on 'debugger;'
f() returns 1
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright 2020 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.

let {session, contextGroup, Protocol} = InspectorTest.start('Tests side-effect-free evaluation with coverage enabled');

contextGroup.addScript(`
function testFunction()
{
var o = 0;
function f() { return 1; }
function g() { o = 2; return o; }
f,g;
debugger;
}
//# sourceURL=foo.js`);

// Side effect free call should not result in EvalError when coverage
// is enabled:
Protocol.Profiler.enable()
Protocol.Profiler.startPreciseCoverage({callCount: true, detailed: true})

Protocol.Debugger.enable();

Protocol.Debugger.oncePaused().then(debuggerPaused);

Protocol.Runtime.evaluate({ "expression": "setTimeout(testFunction, 0)" });

var topFrameId;

function debuggerPaused(messageObject)
{
InspectorTest.log("Paused on 'debugger;'");

topFrameId = messageObject.params.callFrames[0].callFrameId;
Protocol.Debugger.evaluateOnCallFrame({ callFrameId: topFrameId, expression: "f()", throwOnSideEffect: true}).then(evaluatedFirst);
}

function evaluatedFirst(response)
{
InspectorTest.log("f() returns " + response.result.result.value);
InspectorTest.completeTest();
}
1 change: 1 addition & 0 deletions deps/v8/test/inspector/inspector.status
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
'debugger/eval-scopes': [PASS, FAIL],
'debugger/scope-skip-variables-with-empty-name': [PASS, FAIL],
'debugger/update-call-frame-scopes': [PASS, FAIL],
'debugger/side-effect-free-coverage-enabled': [PASS, FAIL],
'debugger/side-effect-free-debug-evaluate': [PASS, FAIL],
'debugger/evaluate-on-call-frame-in-module': [PASS, FAIL],
}], # variant != default
Expand Down

0 comments on commit 791a85b

Please sign in to comment.