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

runtime/vm/profiler.cc

Index: runtime/vm/profiler.cc
diff --git a/runtime/vm/profiler.cc b/runtime/vm/profiler.cc
index d8ec5da69b8c6796a9570be71e38a0e308e84d52..6c5d0d747daf21c3b892c2ee9606c496dc5c8ece 100644
--- a/runtime/vm/profiler.cc
+++ b/runtime/vm/profiler.cc
@@ -910,6 +910,95 @@ class CodeRegionTable : public ValueObject {
 };
 
 
+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,
@@ -1356,6 +1445,12 @@ void Profiler::PrintJSON(Isolate* isolate, JSONStream* stream,
                                      &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);
@@ -1600,6 +1695,8 @@ class ProfilerNativeStackWalker : public ValueObject {
       // 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) {
@@ -1662,6 +1759,13 @@ class ProfilerNativeStackWalker : public ValueObject {
     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;
@@ -1672,7 +1776,6 @@ class ProfilerNativeStackWalker : public ValueObject {
     return r;
   }
 
-
   Sample* sample_;
   const uword stack_upper_;
   const uword original_pc_;
@@ -1705,6 +1808,8 @@ void Profiler::RecordSampleInterruptCallback(
   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;