Skip to content

Commit

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

    [debug] Revert to old line number behavior for new Function()

    Reverting https://chromium-review.googlesource.com/c/v8/v8/+/1741660

    This fixed one bug but caused a lot of others and on balance I think
    reverting it is the lesser evil.

    This also fixed generator-relocation.js because
    (function*(){}).constructor is the function constructor and we try to
    set a breakpoint on line 3.

    Bug: chromium:109362, chromium:1028689
    Fixes: v8:9721
    Change-Id: I1bfe6ec57ce77ea7292df91266311f5c0194947e
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1940259
    Commit-Queue: Peter Marshall <petermarshall@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#65232}

Refs: v8/v8@dc3a90b

Revert "assert: fix line number calculation after V8 upgrade"

This reverts commit 5981fb7.

Fixes: #32688

PR-URL: #32795
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
targos committed Apr 28, 2020
1 parent 947ddec commit ea85768
Show file tree
Hide file tree
Showing 7 changed files with 14 additions and 31 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,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.30',
'v8_embedder_string': '-node.31',

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

Expand Down
11 changes: 0 additions & 11 deletions deps/v8/src/builtins/builtins-function.cc
Original file line number Diff line number Diff line change
Expand Up @@ -93,17 +93,6 @@ MaybeHandle<Object> CreateDynamicFunction(Isolate* isolate,
function->shared().set_name_should_print_as_anonymous(true);
}

// The spec says that we have to wrap code created via the function
// constructor in e.g. 'function anonymous(' as above, including with extra
// line breaks. Ths is confusing when reporting stack traces from the eval'd
// code as the line number of the error is always reported with 2 extra line
// breaks e.g. line 1 is reported as line 3. We fix this up here by setting
// line_offset which is read by stack trace code.
Handle<Script> script(Script::cast(function->shared().script()), isolate);
if (script->line_offset() == 0) {
script->set_line_offset(-2);
}

// If new.target is equal to target then the function created
// is already correctly setup and nothing else should be done
// here. But if new.target is not equal to target then we are
Expand Down
10 changes: 5 additions & 5 deletions deps/v8/test/cctest/test-api-stack-traces.cc
Original file line number Diff line number Diff line change
Expand Up @@ -250,31 +250,31 @@ static void AnalyzeStackInNativeCode(
v8::Local<v8::StackTrace> stackTrace = v8::StackTrace::CurrentStackTrace(
args.GetIsolate(), 5, v8::StackTrace::kOverview);
CHECK_EQ(3, stackTrace->GetFrameCount());
checkStackFrame(nullptr, "function.name", 1, 1, true, false,
checkStackFrame(nullptr, "function.name", 3, 1, true, false,
stackTrace->GetFrame(isolate, 0));
} else if (testGroup == kDisplayName) {
v8::Local<v8::StackTrace> stackTrace = v8::StackTrace::CurrentStackTrace(
args.GetIsolate(), 5, v8::StackTrace::kOverview);
CHECK_EQ(3, stackTrace->GetFrameCount());
checkStackFrame(nullptr, "function.displayName", 1, 1, true, false,
checkStackFrame(nullptr, "function.displayName", 3, 1, true, false,
stackTrace->GetFrame(isolate, 0));
} else if (testGroup == kFunctionNameAndDisplayName) {
v8::Local<v8::StackTrace> stackTrace = v8::StackTrace::CurrentStackTrace(
args.GetIsolate(), 5, v8::StackTrace::kOverview);
CHECK_EQ(3, stackTrace->GetFrameCount());
checkStackFrame(nullptr, "function.displayName", 1, 1, true, false,
checkStackFrame(nullptr, "function.displayName", 3, 1, true, false,
stackTrace->GetFrame(isolate, 0));
} else if (testGroup == kDisplayNameIsNotString) {
v8::Local<v8::StackTrace> stackTrace = v8::StackTrace::CurrentStackTrace(
args.GetIsolate(), 5, v8::StackTrace::kOverview);
CHECK_EQ(3, stackTrace->GetFrameCount());
checkStackFrame(nullptr, "function.name", 1, 1, true, false,
checkStackFrame(nullptr, "function.name", 3, 1, true, false,
stackTrace->GetFrame(isolate, 0));
} else if (testGroup == kFunctionNameIsNotString) {
v8::Local<v8::StackTrace> stackTrace = v8::StackTrace::CurrentStackTrace(
args.GetIsolate(), 5, v8::StackTrace::kOverview);
CHECK_EQ(3, stackTrace->GetFrameCount());
checkStackFrame(nullptr, "", 1, 1, true, false,
checkStackFrame(nullptr, "", 3, 1, true, false,
stackTrace->GetFrame(isolate, 0));
}
}
Expand Down
3 changes: 0 additions & 3 deletions deps/v8/test/debugger/debugger.status
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@
# not work, but we expect it to not crash.
'debug/debug-step-turbofan': [PASS, FAIL],

# BUG (v8:9721)
'debug/es6/generators-relocation': [FAIL],

# Issue 3641: The new 'then' semantics suppress some exceptions.
# These tests may be changed or removed when 'chain' is deprecated.
'debug/es6/debug-promises/reject-with-throw-in-reject': [FAIL],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,19 @@ Tests that Runtime.evaluate has the correct error line number for 'new Function(
columnNumber : 3
exception : {
className : TypeError
description : TypeError: 0 is not a function at eval (eval at <anonymous> (:1:1), <anonymous>:1:4) at <anonymous>:1:22
description : TypeError: 0 is not a function at eval (eval at <anonymous> (:1:1), <anonymous>:3:4) at <anonymous>:1:22
objectId : <objectId>
subtype : error
type : object
}
exceptionId : <exceptionId>
lineNumber : 0
lineNumber : 2
scriptId : <scriptId>
text : Uncaught
}
result : {
className : TypeError
description : TypeError: 0 is not a function at eval (eval at <anonymous> (:1:1), <anonymous>:1:4) at <anonymous>:1:22
description : TypeError: 0 is not a function at eval (eval at <anonymous> (:1:1), <anonymous>:3:4) at <anonymous>:1:22
objectId : <objectId>
subtype : error
type : object
Expand Down
8 changes: 4 additions & 4 deletions deps/v8/test/mjsunit/regress/regress-crbug-109362.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 1 addition & 4 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ function getErrMessage(message, fn) {
const call = err.stack[0];

const filename = call.getFileName();
let line = call.getLineNumber() - 1;
const line = call.getLineNumber() - 1;
let column = call.getColumnNumber() - 1;
let identifier;
let code;
Expand All @@ -297,9 +297,6 @@ function getErrMessage(message, fn) {
return message;
}
code = String(fn);
// For functions created with the Function constructor, V8 does not count
// the lines containing the function header.
line += 2;
identifier = `${code}${line}${column}`;
}

Expand Down

0 comments on commit ea85768

Please sign in to comment.