[stack-traces] Don't hold on to code objects from StackFrameInfos.

Previously every `StackFrameInfo` instance would maintain a reference to
an AbstractCode object, which was used to resolve the `code_offset` on
that stack frame. However, it turns out that nowadays this is not
necessary anymore, since all `code_offset`s reported for JavaScript
frames are already bytecode offsets and thus can be resolved by just
looking at the functions' bytecode.

For WebAssembly frames we will also eagerly resolve the `code_offset`
(which is different depending on whether we're looking at Liftoff or
TurboFan code) to the byte offset (relative to the function start) and
stash that away in the `StackFrameInfo`.

For builtin exit frames, the `abstract_code` on the function always
refers to the builtin code object and thus, there's no point in keeping
an extra pointer to it around on the `StackFrameInfo`.

This way the `StackFrameInfo` representation is somewhat uniform, and
more importantly, the `StackFrameInfo` instances will no longer need to
hold to concrete code objects.

Drive-by-fix: Use `FixedArray::SetAndGrow()` when adding to the elements
in the `StackTraceBuilder`.

Also-By: szuend@chromium.org, jarin@chromium.org
Bug: chromium:1258599, chromium:1077657, v8:8742, chromium:1069425
Change-Id: I650e400e0e1acd920281669bdc7b5e1199683ae8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3323073Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78320}

src/execution/isolate.cc

@@ -710,8 +710,6 @@ class StackTraceBuilder {
     if (IsStrictFrame(function)) flags |= StackFrameInfo::kIsStrict;
 
     Handle<Object> receiver(generator_object->receiver(), isolate_);
-    Handle<BytecodeArray> code(function->shared().GetBytecodeArray(isolate_),
-                               isolate_);
     // The stored bytecode offset is relative to a different base than what
     // is used in the source position table, hence the subtraction.
     int offset = Smi::ToInt(generator_object->input_or_debug_pos()) -
@@ -725,7 +723,7 @@ class StackTraceBuilder {
               .internal_formal_parameter_count_without_receiver());
     }
 
-    AppendFrame(receiver, function, code, offset, flags, parameters);
+    AppendFrame(receiver, function, offset, flags, parameters);
   }
 
   void AppendPromiseCombinatorFrame(Handle<JSFunction> element_function,
@@ -736,7 +734,6 @@ class StackTraceBuilder {
 
     Handle<Object> receiver(combinator->native_context().promise_function(),
                             isolate_);
-    Handle<Code> code(combinator->code(), isolate_);
 
     // TODO(mmarchini) save Promises list from the Promise combinator
     Handle<FixedArray> parameters = isolate_->factory()->empty_fixed_array();
@@ -746,7 +743,7 @@ class StackTraceBuilder {
     int promise_index =
         Smi::ToInt(Smi::cast(element_function->GetIdentityHash())) - 1;
 
-    AppendFrame(receiver, combinator, code, promise_index, flags, parameters);
+    AppendFrame(receiver, combinator, promise_index, flags, parameters);
   }
 
   void AppendJavaScriptFrame(
@@ -759,8 +756,8 @@ class StackTraceBuilder {
     if (IsStrictFrame(function)) flags |= StackFrameInfo::kIsStrict;
     if (summary.is_constructor()) flags |= StackFrameInfo::kIsConstructor;
 
-    AppendFrame(summary.receiver(), function, summary.abstract_code(),
-                summary.code_offset(), flags, summary.parameters());
+    AppendFrame(summary.receiver(), function, summary.code_offset(), flags,
+                summary.parameters());
   }
 
 #if V8_ENABLE_WEBASSEMBLY
@@ -775,13 +772,10 @@ class StackTraceBuilder {
       }
     }
 
-    auto code = Managed<wasm::GlobalWasmCodeRef>::Allocate(
-        isolate_, 0, summary.code(),
-        instance->module_object().shared_native_module());
+    int offset = summary.code()->GetSourcePositionBefore(summary.code_offset());
     AppendFrame(instance,
-                handle(Smi::FromInt(summary.function_index()), isolate_), code,
-                summary.code_offset(), flags,
-                isolate_->factory()->empty_fixed_array());
+                handle(Smi::FromInt(summary.function_index()), isolate_),
+                offset, flags, isolate_->factory()->empty_fixed_array());
   }
 #endif  // V8_ENABLE_WEBASSEMBLY
 
@@ -797,9 +791,8 @@ class StackTraceBuilder {
     }
 
     Handle<Object> receiver(exit_frame->receiver(), isolate_);
