From 7558e63725f8f6ecad35e1ae76710308b989c07f Mon Sep 17 00:00:00 2001
From: Tess Strickland <sstrickl@google.com>
Date: Tue, 2 Jun 2026 04:28:41 -0700
Subject: [PATCH] [vm,dyn_modules] Change handling of source positions for
 async returns.

The debugger assumes a null suspend state variable in an asynchronous
function means that the function is still in the prologue prior
to setting up the suspend state. However, the interpreter clears the
suspend state variable before returning, and the debugger needs to be
able to pause before returning when single stepping, so earlier a hack
was added to the debugger that detects being at the direct call of the
async return method and/or the return instruction with a null suspend
state variable.

However, there's a much simpler way of ensuring the debugger pauses
before returning: just emit the source position for the return prior to
clearing the suspend state variable. This also ensures that the debugger
still has access to the function's suspend state when pausing before
the return, instead of waiting until it has been cleared and thus is no
longer accessible.

TEST=ci (should not change the result of any current tests)

Cq-Include-Trybots: luci.dart.try:vm-dyn-linux-debug-x64-try
Change-Id: I401cceb169d8692ac379cdc5a531e07cafbe9a65
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/500740
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
---
 pkg/dart2bytecode/lib/bytecode_generator.dart |  9 ++--
 runtime/vm/debugger.cc                        | 44 +------------------
 2 files changed, 7 insertions(+), 46 deletions(-)

diff --git a/pkg/dart2bytecode/lib/bytecode_generator.dart b/pkg/dart2bytecode/lib/bytecode_generator.dart
index 7dcfe40134e..bef3a778a5b 100644
--- a/pkg/dart2bytecode/lib/bytecode_generator.dart
+++ b/pkg/dart2bytecode/lib/bytecode_generator.dart
@@ -1459,9 +1459,12 @@ class BytecodeGenerator extends RecursiveVisitor {
           break;
       }
       if (returnMethod != null) {
-        // Unlike other async machinery, this can't be marked synthetic
-        // as the method may return directly from the direct call and so
-        // the debugger needs to pause at it, not the following return.
+        // Before returning, the suspend state variable is cleared, and
+        // the debugger assumes a null suspend state variable in an asynchronous
+        // function means that it is still in the function prologue. Emit
+        // an appropriate source position first so the debugger pauses prior
+        // to returning without requiring special detection of the call/return.
+        asm.emitSourcePosition();
         asm.emitPopLocal(locals.returnVarIndexInFrame);
         asm.emitPush(locals.suspendStateVarIndexInFrame);
         asm.emitPush(locals.returnVarIndexInFrame);
diff --git a/runtime/vm/debugger.cc b/runtime/vm/debugger.cc
index 59b622ca2f7..e8e80e2e794 100644
--- a/runtime/vm/debugger.cc
+++ b/runtime/vm/debugger.cc
@@ -3998,45 +3998,6 @@ static bool IsAtAsyncJump(ActivationFrame* top_frame) {
   return false;
 }
 
-static bool IsAtBytecodeAsyncReturn(ActivationFrame* top_frame) {
-#if defined(DART_DYNAMIC_MODULES)
-  if (!top_frame->IsInterpreted()) return false;
-  const auto& bytecode = top_frame->bytecode();
-  const uword return_address = top_frame->pc();
-  const uword prev = bytecode.GetInstructionBefore(return_address);
-  if (prev == 0) {
-    const uword start = bytecode.PayloadStart();
-    // Async awaiter frames have an PC offset of 0 or 1.
-    ASSERT(return_address == start ||
-           return_address == start + StackTraceUtils::kFutureListenerPcOffset);
-    return false;
-  }
-  auto* const instr = reinterpret_cast<const KBCInstr*>(prev);
-  // Async returns in bytecode are implemented via direct calls to
-  // the appropriate Dart async return method. Note that unlike other async
-  // machinery, the direct call to _returnAsync is not marked synthetic.
-  if (!KernelBytecode::IsDirectCallOpcode(instr)) {
-    return false;
-  }
-  auto const index = KernelBytecode::DecodeD(instr);
-  auto* const thread = Thread::Current();
-  Zone* const zone = thread->zone();
-  const auto& pool = ObjectPool::Handle(zone, bytecode.object_pool());
-  const auto& obj = Object::Handle(zone, pool.ObjectAt(index));
-  if (obj.IsNull() || !obj.IsFunction()) {
-    return false;
-  }
-  const auto& target = Function::Cast(obj);
-  auto* const object_store = thread->isolate_group()->object_store();
-  return target.ptr() == object_store->suspend_state_return_async() ||
-         target.ptr() ==
-             object_store->suspend_state_return_async_not_future() ||
-         target.ptr() == object_store->suspend_state_return_async_star();
-#else
-  return false;
-#endif
-}
-
 #if defined(DART_DYNAMIC_MODULES)
 static ActivationFrame::Relation CompareTopDartFrameTo(uword other_fp,
                                                        bool is_interpreted) {
@@ -4130,10 +4091,7 @@ ErrorPtr Debugger::PauseStepping() {
   // with regular function wrt the first stop in the function prologue.
   if ((frame->function().IsAsyncFunction() ||
        frame->function().IsAsyncGenerator()) &&
-      frame->GetSuspendStateVar() == Object::null() &&
-      // The bytecode generator sets the suspend state var to null prior
-      // to returning.
-      !IsAtBytecodeAsyncReturn(frame)) {
+      frame->GetSuspendStateVar() == Object::null()) {
     return Error::null();
   }