[vm/async] Mark _AsyncAwaitCompleter.start and it's parent function non-visible.

This also removes a hack in StackTrace::ToString() which was skipping
over the parent function (though this was handled only in some cases).

Issue https://github.com/dart-lang/sdk/issues/37668

Change-Id: Ic4232fdd05c998e2c6843339d77a75cbad2aaffd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/135682
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Clement Skau <cskau@google.com>

runtime/observatory/tests/service/positive_token_pos_test.dart

@@ -20,26 +20,25 @@ testMain() {
 
 var tests = <IsolateTest>[
   hasStoppedAtBreakpoint,
-  markDartColonLibrariesDebuggable,
   stoppedAtLine(18),
   stepInto,
   (Isolate isolate) async {
     ServiceMap stack = await isolate.getStack();
-    expect(stack['frames'].length, greaterThan(3));
+    expect(stack['frames'].length, greaterThan(2));
-
-    var frame = stack['frames'][0];
-    expect(frame.function.name, equals('_AsyncAwaitCompleter'));
-    expect(await frame.location.getLine(), greaterThan(0));
-    expect(await frame.location.getColumn(), greaterThan(0));
 
     // We used to return a negative token position for this frame.
     // See issue #27128.
-    frame = stack['frames'][1];
+    var frame = stack['frames'][0];
-    expect(frame.function.name, equals('helper'));
+    expect(frame.function.qualifiedName, equals('helper.async_op'));
-    expect(await frame.location.getLine(), equals(12));
+    if (useCausalAsyncStacks) {
-    expect(await frame.location.getColumn(), equals(16));
+      expect(await frame.location.getLine(), equals(14));
+      expect(await frame.location.getColumn(), equals(1));
+    } else {
+      expect(await frame.location.getLine(), equals(12));
+      expect(await frame.location.getColumn(), equals(7));
+    }
 
-    frame = stack['frames'][2];
+    frame = stack['frames'][1];
     expect(frame.function.name, equals('testMain'));
     expect(await frame.location.getLine(), equals(18));
     expect(await frame.location.getColumn(), equals(3));

runtime/observatory/tests/service/rewind_test.dart

@@ -56,7 +56,7 @@ var tests = <IsolateTest>[
     } on ServerRpcException catch (e) {
       caughtException = true;
       expect(e.code, equals(ServerRpcException.kCannotResume));
-      expect(e.message, 'Frame must be in bounds [1..12]: saw 0');
+      expect(e.message, 'Frame must be in bounds [1..8]: saw 0');
     }
     expect(caughtException, isTrue);
   },
@@ -69,7 +69,7 @@ var tests = <IsolateTest>[
     } on ServerRpcException catch (e) {
       caughtException = true;
       expect(e.code, equals(ServerRpcException.kCannotResume));
-      expect(e.message, 'Frame must be in bounds [1..12]: saw 13');
+      expect(e.message, 'Frame must be in bounds [1..8]: saw 13');
     }
     expect(caughtException, isTrue);
   },

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

@@ -947,26 +947,17 @@ Future<void> doTestsLazy() async {
   final noYieldsExpected = const <String>[
     r'^#0      throwSync \(.*/utils.dart:16(:3)?\)$',
     r'^#1      noYields3 \(.*/utils.dart:54(:3)?\)$',
-    // TODO(dart-vm): Figure out why this frame is flaky:
+    r'^#2      noYields2 \(.*/utils.dart:50(:9)?\)$',
-    r'^#2      _AsyncAwaitCompleter.start ',
+    r'^#3      noYields \(.*/utils.dart:46(:9)?\)$',
-    r'^#3      noYields3 \(.*/utils.dart:53(:23)?\)$',
-    r'^#4      noYields2 \(.*/utils.dart:50(:9)?\)$',
-    r'^#5      _AsyncAwaitCompleter.start ',
-    r'^#6      noYields2 \(.*/utils.dart:49(:23)?\)$',
-    r'^#7      noYields \(.*/utils.dart:46(:9)?\)$',
-    r'^#8      _AsyncAwaitCompleter.start ',
-    r'^#9      noYields \(.*/utils.dart:45(:22)?\)$',
   ];
   await doTestAwait(
       noYields,
       noYieldsExpected +
           const <String>[
-            r'^#10     doTestAwait ',
+            r'^#4      doTestAwait ',
-            r'^#11     _AsyncAwaitCompleter.start ',
+            r'^#5      doTestsLazy ',
-            r'^#12     doTestAwait ',
-            r'^#13     doTestsLazy ',
             r'^<asynchronous suspension>$',
-            r'^#14     main ',
+            r'^#6      main ',
             r'^<asynchronous suspension>$',
             r'^$',
           ]);
@@ -974,12 +965,10 @@ Future<void> doTestsLazy() async {
       noYields,
       noYieldsExpected +
           const <String>[
-            r'^#10     doTestAwaitThen ',
+            r'^#4      doTestAwaitThen ',
-            r'^#11     _AsyncAwaitCompleter.start ',
+            r'^#5      doTestsLazy ',
-            r'^#12     doTestAwaitThen ',
-            r'^#13     doTestsLazy ',
             r'^<asynchronous suspension>$',
-            r'^#14     main ',
+            r'^#6      main ',
             r'^<asynchronous suspension>$',
             r'^$',
           ]);
@@ -987,12 +976,10 @@ Future<void> doTestsLazy() async {
       noYields,
       noYieldsExpected +
           const <String>[
-            r'^#10     doTestAwaitCatchError ',
+            r'^#4      doTestAwaitCatchError ',
-            r'^#11     _AsyncAwaitCompleter.start ',
+            r'^#5      doTestsLazy ',
-            r'^#12     doTestAwaitCatchError ',
-            r'^#13     doTestsLazy ',
             r'^<asynchronous suspension>$',
-            r'^#14     main ',
+            r'^#6      main ',
             r'^<asynchronous suspension>$',
             r'^$',
           ]);

runtime/vm/compiler/frontend/bytecode_reader.cc

@@ -2208,6 +2208,10 @@ void BytecodeReaderHelper::ReadFunctionDeclarations(const Class& cls) {
   Array& parameter_names = Array::Handle(Z);
   AbstractType& type = AbstractType::Handle(Z);
 
+  name = cls.ScrubbedName();
+  const bool is_async_await_completer_owner =
+      Symbols::_AsyncAwaitCompleter().Equals(name);
+
   for (intptr_t i = 0; i < num_functions; ++i) {
     intptr_t flags = reader_.ReadUInt();
 
@@ -2265,16 +2269,30 @@ void BytecodeReaderHelper::ReadFunctionDeclarations(const Class& cls) {
     function.set_is_debuggable((flags & kIsDebuggableFlag) != 0);
     function.set_is_extension_member(is_extension_member);
 
+    // _AsyncAwaitCompleter.start should be made non-visible in stack traces,
+    // since it is an implementation detail of our await/async desugaring.
+    if (is_async_await_completer_owner &&
+        Symbols::_AsyncAwaitStart().Equals(name)) {
+      function.set_is_visible(!FLAG_causal_async_stacks &&
+                              !FLAG_lazy_async_stacks);
+    }
+
     if ((flags & kIsSyncStarFlag) != 0) {
       function.set_modifier(RawFunction::kSyncGen);
+      function.set_is_visible(!FLAG_causal_async_stacks &&
+                              !FLAG_lazy_async_stacks);
     } else if ((flags & kIsAsyncFlag) != 0) {
       function.set_modifier(RawFunction::kAsync);
       function.set_is_inlinable(!FLAG_causal_async_stacks &&
                                 !FLAG_lazy_async_stacks);
+      function.set_is_visible(!FLAG_causal_async_stacks &&
+                              !FLAG_lazy_async_stacks);
     } else if ((flags & kIsAsyncStarFlag) != 0) {
       function.set_modifier(RawFunction::kAsyncGen);
       function.set_is_inlinable(!FLAG_causal_async_stacks &&
                                 !FLAG_lazy_async_stacks);
+      function.set_is_visible(!FLAG_causal_async_stacks &&
+                              !FLAG_lazy_async_stacks);
     }
 
     if ((flags & kHasTypeParamsFlag) != 0) {

runtime/vm/kernel_loader.cc

@@ -1940,29 +1940,48 @@ void KernelLoader::LoadProcedure(const Library& library,
   ASSERT(function_node_tag == kSomething);
   FunctionNodeHelper function_node_helper(&helper_);
   function_node_helper.ReadUntilIncluding(FunctionNodeHelper::kDartAsyncMarker);
+
+  const bool is_async_await_completer_owner =
+      Symbols::_AsyncAwaitCompleter().Equals(
+          String::Handle(Z, owner.ScrubbedName()));
+
   // _AsyncAwaitCompleter.future should be made non-debuggable, otherwise
   // stepping out of async methods will keep hitting breakpoint resulting in
   // infinite loop.
-  bool isAsyncAwaitCompleterFuture =
+  const bool is_async_await_completer_future =
-      Symbols::_AsyncAwaitCompleter().Equals(
+      is_async_await_completer_owner &&
-          String::Handle(owner.ScrubbedName())) &&
+      Symbols::CompleterGetFuture().Equals(name);
-      Symbols::CompleterGetFuture().Equals(String::Handle(function.name()));
   function.set_is_debuggable(function_node_helper.dart_async_marker_ ==
                                  FunctionNodeHelper::kSync &&
-                             !isAsyncAwaitCompleterFuture);
+                             !is_async_await_completer_future);
+
+  // _AsyncAwaitCompleter.start should be made non-visible in stack traces,
+  // since it is an implementation detail of our await/async desugaring.
+  if (is_async_await_completer_owner &&
+      Symbols::_AsyncAwaitStart().Equals(name)) {
+    function.set_is_visible(!FLAG_causal_async_stacks &&
+                            !FLAG_lazy_async_stacks);
+  }
+
   switch (function_node_helper.dart_async_marker_) {
     case FunctionNodeHelper::kSyncStar:
       function.set_modifier(RawFunction::kSyncGen);
+      function.set_is_visible(!FLAG_causal_async_stacks &&
+                              !FLAG_lazy_async_stacks);
       break;
     case FunctionNodeHelper::kAsync:
       function.set_modifier(RawFunction::kAsync);
       function.set_is_inlinable(!FLAG_causal_async_stacks &&
                                 !FLAG_lazy_async_stacks);
+      function.set_is_visible(!FLAG_causal_async_stacks &&
+                              !FLAG_lazy_async_stacks);
       break;
     case FunctionNodeHelper::kAsyncStar:
       function.set_modifier(RawFunction::kAsyncGen);
       function.set_is_inlinable(!FLAG_causal_async_stacks &&
                                 !FLAG_lazy_async_stacks);
+      function.set_is_visible(!FLAG_causal_async_stacks &&
+                              !FLAG_lazy_async_stacks);
       break;
     default:
       // no special modifier

runtime/vm/object.cc

@@ -23074,15 +23074,6 @@ const char* StackTrace::ToDartCString(const StackTrace& stack_trace_in) {
         }
       } else if (code_object.raw() == StubCode::AsynchronousGapMarker().raw()) {
         buffer.AddString("<asynchronous suspension>\n");
-        // With lazy_async_stacks we're constructing the stack correctly
-        // (see `StackTraceUtils::CollectFramesLazy`) so there are no extra
-        // frames to skip.
-        if (!FLAG_lazy_async_stacks) {
-          // The frame immediately after the asynchronous gap marker is the
-          // identical to the frame above the marker. Skip the frame to enhance
-          // the readability of the trace.
-          i++;
-        }
       } else {
         intptr_t pc_offset = Smi::Value(stack_trace.PcOffsetAtFrame(i));
         if (code_object.IsCode()) {