Move stack trace extraction code out of TickSample::Init

Make it a part of V8 API GetStackSample function.
Also expose external_callback_entry in SampleInfo to break dependency
of clients on internal V8 structures.

BUG=v8:4789

Review-Url: https://codereview.chromium.org/2007343003
Cr-Commit-Position: refs/heads/master@{#36831}

include/v8.h

@@ -1613,21 +1613,21 @@ class V8_EXPORT StackFrame {
 // A StateTag represents a possible state of the VM.
 enum StateTag { JS, GC, COMPILER, OTHER, EXTERNAL, IDLE };
 
-
 // A RegisterState represents the current state of registers used
 // by the sampling profiler API.
 struct RegisterState {
-  RegisterState() : pc(NULL), sp(NULL), fp(NULL) {}
+  RegisterState() : pc(nullptr), sp(nullptr), fp(nullptr) {}
   void* pc;  // Instruction pointer.
   void* sp;  // Stack pointer.
   void* fp;  // Frame pointer.
 };
 
-
 // The output structure filled up by GetStackSample API function.
 struct SampleInfo {
-  size_t frames_count;
-  StateTag vm_state;
+  size_t frames_count;            // Number of frames collected.
+  StateTag vm_state;              // Current VM state.
+  void* external_callback_entry;  // External callback address if VM is
+                                  // executing an external callback.
 };
 
 /**

src/profiler/profile-generator.cc

@@ -608,9 +608,8 @@ void ProfileGenerator::RecordTickSample(const TickSample& sample) {
   int src_line = v8::CpuProfileNode::kNoLineNumberInfo;
   bool src_line_not_found = true;
 
-  if (sample.pc != NULL) {
-    if (sample.has_external_callback && sample.state == EXTERNAL &&
-        sample.top_frame_type == StackFrame::EXIT) {
+  if (sample.pc != nullptr) {
+    if (sample.has_external_callback && sample.state == EXTERNAL) {
       // Don't use PC when in external callback code, as it can point
       // inside callback's code, and we will erroneously report
       // that a callback calls itself.
@@ -620,9 +619,7 @@ void ProfileGenerator::RecordTickSample(const TickSample& sample) {
       // If there is no pc_entry we're likely in native code.
       // Find out, if top of stack was pointing inside a JS function
       // meaning that we have encountered a frameless invocation.
-      if (!pc_entry && (sample.top_frame_type == StackFrame::JAVA_SCRIPT ||
-                        sample.top_frame_type == StackFrame::INTERPRETED ||
-                        sample.top_frame_type == StackFrame::OPTIMIZED)) {
+      if (!pc_entry && !sample.has_external_callback) {
         pc_entry = code_map_.FindEntry(sample.tos);
       }
       // If pc is in the function code before it set up stack frame or after the
@@ -647,7 +644,7 @@ void ProfileGenerator::RecordTickSample(const TickSample& sample) {
           // In the latter case we know the caller for sure but in the
           // former case we don't so we simply replace the frame with
           // 'unresolved' entry.
-          if (sample.top_frame_type == StackFrame::JAVA_SCRIPT) {
+          if (!sample.has_external_callback) {
             entries.push_back(unresolved_entry_);
           }
         }

src/profiler/tick-sample.cc

@@ -20,7 +20,6 @@ bool IsSamePage(byte* ptr1, byte* ptr2) {
          (reinterpret_cast<uintptr_t>(ptr2) & mask);
 }
 
-
 // Check if the code at specified address could potentially be a
 // frame setup code.
 bool IsNoFrameRegion(Address address) {
@@ -77,7 +76,6 @@ bool IsNoFrameRegion(Address address) {
 
 }  // namespace
 
-
 //
 // StackTracer implementation
 //
@@ -86,68 +84,63 @@ DISABLE_ASAN void TickSample::Init(Isolate* isolate,
                                    RecordCEntryFrame record_c_entry_frame,
                                    bool update_stats) {
   timestamp = base::TimeTicks::HighResolutionNow();
-  pc = reinterpret_cast<Address>(regs.pc);
-  state = isolate->current_vm_state();
   this->update_stats = update_stats;
 
-  // Avoid collecting traces while doing GC.
-  if (state == GC) return;
-
-  Address js_entry_sp = isolate->js_entry_sp();
-  if (js_entry_sp == 0) return;  // Not executing JS now.
-
-  if (pc && IsNoFrameRegion(pc)) {
-    // Can't collect stack. Mark the sample as spoiled.
-    timestamp = base::TimeTicks();
-    pc = 0;
-    return;
-  }
-
-  ExternalCallbackScope* scope = isolate->external_callback_scope();
-  Address handler = Isolate::handler(isolate->thread_local_top());
-  // If there is a handler on top of the external callback scope then
-  // we have already entrered JavaScript again and the external callback
-  // is not the top function.
-  if (scope && scope->scope_address() < handler) {
-    external_callback_entry = *scope->callback_entrypoint_address();
-    has_external_callback = true;
-  } else {
+  SampleInfo info;
+  if (GetStackSample(isolate, regs, record_c_entry_frame,
+                     reinterpret_cast<void**>(&stack[0]), kMaxFramesCount,
+                     &info)) {
+    state = info.vm_state;
+    pc = static_cast<Address>(regs.pc);
+    frames_count = static_cast<unsigned>(info.frames_count);
+    has_external_callback = info.external_callback_entry != nullptr;
+    if (has_external_callback) {
+      external_callback_entry =
+          static_cast<Address>(info.external_callback_entry);
+    } else if (frames_count) {
       // sp register may point at an arbitrary place in memory, make
       // sure MSAN doesn't complain about it.
       MSAN_MEMORY_IS_INITIALIZED(regs.sp, sizeof(Address));
       // Sample potential return address value for frameless invocation of
       // stubs (we'll figure out later, if this value makes sense).
       tos = Memory::Address_at(reinterpret_cast<Address>(regs.sp));
-    has_external_callback = false;
+    } else {
+      tos = nullptr;
     }
-
-  SafeStackFrameIterator it(isolate, reinterpret_cast<Address>(regs.fp),
-                            reinterpret_cast<Address>(regs.sp), js_entry_sp);
-  top_frame_type = it.top_frame_type();
-
-  SampleInfo info;
-  GetStackSample(isolate, regs, record_c_entry_frame,
-                 reinterpret_cast<void**>(&stack[0]), kMaxFramesCount, &info);
-  frames_count = static_cast<unsigned>(info.frames_count);
-  if (!frames_count) {
+  } else {
     // It is executing JS but failed to collect a stack trace.
     // Mark the sample as spoiled.
     timestamp = base::TimeTicks();
-    pc = 0;
+    pc = nullptr;
   }
 }
 
-
-void TickSample::GetStackSample(Isolate* isolate, const v8::RegisterState& regs,
+bool TickSample::GetStackSample(Isolate* isolate, const v8::RegisterState& regs,
                                 RecordCEntryFrame record_c_entry_frame,
                                 void** frames, size_t frames_limit,
                                 v8::SampleInfo* sample_info) {
   sample_info->frames_count = 0;
   sample_info->vm_state = isolate->current_vm_state();
-  if (sample_info->vm_state == GC) return;
+  sample_info->external_callback_entry = nullptr;
+  if (sample_info->vm_state == GC) return true;
 
   Address js_entry_sp = isolate->js_entry_sp();
-  if (js_entry_sp == 0) return;  // Not executing JS now.
+  if (js_entry_sp == 0) return true;  // Not executing JS now.
+
+  if (regs.pc && IsNoFrameRegion(static_cast<Address>(regs.pc))) {
+    // Can't collect stack.
+    return false;
+  }
+
+  ExternalCallbackScope* scope = isolate->external_callback_scope();
+  Address handler = Isolate::handler(isolate->thread_local_top());
+  // If there is a handler on top of the external callback scope then
+  // we have already entrered JavaScript again and the external callback
+  // is not the top function.
+  if (scope && scope->scope_address() < handler) {
+    sample_info->external_callback_entry =
+        *scope->callback_entrypoint_address();
+  }
 
   SafeStackFrameIterator it(isolate, reinterpret_cast<Address>(regs.fp),
                             reinterpret_cast<Address>(regs.sp), js_entry_sp);
@@ -172,9 +165,9 @@ void TickSample::GetStackSample(Isolate* isolate, const v8::RegisterState& regs,
     it.Advance();
   }
   sample_info->frames_count = i;
+  return true;
 }
 
-
 #if defined(USE_SIMULATOR)
 bool SimulatorHelper::FillRegisters(Isolate* isolate,
                                     v8::RegisterState* state) {

src/profiler/tick-sample.h

@@ -36,11 +36,10 @@ struct TickSample {
         external_callback_entry(NULL),
         frames_count(0),
         has_external_callback(false),
-        update_stats(true),
-        top_frame_type(StackFrame::NONE) {}
+        update_stats(true) {}
   void Init(Isolate* isolate, const v8::RegisterState& state,
             RecordCEntryFrame record_c_entry_frame, bool update_stats);
-  static void GetStackSample(Isolate* isolate, const v8::RegisterState& state,
+  static bool GetStackSample(Isolate* isolate, const v8::RegisterState& state,
                              RecordCEntryFrame record_c_entry_frame,
                              void** frames, size_t frames_limit,
                              v8::SampleInfo* sample_info);
@@ -57,7 +56,6 @@ struct TickSample {
   unsigned frames_count : kMaxFramesCountLog2;  // Number of captured frames.
   bool has_external_callback : 1;
   bool update_stats : 1;  // Whether the sample should update aggregated stats.
-  StackFrame::Type top_frame_type : 5;
 };