From b4cb3e28ca0e90f8b593680bee64f31dedf62f4b Mon Sep 17 00:00:00 2001 From: "yangguo@chromium.org" Date: Mon, 9 Jul 2012 15:18:08 +0000 Subject: [PATCH] Fix Debug::Break crash. BUG=131642 TEST=test-debug/Regress131642 Review URL: https://chromiumcodereview.appspot.com/10698123 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@12019 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/debug.cc | 74 +++++---------------------------------- src/debug.h | 8 ----- src/execution.cc | 6 ++++ src/runtime.cc | 4 ++- test/cctest/test-debug.cc | 43 +++++++++++++++++++++++ 5 files changed, 60 insertions(+), 75 deletions(-) diff --git a/src/debug.cc b/src/debug.cc index 40ddabb3c..da928159d 100644 --- a/src/debug.cc +++ b/src/debug.cc @@ -892,27 +892,6 @@ void Debug::Iterate(ObjectVisitor* v) { } -// TODO(131642): Remove this when fixed. -void Debug::PutValuesOnStackAndDie(int start, - Address c_entry_fp, - Address last_fp, - Address larger_fp, - int count, - char* stack, - int end) { - OS::PrintError("start: %d\n", start); - OS::PrintError("c_entry_fp: %p\n", static_cast(c_entry_fp)); - OS::PrintError("last_fp: %p\n", static_cast(last_fp)); - OS::PrintError("larger_fp: %p\n", static_cast(larger_fp)); - OS::PrintError("count: %d\n", count); - if (stack != NULL) { - OS::PrintError("stack: %s\n", stack); - } - OS::PrintError("end: %d\n", end); - OS::Abort(); -} - - Object* Debug::Break(Arguments args) { Heap* heap = isolate_->heap(); HandleScope scope(isolate_); @@ -1010,53 +989,16 @@ Object* Debug::Break(Arguments args) { it.Advance(); } - // TODO(131642): Remove this when fixed. - // Catch the cases that would lead to crashes and capture - // - C entry FP at which to start stack crawl. - // - FP of the frame at which we plan to stop stepping out (last FP). - // - current FP that's larger than last FP. - // - Counter for the number of steps to step out. - // - stack trace string. - if (it.done()) { - // We crawled the entire stack, never reaching last_fp_. - Handle stack = isolate_->StackTraceString(); - char buffer[8192]; - int length = Min(8192, stack->length()); - String::WriteToFlat(*stack, buffer, 0, length - 1); - PutValuesOnStackAndDie(0xBEEEEEEE, - frame->fp(), - thread_local_.last_fp_, - reinterpret_cast
(0xDEADDEAD), - count, - buffer, - 0xCEEEEEEE); - } else if (it.frame()->fp() != thread_local_.last_fp_) { - // We crawled over last_fp_, without getting a match. - Handle stack = isolate_->StackTraceString(); - char buffer[8192]; - int length = Min(8192, stack->length()); - String::WriteToFlat(*stack, buffer, 0, length - 1); - PutValuesOnStackAndDie(0xDEEEEEEE, - frame->fp(), - thread_local_.last_fp_, - it.frame()->fp(), - count, - buffer, - 0xFEEEEEEE); + // Check that we indeed found the frame we are looking for. + CHECK(!it.done() && (it.frame()->fp() == thread_local_.last_fp_)); + if (step_count > 1) { + // Save old count and action to continue stepping after StepOut. + thread_local_.queued_step_count_ = step_count - 1; } - // If we found original frame - if (it.frame()->fp() == thread_local_.last_fp_) { - if (step_count > 1) { - // Save old count and action to continue stepping after - // StepOut - thread_local_.queued_step_count_ = step_count - 1; - } - - // Set up for StepOut to reach target frame - step_action = StepOut; - step_count = count; - } + // Set up for StepOut to reach target frame. + step_action = StepOut; + step_count = count; } // Clear all current stepping setup. diff --git a/src/debug.h b/src/debug.h index f3215c93d..bb804206c 100644 --- a/src/debug.h +++ b/src/debug.h @@ -232,14 +232,6 @@ class Debug { void PreemptionWhileInDebugger(); void Iterate(ObjectVisitor* v); - // TODO(131642): Remove this when fixed. - NO_INLINE(void PutValuesOnStackAndDie(int start, - Address c_entry_fp, - Address last_fp, - Address larger_fp, - int count, - char* stack, - int end)); Object* Break(Arguments args); void SetBreakPoint(Handle function, Handle break_point_object, diff --git a/src/execution.cc b/src/execution.cc index 561897567..40ed7de41 100644 --- a/src/execution.cc +++ b/src/execution.cc @@ -132,6 +132,12 @@ static Handle Invoke(bool is_construct, V8::FatalProcessOutOfMemory("JS", true); } } +#ifdef ENABLE_DEBUGGER_SUPPORT + // Reset stepping state when script exits with uncaught exception. + if (isolate->debugger()->IsDebuggerActive()) { + isolate->debug()->ClearStepping(); + } +#endif // ENABLE_DEBUGGER_SUPPORT return Handle(); } else { isolate->clear_pending_message(); diff --git a/src/runtime.cc b/src/runtime.cc index 3909da6a7..93479d89a 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -4893,7 +4893,9 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_StoreArrayLiteralElement) { // to a built-in function such as Array.forEach. RUNTIME_FUNCTION(MaybeObject*, Runtime_DebugCallbackSupportsStepping) { #ifdef ENABLE_DEBUGGER_SUPPORT - if (!isolate->IsDebuggerActive()) return isolate->heap()->false_value(); + if (!isolate->IsDebuggerActive() || !isolate->debug()->StepInActive()) { + return isolate->heap()->false_value(); + } CONVERT_ARG_CHECKED(Object, callback, 0); // We do not step into the callback if it's a builtin or not even a function. if (!callback->IsJSFunction() || JSFunction::cast(callback)->IsBuiltin()) { diff --git a/test/cctest/test-debug.cc b/test/cctest/test-debug.cc index 84245946c..9f52cea2b 100644 --- a/test/cctest/test-debug.cc +++ b/test/cctest/test-debug.cc @@ -7349,4 +7349,47 @@ TEST(DebugBreakInline) { } +static void DebugEventStepNext(v8::DebugEvent event, + v8::Handle exec_state, + v8::Handle event_data, + v8::Handle data) { + if (event == v8::Break) { + PrepareStep(StepNext); + } +} + + +static void RunScriptInANewCFrame(const char* source) { + v8::TryCatch try_catch; + CompileRun(source); + CHECK(try_catch.HasCaught()); +} + + +TEST(Regress131642) { + // Bug description: + // When doing StepNext through the first script, the debugger is not reset + // after exiting through exception. A flawed implementation enabling the + // debugger to step into Array.prototype.forEach breaks inside the callback + // for forEach in the second script under the assumption that we are in a + // recursive call. In an attempt to step out, we crawl the stack using the + // recorded frame pointer from the first script and fail when not finding it + // on the stack. + v8::HandleScope scope; + DebugLocalContext env; + v8::Debug::SetDebugEventListener(DebugEventStepNext); + + // We step through the first script. It exits through an exception. We run + // this inside a new frame to record a different FP than the second script + // would expect. + const char* script_1 = "debugger; throw new Error();"; + RunScriptInANewCFrame(script_1); + + // The second script uses forEach. + const char* script_2 = "[0].forEach(function() { });"; + CompileRun(script_2); + + v8::Debug::SetDebugEventListener(NULL); +} + #endif // ENABLE_DEBUGGER_SUPPORT -- 2.34.1