Revision: 13202
Author: [email protected]
Date: Tue Dec 11 15:27:38 2012
Log: Issue 2399 part 2: In debugger allow modifying local variable
values
Review URL: https://codereview.chromium.org/11412310
http://code.google.com/p/v8/source/detail?r=13202
Modified:
/branches/bleeding_edge/src/frames.cc
/branches/bleeding_edge/src/frames.h
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/test/mjsunit/debug-set-variable-value.js
=======================================
--- /branches/bleeding_edge/src/frames.cc Mon Dec 10 03:09:12 2012
+++ /branches/bleeding_edge/src/frames.cc Tue Dec 11 15:27:38 2012
@@ -690,6 +690,11 @@
// Visit the return address in the callee and incoming arguments.
IteratePc(v, pc_address(), code);
}
+
+
+void JavaScriptFrame::SetParameterValue(int index, Object* value) const {
+ Memory::Object_at(GetParameterSlot(index)) = value;
+}
bool JavaScriptFrame::IsConstructor() const {
=======================================
--- /branches/bleeding_edge/src/frames.h Mon Dec 10 03:09:12 2012
+++ /branches/bleeding_edge/src/frames.h Tue Dec 11 15:27:38 2012
@@ -499,6 +499,9 @@
inline int ComputeParametersCount() const {
return GetNumberOfIncomingArguments();
}
+
+ // Debugger access.
+ void SetParameterValue(int index, Object* value) const;
// Check if this frame is a constructor frame invoked through 'new'.
bool IsConstructor() const;
=======================================
--- /branches/bleeding_edge/src/runtime.cc Tue Dec 11 06:55:23 2012
+++ /branches/bleeding_edge/src/runtime.cc Tue Dec 11 15:27:38 2012
@@ -10796,6 +10796,92 @@
frame,
&frame_inspector);
}
+
+
+// Set the context local variable value.
+static bool SetContextLocalValue(Isolate* isolate,
+ Handle<ScopeInfo> scope_info,
+ Handle<Context> context,
+ Handle<String> variable_name,
+ Handle<Object> new_value) {
+ for (int i = 0; i < scope_info->ContextLocalCount(); i++) {
+ Handle<String> 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);
+ context->set(context_index, *new_value);
+ return true;
+ }
+ }
+
+ return false;
+}
+
+
+static bool SetLocalVariableValue(Isolate* isolate,
+ JavaScriptFrame* frame,
+ int inlined_jsframe_index,
+ Handle<String> variable_name,
+ Handle<Object> new_value) {
+ if (inlined_jsframe_index != 0 || frame->is_optimized()) {
+ // Optimized frames are not supported.
+ return false;
+ }
+
+ Handle<JSFunction> function(JSFunction::cast(frame->function()));
+ Handle<SharedFunctionInfo> shared(function->shared());
+ Handle<ScopeInfo> scope_info(shared->scope_info());
+
+ // Parameters.
+ for (int i = 0; i < scope_info->ParameterCount(); ++i) {
+ if (scope_info->ParameterName(i)->Equals(*variable_name)) {
+ frame->SetParameterValue(i, *new_value);
+ return true;
+ }
+ }
+
+ // Stack locals.
+ for (int i = 0; i < scope_info->StackLocalCount(); ++i) {
+ if (scope_info->StackLocalName(i)->Equals(*variable_name)) {
+ frame->SetExpression(i, *new_value);
+ return true;
+ }
+ }
+
+ if (scope_info->HasContext()) {
+ // Context locals.
+ Handle<Context> frame_context(Context::cast(frame->context()));
+ Handle<Context> function_context(frame_context->declaration_context());
+ if (SetContextLocalValue(
+ isolate, scope_info, function_context, variable_name, new_value)) {
+ return true;
+ }
+
+ // Function context extension. These are variables introduced by eval.
+ if (function_context->closure() == *function) {
+ if (function_context->has_extension() &&
+ !function_context->IsNativeContext()) {
+ Handle<JSObject>
ext(JSObject::cast(function_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 closure content for the
@@ -10858,19 +10944,9 @@
Handle<ScopeInfo> scope_info(shared->scope_info());
// Context locals to the context extension.
- for (int i = 0; i < scope_info->ContextLocalCount(); i++) {
- Handle<String> 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;
- }
+ if (SetContextLocalValue(
+ isolate, scope_info, context, variable_name, new_value)) {
+ return true;
}
// Properties from the function context extension. This will
@@ -10913,6 +10989,20 @@
Handle<JSObject>());
return catch_scope;
}
+
+
+static bool SetCatchVariableValue(Isolate* isolate,
+ Handle<Context> context,
+ Handle<String> variable_name,
+ Handle<Object> new_value) {
+ ASSERT(context->IsCatchContext());
+ Handle<String> name(String::cast(context->extension()));
+ if (!name->Equals(*variable_name)) {
+ return false;
+ }
+ context->set(Context::THROWN_OBJECT_INDEX, *new_value);
+ return true;
+}
// Create a plain JSObject which materializes the block scope for the
specified
@@ -11180,13 +11270,13 @@
case ScopeIterator::ScopeTypeGlobal:
break;
case ScopeIterator::ScopeTypeLocal:
- // TODO(2399): implement.
- break;
+ return SetLocalVariableValue(isolate_, frame_,
inlined_jsframe_index_,
+ variable_name, new_value);
case ScopeIterator::ScopeTypeWith:
break;
case ScopeIterator::ScopeTypeCatch:
- // TODO(2399): implement.
- break;
+ return SetCatchVariableValue(isolate_, CurrentContext(),
+ variable_name, new_value);
case ScopeIterator::ScopeTypeClosure:
return SetClosureVariableValue(isolate_, CurrentContext(),
variable_name, new_value);
=======================================
--- /branches/bleeding_edge/test/mjsunit/debug-set-variable-value.js Mon
Dec 3 12:29:29 2012
+++ /branches/bleeding_edge/test/mjsunit/debug-set-variable-value.js Tue
Dec 11 15:27:38 2012
@@ -34,8 +34,10 @@
// 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();
+function RunPauseTest(scope_number, expected_old_result, variable_name,
+ new_value, expected_new_result, fun) {
+ var actual_old_result = fun();
+ assertEquals(expected_old_result, actual_old_result);
var listener_delegate;
var listener_called = false;
@@ -43,7 +45,7 @@
function listener_delegate(exec_state) {
var scope = exec_state.frame(0).scope(scope_number);
- scope.setVariableValue(variable_name, expected_new_value);
+ scope.setVariableValue(variable_name, new_value);
}
function listener(event, exec_state, event_data, data) {
@@ -62,102 +64,201 @@
var actual_new_value;
try {
- actual_new_value = fun();
+ actual_new_result = fun();
} finally {
Debug.setListener(null);
}
if (exception != null) {
- assertUnreachable("Exception: " + exception);
+ assertUnreachable("Exception in listener\n" + exception.stack);
}
assertTrue(listener_called);
- assertTrue(old_value != actual_new_value);
- assertTrue(expected_new_value == actual_new_value);
+ assertEquals(expected_new_result, actual_new_result);
}
// 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();
+function RunClosureTest(scope_number, expected_old_result, variable_name,
+ new_value, expected_new_result, fun) {
+ var actual_old_result = fun();
+ assertEquals(expected_old_result, actual_old_result);
var fun_mirror = Debug.MakeMirror(fun);
var scope = fun_mirror.scope(scope_number);
- scope.setVariableValue(variable_name, expected_new_value);
+ scope.setVariableValue(variable_name, new_value);
- var actual_new_value = fun();
+ var actual_new_result = fun();
- assertTrue(old_value != actual_new_value);
- assertTrue(expected_new_value == actual_new_value);
+ assertEquals(expected_new_result, actual_new_result);
}
-// Test changing variable value when in pause
-RunPauseTest(1, 'v1', 5, (function Factory() {
- var v1 = 'cat';
+
+function ClosureTestCase(scope_index, old_result, variable_name, new_value,
+ new_result, success_expected, factory) {
+ this.scope_index_ = scope_index;
+ this.old_result_ = old_result;
+ this.variable_name_ = variable_name;
+ this.new_value_ = new_value;
+ this.new_result_ = new_result;
+ this.success_expected_ = success_expected;
+ this.factory_ = factory;
+}
+
+ClosureTestCase.prototype.run_pause_test = function() {
+ var th = this;
+ var fun = this.factory_(true);
+ this.run_and_catch_(function() {
+ RunPauseTest(th.scope_index_ + 1, th.old_result_, th.variable_name_,
+ th.new_value_, th.new_result_, fun);
+ });
+}
+
+ClosureTestCase.prototype.run_closure_test = function() {
+ var th = this;
+ var fun = this.factory_(false);
+ this.run_and_catch_(function() {
+ RunClosureTest(th.scope_index_, th.old_result_, th.variable_name_,
+ th.new_value_, th.new_result_, fun);
+ });
+}
+
+ClosureTestCase.prototype.run_and_catch_ = function(runnable) {
+ if (this.success_expected_) {
+ runnable();
+ } else {
+ assertThrows(runnable);
+ }
+}
+
+
+// Test scopes visible from closures.
+
+var closure_test_cases = [
+ new ClosureTestCase(0, 'cat', 'v1', 5, 5, true,
+ function Factory(debug_stop) {
+ var v1 = 'cat';
+ return function() {
+ if (debug_stop) debugger;
+ return v1;
+ }
+ }),
+
+ new ClosureTestCase(0, 4, 't', 7, 9, true, function Factory(debug_stop) {
+ var t = 2;
+ var r = eval("t");
+ return function() {
+ if (debug_stop) debugger;
+ return r + t;
+ }
+ }),
+
+ new ClosureTestCase(0, 6, 't', 10, 13, true, function
Factory(debug_stop) {
+ var t = 2;
+ var r = eval("t = 3");
+ return function() {
+ if (debug_stop) debugger;
+ return r + t;
+ }
+ }),
+
+ new ClosureTestCase(0, 17, 's', 'Bird', 'Bird', true,
+ function Factory(debug_stop) {
+ eval("var s = 17");
+ return function() {
+ if (debug_stop) debugger;
+ return s;
+ }
+ }),
+
+ new ClosureTestCase(2, 'capybara', 'foo', 77, 77, true,
+ function Factory(debug_stop) {
+ var foo = "capybara";
+ return (function() {
+ var bar = "fish";
+ try {
+ throw {name: "test exception"};
+ } catch (e) {
+ return function() {
+ if (debug_stop) debugger;
+ bar = "beast";
+ return foo;
+ }
+ }
+ })();
+ }),
+
+ new ClosureTestCase(0, 'AlphaBeta', 'eee', 5, '5Beta', true,
+ function Factory(debug_stop) {
+ var foo = "Beta";
+ return (function() {
+ var bar = "fish";
+ try {
+ throw "Alpha";
+ } catch (eee) {
+ return function() {
+ if (debug_stop) debugger;
+ return eee + foo;
+ }
+ }
+ })();
+ })
+];
+
+for (var i = 0; i < closure_test_cases.length; i++) {
+ closure_test_cases[i].run_pause_test();
+}
+
+for (var i = 0; i < closure_test_cases.length; i++) {
+ closure_test_cases[i].run_closure_test();
+}
+
+
+// Test local scope.
+
+RunPauseTest(0, 'HelloYou', 'u', 'We', 'HelloWe', (function Factory() {
return function() {
+ var u = "You";
+ var v = "Hello";
debugger;
- return v1;
+ return v + u;
}
})());
-RunPauseTest(1, 'v2', 11, (function Factory(v2) {
+RunPauseTest(0, 'Helloworld', 'p', 'GoodBye', 'HelloGoodBye',
+ (function Factory() {
+ function H(p) {
+ var v = "Hello";
+ debugger;
+ return v + p;
+ }
return function() {
- debugger;
- return v2;
+ return H("world");
}
-})('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';
+RunPauseTest(0, 'mouse', 'v1', 'dog', 'dog', (function Factory() {
return function() {
+ var v1 = 'cat';
+ eval("v1 = 'mouse'");
+ debugger;
return v1;
}
})());
-RunClosureTest(0, 'v2', 11, (function Factory(v2) {
+RunPauseTest(0, 'mouse', 'v1', 'dog', 'dog', (function Factory() {
return function() {
- return v2;
+ eval("var v1 = 'mouse'");
+ debugger;
+ return v1;
}
-})('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"}));
@@ -173,4 +274,3 @@
{handle: Debug.MakeMirror(Number).handle()}));
assertSame(RunClosureTest, Debug.TestApi.CommandProcessorResolveValue(
{handle: Debug.MakeMirror(RunClosureTest).handle()}));
-
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
