From be6e5d871785ec3e556499d47be71ddfc298383c Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Mon, 11 Nov 2019 23:56:00 +0000 Subject: [PATCH] [vm] Try to symbolize Dart frames on crash or assertion failure when there is an exit frame. Remove disabled/broken symbolization based on heap iteration. Bug: https://bugs.fuchsia.dev/p/fuchsia/issues/detail?id=41154 Change-Id: I5e6644e6ef72177cefec920fbe653000a4868f5d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/124826 Reviewed-by: Alexander Markov Commit-Queue: Ryan Macnak --- runtime/vm/gdb_helpers.cc | 9 +----- runtime/vm/profiler.cc | 58 ++++++++++++--------------------------- runtime/vm/stack_frame.cc | 11 ++++++++ runtime/vm/stack_frame.h | 2 ++ 4 files changed, 31 insertions(+), 49 deletions(-) diff --git a/runtime/vm/gdb_helpers.cc b/runtime/vm/gdb_helpers.cc index a8a1a609d48..6cb98f04c71 100644 --- a/runtime/vm/gdb_helpers.cc +++ b/runtime/vm/gdb_helpers.cc @@ -36,14 +36,7 @@ void _printDartStackTrace() { // in the middle of a GC or interested in stub frames. DART_EXPORT void _printStackTrace() { - StackFrameIterator frames(ValidationPolicy::kDontValidateFrames, - Thread::Current(), - StackFrameIterator::kNoCrossThreadIteration); - StackFrame* frame = frames.NextFrame(); - while (frame != nullptr) { - OS::PrintErr("%s\n", frame->ToCString()); - frame = frames.NextFrame(); - } + StackFrame::DumpCurrentTrace(); } // Like _printDartStackTrace, but works when stopped in generated code. diff --git a/runtime/vm/profiler.cc b/runtime/vm/profiler.cc index 9b4e734605f..a86665c9e42 100644 --- a/runtime/vm/profiler.cc +++ b/runtime/vm/profiler.cc @@ -461,29 +461,7 @@ void ClearProfileVisitor::VisitSample(Sample* sample) { sample->Clear(); } -static void DumpStackFrame(intptr_t frame_index, - uword pc, - uword fp, - bool try_symbolize_dart_frames) { - Thread* thread = Thread::Current(); - if ((thread != NULL) && !thread->IsAtSafepoint() && - try_symbolize_dart_frames) { - Isolate* isolate = thread->isolate(); - if ((isolate != NULL) && isolate->is_runnable()) { - // Only attempt to symbolize Dart frames if we can safely iterate the - // current isolate's heap. - Code& code = Code::Handle(Code::LookupCodeInVmIsolate(pc)); - if (code.IsNull()) { - code = Code::LookupCode(pc); // In current isolate. - } - if (!code.IsNull()) { - OS::PrintErr(" pc 0x%" Pp " fp 0x%" Pp " %s\n", pc, fp, - code.QualifiedName()); - return; - } - } - } - +static void DumpStackFrame(intptr_t frame_index, uword pc, uword fp) { uintptr_t start = 0; char* native_symbol_name = NativeSymbolResolver::LookupSymbolName(pc, &start); if (native_symbol_name != NULL) { @@ -511,16 +489,14 @@ class ProfilerStackWalker : public ValueObject { ProfilerStackWalker(Dart_Port port_id, Sample* head_sample, SampleBuffer* sample_buffer, - intptr_t skip_count = 0, - bool try_symbolize_dart_frames = true) + intptr_t skip_count = 0) : port_id_(port_id), sample_(head_sample), sample_buffer_(sample_buffer), skip_count_(skip_count), frames_skipped_(0), frame_index_(0), - total_frames_(0), - try_symbolize_dart_frames_(try_symbolize_dart_frames) { + total_frames_(0) { if (sample_ == NULL) { ASSERT(sample_buffer_ == NULL); } else { @@ -536,7 +512,7 @@ class ProfilerStackWalker : public ValueObject { } if (sample_ == NULL) { - DumpStackFrame(frame_index_, pc, fp, try_symbolize_dart_frames_); + DumpStackFrame(frame_index_, pc, fp); frame_index_++; total_frames_++; return true; @@ -571,7 +547,6 @@ class ProfilerStackWalker : public ValueObject { intptr_t frames_skipped_; intptr_t frame_index_; intptr_t total_frames_; - const bool try_symbolize_dart_frames_; }; // Executing Dart code, walk the stack. @@ -768,13 +743,8 @@ class ProfilerNativeStackWalker : public ProfilerStackWalker { uword pc, uword fp, uword sp, - intptr_t skip_count = 0, - bool try_symbolize_dart_frames = true) - : ProfilerStackWalker(port_id, - sample, - sample_buffer, - skip_count, - try_symbolize_dart_frames), + intptr_t skip_count = 0) + : ProfilerStackWalker(port_id, sample, sample_buffer, skip_count), counters_(counters), stack_upper_(stack_upper), original_pc_(pc), @@ -1190,13 +1160,19 @@ void Profiler::DumpStackTrace(uword sp, uword fp, uword pc, bool for_crash) { return; } - ProfilerNativeStackWalker native_stack_walker( - &counters_, ILLEGAL_PORT, NULL, NULL, stack_lower, stack_upper, pc, fp, - sp, - /*skip_count=*/0, - /*try_symbolize_dart_frames=*/!for_crash); + ProfilerNativeStackWalker native_stack_walker(&counters_, ILLEGAL_PORT, NULL, + NULL, stack_lower, stack_upper, + pc, fp, sp, + /*skip_count=*/0); native_stack_walker.walk(); OS::PrintErr("-- End of DumpStackTrace\n"); + + if (thread->execution_state() == Thread::kThreadInNative) { + TransitionNativeToVM transition(thread); + StackFrame::DumpCurrentTrace(); + } else if (thread->execution_state() == Thread::kThreadInVM) { + StackFrame::DumpCurrentTrace(); + } } void Profiler::SampleAllocation(Thread* thread, intptr_t cid) { diff --git a/runtime/vm/stack_frame.cc b/runtime/vm/stack_frame.cc index 260e99fe42e..b2ad52a8c60 100644 --- a/runtime/vm/stack_frame.cc +++ b/runtime/vm/stack_frame.cc @@ -532,6 +532,17 @@ bool StackFrame::IsValid() const { return (LookupDartCode() != Code::null()); } +void StackFrame::DumpCurrentTrace() { + StackFrameIterator frames(ValidationPolicy::kDontValidateFrames, + Thread::Current(), + StackFrameIterator::kNoCrossThreadIteration); + StackFrame* frame = frames.NextFrame(); + while (frame != nullptr) { + OS::PrintErr("%s\n", frame->ToCString()); + frame = frames.NextFrame(); + } +} + void StackFrameIterator::SetupLastExitFrameData() { ASSERT(thread_ != NULL); uword exit_marker = thread_->top_exit_frame_info(); diff --git a/runtime/vm/stack_frame.h b/runtime/vm/stack_frame.h index 69368fe3e5f..72699aeced6 100644 --- a/runtime/vm/stack_frame.h +++ b/runtime/vm/stack_frame.h @@ -130,6 +130,8 @@ class StackFrame : public ValueObject { // Returns token_pos of the pc(), or -1 if none exists. TokenPosition GetTokenPos() const; + static void DumpCurrentTrace(); + protected: explicit StackFrame(Thread* thread) : fp_(0), sp_(0), pc_(0), thread_(thread), is_interpreted_(false) {}