[refactor] Introduce VisitStack choke point.

This decouples the stack trace visitation logic from the creation
of actual stack frame objects, in preparation to introduce a
second kind of stack frame object (`v8::internal::StackFrameInfo`
as part of http://crrev.com/c/3302794) in addition to the existing
`v8::internal::CallSiteInfo`.

Doc: https://bit.ly/v8-stack-frame
Bug: chromium:1258599, chromium:1278647, chromium:1278650
Change-Id: I398933653e29cc2fe5c222526d9dd686ef8239b4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3334781
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78374}

src/execution/isolate.cc

@@ -44,6 +44,7 @@
 #include "src/diagnostics/basic-block-profiler.h"
 #include "src/diagnostics/compilation-statistics.h"
 #include "src/execution/frames-inl.h"
+#include "src/execution/frames.h"
 #include "src/execution/isolate-inl.h"
 #include "src/execution/local-isolate.h"
 #include "src/execution/messages.h"
@@ -683,11 +684,15 @@ StackTraceFailureMessage::StackTraceFailureMessage(
   }
 }
 
-class StackTraceBuilder {
+bool NoExtension(const v8::FunctionCallbackInfo<v8::Value>&) { return false; }
+
+namespace {
+
+class CallSiteBuilder {
  public:
   enum FrameFilterMode { ALL, CURRENT_SECURITY_CONTEXT };
 
-  StackTraceBuilder(Isolate* isolate, FrameSkipMode mode, int limit,
+  CallSiteBuilder(Isolate* isolate, FrameSkipMode mode, int limit,
                   Handle<Object> caller, FrameFilterMode filter_mode)
       : isolate_(isolate),
         mode_(mode),
@@ -703,6 +708,18 @@ class StackTraceBuilder {
     elements_ = isolate->factory()->NewFixedArray(std::min(64, limit));
   }
 
+  bool Visit(FrameSummary const& summary) {
+    if (Full()) return false;
+#if V8_ENABLE_WEBASSEMBLY
+    if (summary.IsWasm()) {
+      AppendWasmFrame(summary.AsWasm());
+      return true;
+    }
+#endif  // V8_ENABLE_WEBASSEMBLY
+    AppendJavaScriptFrame(summary.AsJavaScript());
+    return true;
+  }
+
   void AppendAsyncFrame(Handle<JSGeneratorObject> generator_object) {
     Handle<JSFunction> function(generator_object->function(), isolate_);
     if (!IsVisibleInStackTrace(function)) return;
@@ -905,10 +922,6 @@ bool GetStackTraceLimit(Isolate* isolate, int* result) {
   return true;
 }
 
-bool NoExtension(const v8::FunctionCallbackInfo<v8::Value>&) { return false; }
-
-namespace {
-
 bool IsBuiltinFunction(Isolate* isolate, HeapObject object, Builtin builtin) {
   if (!object.IsJSFunction()) return false;
   JSFunction const function = JSFunction::cast(object);
@@ -916,7 +929,7 @@ bool IsBuiltinFunction(Isolate* isolate, HeapObject object, Builtin builtin) {
 }
 
 void CaptureAsyncStackTrace(Isolate* isolate, Handle<JSPromise> promise,
-                            StackTraceBuilder* builder) {
+                            CallSiteBuilder* builder) {
   while (!builder->Full()) {
     // Check that the {promise} is not settled.
     if (promise->status() != Promise::kPending) return;
@@ -1026,89 +1039,7 @@ void CaptureAsyncStackTrace(Isolate* isolate, Handle<JSPromise> promise,
   }
 }
 
-struct CaptureStackTraceOptions {
-  int limit;
-  // 'filter_mode' and 'skip_mode' are somewhat orthogonal. 'filter_mode'
-  // specifies whether to capture all frames, or just frames in the same
-  // security context. While 'skip_mode' allows skipping the first frame.
-  FrameSkipMode skip_mode;
-  StackTraceBuilder::FrameFilterMode filter_mode;
-
-  bool capture_only_frames_subject_to_debugging;
-  bool async_stack_trace;
-};
-
-Handle<FixedArray> CaptureStackTrace(Isolate* isolate, Handle<Object> caller,
-                                     CaptureStackTraceOptions options) {
-  DisallowJavascriptExecution no_js(isolate);
-
-  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);
-
-  // Build the regular stack trace, and remember the last relevant
-  // frame ID and inlined index (for the async stack trace handling
-  // below, which starts from this last frame).
-  for (StackFrameIterator it(isolate); !it.done() && !builder.Full();
-       it.Advance()) {
-    StackFrame* const frame = it.frame();
-    switch (frame->type()) {
-      case StackFrame::BUILTIN_EXIT:
-      case StackFrame::JAVA_SCRIPT_BUILTIN_CONTINUATION:
-      case StackFrame::JAVA_SCRIPT_BUILTIN_CONTINUATION_WITH_CATCH:
-      case StackFrame::OPTIMIZED:
-      case StackFrame::INTERPRETED:
-      case StackFrame::BASELINE:
-      case StackFrame::BUILTIN:
-#if V8_ENABLE_WEBASSEMBLY
-      case StackFrame::WASM:
-#endif  // V8_ENABLE_WEBASSEMBLY
-      {
-        // A standard frame may include many summarized frames (due to
-        // inlining).
-        std::vector<FrameSummary> frames;
-        CommonFrame::cast(frame)->Summarize(&frames);
-        for (size_t i = frames.size(); i-- != 0 && !builder.Full();) {
-          auto& summary = frames[i];
-          if (options.capture_only_frames_subject_to_debugging &&
-              !summary.is_subject_to_debugging()) {
-            continue;
-          }
-
-          if (summary.IsJavaScript()) {
-            //=========================================================
-            // Handle a JavaScript frame.
-            //=========================================================
-            auto const& java_script = summary.AsJavaScript();
-            builder.AppendJavaScriptFrame(java_script);
-#if V8_ENABLE_WEBASSEMBLY
-          } else if (summary.IsWasm()) {
-            //=========================================================
-            // Handle a Wasm frame.
-            //=========================================================
-            auto const& wasm = summary.AsWasm();
-            builder.AppendWasmFrame(wasm);
-#endif  // V8_ENABLE_WEBASSEMBLY
-          }
-        }
-        break;
-      }
-
-      default:
-        break;
-    }
-  }
-
-  // If --async-stack-traces are enabled and the "current microtask" is a
-  // PromiseReactionJobTask, we try to enrich the stack trace with async
-  // frames.
-  if (options.async_stack_trace) {
+void CaptureAsyncStackTrace(Isolate* isolate, CallSiteBuilder* builder) {
   Handle<Object> current_microtask = isolate->factory()->current_microtask();
   if (current_microtask->IsPromiseReactionJobTask()) {
     Handle<PromiseReactionJobTask> promise_reaction_job_task =
@@ -1136,9 +1067,8 @@ Handle<FixedArray> CaptureStackTrace(Isolate* isolate, Handle<Object> caller,
         if (generator_object->IsJSAsyncFunctionObject()) {
           Handle<JSAsyncFunctionObject> async_function_object =
               Handle<JSAsyncFunctionObject>::cast(generator_object);
-            Handle<JSPromise> promise(async_function_object->promise(),
-                                      isolate);
-            CaptureAsyncStackTrace(isolate, promise, &builder);
+          Handle<JSPromise> promise(async_function_object->promise(), isolate);
+          CaptureAsyncStackTrace(isolate, promise, builder);
         } else {
           Handle<JSAsyncGeneratorObject> async_generator_object =
               Handle<JSAsyncGeneratorObject>::cast(generator_object);
@@ -1148,7 +1078,7 @@ Handle<FixedArray> CaptureStackTrace(Isolate* isolate, Handle<Object> caller,
                 Handle<AsyncGeneratorRequest>::cast(queue);
             Handle<JSPromise> promise(
                 JSPromise::cast(async_generator_request->promise()), isolate);
-              CaptureAsyncStackTrace(isolate, promise, &builder);
+            CaptureAsyncStackTrace(isolate, promise, builder);
           }
         }
       }
@@ -1162,10 +1092,82 @@ Handle<FixedArray> CaptureStackTrace(Isolate* isolate, Handle<Object> caller,
       if (promise_or_capability->IsJSPromise()) {
         Handle<JSPromise> promise =
             Handle<JSPromise>::cast(promise_or_capability);
-          CaptureAsyncStackTrace(isolate, promise, &builder);
+        CaptureAsyncStackTrace(isolate, promise, builder);
+      }
+    }
+  }
+}
+
+template <typename Visitor, typename Options>
+void VisitStack(Isolate* isolate, Visitor* visitor, Options const& options) {
+  DisallowJavascriptExecution no_js(isolate);
+  for (StackFrameIterator it(isolate); !it.done(); it.Advance()) {
+    StackFrame* frame = it.frame();
+    switch (frame->type()) {
+      case StackFrame::BUILTIN_EXIT:
+      case StackFrame::JAVA_SCRIPT_BUILTIN_CONTINUATION:
+      case StackFrame::JAVA_SCRIPT_BUILTIN_CONTINUATION_WITH_CATCH:
+      case StackFrame::OPTIMIZED:
+      case StackFrame::INTERPRETED:
+      case StackFrame::BASELINE:
+      case StackFrame::BUILTIN:
+#if V8_ENABLE_WEBASSEMBLY
+      case StackFrame::WASM:
+#endif  // V8_ENABLE_WEBASSEMBLY
+      {
+        // A standard frame may include many summarized frames (due to
+        // inlining).
+        std::vector<FrameSummary> summaries;
+        CommonFrame::cast(frame)->Summarize(&summaries);
+        for (auto rit = summaries.rbegin(); rit != summaries.rend(); ++rit) {
+          FrameSummary& summary = *rit;
+          if (options.capture_only_frames_subject_to_debugging &&
+              !summary.is_subject_to_debugging()) {
+            continue;
+          }
+          if (!visitor->Visit(summary)) return;
+        }
+        break;
       }
+
+      default:
+        break;
     }
   }
+}
+
+struct CaptureStackTraceOptions {
+  int limit;
+  // 'filter_mode' and 'skip_mode' are somewhat orthogonal. 'filter_mode'
+  // specifies whether to capture all frames, or just frames in the same
+  // security context. While 'skip_mode' allows skipping the first frame.
+  FrameSkipMode skip_mode;
+  CallSiteBuilder::FrameFilterMode filter_mode;
+
+  bool capture_only_frames_subject_to_debugging;
+  bool async_stack_trace;
+};
+
+Handle<FixedArray> CaptureStackTrace(Isolate* isolate, Handle<Object> caller,
+                                     CaptureStackTraceOptions options) {
+  DisallowJavascriptExecution no_js(isolate);
+
+  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
+
+  CallSiteBuilder builder(isolate, options.skip_mode, options.limit, caller,
+                          options.filter_mode);
+  VisitStack(isolate, &builder, options);
+
+  // If --async-stack-traces are enabled and the "current microtask" is a
+  // PromiseReactionJobTask, we try to enrich the stack trace with async
+  // frames.
+  if (options.async_stack_trace) {
+    CaptureAsyncStackTrace(isolate, &builder);
   }
 
   Handle<FixedArray> stack_trace = builder.Build();
@@ -1189,7 +1191,7 @@ Handle<Object> Isolate::CaptureSimpleStackTrace(Handle<JSReceiver> error_object,
   options.limit = limit;
   options.skip_mode = mode;
   options.async_stack_trace = FLAG_async_stack_traces;
-  options.filter_mode = StackTraceBuilder::CURRENT_SECURITY_CONTEXT;
+  options.filter_mode = CallSiteBuilder::CURRENT_SECURITY_CONTEXT;
   options.capture_only_frames_subject_to_debugging = false;
 
   return CaptureStackTrace(this, caller, options);
@@ -1289,8 +1291,8 @@ Handle<FixedArray> Isolate::CaptureDetailedStackTrace(
   options.async_stack_trace = false;
   options.filter_mode =
       (stack_trace_options & StackTrace::kExposeFramesAcrossSecurityOrigins)
-          ? StackTraceBuilder::ALL
-          : StackTraceBuilder::CURRENT_SECURITY_CONTEXT;
+          ? CallSiteBuilder::ALL
+          : CallSiteBuilder::CURRENT_SECURITY_CONTEXT;
   options.capture_only_frames_subject_to_debugging = true;
 
   return CaptureStackTrace(this, factory()->undefined_value(), options);
@@ -2203,7 +2205,7 @@ void Isolate::PrintCurrentStackTrace(std::ostream& out) {
   options.limit = 0;
   options.skip_mode = SKIP_NONE;
   options.async_stack_trace = FLAG_async_stack_traces;
-  options.filter_mode = StackTraceBuilder::CURRENT_SECURITY_CONTEXT;
+  options.filter_mode = CallSiteBuilder::CURRENT_SECURITY_CONTEXT;
   options.capture_only_frames_subject_to_debugging = false;
 
   Handle<FixedArray> frames =