From c6815e8522f478cb16070042e6e83bb4ed68074a Mon Sep 17 00:00:00 2001 From: Vyacheslav Egorov Date: Mon, 17 Jul 2017 20:56:26 +0200 Subject: [PATCH] Reapply 0489249d29131ed7bf047485c74f557d88eb9974 with a fix for front_end tests. Proper sequencing of _asyncStackTraceHelper in Kernel This helper function was being called before its argument was initialized so it was passing null. Instead, it should be called after its argument is initialized. Because the initialization happens in Kernel code, it is simplest to insert the call explicitly in Kernel code as well as part of the async transformation. This has the consequence that we now call the helper function even when the flag causal_async_stacks is false. Fixes issue #29771. Fixes issue #30178 Fixes issue #30058 BUG= R=aam@google.com, asiva@google.com Review-Url: https://codereview.chromium.org/2936793003 . Review-Url: https://codereview.chromium.org/2982943002 . --- .../test/src/incremental/mock_sdk.dart | 1 + pkg/kernel/lib/core_types.dart | 6 +++++ .../lib/transformations/continuation.dart | 15 +++++++++++ runtime/lib/stacktrace.cc | 18 ++++++++----- .../tests/service/service_kernel.status | 1 - runtime/vm/kernel_binary_flowgraph.cc | 26 ------------------- runtime/vm/parser.cc | 2 +- 7 files changed, 35 insertions(+), 34 deletions(-) diff --git a/pkg/front_end/test/src/incremental/mock_sdk.dart b/pkg/front_end/test/src/incremental/mock_sdk.dart index 678583d79c3..b005e2606c3 100644 --- a/pkg/front_end/test/src/incremental/mock_sdk.dart +++ b/pkg/front_end/test/src/incremental/mock_sdk.dart @@ -233,6 +233,7 @@ abstract class Completer { class _StreamIterator implements StreamIterator {} class _AsyncStarStreamController {} +Object _asyncStackTraceHelper(Function async_op) { } Function _asyncThenWrapperHelper(continuation) {} Function _asyncErrorWrapperHelper(continuation) {} Future _awaitHelper( diff --git a/pkg/kernel/lib/core_types.dart b/pkg/kernel/lib/core_types.dart index 13c1a63417d..2cf0e20c7f7 100644 --- a/pkg/kernel/lib/core_types.dart +++ b/pkg/kernel/lib/core_types.dart @@ -80,6 +80,7 @@ class CoreTypes { Constructor _syncIterableDefaultConstructor; Constructor _streamIteratorDefaultConstructor; Constructor _asyncStarStreamControllerDefaultConstructor; + Procedure _asyncStackTraceHelperProcedure; Procedure _asyncThenWrapperHelperProcedure; Procedure _asyncErrorWrapperHelperProcedure; Procedure _awaitHelperProcedure; @@ -103,6 +104,11 @@ class CoreTypes { _index.getMember('dart:async', '_AsyncStarStreamController', ''); } + Procedure get asyncStackTraceHelperProcedure { + return _asyncStackTraceHelperProcedure ??= + _index.getTopLevelMember('dart:async', '_asyncStackTraceHelper'); + } + Procedure get asyncThenWrapperHelperProcedure { return _asyncThenWrapperHelperProcedure ??= _index.getTopLevelMember('dart:async', '_asyncThenWrapperHelper'); diff --git a/pkg/kernel/lib/transformations/continuation.dart b/pkg/kernel/lib/transformations/continuation.dart index 632731756d6..cce8f561545 100644 --- a/pkg/kernel/lib/transformations/continuation.dart +++ b/pkg/kernel/lib/transformations/continuation.dart @@ -215,6 +215,8 @@ class SyncStarFunctionRewriter extends ContinuationRewriterBase { abstract class AsyncRewriterBase extends ContinuationRewriterBase { final VariableDeclaration nestedClosureVariable = new VariableDeclaration(":async_op"); + final VariableDeclaration stackTraceVariable = + new VariableDeclaration(":async_stack_trace"); final VariableDeclaration thenContinuationVariable = new VariableDeclaration(":async_op_then"); final VariableDeclaration catchErrorContinuationVariable = @@ -230,6 +232,9 @@ abstract class AsyncRewriterBase extends ContinuationRewriterBase { void setupAsyncContinuations(List statements) { expressionRewriter = new ExpressionLifter(this); + // var :async_stack_trace; + statements.add(stackTraceVariable); + // var :async_op_then; statements.add(thenContinuationVariable); @@ -269,6 +274,13 @@ abstract class AsyncRewriterBase extends ContinuationRewriterBase { ..fileOffset = enclosingFunction.parent.fileOffset; statements.add(closureFunction); + // :async_stack_trace = _asyncStackTraceHelper(asyncBody); + final stackTrace = new StaticInvocation(helper.asyncStackTraceHelper, + new Arguments([new VariableGet(nestedClosureVariable)])); + final stackTraceAssign = new ExpressionStatement( + new VariableSet(stackTraceVariable, stackTrace)); + statements.add(stackTraceAssign); + // :async_op_then = _asyncThenWrapperHelper(asyncBody); final boundThenClosure = new StaticInvocation(helper.asyncThenWrapper, new Arguments([new VariableGet(nestedClosureVariable)])); @@ -905,6 +917,7 @@ class HelperNodes { final Constructor streamControllerConstructor; final Constructor syncIterableConstructor; final Constructor streamIteratorConstructor; + final Procedure asyncStackTraceHelper; final Procedure asyncThenWrapper; final Procedure asyncErrorWrapper; final Procedure awaitHelper; @@ -923,6 +936,7 @@ class HelperNodes { this.streamIteratorConstructor, this.futureMicrotaskConstructor, this.streamControllerConstructor, + this.asyncStackTraceHelper, this.asyncThenWrapper, this.asyncErrorWrapper, this.awaitHelper, @@ -942,6 +956,7 @@ class HelperNodes { coreTypes.streamIteratorDefaultConstructor, coreTypes.futureMicrotaskConstructor, coreTypes.asyncStarStreamControllerDefaultConstructor, + coreTypes.asyncStackTraceHelperProcedure, coreTypes.asyncThenWrapperHelperProcedure, coreTypes.asyncErrorWrapperHelperProcedure, coreTypes.awaitHelperProcedure, diff --git a/runtime/lib/stacktrace.cc b/runtime/lib/stacktrace.cc index d8b8cab176a..c5c41f2573d 100644 --- a/runtime/lib/stacktrace.cc +++ b/runtime/lib/stacktrace.cc @@ -107,13 +107,15 @@ DEFINE_NATIVE_ENTRY(StackTrace_current, 0) { } DEFINE_NATIVE_ENTRY(StackTrace_asyncStackTraceHelper, 1) { + if (!FLAG_causal_async_stacks) { + return Object::null(); + } + GET_NATIVE_ARGUMENT(Closure, async_op, arguments->NativeArgAt(0)); - if (!async_op.IsNull()) { - if (FLAG_support_debugger) { - Debugger* debugger = isolate->debugger(); - if (debugger != NULL) { - debugger->MaybeAsyncStepInto(async_op); - } + if (FLAG_support_debugger) { + Debugger* debugger = isolate->debugger(); + if (debugger != NULL) { + debugger->MaybeAsyncStepInto(async_op); } } return CurrentStackTrace(thread, true); @@ -125,6 +127,10 @@ DEFINE_NATIVE_ENTRY(StackTrace_clearAsyncThreadStackTrace, 0) { } DEFINE_NATIVE_ENTRY(StackTrace_setAsyncThreadStackTrace, 1) { + if (!FLAG_causal_async_stacks) { + return Object::null(); + } + GET_NON_NULL_NATIVE_ARGUMENT(StackTrace, stack_trace, arguments->NativeArgAt(0)); thread->set_async_stack_trace(stack_trace); diff --git a/runtime/observatory/tests/service/service_kernel.status b/runtime/observatory/tests/service/service_kernel.status index 88dfd1a40c1..6549d9634d1 100644 --- a/runtime/observatory/tests/service/service_kernel.status +++ b/runtime/observatory/tests/service/service_kernel.status @@ -35,7 +35,6 @@ evaluate_*: Skip # no evaluation test for now ### ### Async debugging ### -async_single_step_into_test: RuntimeError # Issue 29158 async_star_single_step_into_test: RuntimeError # Issue 29158 async_step_out_test: RuntimeError # Issue 29158 async_star_step_out_test: RuntimeError # Issue 29158 diff --git a/runtime/vm/kernel_binary_flowgraph.cc b/runtime/vm/kernel_binary_flowgraph.cc index 9b7cc9fa703..7b7d23f675c 100644 --- a/runtime/vm/kernel_binary_flowgraph.cc +++ b/runtime/vm/kernel_binary_flowgraph.cc @@ -3115,32 +3115,6 @@ FlowGraph* StreamingFlowGraphBuilder::BuildGraphOfFunction( function_node_helper.ReadUntilExcluding(FunctionNodeHelper::kBody); - if (FLAG_causal_async_stacks && - (dart_function.IsAsyncFunction() || dart_function.IsAsyncGenerator())) { - LocalScope* scope = parsed_function()->node_sequence()->scope(); - // :async_stack_trace = _asyncStackTraceHelper(:async_op); - const dart::Library& async_lib = - dart::Library::Handle(dart::Library::AsyncLibrary()); - const Function& target = Function::ZoneHandle( - Z, - async_lib.LookupFunctionAllowPrivate(Symbols::AsyncStackTraceHelper())); - ASSERT(!target.IsNull()); - - // TODO(johnmccutchan): Why does this have the null value? - LocalVariable* async_op = - scope->child()->LookupVariable(Symbols::AsyncOperation(), false); - ASSERT(async_op != NULL); - ASSERT(async_op->is_captured()); - body += LoadLocal(async_op); - body += PushArgument(); - body += StaticCall(TokenPosition::kNoSource, target, 1); - LocalVariable* async_stack_trace_var = - scope->LookupVariable(Symbols::AsyncStackTraceVar(), false); - ASSERT(async_stack_trace_var != NULL); - body += StoreLocal(TokenPosition::kNoSource, async_stack_trace_var); - body += Drop(); - } - bool has_body = ReadTag() == kSomething; // read first part of body. if (dart_function.is_native()) { diff --git a/runtime/vm/parser.cc b/runtime/vm/parser.cc index 36c5a33d919..3c9e08c61c7 100644 --- a/runtime/vm/parser.cc +++ b/runtime/vm/parser.cc @@ -7288,7 +7288,7 @@ SequenceNode* Parser::CloseAsyncGeneratorFunction(const Function& closure_func, if (FLAG_causal_async_stacks) { // Add to AST: - // :async_stack_trace = _asyncStackTraceHelper(); + // :async_stack_trace = _asyncStackTraceHelper(:async_op); const Function& async_stack_trace_helper = Function::ZoneHandle( Z, async_lib.LookupFunctionAllowPrivate(Symbols::AsyncStackTraceHelper()));