[debug] Introduce a dedicated StackFrameInfo for v8::StackFrame.

This is the final change list in the list of refactorings to split off
the implementations of v8::StackFrame and CallSite objects (as used by
the V8 JavaScript stack API). See https://bit.ly/v8-stack-frame for the
whole story.

This CL adds the v8::internal::StackFrameInfo class as new backing
implementation of v8::StackFrame, and puts it into debug-objects.tq
to indicate that it's used for the debugger API only. This new class
is lightweight and only holds on to static information about the
stack frame, and is thus usable for the V8 inspector to implement
async stack traces in a cheaper manner going forward.

Doc: https://bit.ly/v8-stack-frame
Bug: chromium:1258599, chromium:1278650
Fixed: chromium:1278647
Change-Id: I4dbf2d850f47797263af225895129499169aad02
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3302794
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78382}

src/api/api-inl.h

@@ -12,7 +12,6 @@
 #include "src/execution/protectors.h"
 #include "src/handles/handles-inl.h"
 #include "src/heap/heap-inl.h"
-#include "src/objects/call-site-info.h"
 #include "src/objects/foreign-inl.h"
 #include "src/objects/js-weak-refs.h"
 #include "src/objects/objects-inl.h"
@@ -101,7 +100,7 @@ MAKE_TO_LOCAL(AccessorSignatureToLocal, FunctionTemplateInfo, AccessorSignature)
 MAKE_TO_LOCAL(MessageToLocal, Object, Message)
 MAKE_TO_LOCAL(PromiseToLocal, JSObject, Promise)
 MAKE_TO_LOCAL(StackTraceToLocal, FixedArray, StackTrace)
-MAKE_TO_LOCAL(StackFrameToLocal, CallSiteInfo, StackFrame)
+MAKE_TO_LOCAL(StackFrameToLocal, StackFrameInfo, StackFrame)
 MAKE_TO_LOCAL(NumberToLocal, Object, Number)
 MAKE_TO_LOCAL(IntegerToLocal, Object, Integer)
 MAKE_TO_LOCAL(Uint32ToLocal, Object, Uint32)

src/api/api.cc

@@ -44,6 +44,7 @@
 #include "src/common/globals.h"
 #include "src/compiler-dispatcher/lazy-compile-dispatcher.h"
 #include "src/date/date.h"
+#include "src/objects/primitive-heap-object.h"
 #if V8_ENABLE_WEBASSEMBLY
 #include "src/debug/debug-wasm-objects.h"
 #endif  // V8_ENABLE_WEBASSEMBLY
@@ -78,7 +79,6 @@
 #include "src/logging/tracing-flags.h"
 #include "src/numbers/conversions-inl.h"
 #include "src/objects/api-callbacks.h"
-#include "src/objects/call-site-info-inl.h"
 #include "src/objects/contexts.h"
 #include "src/objects/embedder-data-array-inl.h"
 #include "src/objects/embedder-data-slot-inl.h"
