AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[debugger] debug-evaluate should not not modify local values.

Browse Source 
Debug evaluate no longer writes back changes to the replicated
context chain to the original after execution. Changes to the
global object or script contexts still stick. Calling functions
that bind to the original context chain also have their expected
side effects.

As far as I can tell, DevTools is not interested in modifying
local variable values. Modifying global variable values still
works as expected. However, I have not yet removed the old
implementation, but merely keep it behind a flag.

R=mstarzinger@chromium.org, rossberg@chromium.org

Committed: https://crrev.com/92caa9b85eefffbef51c67428397951bd2e2c330
Cr-Commit-Position: refs/heads/master@{#32841}

Review URL: https://codereview.chromium.org/1513183003

Cr-Commit-Position: refs/heads/master@{#32857}
This commit is contained in:
yangguo 2015-12-15 01:53:55 -08:00 committed by Commit bot
parent 6d8a2611c0
commit abe2feb081
13 changed files with 94 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
src/debug/debug-evaluate.cc
View File

@ -68,16 +68,19 @@ MaybeHandle<Object> DebugEvaluate::Local(Isolate* isolate,
// variables accessible by the function we are evaluating from are
// materialized and included on top of the native context. Changes to
// the materialized object are written back afterwards.
// Note that the native context is taken from the original context chain,
// which may not be the current native context of the isolate.
ContextBuilder context_builder(isolate, frame, inlined_jsframe_index);
if (isolate->has_pending_exception()) return MaybeHandle<Object>();
Handle<Context> context = isolate->native_context();
Handle<Context> context = context_builder.native_context();
Handle<JSObject> receiver(context->global_proxy());
Handle<SharedFunctionInfo> outer_info(context->closure()->shared(), isolate);
MaybeHandle<Object> maybe_result = Evaluate(
isolate, context_builder.outer_info(),
context_builder.innermost_context(), context_extension, receiver, source);
if (!maybe_result.is_null()) context_builder.UpdateValues();
if (!maybe_result.is_null() && !FLAG_debug_eval_readonly_locals) {
context_builder.UpdateValues();
}
return maybe_result;
}
@ -127,8 +130,8 @@ DebugEvaluate::ContextBuilder::ContextBuilder(Isolate* isolate,
Handle<JSFunction> local_function =
handle(JSFunction::cast(frame_inspector.GetFunction()));
Handle<Context> outer_context(local_function->context());
Handle<Context> native_context = isolate->native_context();
Handle<JSFunction> global_function(native_context->closure());
native_context_ = Handle<Context>(outer_context->native_context());
Handle<JSFunction> global_function(native_context_->closure());
outer_info_ = handle(global_function->shared());
Handle<Context> inner_context;
@ -166,7 +169,7 @@ DebugEvaluate::ContextBuilder::ContextBuilder(Isolate* isolate,
// The "this" binding, if any, can't be bound via "with". If we need
// to, add another node onto the outer context to bind "this".
Handle<Context> receiver_context =
MaterializeReceiver(native_context, local_context, local_function,
MaterializeReceiver(native_context_, local_context, local_function,
global_function, it.ThisIsNonLocal());
Handle<JSObject> materialized_function = NewJSObjectWithNullProto();
@ -309,7 +312,7 @@ void DebugEvaluate::ContextBuilder::MaterializeArgumentsObject(
MaybeHandle<Object> DebugEvaluate::ContextBuilder::LoadFromContext(
Handle<Context> context, Handle<String> name) {
Handle<Context> context, Handle<String> name, bool* global) {
static const ContextLookupFlags flags = FOLLOW_CONTEXT_CHAIN;
int index;
PropertyAttributes attributes;
@ -320,9 +323,13 @@ MaybeHandle<Object> DebugEvaluate::ContextBuilder::LoadFromContext(
Handle<Object> value;
if (index != Context::kNotFound) { // Found on context.
Handle<Context> context = Handle<Context>::cast(holder);
// Do not shadow variables on the script context.
*global = context->IsScriptContext();
return Handle<Object>(context->get(index), isolate_);
} else { // Found on object.
Handle<JSReceiver> object = Handle<JSReceiver>::cast(holder);
// Do not shadow properties on the global object.
*global = object->IsJSGlobalObject();
return JSReceiver::GetDataProperty(object, name);
}
}
@ -333,7 +340,13 @@ void DebugEvaluate::ContextBuilder::MaterializeContextChain(
for (const Handle<String>& name : non_locals_) {
HandleScope scope(isolate_);
Handle<Object> value;
if (!LoadFromContext(context, name).ToHandle(&value)) continue;
bool global;
if (!LoadFromContext(context, name, &global).ToHandle(&value) || global) {
// If resolving the variable fails, skip it. If it resolves to a global
// variable, skip it as well since it's not read-only and can be resolved
// within debug-evaluate.
continue;
}
JSObject::SetOwnPropertyIgnoreAttributes(target, name, value, NONE).Check();
}
}
@ -381,7 +394,8 @@ Handle<Context> DebugEvaluate::ContextBuilder::MaterializeReceiver(
Handle<Object> receiver = isolate_->factory()->undefined_value();
Handle<String> this_string = isolate_->factory()->this_string();
if (this_is_non_local) {
LoadFromContext(lookup_context, this_string).ToHandle(&receiver);
bool global;
LoadFromContext(lookup_context, this_string, &global).ToHandle(&receiver);
} else if (local_function->shared()->scope_info()->HasReceiver()) {
receiver = handle(frame_->receiver(), isolate_);
}

4
src/debug/debug-evaluate.h
View File

@ -54,6 +54,7 @@ class DebugEvaluate : public AllStatic {
void UpdateValues();
Handle<Context> innermost_context() const { return innermost_context_; }
Handle<Context> native_context() const { return native_context_; }
Handle<SharedFunctionInfo> outer_info() const { return outer_info_; }
private:
@ -87,13 +88,14 @@ class DebugEvaluate : public AllStatic {
bool this_is_non_local);
MaybeHandle<Object> LoadFromContext(Handle<Context> context,
Handle<String> name);
Handle<String> name, bool* global);
void StoreToContext(Handle<Context> context, Handle<String> name,
Handle<Object> value);
Handle<SharedFunctionInfo> outer_info_;
Handle<Context> innermost_context_;
Handle<Context> native_context_;
List<ContextChainElement> context_chain_;
List<Handle<String> > non_locals_;
Isolate* isolate_;

8
src/flag-definitions.h
View File

@ -597,8 +597,7 @@ DEFINE_BOOL(cache_prototype_transitions, true, "cache prototype transitions")
DEFINE_INT(cpu_profiler_sampling_interval, 1000,
"CPU profiler sampling interval in microseconds")
// debug.cc
DEFINE_BOOL(trace_debug_json, false, "trace debugging JSON request/response")
// Array abuse tracing
DEFINE_BOOL(trace_js_array_abuse, false,
"trace out-of-bounds accesses to JS arrays")
DEFINE_BOOL(trace_external_array_abuse, false,
@ -607,6 +606,11 @@ DEFINE_BOOL(trace_array_abuse, false,
"trace out-of-bounds accesses to all arrays")
DEFINE_IMPLICATION(trace_array_abuse, trace_js_array_abuse)
DEFINE_IMPLICATION(trace_array_abuse, trace_external_array_abuse)
// debugger
DEFINE_BOOL(debug_eval_readonly_locals, true,
"do not update locals after debug-evaluate")
DEFINE_BOOL(trace_debug_json, false, "trace debugging JSON request/response")
DEFINE_BOOL(enable_liveedit, true, "enable liveedit experimental feature")
DEFINE_BOOL(hard_abort, true, "abort by crashing")

27
test/mjsunit/debug-evaluate-closure.js
View File

@ -26,6 +26,7 @@
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
// Flags: --expose-debug-as debug --allow-natives-syntax
// Flags: --debug-eval-readonly-locals
Debug = debug.Debug;
var listened = false;
@ -34,20 +35,18 @@ function listener(event, exec_state, event_data, data) {
if (event != Debug.DebugEvent.Break) return;
try {
assertEquals("goo", exec_state.frame(0).evaluate("goo").value());
exec_state.frame(0).evaluate("goo = 'goo foo'");
exec_state.frame(0).evaluate("goo = 'goo foo'"); // no effect
assertEquals("bar return", exec_state.frame(0).evaluate("bar()").value());
// Check that calling bar() has no effect to context-allocated variables.
// TODO(yangguo): reevaluate this if we no longer update context from copy.
assertEquals("inner", exec_state.frame(0).evaluate("inner").value());
assertEquals("outer", exec_state.frame(0).evaluate("outer").value());
assertEquals("inner bar", exec_state.frame(0).evaluate("inner").value());
assertEquals("outer bar", exec_state.frame(0).evaluate("outer").value());
assertEquals("baz inner", exec_state.frame(0).evaluate("baz").value());
assertEquals("baz outer", exec_state.frame(1).evaluate("baz").value());
exec_state.frame(0).evaluate("w = 'w foo'");
exec_state.frame(0).evaluate("inner = 'inner foo'");
exec_state.frame(0).evaluate("outer = 'outer foo'");
exec_state.frame(0).evaluate("baz = 'baz inner foo'");
exec_state.frame(1).evaluate("baz = 'baz outer foo'");
exec_state.frame(0).evaluate("inner = 'inner foo'"); // no effect
exec_state.frame(0).evaluate("outer = 'outer foo'"); // has effect
exec_state.frame(0).evaluate("baz = 'baz inner foo'"); // no effect
exec_state.frame(1).evaluate("baz = 'baz outer foo'"); // has effect
listened = true;
} catch (e) {
print(e);
@ -69,9 +68,9 @@ function foo() {
with (withv) {
var bar = function bar() {
assertEquals("goo foo", goo);
inner = "inner bar"; // this has no effect, when called from debug-eval
outer = "outer bar"; // this has no effect, when called from debug-eval
assertEquals("goo", goo);
inner = "inner bar";
outer = "outer bar";
v = "v bar";
return "bar return";
};
@ -81,8 +80,8 @@ function foo() {
debugger;
}
assertEquals("inner foo", inner);
assertEquals("baz inner foo", baz);
assertEquals("inner bar", inner);
assertEquals("baz inner", baz);
assertEquals("w foo", withw.w);
assertEquals("v bar", withv.v);
}

10
test/mjsunit/debug-evaluate-const.js
View File

@ -25,7 +25,7 @@
// (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
// Flags: --expose-debug-as debug --debug-eval-readonly-locals
Debug = debug.Debug
@ -50,10 +50,8 @@ function f() {
debugger; // Break point.
assertEquals(30, var0);
// TODO(yangguo): debug evaluate should not be able to alter
// stack-allocated const values
// assertEquals(0, const0);
assertEquals(undefined, var0);
assertEquals(0, const0);
assertEquals(undefined, const1);
assertEquals(undefined, const2);
var var0 = 20;
@ -66,7 +64,7 @@ function f() {
g();
assertEquals(31, var1);
assertEquals(21, var1);
assertEquals(3, const3);
}

8
test/mjsunit/debug-evaluate-locals-capturing.js
View File

@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --expose-debug-as debug
// Flags: --expose-debug-as debug --debug-eval-readonly-locals
Debug = debug.Debug
var exception = null;
@ -15,8 +15,8 @@ function listener(event, exec_state, event_data, data) {
if (event == Debug.DebugEvent.Break) {
var frameMirror = exec_state.frame(0);
f = frameMirror.evaluate('f = function() { i = 5; }, f(), f').value();
print(f);
var i = frameMirror.evaluate('f = function() { i = 5; }, f(), i').value();
assertEquals(5, i);
}
} catch(e) {
exception = e;
@ -35,7 +35,7 @@ Debug.setListener(listener);
} catch (e) {
assertEquals(0, i);
debugger;
assertEquals(5, i);
assertEquals(0, i);
}
}());

5
test/mjsunit/debug-evaluate-locals.js
View File

@ -25,7 +25,7 @@
// (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
// Flags: --expose-debug-as debug --debug-eval-readonly-locals
// Get the Debug object exposed from the debug context global object.
Debug = debug.Debug
@ -127,6 +127,7 @@ function listener(event, exec_state, event_data, data) {
assertEquals(6, exec_state.frame(2).evaluate('b').value());
assertEquals("function",
typeof exec_state.frame(2).evaluate('eval').value());
// Assignments to local variables only have temporary effect.
assertEquals("foo",
exec_state.frame(0).evaluate('a = "foo"').value());
assertEquals("bar",
@ -145,7 +146,7 @@ Debug.setListener(listener);
var f_result = f();
assertEquals('foobar', f_result);
assertEquals(4, f_result);
// Make sure that the debug event listener was invoked.
assertFalse(exception, "exception in listener")

10
test/mjsunit/debug-evaluate-modify-catch-block-scope.js
View File

@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --expose-debug-as debug
// Flags: --expose-debug-as debug --debug-eval-readonly-locals
Debug = debug.Debug
@ -26,12 +26,12 @@ Debug.setListener(listener);
} catch (e) {
let a = 1;
function bar() {
a = 2; // this has no effect, when called from debug-eval
e = 2; // this has no effect, when called from debug-eval.
a = 2;
e = 2;
}
debugger;
assertEquals(1, a);
assertEquals(1, e);
assertEquals(2, a);
assertEquals(2, e);
}
})();

11
test/mjsunit/debug-evaluate-shadowed-context.js
View File

@ -29,7 +29,8 @@ function listener(event, exec_state, event_data, data) {
}
assertEquals("[object global]",
String(exec_state.frame(0).evaluate("this").value()));
exec_state.frame(0).evaluate("y = 'Y'");
assertEquals("y", exec_state.frame(0).evaluate("y").value());
assertEquals("a", exec_state.frame(0).evaluate("a").value());
exec_state.frame(0).evaluate("a = 'A'");
assertThrows(() => exec_state.frame(0).evaluate("z"), ReferenceError);
} catch (e) {
@ -41,7 +42,7 @@ function listener(event, exec_state, event_data, data) {
Debug.setListener(listener);
var a = "a";
assertEquals("Y", (function() {
(function() {
var x = 1; // context allocate x
(() => x);
var y = "y";
@ -54,12 +55,12 @@ assertEquals("Y", (function() {
})(); // 2
})(); // 1
return y;
})());
})();
assertEquals("A", a);
a = "a";
assertEquals("Y", (function() {
(function() {
var x = 1; // context allocate x
(() => x);
var y = "y";
@ -74,7 +75,7 @@ assertEquals("Y", (function() {
})(); // 2
})(); // 1
return y;
})());
})();
assertEquals("A", a);

11
test/mjsunit/es6/debug-blockscopes.js
View File

@ -26,6 +26,7 @@
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
// Flags: --expose-debug-as debug --allow-natives-syntax
// Flags: --debug-eval-readonly-locals
// The functions used for testing backtraces. They are at the top to make the
// testing of source line/column easier.
@ -525,15 +526,12 @@ function shadowing_1() {
{
let i = 5;
debugger;
assertEqualsUnlessOptimized(27, i, shadowing_1);
}
assertEquals(0, i);
debugger;
assertEqualsUnlessOptimized(27, i, shadowing_1);
}
listener_delegate = function (exec_state) {
exec_state.frame(0).evaluate("i = 27");
assertEqualsUnlessOptimized(5, exec_state.frame(0).evaluate("i").value());
}
shadowing_1();
EndTest();
@ -546,13 +544,12 @@ function shadowing_2() {
{
let j = 5;
debugger;
assertEqualsUnlessOptimized(27, j, shadowing_2);
}
assertEqualsUnlessOptimized(0, i, shadowing_2);
}
listener_delegate = function (exec_state) {
exec_state.frame(0).evaluate("j = 27");
assertEqualsUnlessOptimized(0, exec_state.frame(0).evaluate("i").value());
assertEqualsUnlessOptimized(5, exec_state.frame(0).evaluate("j").value());
}
shadowing_2();
EndTest();

14
test/mjsunit/regress-3225.js
View File

@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --expose-debug-as debug
// Flags: --expose-debug-as debug --debug-eval-readonly-locals
Debug = debug.Debug
@ -15,12 +15,13 @@ function listener(event, exec_state, event_data, data) {
if (debug_step == 0) {
assertEquals(1, exec_state.frame(0).evaluate('a').value());
assertEquals(3, exec_state.frame(0).evaluate('b').value());
exec_state.frame(0).evaluate("a = 4").value();
exec_state.frame(0).evaluate("a = 4").value(); // no effect.
debug_step++;
} else {
assertEquals(4, exec_state.frame(0).evaluate('a').value());
assertEquals(1, exec_state.frame(0).evaluate('a').value());
assertEquals(3, exec_state.frame(0).evaluate('b').value());
exec_state.frame(0).evaluate("b = 5").value();
exec_state.frame(0).evaluate("set_a_to_5()");
exec_state.frame(0).evaluate("b = 5").value(); // no effect.
}
} catch (e) {
failure = e;
@ -30,19 +31,22 @@ function listener(event, exec_state, event_data, data) {
Debug.setListener(listener);
function* generator(a, b) {
function set_a_to_5() { a = 5 }
var b = 3; // Shadows a parameter.
debugger;
yield a;
yield b;
debugger;
yield a;
return b;
}
var foo = generator(1, 2);
assertEquals(4, foo.next().value);
assertEquals(1, foo.next().value);
assertEquals(3, foo.next().value);
assertEquals(5, foo.next().value);
assertEquals(3, foo.next().value);
assertNull(failure);
Debug.setListener(null);

8
test/mjsunit/regress/regress-325676.js
View File

@ -25,7 +25,7 @@
// (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
// Flags: --expose-debug-as debug --debug-eval-readonly-locals
// If a function parameter is forced to be context allocated,
// debug evaluate need to resolve it to a context slot instead of
@ -40,7 +40,7 @@ function listener(event, exec_state, event_data, data) {
if (event != Debug.DebugEvent.Break) return;
try {
assertEquals(expected, exec_state.frame(0).evaluate('arg').value());
exec_state.frame(0).evaluate('arg = "evaluated";');
exec_state.frame(0).evaluate('arg = "evaluated";'); // no effect
} catch (e) {
exception = e;
}
@ -51,12 +51,12 @@ Debug.setListener(listener);
function f(arg) {
expected = arg;
debugger;
assertEquals("evaluated", arg);
assertEquals(expected, arg);
arg = "value";
expected = arg;
debugger;
assertEquals("evaluated", arg);
assertEquals(expected, arg);
// Forces arg to be context allocated even though a parameter.
function g() { arg; }

16
test/mjsunit/regress/regress-crbug-323936.js
View File

@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --expose-debug-as debug
// Flags: --expose-debug-as debug --debug-eval-readonly-locals
Debug = debug.Debug;
@ -14,11 +14,11 @@ function listener(event, exec_state, event_data, data) {
try {
if (step == 0) {
assertEquals("error", exec_state.frame(0).evaluate("e").value());
exec_state.frame(0).evaluate("e = 'foo'");
exec_state.frame(0).evaluate("x = 'modified'");
exec_state.frame(0).evaluate("write_0('foo')");
exec_state.frame(0).evaluate("write_1('modified')");
} else {
assertEquals("argument", exec_state.frame(0).evaluate("e").value());
exec_state.frame(0).evaluate("e = 'bar'");
assertEquals("foo", exec_state.frame(0).evaluate("e").value());
exec_state.frame(0).evaluate("write_2('bar')");
}
step++;
} catch (e) {
@ -33,9 +33,13 @@ function f(e, x) {
try {
throw "error";
} catch(e) {
// 'e' and 'x' bind to the argument due to hoisting
function write_0(v) { e = v }
function write_1(v) { x = v }
debugger;
assertEquals("foo", e);
assertEquals("error", e);
}
function write_2(v) { e = v }
debugger;
assertEquals("bar", e);
assertEquals("modified", x);