[SDK] Fixes remaining issues with --lazy-async-stacks on.

- Handle bytecode for e.g. dartkb-simarm64 does not have source position.
- Use existing GetCallerSp() instead of working across frames.
- Nit: Adds clarifying comments to test.
- Nit: Updates name of non-async-stack test to clarify flags used.

Tested:
- CQ with --lazy-async-stacks on by default.
- vm/dart/causal_stacks and language_2/vm/causal_async_exception_stack_test with current flags.

Bug: https://github.com/dart-lang/sdk/issues/39525
Change-Id: Ie6581a734cdcafbd4fb641bd86bffc03ed241532
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/133063
Commit-Queue: Clement Skau <cskau@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>

runtime/tests/vm/dart/causal_stacks/async_throws_stack_no_causal_test.dart

@@ -8,4 +8,4 @@ import 'dart:async';
 
 import 'utils.dart';
 
-Future<void> main(List<String> args) async => await doTestsNoCausal();
+Future<void> main(List<String> args) async => await doTestsNoCausalNoLazy();

runtime/tests/vm/dart/causal_stacks/utils.dart

@@ -591,8 +591,8 @@ Future<void> doTestsCausal() async {
           ]);
 }
 
-// For: --no-causal-async-stacks
-Future<void> doTestsNoCausal() async {
+// For: --no-causal-async-stacks --no-lazy-async-stacks
+Future<void> doTestsNoCausalNoLazy() async {
   final allYieldExpected = const <String>[
     r'^#0      throwSync \(.*/utils.dart:16(:3)?\)$',
     r'^#1      allYield3 \(.*/utils.dart:39(:3)?\)$',
@@ -631,7 +631,7 @@ Future<void> doTestsNoCausal() async {
             r'^#10     doTestAwait ',
             r'^#11     _AsyncAwaitCompleter.start ',
             r'^#12     doTestAwait ',
-            r'^#13     doTestsNoCausal ',
+            r'^#13     doTestsNoCausalNoLazy ',
             r'^#14     _RootZone.runUnary ',
             r'^#15     _FutureListener.handleValue ',
             r'^#16     Future._propagateToListeners.handleValueCallback ',
@@ -682,7 +682,7 @@ Future<void> doTestsNoCausal() async {
             r'^#10     doTestAwaitThen ',
             r'^#11     _AsyncAwaitCompleter.start ',
             r'^#12     doTestAwaitThen ',
-            r'^#13     doTestsNoCausal ',
+            r'^#13     doTestsNoCausalNoLazy ',
             r'^#14     _RootZone.runUnary ',
             r'^#15     _FutureListener.handleValue ',
             r'^#16     Future._propagateToListeners.handleValueCallback ',
@@ -728,7 +728,7 @@ Future<void> doTestsNoCausal() async {
             r'^#10     doTestAwaitCatchError ',
             r'^#11     _AsyncAwaitCompleter.start ',
             r'^#12     doTestAwaitCatchError ',
-            r'^#13     doTestsNoCausal ',
+            r'^#13     doTestsNoCausalNoLazy ',
             r'^#14     _RootZone.runUnary ',
             r'^#15     _FutureListener.handleValue ',
             r'^#16     Future._propagateToListeners.handleValueCallback ',

runtime/vm/stack_frame.h

@@ -132,6 +132,12 @@ class StackFrame : public ValueObject {
 
   static void DumpCurrentTrace();
 
+  uword GetCallerSp() const {
+    return fp() +
+           ((is_interpreted() ? kKBCCallerSpSlotFromFp : kCallerSpSlotFromFp) *
+            kWordSize);
+  }
+
  protected:
   explicit StackFrame(Thread* thread)
       : fp_(0), sp_(0), pc_(0), thread_(thread), is_interpreted_(false) {}
@@ -151,11 +157,6 @@ class StackFrame : public ValueObject {
   RawCode* GetCodeObject() const;
   RawBytecode* GetBytecodeObject() const;
 
-  uword GetCallerSp() const {
-    return fp() +
-           ((is_interpreted() ? kKBCCallerSpSlotFromFp : kCallerSpSlotFromFp) *
-            kWordSize);
-  }
 
   uword GetCallerFp() const {
     return *(reinterpret_cast<uword*>(

runtime/vm/stack_trace.cc

@@ -73,6 +73,9 @@ intptr_t FindPcOffset(const Bytecode& bytecode, intptr_t yield_index) {
   if (yield_index == RawPcDescriptors::kInvalidYieldIndex) {
     return 0;
   }
+  if (!bytecode.HasSourcePositions()) {
+    return 0;
+  }
   intptr_t last_yield_point = 0;
   kernel::BytecodeSourcePositionsIterator iter(Thread::Current()->zone(),
                                                bytecode);
@@ -373,10 +376,9 @@ void StackTraceUtils::CollectFramesLazy(
   CallerClosureFinder caller_closure_finder(zone);
   auto& pc_descs = PcDescriptors::Handle();
 
-  while (frame != nullptr) {
+  for (; frame != nullptr; frame = frames.NextFrame()) {
     if (skip_frames > 0) {
       skip_frames--;
-      frame = frames.NextFrame();
       continue;
     }
 
@@ -385,7 +387,6 @@ void StackTraceUtils::CollectFramesLazy(
       ASSERT(!bytecode.IsNull());
       function = bytecode.function();
       if (function.IsNull()) {
-        frame = frames.NextFrame();
         continue;
       }
       RELEASE_ASSERT(function.raw() == frame->LookupDartFunction());
@@ -419,15 +420,14 @@ void StackTraceUtils::CollectFramesLazy(
 
       // Next, look up caller's closure on the stack and walk backwards through
       // the yields.
-      frame = frames.NextFrame();
-      RawObject** last_caller_obj = reinterpret_cast<RawObject**>(frame->sp());
+      RawObject** last_caller_obj = reinterpret_cast<RawObject**>(
+          frame->GetCallerSp());
       closure = FindClosureInFrame(last_caller_obj, function,
                                    frame->is_interpreted());
 
       // If this async function hasn't yielded yet, we're still dealing with a
       // normal stack. Continue to next frame as usual.
       if (!caller_closure_finder.IsRunningAsync(closure)) {
-        // Don't advance frame since we already did so just above.
         continue;
       }
 
@@ -474,8 +474,6 @@ void StackTraceUtils::CollectFramesLazy(
       // Ignore the rest of the stack; already unwound all async calls.
       return;
     }
-
-    frame = frames.NextFrame();
   }
 
   return;

tests/language_2/vm/causal_async_exception_stack_test.dart

@@ -34,11 +34,11 @@ main() async {
     } catch (e, st) {
       expect(
           h.stringContainsInOrder(st.toString(), [
-            'thrower', '.dart:10', //
-            'generator', '.dart:19', //
-            '<asynchronous suspension>', //
-            'foo', '.dart:23', //
-            'main', //
+            'thrower', '.dart:10', // no auto-format.
+            'generator', '.dart:19', // no auto-format.
+            '<asynchronous suspension>', // no auto-format.
+            'foo', '.dart', // no auto-format.
+            'main',
           ]),
           isTrue);
     }
@@ -72,8 +72,8 @@ main() async {
     } catch (e, st) {
       expect(
           h.stringContainsInOrder(st.toString(), [
-            'thrower', '.dart:10', //
-            'main.<anonymous closure>', '.dart:71', //
+            'thrower', '.dart:10', // no auto-format.
+            'main.<anonymous closure>', '.dart:71', // no auto-format.
           ]),
           isTrue);
     }