Support profiler stack sampling in any situation. After this change, almost...

Support profiler stack sampling in any situation. After this change, almost all profiler ticks (except GC ones) have a stack sample data associated.

Tested under Linux, OS X, and Windows.

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

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

SConstruct

@@ -574,7 +574,7 @@ def BuildSpecific(env, mode, env_overrides):
   library_name = 'v8' + suffix
   env['LIBRARY'] = library_name
 
-  # Build the object files by invoking SCons recursively.  
+  # Build the object files by invoking SCons recursively.
   (object_files, shell_files, mksnapshot) = env.SConscript(
     join('src', 'SConscript'),
     build_dir=join('obj', target_id),
@@ -596,7 +596,7 @@ def BuildSpecific(env, mode, env_overrides):
     pdb_name = library_name + '.dll.pdb'
     library = env.SharedLibrary(library_name, object_files, PDB=pdb_name)
   context.library_targets.append(library)
-  
+
   d8_env = Environment()
   d8_env.Replace(**context.flags['d8'])
   shell = d8_env.Program('d8' + suffix, object_files + shell_files)
@@ -616,7 +616,7 @@ def BuildSpecific(env, mode, env_overrides):
     sample_program = sample_env.Program(sample_name, sample_object)
     sample_env.Depends(sample_program, library)
     context.sample_targets.append(sample_program)
-  
+
   cctest_program = env.SConscript(
     join('test', 'cctest', 'SConscript'),
     build_dir=join('obj', 'test', target_id),
@@ -624,7 +624,7 @@ def BuildSpecific(env, mode, env_overrides):
     duplicate=False
   )
   context.cctest_targets.append(cctest_program)
-  
+
   return context
 
 

src/frames-arm.h

@@ -154,11 +154,9 @@ class InternalFrameConstants : public AllStatic {
 };
 
 
-inline Object* JavaScriptFrame::function() const {
+inline Object* JavaScriptFrame::function_slot_object() const {
   const int offset = JavaScriptFrameConstants::kFunctionOffset;
-  Object* result = Memory::Object_at(fp() + offset);
-  ASSERT(result->IsJSFunction());
-  return result;
+  return Memory::Object_at(fp() + offset);
 }
 
 

src/frames-ia32.h

@@ -129,11 +129,9 @@ class InternalFrameConstants : public AllStatic {
 };
 
 
-inline Object* JavaScriptFrame::function() const {
+inline Object* JavaScriptFrame::function_slot_object() const {
   const int offset = JavaScriptFrameConstants::kFunctionOffset;
-  Object* result = Memory::Object_at(fp() + offset);
-  ASSERT(result->IsJSFunction());
-  return result;
+  return Memory::Object_at(fp() + offset);
 }
 
 

src/frames-inl.h

@@ -169,6 +169,20 @@ inline bool JavaScriptFrame::has_adapted_arguments() const {
 }
 
 
+inline bool JavaScriptFrame::is_at_function() const {
+  Object* result = function_slot_object();
+  return Heap::Contains(reinterpret_cast<Address>(result)) &&
+      result->IsJSFunction();
+}
+
+
+inline Object* JavaScriptFrame::function() const {
+  Object* result = function_slot_object();
+  ASSERT(result->IsJSFunction());
+  return result;
+}
+
+
 template<typename Iterator>
 inline JavaScriptFrame* JavaScriptFrameIteratorTemp<Iterator>::frame() const {
   // TODO(1233797): The frame hierarchy needs to change. It's

src/frames.cc

@@ -66,23 +66,32 @@ class StackHandlerIterator BASE_EMBEDDED {
 #define INITIALIZE_SINGLETON(type, field) field##_(this),
 StackFrameIterator::StackFrameIterator()
     : STACK_FRAME_TYPE_LIST(INITIALIZE_SINGLETON)
-      frame_(NULL), handler_(NULL), thread_(Top::GetCurrentThread()) {
+      frame_(NULL), handler_(NULL), thread_(Top::GetCurrentThread()),
+      fp_(NULL), sp_(NULL), advance_(&StackFrameIterator::AdvanceWithHandler) {
   Reset();
 }
 StackFrameIterator::StackFrameIterator(ThreadLocalTop* t)
     : STACK_FRAME_TYPE_LIST(INITIALIZE_SINGLETON)
-      frame_(NULL), handler_(NULL), thread_(t) {
+      frame_(NULL), handler_(NULL), thread_(t),
+      fp_(NULL), sp_(NULL), advance_(&StackFrameIterator::AdvanceWithHandler) {
   Reset();
 }
-StackFrameIterator::StackFrameIterator(bool reset)
+StackFrameIterator::StackFrameIterator(bool use_top, Address fp, Address sp)
     : STACK_FRAME_TYPE_LIST(INITIALIZE_SINGLETON)
-      frame_(NULL), handler_(NULL), thread_(Top::GetCurrentThread()) {
-  if (reset) Reset();
+      frame_(NULL), handler_(NULL),
+      thread_(use_top ? Top::GetCurrentThread() : NULL),
+      fp_(use_top ? NULL : fp), sp_(sp),
+      advance_(use_top ? &StackFrameIterator::AdvanceWithHandler :
+               &StackFrameIterator::AdvanceWithoutHandler) {
+  if (use_top || fp != NULL) {
+    Reset();
+  }
 }
+
 #undef INITIALIZE_SINGLETON
 
 
-void StackFrameIterator::Advance() {
+void StackFrameIterator::AdvanceWithHandler() {
   ASSERT(!done());
   // Compute the state of the calling frame before restoring
   // callee-saved registers and unwinding handlers. This allows the
@@ -105,17 +114,45 @@ void StackFrameIterator::Advance() {
 }
 
 
+void StackFrameIterator::AdvanceWithoutHandler() {
+  // A simpler version of Advance which doesn't care about handler.
+  ASSERT(!done());
+  StackFrame::State state;
+  StackFrame::Type type = frame_->GetCallerState(&state);
+  frame_ = SingletonFor(type, &state);
+}
+
+
 void StackFrameIterator::Reset() {
-  Address fp = Top::c_entry_fp(thread_);
   StackFrame::State state;
-  StackFrame::Type type = ExitFrame::GetStateForFramePointer(fp, &state);
+  StackFrame::Type type;
+  if (thread_ != NULL) {
+    type = ExitFrame::GetStateForFramePointer(Top::c_entry_fp(thread_), &state);
+    handler_ = StackHandler::FromAddress(Top::handler(thread_));
+  } else {
+    ASSERT(fp_ != NULL);
+    state.fp = fp_;
+    state.sp = sp_;
+    state.pc_address =
+        reinterpret_cast<Address*>(StandardFrame::ComputePCAddress(fp_));
+    type = StackFrame::ComputeType(&state);
+    if (SingletonFor(type) == NULL) return;
+  }
   frame_ = SingletonFor(type, &state);
-  handler_ = StackHandler::FromAddress(Top::handler(thread_));
 }
 
 
 StackFrame* StackFrameIterator::SingletonFor(StackFrame::Type type,
                                              StackFrame::State* state) {
+  if (type == StackFrame::NONE) return NULL;
+  StackFrame* result = SingletonFor(type);
+  ASSERT(result != NULL);
+  result->state_ = *state;
+  return result;
+}
+
+
+StackFrame* StackFrameIterator::SingletonFor(StackFrame::Type type) {
 #define FRAME_TYPE_CASE(type, field) \
   case StackFrame::type: result = &field##_; break;
 
@@ -125,8 +162,6 @@ StackFrame* StackFrameIterator::SingletonFor(StackFrame::Type type,
     STACK_FRAME_TYPE_LIST(FRAME_TYPE_CASE)
     default: break;
   }
-  ASSERT(result != NULL);
-  result->state_ = *state;
   return result;
 
 #undef FRAME_TYPE_CASE
@@ -154,29 +189,50 @@ void StackTraceFrameIterator::Advance() {
 
 
 SafeStackFrameIterator::SafeStackFrameIterator(
-    Address low_bound, Address high_bound) :
+    Address fp, Address sp, 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_) {
+    is_valid_top_(
+        IsWithinBounds(low_bound, high_bound,
+                       Top::c_entry_fp(Top::GetCurrentThread())) &&
+        Top::handler(Top::GetCurrentThread()) != NULL),
+    is_valid_fp_(IsWithinBounds(low_bound, high_bound, fp)),
+    is_working_iterator_(is_valid_top_ || is_valid_fp_),
+    iteration_done_(!is_working_iterator_),
+    iterator_(is_valid_top_, is_valid_fp_ ? fp : NULL, sp) {
 }
 
 
 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();
-    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();
-    }
-  }
+  StackFrame* last_frame = iterator_.frame();
+  Address last_sp = last_frame->sp(), last_fp = last_frame->fp();
+  // Before advancing to the next stack frame, perform pointer validity tests
+  iteration_done_ = !IsValidFrame(last_frame) || !IsValidCaller(last_frame);
+  if (iteration_done_) return;
+
+  iterator_.Advance();
+  if (iterator_.done()) return;
+  // Check that we have actually moved to the previous frame in the stack
+  StackFrame* prev_frame = iterator_.frame();
+  iteration_done_ = prev_frame->sp() < last_sp || prev_frame->fp() < last_fp;
+}
+
+
+bool SafeStackFrameIterator::IsValidFrame(StackFrame* frame) const {
+  return IsValidStackAddress(frame->sp()) && IsValidStackAddress(frame->fp()) &&
+      // JavaScriptFrame uses function shared info to advance, hence it must
+      // point to a valid function object.
+      (!frame->is_java_script() ||
+       reinterpret_cast<JavaScriptFrame*>(frame)->is_at_function());
+}
+
+
+bool SafeStackFrameIterator::IsValidCaller(StackFrame* frame) {
+  StackFrame::State state;
+  frame->ComputeCallerState(&state);
+  return IsValidStackAddress(state.sp) && IsValidStackAddress(state.fp) &&
+      iterator_.SingletonFor(frame->GetCallerState(&state)) != NULL;
 }
 
 
@@ -193,9 +249,9 @@ void SafeStackFrameIterator::Reset() {
 
 #ifdef ENABLE_LOGGING_AND_PROFILING
 SafeStackTraceFrameIterator::SafeStackTraceFrameIterator(
-    Address low_bound, Address high_bound) :
-    SafeJavaScriptFrameIterator(low_bound, high_bound) {
-  if (!done() && !frame()->function()->IsJSFunction()) Advance();
+    Address fp, Address sp, Address low_bound, Address high_bound) :
+    SafeJavaScriptFrameIterator(fp, sp, low_bound, high_bound) {
+  if (!done() && !frame()->is_at_function()) Advance();
 }
 
 
@@ -203,7 +259,7 @@ void SafeStackTraceFrameIterator::Advance() {
   while (true) {
     SafeJavaScriptFrameIterator::Advance();
     if (done()) return;
-    if (frame()->function()->IsJSFunction()) return;
+    if (frame()->is_at_function()) return;
   }
 }
 #endif
@@ -279,11 +335,22 @@ void StackFrame::Uncook() {
 }
 
 
+StackFrame::Type StackFrame::GetCallerState(State* state) const {
+  ComputeCallerState(state);
+  return ComputeType(state);
+}
+
+
 Code* EntryFrame::code() const {
   return Heap::js_entry_code();
 }
 
 
+void EntryFrame::ComputeCallerState(State* state) const {
+  GetCallerState(state);
+}
+
+
 StackFrame::Type EntryFrame::GetCallerState(State* state) const {
   const int offset = EntryFrameConstants::kCallerFPOffset;
   Address fp = Memory::Address_at(this->fp() + offset);
@@ -301,13 +368,12 @@ Code* ExitFrame::code() const {
 }
 
 
-StackFrame::Type ExitFrame::GetCallerState(State* state) const {
+void ExitFrame::ComputeCallerState(State* state) const {
   // Setup the caller state.
   state->sp = pp();
   state->fp = Memory::Address_at(fp() + ExitFrameConstants::kCallerFPOffset);
   state->pc_address
       = reinterpret_cast<Address*>(fp() + ExitFrameConstants::kCallerPCOffset);
-  return ComputeType(state);
 }
 
 
@@ -338,11 +404,10 @@ int StandardFrame::ComputeExpressionsCount() const {
 }
 
 
-StackFrame::Type StandardFrame::GetCallerState(State* state) const {
+void StandardFrame::ComputeCallerState(State* state) const {
   state->sp = caller_sp();
   state->fp = caller_fp();
   state->pc_address = reinterpret_cast<Address*>(ComputePCAddress(fp()));
-  return ComputeType(state);
 }
 
 

src/frames.h

@@ -190,8 +190,11 @@ class StackFrame BASE_EMBEDDED {
   const StackFrameIterator* iterator_;
   State state_;
 
+  // Fill in the state of the calling frame.
+  virtual void ComputeCallerState(State* state) const = 0;
+
   // Get the type and the state of the calling frame.
-  virtual Type GetCallerState(State* state) const = 0;
+  virtual Type GetCallerState(State* state) const;
 
   // Cooking/uncooking support.
   void Cook();
@@ -199,6 +202,7 @@ class StackFrame BASE_EMBEDDED {
 
   friend class StackFrameIterator;
   friend class StackHandlerIterator;
+  friend class SafeStackFrameIterator;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(StackFrame);
 };
@@ -228,6 +232,7 @@ class EntryFrame: public StackFrame {
   virtual Address GetCallerStackPointer() const { return 0; }
 
  private:
+  virtual void ComputeCallerState(State* state) const;
   virtual Type GetCallerState(State* state) const;
 
   friend class StackFrameIterator;
@@ -280,7 +285,7 @@ class ExitFrame: public StackFrame {
   virtual Address GetCallerStackPointer() const;
 
  private:
-  virtual Type GetCallerState(State* state) const;
+  virtual void ComputeCallerState(State* state) const;
 
   friend class StackFrameIterator;
 };
@@ -328,7 +333,7 @@ class StandardFrame: public StackFrame {
   explicit StandardFrame(StackFrameIterator* iterator)
       : StackFrame(iterator) { }
 
-  virtual Type GetCallerState(State* state) const;
+  virtual void ComputeCallerState(State* state) const;
 
   // Accessors.
   inline Address caller_sp() const;
@@ -368,6 +373,7 @@ class JavaScriptFrame: public StandardFrame {
   virtual Type type() const { return JAVA_SCRIPT; }
 
   // Accessors.
+  inline bool is_at_function() const;
   inline Object* function() const;
   inline Object* receiver() const;
   inline void set_receiver(Object* value);
@@ -413,6 +419,8 @@ class JavaScriptFrame: public StandardFrame {
   virtual Address GetCallerStackPointer() const;
 
  private:
+  inline Object* function_slot_object() const;
+
   friend class StackFrameIterator;
 };
 
@@ -509,8 +517,10 @@ 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);
+  // An iterator that can start from a given FP address.
+  // If use_top, then work as usual, if fp isn't NULL, use it,
+  // otherwise, do nothing.
+  StackFrameIterator(bool use_top, Address fp, Address sp);
 
   StackFrame* frame() const {
     ASSERT(!done());
@@ -518,7 +528,7 @@ class StackFrameIterator BASE_EMBEDDED {
   }
 
   bool done() const { return frame_ == NULL; }
-  void Advance();
+  void Advance() { (this->*advance_)(); }
 
   // Go back to the first frame.
   void Reset();
@@ -530,6 +540,9 @@ class StackFrameIterator BASE_EMBEDDED {
   StackFrame* frame_;
   StackHandler* handler_;
   ThreadLocalTop* thread_;
+  Address fp_;
+  Address sp_;
+  void (StackFrameIterator::*advance_)();
 
   StackHandler* handler() const {
     ASSERT(!done());
@@ -538,8 +551,14 @@ class StackFrameIterator BASE_EMBEDDED {
 
   // Get the type-specific frame singleton in a given state.
   StackFrame* SingletonFor(StackFrame::Type type, StackFrame::State* state);
+  // A helper function, can return a NULL pointer.
+  StackFrame* SingletonFor(StackFrame::Type type);
+
+  void AdvanceWithHandler();
+  void AdvanceWithoutHandler();
 
   friend class StackFrame;
+  friend class SafeStackFrameIterator;
   DISALLOW_COPY_AND_ASSIGN(StackFrameIterator);
 };
 
@@ -558,8 +577,11 @@ class JavaScriptFrameIteratorTemp BASE_EMBEDDED {
   // Skip frames until the frame with the given id is reached.
   explicit JavaScriptFrameIteratorTemp(StackFrame::Id id);
 
-  explicit JavaScriptFrameIteratorTemp(Address low_bound, Address high_bound) :
-      iterator_(low_bound, high_bound) { if (!done()) Advance(); }
+  JavaScriptFrameIteratorTemp(Address fp, Address sp,
+                              Address low_bound, Address high_bound) :
+      iterator_(fp, sp, low_bound, high_bound) {
+    if (!done()) Advance();
+  }
 
   inline JavaScriptFrame* frame() const;
 
@@ -595,7 +617,8 @@ class StackTraceFrameIterator: public JavaScriptFrameIterator {
 
 class SafeStackFrameIterator BASE_EMBEDDED {
  public:
-  explicit SafeStackFrameIterator(Address low_bound, Address high_bound);
+  SafeStackFrameIterator(Address fp, Address sp,
+                         Address low_bound, Address high_bound);
 
   StackFrame* frame() const {
     ASSERT(is_working_iterator_);
@@ -608,16 +631,20 @@ class SafeStackFrameIterator BASE_EMBEDDED {
   void Reset();
 
  private:
-  static bool IsInBounds(
+  static bool IsWithinBounds(
       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);
+  bool IsValidStackAddress(Address addr) const {
+    return IsWithinBounds(low_bound_, high_bound_, addr);
   }
+  bool IsValidFrame(StackFrame* frame) const;
+  bool IsValidCaller(StackFrame* frame);
 
   Address low_bound_;
   Address high_bound_;
+  const bool is_valid_top_;
+  const bool is_valid_fp_;
   const bool is_working_iterator_;
   bool iteration_done_;
   StackFrameIterator iterator_;
@@ -631,7 +658,8 @@ typedef JavaScriptFrameIteratorTemp<SafeStackFrameIterator>
 
 class SafeStackTraceFrameIterator: public SafeJavaScriptFrameIterator {
  public:
-  explicit SafeStackTraceFrameIterator(Address low_bound, Address high_bound);
+  explicit SafeStackTraceFrameIterator(Address fp, Address sp,
+                                       Address low_bound, Address high_bound);
   void Advance();
 };
 #endif

src/log.cc

@@ -142,28 +142,17 @@ void StackTracer::Trace(TickSample* sample) {
     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_));
-    int i = 0;
-    while (!it.done() && i < TickSample::kMaxFramesCount) {
-      sample->stack[i++] = it.frame()->pc();
-      it.Advance();
-    }
-    sample->frames_count = i;
-  } else if (sample->sp < sample->fp && sample->fp < low_stack_bound_) {
-    // The check assumes that stack grows from lower addresses.
-    sample->stack[0] = Memory::Address_at(
-        (Address)(sample->fp + StandardFrameConstants::kCallerPCOffset));
-    sample->frames_count = 1;
-  } else {
-    // FP seems to be in some intermediate state,
-    // better discard this sample
-    sample->frames_count = 0;
+  SafeStackTraceFrameIterator it(
+      reinterpret_cast<Address>(sample->fp),
+      reinterpret_cast<Address>(sample->sp),
+      reinterpret_cast<Address>(sample->sp),
+      reinterpret_cast<Address>(low_stack_bound_));
+  int i = 0;
+  while (!it.done() && i < TickSample::kMaxFramesCount) {
+    sample->stack[i++] = it.frame()->pc();
+    it.Advance();
   }
+  sample->frames_count = i;
 }
 
 
@@ -936,7 +925,7 @@ void Logger::TickEvent(TickSample* sample, bool overflow) {
     msg.Append(",overflow");
   }
   for (int i = 0; i < sample->frames_count; ++i) {
-    msg.Append(",%p", sample->stack[i]);
+    msg.Append(",0x%x", reinterpret_cast<uint32_t>(sample->stack[i]));
   }
   msg.Append('\n');
   msg.WriteToLogFile();

src/log.h

@@ -270,7 +270,7 @@ class Logger {
 };
 
 
-// Class that extracts stack trace, used for profiling
+// Class that extracts stack trace, used for profiling.
 class StackTracer BASE_EMBEDDED {
  public:
   explicit StackTracer(unsigned int low_stack_bound)

src/platform-win32.cc

@@ -1750,7 +1750,6 @@ class Sampler::PlatformData : public Malloced {
         SuspendThread(profiled_thread_);
         context.ContextFlags = CONTEXT_FULL;
         GetThreadContext(profiled_thread_, &context);
-        ResumeThread(profiled_thread_);
         // Invoke tick handler with program counter and stack pointer.
         sample.pc = context.Eip;
         sample.sp = context.Esp;
@@ -1761,6 +1760,10 @@ class Sampler::PlatformData : public Malloced {
       sample.state = Logger::state();
       sampler_->Tick(&sample);
 
+      if (sampler_->IsProfiling()) {
+        ResumeThread(profiled_thread_);
+      }
+
       // Wait until next sampling.
       Sleep(sampler_->interval_);
     }

test/cctest/test-log-ia32.cc

@@ -9,6 +9,7 @@
 #include "v8.h"
 
 #include "log.h"
+#include "top.h"
 #include "cctest.h"
 
 using v8::Function;
@@ -23,6 +24,7 @@ using v8::internal::Handle;
 using v8::internal::JSFunction;
 using v8::internal::StackTracer;
 using v8::internal::TickSample;
+using v8::internal::Top;
 
 
 static v8::Persistent<v8::Context> env;
@@ -31,7 +33,7 @@ static v8::Persistent<v8::Context> env;
 static struct {
   StackTracer* tracer;
   TickSample* sample;
-} trace_env;
+} trace_env = { NULL, NULL };
 
 
 static void InitTraceEnv(StackTracer* tracer, TickSample* sample) {
@@ -42,62 +44,43 @@ 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 - 100;
+  // sp is only used to define stack high bound
+  trace_env.sample->sp =
+      reinterpret_cast<unsigned int>(trace_env.sample) - 10240;
   trace_env.tracer->Trace(trace_env.sample);
 }
 
 
-static void CFuncDoTrace() {
-  unsigned int fp;
-#ifdef __GNUC__
-  fp = reinterpret_cast<unsigned int>(__builtin_frame_address(0));
-#elif defined _MSC_VER
-  __asm mov [fp], ebp  // NOLINT
-#endif
+// Hide c_entry_fp to emulate situation when sampling is done while
+// pure JS code is being executed
+static void DoTraceHideCEntryFPAddress(unsigned int fp) {
+  v8::internal::Address saved_c_frame_fp = *(Top::c_entry_fp_address());
+  CHECK(saved_c_frame_fp);
+  *(Top::c_entry_fp_address()) = 0;
   DoTrace(fp);
+  *(Top::c_entry_fp_address()) = saved_c_frame_fp;
 }
 
 
-static void CFunc(int i) {
-  for (int j = i; j >= 0; --j) {
-    CFuncDoTrace();
-  }
-}
-
-
-static void CheckRetAddrIsInFunction(unsigned int ret_addr,
+static void CheckRetAddrIsInFunction(const char* func_name,
+                                     unsigned int ret_addr,
                                      unsigned int func_start_addr,
                                      unsigned int func_len) {
-  printf("CheckRetAddrIsInFunction: %08x %08x %08x\n",
-         func_start_addr, ret_addr, func_start_addr + func_len);
+  printf("CheckRetAddrIsInFunction \"%s\": %08x %08x %08x\n",
+         func_name, func_start_addr, ret_addr, func_start_addr + func_len);
   CHECK_GE(ret_addr, func_start_addr);
   CHECK_GE(func_start_addr + func_len, ret_addr);
 }
 
 
-#ifdef DEBUG
-static const int kMaxCFuncLen = 0x40;  // seems enough for a small C function
-
-static void CheckRetAddrIsInCFunction(unsigned int ret_addr,
-                                      unsigned int func_start_addr) {
-  CheckRetAddrIsInFunction(ret_addr, func_start_addr, kMaxCFuncLen);
-}
-#endif
-
-
-TEST(PureCStackTrace) {
-  TickSample sample;
-  StackTracer tracer(reinterpret_cast<unsigned int>(&sample));
-  InitTraceEnv(&tracer, &sample);
-  CFunc(0);
-#ifdef DEBUG
-  // C stack trace works only in debug mode, in release mode EBP is
-  // usually treated as a general-purpose register
-  CHECK_GT(sample.frames_count, 0);
-  CheckRetAddrIsInCFunction(reinterpret_cast<unsigned int>(sample.stack[0]),
-                            reinterpret_cast<unsigned int>(&CFunc));
-#endif
+static void CheckRetAddrIsInJSFunction(const char* func_name,
+                                       unsigned int ret_addr,
+                                       Handle<JSFunction> func) {
+  v8::internal::Code* func_code = func->code();
+  CheckRetAddrIsInFunction(
+      func_name, ret_addr,
+      reinterpret_cast<unsigned int>(func_code->instruction_start()),
+      func_code->ExecutableSize());
 }
 
 
@@ -107,27 +90,49 @@ class TraceExtension : public v8::Extension {
  public:
   TraceExtension() : v8::Extension("v8/trace", kSource) { }
   virtual v8::Handle<v8::FunctionTemplate> GetNativeFunction(
-      v8::Handle<v8::String> name);
+      v8::Handle<String> name);
   static v8::Handle<v8::Value> Trace(const v8::Arguments& args);
+  static v8::Handle<v8::Value> JSTrace(const v8::Arguments& args);
  private:
+  static unsigned int GetFP(const v8::Arguments& args);
   static const char* kSource;
 };
 
 
-const char* TraceExtension::kSource = "native function trace();";
+const char* TraceExtension::kSource =
+    "native function trace();"
+    "native function js_trace();";
 
 
 v8::Handle<v8::FunctionTemplate> TraceExtension::GetNativeFunction(
-    v8::Handle<v8::String> str) {
-  return v8::FunctionTemplate::New(TraceExtension::Trace);
+    v8::Handle<String> name) {
+  if (name->Equals(String::New("trace"))) {
+    return v8::FunctionTemplate::New(TraceExtension::Trace);
+  } else if (name->Equals(String::New("js_trace"))) {
+    return v8::FunctionTemplate::New(TraceExtension::JSTrace);
+  } else {
+    CHECK(false);
+    return v8::Handle<v8::FunctionTemplate>();
+  }
 }
 
 
-v8::Handle<v8::Value> TraceExtension::Trace(const v8::Arguments& args) {
+unsigned int TraceExtension::GetFP(const v8::Arguments& args) {
   CHECK_EQ(1, args.Length());
   unsigned int fp = args[0]->Int32Value() << 2;
   printf("Trace: %08x\n", fp);
-  DoTrace(fp);
+  return fp;
+}
+
+
+v8::Handle<v8::Value> TraceExtension::Trace(const v8::Arguments& args) {
+  DoTrace(GetFP(args));
+  return v8::Undefined();
+}
+
+
+v8::Handle<v8::Value> TraceExtension::JSTrace(const v8::Arguments& args) {
+  DoTraceHideCEntryFPAddress(GetFP(args));
   return v8::Undefined();
 }
 
@@ -163,6 +168,21 @@ static Local<Value> GetGlobalProperty(const char* name) {
 }
 
 
+static Handle<JSFunction> GetGlobalJSFunction(const char* name) {
+  Handle<JSFunction> js_func(JSFunction::cast(
+                                 *(v8::Utils::OpenHandle(
+                                       *GetGlobalProperty(name)))));
+  return js_func;
+}
+
+
+static void CheckRetAddrIsInJSFunction(const char* func_name,
+                                       unsigned int ret_addr) {
+  CheckRetAddrIsInJSFunction(func_name, ret_addr,
+                             GetGlobalJSFunction(func_name));
+}
+
+
 static void SetGlobalProperty(const char* name, Local<Value> value) {
   env->Global()->Set(String::New(name), value);
 }
@@ -188,17 +208,17 @@ static bool Patch(byte* from,
 }
 
 
-TEST(PureJSStackTrace) {
-  TickSample sample;
-  StackTracer tracer(reinterpret_cast<unsigned int>(&sample));
-  InitTraceEnv(&tracer, &sample);
-
-  InitializeVM();
-  v8::HandleScope scope;
-  Handle<JSFunction> call_trace = CompileFunction("trace(0x6666);");
-  CHECK(!call_trace.is_null());
-  v8::internal::Code* call_trace_code = call_trace->code();
-  CHECK(call_trace_code->IsCode());
+// Creates a global function named 'func_name' that calls the tracing
+// function 'trace_func_name' with an actual EBP register value,
+// shifted right to be presented as Smi.
+static void CreateTraceCallerFunction(const char* func_name,
+                                      const char* trace_func_name) {
+  ::v8::internal::EmbeddedVector<char, 256> trace_call_buf;
+  ::v8::internal::OS::SNPrintF(trace_call_buf, "%s(0x6666);", trace_func_name);
+  Handle<JSFunction> func = CompileFunction(trace_call_buf.start());
+  CHECK(!func.is_null());
+  v8::internal::Code* func_code = func->code();
+  CHECK(func_code->IsCode());
 
   // push 0xcccc (= 0x6666 << 1)
   byte original[] = { 0x68, 0xcc, 0xcc, 0x00, 0x00 };
@@ -206,29 +226,89 @@ TEST(PureJSStackTrace) {
   byte patch[] = { 0x89, 0xe8, 0xd1, 0xe8, 0x50 };
   // Patch generated code to replace pushing of a constant with
   // pushing of ebp contents in a Smi
-  CHECK(Patch(call_trace_code->instruction_start(),
-              call_trace_code->instruction_size(),
+  CHECK(Patch(func_code->instruction_start(),
+              func_code->instruction_size(),
               original, patch, sizeof(patch)));
 
-  SetGlobalProperty("JSFuncDoTrace", v8::ToApi<Value>(call_trace));
+  SetGlobalProperty(func_name, v8::ToApi<Value>(func));
+}
+
 
+TEST(CFromJSStackTrace) {
+  TickSample sample;
+  StackTracer tracer(reinterpret_cast<unsigned int>(&sample));
+  InitTraceEnv(&tracer, &sample);
+
+  InitializeVM();
+  v8::HandleScope scope;
+  CreateTraceCallerFunction("JSFuncDoTrace", "trace");
   CompileRun(
       "function JSTrace() {"
       "  JSFuncDoTrace();"
       "};\n"
       "JSTrace();");
   CHECK_GT(sample.frames_count, 1);
-  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[1]),
-      reinterpret_cast<unsigned int>(js_trace_code->instruction_start()),
-      js_trace_code->instruction_size());
+  // Stack sampling will start from the first JS function, i.e. "JSFuncDoTrace"
+  CheckRetAddrIsInJSFunction("JSFuncDoTrace",
+                             reinterpret_cast<unsigned int>(sample.stack[0]));
+  CheckRetAddrIsInJSFunction("JSTrace",
+                             reinterpret_cast<unsigned int>(sample.stack[1]));
+}
+
+
+TEST(PureJSStackTrace) {
+  TickSample sample;
+  StackTracer tracer(reinterpret_cast<unsigned int>(&sample));
+  InitTraceEnv(&tracer, &sample);
+
+  InitializeVM();
+  v8::HandleScope scope;
+  CreateTraceCallerFunction("JSFuncDoTrace", "js_trace");
+  CompileRun(
+      "function JSTrace() {"
+      "  JSFuncDoTrace();"
+      "};\n"
+      "function OuterJSTrace() {"
+      "  JSTrace();"
+      "};\n"
+      "OuterJSTrace();");
+  CHECK_GT(sample.frames_count, 1);
+  // Stack sampling will start from the caller of JSFuncDoTrace, i.e. "JSTrace"
+  CheckRetAddrIsInJSFunction("JSTrace",
+                             reinterpret_cast<unsigned int>(sample.stack[0]));
+  CheckRetAddrIsInJSFunction("OuterJSTrace",
+                             reinterpret_cast<unsigned int>(sample.stack[1]));
 }
 
+
+static void CFuncDoTrace() {
+  unsigned int fp;
+#ifdef __GNUC__
+  fp = reinterpret_cast<unsigned int>(__builtin_frame_address(0));
+#elif defined _MSC_VER
+  __asm mov [fp], ebp  // NOLINT
+#endif
+  DoTrace(fp);
+}
+
+
+static int CFunc(int depth) {
+  if (depth <= 0) {
+    CFuncDoTrace();
+    return 0;
+  } else {
+    return CFunc(depth - 1) + 1;
+  }
+}
+
+
+TEST(PureCStackTrace) {
+  TickSample sample;
+  StackTracer tracer(reinterpret_cast<unsigned int>(&sample));
+  InitTraceEnv(&tracer, &sample);
+  // Check that sampler doesn't crash
+  CHECK_EQ(10, CFunc(10));
+}
+
+
 #endif  // ENABLE_LOGGING_AND_PROFILING