[stack-trace] Always return lazy wrapper objects from CaptureStackTrace

This CL is the starting point to convert all FrameArray users to
use StackTraceFrame objects instead.

Bug: v8:8742
Change-Id: I7bd0081dfd428e9914dedebd5065ac262aacec0b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1627332Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62076}

src/execution/isolate.cc

@@ -737,15 +737,19 @@ class FrameArrayBuilder {
   }
 
   // Creates a StackTraceFrame object for each frame in the FrameArray.
-  Handle<FixedArray> GetElementsAsStackTraceFrameArray() {
+  Handle<FixedArray> GetElementsAsStackTraceFrameArray(
+      bool enable_frame_caching) {
     elements_->ShrinkToFit(isolate_);
     const int frame_count = elements_->FrameCount();
     Handle<FixedArray> stack_trace =
         isolate_->factory()->NewFixedArray(frame_count);
 
     for (int i = 0; i < frame_count; ++i) {
-      // Caching stack frames only happens for non-Wasm frames.
-      if (!elements_->IsAnyWasmFrame(i)) {
+      // Caching stack frames only happens for user JS frames.
+      const bool cache_frame =
+          enable_frame_caching && !elements_->IsAnyWasmFrame(i) &&
+          elements_->Function(i).shared().IsUserJavaScript();
+      if (cache_frame) {
         MaybeHandle<StackTraceFrame> maybe_frame =
             StackFrameCacheHelper::LookupCachedFrame(
                 isolate_, handle(elements_->Code(i), isolate_),
@@ -761,7 +765,7 @@ class FrameArrayBuilder {
           isolate_->factory()->NewStackTraceFrame(elements_, i);
       stack_trace->set(i, *frame);
 
-      if (!elements_->IsAnyWasmFrame(i)) {
+      if (cache_frame) {
         StackFrameCacheHelper::CacheFrameAndUpdateCache(
             isolate_, handle(elements_->Code(i), isolate_),
             Smi::ToInt(elements_->Offset(i)), frame);
@@ -974,9 +978,7 @@ struct CaptureStackTraceOptions {
   bool capture_builtin_exit_frames;
   bool capture_only_frames_subject_to_debugging;
   bool async_stack_trace;
-
-  enum CaptureResult { RAW_FRAME_ARRAY, STACK_TRACE_FRAME_ARRAY };
-  CaptureResult capture_result;
+  bool enable_frame_caching;
 };
 
 Handle<Object> CaptureStackTrace(Isolate* isolate, Handle<Object> caller,
@@ -1106,10 +1108,8 @@ Handle<Object> CaptureStackTrace(Isolate* isolate, Handle<Object> caller,
   }
 
   // TODO(yangguo): Queue this structured stack trace for preprocessing on GC.
-  if (options.capture_result == CaptureStackTraceOptions::RAW_FRAME_ARRAY) {
-    return builder.GetElements();
-  }
-  return builder.GetElementsAsStackTraceFrameArray();
+  return builder.GetElementsAsStackTraceFrameArray(
+      options.enable_frame_caching);
 }
 
 }  // namespace
@@ -1127,7 +1127,7 @@ Handle<Object> Isolate::CaptureSimpleStackTrace(Handle<JSReceiver> error_object,
   options.async_stack_trace = FLAG_async_stack_traces;
   options.filter_mode = FrameArrayBuilder::CURRENT_SECURITY_CONTEXT;
   options.capture_only_frames_subject_to_debugging = false;
-  options.capture_result = CaptureStackTraceOptions::RAW_FRAME_ARRAY;
+  options.enable_frame_caching = false;
 
   return CaptureStackTrace(this, caller, options);
 }
@@ -1223,7 +1223,7 @@ Handle<FixedArray> Isolate::CaptureCurrentStackTrace(
           ? FrameArrayBuilder::ALL
           : FrameArrayBuilder::CURRENT_SECURITY_CONTEXT;
   options.capture_only_frames_subject_to_debugging = true;
-  options.capture_result = CaptureStackTraceOptions::STACK_TRACE_FRAME_ARRAY;
+  options.enable_frame_caching = true;
 
   return Handle<FixedArray>::cast(
       CaptureStackTrace(this, factory()->undefined_value(), options));
@@ -2114,7 +2114,8 @@ bool Isolate::ComputeLocationFromStackTrace(MessageLocation* target,
       JSReceiver::GetDataProperty(Handle<JSObject>::cast(exception), key);
   if (!property->IsFixedArray()) return false;
 
-  Handle<FrameArray> elements = Handle<FrameArray>::cast(property);
+  Handle<FrameArray> elements =
+      GetFrameArrayFromStackTrace(this, Handle<FixedArray>::cast(property));
 
   const int frame_count = elements->FrameCount();
   for (int i = 0; i < frame_count; i++) {

src/execution/messages.cc

@@ -14,6 +14,7 @@
 #include "src/objects/frame-array-inl.h"
 #include "src/objects/js-array-inl.h"
 #include "src/objects/keys.h"
+#include "src/objects/stack-frame-info-inl.h"
 #include "src/objects/struct-inl.h"
 #include "src/strings/string-builder-inl.h"
 #include "src/wasm/wasm-code-manager.h"
@@ -905,8 +906,7 @@ StackFrameBase* FrameArrayIterator::Frame() {
 namespace {
 
 MaybeHandle<Object> ConstructCallSite(Isolate* isolate,
-                                      Handle<FrameArray> frame_array,
-                                      int frame_index) {
+                                      Handle<StackTraceFrame> frame) {
   Handle<JSFunction> target =
       handle(isolate->native_context()->callsite_function(), isolate);
 
@@ -915,6 +915,14 @@ MaybeHandle<Object> ConstructCallSite(Isolate* isolate,
       isolate, obj,
       JSObject::New(target, target, Handle<AllocationSite>::null()), Object);
 
+  // TODO(szuend): Introduce a new symbol "call_site_frame_symbol" and set
+  //               it to the StackTraceFrame. The CallSite API builtins can then
+  //               be implemented using StackFrameInfo objects.
+
+  Handle<FrameArray> frame_array(FrameArray::cast(frame->frame_array()),
+                                 isolate);
+  int frame_index = frame->frame_index();
+
   Handle<Symbol> key = isolate->factory()->call_site_frame_array_symbol();
   RETURN_ON_EXCEPTION(isolate,
                       JSObject::SetOwnPropertyIgnoreAttributes(
@@ -934,14 +942,16 @@ MaybeHandle<Object> ConstructCallSite(Isolate* isolate,
 // Convert the raw frames as written by Isolate::CaptureSimpleStackTrace into
 // a JSArray of JSCallSite objects.
 MaybeHandle<JSArray> GetStackFrames(Isolate* isolate,
-                                    Handle<FrameArray> elems) {
-  const int frame_count = elems->FrameCount();
+                                    Handle<FixedArray> elems) {
+  const int frame_count = elems->length();
 
   Handle<FixedArray> frames = isolate->factory()->NewFixedArray(frame_count);
   for (int i = 0; i < frame_count; i++) {
     Handle<Object> site;
-    ASSIGN_RETURN_ON_EXCEPTION(isolate, site,
-                               ConstructCallSite(isolate, elems, i), JSArray);
+    Handle<StackTraceFrame> frame(StackTraceFrame::cast(elems->get(i)),
+                                  isolate);
+    ASSIGN_RETURN_ON_EXCEPTION(isolate, site, ConstructCallSite(isolate, frame),
+                               JSArray);
     frames->set(i, *site);
   }
 
@@ -1004,7 +1014,7 @@ MaybeHandle<Object> ErrorUtils::FormatStackTrace(Isolate* isolate,
                                                  Handle<JSObject> error,
                                                  Handle<Object> raw_stack) {
   DCHECK(raw_stack->IsFixedArray());
-  Handle<FrameArray> elems = Handle<FrameArray>::cast(raw_stack);
+  Handle<FixedArray> elems = Handle<FixedArray>::cast(raw_stack);
 
   const bool in_recursion = isolate->formatting_stack_trace();
   if (!in_recursion) {
@@ -1071,7 +1081,9 @@ MaybeHandle<Object> ErrorUtils::FormatStackTrace(Isolate* isolate,
 
   wasm::WasmCodeRefScope wasm_code_ref_scope;
 
-  for (FrameArrayIterator it(isolate, elems); it.HasFrame(); it.Advance()) {
+  Handle<FrameArray> frame_array = GetFrameArrayFromStackTrace(isolate, elems);
+  for (FrameArrayIterator it(isolate, frame_array); it.HasFrame();
+       it.Advance()) {
     builder.AppendCString("\n    at ");
 
     StackFrameBase* frame = it.Frame();

src/objects/stack-frame-info.cc

@@ -137,5 +137,20 @@ void StackTraceFrame::InitializeFrameInfo(Handle<StackTraceFrame> frame) {
   frame->set_frame_index(-1);
 }
 
+Handle<FrameArray> GetFrameArrayFromStackTrace(Isolate* isolate,
+                                               Handle<FixedArray> stack_trace) {
+  // For the empty case, a empty FrameArray needs to be allocated so the rest
+  // of the code doesn't has to be special cased everywhere.
+  if (stack_trace->length() == 0) {
+    return isolate->factory()->NewFrameArray(0);
+  }
+
+  // Retrieve the FrameArray from the first StackTraceFrame.
+  DCHECK_GT(stack_trace->length(), 0);
+  Handle<StackTraceFrame> frame(StackTraceFrame::cast(stack_trace->get(0)),
+                                isolate);
+  return handle(FrameArray::cast(frame->frame_array()), isolate);
+}
+
 }  // namespace internal
 }  // namespace v8

src/objects/stack-frame-info.h

@@ -113,6 +113,14 @@ class StackTraceFrame : public Struct {
   static void InitializeFrameInfo(Handle<StackTraceFrame> frame);
 };
 
+// Small helper that retrieves the FrameArray from a stack-trace
+// consisting of a FixedArray of StackTraceFrame objects.
+// This helper is only temporary until all FrameArray use-sites have
+// been converted to use StackTraceFrame and StackFrameInfo objects.
+V8_EXPORT_PRIVATE
+Handle<FrameArray> GetFrameArrayFromStackTrace(Isolate* isolate,
+                                               Handle<FixedArray> stack_trace);
+
 }  // namespace internal
 }  // namespace v8
 

test/cctest/heap/test-heap.cc

@@ -3444,10 +3444,10 @@ void DetailedErrorStackTraceTest(const char* src,
   Isolate* isolate = CcTest::i_isolate();
   Handle<Name> key = isolate->factory()->stack_trace_symbol();
 
-  Handle<FrameArray> stack_trace(Handle<FrameArray>::cast(
+  Handle<FixedArray> stack_trace(Handle<FixedArray>::cast(
       Object::GetProperty(isolate, exception, key).ToHandleChecked()));
 
-  test(stack_trace);
+  test(GetFrameArrayFromStackTrace(isolate, stack_trace));
 }
 
 // * Test interpreted function error