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

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

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)));

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();

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);
 };
 
 

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;

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_; }
 

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