[VM] Removes extra frames from debug lazy async stacks.

Clement Skau · commit-bot@chromium.org · commit 000458992895 · 2020-08-14T12:45:37.000Z
This adds a debugger mechanism to replace part of --causal-async-stack's _asyncStackTraceHelper runtime entry. It recognises async and async* functions and adds a synthetic breakpoint on entry into the wrapped async_op, making sure we don't get "synthetic" frames as we're stepping in and out of the async code. Change-Id: I1df6e6874de2fa9185f27a1a8873ad0071ad9fb6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/157480 Commit-Queue: Clement Skau <cskau@google.com> Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
diff --git a/pkg/vm_service/test/async_single_step_exception_test.dart b/pkg/vm_service/test/async_single_step_exception_test.dart
@@ -45,12 +45,6 @@ var tests = <IsolateTest>[
   stoppedAtLine(LINE_D), // await helper
   stepInto,
 
-  ...ifLazyAsyncStacks(<IsolateTest>[
-    hasStoppedAtBreakpoint,
-    stoppedAtLine(19), // helper() async { ... }
-    stepInto,
-  ]),
-
   hasStoppedAtBreakpoint,
   stoppedAtLine(LINE_A), // print helper
   smartNext,
diff --git a/pkg/vm_service/test/async_single_step_into_test.dart b/pkg/vm_service/test/async_single_step_into_test.dart
@@ -34,12 +34,6 @@ var tests = <IsolateTest>[
   stoppedAtLine(LINE_D),
   stepInto,
 
-  ...ifLazyAsyncStacks(<IsolateTest>[
-    hasStoppedAtBreakpoint,
-    stoppedAtLine(16), // helper() async { ... }
-    stepInto,
-  ]),
-
   hasStoppedAtBreakpoint,
   stoppedAtLine(LINE_A),
   stepOver, // print.
diff --git a/pkg/vm_service/test/async_single_step_out_test.dart b/pkg/vm_service/test/async_single_step_out_test.dart
@@ -35,12 +35,6 @@ var tests = <IsolateTest>[
   stoppedAtLine(LINE_D), // await helper
   stepInto,
 
-  ...ifLazyAsyncStacks(<IsolateTest>[
-    hasStoppedAtBreakpoint,
-    stoppedAtLine(17), // helper() async { ... }
-    stepInto,
-  ]),
-
   hasStoppedAtBreakpoint,
   stoppedAtLine(LINE_A), // print.
   stepOver,
diff --git a/pkg/vm_service/test/async_star_single_step_into_test.dart b/pkg/vm_service/test/async_star_single_step_into_test.dart
@@ -45,25 +45,13 @@ var tests = <IsolateTest>[
   stoppedAtLine(LINE_F),
   stepInto,
 
-  ...ifLazyAsyncStacks(<IsolateTest>[
-    hasStoppedAtBreakpoint,
-    stoppedAtLine(23), // helper() async { ... }
-    stepInto,
-  ]),
-
   hasStoppedAtBreakpoint,
   stoppedAtLine(LINE_C),
   stepOver, // print.
 
   hasStoppedAtBreakpoint,
   stepInto,
 
-  ...ifLazyAsyncStacks(<IsolateTest>[
-    hasStoppedAtBreakpoint,
-    stoppedAtLine(26), // await for (...) { ... }
-    stepInto,
-  ]),
-
   hasStoppedAtBreakpoint,
   stoppedAtLine(LINE_A),
   // Resume here to exit the generator function.
diff --git a/pkg/vm_service/test/async_star_step_out_test.dart b/pkg/vm_service/test/async_star_step_out_test.dart
@@ -49,25 +49,13 @@ var tests = <IsolateTest>[
   stoppedAtLine(LINE_F),
   stepInto,
 
-  ...ifLazyAsyncStacks(<IsolateTest>[
-    hasStoppedAtBreakpoint,
-    stoppedAtLine(26), // helper() async { ... }
-    stepInto,
-  ]),
-
   hasStoppedAtBreakpoint,
   stoppedAtLine(LINE_C),
   stepOver, // print.
 
   hasStoppedAtBreakpoint,
   stepInto,
 
-  ...ifLazyAsyncStacks(<IsolateTest>[
-    hasStoppedAtBreakpoint,
-    stoppedAtLine(29), // await for (...) {}
-    stepInto,
-  ]),
-
   hasStoppedAtBreakpoint,
   stoppedAtLine(LINE_A),
   stepOut, // step out of generator.
diff --git a/pkg/vm_service/test/async_step_out_test.dart b/pkg/vm_service/test/async_step_out_test.dart
@@ -37,12 +37,6 @@ var tests = <IsolateTest>[
   stoppedAtLine(LINE_E),
   stepInto,
 
-  ...ifLazyAsyncStacks(<IsolateTest>[
-    hasStoppedAtBreakpoint,
-    stoppedAtLine(18), // helper() async { ... }
-    stepInto,
-  ]),
-
   hasStoppedAtBreakpoint,
   stoppedAtLine(LINE_A),
   asyncNext,
diff --git a/pkg/vm_service/test/common/test_helper.dart b/pkg/vm_service/test/common/test_helper.dart
@@ -22,11 +22,6 @@ const List<String> extraDebuggingArgs = useCausalAsyncStacks
     ? ['--causal-async-stacks', '--no-lazy-async-stacks']
     : ['--no-causal-async-stacks', '--lazy-async-stacks'];
 
-List<IsolateTest> ifLazyAsyncStacks(List<IsolateTest> lazyTests) {
-  if (useCausalAsyncStacks) return const <IsolateTest>[];
-  return lazyTests;
-}
-
 /// Will be set to the http address of the VM's service protocol before
 /// any tests are invoked.
 String serviceHttpAddress;
@@ -222,13 +217,13 @@ class _ServiceTesteeLauncher {
           first = false;
           print('** Signaled to run test queries on $uri');
         }
-        print('>testee>out> $line');
+        stdout.write('>testee>out> ${line}\n');
       });
       process.stderr
           .transform(utf8.decoder)
           .transform(LineSplitter())
           .listen((line) {
-        print('>testee>err> $line');
+        stdout.write('>testee>err> ${line}\n');
       });
       process.exitCode.then((exitCode) {
         if ((exitCode != 0) && !killedByTester) {
diff --git a/runtime/observatory/tests/service/async_single_step_exception_test.dart b/runtime/observatory/tests/service/async_single_step_exception_test.dart
@@ -46,12 +46,6 @@ var tests = <IsolateTest>[
   stoppedAtLine(LINE_D), // await helper
   stepInto,
 
-  ...ifLazyAsyncStacks(<IsolateTest>[
-    hasStoppedAtBreakpoint,
-    stoppedAtLine(19), // helper() async { ... }
-    stepInto,
-  ]),
-
   hasStoppedAtBreakpoint,
   stoppedAtLine(LINE_A), // print helper
   smartNext,
diff --git a/runtime/observatory/tests/service/async_single_step_into_test.dart b/runtime/observatory/tests/service/async_single_step_into_test.dart
@@ -34,12 +34,6 @@ var tests = <IsolateTest>[
   stoppedAtLine(LINE_D),
   stepInto,
 
-  ...ifLazyAsyncStacks(<IsolateTest>[
-    hasStoppedAtBreakpoint,
-    stoppedAtLine(16),
-    stepInto, // helper() async { ... }
-  ]),
-
   hasStoppedAtBreakpoint,
   stoppedAtLine(LINE_A),
   stepOver, // print.
diff --git a/runtime/observatory/tests/service/async_single_step_out_test.dart b/runtime/observatory/tests/service/async_single_step_out_test.dart
@@ -35,12 +35,6 @@ var tests = <IsolateTest>[
   stoppedAtLine(LINE_D), // await helper
   stepInto,
 
-  ...ifLazyAsyncStacks(<IsolateTest>[
-    hasStoppedAtBreakpoint,
-    stoppedAtLine(17), // helper() async {}
-    stepInto,
-  ]),
-
   hasStoppedAtBreakpoint,
   stoppedAtLine(LINE_A), // print.
   stepOver,
@@ -57,7 +51,7 @@ var tests = <IsolateTest>[
   stoppedAtLine(25), // await helper (weird dispatching)
   smartNext,
 
-  hasStoppedAtBreakpoint, //19
+  hasStoppedAtBreakpoint,
   stoppedAtLine(LINE_E), // arrive after the await.
   resumeIsolate
 ];
diff --git a/runtime/observatory/tests/service/async_star_single_step_into_test.dart b/runtime/observatory/tests/service/async_star_single_step_into_test.dart
@@ -44,22 +44,10 @@ var tests = <IsolateTest>[
   stoppedAtLine(LINE_F),
   stepInto,
 
-  ...ifLazyAsyncStacks(<IsolateTest>[
-    hasStoppedAtBreakpoint,
-    stoppedAtLine(23), // helper() async { ... }
-    stepInto,
-  ]),
-
   hasStoppedAtBreakpoint,
   stoppedAtLine(LINE_C),
   stepOver, // print.
 
-  ...ifLazyAsyncStacks(<IsolateTest>[
-    hasStoppedAtBreakpoint,
-    stoppedAtLine(25), // await for ...
-    stepInto,
-  ]),
-
   hasStoppedAtBreakpoint,
   stepInto,
 
diff --git a/runtime/observatory/tests/service/async_star_step_out_test.dart b/runtime/observatory/tests/service/async_star_step_out_test.dart
@@ -48,25 +48,13 @@ var tests = <IsolateTest>[
   stoppedAtLine(LINE_F),
   stepInto,
 
-  ...ifLazyAsyncStacks(<IsolateTest>[
-    hasStoppedAtBreakpoint,
-    stoppedAtLine(26), // helper() async {}
-    stepInto,
-  ]),
-
   hasStoppedAtBreakpoint,
   stoppedAtLine(LINE_C),
   stepOver, // print.
 
   hasStoppedAtBreakpoint,
   stepInto,
 
-  ...ifLazyAsyncStacks(<IsolateTest>[
-    hasStoppedAtBreakpoint,
-    stoppedAtLine(28), // await for ...
-    stepInto,
-  ]),
-
   hasStoppedAtBreakpoint,
   stoppedAtLine(LINE_A),
   stepOut, // step out of generator.
diff --git a/runtime/observatory/tests/service/async_step_out_test.dart b/runtime/observatory/tests/service/async_step_out_test.dart
@@ -37,12 +37,6 @@ var tests = <IsolateTest>[
   stoppedAtLine(LINE_E),
   stepInto,
 
-  ...ifLazyAsyncStacks(<IsolateTest>[
-    hasStoppedAtBreakpoint,
-    stoppedAtLine(18), // helper() async { ... }
-    stepInto,
-  ]),
-
   hasStoppedAtBreakpoint,
   stoppedAtLine(LINE_A),
   asyncNext,
diff --git a/runtime/observatory/tests/service/test_helper.dart b/runtime/observatory/tests/service/test_helper.dart
@@ -25,11 +25,6 @@ const List<String> extraDebuggingArgs = useCausalAsyncStacks
     ? const ['--causal-async-stacks', '--no-lazy-async-stacks']
     : const ['--no-causal-async-stacks', '--lazy-async-stacks'];
 
-List<IsolateTest> ifLazyAsyncStacks(List<IsolateTest> lazyTests) {
-  if (useCausalAsyncStacks) return const <IsolateTest>[];
-  return lazyTests;
-}
-
 /// Will be set to the http address of the VM's service protocol before
 /// any tests are invoked.
 String serviceHttpAddress;
@@ -286,7 +281,7 @@ class _ServiceTesteeLauncher {
           .transform(utf8.decoder)
           .transform(new LineSplitter())
           .listen((line) {
-        print('>testee>err> $line');
+        print('>testee>err> ${line.trim()}');
       });
       process.exitCode.then((exitCode) async {
         await serviceInfoDir.delete(recursive: true);
diff --git a/runtime/vm/debugger.cc b/runtime/vm/debugger.cc
@@ -2207,6 +2207,8 @@ DebuggerStackTrace* Debugger::CollectAsyncCausalStackTrace() {
   class StackTrace& async_stack_trace = StackTrace::Handle(zone);
   Array& async_code_array = Array::Handle(zone);
   Array& async_pc_offset_array = Array::Handle(zone);
+
+  // Extract the eagerly recorded async stack from the current thread.
   StackTraceUtils::ExtractAsyncStackTraceInfo(
       thread, &async_function, &async_stack_trace, &async_code_array,
       &async_pc_offset_array);
@@ -3724,6 +3726,16 @@ Breakpoint* Debugger::SetBreakpointAtActivation(const Instance& closure,
   return bpt_location->AddPerClosure(this, closure, for_over_await);
 }
 
+Breakpoint* Debugger::SetBreakpointAtAsyncOp(const Function& async_op) {
+  const Script& script = Script::Handle(async_op.script());
+  BreakpointLocation* bpt_location =
+      SetBreakpoint(script, async_op.token_pos(), async_op.end_token_pos(), -1,
+                    -1 /* no line/col */, async_op);
+  auto bpt = bpt_location->AddSingleShot(this);
+  bpt->set_is_synthetic_async(true);
+  return bpt;
+}
+
 Breakpoint* Debugger::BreakpointAtActivation(const Instance& closure) {
   if (!closure.IsClosure()) {
     return NULL;
@@ -4458,6 +4470,37 @@ ErrorPtr Debugger::PauseStepping() {
   ActivationFrame* frame = TopDartFrame();
   ASSERT(frame != NULL);
 
+  // Since lazy async stacks doesn't use the _asyncStackTraceHelper runtime
+  // entry, we need to manually set a synthetic breakpoint for async_op before
+  // we enter it.
+  if (FLAG_lazy_async_stacks) {
+    // async and async* functions always contain synthetic async_ops.
+    if ((frame->function().IsAsyncFunction() ||
+         frame->function().IsAsyncGenerator())) {
+      ASSERT(!frame->GetSavedCurrentContext().IsNull());
+      ASSERT(frame->GetSavedCurrentContext().num_variables() >
+             Context::kAsyncCompleterIndex);
+
+      const Object& jump_var = Object::Handle(
+          frame->GetSavedCurrentContext().At(Context::kAsyncCompleterIndex));
+
+      // Only set breakpoint when entering async_op the first time.
+      // :async_completer_var should be uninitialised at this point:
+      if (jump_var.IsNull()) {
+        const Function& async_op =
+            Function::Handle(frame->function().GetGeneratedClosure());
+        if (!async_op.IsNull()) {
+          SetBreakpointAtAsyncOp(async_op);
+          // After setting the breakpoint we stop stepping and continue the
+          // debugger until the next breakpoint, to step over all the
+          // synthetic code.
+          Continue();
+          return Error::null();
+        }
+      }
+    }
+  }
+
   if (FLAG_async_debugger) {
     if ((async_stepping_fp_ != 0) && (top_frame_awaiter_ != Object::null())) {
       // Check if the user has single stepped out of an async function with
diff --git a/runtime/vm/debugger.h b/runtime/vm/debugger.h
@@ -519,6 +519,10 @@ class Debugger {
                                      intptr_t line_number,
                                      intptr_t column_number);
 
+  // Sets synthetic breakpoint at async_op to step over the synthetic part of
+  // the stack trace.
+  Breakpoint* SetBreakpointAtAsyncOp(const Function& async_op);
+
   BreakpointLocation* BreakpointLocationAtLineCol(const String& script_url,
                                                   intptr_t line_number,
                                                   intptr_t column_number);
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
@@ -6969,6 +6969,26 @@ void Function::set_parent_function(const Function& value) const {
   }
 }
 
+FunctionPtr Function::GetGeneratedClosure() const {
+  const auto& closure_functions = GrowableObjectArray::Handle(
+      Isolate::Current()->object_store()->closure_functions());
+  auto& entry = Object::Handle();
+
+  for (auto i = (closure_functions.Length() - 1); i >= 0; i--) {
+    entry = closure_functions.At(i);
+
+    ASSERT(entry.IsFunction());
+
+    const auto& closure_function = Function::Cast(entry);
+    if (closure_function.parent_function() == raw() &&
+        closure_function.is_generated_body()) {
+      return closure_function.raw();
+    }
+  }
+
+  return Function::null();
+}
+
 // Enclosing outermost function of this local function.
 FunctionPtr Function::GetOutermostFunction() const {
   FunctionPtr parent = parent_function();
diff --git a/runtime/vm/object.h b/runtime/vm/object.h
@@ -2746,6 +2746,11 @@ class Function : public Object {
   // Enclosing function of this local function.
   FunctionPtr parent_function() const;
 
+  // Enclosed generated closure function of this local function.
+  // This will only work after the closure function has been allocated in the
+  // isolate's object_store.
+  FunctionPtr GetGeneratedClosure() const;
+
   // Enclosing outermost function of this local function.
   FunctionPtr GetOutermostFunction() const;
 
diff --git a/runtime/vm/stack_trace.cc b/runtime/vm/stack_trace.cc
@@ -395,6 +395,7 @@ void StackTraceUtils::CollectFramesLazy(
   CallerClosureFinder caller_closure_finder(zone);
   auto& pc_descs = PcDescriptors::Handle();
 
+  // Start by traversing the sync. part of the stack.
   for (; frame != nullptr; frame = frames.NextFrame()) {
     if (skip_frames > 0) {
       skip_frames--;
@@ -461,6 +462,7 @@ void StackTraceUtils::CollectFramesLazy(
       // Skip: Already handled this frame's function above.
       closure = caller_closure_finder.FindCaller(closure);
 
+      // Traverse the trail of async futures all the way up.
       for (; !closure.IsNull();
            closure = caller_closure_finder.FindCaller(closure)) {
         function = closure.function();
