Dump more stack frames to perf log when executing a C++ function.

JavaScriptFrameIterator is templatized on the iterator type and renamed to JavaScriptFrameIteratorTemp.
The original JSFI is now a typedef for JavaScriptFrameIteratorTemp<StackFrameIterator>. Because of templatizing, JSFI code is moved to frames-inl.h

StackTraceFrameIterator moved to frames.*

Implemented SafeStackFrameIterator which wraps StackFrameIterator and have the same interface. It performs additional checks of stack addresses prior to delegating to StackFrameIterator. SafeSFI is used in an another specialization of JavaScriptFrameIteratorTemp template to perform safe JS frames iteration on sampler ticks.

I haven't took an advantage of having multiple stack frames in tickprocessor yet.

Review URL: http://codereview.chromium.org/39009

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@1404 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/frames-inl.h

@@ -169,7 +169,8 @@ inline bool JavaScriptFrame::has_adapted_arguments() const {
 }
 
 
-inline JavaScriptFrame* JavaScriptFrameIterator::frame() const {
+template<typename Iterator>
+inline JavaScriptFrame* JavaScriptFrameIteratorTemp<Iterator>::frame() const {
   // TODO(1233797): The frame hierarchy needs to change. It's
   // problematic that we can't use the safe-cast operator to cast to
   // the JavaScript frame type, because we may encounter arguments
@@ -180,6 +181,39 @@ inline JavaScriptFrame* JavaScriptFrameIterator::frame() const {
 }
 
 
+template<typename Iterator>
+JavaScriptFrameIteratorTemp<Iterator>::JavaScriptFrameIteratorTemp(
+    StackFrame::Id id) {
+  while (!done()) {
+    Advance();
+    if (frame()->id() == id) return;
+  }
+}
+
+
+template<typename Iterator>
+void JavaScriptFrameIteratorTemp<Iterator>::Advance() {
+  do {
+    iterator_.Advance();
+  } while (!iterator_.done() && !iterator_.frame()->is_java_script());
+}
+
+
+template<typename Iterator>
+void JavaScriptFrameIteratorTemp<Iterator>::AdvanceToArgumentsFrame() {
+  if (!frame()->has_adapted_arguments()) return;
+  iterator_.Advance();
+  ASSERT(iterator_.frame()->is_arguments_adaptor());
+}
+
+
+template<typename Iterator>
+void JavaScriptFrameIteratorTemp<Iterator>::Reset() {
+  iterator_.Reset();
+  if (!done()) Advance();
+}
+
+
 } }  // namespace v8::internal
 
 #endif  // V8_FRAMES_INL_H_

src/frames.cc

@@ -74,6 +74,11 @@ StackFrameIterator::StackFrameIterator(ThreadLocalTop* t)
       frame_(NULL), handler_(NULL), thread_(t) {
   Reset();
 }
+StackFrameIterator::StackFrameIterator(bool reset)
+    : STACK_FRAME_TYPE_LIST(INITIALIZE_SINGLETON)
+      frame_(NULL), handler_(NULL), thread_(Top::GetCurrentThread()) {
+  if (reset) Reset();
+}
 #undef INITIALIZE_SINGLETON
 
 