-    Handle<Code> code(exit_frame->LookupCode(), isolate_);
-    const int offset =
-        code->GetOffsetFromInstructionStart(isolate_, exit_frame->pc());
+    const int offset = exit_frame->LookupCode().GetOffsetFromInstructionStart(
+        isolate_, exit_frame->pc());
 
     int flags = 0;
     if (IsStrictFrame(function)) flags |= StackFrameInfo::kIsStrict;
@@ -814,7 +807,7 @@ class StackTraceBuilder {
       }
     }
 
-    AppendFrame(receiver, function, code, offset, flags, parameters);
+    AppendFrame(receiver, function, offset, flags, parameters);
   }
 
   bool Full() { return index_ >= limit_; }
@@ -883,22 +876,15 @@ class StackTraceBuilder {
   }
 
   void AppendFrame(Handle<Object> receiver_or_instance, Handle<Object> function,
-                   Handle<HeapObject> code, int offset, int flags,
-                   Handle<FixedArray> parameters) {
-    DCHECK_LE(index_, elements_->length());
-    DCHECK_LE(elements_->length(), limit_);
-    if (index_ == elements_->length()) {
-      elements_ = isolate_->factory()->CopyFixedArrayAndGrow(
-          elements_, std::min(16, limit_ - elements_->length()));
-    }
+                   int offset, int flags, Handle<FixedArray> parameters) {
     if (receiver_or_instance->IsTheHole(isolate_)) {
       // TODO(jgruber): Fix all cases in which frames give us a hole value
       // (e.g. the receiver in RegExp constructor frames).
       receiver_or_instance = isolate_->factory()->undefined_value();
     }
     auto info = isolate_->factory()->NewStackFrameInfo(
-        receiver_or_instance, function, code, offset, flags, parameters);
-    elements_->set(index_++, *info);
+        receiver_or_instance, function, offset, flags, parameters);
+    elements_ = FixedArray::SetAndGrow(isolate_, elements_, index_++, info);
   }
 
   Isolate* isolate_;
