Do not iterate stack handlers in SafeStackFrameIterator

CPU profiler doesn't use stack handlers so there is no need to iterate through them while traversing stack. This change SafeStackFrameIterator always iterate only frames and removes checks corresponding to the handlers iteration.

The problem described in the bug occurred because of a false assumption in SafeStackFrameIterator that if Isolate::c_entry_fp is not NULL then the top frame on the stack is always a C++ frame. It is false because we may have entered JS code again, in which case JS_ENTRY code stub generated by JSEntryStub::GenerateBody() will save current c_entry_fp value but not reset it to NULL and after that it will create ENTRY stack frame and JS_ENTRY handler on the stack and put the latter into Isolate::handler(top). This means that if we start iterating from c_entry_fp frame and try to compare the frame's sp with Isolate::handler()->address() it will turn out that frame->sp() > handler->address() and the condition in SafeStackFrameIterator::CanIterateHandles is not held.

BUG=252097
R=loislo@chromium.org, svenpanne@chromium.org

Review URL: https://codereview.chromium.org/17589022

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

src/frames.cc

@@ -112,8 +112,7 @@ StackFrameIterator::StackFrameIterator(Isolate* isolate,
       frame_(NULL), handler_(NULL),
       thread_(use_top ? isolate_->thread_local_top() : NULL),
       fp_(use_top ? NULL : fp), sp_(sp),
-      advance_(use_top ? &StackFrameIterator::AdvanceWithHandler :
-               &StackFrameIterator::AdvanceWithoutHandler),
+      advance_(&StackFrameIterator::AdvanceWithoutHandler),
       can_access_heap_objects_(false) {
   if (use_top || fp != NULL) {
     Reset();
@@ -299,7 +298,6 @@ void SafeStackFrameIterator::AdvanceOneFrame() {
   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) ||
-      !CanIterateHandles(last_frame, iterator_.handler()) ||
       !IsValidCaller(last_frame);
   if (iteration_done_) return;
 
@@ -311,15 +309,6 @@ void SafeStackFrameIterator::AdvanceOneFrame() {
 }
 
 
-bool SafeStackFrameIterator::CanIterateHandles(StackFrame* frame,
-                                               StackHandler* handler) {
-  // If StackIterator iterates over StackHandles, verify that
-  // StackHandlerIterator can be instantiated (see StackHandlerIterator
-  // constructor.)
-  return !is_valid_top_ || (frame->sp() <= handler->address());
-}
-
-
 bool SafeStackFrameIterator::IsValidFrame(StackFrame* frame) const {
   return IsValidStackAddress(frame->sp()) && IsValidStackAddress(frame->fp());
 }

src/frames.h

@@ -919,7 +919,6 @@ class SafeStackFrameIterator BASE_EMBEDDED {
   bool IsValidStackAddress(Address addr) const {
     return stack_validator_.IsValid(addr);
   }
-  bool CanIterateHandles(StackFrame* frame, StackHandler* handler);
   bool IsValidFrame(StackFrame* frame) const;
   bool IsValidCaller(StackFrame* frame);
   static bool IsValidTop(Isolate* isolate,

test/cctest/test-cpu-profiler.cc

@@ -914,3 +914,54 @@ TEST(NativeMethodMonomorphicIC) {
 
   cpu_profiler->DeleteAllCpuProfiles();
 }
+
+
+static const char* bound_function_test_source = "function foo(iterations) {\n"
+"  var r = 0;\n"
+"  for (var i = 0; i < iterations; i++) { r += i; }\n"
+"  return r;\n"
+"}\n"
+"function start(duration) {\n"
+"  var callback = foo.bind(this);\n"
+"  var start = Date.now();\n"
+"  while (Date.now() - start < duration) {\n"
+"    callback(10 * 1000);\n"
+"  }\n"
+"}";
+
+
+TEST(BoundFunctionCall) {
+  LocalContext env;
+  v8::HandleScope scope(env->GetIsolate());
+
+  v8::Script::Compile(v8::String::New(bound_function_test_source))->Run();
+  v8::Local<v8::Function> function = v8::Local<v8::Function>::Cast(
+      env->Global()->Get(v8::String::New("start")));
+
+  v8::CpuProfiler* cpu_profiler = env->GetIsolate()->GetCpuProfiler();
+  v8::Local<v8::String> profile_name = v8::String::New("my_profile");
+
+  cpu_profiler->StartCpuProfiling(profile_name);
+  int32_t duration_ms = 100;
+  v8::Handle<v8::Value> args[] = { v8::Integer::New(duration_ms) };
+  function->Call(env->Global(), ARRAY_SIZE(args), args);
+  const v8::CpuProfile* profile = cpu_profiler->StopCpuProfiling(profile_name);
+
+  CHECK_NE(NULL, profile);
+  // Dump collected profile to have a better diagnostic in case of failure.
+  reinterpret_cast<i::CpuProfile*>(
+      const_cast<v8::CpuProfile*>(profile))->Print();
+
+  const v8::CpuProfileNode* root = profile->GetTopDownRoot();
+  ScopedVector<v8::Handle<v8::String> > names(3);
+  names[0] = v8::String::New(ProfileGenerator::kGarbageCollectorEntryName);
+  names[1] = v8::String::New(ProfileGenerator::kProgramEntryName);
+  names[2] = v8::String::New("start");
+  // Don't allow |foo| node to be at the top level.
+  CheckChildrenNames(root, names);
+
+  const v8::CpuProfileNode* startNode = GetChild(root, "start");
+  GetChild(startNode, "foo");
+
+  cpu_profiler->DeleteAllCpuProfiles();
+}