@@ -131,32 +136,77 @@ StackFrame* StackFrameIterator::SingletonFor(StackFrame::Type type,
 // -------------------------------------------------------------------------
 
 
-JavaScriptFrameIterator::JavaScriptFrameIterator(StackFrame::Id id) {
+StackTraceFrameIterator::StackTraceFrameIterator() {
+  if (!done() && !frame()->function()->IsJSFunction()) Advance();
+}
+
+
+void StackTraceFrameIterator::Advance() {
   while (true) {
-    Advance();
-    if (frame()->id() == id) return;
+    JavaScriptFrameIterator::Advance();
+    if (done()) return;
+    if (frame()->function()->IsJSFunction()) return;
   }
 }
 
 
-void JavaScriptFrameIterator::Advance() {
-  do {
+// -------------------------------------------------------------------------
+
+
+SafeStackFrameIterator::SafeStackFrameIterator(
+    Address low_bound, Address high_bound) :
+    low_bound_(low_bound), high_bound_(high_bound),
+    is_working_iterator_(IsInBounds(low_bound, high_bound,
+                                    Top::c_entry_fp(Top::GetCurrentThread()))),
+    iteration_done_(!is_working_iterator_), iterator_(is_working_iterator_) {
+}
+
+
+void SafeStackFrameIterator::Advance() {
+  ASSERT(is_working_iterator_);
+  ASSERT(!done());
+  StackFrame* frame = iterator_.frame();
+  iteration_done_ =
+      !IsGoodStackAddress(frame->sp()) || !IsGoodStackAddress(frame->fp());
+  if (!iteration_done_) {
     iterator_.Advance();
-  } while (!iterator_.done() && !iterator_.frame()->is_java_script());
+    if (!iterator_.done()) {
+      // Check that we have actually moved to the previous frame in the stack
+      StackFrame* prev_frame = iterator_.frame();
+      iteration_done_ =
+          prev_frame->sp() < frame->sp() || prev_frame->fp() < frame->fp();
+    }
+  }
+}
+
+
+void SafeStackFrameIterator::Reset() {
+  if (is_working_iterator_) {
+    iterator_.Reset();
+    iteration_done_ = false;
+  }
 }
 
 
-void JavaScriptFrameIterator::AdvanceToArgumentsFrame() {
-  if (!frame()->has_adapted_arguments()) return;
-  iterator_.Advance();
-  ASSERT(iterator_.frame()->is_arguments_adaptor());
+// -------------------------------------------------------------------------
+
+
+#ifdef ENABLE_LOGGING_AND_PROFILING
+SafeStackTraceFrameIterator::SafeStackTraceFrameIterator(
+    Address low_bound, Address high_bound) :
+    SafeJavaScriptFrameIterator(low_bound, high_bound) {
+  if (!done() && !frame()->function()->IsJSFunction()) Advance();
 }
 
 
-void JavaScriptFrameIterator::Reset() {
-  iterator_.Reset();
-  Advance();
+void SafeStackTraceFrameIterator::Advance() {
+  while (true) {
+    SafeJavaScriptFrameIterator::Advance();
+    if (done()) return;
+    if (frame()->function()->IsJSFunction()) return;
+  }
 }
+#endif
 
 
 // -------------------------------------------------------------------------

src/frames.h

@@ -509,6 +509,9 @@ class StackFrameIterator BASE_EMBEDDED {
   // An iterator that iterates over a given thread's stack.
   explicit StackFrameIterator(ThreadLocalTop* thread);
 
+  // An iterator that conditionally resets itself on init.
+  explicit StackFrameIterator(bool reset);
+
   StackFrame* frame() const {
     ASSERT(!done());
     return frame_;
@@ -542,16 +545,21 @@ class StackFrameIterator BASE_EMBEDDED {
 
 
 // Iterator that supports iterating through all JavaScript frames.
-class JavaScriptFrameIterator BASE_EMBEDDED {
+template<typename Iterator>
+class JavaScriptFrameIteratorTemp BASE_EMBEDDED {
  public:
-  JavaScriptFrameIterator() { if (!done()) Advance(); }
+  JavaScriptFrameIteratorTemp() { if (!done()) Advance(); }
 
-  explicit JavaScriptFrameIterator(ThreadLocalTop* thread) : iterator_(thread) {
+  explicit JavaScriptFrameIteratorTemp(ThreadLocalTop* thread) :
+      iterator_(thread) {
     if (!done()) Advance();
   }
 
   // Skip frames until the frame with the given id is reached.
-  explicit JavaScriptFrameIterator(StackFrame::Id id);
+  explicit JavaScriptFrameIteratorTemp(StackFrame::Id id);
+
+  explicit JavaScriptFrameIteratorTemp(Address low_bound, Address high_bound) :
+      iterator_(low_bound, high_bound) { if (!done()) Advance(); }
 
   inline JavaScriptFrame* frame() const;
 
@@ -567,10 +575,68 @@ class JavaScriptFrameIterator BASE_EMBEDDED {
   void Reset();
 
  private:
+  Iterator iterator_;
+};
+
+
+typedef JavaScriptFrameIteratorTemp<StackFrameIterator> JavaScriptFrameIterator;
+
+
+// NOTE: The stack trace frame iterator is an iterator that only
+// traverse proper JavaScript frames; that is JavaScript frames that
+// have proper JavaScript functions. This excludes the problematic
+// functions in runtime.js.
+class StackTraceFrameIterator: public JavaScriptFrameIterator {
+ public:
+  StackTraceFrameIterator();
+  void Advance();
+};
+
+
+class SafeStackFrameIterator BASE_EMBEDDED {
+ public:
+  explicit SafeStackFrameIterator(Address low_bound, Address high_bound);
+
+  StackFrame* frame() const {
+    ASSERT(is_working_iterator_);
+    return iterator_.frame();
+  }
+
+  bool done() const { return iteration_done_ ? true : iterator_.done(); }
+
+  void Advance();
+  void Reset();
+
+ private:
+  static bool IsInBounds(
+      Address low_bound, Address high_bound, Address addr) {
+    return low_bound <= addr && addr <= high_bound;
+  }
+  bool IsGoodStackAddress(Address addr) const {
+    return IsInBounds(low_bound_, high_bound_, addr);
+  }
+
+  Address low_bound_;
+  Address high_bound_;
+  const bool is_working_iterator_;
+  bool iteration_done_;
   StackFrameIterator iterator_;
 };
 
 
+#ifdef ENABLE_LOGGING_AND_PROFILING
+typedef JavaScriptFrameIteratorTemp<SafeStackFrameIterator>
+    SafeJavaScriptFrameIterator;
+
+
+class SafeStackTraceFrameIterator: public SafeJavaScriptFrameIterator {
+ public:
+  explicit SafeStackTraceFrameIterator(Address low_bound, Address high_bound);
+  void Advance();
+};
+#endif
+
+
 class StackFrameLocator BASE_EMBEDDED {
  public:
   // Find the nth JavaScript frame on the stack. The caller must

src/log.cc

@@ -137,14 +137,38 @@ bool Profiler::paused_ = false;
 // StackTracer implementation
 //
 void StackTracer::Trace(TickSample* sample) {
-  // Assuming that stack grows from lower addresses
-  if (sample->state != GC
-      && (sample->sp < sample->fp && sample->fp < low_stack_bound_)) {
+  if (sample->state == GC) {
+    sample->InitStack(0);
+    return;
+  }
+
+  // If c_entry_fp is available, this means that we are inside a C++
+  // function and sample->fp value isn't reliable due to FPO
+  if (Top::c_entry_fp(Top::GetCurrentThread()) != NULL) {
+    SafeStackTraceFrameIterator it(
+        reinterpret_cast<Address>(sample->sp),
+        reinterpret_cast<Address>(low_stack_bound_));
+    // Pass 1: Calculate depth
+    int depth = 0;
+    while (!it.done() && depth <= kMaxStackFrames) {
+      ++depth;
+      it.Advance();
+    }
+    // Pass 2: Save stack
+    sample->InitStack(depth);
+    if (depth > 0) {
+      it.Reset();
+      for (int i = 0; i < depth && !it.done(); ++i, it.Advance()) {
+        sample->stack[i] = it.frame()->pc();
+      }
+    }
+  } else if (sample->sp < sample->fp && sample->fp < low_stack_bound_) {
+    // The check assumes that stack grows from lower addresses
     sample->InitStack(1);
     sample->stack[0] = Memory::Address_at(
         (Address)(sample->fp + StandardFrameConstants::kCallerPCOffset));
   } else {
-    // GC runs or FP seems to be in some intermediate state,
+    // FP seems to be in some intermediate state,
     // better discard this sample
     sample->InitStack(0);
   }

src/log.h

@@ -279,6 +279,9 @@ class StackTracer BASE_EMBEDDED {
       : low_stack_bound_(low_stack_bound) { }
   void Trace(TickSample* sample);
  private:
+  // Maximum number of stack frames to capture
+  static const int kMaxStackFrames = 5;
+
   unsigned int low_stack_bound_;
 };
 

src/platform.h

@@ -481,9 +481,11 @@ class TickSample {
   }
 
   inline void InitStack(int depth) {
-    stack = SmartPointer<Address>(NewArray<Address>(depth + 1));
-    // null-terminate
-    stack[depth] = 0;
+    if (depth) {
+      stack = SmartPointer<Address>(NewArray<Address>(depth + 1));
+      // null-terminate
+      stack[depth] = 0;
+    }
   }
 };
 

src/top.cc

@@ -668,26 +668,6 @@ Object* Top::PromoteScheduledException() {
 }
 
 
-// NOTE: The stack trace frame iterator is an iterator that only
-// traverse proper JavaScript frames; that is JavaScript frames that
-// have proper JavaScript functions. This excludes the problematic
-// functions in runtime.js.
-class StackTraceFrameIterator: public JavaScriptFrameIterator {
- public:
-  StackTraceFrameIterator() {
-    if (!done() && !frame()->function()->IsJSFunction()) Advance();
-  }
-
-  void Advance() {
-    while (true) {
-      JavaScriptFrameIterator::Advance();
-      if (done()) return;
-      if (frame()->function()->IsJSFunction()) return;
-    }
-  }
-};
-
-
 void Top::PrintCurrentStackTrace(FILE* out) {
   StackTraceFrameIterator it;
   while (!it.done()) {

test/cctest/test-log-ia32.cc

@@ -43,7 +43,7 @@ static void InitTraceEnv(StackTracer* tracer, TickSample* sample) {
 static void DoTrace(unsigned int fp) {
   trace_env.sample->fp = fp;
   // something that is less than fp
-  trace_env.sample->sp = trace_env.sample->fp - sizeof(unsigned int);
+  trace_env.sample->sp = trace_env.sample->fp - 100;
   trace_env.tracer->Trace(trace_env.sample);
 }
 
@@ -217,15 +217,18 @@ TEST(PureJSStackTrace) {
       "  JSFuncDoTrace();"
       "};\n"
       "JSTrace();");
+  CHECK_NE(0, *(sample.stack));
+  CheckRetAddrIsInFunction(
+      reinterpret_cast<unsigned int>(sample.stack[0]),
+      reinterpret_cast<unsigned int>(call_trace_code->instruction_start()),
+      call_trace_code->instruction_size());
   Handle<JSFunction> js_trace(JSFunction::cast(*(v8::Utils::OpenHandle(
       *GetGlobalProperty("JSTrace")))));
   v8::internal::Code* js_trace_code = js_trace->code();
   CheckRetAddrIsInFunction(
-      reinterpret_cast<unsigned int>(sample.stack[0]),
+      reinterpret_cast<unsigned int>(sample.stack[1]),
       reinterpret_cast<unsigned int>(js_trace_code->instruction_start()),
       js_trace_code->instruction_size());
-  CHECK_EQ(0, sample.stack[1]);
 }
 
 #endif  // ENABLE_LOGGING_AND_PROFILING
-