Fix edge case where profiler would miss the caller of the top frame

runtime/vm/profiler.cc

 // Copyright (c) 2013, 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.
 
 #include "platform/utils.h"
 
 #include "vm/allocation.h"
 #include "vm/atomic.h"
 #include "vm/code_patcher.h"
 #include "vm/isolate.h"
@@ skipping 892 matching lines @@
                !b->contains(a->end() - 1));
       }
     }
   }
 #endif
 
   ZoneGrowableArray<CodeRegion*>* code_region_table_;
 };
 
 
+class FixTopFrameVisitor : public SampleVisitor {

[srdjan, 2014/05/02 18:20:29]

Make SampleVisitor a ValueObject if possible so that it cannot be allocated but
on the stack.

[Cutch, 2014/05/02 18:37:03]

Done.

+ public:
+  explicit FixTopFrameVisitor(Isolate* isolate)
+      : SampleVisitor(isolate),
+        code_(Code::Handle(isolate)),
+        vm_isolate_(Dart::vm_isolate()) {
+  }
+
+  void VisitSample(Sample* sample) {
+    if (sample->processed()) {
+      // Already processed.
+      return;
+    }
+    // Mark that we've processed this sample.
+    sample->set_processed(true);
+    // Lookup code object for leaf frame.
+    code_ = FindCodeForPC(sample->At(0));
+    sample->set_leaf_frame_is_dart(!code_.IsNull());
+    if (!code_.IsNull() && (code_.compile_timestamp() > sample->timestamp())) {
+      // Code compiled after sample. Ignore.
+      return;
+    }
+    if (sample->leaf_frame_is_dart()) {
+      FixMissingDartFrame(sample);
+    }
+  }
+
+ private:
+  void FixMissingDartFrame(Sample* sample) {

[srdjan, 2014/05/02 18:20:29]

s/FixMissingDartFrame/CheckForMissingDartFrame/

[Cutch, 2014/05/02 18:37:03]

Done.

+    // Some stubs (and intrinsics) do not push a frame onto the stack leaving
+    // the frame pointer in the caller.
+    //
+    // PC -> STUB
+    // FP -> DART3  <-+
+    //       DART2  <-|  <- TOP FRAME RETURN ADDRESS.
+    //       DART1  <-|
+    //       .....
+    //
+    // In this case, traversing the linked stack frames will not collect a PC
+    // inside DART3. The stack will incorrectly be: STUB, DART2, DART1.
+    // In Dart code, after pushing the FP onto the stack, an IP in the current
+    // function is pushed onto the stack as well. This stack slot is called
+    // the PC marker. We can use the PC marker to insert DART3 into the stack
+    // so that it will correctly be: STUB, DART3, DART2, DART1. Note the
+    // inserted PC may not accurately reflect the true return address from STUB.
+    ASSERT(!code_.IsNull());
+    if (sample->sp() == sample->fp()) {
+      // Haven't pushed pc marker yet.
+      return;
+    }
+    uword pc_marker = sample->pc_marker();
+    if (code_.ContainsInstructionAt(pc_marker)) {
+      // PC marker is in the same code as pc, no missing frame.
+      return;
+    }
+    if (!ContainedInDartCodeHeaps(pc_marker)) {
+      // Not a valid PC marker.
+      return;
+    }
+    sample->InsertCallerForTopFrame(pc_marker);
+  }
+
+  bool ContainedInDartCodeHeaps(uword pc) {
+    return isolate()->heap()->CodeContains(pc) ||
+           vm_isolate()->heap()->CodeContains(pc);
+  }
+
+  Isolate* vm_isolate() const {
+    return vm_isolate_;
+  }
+
+  RawCode* FindCodeForPC(uword pc) {
+    // Check current isolate for pc.
+    if (isolate()->heap()->CodeContains(pc)) {
+      return Code::LookupCode(pc);
+    }
+    // Check VM isolate for pc.
+    if (vm_isolate()->heap()->CodeContains(pc)) {
+      return Code::LookupCodeInVmIsolate(pc);
+    }
+    return Code::null();
+  }
+
+  Code& code_;

[srdjan, 2014/05/02 18:20:29]

Code*. We try to use reference with const.

+
+  Isolate* vm_isolate_;
+};
+
+
 class CodeRegionTableBuilder : public SampleVisitor {
  public:
   CodeRegionTableBuilder(Isolate* isolate,
                          CodeRegionTable* live_code_table,
                          CodeRegionTable* dead_code_table,
                          CodeRegionTable* tag_code_table)
       : SampleVisitor(isolate),
         live_code_table_(live_code_table),
         dead_code_table_(dead_code_table),
         tag_code_table_(tag_code_table),
@@ skipping 426 matching lines @@
       CodeRegionTable live_code_table;
       // Dead code holds Overwritten CodeRegions.
       CodeRegionTable dead_code_table;
       // Tag code holds Tag CodeRegions.
       CodeRegionTable tag_code_table;
       CodeRegionTableBuilder builder(isolate,
                                      &live_code_table,
                                      &dead_code_table,
                                      &tag_code_table);
       {
+        // Preprocess samples and fix the caller when the top PC is in a
+        // stub or intrinsic without a frame.
+        FixTopFrameVisitor fixTopFrame(isolate);
+        sample_buffer->VisitSamplesByReference(&fixTopFrame);
+      }
+      {
         // Build CodeRegion tables.
         ScopeStopwatch sw("CodeRegionTableBuilder");
         sample_buffer->VisitSamples(&builder);
       }
       intptr_t samples = builder.visited();
       intptr_t frames = builder.frames();
       if (FLAG_trace_profiled_isolates) {
         intptr_t total_live_code_objects = live_code_table.Length();
         intptr_t total_dead_code_objects = dead_code_table.Length();
         intptr_t total_tag_code_objects = tag_code_table.Length();
@@ skipping 224 matching lines @@
     if (gap >= kMaxStep) {
       // Gap between frame pointer and stack pointer is
       // too large.
       return 1;
     }
     if (original_sp_ < lower_bound_) {
       // The stack pointer gives us a better lower bound than
       // the isolates stack limit.
       lower_bound_ = original_sp_;
     }
+    // Store the PC marker for the top frame.
+    sample_->set_pc_marker(GetCurrentFramePcMarker(fp));
     int i = 0;
     for (; i < FLAG_profile_depth; i++) {
       if (FLAG_profile_verify_stack_walk) {
         VerifyCodeAddress(heap, i, reinterpret_cast<uword>(pc));
       }
       sample_->SetAt(i, reinterpret_cast<uword>(pc));
       if (fp == NULL) {
         return i + 1;
       }
       if (!ValidFramePointer(fp)) {
@@ skipping 42 matching lines @@
   uword* CallerPC(uword* fp) const {
     ASSERT(fp != NULL);
     return reinterpret_cast<uword*>(*(fp + kSavedCallerPcSlotFromFp));
   }
 
   uword* CallerFP(uword* fp) const {
     ASSERT(fp != NULL);
     return reinterpret_cast<uword*>(*(fp + kSavedCallerFpSlotFromFp));
   }
 
+  uword GetCurrentFramePcMarker(uword* fp) {
+    if (!ValidFramePointer(fp)) {
+      return 0;
+    }
+    return *(fp + kPcMarkerSlotFromFp);
+  }
+
   bool ValidFramePointer(uword* fp) const {
     if (fp == NULL) {
       return false;
     }
     uword cursor = reinterpret_cast<uword>(fp);
     cursor += sizeof(fp);
     bool r = cursor >= lower_bound_ && cursor < stack_upper_;
     return r;
   }
 
-
   Sample* sample_;
   const uword stack_upper_;
   const uword original_pc_;
   const uword original_fp_;
   const uword original_sp_;
   uword lower_bound_;
 };
 
 
 void Profiler::RecordSampleInterruptCallback(
@@ skipping 12 matching lines @@
     return;
   }
   SampleBuffer* sample_buffer = profiler_data->sample_buffer();
   if (sample_buffer == NULL) {
     return;
   }
   Sample* sample = sample_buffer->ReserveSample();
   sample->Init(isolate, OS::GetCurrentTimeMicros(), state.tid);
   sample->set_vm_tag(isolate->vm_tag());
   sample->set_user_tag(isolate->user_tag());
+  sample->set_sp(state.sp);
+  sample->set_fp(state.fp);
   if (FLAG_profile_native_stack) {
     // Collect native and Dart frames.
     uword stack_lower = 0;
     uword stack_upper = 0;
     isolate->GetStackBounds(&stack_lower, &stack_upper);
     if ((stack_lower == 0) || (stack_upper == 0)) {
       stack_lower = 0;
       stack_upper = 0;
     }
     ProfilerNativeStackWalker stackWalker(sample, stack_lower, stack_upper,
                                           state.pc, state.fp, state.sp);
     stackWalker.walk(isolate->heap());
   } else {
     if ((isolate->top_exit_frame_info() != 0) &&
         (isolate->stub_code() != NULL)) {
       ProfilerDartStackWalker stackWalker(sample);
       stackWalker.walk();
     } else {
       // TODO(johnmccutchan): Support collecting only Dart frames with
       // ProfilerNativeStackWalker.
     }
   }
 }
 
 }  // namespace dart