Reviewers: aandrey, ulan,
Description:
Insert materialized context at the right place in DebugEvaluate.
[email protected], [email protected]
BUG=chromium:323936
LOG=N
Please review this at https://codereview.chromium.org/599113002/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+88, -9 lines):
M src/runtime.cc
A test/mjsunit/regress/regress-crbug-323936.js
Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index
5209de4684be1bb697cdd989b2acf68035dfcf31..e2695051d5c61462e4e9ad95a7882d63ab12f0e8
100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -13055,10 +13055,6 @@ RUNTIME_FUNCTION(Runtime_DebugEvaluate) {
SaveContext savex(isolate);
isolate->set_context(*(save->context()));
- // Evaluate on the context of the frame.
- Handle<Context> context(Context::cast(frame_inspector.GetContext()));
- DCHECK(!context.is_null());
-
// Materialize stack locals and the arguments object.
Handle<JSObject> materialized = NewJSObjectWithNullProto(isolate);
@@ -13071,14 +13067,53 @@ RUNTIME_FUNCTION(Runtime_DebugEvaluate) {
isolate, materialized,
MaterializeArgumentsObject(isolate, materialized, function));
- // Add the materialized object in a with-scope to shadow the stack
locals.
- context = isolate->factory()->NewWithContext(function, context,
materialized);
+ // At this point, the lookup chain may look like this:
+ // [inner context] -> [function stack]+[function context] -> [outer
context]
+ // The function stack is not an actual context, it complements the
function
+ // context. In order to have the same lookup chain when debug-evaluating,
+ // we materialize the stack and insert it into the context chain as a
+ // with-context before the function context.
+ // [inner context] -> [with context] -> [function context] -> [outer
context]
+ // Ordering the with-context before the function context forces a dynamic
+ // lookup instead of a static lookup that could fail as the scope info is
+ // outdated and may expect variables to still be stack-allocated.
+ // Afterwards, we write changes to the with-context back to the stack
+ // and remove it from the context chain.
+
+ Handle<Context>
eval_context(Context::cast(frame_inspector.GetContext()));
+ DCHECK(!eval_context.is_null());
+ Handle<Context> function_context = eval_context;
+ Handle<Context> outer_context(function->context(), isolate);
+ Handle<Context> inner_context;
+ // We iterate to find the function's context. If the function has no
+ // context-allocated variables, we iterate until we hit the outer
context.
+ while (!function_context->IsFunctionContext() &&
+ !function_context.is_identical_to(outer_context)) {
+ inner_context = function_context;
+ function_context = Handle<Context>(function_context->previous(),
isolate);
+ }
+
+ Handle<Context> materialized_context =
isolate->factory()->NewWithContext(
+ function, function_context, materialized);
+
+ if (inner_context.is_null()) {
+ // No inner context. The with-context is now inner-most.
+ eval_context = materialized_context;
+ } else {
+ inner_context->set_previous(*materialized_context);
+ }
Handle<Object> receiver(frame->receiver(), isolate);
+ MaybeHandle<Object> maybe_result =
+ DebugEvaluate(isolate, eval_context, context_extension, receiver,
source);
+
+ // Remove with-context if it was inserted in between.
+ if (!inner_context.is_null()) {
+ inner_context->set_previous(materialized_context->previous());
+ }
+
Handle<Object> result;
- ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
- isolate, result,
- DebugEvaluate(isolate, context, context_extension, receiver,
source));
+ ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, result, maybe_result);
// Write back potential changes to materialized stack locals to the
stack.
UpdateStackLocalsFromMaterializedObject(
Index: test/mjsunit/regress/regress-crbug-323936.js
diff --git a/test/mjsunit/regress/regress-crbug-323936.js
b/test/mjsunit/regress/regress-crbug-323936.js
new file mode 100644
index
0000000000000000000000000000000000000000..0695e86bfa010acd62316ae3748b589edcb894bc
--- /dev/null
+++ b/test/mjsunit/regress/regress-crbug-323936.js
@@ -0,0 +1,44 @@
+// Copyright 2014 the V8 project authors. All rights reserved.
+// 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 = debug.Debug;
+
+var step = 0;
+var exception = null;
+
+function listener(event, exec_state, event_data, data) {
+ if (event != Debug.DebugEvent.Break) return;
+ try {
+ if (step == 0) {
+ assertEquals("Error", exec_state.frame(0).evaluate("e").value());
+ exec_state.frame(0).evaluate("e = 'foo'");
+ } else {
+ assertEquals("Argument", exec_state.frame(0).evaluate("e").value());
+ exec_state.frame(0).evaluate("e = 'bar'");
+ }
+ step++;
+ } catch (e) {
+ print(e + e.stack);
+ exception = e;
+ }
+}
+
+Debug.setListener(listener);
+
+function f(e) {
+ try {
+ throw "Error";
+ } catch(e) {
+ debugger;
+ assertEquals("foo", e);
+ }
+ debugger;
+ assertEquals("bar", e);
+}
+
+f("Argument")
+assertNull(exception);
+assertEquals(2, step);
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
