[frames] Add convenience function to get the top valid from

.. from a StackTraceFrameIterator (STFI). This replaces the (incorrect)
pattern

 StackTraceFrameIterator it(isolate);
 FrameSummary fs = FrameSummary::GetTop(it.javascript_frame());

The STFI has filtering semantics that only iterate over certain JS and
Wasm frames. These semantics (e.g. skipping over frames that are not
subject to debugging) must be preserved when looking into inlined
optimized frames.

The new convenience function GetTopValidFrame encapsulates this logic.

Bug: chromium:1237730
Change-Id: I060b36b5ac6a5decef90da4de45e679516ff93fd
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3110611Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/main@{#76445}

src/codegen/compiler.cc

@@ -2210,7 +2210,7 @@ MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
       // position, but store it as negative value for lazy translation.
       StackTraceFrameIterator it(isolate);
       if (!it.done() && it.is_javascript()) {
-        FrameSummary summary = FrameSummary::GetTop(it.javascript_frame());
+        FrameSummary summary = it.GetTopValidFrame();
         script->set_eval_from_shared(
             summary.AsJavaScript().function()->shared());
         script->set_origin_options(OriginOptionsForEval(*summary.script()));

src/debug/debug.cc

@@ -2183,8 +2183,7 @@ bool Debug::ShouldBeSkipped() {
   DisableBreak no_recursive_break(this);
 
   StackTraceFrameIterator iterator(isolate_);
-  CommonFrame* frame = iterator.frame();
-  FrameSummary summary = FrameSummary::GetTop(frame);
+  FrameSummary summary = iterator.GetTopValidFrame();
   Handle<Object> script_obj = summary.script();
   if (!script_obj->IsScript()) return false;
 

src/execution/frames.cc

@@ -206,19 +206,42 @@ int StackTraceFrameIterator::FrameFunctionCount() const {
   return static_cast<int>(infos.size());
 }
 
-bool StackTraceFrameIterator::IsValidFrame(StackFrame* frame) const {
+FrameSummary StackTraceFrameIterator::GetTopValidFrame() const {
+  DCHECK(!done());
+  // Like FrameSummary::GetTop, but additionally observes
+  // StackTraceFrameIterator filtering semantics.
+  std::vector<FrameSummary> frames;
+  frame()->Summarize(&frames);
+  if (is_javascript()) {
+    for (int i = static_cast<int>(frames.size()) - 1; i >= 0; i--) {
+      if (!IsValidJSFunction(*frames[i].AsJavaScript().function())) continue;
+      return frames[i];
+    }
+    UNREACHABLE();
+  }
+#if V8_ENABLE_WEBASSEMBLY
+  if (is_wasm()) return frames.back();
+#endif  // V8_ENABLE_WEBASSEMBLY
+  UNREACHABLE();
+}
+
+// static
+bool StackTraceFrameIterator::IsValidFrame(StackFrame* frame) {
   if (frame->is_java_script()) {
-    JavaScriptFrame* js_frame = static_cast<JavaScriptFrame*>(frame);
-    if (!js_frame->function().IsJSFunction()) return false;
-    return js_frame->function().shared().IsSubjectToDebugging();
+    return IsValidJSFunction(static_cast<JavaScriptFrame*>(frame)->function());
   }
-  // Apart from JavaScript frames, only Wasm frames are valid.
 #if V8_ENABLE_WEBASSEMBLY
   if (frame->is_wasm()) return true;
 #endif  // V8_ENABLE_WEBASSEMBLY
   return false;
 }
 
+// static
+bool StackTraceFrameIterator::IsValidJSFunction(JSFunction f) {
+  if (!f.IsJSFunction()) return false;
+  return f.shared().IsSubjectToDebugging();
+}
+
 // -------------------------------------------------------------------------
 
 namespace {

src/execution/frames.h

@@ -1273,9 +1273,14 @@ class V8_EXPORT_PRIVATE StackTraceFrameIterator {
 #endif  // V8_ENABLE_WEBASSEMBLY
   inline JavaScriptFrame* javascript_frame() const;
 
+  // Use this instead of FrameSummary::GetTop(javascript_frame) to keep
+  // filtering behavior consistent with the rest of StackTraceFrameIterator.
+  FrameSummary GetTopValidFrame() const;
+
  private:
   StackFrameIterator iterator_;
-  bool IsValidFrame(StackFrame* frame) const;
+  static bool IsValidFrame(StackFrame* frame);
+  static bool IsValidJSFunction(JSFunction f);
 };
 
 class SafeStackFrameIterator : public StackFrameIteratorBase {

src/execution/isolate.cc

@@ -2148,20 +2148,16 @@ void Isolate::PrintCurrentStackTrace(FILE* out) {
 bool Isolate::ComputeLocation(MessageLocation* target) {
   StackTraceFrameIterator it(this);
   if (it.done()) return false;
-  CommonFrame* frame = it.frame();
   // Compute the location from the function and the relocation info of the
   // baseline code. For optimized code this will use the deoptimization
   // information to get canonical location information.
-  std::vector<FrameSummary> frames;
 #if V8_ENABLE_WEBASSEMBLY
   wasm::WasmCodeRefScope code_ref_scope;
 #endif  // V8_ENABLE_WEBASSEMBLY
-  frame->Summarize(&frames);
-  FrameSummary& summary = frames.back();
+  FrameSummary summary = it.GetTopValidFrame();
   Handle<SharedFunctionInfo> shared;
   Handle<Object> script = summary.script();
-  if (!script->IsScript() ||
-      (Script::cast(*script).source().IsUndefined(this))) {
+  if (!script->IsScript() || Script::cast(*script).source().IsUndefined(this)) {
     return false;
   }