From 16709ee695a5dbb58a1ebcf75f705bf1015e0f66 Mon Sep 17 00:00:00 2001 From: "peter.rybin@gmail.com" Date: Thu, 29 Jul 2010 16:40:14 +0000 Subject: [PATCH] Fix 'step in' after live edit stack manipulation Review URL: http://codereview.chromium.org/3029033 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@5150 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/debug-arm.cc | 5 ++-- src/debug-debugger.js | 4 +++ src/debug.cc | 78 +++++++++++++++++++++++++++++++------------------- src/debug.h | 35 ++++++++++++++++++++-- src/ia32/debug-ia32.cc | 13 +++++++-- src/liveedit.cc | 15 +++++++--- src/mips/debug-mips.cc | 5 ++-- src/x64/debug-x64.cc | 5 ++-- 8 files changed, 115 insertions(+), 45 deletions(-) diff --git a/src/arm/debug-arm.cc b/src/arm/debug-arm.cc index 65f5eea..e87d265 100644 --- a/src/arm/debug-arm.cc +++ b/src/arm/debug-arm.cc @@ -296,9 +296,10 @@ void Debug::GenerateFrameDropperLiveEdit(MacroAssembler* masm) { #undef __ -void Debug::SetUpFrameDropperFrame(StackFrame* bottom_js_frame, - Handle code) { +Object** Debug::SetUpFrameDropperFrame(StackFrame* bottom_js_frame, + Handle code) { UNREACHABLE(); + return NULL; } const int Debug::kFrameDropperFrameSize = -1; diff --git a/src/debug-debugger.js b/src/debug-debugger.js index 8315da8..0b02e21 100644 --- a/src/debug-debugger.js +++ b/src/debug-debugger.js @@ -2115,6 +2115,10 @@ DebugCommandProcessor.prototype.changeLiveRequest_ = function(request, response) var result_description = Debug.LiveEdit.SetScriptSource(the_script, new_source, preview_only, change_log); response.body = {change_log: change_log, result: result_description}; + + if (!preview_only && !this.running_ && result_description.stack_modified) { + response.body.stepin_recommended = true; + } }; diff --git a/src/debug.cc b/src/debug.cc index 0957cbc..5d386cc 100644 --- a/src/debug.cc +++ b/src/debug.cc @@ -1224,36 +1224,42 @@ void Debug::PrepareStep(StepAction step_action, int step_count) { it.FindBreakLocationFromAddress(frame->pc()); // Compute whether or not the target is a call target. - bool is_call_target = false; bool is_load_or_store = false; bool is_inline_cache_stub = false; + bool is_at_restarted_function = false; Handle call_function_stub; - if (RelocInfo::IsCodeTarget(it.rinfo()->rmode())) { - Address target = it.rinfo()->target_address(); - Code* code = Code::GetCodeFromTargetAddress(target); - if (code->is_call_stub() || code->is_keyed_call_stub()) { - is_call_target = true; - } - if (code->is_inline_cache_stub()) { - 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 (thread_local_.restarter_frame_function_pointer_ == NULL) { + if (RelocInfo::IsCodeTarget(it.rinfo()->rmode())) { + bool is_call_target = false; + Address target = it.rinfo()->target_address(); + Code* code = Code::GetCodeFromTargetAddress(target); + if (code->is_call_stub() || code->is_keyed_call_stub()) { + is_call_target = true; + } + if (code->is_inline_cache_stub()) { + 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); + } } + } else { + is_at_restarted_function = true; } // If this is the last break code target step out is the only possibility. @@ -1282,7 +1288,7 @@ void Debug::PrepareStep(StepAction step_action, int step_count) { ActivateStepOut(frames_it.frame()); } } else if (!(is_inline_cache_stub || RelocInfo::IsConstructCall(it.rmode()) || - !call_function_stub.is_null()) + !call_function_stub.is_null() || is_at_restarted_function) || step_action == StepNext || step_action == StepMin) { // Step next or step min. @@ -1294,9 +1300,18 @@ 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()) { + // If there's restarter frame on top of the stack, just get the pointer + // to function which is going to be restarted. + if (is_at_restarted_function) { + Handle restarted_function( + JSFunction::cast(*thread_local_.restarter_frame_function_pointer_)); + Handle restarted_shared( + restarted_function->shared()); + FloodWithOneShot(restarted_shared); + } else if (!call_function_stub.is_null()) { + // If it's CallFunction stub ensure target function is compiled and flood + // it with one shot breakpoints. + // Find out number of arguments from the stub minor key. // Reverse lookup required as the minor key cannot be retrieved // from the code object. @@ -1767,9 +1782,12 @@ bool Debug::IsBreakAtReturn(JavaScriptFrame* frame) { void Debug::FramesHaveBeenDropped(StackFrame::Id new_break_frame_id, - FrameDropMode mode) { + FrameDropMode mode, + Object** restarter_frame_function_pointer) { thread_local_.frame_drop_mode_ = mode; thread_local_.break_frame_id_ = new_break_frame_id; + thread_local_.restarter_frame_function_pointer_ = + restarter_frame_function_pointer; } diff --git a/src/debug.h b/src/debug.h index 948055d..b6aba5a 100644 --- a/src/debug.h +++ b/src/debug.h @@ -332,6 +332,7 @@ class Debug { k_after_break_target_address, k_debug_break_return_address, k_debug_break_slot_address, + k_restarter_frame_function_pointer, k_register_address }; @@ -339,6 +340,10 @@ class Debug { static Address* after_break_target_address() { return reinterpret_cast(&thread_local_.after_break_target_); } + static Address* restarter_frame_function_pointer_address() { + Object*** address = &thread_local_.restarter_frame_function_pointer_; + return reinterpret_cast(address); + } // Support for saving/restoring registers when handling debug break calls. static Object** register_address(int r) { @@ -415,10 +420,22 @@ class Debug { }; static void FramesHaveBeenDropped(StackFrame::Id new_break_frame_id, - FrameDropMode mode); + FrameDropMode mode, + Object** restarter_frame_function_pointer); + + // Initializes an artificial stack frame. The data it contains is used for: + // a. successful work of frame dropper code which eventually gets control, + // b. being compatible with regular stack structure for various stack + // iterators. + // Returns address of stack allocated pointer to restarted function, + // the value that is called 'restarter_frame_function_pointer'. The value + // at this address (possibly updated by GC) may be used later when preparing + // 'step in' operation. + // The implementation is architecture-specific. + // TODO(LiveEdit): consider reviewing it as architecture-independent. + static Object** SetUpFrameDropperFrame(StackFrame* bottom_js_frame, + Handle code); - static void SetUpFrameDropperFrame(StackFrame* bottom_js_frame, - Handle code); static const int kFrameDropperFrameSize; private: @@ -495,6 +512,11 @@ class Debug { // Pending interrupts scheduled while debugging. int pending_interrupts_; + + // When restarter frame is on stack, stores the address + // of the pointer to function being restarted. Otherwise (most of the time) + // stores NULL. This pointer is used with 'step in' implementation. + Object** restarter_frame_function_pointer_; }; // Storage location for registers when handling debug break calls @@ -938,6 +960,10 @@ class Debug_Address { return Debug_Address(Debug::k_debug_break_return_address); } + static Debug_Address RestarterFrameFunctionPointer() { + return Debug_Address(Debug::k_restarter_frame_function_pointer); + } + static Debug_Address Register(int reg) { return Debug_Address(Debug::k_register_address, reg); } @@ -950,6 +976,9 @@ class Debug_Address { return reinterpret_cast
(Debug::debug_break_return_address()); case Debug::k_debug_break_slot_address: return reinterpret_cast
(Debug::debug_break_slot_address()); + case Debug::k_restarter_frame_function_pointer: + return reinterpret_cast
( + Debug::restarter_frame_function_pointer_address()); case Debug::k_register_address: return reinterpret_cast
(Debug::register_address(reg_)); default: diff --git a/src/ia32/debug-ia32.cc b/src/ia32/debug-ia32.cc index 9b558bd..dfa6634 100644 --- a/src/ia32/debug-ia32.cc +++ b/src/ia32/debug-ia32.cc @@ -265,6 +265,10 @@ void Debug::GeneratePlainReturnLiveEdit(MacroAssembler* masm) { // -- context // -- frame base void Debug::GenerateFrameDropperLiveEdit(MacroAssembler* masm) { + ExternalReference restarter_frame_function_slot = + ExternalReference(Debug_Address::RestarterFrameFunctionPointer()); + __ mov(Operand::StaticVariable(restarter_frame_function_slot), Immediate(0)); + // We do not know our frame height, but set esp based on ebp. __ lea(esp, Operand(ebp, -4 * kPointerSize)); @@ -288,8 +292,10 @@ void Debug::GenerateFrameDropperLiveEdit(MacroAssembler* masm) { #undef __ -void Debug::SetUpFrameDropperFrame(StackFrame* bottom_js_frame, - Handle code) { +// TODO(LiveEdit): consider making it platform-independent. +// TODO(LiveEdit): use more named constants instead of numbers. +Object** Debug::SetUpFrameDropperFrame(StackFrame* bottom_js_frame, + Handle code) { ASSERT(bottom_js_frame->is_java_script()); Address fp = bottom_js_frame->fp(); @@ -298,7 +304,10 @@ void Debug::SetUpFrameDropperFrame(StackFrame* bottom_js_frame, Memory::Object_at(fp - 3 * kPointerSize) = *code; Memory::Object_at(fp - 2 * kPointerSize) = Smi::FromInt(StackFrame::INTERNAL); + + return reinterpret_cast(&Memory::Object_at(fp - 4 * kPointerSize)); } + const int Debug::kFrameDropperFrameSize = 5; diff --git a/src/liveedit.cc b/src/liveedit.cc index 04631a3..346d9ea 100644 --- a/src/liveedit.cc +++ b/src/liveedit.cc @@ -1188,7 +1188,8 @@ static bool FixTryCatchHandler(StackFrame* top_frame, static const char* DropFrames(Vector frames, int top_frame_index, int bottom_js_frame_index, - Debug::FrameDropMode* mode) { + Debug::FrameDropMode* mode, + Object*** restarter_frame_function_pointer) { if (Debug::kFrameDropperFrameSize < 0) { return "Stack manipulations are not supported in this architecture."; } @@ -1238,7 +1239,10 @@ static const char* DropFrames(Vector frames, top_frame->set_pc(code->entry()); pre_top_frame->SetCallerFp(bottom_js_frame->fp()); - Debug::SetUpFrameDropperFrame(bottom_js_frame, code); + *restarter_frame_function_pointer = + Debug::SetUpFrameDropperFrame(bottom_js_frame, code); + + ASSERT((**restarter_frame_function_pointer)->IsJSFunction()); for (Address a = unused_stack_top; a < unused_stack_bottom; @@ -1328,8 +1332,10 @@ static const char* DropActivationsInActiveThread( } Debug::FrameDropMode drop_mode = Debug::FRAMES_UNTOUCHED; + Object** restarter_frame_function_pointer = NULL; const char* error_message = DropFrames(frames, top_frame_index, - bottom_js_frame_index, &drop_mode); + bottom_js_frame_index, &drop_mode, + &restarter_frame_function_pointer); if (error_message != NULL) { return error_message; @@ -1343,7 +1349,8 @@ static const char* DropActivationsInActiveThread( break; } } - Debug::FramesHaveBeenDropped(new_id, drop_mode); + Debug::FramesHaveBeenDropped(new_id, drop_mode, + restarter_frame_function_pointer); // Replace "blocked on active" with "replaced on active" status. for (int i = 0; i < array_len; i++) { diff --git a/src/mips/debug-mips.cc b/src/mips/debug-mips.cc index 8c40930..47961fa 100644 --- a/src/mips/debug-mips.cc +++ b/src/mips/debug-mips.cc @@ -117,9 +117,10 @@ void Debug::GenerateFrameDropperLiveEdit(MacroAssembler* masm) { #undef __ -void Debug::SetUpFrameDropperFrame(StackFrame* bottom_js_frame, - Handle code) { +Object** Debug::SetUpFrameDropperFrame(StackFrame* bottom_js_frame, + Handle code) { UNREACHABLE(); + return NULL; } const int Debug::kFrameDropperFrameSize = -1; diff --git a/src/x64/debug-x64.cc b/src/x64/debug-x64.cc index 9659254..2aa77e7 100644 --- a/src/x64/debug-x64.cc +++ b/src/x64/debug-x64.cc @@ -213,9 +213,10 @@ void Debug::GenerateFrameDropperLiveEdit(MacroAssembler* masm) { #undef __ -void Debug::SetUpFrameDropperFrame(StackFrame* bottom_js_frame, - Handle code) { +Object** Debug::SetUpFrameDropperFrame(StackFrame* bottom_js_frame, + Handle code) { UNREACHABLE(); + return NULL; } const int Debug::kFrameDropperFrameSize = -1; -- 2.7.4