Skip to content

Commit

Permalink
deps: V8: cherry-pick 86991d0587a1
Browse files Browse the repository at this point in the history
Adds methods for fetching stack trace information about
enclosing function.

Refs #36042

Original commit message:

    Reland "stack-trace-api: implement getEnclosingLine/Column"

    This reverts commit 5557a63beb5a53c93e9b590eaf2933e21bcb3768.

    Reason for revert: Sheriff's mistake, failing test was previously flaking.

    Original change's description:
    > Revert "stack-trace-api: implement getEnclosingLine/Column"
    >
    > This reverts commit c48ae2d96cbfdc2216706a5e9a79ae1dce5a638b.
    >
    > Reason for revert: Breaks a profiling test:
    > https://ci.chromium.org/p/v8/builders/ci/V8%20Win32/30010
    >
    > Original change's description:
    > > stack-trace-api: implement getEnclosingLine/Column
    > >
    > > Introduces getEnclosingColumn and getEnclosingLine on CallSite
    > > so that the position can be used to lookup the original symbol
    > > for function when source maps are used.
    > >
    > > BUG=v8:11157
    > >
    > > Change-Id: I06c4c374d172d206579abb170c7b7a2bd3bb159f
    > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2547218
    > > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    > > Commit-Queue: Benjamin Coe <bencoe@google.com>
    > > Cr-Commit-Position: refs/heads/master@{#71343}
    >
    > TBR=jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com
    >
    > Change-Id: Iab5c250c1c4fbdab86971f4a7e40abc8f87cf79c
    > No-Presubmit: true
    > No-Tree-Checks: true
    > No-Try: true
    > Bug: v8:11157
    > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555384
    > Reviewed-by: Bill Budge <bbudge@chromium.org>
    > Commit-Queue: Bill Budge <bbudge@chromium.org>
    > Cr-Commit-Position: refs/heads/master@{#71345}

    TBR=bbudge@chromium.org,jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com

    # Not skipping CQ checks because this is a reland.

    Bug: v8:11157
    Change-Id: I8dba19ceb29a24594469d2cf79626f741dc4cad3
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555499
    Reviewed-by: Bill Budge <bbudge@chromium.org>
    Commit-Queue: Bill Budge <bbudge@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#71348}

Refs: v8/v8@86991d0

PR-URL: #36254
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
  • Loading branch information
bcoe authored and targos committed Jun 11, 2021
1 parent bb4900d commit acc9fad
Show file tree
Hide file tree
Showing 8 changed files with 115 additions and 1 deletion.
2 changes: 1 addition & 1 deletion common.gypi
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.64',
'v8_embedder_string': '-node.65',

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

Expand Down
16 changes: 16 additions & 0 deletions deps/v8/src/builtins/builtins-callsite.cc
Expand Up @@ -53,6 +53,22 @@ BUILTIN(CallSitePrototypeGetColumnNumber) {
return PositiveNumberOrNull(it.Frame()->GetColumnNumber(), isolate);
}

BUILTIN(CallSitePrototypeGetEnclosingColumnNumber) {
HandleScope scope(isolate);
CHECK_CALLSITE(recv, "getEnclosingColumnNumber");
FrameArrayIterator it(isolate, GetFrameArray(isolate, recv),
GetFrameIndex(isolate, recv));
return PositiveNumberOrNull(it.Frame()->GetEnclosingColumnNumber(), isolate);
}

BUILTIN(CallSitePrototypeGetEnclosingLineNumber) {
HandleScope scope(isolate);
CHECK_CALLSITE(recv, "getEnclosingLineNumber");
FrameArrayIterator it(isolate, GetFrameArray(isolate, recv),
GetFrameIndex(isolate, recv));
return PositiveNumberOrNull(it.Frame()->GetEnclosingLineNumber(), isolate);
}

BUILTIN(CallSitePrototypeGetEvalOrigin) {
HandleScope scope(isolate);
CHECK_CALLSITE(recv, "getEvalOrigin");
Expand Down
2 changes: 2 additions & 0 deletions deps/v8/src/builtins/builtins-definitions.h
Expand Up @@ -376,6 +376,8 @@ namespace internal {
\
/* CallSite */ \
CPP(CallSitePrototypeGetColumnNumber) \
CPP(CallSitePrototypeGetEnclosingColumnNumber) \
CPP(CallSitePrototypeGetEnclosingLineNumber) \
CPP(CallSitePrototypeGetEvalOrigin) \
CPP(CallSitePrototypeGetFileName) \
CPP(CallSitePrototypeGetFunction) \
Expand Down
46 changes: 46 additions & 0 deletions deps/v8/src/execution/messages.cc
Expand Up @@ -513,6 +513,26 @@ int JSStackFrame::GetColumnNumber() {
return kNone;
}

int JSStackFrame::GetEnclosingLineNumber() {
if (HasScript()) {
Handle<SharedFunctionInfo> shared = handle(function_->shared(), isolate_);
return Script::GetLineNumber(GetScript(),
shared->function_token_position()) + 1;
} else {
return kNone;
}
}

int JSStackFrame::GetEnclosingColumnNumber() {
if (HasScript()) {
Handle<SharedFunctionInfo> shared = handle(function_->shared(), isolate_);
return Script::GetColumnNumber(GetScript(),
shared->function_token_position()) + 1;
} else {
return kNone;
}
}

int JSStackFrame::GetPromiseIndex() const {
return is_promise_all_ ? offset_ : kNone;
}
Expand Down Expand Up @@ -601,6 +621,12 @@ int WasmStackFrame::GetPosition() const {

int WasmStackFrame::GetColumnNumber() { return GetModuleOffset(); }

int WasmStackFrame::GetEnclosingColumnNumber() {
const int function_offset =
GetWasmFunctionOffset(wasm_instance_->module(), wasm_func_index_);
return function_offset;
}

int WasmStackFrame::GetModuleOffset() const {
const int function_offset =
GetWasmFunctionOffset(wasm_instance_->module(), wasm_func_index_);
Expand Down Expand Up @@ -669,6 +695,26 @@ int AsmJsWasmStackFrame::GetColumnNumber() {
return Script::GetColumnNumber(script, GetPosition()) + 1;
}

int AsmJsWasmStackFrame::GetEnclosingLineNumber() {
DCHECK_LE(0, GetPosition());
Handle<Script> script(wasm_instance_->module_object().script(), isolate_);
DCHECK(script->IsUserJavaScript());
int byte_offset = GetSourcePosition(wasm_instance_->module(),
wasm_func_index_, 0,
is_at_number_conversion_);
return Script::GetLineNumber(script, byte_offset) + 1;
}

int AsmJsWasmStackFrame::GetEnclosingColumnNumber() {
DCHECK_LE(0, GetPosition());
Handle<Script> script(wasm_instance_->module_object().script(), isolate_);
DCHECK(script->IsUserJavaScript());
int byte_offset = GetSourcePosition(wasm_instance_->module(),
wasm_func_index_, 0,
is_at_number_conversion_);
return Script::GetColumnNumber(script, byte_offset) + 1;
}

FrameArrayIterator::FrameArrayIterator(Isolate* isolate,
Handle<FrameArray> array, int frame_ix)
: isolate_(isolate), array_(array), frame_ix_(frame_ix) {}
Expand Down
11 changes: 11 additions & 0 deletions deps/v8/src/execution/messages.h
Expand Up @@ -86,6 +86,9 @@ class StackFrameBase {
// Return 0-based Wasm function index. Returns -1 for non-Wasm frames.
virtual int GetWasmFunctionIndex();

virtual int GetEnclosingColumnNumber() = 0;
virtual int GetEnclosingLineNumber() = 0;

// Returns index for Promise.all() async frames, or -1 for other frames.
virtual int GetPromiseIndex() const = 0;

Expand Down Expand Up @@ -130,6 +133,9 @@ class JSStackFrame : public StackFrameBase {
int GetLineNumber() override;
int GetColumnNumber() override;

int GetEnclosingColumnNumber() override;
int GetEnclosingLineNumber() override;

int GetPromiseIndex() const override;

bool IsNative() override;
Expand Down Expand Up @@ -178,6 +184,8 @@ class WasmStackFrame : public StackFrameBase {
int GetPosition() const override;
int GetLineNumber() override { return 0; }
int GetColumnNumber() override;
int GetEnclosingColumnNumber() override;
int GetEnclosingLineNumber() override { return 0; }
int GetWasmFunctionIndex() override { return wasm_func_index_; }

int GetPromiseIndex() const override { return GetPosition(); }
Expand Down Expand Up @@ -225,6 +233,9 @@ class AsmJsWasmStackFrame : public WasmStackFrame {
int GetLineNumber() override;
int GetColumnNumber() override;

int GetEnclosingColumnNumber() override;
int GetEnclosingLineNumber() override;

private:
friend class FrameArrayIterator;
AsmJsWasmStackFrame() = default;
Expand Down
4 changes: 4 additions & 0 deletions deps/v8/src/init/bootstrapper.cc
Expand Up @@ -4079,6 +4079,10 @@ void Genesis::InitializeCallSiteBuiltins() {

FunctionInfo infos[] = {
{"getColumnNumber", Builtins::kCallSitePrototypeGetColumnNumber},
{"getEnclosingColumnNumber",
Builtins::kCallSitePrototypeGetEnclosingColumnNumber},
{"getEnclosingLineNumber",
Builtins::kCallSitePrototypeGetEnclosingLineNumber},
{"getEvalOrigin", Builtins::kCallSitePrototypeGetEvalOrigin},
{"getFileName", Builtins::kCallSitePrototypeGetFileName},
{"getFunction", Builtins::kCallSitePrototypeGetFunction},
Expand Down
20 changes: 20 additions & 0 deletions deps/v8/test/mjsunit/stack-traces.js
Expand Up @@ -439,3 +439,23 @@ var constructor = new Error().stack[0].constructor;
assertThrows(() => constructor.call());
assertThrows(() => constructor.call(
null, {}, () => undefined, {valueOf() { return 0 }}, false));

// Test stack frames populated with line/column information for both call site
// and enclosing function:
Error.prepareStackTrace = function(e, frames) {
assertMatches(/stack-traces\.js/, frames[0].getFileName());
assertEquals(3, frames[0].getEnclosingColumnNumber());
assertEquals(11, frames[0].getColumnNumber());
assertTrue(frames[0].getEnclosingLineNumber() < frames[0].getLineNumber());
}
try {
function a() {
b();
}
function b() {
throw Error('hello world');
}
a();
} catch (err) {
err.stack;
}
15 changes: 15 additions & 0 deletions deps/v8/test/mjsunit/wasm/asm-wasm-stack.js
Expand Up @@ -154,3 +154,18 @@ function generateOverflowWasmFromAsmJs() {
['f', 135, 12] // --
]);
})();

(function EnclosingFunctionOffsets() {
const fun = generateWasmFromAsmJs(this, {throwFunc: throwException});
assertTrue(%IsWasmCode(fun));
let e = null;
try {
fun(0);
} catch (ex) {
e = ex;
}
assertEquals(68, e.stack[2].getLineNumber());
assertEquals(15, e.stack[2].getColumnNumber());
assertEquals(65, e.stack[2].getEnclosingLineNumber());
assertEquals(3, e.stack[2].getEnclosingColumnNumber());
})();

0 comments on commit acc9fad

Please sign in to comment.