Support stepping in functions called using CallFunction stub. When Debug::PrepareStep...
authoryurys@chromium.org <yurys@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Mon, 7 Sep 2009 07:20:05 +0000 (07:20 +0000)
committeryurys@chromium.org <yurys@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Mon, 7 Sep 2009 07:20:05 +0000 (07:20 +0000)
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
src/debug.h
test/mjsunit/debug-step-stub-callfunction.js
test/mjsunit/debug-stepin-call-function-stub.js [new file with mode: 0644]
test/mjsunit/mjsunit.status

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