Commit 54584461 authored by Benedikt Meurer's avatar Benedikt Meurer Committed by V8 LUCI CQ

[inspector] Consistent frame function name in V8 Inspector and API.

On the way to a cheaper and more scalable stack frame representation
for the inspector (crbug/1258599), this removes the need to expose
both what was called "function name" and what was called "function
debug name" on a v8::StackFrame instance.

The reason to having a distinction between that the V8 API exposes
and what the inspector exposes as frame function name is that after
the initial refactoring around v8::internal::StackFrameInfo, some
wasm cctests would still dig into the implementation details and
insist on seeing the "function name" rather than the "function
debug name". This CL now addresses that detail in the wasm cctests
and going forward unifies the function names used by the inspector
and the V8 API (which is not only needed for internal consistency
and reduced storage requirements in the future, but also because
Blink for example uses v8 API and v8_inspector API interchangeably
and assumes that they agree, even though at this point Blink
luckily wasn't paying attention to the function name):

- The so-called "detailed stack trace", which is produced for the
  inspector and exposed by the v8 API, always yields the "function
  debug name" (which for example in case of wasm will be a WAT
  compatible name),
- while the so-called "simple stack trace", which is what is used
  to implement the CallSite API and underlies Error.stack continues
  to stick to the "function name" which in case of wasm is not
  WAT compatible).

