Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1795)

Unified Diff: src/runtime/runtime-debug.cc

Issue 1088503003: Avoid modifying the real context chain for debug evaluation. (Closed) Base URL: https://chromium.googlesource.com/v8/v8.git@master
Patch Set: Minor fixes Created 5 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | test/mjsunit/debug-evaluate-locals-capturing.js » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/runtime/runtime-debug.cc
diff --git a/src/runtime/runtime-debug.cc b/src/runtime/runtime-debug.cc
index d1e857add81241d44434d1e55b2b701458a3aac2..736d33dadce30cdfcc9571eb4b50dc13d6ad3c73 100644
--- a/src/runtime/runtime-debug.cc
+++ b/src/runtime/runtime-debug.cc
@@ -2253,27 +2253,38 @@ RUNTIME_FUNCTION(Runtime_DebugEvaluate) {
// [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]
+ // we:
+ // - clone inner context
+ // - materialize the stack and insert it into the context chain as a
+ // with-context before the function context.
+ // [inner context clone] -> [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.
- // This could cause lookup failures if debug-evaluate creates a closure that
- // uses this temporary context chain.
+ // Afterwards, we write changes to the with-context back to the stack, and
+ // write changes in cloned contexts back to original contexts.
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.
+ Handle<Context> innermost_context;
+
+ // We iterate to find the function's context, cloning until we hit it.
+ // If the function has no context-allocated variables, we iterate until
+ // we hit the outer context.
while (!function_context->IsFunctionContext() &&
!function_context->IsScriptContext() &&
!function_context.is_identical_to(outer_context)) {
- inner_context = function_context;
+ Handle<Context> clone = Handle<Context>::cast(
+ FixedArray::CopySize(function_context, function_context->length()));
+ if (!inner_context.is_null()) {
+ inner_context->set_previous(*clone);
+ } else {
+ innermost_context = clone;
+ }
+ inner_context = clone;
function_context = Handle<Context>(function_context->previous(), isolate);
}
@@ -2282,17 +2293,15 @@ RUNTIME_FUNCTION(Runtime_DebugEvaluate) {
if (inner_context.is_null()) {
// No inner context. The with-context is now inner-most.
- eval_context = materialized_context;
+ innermost_context = materialized_context;
} else {
inner_context->set_previous(*materialized_context);
}
Handle<Object> receiver(frame->receiver(), isolate);
- MaybeHandle<Object> maybe_result = DebugEvaluate(
- isolate, outer_info, eval_context, context_extension, receiver, source);
-
- // Remove with-context if it was inserted in between.
- if (!inner_context.is_null()) inner_context->set_previous(*function_context);
+ MaybeHandle<Object> maybe_result =
+ DebugEvaluate(isolate, outer_info, innermost_context, context_extension,
+ receiver, source);
Handle<Object> result;
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, result, maybe_result);
@@ -2301,6 +2310,15 @@ RUNTIME_FUNCTION(Runtime_DebugEvaluate) {
UpdateStackLocalsFromMaterializedObject(isolate, materialized, function,
frame, inlined_jsframe_index);
+ while (!innermost_context.is_identical_to(materialized_context)) {
+ DCHECK(eval_context->map() == innermost_context->map());
+ innermost_context->CopyTo(
+ Context::MIN_CONTEXT_SLOTS, *eval_context, Context::MIN_CONTEXT_SLOTS,
+ innermost_context->length() - Context::MIN_CONTEXT_SLOTS);
+ innermost_context = handle(innermost_context->previous(), isolate);
+ eval_context = handle(eval_context->previous(), isolate);
+ }
+
return *result;
}
« no previous file with comments | « no previous file | test/mjsunit/debug-evaluate-locals-capturing.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698