From 4b3e67070e7cc46f897f69e5b221f5d1db50a214 Mon Sep 17 00:00:00 2001 From: "peter.rybin@gmail.com" Date: Mon, 3 Dec 2012 20:29:29 +0000 Subject: [PATCH] Issue 2399 part 1: In debugger allow modifying local variable values Review URL: https://codereview.chromium.org/11415042 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@13122 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/debug-debugger.js | 100 ++++++++++++++++-- src/mirror-debugger.js | 25 +++++ src/runtime.cc | 131 +++++++++++++++++++++++ src/runtime.h | 1 + test/mjsunit/debug-set-variable-value.js | 176 +++++++++++++++++++++++++++++++ 5 files changed, 422 insertions(+), 11 deletions(-) create mode 100644 test/mjsunit/debug-set-variable-value.js diff --git a/src/debug-debugger.js b/src/debug-debugger.js index 163a0bd..dfad902 100644 --- a/src/debug-debugger.js +++ b/src/debug-debugger.js @@ -1427,6 +1427,8 @@ DebugCommandProcessor.prototype.processDebugJSONRequest = function( this.scopesRequest_(request, response); } else if (request.command == 'scope') { this.scopeRequest_(request, response); + } else if (request.command == 'setVariableValue') { + this.setVariableValueRequest_(request, response); } else if (request.command == 'evaluate') { this.evaluateRequest_(request, response); } else if (lol_is_enabled && request.command == 'getobj') { @@ -1953,11 +1955,12 @@ DebugCommandProcessor.prototype.frameRequest_ = function(request, response) { }; -DebugCommandProcessor.prototype.frameForScopeRequest_ = function(request) { +DebugCommandProcessor.prototype.resolveFrameFromScopeDescription_ = + function(scope_description) { // Get the frame for which the scope or scopes are requested. // With no frameNumber argument use the currently selected frame. - if (request.arguments && !IS_UNDEFINED(request.arguments.frameNumber)) { - frame_index = request.arguments.frameNumber; + if (scope_description && !IS_UNDEFINED(scope_description.frameNumber)) { + frame_index = scope_description.frameNumber; if (frame_index < 0 || this.exec_state_.frameCount() <= frame_index) { throw new Error('Invalid frame number'); } @@ -1971,13 +1974,13 @@ DebugCommandProcessor.prototype.frameForScopeRequest_ = function(request) { // Gets scope host object from request. It is either a function // ('functionHandle' argument must be specified) or a stack frame // ('frameNumber' may be specified and the current frame is taken by default). -DebugCommandProcessor.prototype.scopeHolderForScopeRequest_ = - function(request) { - if (request.arguments && "functionHandle" in request.arguments) { - if (!IS_NUMBER(request.arguments.functionHandle)) { +DebugCommandProcessor.prototype.resolveScopeHolder_ = + function(scope_description) { + if (scope_description && "functionHandle" in scope_description) { + if (!IS_NUMBER(scope_description.functionHandle)) { throw new Error('Function handle must be a number'); } - var function_mirror = LookupMirror(request.arguments.functionHandle); + var function_mirror = LookupMirror(scope_description.functionHandle); if (!function_mirror) { throw new Error('Failed to find function object by handle'); } @@ -1992,14 +1995,14 @@ DebugCommandProcessor.prototype.scopeHolderForScopeRequest_ = } // Get the frame for which the scopes are requested. - var frame = this.frameForScopeRequest_(request); + var frame = this.resolveFrameFromScopeDescription_(scope_description); return frame; } } DebugCommandProcessor.prototype.scopesRequest_ = function(request, response) { - var scope_holder = this.scopeHolderForScopeRequest_(request); + var scope_holder = this.resolveScopeHolder_(request.arguments); // Fill all scopes for this frame or function. var total_scopes = scope_holder.scopeCount(); @@ -2018,7 +2021,7 @@ DebugCommandProcessor.prototype.scopesRequest_ = function(request, response) { DebugCommandProcessor.prototype.scopeRequest_ = function(request, response) { // Get the frame or function for which the scope is requested. - var scope_holder = this.scopeHolderForScopeRequest_(request); + var scope_holder = this.resolveScopeHolder_(request.arguments); // With no scope argument just return top scope. var scope_index = 0; @@ -2033,6 +2036,77 @@ DebugCommandProcessor.prototype.scopeRequest_ = function(request, response) { }; +// Reads value from protocol description. Description may be in form of type +// (for singletons), raw value (primitive types supported in JSON), +// string value description plus type (for primitive values) or handle id. +// Returns raw value or throws exception. +DebugCommandProcessor.resolveValue_ = function(value_description) { + if ("handle" in value_description) { + var value_mirror = LookupMirror(value_description.handle); + if (!value_mirror) { + throw new Error("Failed to resolve value by handle, ' #" + + mapping.handle + "# not found"); + } + return value_mirror.value(); + } else if ("stringDescription" in value_description) { + if (value_description.type == BOOLEAN_TYPE) { + return Boolean(value_description.stringDescription); + } else if (value_description.type == NUMBER_TYPE) { + return Number(value_description.stringDescription); + } if (value_description.type == STRING_TYPE) { + return String(value_description.stringDescription); + } else { + throw new Error("Unknown type"); + } + } else if ("value" in value_description) { + return value_description.value; + } else if (value_description.type == UNDEFINED_TYPE) { + return void 0; + } else if (value_description.type == NULL_TYPE) { + return null; + } else { + throw new Error("Failed to parse value description"); + } +}; + + +DebugCommandProcessor.prototype.setVariableValueRequest_ = + function(request, response) { + if (!request.arguments) { + response.failed('Missing arguments'); + return; + } + + if (IS_UNDEFINED(request.arguments.name)) { + response.failed('Missing variable name'); + } + var variable_name = request.arguments.name; + + var scope_description = request.arguments.scope; + + // Get the frame or function for which the scope is requested. + var scope_holder = this.resolveScopeHolder_(scope_description); + + if (IS_UNDEFINED(scope_description.number)) { + response.failed('Missing scope number'); + } + var scope_index = %ToNumber(scope_description.number); + + var scope = scope_holder.scope(scope_index); + + var new_value = + DebugCommandProcessor.resolveValue_(request.arguments.newValue); + + scope.setVariableValue(variable_name, new_value); + + var new_value_mirror = MakeMirror(new_value); + + response.body = { + newValue: new_value_mirror + }; +}; + + DebugCommandProcessor.prototype.evaluateRequest_ = function(request, response) { if (!request.arguments) { return response.failed('Missing arguments'); @@ -2663,3 +2737,7 @@ function ValueToProtocolValue_(value, mirror_serializer) { } return json; } + +Debug.TestApi = { + CommandProcessorResolveValue: DebugCommandProcessor.resolveValue_ +}; diff --git a/src/mirror-debugger.js b/src/mirror-debugger.js index a5331a0..7f1a05a 100644 --- a/src/mirror-debugger.js +++ b/src/mirror-debugger.js @@ -1844,10 +1844,14 @@ function ScopeDetails(frame, fun, index) { frame.details_.frameId(), frame.details_.inlinedFrameIndex(), index); + this.frame_id_ = frame.details_.frameId(); + this.inlined_frame_id_ = frame.details_.inlinedFrameIndex(); } else { this.details_ = %GetFunctionScopeDetails(fun.value(), index); + this.fun_value_ = fun.value(); this.break_id_ = undefined; } + this.index_ = index; } @@ -1867,6 +1871,22 @@ ScopeDetails.prototype.object = function() { }; +ScopeDetails.prototype.setVariableValueImpl = function(name, new_value) { + var raw_res; + if (!IS_UNDEFINED(this.break_id_)) { + %CheckExecutionState(this.break_id_); + raw_res = %SetScopeVariableValue(this.break_id_, this.frame_id_, + this.inlined_frame_id_, this.index_, name, new_value); + } else { + raw_res = %SetScopeVariableValue(this.fun_value_, null, null, this.index_, + name, new_value); + } + if (!raw_res) { + throw new Error("Failed to set variable value"); + } +}; + + /** * Mirror object for scope of frame or function. Either frame or function must * be specified. @@ -1914,6 +1934,11 @@ ScopeMirror.prototype.scopeObject = function() { }; +ScopeMirror.prototype.setVariableValue = function(name, new_value) { + this.details_.setVariableValueImpl(name, new_value); +}; + + /** * Mirror object for script source. * @param {Script} script The script object diff --git a/src/runtime.cc b/src/runtime.cc index c162dcb..5cf2d44 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -10902,6 +10902,52 @@ static Handle MaterializeClosure(Isolate* isolate, } +// This method copies structure of MaterializeClosure method above. +static bool SetClosureVariableValue(Isolate* isolate, + Handle context, + Handle variable_name, + Handle new_value) { + ASSERT(context->IsFunctionContext()); + + Handle shared(context->closure()->shared()); + Handle scope_info(shared->scope_info()); + + // Context locals to the context extension. + for (int i = 0; i < scope_info->ContextLocalCount(); i++) { + Handle next_name(scope_info->ContextLocalName(i)); + if (variable_name->Equals(*next_name)) { + VariableMode mode; + InitializationFlag init_flag; + int context_index = + scope_info->ContextSlotIndex(*next_name, &mode, &init_flag); + if (context_index < 0) { + return false; + } + context->set(context_index, *new_value); + return true; + } + } + + // Properties from the function context extension. This will + // be variables introduced by eval. + if (context->has_extension()) { + Handle ext(JSObject::cast(context->extension())); + if (ext->HasProperty(*variable_name)) { + // We don't expect this to do anything except replacing property value. + SetProperty(isolate, + ext, + variable_name, + new_value, + NONE, + kNonStrictMode); + return true; + } + } + + return false; +} + + // Create a plain JSObject which materializes the scope for the specified // catch context. static Handle MaterializeCatchScope(Isolate* isolate, @@ -11182,6 +11228,33 @@ class ScopeIterator { return Handle(); } + bool SetVariableValue(Handle variable_name, + Handle new_value) { + ASSERT(!failed_); + switch (Type()) { + case ScopeIterator::ScopeTypeGlobal: + break; + case ScopeIterator::ScopeTypeLocal: + // TODO(2399): implement. + break; + case ScopeIterator::ScopeTypeWith: + break; + case ScopeIterator::ScopeTypeCatch: + // TODO(2399): implement. + break; + case ScopeIterator::ScopeTypeClosure: + return SetClosureVariableValue(isolate_, CurrentContext(), + variable_name, new_value); + case ScopeIterator::ScopeTypeBlock: + // TODO(2399): should we implement it? + break; + case ScopeIterator::ScopeTypeModule: + // TODO(2399): should we implement it? + break; + } + return false; + } + Handle CurrentScopeInfo() { ASSERT(!failed_); if (!nested_scope_chain_.is_empty()) { @@ -11421,6 +11494,64 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetFunctionScopeDetails) { } +static bool SetScopeVariableValue(ScopeIterator* it, int index, + Handle variable_name, + Handle new_value) { + for (int n = 0; !it->Done() && n < index; it->Next()) { + n++; + } + if (it->Done()) { + return false; + } + return it->SetVariableValue(variable_name, new_value); +} + + +// Change variable value in closure or local scope +// args[0]: number or JsFunction: break id or function +// args[1]: number: frame index (when arg[0] is break id) +// args[2]: number: inlined frame index (when arg[0] is break id) +// args[3]: number: scope index +// args[4]: string: variable name +// args[5]: object: new value +// +// Return true if success and false otherwise +RUNTIME_FUNCTION(MaybeObject*, Runtime_SetScopeVariableValue) { + HandleScope scope(isolate); + ASSERT(args.length() == 6); + + // Check arguments. + CONVERT_NUMBER_CHECKED(int, index, Int32, args[3]); + CONVERT_ARG_HANDLE_CHECKED(String, variable_name, 4); + Handle new_value = args.at(5); + + bool res; + if (args[0]->IsNumber()) { + Object* check; + { MaybeObject* maybe_check = Runtime_CheckExecutionState( + RUNTIME_ARGUMENTS(isolate, args)); + if (!maybe_check->ToObject(&check)) return maybe_check; + } + CONVERT_SMI_ARG_CHECKED(wrapped_id, 1); + CONVERT_NUMBER_CHECKED(int, inlined_jsframe_index, Int32, args[2]); + + // Get the frame where the debugging is performed. + StackFrame::Id id = UnwrapFrameId(wrapped_id); + JavaScriptFrameIterator frame_it(isolate, id); + JavaScriptFrame* frame = frame_it.frame(); + + ScopeIterator it(isolate, frame, inlined_jsframe_index); + res = SetScopeVariableValue(&it, index, variable_name, new_value); + } else { + CONVERT_ARG_HANDLE_CHECKED(JSFunction, fun, 0); + ScopeIterator it(isolate, fun); + res = SetScopeVariableValue(&it, index, variable_name, new_value); + } + + return isolate->heap()->ToBoolean(res); +} + + RUNTIME_FUNCTION(MaybeObject*, Runtime_DebugPrintScopes) { HandleScope scope(isolate); ASSERT(args.length() == 0); diff --git a/src/runtime.h b/src/runtime.h index 19ff62d..9d53c35 100644 --- a/src/runtime.h +++ b/src/runtime.h @@ -433,6 +433,7 @@ namespace internal { F(GetScopeDetails, 4, 1) \ F(GetFunctionScopeCount, 1, 1) \ F(GetFunctionScopeDetails, 2, 1) \ + F(SetScopeVariableValue, 6, 1) \ F(DebugPrintScopes, 0, 1) \ F(GetThreadCount, 1, 1) \ F(GetThreadDetails, 2, 1) \ diff --git a/test/mjsunit/debug-set-variable-value.js b/test/mjsunit/debug-set-variable-value.js new file mode 100644 index 0000000..dac8861 --- /dev/null +++ b/test/mjsunit/debug-set-variable-value.js @@ -0,0 +1,176 @@ +// Copyright 2012 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. +var Debug = debug.Debug; + +// Accepts a function/closure 'fun' that must have a debugger statement inside. +// A variable 'variable_name' must be initialized before debugger statement +// and returned after the statement. The test will alter variable value when +// on debugger statement and check that returned value reflects the change. +function RunPauseTest(scope_number, variable_name, expected_new_value, fun) { + var old_value = fun(); + + var listener_delegate; + var listener_called = false; + var exception = null; + + function listener_delegate(exec_state) { + var scope = exec_state.frame(0).scope(scope_number); + scope.setVariableValue(variable_name, expected_new_value); + } + + function listener(event, exec_state, event_data, data) { + try { + if (event == Debug.DebugEvent.Break) { + listener_called = true; + listener_delegate(exec_state); + } + } catch (e) { + exception = e; + } + } + + // Add the debug event listener. + Debug.setListener(listener); + + var actual_new_value; + try { + actual_new_value = fun(); + } finally { + Debug.setListener(null); + } + + if (exception != null) { + assertUnreachable("Exception: " + exception); + } + assertTrue(listener_called); + + assertTrue(old_value != actual_new_value); + assertTrue(expected_new_value == actual_new_value); +} + +// Accepts a closure 'fun' that returns a variable from it's outer scope. +// The test changes the value of variable via the handle to function and checks +// that the return value changed accordingly. +function RunClosureTest(scope_number, variable_name, expected_new_value, fun) { + var old_value = fun(); + + var fun_mirror = Debug.MakeMirror(fun); + + var scope = fun_mirror.scope(scope_number); + scope.setVariableValue(variable_name, expected_new_value); + + var actual_new_value = fun(); + + assertTrue(old_value != actual_new_value); + assertTrue(expected_new_value == actual_new_value); +} + +// Test changing variable value when in pause +RunPauseTest(1, 'v1', 5, (function Factory() { + var v1 = 'cat'; + return function() { + debugger; + return v1; + } +})()); + +RunPauseTest(1, 'v2', 11, (function Factory(v2) { + return function() { + debugger; + return v2; + } +})('dog')); + +RunPauseTest(3, 'foo', 77, (function Factory() { + var foo = "capybara"; + return (function() { + var bar = "fish"; + try { + throw {name: "test exception"}; + } catch (e) { + return function() { + debugger; + bar = "beast"; + return foo; + } + } + })(); +})()); + + + +// Test changing variable value in closure by handle +RunClosureTest(0, 'v1', 5, (function Factory() { + var v1 = 'cat'; + return function() { + return v1; + } +})()); + +RunClosureTest(0, 'v2', 11, (function Factory(v2) { + return function() { + return v2; + } +})('dog')); + +RunClosureTest(2, 'foo', 77, (function Factory() { + var foo = "capybara"; + return (function() { + var bar = "fish"; + try { + throw {name: "test exception"}; + } catch (e) { + return function() { + bar = "beast"; + return foo; + } + } + })(); +})()); + + +// Test value description protocol JSON +assertEquals(true, Debug.TestApi.CommandProcessorResolveValue({value: true})); + +assertSame(null, Debug.TestApi.CommandProcessorResolveValue({type: "null"})); +assertSame(undefined, + Debug.TestApi.CommandProcessorResolveValue({type: "undefined"})); + +assertSame("123", Debug.TestApi.CommandProcessorResolveValue( + {type: "string", stringDescription: "123"})); +assertSame(123, Debug.TestApi.CommandProcessorResolveValue( + {type: "number", stringDescription: "123"})); + +assertSame(Number, Debug.TestApi.CommandProcessorResolveValue( + {handle: Debug.MakeMirror(Number).handle()})); +assertSame(RunClosureTest, Debug.TestApi.CommandProcessorResolveValue( + {handle: Debug.MakeMirror(RunClosureTest).handle()})); + -- 2.7.4