Fix determining of JS lower stack bottom used in profiler's JS stack tracer to work...

My assumption that log initialization happens somewhere near the stack's bottom is true for V8's sample shell but isn't true for Chromium, causing many otherwise valid stack addresses to be thrown out. The solution proposed is to save stack pointer value for the outermost JS function in ThreadLocalTop similar to c_entry_fp.

Implemented only for IA-32. Currently I'm not dealing with profiling on ARM and x86-64 anyway.

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

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

diff --git a/src/ia32/codegen-ia32.cc b/src/ia32/codegen-ia32.cc
index c72c126f01250eba475f668f2062c765afec5103..7c2dde120b11658ce08075ad64dccd5d03d7e8bc 100644 (file)
--- a/src/ia32/codegen-ia32.cc
+++ b/src/ia32/codegen-ia32.cc
@@ -7159,6 +7159,9 @@ void CEntryStub::GenerateBody(MacroAssembler* masm, bool is_debug_break) {
 
 void JSEntryStub::GenerateBody(MacroAssembler* masm, bool is_construct) {
   Label invoke, exit;
+#ifdef ENABLE_LOGGING_AND_PROFILING
+  Label not_outermost_js, not_outermost_js_2;
+#endif
 
   // Setup frame.
   __ push(ebp);
@@ -7177,6 +7180,15 @@ void JSEntryStub::GenerateBody(MacroAssembler* masm, bool is_construct) {
   ExternalReference c_entry_fp(Top::k_c_entry_fp_address);
   __ push(Operand::StaticVariable(c_entry_fp));
 
+#ifdef ENABLE_LOGGING_AND_PROFILING
+  // If this is the outermost JS call, set js_entry_sp value.
+  ExternalReference js_entry_sp(Top::k_js_entry_sp_address);
+  __ cmp(Operand::StaticVariable(js_entry_sp), Immediate(0));
+  __ j(NegateCondition(equal), &not_outermost_js);
+  __ mov(Operand::StaticVariable(js_entry_sp), ebp);
+  __ bind(&not_outermost_js);
+#endif
+
   // Call a faked try-block that does the invoke.
   __ call(&invoke);
 
@@ -7220,6 +7232,15 @@ void JSEntryStub::GenerateBody(MacroAssembler* masm, bool is_construct) {
   // Pop next_sp.
   __ add(Operand(esp), Immediate(StackHandlerConstants::kSize - kPointerSize));
 
+#ifdef ENABLE_LOGGING_AND_PROFILING
+  // If current EBP value is the same as js_entry_sp value, it means that
+  // the current function is the outermost.
+  __ cmp(ebp, Operand::StaticVariable(js_entry_sp));
+  __ j(NegateCondition(equal), &not_outermost_js_2);
+  __ mov(Operand::StaticVariable(js_entry_sp), Immediate(0));
+  __ bind(&not_outermost_js_2);
+#endif
+
   // Restore the top frame descriptor from the stack.
   __ bind(&exit);
   __ pop(Operand::StaticVariable(ExternalReference(Top::k_c_entry_fp_address)));

diff --git a/src/log.cc b/src/log.cc
index c66a422fde35dfdfbd215ab604723c979d6cccc1..c1edf4d1855a27c37272a12052e9bf0fdc70bd3a 100644 (file)
--- a/src/log.cc
+++ b/src/log.cc
@@ -146,11 +146,18 @@ void StackTracer::Trace(TickSample* sample) {
     return;
   }
 
+  const Address js_entry_sp = Top::js_entry_sp(Top::GetCurrentThread());
+  if (js_entry_sp == 0) {
+    // Not executing JS now.
+    sample->frames_count = 0;
+    return;
+  }
+
   SafeStackTraceFrameIterator it(
       reinterpret_cast<Address>(sample->fp),
       reinterpret_cast<Address>(sample->sp),
       reinterpret_cast<Address>(sample->sp),
-      reinterpret_cast<Address>(low_stack_bound_));
+      js_entry_sp);
   int i = 0;
   while (!it.done() && i < TickSample::kMaxFramesCount) {
     sample->stack[i++] = it.frame()->pc();
@@ -166,14 +173,13 @@ void StackTracer::Trace(TickSample* sample) {
 //
 class Ticker: public Sampler {
  public:
-  explicit Ticker(int interval, uintptr_t low_stack_bound):
-      Sampler(interval, FLAG_prof), window_(NULL), profiler_(NULL),
-      stack_tracer_(low_stack_bound) {}
+  explicit Ticker(int interval):
+      Sampler(interval, FLAG_prof), window_(NULL), profiler_(NULL) {}
 
   ~Ticker() { if (IsActive()) Stop(); }
 
   void Tick(TickSample* sample) {
-    if (IsProfiling()) stack_tracer_.Trace(sample);
+    if (IsProfiling()) StackTracer::Trace(sample);
     if (profiler_) profiler_->Insert(sample);
     if (window_) window_->AddState(sample->state);
   }
@@ -201,7 +207,6 @@ class Ticker: public Sampler {
  private:
   SlidingStateWindow* window_;
   Profiler* profiler_;
-  StackTracer stack_tracer_;
 };
 
 
@@ -1002,11 +1007,7 @@ bool Logger::Setup() {
 
   current_state_ = &bottom_state_;
 
-  // as log is initialized early with V8, we can assume that JS execution
-  // frames can never reach this point on stack
-  int stack_var;
-  ticker_ = new Ticker(
-      kSamplingIntervalMs, reinterpret_cast<uintptr_t>(&stack_var));
+  ticker_ = new Ticker(kSamplingIntervalMs);
 
   if (FLAG_sliding_state_window && sliding_state_window_ == NULL) {
     sliding_state_window_ = new SlidingStateWindow();

diff --git a/src/log.h b/src/log.h
index 10d88330ee18cd10862747c5a0ddf48eedac423b..2f8f81c95bad62dfa9659e8e91f0a4232f7a26c4 100644 (file)
--- a/src/log.h
+++ b/src/log.h
@@ -277,14 +277,9 @@ class Logger {
 
 
 // Class that extracts stack trace, used for profiling.
-class StackTracer BASE_EMBEDDED {
+class StackTracer : public AllStatic {
  public:
-  explicit StackTracer(uintptr_t low_stack_bound)
-      : low_stack_bound_(low_stack_bound) { }
-  void Trace(TickSample* sample);
- private:
-
-  uintptr_t low_stack_bound_;
+  static void Trace(TickSample* sample);
 };
 
 

diff --git a/src/top.cc b/src/top.cc
index b2583dbcbac1941274c6e67e50fd82d121b3bea0..42a2b7edfb8c02defa09b254664ab4b60c092dc7 100644 (file)
--- a/src/top.cc
+++ b/src/top.cc
@@ -45,6 +45,7 @@ NoAllocationStringAllocator* preallocated_message_space = NULL;
 Address top_addresses[] = {
 #define C(name) reinterpret_cast<Address>(Top::name()),
     TOP_ADDRESS_LIST(C)
+    TOP_ADDRESS_LIST_PROF(C)
 #undef C
     NULL
 };
@@ -91,6 +92,9 @@ void Top::Iterate(ObjectVisitor* v) {
 void Top::InitializeThreadLocal() {
   thread_local_.c_entry_fp_ = 0;
   thread_local_.handler_ = 0;
+#ifdef ENABLE_LOGGING_AND_PROFILING
+  thread_local_.js_entry_sp_ = 0;
+#endif
   thread_local_.stack_is_cooked_ = false;
   thread_local_.try_catch_handler_ = NULL;
   thread_local_.context_ = NULL;

diff --git a/src/top.h b/src/top.h
index 8e928ed6313645acb389ed7ebdafc6dc7e5922f7..53d67e555fe13b5aeb4befe1dbbb6a95a7e8ba13 100644 (file)
--- a/src/top.h
+++ b/src/top.h
@@ -65,6 +65,9 @@ class ThreadLocalTop BASE_EMBEDDED {
   // Stack.
   Address c_entry_fp_;  // the frame pointer of the top c entry frame
   Address handler_;   // try-blocks are chained through the stack
+#ifdef ENABLE_LOGGING_AND_PROFILING
+  Address js_entry_sp_;  // the stack pointer of the bottom js entry frame
+#endif
   bool stack_is_cooked_;
   inline bool stack_is_cooked() { return stack_is_cooked_; }
   inline void set_stack_is_cooked(bool value) { stack_is_cooked_ = value; }
@@ -83,11 +86,20 @@ class ThreadLocalTop BASE_EMBEDDED {
   C(pending_exception_address)         \
   C(external_caught_exception_address)
 
+#ifdef ENABLE_LOGGING_AND_PROFILING
+#define TOP_ADDRESS_LIST_PROF(C)       \
+  C(js_entry_sp_address)
+#else
+#define TOP_ADDRESS_LIST_PROF(C)
+#endif
+
+
 class Top {
  public:
   enum AddressId {
 #define C(name) k_##name,
     TOP_ADDRESS_LIST(C)
+    TOP_ADDRESS_LIST_PROF(C)
 #undef C
     k_top_address_count
   };
@@ -179,6 +191,16 @@ class Top {
   }
   static inline Address* handler_address() { return &thread_local_.handler_; }
 
+#ifdef ENABLE_LOGGING_AND_PROFILING
+  // Bottom JS entry (see StackTracer::Trace in log.cc).
+  static Address js_entry_sp(ThreadLocalTop* thread) {
+    return thread->js_entry_sp_;
+  }
+  static inline Address* js_entry_sp_address() {
+    return &thread_local_.js_entry_sp_;
+  }
+#endif
+
   // Generated code scratch locations.
   static void* formal_count_address() { return &thread_local_.formal_count_; }
 

diff --git a/test/cctest/test-log-ia32.cc b/test/cctest/test-log-ia32.cc
index 7312901ccaa20f3b7df206d4b64db91ca3244c11..a40a800ee3a55c070eddafcf5ee7c05b8b9d6cfd 100644 (file)
--- a/test/cctest/test-log-ia32.cc
+++ b/test/cctest/test-log-ia32.cc
@@ -37,13 +37,11 @@ static v8::Persistent<v8::Context> env;
 
 
 static struct {
-  StackTracer* tracer;
   TickSample* sample;
-} trace_env = { NULL, NULL };
+} trace_env = { NULL };
 
 
-static void InitTraceEnv(StackTracer* tracer, TickSample* sample) {
-  trace_env.tracer = tracer;
+static void InitTraceEnv(TickSample* sample) {
   trace_env.sample = sample;
 }
 
@@ -53,7 +51,7 @@ static void DoTrace(Address fp) {
   // 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);
+  StackTracer::Trace(trace_env.sample);
 }
 
 
@@ -99,6 +97,8 @@ class TraceExtension : public v8::Extension {
       v8::Handle<String> name);
   static v8::Handle<v8::Value> Trace(const v8::Arguments& args);
   static v8::Handle<v8::Value> JSTrace(const v8::Arguments& args);
+  static v8::Handle<v8::Value> JSEntrySP(const v8::Arguments& args);
+  static v8::Handle<v8::Value> JSEntrySPLevel2(const v8::Arguments& args);
  private:
   static Address GetFP(const v8::Arguments& args);
   static const char* kSource;
@@ -107,8 +107,9 @@ class TraceExtension : public v8::Extension {
 
 const char* TraceExtension::kSource =
     "native function trace();"
-    "native function js_trace();";
-
+    "native function js_trace();"
+    "native function js_entry_sp();"
+    "native function js_entry_sp_level2();";
 
 v8::Handle<v8::FunctionTemplate> TraceExtension::GetNativeFunction(
     v8::Handle<String> name) {
@@ -116,6 +117,10 @@ v8::Handle<v8::FunctionTemplate> TraceExtension::GetNativeFunction(
     return v8::FunctionTemplate::New(TraceExtension::Trace);
   } else if (name->Equals(String::New("js_trace"))) {
     return v8::FunctionTemplate::New(TraceExtension::JSTrace);
+  } else if (name->Equals(String::New("js_entry_sp"))) {
+    return v8::FunctionTemplate::New(TraceExtension::JSEntrySP);
+  } else if (name->Equals(String::New("js_entry_sp_level2"))) {
+    return v8::FunctionTemplate::New(TraceExtension::JSEntrySPLevel2);
   } else {
     CHECK(false);
     return v8::Handle<v8::FunctionTemplate>();
@@ -143,6 +148,34 @@ v8::Handle<v8::Value> TraceExtension::JSTrace(const v8::Arguments& args) {
 }
 
 
+static Address GetJsEntrySp() {
+  CHECK_NE(NULL, Top::GetCurrentThread());
+  return Top::js_entry_sp(Top::GetCurrentThread());
+}
+
+
+v8::Handle<v8::Value> TraceExtension::JSEntrySP(const v8::Arguments& args) {
+  CHECK_NE(0, GetJsEntrySp());
+  return v8::Undefined();
+}
+
+
+static void CompileRun(const char* source) {
+  Script::Compile(String::New(source))->Run();
+}
+
+
+v8::Handle<v8::Value> TraceExtension::JSEntrySPLevel2(
+    const v8::Arguments& args) {
+  v8::HandleScope scope;
+  const Address js_entry_sp = GetJsEntrySp();
+  CHECK_NE(0, js_entry_sp);
+  CompileRun("js_entry_sp();");
+  CHECK_EQ(js_entry_sp, GetJsEntrySp());
+  return v8::Undefined();
+}
+
+
 static TraceExtension kTraceExtension;
 v8::DeclareExtension kTraceExtensionDeclaration(&kTraceExtension);
 
@@ -164,11 +197,6 @@ static Handle<JSFunction> CompileFunction(const char* source) {
 }
 
 
-static void CompileRun(const char* source) {
-  Script::Compile(String::New(source))->Run();
-}
-
-
 static Local<Value> GetGlobalProperty(const char* name) {
   return env->Global()->Get(String::New(name));
 }
@@ -255,8 +283,7 @@ static void CreateTraceCallerFunction(const char* func_name,
 
 TEST(CFromJSStackTrace) {
   TickSample sample;
-  StackTracer tracer(reinterpret_cast<uintptr_t>(&sample));
-  InitTraceEnv(&tracer, &sample);
+  InitTraceEnv(&sample);
 
   InitializeVM();
   v8::HandleScope scope;
@@ -277,8 +304,7 @@ TEST(CFromJSStackTrace) {
 
 TEST(PureJSStackTrace) {
   TickSample sample;
-  StackTracer tracer(reinterpret_cast<uintptr_t>(&sample));
-  InitTraceEnv(&tracer, &sample);
+  InitTraceEnv(&sample);
 
   InitializeVM();
   v8::HandleScope scope;
@@ -323,11 +349,22 @@ static int CFunc(int depth) {
 
 TEST(PureCStackTrace) {
   TickSample sample;
-  StackTracer tracer(reinterpret_cast<uintptr_t>(&sample));
-  InitTraceEnv(&tracer, &sample);
+  InitTraceEnv(&sample);
   // Check that sampler doesn't crash
   CHECK_EQ(10, CFunc(10));
 }
 
 
+TEST(JsEntrySp) {
+  InitializeVM();
+  v8::HandleScope scope;
+  CHECK_EQ(0, GetJsEntrySp());
+  CompileRun("a = 1; b = a + 1;");
+  CHECK_EQ(0, GetJsEntrySp());
+  CompileRun("js_entry_sp();");
+  CHECK_EQ(0, GetJsEntrySp());
+  CompileRun("js_entry_sp_level2();");
+  CHECK_EQ(0, GetJsEntrySp());
+}
+
 #endif  // ENABLE_LOGGING_AND_PROFILING