From b295a51e7e26a6da8f533cf5676c3521a914e96d Mon Sep 17 00:00:00 2001 From: "yurys@chromium.org" Date: Mon, 24 Aug 2009 15:21:49 +0000 Subject: [PATCH] Allow 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.Related Chromium issue: http://code.google.com/p/chromium/issues/detail?id=17978 Review URL: http://codereview.chromium.org/159703 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@2745 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/debug.cc | 81 +++++++++++++++-- test/mjsunit/debug-step-stub-callfunction.js | 4 +- test/mjsunit/debug-stepin-call-function-stub.js | 115 ++++++++++++++++++++++++ test/mjsunit/mjsunit.status | 1 + 4 files changed, 194 insertions(+), 7 deletions(-) create mode 100644 test/mjsunit/debug-stepin-call-function-stub.js diff --git a/src/debug.cc b/src/debug.cc index f2a2814..35aa339 100644 --- a/src/debug.cc +++ b/src/debug.cc @@ -317,10 +317,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::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 +334,27 @@ void BreakLocationIterator::PrepareStepIn() { rinfo()->set_target_address(stub->entry()); } } else { +#ifdef DEBUG + Handle maybe_call_function_stub = code; + if (IsDebugBreak()) { + Address original_target = original_rinfo()->target_address(); + maybe_call_function_stub = + Handle(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 } } @@ -1092,6 +1108,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 call_function_stub; if (RelocInfo::IsCodeTarget(it.rinfo()->rmode())) { Address target = it.rinfo()->target_address(); Code* code = Code::GetCodeFromTargetAddress(target); @@ -1102,6 +1119,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(maybe_call_function_stub); + } } // If this is the last break code target step out is the only possibility. @@ -1114,7 +1147,8 @@ void Debug::PrepareStep(StepAction step_action, int step_count) { JSFunction* function = JSFunction::cast(frames_it.frame()->function()); FloodWithOneShot(Handle(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 +1160,43 @@ 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 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(); + 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 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(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 diff --git a/test/mjsunit/debug-step-stub-callfunction.js b/test/mjsunit/debug-step-stub-callfunction.js index fbb8078..ec16320 100644 --- a/test/mjsunit/debug-step-stub-callfunction.js +++ b/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,7 @@ function g() { break_break_point_hit_count = 0; g(); -assertEquals(4, break_break_point_hit_count); +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 index 0000000..7d5d71c --- /dev/null +++ b/test/mjsunit/debug-stepin-call-function-stub.js @@ -0,0 +1,115 @@ +// 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 twice: first time function will be compiled lazily, second time +// cached version will be used. +for (var i = 0; i < 2; 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 < 2; 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); \ No newline at end of file diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status index 6ac4938..839329d 100644 --- a/test/mjsunit/mjsunit.status +++ b/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 -- 2.7.4