@@ -3229,9 +3229,9 @@ void Message::PrintCurrentStackTrace(Isolate* isolate, std::ostream& out) {
 Local<StackFrame> StackTrace::GetFrame(Isolate* v8_isolate,
                                        uint32_t index) const {
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
-  i::Handle<i::CallSiteInfo> frame(
-      i::CallSiteInfo::cast(Utils::OpenHandle(this)->get(index)), isolate);
-  return Utils::StackFrameToLocal(frame);
+  i::Handle<i::StackFrameInfo> info(
+      i::StackFrameInfo::cast(Utils::OpenHandle(this)->get(index)), isolate);
+  return Utils::StackFrameToLocal(info);
 }
 
 int StackTrace::GetFrameCount() const {
@@ -3251,82 +3251,89 @@ Local<StackTrace> StackTrace::CurrentStackTrace(Isolate* isolate,
 // --- S t a c k F r a m e ---
 
 int StackFrame::GetLineNumber() const {
-  return i::CallSiteInfo::GetLineNumber(Utils::OpenHandle(this));
+  i::Handle<i::StackFrameInfo> self = Utils::OpenHandle(this);
+  i::Handle<i::Script> script(self->script(), self->GetIsolate());
+  int position = self->source_position();
+  int line_number = i::Script::GetLineNumber(script, position) + 1;
+  if (script->HasSourceURLComment()) {
+    line_number -= script->line_offset();
+  }
+  return line_number;
 }
 
 int StackFrame::GetColumn() const {
-  return i::CallSiteInfo::GetColumnNumber(Utils::OpenHandle(this));
+  i::Handle<i::StackFrameInfo> self = Utils::OpenHandle(this);
+  i::Handle<i::Script> script(self->script(), self->GetIsolate());
+  int position = self->source_position();
+  int column_number = i::Script::GetColumnNumber(script, position) + 1;
+  if (script->HasSourceURLComment()) {
+    if (i::Script::GetLineNumber(script, position) == script->line_offset()) {
+      column_number -= script->column_offset();
+    }
+  }
+  return column_number;
 }
 
 int StackFrame::GetScriptId() const {
-  return Utils::OpenHandle(this)->GetScriptId();
+  return Utils::OpenHandle(this)->script().id();
 }
 
 Local<String> StackFrame::GetScriptName() const {
-  auto self = Utils::OpenHandle(this);
-  auto isolate = self->GetIsolate();
-  i::Handle<i::Object> name(self->GetScriptName(), isolate);
+  i::Handle<i::StackFrameInfo> self = Utils::OpenHandle(this);
+  i::Isolate* isolate = self->GetIsolate();
+  i::Handle<i::Object> name(self->script().name(), isolate);
   if (!name->IsString()) return {};
-  return Local<String>::Cast(Utils::ToLocal(name));
+  return Utils::ToLocal(i::Handle<i::String>::cast(name));
 }
 
 Local<String> StackFrame::GetScriptNameOrSourceURL() const {
-  auto self = Utils::OpenHandle(this);
-  auto isolate = self->GetIsolate();
-  i::Handle<i::Object> name_or_url(self->GetScriptNameOrSourceURL(), isolate);
-  if (!name_or_url->IsString()) return {};
-  return Local<String>::Cast(Utils::ToLocal(name_or_url));
+  i::Handle<i::StackFrameInfo> self = Utils::OpenHandle(this);
+  i::Isolate* isolate = self->GetIsolate();
+  i::Handle<i::Object> name_or_source_url(self->script().GetNameOrSourceURL(),
+                                          isolate);
+  if (!name_or_source_url->IsString()) return {};
+  return Utils::ToLocal(i::Handle<i::String>::cast(name_or_source_url));
 }
 
 Local<String> StackFrame::GetScriptSource() const {
-  auto self = Utils::OpenHandle(this);
-  auto isolate = self->GetIsolate();
-  i::Handle<i::Object> source(self->GetScriptSource(), isolate);
+  i::Handle<i::StackFrameInfo> self = Utils::OpenHandle(this);
+  i::Isolate* isolate = self->GetIsolate();
+  if (!self->script().HasValidSource()) return {};
+  i::Handle<i::PrimitiveHeapObject> source(self->script().source(), isolate);
   if (!source->IsString()) return {};
-  return Local<String>::Cast(Utils::ToLocal(source));
+  return Utils::ToLocal(i::Handle<i::String>::cast(source));
 }
 
 Local<String> StackFrame::GetScriptSourceMappingURL() const {
-  auto self = Utils::OpenHandle(this);
-  auto isolate = self->GetIsolate();
-  i::Handle<i::Object> sourceMappingURL(self->GetScriptSourceMappingURL(),
-                                        isolate);
-  if (!sourceMappingURL->IsString()) return {};
-  return Local<String>::Cast(Utils::ToLocal(sourceMappingURL));
+  i::Handle<i::StackFrameInfo> self = Utils::OpenHandle(this);
+  i::Isolate* isolate = self->GetIsolate();
+  i::Handle<i::Object> source_mapping_url(self->script().source_mapping_url(),
+                                          isolate);
+  if (!source_mapping_url->IsString()) return {};
+  return Utils::ToLocal(i::Handle<i::String>::cast(source_mapping_url));
 }
 
 Local<String> StackFrame::GetFunctionName() const {
-  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::CallSiteInfo::GetFunctionName(self);
+  i::Handle<i::StackFrameInfo> self = Utils::OpenHandle(this);
+  i::Isolate* isolate = self->GetIsolate();
+  i::Handle<i::PrimitiveHeapObject> name(self->function_name(), isolate);
   if (!name->IsString()) return {};
-  return Local<String>::Cast(Utils::ToLocal(name));
+  return Utils::ToLocal(i::Handle<i::String>::cast(name));
 }
 
-bool StackFrame::IsEval() const { return Utils::OpenHandle(this)->IsEval(); }
+bool StackFrame::IsEval() const {
+  i::Handle<i::StackFrameInfo> self = Utils::OpenHandle(this);
+  return self->script().compilation_type() == i::Script::COMPILATION_TYPE_EVAL;
+}
 
 bool StackFrame::IsConstructor() const {
-  return Utils::OpenHandle(this)->IsConstructor();
+  return Utils::OpenHandle(this)->is_constructor();
 }
 
-bool StackFrame::IsWasm() const {
-#if V8_ENABLE_WEBASSEMBLY
-  return Utils::OpenHandle(this)->IsWasm();
-#else
-  return false;
-#endif  // V8_ENABLE_WEBASSEMBLY
-}
+bool StackFrame::IsWasm() const { return !IsUserJavaScript(); }
 
 bool StackFrame::IsUserJavaScript() const {
-  return Utils::OpenHandle(this)->IsUserJavaScript();
+  return Utils::OpenHandle(this)->script().IsUserJavaScript();
 }
 
 // --- J S O N ---

src/api/api.h

@@ -131,7 +131,7 @@ class RegisteredExtension {
   V(Context, Context)                          \
   V(External, Object)                          \
   V(StackTrace, FixedArray)                    \
-  V(StackFrame, CallSiteInfo)                  \
+  V(StackFrame, StackFrameInfo)                \
   V(Proxy, JSProxy)                            \
   V(debug::GeneratorObject, JSGeneratorObject) \
   V(debug::Script, Script)                     \
@@ -227,7 +227,7 @@ class Utils {
   static inline Local<StackTrace> StackTraceToLocal(
       v8::internal::Handle<v8::internal::FixedArray> obj);
   static inline Local<StackFrame> StackFrameToLocal(
-      v8::internal::Handle<v8::internal::CallSiteInfo> obj);
+      v8::internal::Handle<v8::internal::StackFrameInfo> obj);
   static inline Local<Number> NumberToLocal(
       v8::internal::Handle<v8::internal::Object> obj);
   static inline Local<Integer> IntegerToLocal(

src/debug/debug-interface.cc

@@ -15,7 +15,6 @@
 #include "src/debug/debug-type-profile.h"
 #include "src/debug/debug.h"
 #include "src/execution/vm-state-inl.h"
-#include "src/objects/call-site-info-inl.h"
 #include "src/objects/js-generator-inl.h"
 #include "src/profiler/heap-profiler.h"
 #include "src/strings/string-builder-inl.h"

src/diagnostics/objects-debug.cc

@@ -1802,6 +1802,10 @@ void FunctionTemplateRareData::FunctionTemplateRareDataVerify(
         c_function_overloads().IsUndefined(isolate));
 }
 
+void StackFrameInfo::StackFrameInfoVerify(Isolate* isolate) {
+  TorqueGeneratedClassVerifiers::StackFrameInfoVerify(*this, isolate);
+}
+
 // Helper class for verifying the string table.
 class StringTableVerifier : public RootVisitor {
  public:

src/execution/frames.cc

@@ -28,6 +28,7 @@
 #include "src/zone/zone-containers.h"
 
 #if V8_ENABLE_WEBASSEMBLY
+#include "src/debug/debug-wasm-objects.h"
 #include "src/wasm/wasm-code-manager.h"
 #include "src/wasm/wasm-engine.h"
 #include "src/wasm/wasm-objects-inl.h"
@@ -1592,6 +1593,19 @@ Handle<Context> FrameSummary::JavaScriptFrameSummary::native_context() const {
   return handle(function_->context().native_context(), isolate());
 }
 
+Handle<PrimitiveHeapObject> FrameSummary::JavaScriptFrameSummary::FunctionName()
+    const {
+  Handle<JSFunction> func = function();
+  Handle<String> name = JSFunction::GetDebugName(func);
+  if (name->length() != 0) return name;
+  if (func->shared().script().IsScript() &&
+      Script::cast(func->shared().script()).compilation_type() ==
+          Script::COMPILATION_TYPE_EVAL) {
+    return isolate()->factory()->eval_string();
+  }
+  return isolate()->factory()->null_value();
+}
+
 #if V8_ENABLE_WEBASSEMBLY
 FrameSummary::WasmFrameSummary::WasmFrameSummary(
     Isolate* isolate, Handle<WasmInstanceObject> instance, wasm::WasmCode* code,
@@ -1628,6 +1642,10 @@ Handle<Script> FrameSummary::WasmFrameSummary::script() const {
 Handle<Context> FrameSummary::WasmFrameSummary::native_context() const {
   return handle(wasm_instance()->native_context(), isolate());
 }
+
+Handle<String> FrameSummary::WasmFrameSummary::FunctionName() const {
+  return GetWasmFunctionDebugName(isolate(), wasm_instance(), function_index());
+}
 #endif  // V8_ENABLE_WEBASSEMBLY
 
 FrameSummary::~FrameSummary() {
@@ -1697,6 +1715,7 @@ FRAME_SUMMARY_DISPATCH(Handle<Object>, script)
 FRAME_SUMMARY_DISPATCH(int, SourcePosition)
 FRAME_SUMMARY_DISPATCH(int, SourceStatementPosition)
 FRAME_SUMMARY_DISPATCH(Handle<Context>, native_context)
+FRAME_SUMMARY_DISPATCH(Handle<PrimitiveHeapObject>, FunctionName)
 
 #undef FRAME_SUMMARY_DISPATCH
 

src/execution/frames.h

@@ -394,6 +394,7 @@ class V8_EXPORT_PRIVATE FrameSummary {
     int SourceStatementPosition() const;
     Handle<Object> script() const;
     Handle<Context> native_context() const;
+    Handle<PrimitiveHeapObject> FunctionName() const;
 
    private:
     Handle<Object> receiver_;
@@ -423,6 +424,7 @@ class V8_EXPORT_PRIVATE FrameSummary {
     Handle<WasmInstanceObject> wasm_instance() const { return wasm_instance_; }
     Handle<Context> native_context() const;
     bool at_to_number_conversion() const { return at_to_number_conversion_; }
+    Handle<String> FunctionName() const;
 
    private:
     Handle<WasmInstanceObject> wasm_instance_;
@@ -456,6 +458,7 @@ class V8_EXPORT_PRIVATE FrameSummary {
   int SourcePosition() const;
   int SourceStatementPosition() const;
   Handle<Context> native_context() const;
+  Handle<PrimitiveHeapObject> FunctionName() const;
 
 #define FRAME_SUMMARY_CAST(kind_, type, field, desc)      \
   bool Is##desc() const { return base_.kind() == kind_; } \

src/execution/isolate.h

@@ -898,9 +898,8 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
                                     void* ptr3 = nullptr, void* ptr4 = nullptr);
   Handle<FixedArray> CaptureDetailedStackTrace(
       int limit, StackTrace::StackTraceOptions options);
-  Handle<Object> CaptureSimpleStackTrace(Handle<JSReceiver> error_object,
-                                         FrameSkipMode mode,
-                                         Handle<Object> caller);
+  Handle<FixedArray> CaptureSimpleStackTrace(int limit, FrameSkipMode mode,
+                                             Handle<Object> caller);
   MaybeHandle<JSReceiver> CaptureAndSetDetailedStackTrace(
       Handle<JSReceiver> error_object);
   MaybeHandle<JSReceiver> CaptureAndSetSimpleStackTrace(

src/heap/factory.cc

@@ -3357,6 +3357,20 @@ Handle<CallSiteInfo> Factory::NewCallSiteInfo(
   return handle(info, isolate());
 }
 
+Handle<StackFrameInfo> Factory::NewStackFrameInfo(
+    Handle<Script> script, int source_position,
+    Handle<PrimitiveHeapObject> function_name, bool is_constructor) {
+  StackFrameInfo info = NewStructInternal<StackFrameInfo>(
+      STACK_FRAME_INFO_TYPE, AllocationType::kYoung);
+  DisallowGarbageCollection no_gc;
+  info.set_flags(0);
+  info.set_script(*script, SKIP_WRITE_BARRIER);
+  info.set_source_position(source_position);
+  info.set_function_name(*function_name, SKIP_WRITE_BARRIER);
+  info.set_is_constructor(is_constructor);
+  return handle(info, isolate());
+}
+
 Handle<JSObject> Factory::NewArgumentsObject(Handle<JSFunction> callee,
                                              int length) {
   bool strict_mode_callee = is_strict(callee->shared().language_mode()) ||

src/heap/factory.h

@@ -63,6 +63,7 @@ class PromiseResolveThenableJobTask;
 class RegExpMatchInfo;
 class ScriptContextTable;
 class SourceTextModule;
+class StackFrameInfo;
 class StringSet;
 class StoreHandler;
 class SyntheticModule;
@@ -401,6 +402,9 @@ class V8_EXPORT_PRIVATE Factory : public FactoryBase<Factory> {
                                        int code_offset_or_source_position,
                                        int flags,
                                        Handle<FixedArray> parameters);
+  Handle<StackFrameInfo> NewStackFrameInfo(
+      Handle<Script> script, int source_position,
+      Handle<PrimitiveHeapObject> function_name, bool is_constructor);
 
   // Allocate various microtasks.
   Handle<CallableTask> NewCallableTask(Handle<JSReceiver> callable,

src/objects/call-site-info.cc

@@ -254,7 +254,8 @@ Handle<PrimitiveHeapObject> CallSiteInfo::GetEvalOrigin(
 }
 
 // static
-Handle<Object> CallSiteInfo::GetFunctionName(Handle<CallSiteInfo> info) {
+Handle<PrimitiveHeapObject> CallSiteInfo::GetFunctionName(
+    Handle<CallSiteInfo> info) {
   Isolate* isolate = info->GetIsolate();
 #if V8_ENABLE_WEBASSEMBLY
   if (info->IsWasm()) {

src/objects/call-site-info.h

@@ -58,6 +58,8 @@ class CallSiteInfo : public TorqueGeneratedCallSiteInfo<CallSiteInfo, Struct> {
 
   // Returns the script ID if one is attached,
   // Message::kNoScriptIdInfo otherwise.
+  static MaybeHandle<Script> GetScript(Isolate* isolate,
+                                       Handle<CallSiteInfo> info);
   int GetScriptId() const;
   Object GetScriptName() const;
   Object GetScriptNameOrSourceURL() const;
@@ -65,7 +67,7 @@ class CallSiteInfo : public TorqueGeneratedCallSiteInfo<CallSiteInfo, Struct> {
   Object GetScriptSourceMappingURL() const;
 
   static Handle<PrimitiveHeapObject> GetEvalOrigin(Handle<CallSiteInfo> info);
-  V8_EXPORT_PRIVATE static Handle<Object> GetFunctionName(
+  V8_EXPORT_PRIVATE static Handle<PrimitiveHeapObject> GetFunctionName(
       Handle<CallSiteInfo> info);
   static Handle<Object> GetMethodName(Handle<CallSiteInfo> info);
   static Handle<Object> GetTypeName(Handle<CallSiteInfo> info);
@@ -96,9 +98,6 @@ class CallSiteInfo : public TorqueGeneratedCallSiteInfo<CallSiteInfo, Struct> {
   base::Optional<Script> GetScript() const;
   SharedFunctionInfo GetSharedFunctionInfo() const;
 
-  static MaybeHandle<Script> GetScript(Isolate* isolate,
-                                       Handle<CallSiteInfo> info);
-
   TQ_OBJECT_CONSTRUCTORS(CallSiteInfo)
 };
 

src/objects/debug-objects-inl.h

@@ -52,6 +52,14 @@ BytecodeArray DebugInfo::DebugBytecodeArray() {
   return BytecodeArray::cast(debug_bytecode_array(kAcquireLoad));
 }
 
+TQ_OBJECT_CONSTRUCTORS_IMPL(StackFrameInfo)
+NEVER_READ_ONLY_SPACE_IMPL(StackFrameInfo)
+
+BIT_FIELD_ACCESSORS(StackFrameInfo, flags, source_position,
+                    StackFrameInfo::SourcePositionBits)
+BIT_FIELD_ACCESSORS(StackFrameInfo, flags, is_constructor,
+                    StackFrameInfo::IsConstructorBit)
+
 }  // namespace internal
 }  // namespace v8
 

src/objects/debug-objects.h

@@ -203,6 +203,29 @@ class BreakPoint : public TorqueGeneratedBreakPoint<BreakPoint, Struct> {
   TQ_OBJECT_CONSTRUCTORS(BreakPoint)
 };
 
+class StackFrameInfo
+    : public TorqueGeneratedStackFrameInfo<StackFrameInfo, Struct> {
+ public:
+  NEVER_READ_ONLY_SPACE
+
+  // The source position for the stack frame.
+  DECL_INT_ACCESSORS(source_position)
+
+  // Indicates that the frame corresponds to a 'new' invocation.
+  DECL_BOOLEAN_ACCESSORS(is_constructor)
+
+  // Dispatched behavior.
+  DECL_VERIFIER(StackFrameInfo)
+
+  // Bit positions in |flags|.
+  DEFINE_TORQUE_GENERATED_STACK_FRAME_INFO_FLAGS()
+
+  using BodyDescriptor = StructBodyDescriptor;
+
+ private:
+  TQ_OBJECT_CONSTRUCTORS(StackFrameInfo)
+};
+
 }  // namespace internal
 }  // namespace v8
 

src/objects/debug-objects.tq

@@ -67,3 +67,14 @@ extern class CoverageInfo extends HeapObject {
   const slot_count: int32;
   slots[slot_count]: CoverageInfoSlot;
 }
+
+bitfield struct StackFrameInfoFlags extends uint31 {
+  is_constructor: bool: 1 bit;
+  source_position: int32: 30 bit;
+}
+
+extern class StackFrameInfo extends Struct {
+  script: Script;
+  function_name: String|Null;
+  flags: SmiTagged<StackFrameInfoFlags>;
+}

src/objects/objects-definitions.h

@@ -160,6 +160,7 @@ namespace internal {
   V(_, SCRIPT_OR_MODULE_TYPE, ScriptOrModule, script_or_module)                \
   V(_, SOURCE_TEXT_MODULE_INFO_ENTRY_TYPE, SourceTextModuleInfoEntry,          \
     module_info_entry)                                                         \
+  V(_, STACK_FRAME_INFO_TYPE, StackFrameInfo, stack_frame_info)                \
   V(_, TEMPLATE_OBJECT_DESCRIPTION_TYPE, TemplateObjectDescription,            \
     template_object_description)                                               \
   V(_, TUPLE2_TYPE, Tuple2, tuple2)                                            \

src/objects/objects.h

@@ -190,6 +190,7 @@
 //         - SourceTextModule
 //         - SyntheticModule
 //       - SourceTextModuleInfoEntry
+//       - StackFrameInfo
 //     - FeedbackCell
 //     - FeedbackVector
 //     - PreparseData