Bug: chromium:1258599
Change-Id: Ib15d038f3ec893703d0f7b03f6e7573a38e82b39
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3312274Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
Reviewed-by: 's avatarSimon Zünd <szuend@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78283}
parent 833eba71
...@@ -44,6 +44,9 @@ ...@@ -44,6 +44,9 @@
#include "src/common/globals.h" #include "src/common/globals.h"
#include "src/compiler-dispatcher/lazy-compile-dispatcher.h" #include "src/compiler-dispatcher/lazy-compile-dispatcher.h"
#include "src/date/date.h" #include "src/date/date.h"
#if V8_ENABLE_WEBASSEMBLY
#include "src/debug/debug-wasm-objects.h"
#endif // V8_ENABLE_WEBASSEMBLY
#include "src/debug/liveedit.h" #include "src/debug/liveedit.h"
#include "src/deoptimizer/deoptimizer.h" #include "src/deoptimizer/deoptimizer.h"
#include "src/diagnostics/gdb-jit.h" #include "src/diagnostics/gdb-jit.h"
...@@ -3294,6 +3297,15 @@ Local<String> StackFrame::GetScriptSourceMappingURL() const { ...@@ -3294,6 +3297,15 @@ Local<String> StackFrame::GetScriptSourceMappingURL() const {
Local<String> StackFrame::GetFunctionName() const { Local<String> StackFrame::GetFunctionName() const {
auto self = Utils::OpenHandle(this); auto self = Utils::OpenHandle(this);
#if V8_ENABLE_WEBASSEMBLY
if (self->IsWasm()) {
auto isolate = self->GetIsolate();
auto instance = handle(self->GetWasmInstance(), isolate);
auto func_index = self->GetWasmFunctionIndex();
return Utils::ToLocal(
i::GetWasmFunctionDebugName(isolate, instance, func_index));
}
#endif // V8_ENABLE_WEBASSEMBLY
auto name = i::StackFrameInfo::GetFunctionName(self); auto name = i::StackFrameInfo::GetFunctionName(self);
if (!name->IsString()) return {}; if (!name->IsString()) return {};
return Local<String>::Cast(Utils::ToLocal(name)); return Local<String>::Cast(Utils::ToLocal(name));
......
...@@ -49,20 +49,6 @@ v8_inspector::V8Inspector* GetInspector(Isolate* isolate) { ...@@ -49,20 +49,6 @@ v8_inspector::V8Inspector* GetInspector(Isolate* isolate) {
return reinterpret_cast<i::Isolate*>(isolate)->inspector(); return reinterpret_cast<i::Isolate*>(isolate)->inspector();
} }
Local<String> GetFunctionDebugName(Local<StackFrame> frame) {
#if V8_ENABLE_WEBASSEMBLY
auto info = Utils::OpenHandle(*frame);
if (info->IsWasm()) {
auto isolate = info->GetIsolate();
auto instance = handle(info->GetWasmInstance(), isolate);
auto func_index = info->GetWasmFunctionIndex();
return Utils::ToLocal(
i::GetWasmFunctionDebugName(isolate, instance, func_index));
}
#endif // V8_ENABLE_WEBASSEMBLY
return frame->GetFunctionName();
}
Local<String> GetFunctionDescription(Local<Function> function) { Local<String> GetFunctionDescription(Local<Function> function) {
auto receiver = Utils::OpenHandle(*function); auto receiver = Utils::OpenHandle(*function);
if (receiver->IsJSBoundFunction()) { if (receiver->IsJSBoundFunction()) {
......
...@@ -49,13 +49,6 @@ int GetContextId(Local<Context> context); ...@@ -49,13 +49,6 @@ int GetContextId(Local<Context> context);
void SetInspector(Isolate* isolate, v8_inspector::V8Inspector*); void SetInspector(Isolate* isolate, v8_inspector::V8Inspector*);
v8_inspector::V8Inspector* GetInspector(Isolate* isolate); v8_inspector::V8Inspector* GetInspector(Isolate* isolate);
// Returns the debug name for the function, which is supposed to be used
// by the debugger and the developer tools. This can thus be different from
// the name returned by the StackFrame::GetFunctionName() method. For example,
// in case of WebAssembly, the debug name is WAT-compatible and thus always
// preceeded by a dollar ('$').
Local<String> GetFunctionDebugName(Local<StackFrame> frame);
// Returns a debug string representation of the function. // Returns a debug string representation of the function.
Local<String> GetFunctionDescription(Local<Function> function); Local<String> GetFunctionDescription(Local<Function> function);
......
...@@ -1094,8 +1094,7 @@ std::shared_ptr<StackFrame> V8Debugger::symbolize( ...@@ -1094,8 +1094,7 @@ std::shared_ptr<StackFrame> V8Debugger::symbolize(
int lineNumber = v8Frame->GetLineNumber() - 1; int lineNumber = v8Frame->GetLineNumber() - 1;
int columnNumber = v8Frame->GetColumn() - 1; int columnNumber = v8Frame->GetColumn() - 1;
CachedStackFrameKey key{scriptId, lineNumber, columnNumber}; CachedStackFrameKey key{scriptId, lineNumber, columnNumber};
auto functionName = auto functionName = toProtocolString(isolate(), v8Frame->GetFunctionName());
toProtocolString(isolate(), v8::debug::GetFunctionDebugName(v8Frame));
auto it = m_cachedStackFrames.find(key); auto it = m_cachedStackFrames.find(key);
if (it != m_cachedStackFrames.end() && !it->second.expired()) { if (it != m_cachedStackFrames.end() && !it->second.expired()) {
auto stackFrame = it->second.lock(); auto stackFrame = it->second.lock();
......
...@@ -50,8 +50,8 @@ class StackFrameInfo ...@@ -50,8 +50,8 @@ class StackFrameInfo
// Used to signal that the requested field is unknown. // Used to signal that the requested field is unknown.
static constexpr int kUnknown = kNoSourcePosition; static constexpr int kUnknown = kNoSourcePosition;
static int GetLineNumber(Handle<StackFrameInfo> info); V8_EXPORT_PRIVATE static int GetLineNumber(Handle<StackFrameInfo> info);
static int GetColumnNumber(Handle<StackFrameInfo> info); V8_EXPORT_PRIVATE static int GetColumnNumber(Handle<StackFrameInfo> info);
static int GetEnclosingLineNumber(Handle<StackFrameInfo> info); static int GetEnclosingLineNumber(Handle<StackFrameInfo> info);
static int GetEnclosingColumnNumber(Handle<StackFrameInfo> info); static int GetEnclosingColumnNumber(Handle<StackFrameInfo> info);
...@@ -65,7 +65,8 @@ class StackFrameInfo ...@@ -65,7 +65,8 @@ class StackFrameInfo
Object GetScriptSourceMappingURL() const; Object GetScriptSourceMappingURL() const;
static Handle<PrimitiveHeapObject> GetEvalOrigin(Handle<StackFrameInfo> info); static Handle<PrimitiveHeapObject> GetEvalOrigin(Handle<StackFrameInfo> info);
static Handle<Object> GetFunctionName(Handle<StackFrameInfo> info); V8_EXPORT_PRIVATE static Handle<Object> GetFunctionName(
Handle<StackFrameInfo> info);
static Handle<Object> GetMethodName(Handle<StackFrameInfo> info); static Handle<Object> GetMethodName(Handle<StackFrameInfo> info);
static Handle<Object> GetTypeName(Handle<StackFrameInfo> info); static Handle<Object> GetTypeName(Handle<StackFrameInfo> info);
......
...@@ -160,8 +160,8 @@ WASM_COMPILED_EXEC_TEST(CollectDetailedWasmStack_ExplicitThrowFromJs) { ...@@ -160,8 +160,8 @@ WASM_COMPILED_EXEC_TEST(CollectDetailedWasmStack_ExplicitThrowFromJs) {
ExceptionInfo expected_exceptions[] = { ExceptionInfo expected_exceptions[] = {
{"a", 3, 8}, // - {"a", 3, 8}, // -
{"js", 4, 2}, // - {"js", 4, 2}, // -
{"main", 1, 8}, // - {"$main", 1, 8}, // -
{"call_main", 1, 21}, // - {"$call_main", 1, 21}, // -
{"callFn", 1, 24} // - {"callFn", 1, 24} // -
}; };
CheckExceptionInfos(isolate, maybe_exc.ToHandleChecked(), CheckExceptionInfos(isolate, maybe_exc.ToHandleChecked(),
...@@ -275,8 +275,8 @@ WASM_COMPILED_EXEC_TEST(CollectDetailedWasmStack_WasmError) { ...@@ -275,8 +275,8 @@ WASM_COMPILED_EXEC_TEST(CollectDetailedWasmStack_WasmError) {
unreachable_pos + main_offset + kMainLocalsLength + 1; unreachable_pos + main_offset + kMainLocalsLength + 1;
const int expected_call_main_pos = call_main_offset + kMainLocalsLength + 1; const int expected_call_main_pos = call_main_offset + kMainLocalsLength + 1;
ExceptionInfo expected_exceptions[] = { ExceptionInfo expected_exceptions[] = {
{"main", 1, expected_main_pos}, // - {"$main", 1, expected_main_pos}, // -
{"call_main", 1, expected_call_main_pos}, // - {"$call_main", 1, expected_call_main_pos}, // -
{"callFn", 1, 24} //- {"callFn", 1, 24} //-
}; };
CheckExceptionInfos(isolate, exception, expected_exceptions); CheckExceptionInfos(isolate, exception, expected_exceptions);
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "include/v8-function.h" #include "include/v8-function.h"
#include "src/api/api-inl.h" #include "src/api/api-inl.h"
#include "src/codegen/assembler-inl.h" #include "src/codegen/assembler-inl.h"
#include "src/objects/stack-frame-info-inl.h"
#include "src/trap-handler/trap-handler.h" #include "src/trap-handler/trap-handler.h"
#include "test/cctest/cctest.h" #include "test/cctest/cctest.h"
#include "test/cctest/compiler/value-helper.h" #include "test/cctest/compiler/value-helper.h"
...@@ -40,25 +41,24 @@ struct ExceptionInfo { ...@@ -40,25 +41,24 @@ struct ExceptionInfo {
}; };
template <int N> template <int N>
void CheckExceptionInfos(v8::internal::Isolate* i_isolate, Handle<Object> exc, void CheckExceptionInfos(v8::internal::Isolate* isolate, Handle<Object> exc,
const ExceptionInfo (&excInfos)[N]) { const ExceptionInfo (&excInfos)[N]) {
// Check that it's indeed an Error object. // Check that it's indeed an Error object.
CHECK(exc->IsJSError()); CHECK(exc->IsJSError());
v8::Isolate* v8_isolate = reinterpret_cast<v8::Isolate*>(i_isolate);
exc->Print(); exc->Print();
// Extract stack frame from the exception. // Extract stack frame from the exception.
Local<v8::Value> localExc = Utils::ToLocal(exc); auto stack = Handle<FixedArray>::cast(JSReceiver::GetDataProperty(
v8::Local<v8::StackTrace> stack = v8::Exception::GetStackTrace(localExc); Handle<JSObject>::cast(exc), isolate->factory()->stack_trace_symbol()));
CHECK(!stack.IsEmpty()); CHECK_EQ(N, stack->length());
CHECK_EQ(N, stack->GetFrameCount());
for (int i = 0; i < N; ++i) {
for (int frameNr = 0; frameNr < N; ++frameNr) { Handle<StackFrameInfo> info(StackFrameInfo::cast(stack->get(i)), isolate);
v8::Local<v8::StackFrame> frame = stack->GetFrame(v8_isolate, frameNr); auto func_name = Handle<String>::cast(StackFrameInfo::GetFunctionName(info))
v8::String::Utf8Value funName(v8_isolate, frame->GetFunctionName()); ->ToCString();
CHECK_CSTREQ(excInfos[frameNr].func_name, *funName); CHECK_CSTREQ(excInfos[i].func_name, func_name.get());
CHECK_EQ(excInfos[frameNr].line_nr, frame->GetLineNumber()); CHECK_EQ(excInfos[i].line_nr, StackFrameInfo::GetLineNumber(info));
CHECK_EQ(excInfos[frameNr].column, frame->GetColumn()); CHECK_EQ(excInfos[i].column, StackFrameInfo::GetColumnNumber(info));
} }
} }
......
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