Commit 0ea2a34b authored by Simon Zünd's avatar Simon Zünd Committed by Commit Bot

[stack-trace] Add the last remaining fields to StackFrameInfo

This CL adds the last fields needed to stringify all stack frames from
StackFrameInfo objects instead of accessing the FrameArray directly.

Drive-by-change: The factory method that creates StackFrameInfo is
refactored to:
  1. collect all values for the fields
  2. allocate a StackFrameInfo
  3. set all the values on the allocated info object.

This fixes undefined evaluation order bugs that GCMole failed to spot,
as well as make one factory method unnecessary.

Note: More precise types on the fields that are currently "Object"
will happen in a follow up CL.

Bug: v8:8742
Change-Id: Ia8c55fc128434f27aadeba78e8483d90296abe3a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1641242Reviewed-by: 's avatarMichael Starzinger <mstarzinger@chromium.org>
Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62010}
parent 8d5f6f96
......@@ -880,6 +880,9 @@ extern class StackFrameInfo extends Struct {
script_name: Object;
script_name_or_source_url: Object;
function_name: Object;
method_name: Object;
type_name: Object;
eval_origin: Object;
wasm_module_name: Object;
flag: Smi;
}
......
......@@ -303,7 +303,7 @@ MaybeHandle<String> FormatEvalOrigin(Isolate* isolate, Handle<Script> script) {
} // namespace
Handle<Object> StackFrameBase::GetEvalOrigin() {
if (!HasScript()) return isolate_->factory()->undefined_value();
if (!HasScript() || !IsEval()) return isolate_->factory()->undefined_value();
return FormatEvalOrigin(isolate_, GetScript()).ToHandleChecked();
}
......
......@@ -3641,62 +3641,68 @@ Handle<StackTraceFrame> Factory::NewStackTraceFrame(
return frame;
}
Handle<StackFrameInfo> Factory::NewStackFrameInfo() {
Handle<StackFrameInfo> stack_frame_info = Handle<StackFrameInfo>::cast(
NewStruct(STACK_FRAME_INFO_TYPE, AllocationType::kYoung));
stack_frame_info->set_line_number(0);
stack_frame_info->set_column_number(0);
stack_frame_info->set_script_id(0);
stack_frame_info->set_promise_all_index(-1);
stack_frame_info->set_script_name(*null_value());
stack_frame_info->set_script_name_or_source_url(*null_value());
stack_frame_info->set_function_name(*null_value());
stack_frame_info->set_flag(0);
return stack_frame_info;
}
Handle<StackFrameInfo> Factory::NewStackFrameInfo(
Handle<FrameArray> frame_array, int index) {
FrameArrayIterator it(isolate(), frame_array, index);
DCHECK(it.HasFrame());
Handle<StackFrameInfo> info = NewStackFrameInfo();
info->set_flag(0);
const bool is_wasm = frame_array->IsAnyWasmFrame(index);
info->set_is_wasm(is_wasm);
// Line numbers are 1-based, for Wasm we need to adjust.
int line = it.Frame()->GetLineNumber();
if (is_wasm && line >= 0) line++;
info->set_line_number(line);
// Column numbers are 1-based. For Wasm we use the position
// as the iterator does not currently provide a column number.
const int column =
is_wasm ? it.Frame()->GetPosition() + 1 : it.Frame()->GetColumnNumber();
info->set_column_number(column);
info->set_script_id(it.Frame()->GetScriptId());
info->set_script_name(*it.Frame()->GetFileName());
info->set_script_name_or_source_url(*it.Frame()->GetScriptNameOrSourceUrl());
const int script_id = it.Frame()->GetScriptId();
Handle<Object> script_name = it.Frame()->GetFileName();
Handle<Object> script_or_url = it.Frame()->GetScriptNameOrSourceUrl();
// TODO(szuend): Adjust this, once it is decided what name to use in both
// "simple" and "detailed" stack traces. This code is for
// backwards compatibility to fullfill test expectations.
auto function_name = it.Frame()->GetFunctionName();
bool is_user_java_script = false;
if (!is_wasm) {
Handle<Object> function = it.Frame()->GetFunction();
if (function->IsJSFunction()) {
Handle<JSFunction> fun = Handle<JSFunction>::cast(function);
function_name = JSFunction::GetDebugName(fun);
const bool is_user_java_script = fun->shared().IsUserJavaScript();
info->set_is_user_java_script(is_user_java_script);
is_user_java_script = fun->shared().IsUserJavaScript();
}
}
Handle<Object> method_name = it.Frame()->GetMethodName();
Handle<Object> type_name = it.Frame()->GetTypeName();
Handle<Object> eval_origin = it.Frame()->GetEvalOrigin();
Handle<Object> wasm_module_name = it.Frame()->GetWasmModuleName();
Handle<StackFrameInfo> info = Handle<StackFrameInfo>::cast(
NewStruct(STACK_FRAME_INFO_TYPE, AllocationType::kYoung));
DisallowHeapAllocation no_gc;
info->set_flag(0);
info->set_is_wasm(is_wasm);
info->set_is_asmjs_wasm(frame_array->IsAsmJsWasmFrame(index));
info->set_is_user_java_script(is_user_java_script);
info->set_line_number(line);
info->set_column_number(column);
info->set_script_id(script_id);
info->set_script_name(*script_name);
info->set_script_name_or_source_url(*script_or_url);
info->set_function_name(*function_name);
info->set_wasm_module_name(*it.Frame()->GetWasmModuleName());
info->set_method_name(*method_name);
info->set_type_name(*type_name);
info->set_eval_origin(*eval_origin);
info->set_wasm_module_name(*wasm_module_name);
info->set_is_eval(it.Frame()->IsEval());
info->set_is_constructor(it.Frame()->IsConstructor());
info->set_is_toplevel(it.Frame()->IsToplevel());
......
......@@ -461,7 +461,6 @@ class V8_EXPORT_PRIVATE Factory {
Handle<BreakPoint> NewBreakPoint(int id, Handle<String> condition);
Handle<StackTraceFrame> NewStackTraceFrame(Handle<FrameArray> frame_array,
int index);
Handle<StackFrameInfo> NewStackFrameInfo();
Handle<StackFrameInfo> NewStackFrameInfo(Handle<FrameArray> frame_array,
int index);
Handle<SourcePositionTableWithFrameCache>
......
......@@ -32,11 +32,15 @@ ACCESSORS(StackFrameInfo, script_name, Object, kScriptNameOffset)
ACCESSORS(StackFrameInfo, script_name_or_source_url, Object,
kScriptNameOrSourceUrlOffset)
ACCESSORS(StackFrameInfo, function_name, Object, kFunctionNameOffset)
ACCESSORS(StackFrameInfo, method_name, Object, kMethodNameOffset)
ACCESSORS(StackFrameInfo, type_name, Object, kTypeNameOffset)
ACCESSORS(StackFrameInfo, eval_origin, Object, kEvalOriginOffset)
ACCESSORS(StackFrameInfo, wasm_module_name, Object, kWasmModuleNameOffset)
SMI_ACCESSORS(StackFrameInfo, flag, kFlagOffset)
BOOL_ACCESSORS(StackFrameInfo, flag, is_eval, kIsEvalBit)
BOOL_ACCESSORS(StackFrameInfo, flag, is_constructor, kIsConstructorBit)
BOOL_ACCESSORS(StackFrameInfo, flag, is_wasm, kIsWasmBit)
BOOL_ACCESSORS(StackFrameInfo, flag, is_asmjs_wasm, kIsAsmJsWasmBit)
BOOL_ACCESSORS(StackFrameInfo, flag, is_user_java_script, kIsUserJavaScriptBit)
BOOL_ACCESSORS(StackFrameInfo, flag, is_toplevel, kIsToplevelBit)
BOOL_ACCESSORS(StackFrameInfo, flag, is_async, kIsAsyncBit)
......
......@@ -9,81 +9,121 @@
namespace v8 {
namespace internal {
// static
int StackTraceFrame::GetLineNumber(Handle<StackTraceFrame> frame) {
int line = GetFrameInfo(frame)->line_number();
return line != StackFrameBase::kNone ? line : Message::kNoLineNumberInfo;
}
// static
int StackTraceFrame::GetColumnNumber(Handle<StackTraceFrame> frame) {
int column = GetFrameInfo(frame)->column_number();
return column != StackFrameBase::kNone ? column : Message::kNoColumnInfo;
}
// static
int StackTraceFrame::GetScriptId(Handle<StackTraceFrame> frame) {
int id = GetFrameInfo(frame)->script_id();
return id != StackFrameBase::kNone ? id : Message::kNoScriptIdInfo;
}
// static
int StackTraceFrame::GetPromiseAllIndex(Handle<StackTraceFrame> frame) {
return GetFrameInfo(frame)->promise_all_index();
}
// static
Handle<Object> StackTraceFrame::GetFileName(Handle<StackTraceFrame> frame) {
auto name = GetFrameInfo(frame)->script_name();
return handle(name, frame->GetIsolate());
}
// static
Handle<Object> StackTraceFrame::GetScriptNameOrSourceUrl(
Handle<StackTraceFrame> frame) {
auto name = GetFrameInfo(frame)->script_name_or_source_url();
return handle(name, frame->GetIsolate());
}
// static
Handle<Object> StackTraceFrame::GetFunctionName(Handle<StackTraceFrame> frame) {
auto name = GetFrameInfo(frame)->function_name();
return handle(name, frame->GetIsolate());
}
// static
Handle<Object> StackTraceFrame::GetMethodName(Handle<StackTraceFrame> frame) {
auto name = GetFrameInfo(frame)->method_name();
return handle(name, frame->GetIsolate());
}
// static
Handle<Object> StackTraceFrame::GetTypeName(Handle<StackTraceFrame> frame) {
auto name = GetFrameInfo(frame)->type_name();
return handle(name, frame->GetIsolate());
}
// static
Handle<Object> StackTraceFrame::GetEvalOrigin(Handle<StackTraceFrame> frame) {
auto origin = GetFrameInfo(frame)->eval_origin();
return handle(origin, frame->GetIsolate());
}
// static
Handle<Object> StackTraceFrame::GetWasmModuleName(
Handle<StackTraceFrame> frame) {
auto module = GetFrameInfo(frame)->wasm_module_name();
return handle(module, frame->GetIsolate());
}
// static
bool StackTraceFrame::IsEval(Handle<StackTraceFrame> frame) {
return GetFrameInfo(frame)->is_eval();
}
// static
bool StackTraceFrame::IsConstructor(Handle<StackTraceFrame> frame) {
return GetFrameInfo(frame)->is_constructor();
}
// static
bool StackTraceFrame::IsWasm(Handle<StackTraceFrame> frame) {
return GetFrameInfo(frame)->is_wasm();
}
// static
bool StackTraceFrame::IsAsmJsWasm(Handle<StackTraceFrame> frame) {
return GetFrameInfo(frame)->is_asmjs_wasm();
}
// static
bool StackTraceFrame::IsUserJavaScript(Handle<StackTraceFrame> frame) {
return GetFrameInfo(frame)->is_user_java_script();
}
// static
bool StackTraceFrame::IsToplevel(Handle<StackTraceFrame> frame) {
return GetFrameInfo(frame)->is_toplevel();
}
// static
bool StackTraceFrame::IsAsync(Handle<StackTraceFrame> frame) {
return GetFrameInfo(frame)->is_async();
}
// static
bool StackTraceFrame::IsPromiseAll(Handle<StackTraceFrame> frame) {
return GetFrameInfo(frame)->is_promise_all();
}
// static
Handle<StackFrameInfo> StackTraceFrame::GetFrameInfo(
Handle<StackTraceFrame> frame) {
if (frame->frame_info().IsUndefined()) InitializeFrameInfo(frame);
return handle(StackFrameInfo::cast(frame->frame_info()), frame->GetIsolate());
}
// static
void StackTraceFrame::InitializeFrameInfo(Handle<StackTraceFrame> frame) {
Isolate* isolate = frame->GetIsolate();
Handle<StackFrameInfo> frame_info = isolate->factory()->NewStackFrameInfo(
......
......@@ -25,10 +25,14 @@ class StackFrameInfo : public Struct {
DECL_ACCESSORS(script_name, Object)
DECL_ACCESSORS(script_name_or_source_url, Object)
DECL_ACCESSORS(function_name, Object)
DECL_ACCESSORS(method_name, Object)
DECL_ACCESSORS(type_name, Object)
DECL_ACCESSORS(eval_origin, Object)
DECL_ACCESSORS(wasm_module_name, Object)
DECL_BOOLEAN_ACCESSORS(is_eval)
DECL_BOOLEAN_ACCESSORS(is_constructor)
DECL_BOOLEAN_ACCESSORS(is_wasm)
DECL_BOOLEAN_ACCESSORS(is_asmjs_wasm)
DECL_BOOLEAN_ACCESSORS(is_user_java_script)
DECL_BOOLEAN_ACCESSORS(is_toplevel)
DECL_BOOLEAN_ACCESSORS(is_async)
......@@ -49,10 +53,11 @@ class StackFrameInfo : public Struct {
static const int kIsEvalBit = 0;
static const int kIsConstructorBit = 1;
static const int kIsWasmBit = 2;
static const int kIsUserJavaScriptBit = 3;
static const int kIsToplevelBit = 4;
static const int kIsAsyncBit = 5;
static const int kIsPromiseAllBit = 6;
static const int kIsAsmJsWasmBit = 3;
static const int kIsUserJavaScriptBit = 4;
static const int kIsToplevelBit = 5;
static const int kIsAsyncBit = 6;
static const int kIsPromiseAllBit = 7;
OBJECT_CONSTRUCTORS(StackFrameInfo, Struct);
};
......@@ -87,11 +92,15 @@ class StackTraceFrame : public Struct {
static Handle<Object> GetFileName(Handle<StackTraceFrame> frame);
static Handle<Object> GetScriptNameOrSourceUrl(Handle<StackTraceFrame> frame);
static Handle<Object> GetFunctionName(Handle<StackTraceFrame> frame);
static Handle<Object> GetMethodName(Handle<StackTraceFrame> frame);
static Handle<Object> GetTypeName(Handle<StackTraceFrame> frame);
static Handle<Object> GetEvalOrigin(Handle<StackTraceFrame> frame);
static Handle<Object> GetWasmModuleName(Handle<StackTraceFrame> frame);
static bool IsEval(Handle<StackTraceFrame> frame);
static bool IsConstructor(Handle<StackTraceFrame> frame);
static bool IsWasm(Handle<StackTraceFrame> frame);
static bool IsAsmJsWasm(Handle<StackTraceFrame> frame);
static bool IsUserJavaScript(Handle<StackTraceFrame> frame);
static bool IsToplevel(Handle<StackTraceFrame> frame);
static bool IsAsync(Handle<StackTraceFrame> frame);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment