diff --git a/runtime/lib/stacktrace.cc b/runtime/lib/stacktrace.cc index 1df00568b3a..1b9600a1ebb 100644 --- a/runtime/lib/stacktrace.cc +++ b/runtime/lib/stacktrace.cc @@ -23,7 +23,7 @@ static RawStackTrace* CurrentSyncStackTrace(Thread* thread, // Determine how big the stack trace is. const intptr_t stack_trace_length = - StackTraceUtils::CountFrames(thread, skip_frames, null_function, nullptr); + StackTraceUtils::CountFrames(thread, skip_frames, null_function); // Allocate once. const Array& code_array = @@ -64,9 +64,8 @@ static RawStackTrace* CurrentStackTrace( // Determine the size of the stack trace. const intptr_t extra_frames = for_async_function ? 1 : 0; - bool sync_async_end = false; - const intptr_t synchronous_stack_trace_length = StackTraceUtils::CountFrames( - thread, skip_frames, async_function, &sync_async_end); + const intptr_t synchronous_stack_trace_length = + StackTraceUtils::CountFrames(thread, skip_frames, async_function); const intptr_t capacity = synchronous_stack_trace_length + extra_frames; // For the asynchronous gap. @@ -95,11 +94,7 @@ static RawStackTrace* CurrentStackTrace( ASSERT(write_cursor == capacity); - const StackTrace& result = StackTrace::Handle( - zone, StackTrace::New(code_array, pc_offset_array, async_stack_trace, - sync_async_end)); - - return result.raw(); + return StackTrace::New(code_array, pc_offset_array, async_stack_trace); } RawStackTrace* GetStackTraceForException() { diff --git a/runtime/observatory/tests/service/causal_async_stack_contents_test.dart b/runtime/observatory/tests/service/causal_async_stack_contents_test.dart index c21879b8753..209c2f6b2bb 100644 --- a/runtime/observatory/tests/service/causal_async_stack_contents_test.dart +++ b/runtime/observatory/tests/service/causal_async_stack_contents_test.dart @@ -11,8 +11,8 @@ import 'service_test_common.dart'; import 'test_helper.dart'; const LINE_C = 19; -const LINE_A = 25; -const LINE_B = 31; +const LINE_A = 24; +const LINE_B = 30; foobar() { debugger(); @@ -20,7 +20,6 @@ foobar() { } helper() async { - await 0; // force async gap debugger(); print('helper'); // LINE_A. foobar(); @@ -65,10 +64,10 @@ var tests = [ expect(asyncStack[3].toString(), contains('testMain')); // Line 19. expect(await asyncStack[0].location.toUserString(), contains('.dart:19')); - // Line 26. - expect(await asyncStack[1].location.toUserString(), contains('.dart:26')); - // Line 31. - expect(await asyncStack[3].location.toUserString(), contains('.dart:31')); + // Line 25. + expect(await asyncStack[1].location.toUserString(), contains('.dart:25')); + // Line 30. + expect(await asyncStack[3].location.toUserString(), contains('.dart:30')); }, ]; diff --git a/runtime/observatory/tests/service/causal_async_star_stack_contents_test.dart b/runtime/observatory/tests/service/causal_async_star_stack_contents_test.dart index 80eb931d549..35251058808 100644 --- a/runtime/observatory/tests/service/causal_async_star_stack_contents_test.dart +++ b/runtime/observatory/tests/service/causal_async_star_stack_contents_test.dart @@ -10,12 +10,11 @@ import 'package:unittest/unittest.dart'; import 'service_test_common.dart'; import 'test_helper.dart'; -const LINE_A = 28; -const LINE_B = 20; -const LINE_C = 22; +const LINE_A = 26; +const LINE_B = 19; +const LINE_C = 21; foobar() async* { - await 0; // force async gap debugger(); yield 1; // LINE_B. debugger(); @@ -23,7 +22,6 @@ foobar() async* { } helper() async { - await 0; // force async gap debugger(); print('helper'); // LINE_A. await for (var i in foobar()) { @@ -78,13 +76,12 @@ var tests = [ expect(asyncStack[2].toString(), contains('helper')); expect(asyncStack[3].kind, equals(M.FrameKind.asyncSuspensionMarker)); expect(asyncStack[4].toString(), contains('testMain')); - // Line 22. - expect( - await asyncStack[0].location.toUserString(), contains('.dart:$LINE_C')); - // Line 29. - expect(await asyncStack[2].location.toUserString(), contains('.dart:29')); - // Line 35. - expect(await asyncStack[4].location.toUserString(), contains('.dart:35')); + // Line 21. + expect(await asyncStack[0].location.toUserString(), contains('.dart:21')); + // Line 27. + expect(await asyncStack[2].location.toUserString(), contains('.dart:27')); + // Line 30. + expect(await asyncStack[4].location.toUserString(), contains('.dart:33')); }, ]; diff --git a/runtime/observatory/tests/service/pause_on_unhandled_async_exceptions2_test.dart b/runtime/observatory/tests/service/pause_on_unhandled_async_exceptions2_test.dart index 90cbea3b979..89ebcbfc585 100644 --- a/runtime/observatory/tests/service/pause_on_unhandled_async_exceptions2_test.dart +++ b/runtime/observatory/tests/service/pause_on_unhandled_async_exceptions2_test.dart @@ -10,7 +10,7 @@ import 'package:unittest/unittest.dart'; import 'test_helper.dart'; import 'service_test_common.dart'; -const LINE_A = 35; +const LINE_A = 34; class Foo {} @@ -20,7 +20,6 @@ doThrow() { } asyncThrower() async { - await 0; // force async gap doThrow(); } diff --git a/runtime/observatory/tests/service/pause_on_unhandled_async_exceptions_test.dart b/runtime/observatory/tests/service/pause_on_unhandled_async_exceptions_test.dart index 8f955f39736..b832931a6a2 100644 --- a/runtime/observatory/tests/service/pause_on_unhandled_async_exceptions_test.dart +++ b/runtime/observatory/tests/service/pause_on_unhandled_async_exceptions_test.dart @@ -9,7 +9,7 @@ import 'package:unittest/unittest.dart'; import 'test_helper.dart'; import 'service_test_common.dart'; -const LINE_A = 35; +const LINE_A = 34; class Foo {} @@ -19,7 +19,6 @@ doThrow() { } asyncThrower() async { - await 0; // force async gap doThrow(); } diff --git a/runtime/tests/vm/dart/causal_stacks/utils.dart b/runtime/tests/vm/dart/causal_stacks/utils.dart index ce5116b1dac..6038e33de8f 100644 --- a/runtime/tests/vm/dart/causal_stacks/utils.dart +++ b/runtime/tests/vm/dart/causal_stacks/utils.dart @@ -124,8 +124,11 @@ Future noYields3() async { Map noYieldsMapCausal = { 0: '#0 throwSync ', 1: '#1 noYields3 ', - 2: '#2 noYields2 ', - 3: '#3 noYields ', + 2: '', + 3: '#2 noYields2 ', + 4: '', + 5: '#3 noYields ', + 4: '', // Callers, like doTest and main .. }; @@ -165,9 +168,11 @@ Map mixedYieldsMapCausal = { 0: '#0 throwAsync ', 1: '', 2: '#1 mixedYields3 ', - 3: '#2 mixedYields2 ', - 4: '', - 5: '#3 mixedYields ', + 3: '', + 4: '#2 mixedYields2 ', + 5: '', + 6: '#3 mixedYields ', + 7: '', // Callers, like doTest and main .. }; @@ -202,6 +207,7 @@ Map syncSuffixMapCausal = { 3: '#2 syncSuffix2 ', 4: '', 5: '#3 syncSuffix ', + 6: '', // Callers, like doTest and main .. }; diff --git a/runtime/tests/vm/dart/sync_async_start_pkg_test_test.dart b/runtime/tests/vm/dart/sync_async_start_pkg_test_test.dart deleted file mode 100644 index e2bcd2fd769..00000000000 --- a/runtime/tests/vm/dart/sync_async_start_pkg_test_test.dart +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. -// -// This test ensures that "pkg:stack_trace" (used by "pkg:test") doesn't break -// when causal async stacks are enabled by dropping frames below a synchronous -// start to an async function. - -import "package:test/test.dart"; -import "package:stack_trace/src/stack_zone_specification.dart"; - -import 'dart:async'; - -void main() { - test("Stacktrace includes sync-starts.", () async { - final st = await firstMethod(); - expect("$st", allOf([contains("firstMethod"), contains("secondMethod")])); - }); -} - -Future firstMethod() async { - return await secondMethod(); -} - -Future secondMethod() async { - return StackTrace.current; -} diff --git a/runtime/vm/debugger.cc b/runtime/vm/debugger.cc index 659ddbc1f2e..392705ab71e 100644 --- a/runtime/vm/debugger.cc +++ b/runtime/vm/debugger.cc @@ -2266,9 +2266,8 @@ DebuggerStackTrace* Debugger::CollectAsyncCausalStackTrace() { return NULL; } - bool sync_async_end = false; intptr_t synchronous_stack_trace_length = - StackTraceUtils::CountFrames(thread, 0, async_function, &sync_async_end); + StackTraceUtils::CountFrames(thread, 0, async_function); // Append the top frames from the synchronous stack trace, up until the active // asynchronous function. We truncate the remainder of the synchronous @@ -2305,11 +2304,8 @@ DebuggerStackTrace* Debugger::CollectAsyncCausalStackTrace() { // Now we append the asynchronous causal stack trace. These are not active // frames but a historical record of how this asynchronous function was // activated. - - intptr_t frame_skip = - sync_async_end ? StackTrace::kSyncAsyncCroppedFrames : 0; while (!async_stack_trace.IsNull()) { - for (intptr_t i = frame_skip; i < async_stack_trace.Length(); i++) { + for (intptr_t i = 0; i < async_stack_trace.Length(); i++) { code_obj = async_stack_trace.CodeAtFrame(i); if (code_obj.IsNull()) { break; @@ -2344,15 +2340,22 @@ DebuggerStackTrace* Debugger::CollectAsyncCausalStackTrace() { } } // Follow the link. - frame_skip = async_stack_trace.skip_sync_start_in_parent_stack() - ? StackTrace::kSyncAsyncCroppedFrames - : 0; async_stack_trace = async_stack_trace.async_link(); } return stack_trace; } +#if !defined(DART_PRECOMPILED_RUNTIME) +static bool CheckAndSkipAsync(int skip_sync_async_frames_count, + const String& function_name) { + return (skip_sync_async_frames_count == 2 && + function_name.Equals(Symbols::_ClosureCall())) || + (skip_sync_async_frames_count == 1 && + function_name.Equals(Symbols::_AsyncAwaitCompleterStart())); +} +#endif + DebuggerStackTrace* Debugger::CollectAwaiterReturnStackTrace() { #if defined(DART_PRECOMPILED_RUNTIME) // Causal async stacks are not supported in the AOT runtime. @@ -2404,8 +2407,7 @@ DebuggerStackTrace* Debugger::CollectAwaiterReturnStackTrace() { if (skip_sync_async_frames_count > 0) { function_name = function.QualifiedScrubbedName(); - if (StackTraceUtils::CheckAndSkipAsync(skip_sync_async_frames_count, - function_name)) { + if (CheckAndSkipAsync(skip_sync_async_frames_count, function_name)) { skip_sync_async_frames_count--; } else { // Unexpected function in synchronous call of async function. @@ -2455,8 +2457,8 @@ DebuggerStackTrace* Debugger::CollectAwaiterReturnStackTrace() { if (skip_sync_async_frames_count > 0) { function_name ^= function.QualifiedScrubbedName(); - if (StackTraceUtils::CheckAndSkipAsync( - skip_sync_async_frames_count, function_name)) { + if (CheckAndSkipAsync(skip_sync_async_frames_count, + function_name)) { skip_sync_async_frames_count--; } else { // Unexpected function in sync async call @@ -2510,8 +2512,8 @@ DebuggerStackTrace* Debugger::CollectAwaiterReturnStackTrace() { if (skip_sync_async_frames_count > 0) { function_name ^= function.QualifiedScrubbedName(); - if (StackTraceUtils::CheckAndSkipAsync(skip_sync_async_frames_count, - function_name)) { + if (CheckAndSkipAsync(skip_sync_async_frames_count, + function_name)) { skip_sync_async_frames_count--; } else { // Unexpected function in synchronous call of async function. diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index 612ea1e49db..5112bc0fc99 100644 --- a/runtime/vm/object.cc +++ b/runtime/vm/object.cc @@ -21886,14 +21886,6 @@ RawFunction* Closure::GetInstantiatedSignature(Zone* zone) const { return sig_fun.raw(); } -bool StackTrace::skip_sync_start_in_parent_stack() const { - return raw_ptr()->skip_sync_start_in_parent_stack; -} - -void StackTrace::set_skip_sync_start_in_parent_stack(bool value) const { - StoreNonPointer(&raw_ptr()->skip_sync_start_in_parent_stack, value); -} - intptr_t StackTrace::Length() const { const Array& code_array = Array::Handle(raw_ptr()->code_array_); return code_array.Length(); @@ -21954,14 +21946,12 @@ RawStackTrace* StackTrace::New(const Array& code_array, result.set_code_array(code_array); result.set_pc_offset_array(pc_offset_array); result.set_expand_inlined(true); // default. - result.set_skip_sync_start_in_parent_stack(false); return result.raw(); } RawStackTrace* StackTrace::New(const Array& code_array, const Array& pc_offset_array, const StackTrace& async_link, - bool skip_sync_start_in_parent_stack, Heap::Space space) { StackTrace& result = StackTrace::Handle(); { @@ -21974,7 +21964,6 @@ RawStackTrace* StackTrace::New(const Array& code_array, result.set_code_array(code_array); result.set_pc_offset_array(pc_offset_array); result.set_expand_inlined(true); // default. - result.set_skip_sync_start_in_parent_stack(skip_sync_start_in_parent_stack); return result.raw(); } @@ -22033,9 +22022,8 @@ const char* StackTrace::ToDartCString(const StackTrace& stack_trace_in) { // Iterate through the stack frames and create C string description // for each frame. intptr_t frame_index = 0; - uint32_t frame_skip = 0; do { - for (intptr_t i = frame_skip; i < stack_trace.Length(); i++) { + for (intptr_t i = 0; i < stack_trace.Length(); i++) { code_object = stack_trace.CodeAtFrame(i); if (code_object.IsNull()) { // Check for a null function, which indicates a gap in a StackOverflow @@ -22095,9 +22083,6 @@ const char* StackTrace::ToDartCString(const StackTrace& stack_trace_in) { } } // Follow the link. - frame_skip = stack_trace.skip_sync_start_in_parent_stack() - ? StackTrace::kSyncAsyncCroppedFrames - : 0; stack_trace = stack_trace.async_link(); } while (!stack_trace.IsNull()); @@ -22125,9 +22110,8 @@ const char* StackTrace::ToDwarfCString(const StackTrace& stack_trace_in) { buffer.Printf("pid: %" Pd ", tid: %" Pd ", name %s\n", OS::ProcessId(), OSThread::ThreadIdToIntPtr(thread->id()), thread->name()); intptr_t frame_index = 0; - uint32_t frame_skip = 0; do { - for (intptr_t i = frame_skip; i < stack_trace.Length(); i++) { + for (intptr_t i = 0; i < stack_trace.Length(); i++) { code = stack_trace.CodeAtFrame(i); if (code.IsNull()) { // Check for a null function, which indicates a gap in a StackOverflow @@ -22169,9 +22153,6 @@ const char* StackTrace::ToDwarfCString(const StackTrace& stack_trace_in) { } } // Follow the link. - frame_skip = stack_trace.skip_sync_start_in_parent_stack() - ? StackTrace::kSyncAsyncCroppedFrames - : 0; stack_trace = stack_trace.async_link(); } while (!stack_trace.IsNull()); diff --git a/runtime/vm/object.h b/runtime/vm/object.h index 9e51ec7cdcd..21c28d6ddce 100644 --- a/runtime/vm/object.h +++ b/runtime/vm/object.h @@ -9659,23 +9659,6 @@ class StackTrace : public Instance { RawSmi* PcOffsetAtFrame(intptr_t frame_index) const; void SetPcOffsetAtFrame(intptr_t frame_index, const Smi& pc_offset) const; - bool skip_sync_start_in_parent_stack() const; - void set_skip_sync_start_in_parent_stack(bool value) const; - - // The number of frames that should be cut off the top of an async stack trace - // if it's appended to a synchronous stack trace along a sync-async call. - // - // Without cropping, the border would look like: - // - // - // --------------------------- - // - // - // - // Since it's not actually an async call, we crop off the last two - // frames when concatenating the sync and async stacktraces. - static constexpr intptr_t kSyncAsyncCroppedFrames = 2; - static intptr_t InstanceSize() { return RoundedAllocationSize(sizeof(RawStackTrace)); } @@ -9686,7 +9669,6 @@ class StackTrace : public Instance { static RawStackTrace* New(const Array& code_array, const Array& pc_offset_array, const StackTrace& async_link, - bool skip_sync_start_in_parent_stack, Heap::Space space = Heap::kNew); private: diff --git a/runtime/vm/raw_object.h b/runtime/vm/raw_object.h index d29999c6107..d3dd8b91504 100644 --- a/runtime/vm/raw_object.h +++ b/runtime/vm/raw_object.h @@ -2519,10 +2519,6 @@ class RawStackTrace : public RawInstance { // False for pre-allocated stack trace (used in OOM and Stack overflow). bool expand_inlined_; - // Whether the link between the stack and the async-link represents a - // synchronous start to an asynchronous function. In this case, we omit the - // marker when concatenating the stacks. - bool skip_sync_start_in_parent_stack; }; // VM type for capturing JS regular expressions. diff --git a/runtime/vm/stack_trace.cc b/runtime/vm/stack_trace.cc index 34ab09a6a42..17724a3b5d0 100644 --- a/runtime/vm/stack_trace.cc +++ b/runtime/vm/stack_trace.cc @@ -4,15 +4,13 @@ #include "vm/stack_trace.h" #include "vm/stack_frame.h" -#include "vm/symbols.h" namespace dart { // Count the number of frames that are on the stack. intptr_t StackTraceUtils::CountFrames(Thread* thread, int skip_frames, - const Function& async_function, - bool* sync_async_end) { + const Function& async_function) { Zone* zone = thread->zone(); intptr_t frame_count = 0; StackFrameIterator frames(ValidationPolicy::kDontValidateFrames, thread, @@ -22,12 +20,8 @@ intptr_t StackTraceUtils::CountFrames(Thread* thread, Function& function = Function::Handle(zone); Code& code = Code::Handle(zone); Bytecode& bytecode = Bytecode::Handle(zone); - String& function_name = String::Handle(zone); const bool async_function_is_null = async_function.IsNull(); - intptr_t sync_async_gap_frames = -1; - ASSERT(async_function_is_null || sync_async_end != NULL); - for (; frame != NULL && sync_async_gap_frames != 0; - frame = frames.NextFrame()) { + for (; frame != NULL; frame = frames.NextFrame()) { if (!frame->IsDartFrame()) { continue; } @@ -45,24 +39,14 @@ intptr_t StackTraceUtils::CountFrames(Thread* thread, code = frame->LookupDartCode(); function = code.function(); } - if (sync_async_gap_frames > 0) { - function_name = function.QualifiedScrubbedName(); - if (!CheckAndSkipAsync(sync_async_gap_frames, function_name)) { - *sync_async_end = false; - return frame_count; - } - --sync_async_gap_frames; - } else { - frame_count++; - } + frame_count++; if (!async_function_is_null && (async_function.raw() == function.parent_function())) { - sync_async_gap_frames = kSyncAsyncFrameGap; + return frame_count; } } - if (!async_function_is_null) { - *sync_async_end = sync_async_gap_frames == 0; - } + // We hit the sentinel. + ASSERT(async_function_is_null); return frame_count; } diff --git a/runtime/vm/stack_trace.h b/runtime/vm/stack_trace.h index 81563b64487..394f45b1ad3 100644 --- a/runtime/vm/stack_trace.h +++ b/runtime/vm/stack_trace.h @@ -8,7 +8,6 @@ #include "vm/allocation.h" #include "vm/flag_list.h" #include "vm/object.h" -#include "vm/symbols.h" namespace dart { @@ -17,12 +16,10 @@ class StackTraceUtils : public AllStatic { /// Counts the number of stack frames. /// Skips over the first |skip_frames|. /// If |async_function| is not null, stops at the function that has - /// |async_function| as its parent, and records in 'sync_async_end' whether - /// |async_function| was called synchronously. + /// |async_function| as its parent. static intptr_t CountFrames(Thread* thread, int skip_frames, - const Function& async_function, - bool* sync_async_end); + const Function& async_function); /// Collects |count| frames into |code_array| and |pc_offset_array|. /// Writing begins at |array_offset|. @@ -45,27 +42,6 @@ class StackTraceUtils : public AllStatic { StackTrace* async_stack_trace, Array* async_code_array, Array* async_pc_offset_array); - - // The number of frames involved in a "sync-async" gap: a synchronous initial - // invocation of an asynchronous function. See CheckAndSkipAsync. - static constexpr intptr_t kSyncAsyncFrameGap = 2; - - // A synchronous invocation of an async function involves the following - // frames: - // __ (0) - // _Closure.call (1) - // _AsyncAwaitCompleter.start (2) - // (3) - // - // For 'skip_sync_async_frames_count' in {1, 2}, checks that 'function_name' - // matches the name of method with that index above. - static bool CheckAndSkipAsync(int skip_sync_async_frames_count, - const String& function_name) { - return (skip_sync_async_frames_count == 2 && - function_name.Equals(Symbols::_ClosureCall())) || - (skip_sync_async_frames_count == 1 && - function_name.Equals(Symbols::_AsyncAwaitCompleterStart())); - } }; } // namespace dart diff --git a/tests/language_2/vm/causal_async_exception_stack2_test.dart b/tests/language_2/vm/causal_async_exception_stack2_test.dart index 4efca8fe258..94dda4b8bd2 100644 --- a/tests/language_2/vm/causal_async_exception_stack2_test.dart +++ b/tests/language_2/vm/causal_async_exception_stack2_test.dart @@ -29,8 +29,11 @@ test1() async { expect( h.stringContainsInOrder(st.toString(), [ 'foo3', + '', 'foo2', + '', 'foo', + '', 'test1', ]), isTrue); @@ -44,8 +47,11 @@ test1() async { expect( h.stringContainsInOrder(st.toString(), [ 'bar3', + '', 'bar2', + '', 'bar', + '', 'test1', ]), isTrue); @@ -66,8 +72,11 @@ test2() async { expect( h.stringContainsInOrder(st.toString(), [ 'foo3', + '', 'foo2', + '', 'foo', + '', 'test2', ]), isTrue); @@ -81,8 +90,11 @@ test2() async { expect( h.stringContainsInOrder(st.toString(), [ 'bar3', + '', 'bar2', + '', 'bar', + '', 'test2', ]), isTrue); diff --git a/tests/language_2/vm/causal_async_exception_stack_test.dart b/tests/language_2/vm/causal_async_exception_stack_test.dart index caba5d9f8ba..b4059e9d8f4 100644 --- a/tests/language_2/vm/causal_async_exception_stack_test.dart +++ b/tests/language_2/vm/causal_async_exception_stack_test.dart @@ -35,9 +35,11 @@ main() async { expect( h.stringContainsInOrder(st.toString(), [ 'thrower', '.dart:10', // + '', // 'generator', '.dart:19', // '', // 'foo', '.dart:23', // + '', // 'main', // ]), isTrue); @@ -58,8 +60,11 @@ main() async { expect( h.stringContainsInOrder(st.toString(), [ 'thrower', + '', 'main..inner.deep', + '', 'main..inner', + '', 'main', '', ]), @@ -73,7 +78,8 @@ main() async { expect( h.stringContainsInOrder(st.toString(), [ 'thrower', '.dart:10', // - 'main.', '.dart:71', // + '', // + 'main.', '.dart:76', // ]), isTrue); }