diff --git a/src/debug/debug-evaluate.cc b/src/debug/debug-evaluate.cc index e19b93eebe..9d0d38446e 100644 --- a/src/debug/debug-evaluate.cc +++ b/src/debug/debug-evaluate.cc @@ -68,19 +68,16 @@ MaybeHandle 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(); - Handle context = context_builder.native_context(); + Handle context = isolate->native_context(); Handle receiver(context->global_proxy()); + Handle outer_info(context->closure()->shared(), isolate); MaybeHandle maybe_result = Evaluate( isolate, context_builder.outer_info(), context_builder.innermost_context(), context_extension, receiver, source); - if (!maybe_result.is_null() && !FLAG_debug_eval_readonly_locals) { - context_builder.UpdateValues(); - } + if (!maybe_result.is_null()) context_builder.UpdateValues(); return maybe_result; } @@ -130,8 +127,8 @@ DebugEvaluate::ContextBuilder::ContextBuilder(Isolate* isolate, Handle local_function = handle(JSFunction::cast(frame_inspector.GetFunction())); Handle outer_context(local_function->context()); - native_context_ = Handle(outer_context->native_context()); - Handle global_function(native_context_->closure()); + Handle native_context = isolate->native_context(); + Handle global_function(native_context->closure()); outer_info_ = handle(global_function->shared()); Handle inner_context; @@ -169,7 +166,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 receiver_context = - MaterializeReceiver(native_context_, local_context, local_function, + MaterializeReceiver(native_context, local_context, local_function, global_function, it.ThisIsNonLocal()); Handle materialized_function = NewJSObjectWithNullProto(); @@ -312,7 +309,7 @@ void DebugEvaluate::ContextBuilder::MaterializeArgumentsObject( MaybeHandle DebugEvaluate::ContextBuilder::LoadFromContext( - Handle context, Handle name, bool* global) { + Handle context, Handle name) { static const ContextLookupFlags flags = FOLLOW_CONTEXT_CHAIN; int index; PropertyAttributes attributes; @@ -323,13 +320,9 @@ MaybeHandle DebugEvaluate::ContextBuilder::LoadFromContext( Handle value; if (index != Context::kNotFound) { // Found on context. Handle context = Handle::cast(holder); - // Do not shadow variables on the script context. - *global = context->IsScriptContext(); return Handle(context->get(index), isolate_); } else { // Found on object. Handle object = Handle::cast(holder); - // Do not shadow properties on the global object. - *global = object->IsJSGlobalObject(); return JSReceiver::GetDataProperty(object, name); } } @@ -340,13 +333,7 @@ void DebugEvaluate::ContextBuilder::MaterializeContextChain( for (const Handle& name : non_locals_) { HandleScope scope(isolate_); Handle value; - 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; - } + if (!LoadFromContext(context, name).ToHandle(&value)) continue; JSObject::SetOwnPropertyIgnoreAttributes(target, name, value, NONE).Check(); } } @@ -394,8 +381,7 @@ Handle DebugEvaluate::ContextBuilder::MaterializeReceiver( Handle receiver = isolate_->factory()->undefined_value(); Handle this_string = isolate_->factory()->this_string(); if (this_is_non_local) { - bool global; - LoadFromContext(lookup_context, this_string, &global).ToHandle(&receiver); + LoadFromContext(lookup_context, this_string).ToHandle(&receiver); } else if (local_function->shared()->scope_info()->HasReceiver()) { receiver = handle(frame_->receiver(), isolate_); } diff --git a/src/debug/debug-evaluate.h b/src/debug/debug-evaluate.h index c0b1f027d1..178e237881 100644 --- a/src/debug/debug-evaluate.h +++ b/src/debug/debug-evaluate.h @@ -54,7 +54,6 @@ class DebugEvaluate : public AllStatic { void UpdateValues(); Handle innermost_context() const { return innermost_context_; } - Handle native_context() const { return native_context_; } Handle outer_info() const { return outer_info_; } private: @@ -88,14 +87,13 @@ class DebugEvaluate : public AllStatic { bool this_is_non_local); MaybeHandle LoadFromContext(Handle context, - Handle name, bool* global); + Handle name); void StoreToContext(Handle context, Handle name, Handle value); Handle outer_info_; Handle innermost_context_; - Handle native_context_; List context_chain_; List > non_locals_; Isolate* isolate_; diff --git a/src/flag-definitions.h b/src/flag-definitions.h index b42849e354..b98b995261 100644 --- a/src/flag-definitions.h +++ b/src/flag-definitions.h @@ -597,7 +597,8 @@ DEFINE_BOOL(cache_prototype_transitions, true, "cache prototype transitions") DEFINE_INT(cpu_profiler_sampling_interval, 1000, "CPU profiler sampling interval in microseconds") -// Array abuse tracing +// debug.cc +DEFINE_BOOL(trace_debug_json, false, "trace debugging JSON request/response") DEFINE_BOOL(trace_js_array_abuse, false, "trace out-of-bounds accesses to JS arrays") DEFINE_BOOL(trace_external_array_abuse, false, @@ -606,11 +607,6 @@ 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") diff --git a/test/mjsunit/debug-evaluate-closure.js b/test/mjsunit/debug-evaluate-closure.js index 541dec9d6d..7e8fb91da7 100644 --- a/test/mjsunit/debug-evaluate-closure.js +++ b/test/mjsunit/debug-evaluate-closure.js @@ -26,7 +26,6 @@ // 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; @@ -35,18 +34,20 @@ 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'"); // no effect + exec_state.frame(0).evaluate("goo = 'goo foo'"); assertEquals("bar return", exec_state.frame(0).evaluate("bar()").value()); - assertEquals("inner bar", exec_state.frame(0).evaluate("inner").value()); - assertEquals("outer bar", exec_state.frame(0).evaluate("outer").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("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'"); // 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 + 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'"); listened = true; } catch (e) { print(e); @@ -68,9 +69,9 @@ function foo() { with (withv) { var bar = function bar() { - assertEquals("goo", goo); - inner = "inner bar"; - outer = "outer 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 v = "v bar"; return "bar return"; }; @@ -80,8 +81,8 @@ function foo() { debugger; } - assertEquals("inner bar", inner); - assertEquals("baz inner", baz); + assertEquals("inner foo", inner); + assertEquals("baz inner foo", baz); assertEquals("w foo", withw.w); assertEquals("v bar", withv.v); } diff --git a/test/mjsunit/debug-evaluate-const.js b/test/mjsunit/debug-evaluate-const.js index 08e2f1369b..7fad483cd5 100644 --- a/test/mjsunit/debug-evaluate-const.js +++ b/test/mjsunit/debug-evaluate-const.js @@ -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 --debug-eval-readonly-locals +// Flags: --expose-debug-as debug Debug = debug.Debug @@ -50,8 +50,10 @@ function f() { debugger; // Break point. - assertEquals(undefined, var0); - assertEquals(0, const0); + assertEquals(30, var0); + // TODO(yangguo): debug evaluate should not be able to alter + // stack-allocated const values + // assertEquals(0, const0); assertEquals(undefined, const1); assertEquals(undefined, const2); var var0 = 20; @@ -64,7 +66,7 @@ function f() { g(); - assertEquals(21, var1); + assertEquals(31, var1); assertEquals(3, const3); } diff --git a/test/mjsunit/debug-evaluate-locals-capturing.js b/test/mjsunit/debug-evaluate-locals-capturing.js index 6d65861fc7..77bdfc6294 100644 --- a/test/mjsunit/debug-evaluate-locals-capturing.js +++ b/test/mjsunit/debug-evaluate-locals-capturing.js @@ -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 --debug-eval-readonly-locals +// Flags: --expose-debug-as debug 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); - var i = frameMirror.evaluate('f = function() { i = 5; }, f(), i').value(); - assertEquals(5, i); + f = frameMirror.evaluate('f = function() { i = 5; }, f(), f').value(); + print(f); } } catch(e) { exception = e; @@ -35,7 +35,7 @@ Debug.setListener(listener); } catch (e) { assertEquals(0, i); debugger; - assertEquals(0, i); + assertEquals(5, i); } }()); diff --git a/test/mjsunit/debug-evaluate-locals.js b/test/mjsunit/debug-evaluate-locals.js index 642e0c0682..71d4534509 100644 --- a/test/mjsunit/debug-evaluate-locals.js +++ b/test/mjsunit/debug-evaluate-locals.js @@ -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 --debug-eval-readonly-locals +// Flags: --expose-debug-as debug // Get the Debug object exposed from the debug context global object. Debug = debug.Debug @@ -127,7 +127,6 @@ 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", @@ -146,7 +145,7 @@ Debug.setListener(listener); var f_result = f(); -assertEquals(4, f_result); +assertEquals('foobar', f_result); // Make sure that the debug event listener was invoked. assertFalse(exception, "exception in listener") diff --git a/test/mjsunit/debug-evaluate-modify-catch-block-scope.js b/test/mjsunit/debug-evaluate-modify-catch-block-scope.js index 07d6ccbe6f..63e30b7cf0 100644 --- a/test/mjsunit/debug-evaluate-modify-catch-block-scope.js +++ b/test/mjsunit/debug-evaluate-modify-catch-block-scope.js @@ -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 --debug-eval-readonly-locals +// Flags: --expose-debug-as debug Debug = debug.Debug @@ -26,12 +26,12 @@ Debug.setListener(listener); } catch (e) { let a = 1; function bar() { - a = 2; - e = 2; + a = 2; // this has no effect, when called from debug-eval + e = 2; // this has no effect, when called from debug-eval. } debugger; - assertEquals(2, a); - assertEquals(2, e); + assertEquals(1, a); + assertEquals(1, e); } })(); diff --git a/test/mjsunit/debug-evaluate-shadowed-context.js b/test/mjsunit/debug-evaluate-shadowed-context.js index 6847a93f66..221efb39d0 100644 --- a/test/mjsunit/debug-evaluate-shadowed-context.js +++ b/test/mjsunit/debug-evaluate-shadowed-context.js @@ -29,8 +29,7 @@ function listener(event, exec_state, event_data, data) { } assertEquals("[object global]", String(exec_state.frame(0).evaluate("this").value())); - assertEquals("y", exec_state.frame(0).evaluate("y").value()); - assertEquals("a", exec_state.frame(0).evaluate("a").value()); + exec_state.frame(0).evaluate("y = 'Y'"); exec_state.frame(0).evaluate("a = 'A'"); assertThrows(() => exec_state.frame(0).evaluate("z"), ReferenceError); } catch (e) { @@ -42,7 +41,7 @@ function listener(event, exec_state, event_data, data) { Debug.setListener(listener); var a = "a"; -(function() { +assertEquals("Y", (function() { var x = 1; // context allocate x (() => x); var y = "y"; @@ -55,12 +54,12 @@ var a = "a"; })(); // 2 })(); // 1 return y; -})(); +})()); assertEquals("A", a); a = "a"; -(function() { +assertEquals("Y", (function() { var x = 1; // context allocate x (() => x); var y = "y"; @@ -75,7 +74,7 @@ a = "a"; })(); // 2 })(); // 1 return y; -})(); +})()); assertEquals("A", a); diff --git a/test/mjsunit/es6/debug-blockscopes.js b/test/mjsunit/es6/debug-blockscopes.js index d3c36207f1..3f890ebd54 100644 --- a/test/mjsunit/es6/debug-blockscopes.js +++ b/test/mjsunit/es6/debug-blockscopes.js @@ -26,7 +26,6 @@ // 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. @@ -526,12 +525,15 @@ 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) { - assertEqualsUnlessOptimized(5, exec_state.frame(0).evaluate("i").value()); + exec_state.frame(0).evaluate("i = 27"); } shadowing_1(); EndTest(); @@ -544,12 +546,13 @@ function shadowing_2() { { let j = 5; debugger; + assertEqualsUnlessOptimized(27, j, shadowing_2); } + assertEqualsUnlessOptimized(0, i, shadowing_2); } listener_delegate = function (exec_state) { - assertEqualsUnlessOptimized(0, exec_state.frame(0).evaluate("i").value()); - assertEqualsUnlessOptimized(5, exec_state.frame(0).evaluate("j").value()); + exec_state.frame(0).evaluate("j = 27"); } shadowing_2(); EndTest(); diff --git a/test/mjsunit/regress-3225.js b/test/mjsunit/regress-3225.js index 97165a80dd..fe44b85110 100644 --- a/test/mjsunit/regress-3225.js +++ b/test/mjsunit/regress-3225.js @@ -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 --debug-eval-readonly-locals +// Flags: --expose-debug-as debug Debug = debug.Debug @@ -15,13 +15,12 @@ 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(); // no effect. + exec_state.frame(0).evaluate("a = 4").value(); debug_step++; } else { - assertEquals(1, exec_state.frame(0).evaluate('a').value()); + assertEquals(4, exec_state.frame(0).evaluate('a').value()); assertEquals(3, exec_state.frame(0).evaluate('b').value()); - exec_state.frame(0).evaluate("set_a_to_5()"); - exec_state.frame(0).evaluate("b = 5").value(); // no effect. + exec_state.frame(0).evaluate("b = 5").value(); } } catch (e) { failure = e; @@ -31,22 +30,19 @@ 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(1, foo.next().value); +assertEquals(4, foo.next().value); assertEquals(3, foo.next().value); assertEquals(5, foo.next().value); -assertEquals(3, foo.next().value); assertNull(failure); Debug.setListener(null); diff --git a/test/mjsunit/regress/regress-325676.js b/test/mjsunit/regress/regress-325676.js index 7aae0cdaab..427bbc38dc 100644 --- a/test/mjsunit/regress/regress-325676.js +++ b/test/mjsunit/regress/regress-325676.js @@ -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 --debug-eval-readonly-locals +// Flags: --expose-debug-as debug // 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";'); // no effect + exec_state.frame(0).evaluate('arg = "evaluated";'); } catch (e) { exception = e; } @@ -51,12 +51,12 @@ Debug.setListener(listener); function f(arg) { expected = arg; debugger; - assertEquals(expected, arg); + assertEquals("evaluated", arg); arg = "value"; expected = arg; debugger; - assertEquals(expected, arg); + assertEquals("evaluated", arg); // Forces arg to be context allocated even though a parameter. function g() { arg; } diff --git a/test/mjsunit/regress/regress-crbug-323936.js b/test/mjsunit/regress/regress-crbug-323936.js index c1d0f7d931..d896eadcc4 100644 --- a/test/mjsunit/regress/regress-crbug-323936.js +++ b/test/mjsunit/regress/regress-crbug-323936.js @@ -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 --debug-eval-readonly-locals +// Flags: --expose-debug-as debug 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("write_0('foo')"); - exec_state.frame(0).evaluate("write_1('modified')"); + exec_state.frame(0).evaluate("e = 'foo'"); + exec_state.frame(0).evaluate("x = 'modified'"); } else { - assertEquals("foo", exec_state.frame(0).evaluate("e").value()); - exec_state.frame(0).evaluate("write_2('bar')"); + assertEquals("argument", exec_state.frame(0).evaluate("e").value()); + exec_state.frame(0).evaluate("e = 'bar'"); } step++; } catch (e) { @@ -33,13 +33,9 @@ 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("error", e); + assertEquals("foo", e); } - function write_2(v) { e = v } debugger; assertEquals("bar", e); assertEquals("modified", x);