diff --git a/runtime/tests/vm/dart/causal_stacks/utils.dart b/runtime/tests/vm/dart/causal_stacks/utils.dart index c8b377461ac..077e9bd01ea 100644 --- a/runtime/tests/vm/dart/causal_stacks/utils.dart +++ b/runtime/tests/vm/dart/causal_stacks/utils.dart @@ -850,7 +850,7 @@ Future doTestsLazy([String? debugInfoFilename]) async { r'^$', r'^#1 asyncStarThrowAsync \(.*/utils.dart:126(:5)?\)$', r'^$', - r'^#2 listenAsyncStarThrowAsync. \(.+/utils.dart(:0)?\)$', + r'^#2 listenAsyncStarThrowAsync. \(.+/utils.dart:132(:56)?\)$', r'^$', ]; await doTestAwait( diff --git a/runtime/tests/vm/dart_2/causal_stacks/utils.dart b/runtime/tests/vm/dart_2/causal_stacks/utils.dart index cd46b037f95..a17cade3871 100644 --- a/runtime/tests/vm/dart_2/causal_stacks/utils.dart +++ b/runtime/tests/vm/dart_2/causal_stacks/utils.dart @@ -852,7 +852,7 @@ Future doTestsLazy([String debugInfoFilename]) async { r'^$', r'^#1 asyncStarThrowAsync \(.*/utils.dart:128(:5)?\)$', r'^$', - r'^#2 listenAsyncStarThrowAsync. \(.+/utils.dart(:0)?\)$', + r'^#2 listenAsyncStarThrowAsync. \(.+/utils.dart:134(:56)?\)$', r'^$', ]; await doTestAwait( diff --git a/runtime/vm/code_descriptors.cc b/runtime/vm/code_descriptors.cc index dd8bc1d8b6d..87625f36b6a 100644 --- a/runtime/vm/code_descriptors.cc +++ b/runtime/vm/code_descriptors.cc @@ -417,6 +417,14 @@ void CodeSourceMapBuilder::StartInliningInterval( } } +void CodeSourceMapBuilder::WriteFunctionEntrySourcePosition( + const InstructionSource& source) { + ASSERT(written_pc_offset_ == 0 && buffered_pc_offset_ == 0); + ASSERT(stream_.bytes_written() == 0); + WriteChangePosition(source.token_pos); + WriteAdvancePC(0); +} + void CodeSourceMapBuilder::BeginCodeSourceRange( int32_t pc_offset, const InstructionSource& source) { diff --git a/runtime/vm/code_descriptors.h b/runtime/vm/code_descriptors.h index ac29be74348..bd0d35802ab 100644 --- a/runtime/vm/code_descriptors.h +++ b/runtime/vm/code_descriptors.h @@ -246,6 +246,7 @@ class CodeSourceMapBuilder : public ZoneAllocated { void NoteNullCheck(int32_t pc_offset, const InstructionSource& source, intptr_t name_index); + void WriteFunctionEntrySourcePosition(const InstructionSource& source); // If source is from an inlined call, returns the token position of the // original call in the root function, otherwise the source's token position. diff --git a/runtime/vm/compiler/backend/flow_graph_compiler.cc b/runtime/vm/compiler/backend/flow_graph_compiler.cc index d8ce910906e..88106b27f08 100644 --- a/runtime/vm/compiler/backend/flow_graph_compiler.cc +++ b/runtime/vm/compiler/backend/flow_graph_compiler.cc @@ -560,6 +560,53 @@ bool FlowGraphCompiler::IsPeephole(Instruction* instr) const { return false; } +void FlowGraphCompiler::EmitFunctionEntrySourcePositionDescriptorIfNeeded() { + // When unwinding async stacks we might produce frames which correspond + // to future listeners which are going to be called when the future completes. + // These listeners are not yet called and thus their frame pc_offset is set + // to 0 - which does not actually correspond to any call- or yield- site + // inside the code object. Nevertheless we would like to be able to + // produce proper position information for it when symbolizing the stack. + // To achieve that in AOT mode (where we don't actually have + // |Function::token_pos| available) we instead emit an artificial descriptor + // at the very beginning of the function. + if (FLAG_precompiled_mode && flow_graph().function().IsClosureFunction()) { + code_source_map_builder_->WriteFunctionEntrySourcePosition( + InstructionSource(flow_graph().function().token_pos())); + } +} + +void FlowGraphCompiler::CompileGraph() { + InitCompiler(); + +#if !defined(TARGET_ARCH_IA32) + // For JIT we have multiple entrypoints functionality which moved the frame + // setup into the [TargetEntryInstr] (which will set the constant pool + // allowed bit to true). Despite this we still have to set the + // constant pool allowed bit to true here as well, because we can generate + // code for [CatchEntryInstr]s, which need the pool. + assembler()->set_constant_pool_allowed(true); +#endif + + EmitFunctionEntrySourcePositionDescriptorIfNeeded(); + VisitBlocks(); + +#if defined(DEBUG) + assembler()->Breakpoint(); +#endif + + if (!skip_body_compilation()) { +#if !defined(TARGET_ARCH_IA32) + ASSERT(assembler()->constant_pool_allowed()); +#endif + GenerateDeferredCode(); + } + + for (intptr_t i = 0; i < indirect_gotos_.length(); ++i) { + indirect_gotos_[i]->ComputeOffsetTable(this); + } +} + void FlowGraphCompiler::VisitBlocks() { CompactBlocks(); if (compiler::Assembler::EmittingComments()) { diff --git a/runtime/vm/compiler/backend/flow_graph_compiler.h b/runtime/vm/compiler/backend/flow_graph_compiler.h index 7669e782304..98488ae55f7 100644 --- a/runtime/vm/compiler/backend/flow_graph_compiler.h +++ b/runtime/vm/compiler/backend/flow_graph_compiler.h @@ -559,6 +559,8 @@ class FlowGraphCompiler : public ValueObject { void VisitBlocks(); + void EmitFunctionEntrySourcePositionDescriptorIfNeeded(); + // Bail out of the flow graph compiler. Does not return to the caller. void Bailout(const char* reason); diff --git a/runtime/vm/compiler/backend/flow_graph_compiler_arm64.cc b/runtime/vm/compiler/backend/flow_graph_compiler_arm64.cc index fdbf099aab6..233821af1f3 100644 --- a/runtime/vm/compiler/backend/flow_graph_compiler_arm64.cc +++ b/runtime/vm/compiler/backend/flow_graph_compiler_arm64.cc @@ -372,38 +372,6 @@ void FlowGraphCompiler::EmitPrologue() { EndCodeSourceRange(PrologueSource()); } -// Input parameters: -// LR: return address. -// SP: address of last argument. -// FP: caller's frame pointer. -// PP: caller's pool pointer. -// R4: arguments descriptor array. -void FlowGraphCompiler::CompileGraph() { - InitCompiler(); - - // For JIT we have multiple entrypoints functionality which moved the frame - // setup into the [TargetEntryInstr] (which will set the constant pool - // allowed bit to true). Despite this we still have to set the - // constant pool allowed bit to true here as well, because we can generate - // code for [CatchEntryInstr]s, which need the pool. - __ set_constant_pool_allowed(true); - - VisitBlocks(); - -#if defined(DEBUG) - __ brk(0); -#endif - - if (!skip_body_compilation()) { - ASSERT(assembler()->constant_pool_allowed()); - GenerateDeferredCode(); - } - - for (intptr_t i = 0; i < indirect_gotos_.length(); ++i) { - indirect_gotos_[i]->ComputeOffsetTable(this); - } -} - void FlowGraphCompiler::EmitCallToStub(const Code& stub) { ASSERT(!stub.IsNull()); if (CanPcRelativeCall(stub)) { diff --git a/runtime/vm/compiler/backend/flow_graph_compiler_ia32.cc b/runtime/vm/compiler/backend/flow_graph_compiler_ia32.cc index 0b232ddd619..d613572dd38 100644 --- a/runtime/vm/compiler/backend/flow_graph_compiler_ia32.cc +++ b/runtime/vm/compiler/backend/flow_graph_compiler_ia32.cc @@ -459,24 +459,6 @@ void FlowGraphCompiler::EmitPrologue() { EndCodeSourceRange(PrologueSource()); } -void FlowGraphCompiler::CompileGraph() { - InitCompiler(); - - ASSERT(!block_order().is_empty()); - VisitBlocks(); - - if (!skip_body_compilation()) { -#if defined(DEBUG) - __ int3(); -#endif - GenerateDeferredCode(); - } - - for (intptr_t i = 0; i < indirect_gotos_.length(); ++i) { - indirect_gotos_[i]->ComputeOffsetTable(this); - } -} - void FlowGraphCompiler::EmitCallToStub(const Code& stub) { if (stub.InVMIsolateHeap()) { __ CallVmStub(stub); diff --git a/runtime/vm/compiler/backend/flow_graph_compiler_x64.cc b/runtime/vm/compiler/backend/flow_graph_compiler_x64.cc index 573add6f8d6..3abdf84e10f 100644 --- a/runtime/vm/compiler/backend/flow_graph_compiler_x64.cc +++ b/runtime/vm/compiler/backend/flow_graph_compiler_x64.cc @@ -378,33 +378,6 @@ void FlowGraphCompiler::EmitPrologue() { EndCodeSourceRange(PrologueSource()); } -void FlowGraphCompiler::CompileGraph() { - InitCompiler(); - - // We have multiple entrypoints functionality which moved the frame - // setup into the [FunctionEntryInstr] (which will set the constant pool - // allowed bit to true). Despite this we still have to set the - // constant pool allowed bit to true here as well, because we can generate - // code for [CatchEntryInstr]s, which need the pool. - __ set_constant_pool_allowed(true); - - ASSERT(!block_order().is_empty()); - VisitBlocks(); - -#if defined(DEBUG) - __ int3(); -#endif - - if (!skip_body_compilation()) { - ASSERT(assembler()->constant_pool_allowed()); - GenerateDeferredCode(); - } - - for (intptr_t i = 0; i < indirect_gotos_.length(); ++i) { - indirect_gotos_[i]->ComputeOffsetTable(this); - } -} - void FlowGraphCompiler::EmitCallToStub(const Code& stub) { ASSERT(!stub.IsNull()); if (CanPcRelativeCall(stub)) { diff --git a/runtime/vm/dwarf.cc b/runtime/vm/dwarf.cc index 4f9884882ca..d661bd74cdb 100644 --- a/runtime/vm/dwarf.cc +++ b/runtime/vm/dwarf.cc @@ -717,6 +717,26 @@ void Dwarf::WriteLineNumberProgramFromCodeSourceMaps( function_stack.Clear(); token_positions.Clear(); + // CodeSourceMap might start in the following way: + // + // ChangePosition function.token_pos() + // AdvancePC 0 + // ChangePosition x + // AdvancePC y + // + // This entry is emitted to ensure correct symbolization of + // function listener frames produced by async unwinding. + // (See EmitFunctionEntrySourcePositionDescriptorIfNeeded). + // Directly interpreting this sequence would cause us to emit + // multiple with the same pc into line number table and different + // position information. To avoid this will make an adjustment for + // the second record we emit: if position x is a synthetic one we will + // simply drop the second record, if position x is real then we will + // emit row with a slightly adjusted PC (by 1 byte). This would not + // affect symbolization (you can't have a call that is 1 byte long) + // but will avoid line number table entries with the same PC. + bool function_entry_position_was_emitted = false; + int32_t current_pc_offset = 0; function_stack.Add(&root_function); token_positions.Add(kNoDwarfPositionInfo); @@ -740,9 +760,30 @@ void Dwarf::WriteLineNumberProgramFromCodeSourceMaps( const intptr_t file = LookupScript(script); const intptr_t line = token_positions.Last().line(); const intptr_t column = token_positions.Last().column(); - writer->EmitRow(file, line, column, asm_name, current_pc_offset); + intptr_t pc_offset_adjustment = 0; + bool should_emit = true; + + // If we are at the function entry and have already emitted a row + // then adjust current_pc_offset to avoid duplicated entries. + // See the comment below which explains why this code is here. + if (current_pc_offset == 0 && function_entry_position_was_emitted) { + pc_offset_adjustment = 1; + // Ignore synthetic positions. Function entry position gives + // more information anyway. + should_emit = !(line == 0 && column == 0); + } + + if (should_emit) { + writer->EmitRow(file, line, column, asm_name, + current_pc_offset + pc_offset_adjustment); + } current_pc_offset += arg1; + if (arg1 == 0) { // Special case of AdvancePC 0. + ASSERT(current_pc_offset == 0); + ASSERT(!function_entry_position_was_emitted); + function_entry_position_was_emitted = true; + } break; } case CodeSourceMapOps::kPushFunction: { diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index e272c5cd096..fd8822cb0e8 100644 --- a/runtime/vm/object.cc +++ b/runtime/vm/object.cc @@ -25851,6 +25851,15 @@ const char* StackTrace::ToCString() const { // A visible frame ends any gap we might be in. in_gap = false; + // Zero pc_offset can only occur in the frame produced by the async + // unwinding and it corresponds to the next future listener in the + // chain. This function is not yet called (it will be called when + // the future completes) hence pc_offset is set to 0. This frame + // is very different from other frames which have pc_offsets + // corresponding to call- or yield-sites in the generated code and + // should be handled specially. + const bool is_future_listener = pc_offset == 0; + #if defined(DART_PRECOMPILED_RUNTIME) // When printing non-symbolic frames, we normally print call // addresses, not return addresses, by subtracting one from the PC to @@ -25861,7 +25870,6 @@ const char* StackTrace::ToCString() const { // is invoked with the value of the resolved future. Thus, we must // report the return address, as returning a value before the closure // payload will cause failures to decode the frame using DWARF info. - const bool is_future_listener = pc_offset == 0; const uword call_addr = is_future_listener ? pc : pc - 1; if (FLAG_dwarf_stack_traces_mode) { @@ -25887,7 +25895,11 @@ const char* StackTrace::ToCString() const { } #endif - if (code.is_optimized() && stack_trace.expand_inlined()) { + if (code.is_optimized() && stack_trace.expand_inlined() && + (FLAG_precompiled_mode || !is_future_listener)) { + // Note: In AOT mode EmitFunctionEntrySourcePositionDescriptorIfNeeded + // will take care of emitting a descriptor that would allow us to + // symbolize stack frame with 0 offset. code.GetInlinedFunctionsAtReturnAddress(pc_offset, &inlined_functions, &inlined_token_positions); ASSERT(inlined_functions.length() >= 1); @@ -25901,7 +25913,8 @@ const char* StackTrace::ToCString() const { continue; } - auto const pos = code.GetTokenIndexOfPC(pc); + auto const pos = is_future_listener ? function.token_pos() + : code.GetTokenIndexOfPC(pc); PrintSymbolicStackFrame(zone, &buffer, function, pos, frame_index); frame_index++; }