Support stepping in functions called using CallFunction stub. When...

Support stepping in functions called using CallFunction stub. When Debug::PrepareStep is called to prepare 'step in' and current code target is CallFunction stub, the debugger will find function being called on the expression stack and flood it with one shot breakpoints.

BreakLocationIterator changed to treat 'debugger;' statements as a possible break location. Since 'debugger;' statement should always invoke debugger it is hanled in a special way.

Related Chromium issue:
http://code.google.com/p/chromium/issues/detail?id=17978
Review URL: http://codereview.chromium.org/195015

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

src/debug.cc

@@ -75,6 +75,9 @@ BreakLocationIterator::BreakLocationIterator(Handle<DebugInfo> debug_info,
                                              BreakLocatorType type) {
   debug_info_ = debug_info;
   type_ = type;
+  // Get the stub early to avoid possible GC during iterations. We may need
+  // this stub to detect debugger calls generated from debugger statements.
+  debug_break_stub_ = RuntimeStub(Runtime::kDebugBreak, 0).GetCode();
   reloc_iterator_ = NULL;
   reloc_iterator_original_ = NULL;
   Reset();  // Initialize the rest of the member variables.
@@ -126,6 +129,10 @@ void BreakLocationIterator::Next() {
         return;
       }
       if (code->kind() == Code::STUB) {
+        if (IsDebuggerStatement()) {
+          break_point_++;
+          return;
+        }
         if (type_ == ALL_BREAK_LOCATIONS) {
           if (Debug::IsBreakStub(code)) {
             break_point_++;
@@ -238,7 +245,7 @@ void BreakLocationIterator::SetBreakPoint(Handle<Object> break_point_object) {
   if (!HasBreakPoint()) {
     SetDebugBreak();
   }
-  ASSERT(IsDebugBreak());
+  ASSERT(IsDebugBreak() || IsDebuggerStatement());
   // Set the break point information.
   DebugInfo::SetBreakPoint(debug_info_, code_position(),
                            position(), statement_position(),
@@ -258,6 +265,11 @@ void BreakLocationIterator::ClearBreakPoint(Handle<Object> break_point_object) {
 
 
 void BreakLocationIterator::SetOneShot() {
+  // Debugger statement always calls debugger. No need to modify it.
+  if (IsDebuggerStatement()) {
+    return;
+  }
+
   // If there is a real break point here no more to do.
   if (HasBreakPoint()) {
     ASSERT(IsDebugBreak());
@@ -270,6 +282,11 @@ void BreakLocationIterator::SetOneShot() {
 
 
 void BreakLocationIterator::ClearOneShot() {
+  // Debugger statement always calls debugger. No need to modify it.
+  if (IsDebuggerStatement()) {
+    return;
+  }
+
   // If there is a real break point here no more to do.
   if (HasBreakPoint()) {
     ASSERT(IsDebugBreak());
@@ -283,6 +300,11 @@ void BreakLocationIterator::ClearOneShot() {
 
 
 void BreakLocationIterator::SetDebugBreak() {
+  // Debugger statement always calls debugger. No need to modify it.
+  if (IsDebuggerStatement()) {
+    return;
+  }
+
   // If there is already a break point here just return. This might happen if
   // the same code is flooded with break points twice. Flooding the same
   // function twice might happen when stepping in a function with an exception
@@ -303,6 +325,11 @@ void BreakLocationIterator::SetDebugBreak() {
 
 
 void BreakLocationIterator::ClearDebugBreak() {
+  // Debugger statement always calls debugger. No need to modify it.
+  if (IsDebuggerStatement()) {
+    return;
+  }
+
   if (RelocInfo::IsJSReturn(rmode())) {
     // Restore the frame exit code.
     ClearDebugBreakAtReturn();
@@ -317,10 +344,10 @@ void BreakLocationIterator::ClearDebugBreak() {
 void BreakLocationIterator::PrepareStepIn() {
   HandleScope scope;
 
-  // Step in can only be prepared if currently positioned on an IC call or
-  // construct call.
+  // Step in can only be prepared if currently positioned on an IC call,
+  // construct call or CallFunction stub call.
   Address target = rinfo()->target_address();
-  Code* code = Code::GetCodeFromTargetAddress(target);
+  Handle<Code> code(Code::GetCodeFromTargetAddress(target));
   if (code->is_call_stub()) {
     // Step in through IC call is handled by the runtime system. Therefore make
     // sure that the any current IC is cleared and the runtime system is
@@ -334,11 +361,29 @@ void BreakLocationIterator::PrepareStepIn() {
       rinfo()->set_target_address(stub->entry());
     }
   } else {
+#ifdef DEBUG
+    // All the following stuff is needed only for assertion checks so the code
+    // is wrapped in ifdef.
+    Handle<Code> maybe_call_function_stub = code;
+    if (IsDebugBreak()) {
+      Address original_target = original_rinfo()->target_address();
+      maybe_call_function_stub =
+          Handle<Code>(Code::GetCodeFromTargetAddress(original_target));
+    }
+    bool is_call_function_stub =
+        (maybe_call_function_stub->kind() == Code::STUB &&
+         maybe_call_function_stub->major_key() == CodeStub::CallFunction);
+
     // Step in through construct call requires no changes to the running code.
     // Step in through getters/setters should already be prepared as well
     // because caller of this function (Debug::PrepareStep) is expected to
     // flood the top frame's function with one shot breakpoints.
-    ASSERT(RelocInfo::IsConstructCall(rmode()) || code->is_inline_cache_stub());
+    // Step in through CallFunction stub should also be prepared by caller of
+    // this function (Debug::PrepareStep) which should flood target function
+    // with breakpoints.
+    ASSERT(RelocInfo::IsConstructCall(rmode()) || code->is_inline_cache_stub()
+           || is_call_function_stub);
+#endif
   }
 }
 
@@ -409,6 +454,21 @@ void BreakLocationIterator::ClearDebugBreakAtIC() {
 }
 
 
+bool BreakLocationIterator::IsDebuggerStatement() {
+  if (RelocInfo::IsCodeTarget(rmode())) {
+    Address target = original_rinfo()->target_address();
+    Code* code = Code::GetCodeFromTargetAddress(target);
+    if (code->kind() == Code::STUB) {
+      CodeStub::Major major_key = code->major_key();
+      if (major_key == CodeStub::Runtime) {
+        return (*debug_break_stub_ == code);
+      }
+    }
+  }
+  return false;
+}
+
+
 Object* BreakLocationIterator::BreakPointObjects() {
   return debug_info_->GetBreakPointObjects(code_position());
 }
@@ -1092,6 +1152,7 @@ void Debug::PrepareStep(StepAction step_action, int step_count) {
   bool is_call_target = false;
   bool is_load_or_store = false;
   bool is_inline_cache_stub = false;
+  Handle<Code> call_function_stub;
   if (RelocInfo::IsCodeTarget(it.rinfo()->rmode())) {
     Address target = it.rinfo()->target_address();
     Code* code = Code::GetCodeFromTargetAddress(target);
@@ -1102,6 +1163,22 @@ void Debug::PrepareStep(StepAction step_action, int step_count) {
       is_inline_cache_stub = true;
       is_load_or_store = !is_call_target;
     }
+
+    // Check if target code is CallFunction stub.
+    Code* maybe_call_function_stub = code;
+    // If there is a breakpoint at this line look at the original code to
+    // check if it is a CallFunction stub.
+    if (it.IsDebugBreak()) {
+      Address original_target = it.original_rinfo()->target_address();
+      maybe_call_function_stub =
+          Code::GetCodeFromTargetAddress(original_target);
+    }
+    if (maybe_call_function_stub->kind() == Code::STUB &&
+        maybe_call_function_stub->major_key() == CodeStub::CallFunction) {
+      // Save reference to the code as we may need it to find out arguments
+      // count for 'step in' later.
+      call_function_stub = Handle<Code>(maybe_call_function_stub);
+    }
   }
 
   // If this is the last break code target step out is the only possibility.
@@ -1114,7 +1191,8 @@ void Debug::PrepareStep(StepAction step_action, int step_count) {
       JSFunction* function = JSFunction::cast(frames_it.frame()->function());
       FloodWithOneShot(Handle<SharedFunctionInfo>(function->shared()));
     }
-  } else if (!(is_inline_cache_stub || RelocInfo::IsConstructCall(it.rmode()))
+  } else if (!(is_inline_cache_stub || RelocInfo::IsConstructCall(it.rmode()) ||
+               !call_function_stub.is_null())
              || step_action == StepNext || step_action == StepMin) {
     // Step next or step min.
 
@@ -1126,6 +1204,45 @@ void Debug::PrepareStep(StepAction step_action, int step_count) {
         debug_info->code()->SourceStatementPosition(frame->pc());
     thread_local_.last_fp_ = frame->fp();
   } else {
+    // If it's CallFunction stub ensure target function is compiled and flood
+    // it with one shot breakpoints.
+    if (!call_function_stub.is_null()) {
+      // Find out number of arguments from the stub minor key.
+      // Reverse lookup required as the minor key cannot be retrieved
+      // from the code object.
+      Handle<Object> obj(
+          Heap::code_stubs()->SlowReverseLookup(*call_function_stub));
+      ASSERT(*obj != Heap::undefined_value());
+      ASSERT(obj->IsSmi());
+      // Get the STUB key and extract major and minor key.
+      uint32_t key = Smi::cast(*obj)->value();
+      // Argc in the stub is the number of arguments passed - not the
+      // expected arguments of the called function.
+      int call_function_arg_count = CodeStub::MinorKeyFromKey(key);
+      ASSERT(call_function_stub->major_key() ==
+             CodeStub::MajorKeyFromKey(key));
+
+      // Find target function on the expression stack.
+      // Expression stack lools like this (top to bottom):
+      // argN
+      // ...
+      // arg0
+      // Receiver
+      // Function to call
+      int expressions_count = frame->ComputeExpressionsCount();
+      ASSERT(expressions_count - 2 - call_function_arg_count >= 0);
+      Object* fun = frame->GetExpression(
+          expressions_count - 2 - call_function_arg_count);
+      if (fun->IsJSFunction()) {
+        Handle<JSFunction> js_function(JSFunction::cast(fun));
+        // Don't step into builtins.
+        if (!js_function->IsBuiltin()) {
+          // It will also compile target function if it's not compiled yet.
+          FloodWithOneShot(Handle<SharedFunctionInfo>(js_function->shared()));
+        }
+      }
+    }
+
     // Fill the current function with one-shot break points even for step in on
     // a call target as the function called might be a native function for
     // which step in will not stop. It also prepares for stepping in

src/debug.h

@@ -119,6 +119,8 @@ class BreakLocationIterator {
     return reloc_iterator_original_->rinfo()->rmode();
   }
 
+  bool IsDebuggerStatement();
+
  protected:
   bool RinfoDone() const;
   void RinfoNext();
@@ -128,6 +130,7 @@ class BreakLocationIterator {
   int position_;
   int statement_position_;
   Handle<DebugInfo> debug_info_;
+  Handle<Code> debug_break_stub_;
   RelocIterator* reloc_iterator_;
   RelocIterator* reloc_iterator_original_;
 

test/mjsunit/debug-step-stub-callfunction.js

@@ -54,7 +54,7 @@ function f() {
 
 break_break_point_hit_count = 0;
 f();
-assertEquals(5, break_break_point_hit_count);
+assertEquals(6, break_break_point_hit_count);
 
 // Use an inner function to ensure that the function call is through CodeStub
 // CallFunction see Ia32CodeGenerator::VisitCall and
@@ -67,7 +67,21 @@ function g() {
 
 break_break_point_hit_count = 0;
 g();
-assertEquals(4, break_break_point_hit_count);
+assertEquals(5, break_break_point_hit_count);
+
+
+// Use an inner function to ensure that the function call is through CodeStub
+// CallFunction.
+function testCallInExpreesion() {
+  function h() {}
+  debugger;
+  var x = 's' + h(10, 20);
+};
+
+break_break_point_hit_count = 0;
+testCallInExpreesion();
+assertEquals(5, break_break_point_hit_count);
+
 
 // Get rid of the debug event listener.
 Debug.setListener(null);

test/mjsunit/debug-stepin-call-function-stub.js

+// Copyright 2009 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --expose-debug-as debug
+// Get the Debug object exposed from the debug context global object.
+Debug = debug.Debug
+
+var exception = null;
+var state = 0;
+var expected_function_name = null;
+var expected_source_line_text = null;
+var expected_caller_source_line = null;
+var step_in_count = 2;
+
+// Simple debug event handler which first time will cause 'step in' action
+// to get into g.call and than check that execution is pauesed inside
+// function 'g'.
+function listener(event, exec_state, event_data, data) {
+  try {
+    if (event == Debug.DebugEvent.Break) {
+      if (state == 0) {
+        // Step into f().
+        exec_state.prepareStep(Debug.StepAction.StepIn, step_in_count);
+        state = 2;
+      } else if (state == 2) {
+        assertEquals(expected_source_line_text,
+                     event_data.sourceLineText());
+        assertEquals(expected_function_name, event_data.func().name());
+        state = 3;
+      }
+    }
+  } catch(e) {
+    exception = e;
+  }
+};
+
+// Add the debug event listener.
+Debug.setListener(listener);
+
+
+function g() { 
+   return "s";  // expected line
+}
+
+function testFunction() {
+  var f = g;
+  var s = 1 +f(10);
+}
+
+function g2() { 
+   return "s2";  // expected line
+}
+
+function testFunction2() {
+  var f = g2;
+  var s = 1 +f(10, 20);
+}
+
+// Run three times. First time the function will be compiled lazily,
+// second time cached version will be used.
+for (var i = 0; i < 3; i++) {
+  state = 0;
+  expected_function_name = 'g';
+  expected_source_line_text = '   return "s";  // expected line';
+  step_in_count = 2;
+  // Set a break point and call to invoke the debug event listener.
+  Debug.setBreakPoint(testFunction, 1, 0);
+  testFunction();
+  assertNull(exception);
+  assertEquals(3, state);
+}
+
+// Test stepping into function call when a breakpoint is set at the place
+// of call. Use different pair of functions so that g2 is compiled lazily.
+// Run twice: first time function will be compiled lazily, second time
+// cached version will be used.
+for (var i = 0; i < 3; i++) {
+  state = 0;
+  expected_function_name = 'g2';
+  expected_source_line_text = '   return "s2";  // expected line';
+  step_in_count = 1;
+  // Set a break point and call to invoke the debug event listener.
+  Debug.setBreakPoint(testFunction2, 2, 0);
+  testFunction2();
+  assertNull(exception);
+  assertEquals(3, state);
+}
+
+
+// Get rid of the debug event listener.
+Debug.setListener(null);

test/mjsunit/mjsunit.status

@@ -60,6 +60,7 @@ debug-setbreakpoint: CRASH || FAIL || PASS
 debug-step-stub-callfunction: SKIP
 debug-stepin-accessor: CRASH || FAIL
 debug-stepin-builtin: CRASH || FAIL
+debug-stepin-call-function-stub: CRASH || FAIL
 debug-stepin-constructor: CRASH, FAIL
 debug-stepin-function-call: CRASH || FAIL
 debug-step: SKIP