dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

[vm] Fix symbolization of future listener frame.

Browse Source 
For future listener frames async unwinding produces frames with
`pc_offset` set to 0, because this frames correspond to closures
which will be invoked once the future completes. Such frames
are different from most other frames which correspond to real
call or yield locations within native code and have non-zero
`pc_offset`.

Because there is no call-/yield- site at 0 compiler does not emit
any source location information into CodeSourceMap and consequently
runtime can't produce much beyond file name in the stack trace for
these special frames.

To make matters worse ordering of code objects in the AOT snapshot
could affect symbolization of such frames - sometimes resulting in
an arbitrary token position being attached to the symbolized frame.

This CL adds more special handling of these frames into
StackTrace::ToCString and changes compiler to emit a dummy source
position descriptor for the start of every closure function.

This descriptor is encoded as

    ChangePosition function.token_pos()
    AdvancePC 0

in the CodeSourceMap. This entry is then handled specially when
generating DWARF.

TEST=vm/dart{,_2}/causal_stacks/async_throws_stack_lazy{,_lazy_non_symbolic}_test

Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-product-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-dwarf-linux-product-x64-try
Change-Id: I80740dd2c3cb03d6c03c35e1b3bde2cacbe033e0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/219781
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
Reviewed-by: Clement Skau <cskau@google.com>
This commit is contained in:
Vyacheslav Egorov
2021-11-10 14:50:23 +00:00
committed by commit-bot@chromium.org
parent 4ff053b8b7
commit 64d30f848b
11 changed files with 118 additions and 83 deletions
Download Patch File Download Diff File Expand all files Collapse all files
runtime/tests/vm/dart/causal_stacks/utils.dart
+1 -1
View File
@@ -850,7 +850,7 @@ Future<void> doTestsLazy([String? debugInfoFilename]) async {
r'^<asynchronous suspension>$',
r'^#1 asyncStarThrowAsync \(.*/utils.dart:126(:5)?\)$',
r'^<asynchronous suspension>$',
r'^#2 listenAsyncStarThrowAsync.<anonymous closure> \(.+/utils.dart(:0)?\)$',
r'^#2 listenAsyncStarThrowAsync.<anonymous closure> \(.+/utils.dart:132(:56)?\)$',
r'^<asynchronous suspension>$',
];
await doTestAwait(
runtime/tests/vm/dart_2/causal_stacks/utils.dart
+1 -1
View File
@@ -852,7 +852,7 @@ Future<void> doTestsLazy([String debugInfoFilename]) async {
r'^<asynchronous suspension>$',
r'^#1 asyncStarThrowAsync \(.*/utils.dart:128(:5)?\)$',
r'^<asynchronous suspension>$',
r'^#2 listenAsyncStarThrowAsync.<anonymous closure> \(.+/utils.dart(:0)?\)$',
r'^#2 listenAsyncStarThrowAsync.<anonymous closure> \(.+/utils.dart:134(:56)?\)$',
r'^<asynchronous suspension>$',
];
await doTestAwait(
runtime/vm/code_descriptors.cc
+8
View File
@@ -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) {
runtime/vm/code_descriptors.h
+1
View File
@@ -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.
runtime/vm/compiler/backend/flow_graph_compiler.cc
+47
View File
@@ -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()) {
runtime/vm/compiler/backend/flow_graph_compiler.h
+2
View File
@@ -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);
runtime/vm/compiler/backend/flow_graph_compiler_arm64.cc
-32
View File
@@ -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)) {
runtime/vm/compiler/backend/flow_graph_compiler_ia32.cc
-18
View File
@@ -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);
runtime/vm/compiler/backend/flow_graph_compiler_x64.cc
-27
View File
@@ -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)) {
runtime/vm/dwarf.cc
+42 -1
View File
@@ -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: {
runtime/vm/object.cc
+16 -3
View File
@@ -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++;
}