From 5fcc52fcb967a271eacb646e4c58bc6cb6943e50 Mon Sep 17 00:00:00 2001 From: "yangguo@chromium.org" Date: Thu, 21 Mar 2013 08:50:29 +0000 Subject: [PATCH] Simplify debug evaluate. R=peter.rybin@gmail.com BUG=v8:2585, 173608 Review URL: https://chromiumcodereview.appspot.com/12953002 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@14019 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/runtime.cc | 223 ++++++++++++---------------------- test/cctest/test-heap.cc | 4 + test/mjsunit/debug-evaluate-locals.js | 16 ++- 3 files changed, 94 insertions(+), 149 deletions(-) diff --git a/src/runtime.cc b/src/runtime.cc index 934c82e..0b5d209 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -9015,7 +9015,7 @@ static ObjectPair CompileGlobalEval(Isolate* isolate, // and return the compiled function bound in the local context. Handle shared = Compiler::CompileEval( source, - Handle(isolate->context()), + context, context->IsNativeContext(), language_mode, NO_PARSE_RESTRICTION, @@ -11919,21 +11919,58 @@ static Handle GetArgumentsObject(Isolate* isolate, } -static const char kSourceStr[] = - "(function(arguments,__source__){return eval(__source__);})"; +// Compile and evaluate source for the given context. +static MaybeObject* DebugEvaluate(Isolate* isolate, + Handle context, + Handle context_extension, + Handle receiver, + Handle source) { + if (context_extension->IsJSObject()) { + Handle extension = Handle::cast(context_extension); + Handle closure(context->closure(), isolate); + context = isolate->factory()->NewWithContext(closure, context, extension); + } + + Handle shared = Compiler::CompileEval( + source, + context, + context->IsNativeContext(), + CLASSIC_MODE, + NO_PARSE_RESTRICTION, + RelocInfo::kNoPosition); + if (shared.is_null()) return Failure::Exception(); + + Handle eval_fun = + isolate->factory()->NewFunctionFromSharedFunctionInfo( + shared, context, NOT_TENURED); + bool pending_exception; + Handle result = Execution::Call( + eval_fun, receiver, 0, NULL, &pending_exception); + + if (pending_exception) return Failure::Exception(); + + // Skip the global proxy as it has no properties and always delegates to the + // real global object. + if (result->IsJSGlobalProxy()) { + result = Handle(JSObject::cast(result->GetPrototype(isolate))); + } + + // Clear the oneshot breakpoints so that the debugger does not step further. + isolate->debug()->ClearStepping(); + return *result; +} // Evaluate a piece of JavaScript in the context of a stack frame for -// debugging. This is accomplished by creating a new context which in its -// extension part has all the parameters and locals of the function on the -// stack frame. A function which calls eval with the code to evaluate is then -// compiled in this context and called in this context. As this context -// replaces the context of the function on the stack frame a new (empty) -// function is created as well to be used as the closure for the context. -// This function and the context acts as replacements for the function on the -// stack frame presenting the same view of the values of parameters and -// local variables as if the piece of JavaScript was evaluated at the point -// where the function on the stack frame is currently stopped. +// debugging. This is done by creating a new context which in its extension +// part has all the parameters and locals of the function on the stack frame +// as well as a materialized arguments object. As this context replaces +// the context of the function on the stack frame a new (empty) function +// is created as well to be used as the closure for the context. +// This closure as replacements for the one on the stack frame presenting +// the same view of the values of parameters and local variables as if the +// piece of JavaScript was evaluated at the point where the function on the +// stack frame is currently stopped when we compile and run the (direct) eval. RUNTIME_FUNCTION(MaybeObject*, Runtime_DebugEvaluate) { HandleScope scope(isolate); @@ -11941,17 +11978,15 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DebugEvaluate) { // evaluated. ASSERT(args.length() == 6); Object* check_result; - { MaybeObject* maybe_check_result = Runtime_CheckExecutionState( + { MaybeObject* maybe_result = Runtime_CheckExecutionState( RUNTIME_ARGUMENTS(isolate, args)); - if (!maybe_check_result->ToObject(&check_result)) { - return maybe_check_result; - } + if (!maybe_result->ToObject(&check_result)) return maybe_result; } CONVERT_SMI_ARG_CHECKED(wrapped_id, 1); CONVERT_NUMBER_CHECKED(int, inlined_jsframe_index, Int32, args[2]); CONVERT_ARG_HANDLE_CHECKED(String, source, 3); CONVERT_BOOLEAN_ARG_CHECKED(disable_break, 4); - Handle additional_context(args[5], isolate); + Handle context_extension(args[5], isolate); // Handle the processing of break. DisableBreak disable_break_save(disable_break); @@ -11962,7 +11997,6 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DebugEvaluate) { JavaScriptFrame* frame = it.frame(); FrameInspector frame_inspector(frame, inlined_jsframe_index, isolate); Handle function(JSFunction::cast(frame_inspector.GetFunction())); - Handle scope_info(function->shared()->scope_info()); // Traverse the saved contexts chain to find the active context for the // selected frame. @@ -11987,28 +12021,39 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DebugEvaluate) { ASSERT(go_between_scope_info->ContextLocalCount() == 0); #endif - // Materialize the content of the local scope into a JSObject. + // Materialize the content of the local scope including the arguments object. Handle local_scope = MaterializeLocalScopeWithFrameInspector( isolate, frame, &frame_inspector); RETURN_IF_EMPTY_HANDLE(isolate, local_scope); + Handle frame_context(Context::cast(frame->context())); + Handle function_context; + Handle scope_info(function->shared()->scope_info()); + if (scope_info->HasContext()) { + function_context = Handle(frame_context->declaration_context()); + } + Handle arguments = GetArgumentsObject(isolate, + frame, + &frame_inspector, + scope_info, + function_context); + SetProperty(isolate, + local_scope, + isolate->factory()->arguments_string(), + arguments, + ::NONE, + kNonStrictMode); + // Allocate a new context for the debug evaluation and set the extension // object build. - Handle context = - isolate->factory()->NewFunctionContext(Context::MIN_CONTEXT_SLOTS, - go_between); + Handle context = isolate->factory()->NewFunctionContext( + Context::MIN_CONTEXT_SLOTS, go_between); // Use the materialized local scope in a with context. context = isolate->factory()->NewWithContext(go_between, context, local_scope); // Copy any with contexts present and chain them in front of this context. - Handle frame_context(Context::cast(frame->context())); - Handle function_context; - // Get the function's context if it has one. - if (scope_info->HasContext()) { - function_context = Handle(frame_context->declaration_context()); - } context = CopyNestedScopeContextChain(isolate, go_between, context, @@ -12021,79 +12066,8 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DebugEvaluate) { return exception; } - if (additional_context->IsJSObject()) { - Handle extension = Handle::cast(additional_context); - context = - isolate->factory()->NewWithContext(go_between, context, extension); - } - - // Wrap the evaluation statement in a new function compiled in the newly - // created context. The function has one parameter which has to be called - // 'arguments'. This it to have access to what would have been 'arguments' in - // the function being debugged. - // function(arguments,__source__) {return eval(__source__);} - - Handle function_source = - isolate->factory()->NewStringFromAscii( - Vector(kSourceStr, sizeof(kSourceStr) - 1)); - - // Currently, the eval code will be executed in non-strict mode, - // even in the strict code context. - Handle shared = - Compiler::CompileEval(function_source, - context, - context->IsNativeContext(), - CLASSIC_MODE, - NO_PARSE_RESTRICTION, - RelocInfo::kNoPosition); - if (shared.is_null()) return Failure::Exception(); - Handle compiled_function = - isolate->factory()->NewFunctionFromSharedFunctionInfo(shared, context); - - // Invoke the result of the compilation to get the evaluation function. - bool has_pending_exception; Handle receiver(frame->receiver(), isolate); - Handle evaluation_function = - Execution::Call(compiled_function, receiver, 0, NULL, - &has_pending_exception); - if (has_pending_exception) return Failure::Exception(); - - Handle arguments = GetArgumentsObject(isolate, - frame, - &frame_inspector, - scope_info, - function_context); - - // Check if eval is blocked in the context and temporarily allow it - // for debugger. - Handle native_context = Handle(context->native_context()); - bool eval_disabled = - native_context->allow_code_gen_from_strings()->IsFalse(); - if (eval_disabled) { - native_context->set_allow_code_gen_from_strings( - isolate->heap()->true_value()); - } - // Invoke the evaluation function and return the result. - Handle argv[] = { arguments, source }; - Handle result = - Execution::Call(Handle::cast(evaluation_function), - receiver, - ARRAY_SIZE(argv), - argv, - &has_pending_exception); - if (eval_disabled) { - native_context->set_allow_code_gen_from_strings( - isolate->heap()->false_value()); - } - if (has_pending_exception) return Failure::Exception(); - - // Skip the global proxy as it has no properties and always delegates to the - // real global object. - if (result->IsJSGlobalProxy()) { - result = Handle(JSObject::cast(result->GetPrototype(isolate))); - } - - return *result; + return DebugEvaluate(isolate, context, context_extension, receiver, source); } @@ -12104,15 +12078,13 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DebugEvaluateGlobal) { // evaluated. ASSERT(args.length() == 4); Object* check_result; - { MaybeObject* maybe_check_result = Runtime_CheckExecutionState( + { MaybeObject* maybe_result = Runtime_CheckExecutionState( RUNTIME_ARGUMENTS(isolate, args)); - if (!maybe_check_result->ToObject(&check_result)) { - return maybe_check_result; - } + if (!maybe_result->ToObject(&check_result)) return maybe_result; } CONVERT_ARG_HANDLE_CHECKED(String, source, 1); CONVERT_BOOLEAN_ARG_CHECKED(disable_break, 2); - Handle additional_context(args[3], isolate); + Handle context_extension(args[3], isolate); // Handle the processing of break. DisableBreak disable_break_save(disable_break); @@ -12130,45 +12102,8 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DebugEvaluateGlobal) { // Get the native context now set to the top context from before the // debugger was invoked. Handle context = isolate->native_context(); - - bool is_global = true; - - if (additional_context->IsJSObject()) { - // Create a new with context with the additional context information between - // the context of the debugged function and the eval code to be executed. - context = isolate->factory()->NewWithContext( - Handle(context->closure()), - context, - Handle::cast(additional_context)); - is_global = false; - } - - // Compile the source to be evaluated. - // Currently, the eval code will be executed in non-strict mode, - // even in the strict code context. - Handle shared = - Compiler::CompileEval(source, - context, - is_global, - CLASSIC_MODE, - NO_PARSE_RESTRICTION, - RelocInfo::kNoPosition); - if (shared.is_null()) return Failure::Exception(); - Handle compiled_function = - Handle( - isolate->factory()->NewFunctionFromSharedFunctionInfo(shared, - context)); - - // Invoke the result of the compilation to get the evaluation function. - bool has_pending_exception; Handle receiver = isolate->global_object(); - Handle result = - Execution::Call(compiled_function, receiver, 0, NULL, - &has_pending_exception); - // Clear the oneshot breakpoints so that the debugger does not step further. - isolate->debug()->ClearStepping(); - if (has_pending_exception) return Failure::Exception(); - return *result; + return DebugEvaluate(isolate, context, context_extension, receiver, source); } diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc index a710385..d0bec93 100644 --- a/test/cctest/test-heap.cc +++ b/test/cctest/test-heap.cc @@ -1240,6 +1240,7 @@ TEST(TestCodeFlushingIncrementalAbort) { // code flushing candidate. SimulateIncrementalMarking(); +#ifdef ENABLE_DEBUGGER_SUPPORT // Enable the debugger and add a breakpoint while incremental marking // is running so that incremental marking aborts and code flushing is // disabled. @@ -1247,6 +1248,7 @@ TEST(TestCodeFlushingIncrementalAbort) { Handle breakpoint_object(Smi::FromInt(0), isolate); isolate->debug()->SetBreakPoint(function, breakpoint_object, &position); isolate->debug()->ClearAllBreakPoints(); +#endif // ENABLE_DEBUGGER_SUPPORT // Force optimization now that code flushing is disabled. { v8::HandleScope scope(env->GetIsolate()); @@ -3010,8 +3012,10 @@ TEST(Regress173458) { // explicitly enqueued. SimulateIncrementalMarking(); +#ifdef ENABLE_DEBUGGER_SUPPORT // Now enable the debugger which in turn will disable code flushing. CHECK(isolate->debug()->Load()); +#endif // ENABLE_DEBUGGER_SUPPORT // This cycle will bust the heap and subsequent cycles will go ballistic. heap->CollectAllGarbage(Heap::kNoGCFlags); diff --git a/test/mjsunit/debug-evaluate-locals.js b/test/mjsunit/debug-evaluate-locals.js index 61b6dd9..a68162d 100644 --- a/test/mjsunit/debug-evaluate-locals.js +++ b/test/mjsunit/debug-evaluate-locals.js @@ -36,19 +36,20 @@ exception = false; function h() { var a = 1; var b = 2; + var eval = 5; // Overriding eval should not break anything. debugger; // Breakpoint. } function checkFrame0(frame) { // Frame 0 (h) has normal variables a and b. var count = frame.localCount(); - assertEquals(2, count); + assertEquals(3, count); for (var i = 0; i < count; ++i) { var name = frame.localName(i); var value = frame.localValue(i).value(); if (name == 'a') { assertEquals(1, value); - } else { + } else if (name !='eval') { assertEquals('b', name); assertEquals(2, value); } @@ -115,16 +116,21 @@ function listener(event, exec_state, event_data, data) { // Evaluating a and b on frames 0, 1 and 2 produces 1, 2, 3, 4, 5 and 6. assertEquals(1, exec_state.frame(0).evaluate('a').value()); assertEquals(2, exec_state.frame(0).evaluate('b').value()); + assertEquals(5, exec_state.frame(0).evaluate('eval').value()); assertEquals(3, exec_state.frame(1).evaluate('a').value()); assertEquals(4, exec_state.frame(1).evaluate('b').value()); + assertEquals("function", + typeof exec_state.frame(1).evaluate('eval').value()); assertEquals(5, exec_state.frame(2).evaluate('a').value()); assertEquals(6, exec_state.frame(2).evaluate('b').value()); - + assertEquals("function", + typeof exec_state.frame(2).evaluate('eval').value()); // Indicate that all was processed. listenerComplete = true; } } catch (e) { exception = e + print("Caught something. " + e + " " + e.stack); }; }; @@ -133,6 +139,6 @@ Debug.setListener(listener); f(); -// Make sure that the debug event listener vas invoked. -assertTrue(listenerComplete); +// Make sure that the debug event listener was invoked. assertFalse(exception, "exception in listener") +assertTrue(listenerComplete); -- 2.7.4