[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>
diff --git a/runtime/observatory/tests/service/positive_token_pos_test.dart b/runtime/observatory/tests/service/positive_token_pos_test.dart
index 5bf9cb2..72d54dd 100644
--- a/runtime/observatory/tests/service/positive_token_pos_test.dart
+++ b/runtime/observatory/tests/service/positive_token_pos_test.dart
@@ -20,26 +20,25 @@
var tests = <IsolateTest>[
hasStoppedAtBreakpoint,
- markDartColonLibrariesDebuggable,
stoppedAtLine(18),
stepInto,
(Isolate isolate) async {
ServiceMap stack = await isolate.getStack();
- expect(stack['frames'].length, greaterThan(3));
-
- 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));
+ expect(stack['frames'].length, greaterThan(2));
// We used to return a negative token position for this frame.
// See issue #27128.
- frame = stack['frames'][1];
- expect(frame.function.name, equals('helper'));
- expect(await frame.location.getLine(), equals(12));
- expect(await frame.location.getColumn(), equals(16));
+ var frame = stack['frames'][0];
+ expect(frame.function.qualifiedName, equals('helper.async_op'));
+ if (useCausalAsyncStacks) {
+ 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));
diff --git a/runtime/observatory/tests/service/rewind_test.dart b/runtime/observatory/tests/service/rewind_test.dart
index 80ab4e7..059d5f1 100644
--- a/runtime/observatory/tests/service/rewind_test.dart
+++ b/runtime/observatory/tests/service/rewind_test.dart
@@ -56,7 +56,7 @@
} 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 @@
} 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);
},
diff --git a/runtime/tests/vm/dart/causal_stacks/utils.dart b/runtime/tests/vm/dart/causal_stacks/utils.dart
index 8cca3e5..6501440 100644
--- a/runtime/tests/vm/dart/causal_stacks/utils.dart
+++ b/runtime/tests/vm/dart/causal_stacks/utils.dart
@@ -947,26 +947,17 @@
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 _AsyncAwaitCompleter.start ',
- 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)?\)$',
+ r'^#2 noYields2 \(.*/utils.dart:50(:9)?\)$',
+ r'^#3 noYields \(.*/utils.dart:46(:9)?\)$',
];
await doTestAwait(
noYields,
noYieldsExpected +
const <String>[
- r'^#10 doTestAwait ',
- r'^#11 _AsyncAwaitCompleter.start ',
- r'^#12 doTestAwait ',
- r'^#13 doTestsLazy ',
+ r'^#4 doTestAwait ',
+ r'^#5 doTestsLazy ',
r'^<asynchronous suspension>$',
- r'^#14 main ',
+ r'^#6 main ',
r'^<asynchronous suspension>$',
r'^$',
]);
@@ -974,12 +965,10 @@
noYields,
noYieldsExpected +
const <String>[
- r'^#10 doTestAwaitThen ',
- r'^#11 _AsyncAwaitCompleter.start ',
- r'^#12 doTestAwaitThen ',
- r'^#13 doTestsLazy ',
+ r'^#4 doTestAwaitThen ',
+ r'^#5 doTestsLazy ',
r'^<asynchronous suspension>$',
- r'^#14 main ',
+ r'^#6 main ',
r'^<asynchronous suspension>$',
r'^$',
]);
@@ -987,12 +976,10 @@
noYields,
noYieldsExpected +
const <String>[
- r'^#10 doTestAwaitCatchError ',
- r'^#11 _AsyncAwaitCompleter.start ',
- r'^#12 doTestAwaitCatchError ',
- r'^#13 doTestsLazy ',
+ r'^#4 doTestAwaitCatchError ',
+ r'^#5 doTestsLazy ',
r'^<asynchronous suspension>$',
- r'^#14 main ',
+ r'^#6 main ',
r'^<asynchronous suspension>$',
r'^$',
]);
diff --git a/runtime/vm/compiler/frontend/bytecode_reader.cc b/runtime/vm/compiler/frontend/bytecode_reader.cc
index 9c19f71..06c8339 100644
--- a/runtime/vm/compiler/frontend/bytecode_reader.cc
+++ b/runtime/vm/compiler/frontend/bytecode_reader.cc
@@ -2208,6 +2208,10 @@
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 @@
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) {
diff --git a/runtime/vm/kernel_loader.cc b/runtime/vm/kernel_loader.cc
index 076f004..a2171d2 100644
--- a/runtime/vm/kernel_loader.cc
+++ b/runtime/vm/kernel_loader.cc
@@ -1940,29 +1940,48 @@
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 =
- Symbols::_AsyncAwaitCompleter().Equals(
- String::Handle(owner.ScrubbedName())) &&
- Symbols::CompleterGetFuture().Equals(String::Handle(function.name()));
+ const bool is_async_await_completer_future =
+ is_async_await_completer_owner &&
+ Symbols::CompleterGetFuture().Equals(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
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
index c28ef48..432b280 100644
--- a/runtime/vm/object.cc
+++ b/runtime/vm/object.cc
@@ -23074,15 +23074,6 @@
}
} 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()) {