@@ -1071,10 +1057,6 @@ Handle<FixedArray> CaptureStackTrace(Isolate* isolate, Handle<Object> caller,
   TRACE_EVENT_BEGIN1(TRACE_DISABLED_BY_DEFAULT("v8.stack_trace"),
                      "CaptureStackTrace", "maxFrameCount", options.limit);
 
-#if V8_ENABLE_WEBASSEMBLY
-  wasm::WasmCodeRefScope code_ref_scope;
-#endif  // V8_ENABLE_WEBASSEMBLY
-
   StackTraceBuilder builder(isolate, options.skip_mode, options.limit, caller,
                             options.filter_mode);
 

src/heap/factory.cc

@@ -3335,15 +3335,13 @@ Handle<BreakPoint> Factory::NewBreakPoint(int id, Handle<String> condition) {
 
 Handle<StackFrameInfo> Factory::NewStackFrameInfo(
     Handle<Object> receiver_or_instance, Handle<Object> function,
-    Handle<HeapObject> code_object, int code_offset_or_source_position,
-    int flags, Handle<FixedArray> parameters) {
+    int offset_or_source_position, int flags, Handle<FixedArray> parameters) {
   auto info = NewStructInternal<StackFrameInfo>(STACK_FRAME_INFO_TYPE,
                                                 AllocationType::kYoung);
   DisallowGarbageCollection no_gc;
   info.set_receiver_or_instance(*receiver_or_instance, SKIP_WRITE_BARRIER);
   info.set_function(*function, SKIP_WRITE_BARRIER);
-  info.set_code_object(*code_object, SKIP_WRITE_BARRIER);
-  info.set_code_offset_or_source_position(code_offset_or_source_position);
+  info.set_offset_or_source_position(offset_or_source_position);
   info.set_flags(flags);
   info.set_parameters(*parameters, SKIP_WRITE_BARRIER);
   return handle(info, isolate());

src/heap/factory.h

@@ -397,8 +397,7 @@ class V8_EXPORT_PRIVATE Factory : public FactoryBase<Factory> {
 
   Handle<StackFrameInfo> NewStackFrameInfo(Handle<Object> receiver_or_instance,
                                            Handle<Object> function,
-                                           Handle<HeapObject> code_object,
-                                           int code_offset_or_source_position,
+                                           int offset_or_source_position,
                                            int flags,
                                            Handle<FixedArray> parameters);
 

src/objects/stack-frame-info-inl.h

@@ -32,26 +32,6 @@ BOOL_GETTER(StackFrameInfo, flags, IsStrict, IsStrictBit::kShift)
 BOOL_GETTER(StackFrameInfo, flags, IsConstructor, IsConstructorBit::kShift)
 BOOL_GETTER(StackFrameInfo, flags, IsAsync, IsAsyncBit::kShift)
 
-DEF_GETTER(StackFrameInfo, code_object, HeapObject) {
-  HeapObject value = TorqueGeneratedClass::code_object(cage_base);
-  // The |code_object| field can contain many types of objects, but only CodeT
-  // values have to be converted to Code.
-  if (V8_EXTERNAL_CODE_SPACE_BOOL && value.IsCodeT()) {
-    return FromCodeT(CodeT::cast(value));
-  }
-  return value;
-}
-
-void StackFrameInfo::set_code_object(HeapObject code, WriteBarrierMode mode) {
-  // The |code_object| field can contain many types of objects, but only Code
-  // values have to be converted to CodeT.
-  if (V8_EXTERNAL_CODE_SPACE_BOOL && code.IsCode()) {
-    TorqueGeneratedClass::set_code_object(ToCodeT(Code::cast(code)), mode);
-  } else {
-    TorqueGeneratedClass::set_code_object(code, mode);
-  }
-}
-
 }  // namespace internal
 }  // namespace v8
 

src/objects/stack-frame-info.cc

@@ -481,13 +481,13 @@ Handle<Object> StackFrameInfo::GetWasmModuleName(Handle<StackFrameInfo> info) {
 // static
 int StackFrameInfo::GetSourcePosition(Handle<StackFrameInfo> info) {
   if (info->flags() & kIsSourcePositionComputed) {
-    return info->code_offset_or_source_position();
+    return info->offset_or_source_position();
   }
   DCHECK(!info->IsPromiseAll());
   DCHECK(!info->IsPromiseAny());
   int source_position =
-      ComputeSourcePosition(info, info->code_offset_or_source_position());
-  info->set_code_offset_or_source_position(source_position);
+      ComputeSourcePosition(info, info->offset_or_source_position());
+  info->set_offset_or_source_position(source_position);
   info->set_flags(info->flags() | kIsSourcePositionComputed);
   return source_position;
 }
@@ -516,7 +516,7 @@ bool StackFrameInfo::ComputeLocation(Handle<StackFrameInfo> info,
     int pos = GetSourcePosition(info);
     *location = MessageLocation(script, pos, pos + 1, shared);
   } else {
-    int code_offset = info->code_offset_or_source_position();
+    int code_offset = info->offset_or_source_position();
     *location = MessageLocation(script, shared, code_offset);
   }
   return true;
@@ -528,17 +528,15 @@ int StackFrameInfo::ComputeSourcePosition(Handle<StackFrameInfo> info,
   Isolate* isolate = info->GetIsolate();
 #if V8_ENABLE_WEBASSEMBLY
   if (info->IsWasm()) {
-    auto code_ref = Managed<wasm::GlobalWasmCodeRef>::cast(info->code_object());
-    int byte_offset = code_ref.get()->code()->GetSourcePositionBefore(offset);
     auto module = info->GetWasmInstance().module();
     uint32_t func_index = info->GetWasmFunctionIndex();
-    return wasm::GetSourcePosition(module, func_index, byte_offset,
+    return wasm::GetSourcePosition(module, func_index, offset,
                                    info->IsAsmJsAtNumberConversion());
   }
 #endif  // V8_ENABLE_WEBASSEMBLY
   Handle<SharedFunctionInfo> shared(info->GetSharedFunctionInfo(), isolate);
   SharedFunctionInfo::EnsureSourcePositionsAvailable(isolate, shared);
-  return AbstractCode::cast(info->code_object()).SourcePosition(offset);
+  return shared->abstract_code(isolate).SourcePosition(offset);
 }
 
 base::Optional<Script> StackFrameInfo::GetScript() const {

src/objects/stack-frame-info.tq

@@ -17,8 +17,7 @@ bitfield struct StackFrameInfoFlags extends uint31 {
 extern class StackFrameInfo extends Struct {
   receiver_or_instance: JSAny;
   function: JSFunction|Smi;
-  code_object: HeapObject;
-  code_offset_or_source_position: Smi;
+  offset_or_source_position: Smi;
   flags: SmiTagged<StackFrameInfoFlags>;
   parameters: FixedArray;
 }