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

This reverts commit 6b1fb003.

Reason for revert: breaks gc stress bots - https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux%20-%20gc%20stress/36626/overview

Original change's description:
> [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/+/3323073
> Reviewed-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}

Bug: chromium:1258599, chromium:1077657, v8:8742, chromium:1069425
Change-Id: I20643ad8f0c383b754841fc52f9b3447b004c9d0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3327141
Auto-Submit: Maya Lekova <mslekova@chromium.org>
Owners-Override: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#78323}

src/execution/isolate.cc

@@ -710,6 +710,8 @@ 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()) -
@@ -723,7 +725,7 @@ class StackTraceBuilder {
               .internal_formal_parameter_count_without_receiver());
     }
 
-    AppendFrame(receiver, function, offset, flags, parameters);
+    AppendFrame(receiver, function, code, offset, flags, parameters);
   }
 
   void AppendPromiseCombinatorFrame(Handle<JSFunction> element_function,
@@ -734,6 +736,7 @@ 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();
@@ -743,7 +746,7 @@ class StackTraceBuilder {
     int promise_index =
         Smi::ToInt(Smi::cast(element_function->GetIdentityHash())) - 1;
 
-    AppendFrame(receiver, combinator, promise_index, flags, parameters);
+    AppendFrame(receiver, combinator, code, promise_index, flags, parameters);
   }
 
   void AppendJavaScriptFrame(
@@ -756,8 +759,8 @@ class StackTraceBuilder {
     if (IsStrictFrame(function)) flags |= StackFrameInfo::kIsStrict;
     if (summary.is_constructor()) flags |= StackFrameInfo::kIsConstructor;
 
-    AppendFrame(summary.receiver(), function, summary.code_offset(), flags,
-                summary.parameters());
+    AppendFrame(summary.receiver(), function, summary.abstract_code(),
+                summary.code_offset(), flags, summary.parameters());
   }
 
 #if V8_ENABLE_WEBASSEMBLY
@@ -772,10 +775,13 @@ class StackTraceBuilder {
       }
     }
 
-    int offset = summary.code()->GetSourcePositionBefore(summary.code_offset());
+    auto code = Managed<wasm::GlobalWasmCodeRef>::Allocate(
+        isolate_, 0, summary.code(),
+        instance->module_object().shared_native_module());
     AppendFrame(instance,
-                handle(Smi::FromInt(summary.function_index()), isolate_),
-                offset, flags, isolate_->factory()->empty_fixed_array());
+                handle(Smi::FromInt(summary.function_index()), isolate_), code,
+                summary.code_offset(), flags,
+                isolate_->factory()->empty_fixed_array());
   }
 #endif  // V8_ENABLE_WEBASSEMBLY
 
@@ -791,8 +797,9 @@ class StackTraceBuilder {
     }
 
     Handle<Object> receiver(exit_frame->receiver(), isolate_);
-    const int offset = exit_frame->LookupCode().GetOffsetFromInstructionStart(
-        isolate_, exit_frame->pc());
+    Handle<Code> code(exit_frame->LookupCode(), isolate_);
+    const int offset =
+        code->GetOffsetFromInstructionStart(isolate_, exit_frame->pc());
 
     int flags = 0;
     if (IsStrictFrame(function)) flags |= StackFrameInfo::kIsStrict;
@@ -807,7 +814,7 @@ class StackTraceBuilder {
       }
     }
 
-    AppendFrame(receiver, function, offset, flags, parameters);
+    AppendFrame(receiver, function, code, offset, flags, parameters);
   }
 
   bool Full() { return index_ >= limit_; }
@@ -876,15 +883,22 @@ class StackTraceBuilder {
   }
 
   void AppendFrame(Handle<Object> receiver_or_instance, Handle<Object> function,
-                   int offset, int flags, Handle<FixedArray> parameters) {
+                   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()));
+    }
     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, offset, flags, parameters);
-    elements_ = FixedArray::SetAndGrow(isolate_, elements_, index_++, info);
+        receiver_or_instance, function, code, offset, flags, parameters);
+    elements_->set(index_++, *info);
   }
 
   Isolate* isolate_;
@@ -1057,6 +1071,10 @@ 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,13 +3335,15 @@ Handle<BreakPoint> Factory::NewBreakPoint(int id, Handle<String> condition) {
 
 Handle<StackFrameInfo> Factory::NewStackFrameInfo(
     Handle<Object> receiver_or_instance, Handle<Object> function,
-    int offset_or_source_position, int flags, Handle<FixedArray> parameters) {
+    Handle<HeapObject> code_object, int code_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_offset_or_source_position(offset_or_source_position);
+  info.set_code_object(*code_object, SKIP_WRITE_BARRIER);
+  info.set_code_offset_or_source_position(code_offset_or_source_position);
   info.set_flags(flags);
   info.set_parameters(*parameters, SKIP_WRITE_BARRIER);
   return handle(info, isolate());

src/heap/factory.h

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

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

@@ -32,6 +32,26 @@ 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->offset_or_source_position();
+    return info->code_offset_or_source_position();
   }
   DCHECK(!info->IsPromiseAll());
   DCHECK(!info->IsPromiseAny());
   int source_position =
-      ComputeSourcePosition(info, info->offset_or_source_position());
-  info->set_offset_or_source_position(source_position);
+      ComputeSourcePosition(info, info->code_offset_or_source_position());
+  info->set_code_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->offset_or_source_position();
+    int code_offset = info->code_offset_or_source_position();
     *location = MessageLocation(script, shared, code_offset);
   }
   return true;
@@ -528,15 +528,17 @@ 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, offset,
+    return wasm::GetSourcePosition(module, func_index, byte_offset,
                                    info->IsAsmJsAtNumberConversion());
   }
 #endif  // V8_ENABLE_WEBASSEMBLY
   Handle<SharedFunctionInfo> shared(info->GetSharedFunctionInfo(), isolate);
   SharedFunctionInfo::EnsureSourcePositionsAvailable(isolate, shared);
-  return shared->abstract_code(isolate).SourcePosition(offset);
+  return AbstractCode::cast(info->code_object()).SourcePosition(offset);
 }
 
 base::Optional<Script> StackFrameInfo::GetScript() const {

src/objects/stack-frame-info.tq

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