commit d07f3edd5e19003ce941b41347921f2f35a3972c
author Vyacheslav Egorov <vegorov@google.com> Thu Oct 18 10:52:20 2018 +0000
committer commit-bot@chromium.org <commit-bot@chromium.org> Thu Oct 18 10:52:20 2018 +0000
tree d880e3465c0fc9c2bb1828476fd3b3d88223b578
parent 3039fb00311eab16a45694d9366034a4994cb737

[vm] Disable symbolization of Dart frames in Dart_DumpNativeStackTrace.

Currently symbolization is implemented through heap iteration - and
iterating a heap, which in turn requires creating HeapIterationScope and
extremely sophisticated operation that involves taking multiple locks
and waiting for threads to safepoint. This is too complex to be used
from the signal handler which is invoked in case of a crash - it can
lead to recursive crashes or worse - deadlocks.

For now we simply disable symbolization of Dart frames. In the future
we should switch symbolization to use an off the heap data structure
which can be accessed from a signal handler.

Fixes https://github.com/dart-lang/sdk/issues/34814

Bug: 34814
Change-Id: Ib905acafedf64a1b314374b65a99880f43e79b77
Reviewed-on: https://dart-review.googlesource.com/c/80623
Auto-Submit: Vyacheslav Egorov <vegorov@google.com>
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>

runtime/vm/profiler.cc

diff --git a/runtime/vm/profiler.cc b/runtime/vm/profiler.cc
index 3754ac1..7ca6fa3 100644
--- a/runtime/vm/profiler.cc
+++ b/runtime/vm/profiler.cc
@@ -410,9 +410,12 @@
   sample->Clear();
 }
 
-static void DumpStackFrame(intptr_t frame_index, uword pc) {
+static void DumpStackFrame(intptr_t frame_index,
+                           uword pc,
+                           bool try_symbolize_dart_frames) {
   Thread* thread = Thread::Current();
-  if ((thread != NULL) && !thread->IsAtSafepoint()) {
+  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
@@ -443,14 +446,16 @@
   ProfilerStackWalker(Dart_Port port_id,
                       Sample* head_sample,
                       SampleBuffer* sample_buffer,
-                      intptr_t skip_count = 0)
+                      intptr_t skip_count = 0,
+                      bool try_symbolize_dart_frames = true)
       : port_id_(port_id),
         sample_(head_sample),
         sample_buffer_(sample_buffer),
         skip_count_(skip_count),
         frames_skipped_(0),
         frame_index_(0),
-        total_frames_(0) {
+        total_frames_(0),
+        try_symbolize_dart_frames_(try_symbolize_dart_frames) {
     if (sample_ == NULL) {
       ASSERT(sample_buffer_ == NULL);
     } else {
@@ -466,7 +471,7 @@
     }
 
     if (sample_ == NULL) {
-      DumpStackFrame(frame_index_, pc);
+      DumpStackFrame(frame_index_, pc, try_symbolize_dart_frames_);
       frame_index_++;
       total_frames_++;
       return true;
@@ -501,6 +506,7 @@
   intptr_t frames_skipped_;
   intptr_t frame_index_;
   intptr_t total_frames_;
+  const bool try_symbolize_dart_frames_;
 };
 
 // Executing Dart code, walk the stack.
@@ -714,8 +720,13 @@
                             uword pc,
                             uword fp,
                             uword sp,
-                            intptr_t skip_count = 0)
-      : ProfilerStackWalker(port_id, sample, sample_buffer, skip_count),
+                            intptr_t skip_count = 0,
+                            bool try_symbolize_dart_frames = true)
+      : ProfilerStackWalker(port_id,
+                            sample,
+                            sample_buffer,
+                            skip_count,
+                            try_symbolize_dart_frames),
         stack_upper_(stack_upper),
         original_pc_(pc),
         original_fp_(fp),
@@ -1050,7 +1061,7 @@
 
 void Profiler::DumpStackTrace(void* context) {
   if (context == NULL) {
-    DumpStackTrace();
+    DumpStackTrace(/*for_crash=*/true);
     return;
   }
 #if defined(HOST_OS_LINUX) || defined(HOST_OS_MACOS) || defined(HOST_OS_ANDROID)
@@ -1059,7 +1070,7 @@
   uword pc = SignalHandler::GetProgramCounter(mcontext);
   uword fp = SignalHandler::GetFramePointer(mcontext);
   uword sp = SignalHandler::GetCStackPointer(mcontext);
-  DumpStackTrace(sp, fp, pc, true /* for_crash */);
+  DumpStackTrace(sp, fp, pc, /*for_crash=*/true);
 #elif defined(HOST_OS_WINDOWS)
   CONTEXT* ctx = reinterpret_cast<CONTEXT*>(context);
 #if defined(HOST_ARCH_IA32)
@@ -1073,7 +1084,7 @@
 #else
 #error Unsupported architecture.
 #endif
-  DumpStackTrace(sp, fp, pc, true /* for_crash */);
+  DumpStackTrace(sp, fp, pc, /*for_crash=*/true);
 #else
 // TODO(fschneider): Add support for more platforms.
 // Do nothing on unsupported platforms.
@@ -1122,7 +1133,9 @@
   }
 
   ProfilerNativeStackWalker native_stack_walker(
-      ILLEGAL_PORT, NULL, NULL, stack_lower, stack_upper, pc, fp, sp);
+      ILLEGAL_PORT, NULL, NULL, stack_lower, stack_upper, pc, fp, sp,
+      /*skip_count=*/0,
+      /*try_symbolize_dart_frames=*/!for_crash);
   native_stack_walker.walk();
   OS::PrintErr("-- End of DumpStackTrace\n");
 }