[debugger] allow debug-evaluate to change stack and context values.

src/debug/debug-evaluate.cc

Index: src/debug/debug-evaluate.cc
diff --git a/src/debug/debug-evaluate.cc b/src/debug/debug-evaluate.cc
index 1134c9dd68ae9cfec97deb3a1bed09c7663a6d61..dceaee14261301584ff8e8818e5bece407f33e25 100644
--- a/src/debug/debug-evaluate.cc
+++ b/src/debug/debug-evaluate.cc
@@ -73,14 +73,12 @@ MaybeHandle<Object> DebugEvaluate::Local(Isolate* isolate,
   ContextBuilder context_builder(isolate, frame, inlined_jsframe_index);
   if (isolate->has_pending_exception()) return MaybeHandle<Object>();
 
-  Handle<Context> context = context_builder.native_context();
+  Handle<Context> context = context_builder.evaluation_context();
   Handle<JSObject> receiver(context->global_proxy());
-  MaybeHandle<Object> 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();
-  }
+  MaybeHandle<Object> maybe_result =
+      Evaluate(isolate, context_builder.outer_info(), context,
+               context_extension, receiver, source);
+  if (!maybe_result.is_null()) context_builder.UpdateValues();
   return maybe_result;
 }
 
@@ -130,113 +128,81 @@ DebugEvaluate::ContextBuilder::ContextBuilder(Isolate* isolate,
   Handle<JSFunction> local_function =
       Handle<JSFunction>::cast(frame_inspector.GetFunction());
   Handle<Context> outer_context(local_function->context());
-  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;
-
-  bool stop = false;
-
-  // Iterate the original context chain to create a context chain that reflects
-  // our needs. The original context chain may look like this:
-  // <native context> <outer contexts> <function context> <inner contexts>
-  // In the resulting context chain, we want to materialize the receiver,
-  // the parameters of the current function, the stack locals. We only
-  // materialize context variables that the function already references,
-  // because only for those variables we can be sure that they will be resolved
-  // correctly. Variables that are not referenced by the function may be
-  // context-allocated and thus accessible, but may be shadowed by stack-
-  // allocated variables and the resolution would be incorrect.
-  // The result will look like this:
-  // <native context> <receiver context>
-  //     <materialized stack and accessible context vars> <inner contexts>
-  // All contexts use the closure of the native context, since there is no
-  // function context in the chain. Variables that cannot be resolved are
-  // bound to toplevel (script contexts or global object).
-  // Once debug-evaluate has been executed, the changes to the materialized
-  // objects are written back to the original context chain. Any changes to
-  // the original context chain will therefore be overwritten.
+  evaluation_context_ = outer_context;
+  outer_info_ = handle(local_function->shared());
+  Factory* factory = isolate->factory();
+
+  // To evaluate as if we were running eval at the point of the debug break,
+  // we reconstruct the context chain as follows:
+  //  - To make stack-allocated variables visible, we materialize them and
+  //    use a debug-evaluate context to wrap both the materialized object and
+  //    the original context.
+  //  - We use the original context chain from the function context to the
+  //    native context.
+  //  - Between the function scope and the native context, we only resolve
+  //    variable names that the current function already uses. Only for these
+  //    names we can be sure that they will be correctly resolved. For the
+  //    rest, we only resolve to with, script, and native contexts. We use a
+  //    whitelist to implement that.
+  // Context::Lookup has special handling for debug-evaluate contexts:
+  //  - Look up in the materialized stack variables.
+  //  - Look up in the original context.
+  //  - Check the whitelist to find out whether to skip contexts during lookup.
   const ScopeIterator::Option option = ScopeIterator::COLLECT_NON_LOCALS;
   for (ScopeIterator it(isolate, &frame_inspector, option);
-       !it.Failed() && !it.Done() && !stop; it.Next()) {
+       !it.Failed() && !it.Done(); it.Next()) {
     ScopeIterator::ScopeType scope_type = it.Type();
     if (scope_type == ScopeIterator::ScopeTypeLocal) {
       DCHECK_EQ(FUNCTION_SCOPE, it.CurrentScopeInfo()->scope_type());
-      it.GetNonLocals(&non_locals_);
+      Handle<JSObject> materialized = NewJSObjectWithNullProto();
       Handle<Context> local_context =
           it.HasContext() ? it.CurrentContext() : outer_context;
-
-      // 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,
-                              global_function, it.ThisIsNonLocal());
-
-      Handle<JSObject> materialized_function = NewJSObjectWithNullProto();
-      frame_inspector.MaterializeStackLocals(materialized_function,
-                                             local_function);
-      MaterializeArgumentsObject(materialized_function, local_function);
-      MaterializeContextChain(materialized_function, local_context);
-
-      Handle<Context> with_context = isolate->factory()->NewWithContext(
-          global_function, receiver_context, materialized_function);
-
+      Handle<StringSet> non_locals = it.GetNonLocals();
+      MaterializeReceiver(materialized, local_context, local_function,
+                          non_locals);
+      frame_inspector.MaterializeStackLocals(materialized, local_function);
+      MaterializeArgumentsObject(materialized, local_function);
       ContextChainElement context_chain_element;
-      context_chain_element.original_context = local_context;
-      context_chain_element.materialized_object = materialized_function;
       context_chain_element.scope_info = it.CurrentScopeInfo();
+      context_chain_element.materialized_object = materialized;
+      // Non-locals that are already being referenced by the current function
+      // are guaranteed to be correctly resolved.
+      context_chain_element.whitelist = non_locals;
+      if (it.HasContext()) {
+        context_chain_element.wrapped_context = it.CurrentContext();
+      }
       context_chain_.Add(context_chain_element);
-
-      stop = true;
-      RecordContextsInChain(&inner_context, receiver_context, with_context);
+      evaluation_context_ = outer_context;
+      break;
     } else if (scope_type == ScopeIterator::ScopeTypeCatch ||
                scope_type == ScopeIterator::ScopeTypeWith) {
-      Handle<Context> cloned_context = Handle<Context>::cast(
-          isolate->factory()->CopyFixedArray(it.CurrentContext()));
-
       ContextChainElement context_chain_element;
-      context_chain_element.original_context = it.CurrentContext();
-      context_chain_element.cloned_context = cloned_context;
+      Handle<Context> current_context = it.CurrentContext();
+      if (!current_context->IsDebugEvaluateContext()) {
+        context_chain_element.wrapped_context = current_context;
+      }
       context_chain_.Add(context_chain_element);
-
-      RecordContextsInChain(&inner_context, cloned_context, cloned_context);
     } else if (scope_type == ScopeIterator::ScopeTypeBlock) {
-      Handle<JSObject> materialized_object = NewJSObjectWithNullProto();
-      frame_inspector.MaterializeStackLocals(materialized_object,
+      Handle<JSObject> materialized = NewJSObjectWithNullProto();
+      frame_inspector.MaterializeStackLocals(materialized,
                                              it.CurrentScopeInfo());
+      ContextChainElement context_chain_element;
+      context_chain_element.scope_info = it.CurrentScopeInfo();
+      context_chain_element.materialized_object = materialized;
       if (it.HasContext()) {
-        Handle<Context> cloned_context = Handle<Context>::cast(
-            isolate->factory()->CopyFixedArray(it.CurrentContext()));
-        Handle<Context> with_context = isolate->factory()->NewWithContext(
-            global_function, cloned_context, materialized_object);
-
-        ContextChainElement context_chain_element;
-        context_chain_element.original_context = it.CurrentContext();
-        context_chain_element.cloned_context = cloned_context;
-        context_chain_element.materialized_object = materialized_object;
-        context_chain_element.scope_info = it.CurrentScopeInfo();
-        context_chain_.Add(context_chain_element);
-
-        RecordContextsInChain(&inner_context, cloned_context, with_context);
-      } else {
-        Handle<Context> with_context = isolate->factory()->NewWithContext(
-            global_function, outer_context, materialized_object);
-
-        ContextChainElement context_chain_element;
-        context_chain_element.materialized_object = materialized_object;
-        context_chain_element.scope_info = it.CurrentScopeInfo();
-        context_chain_.Add(context_chain_element);
-
-        RecordContextsInChain(&inner_context, with_context, with_context);
+        context_chain_element.wrapped_context = it.CurrentContext();
       }
+      context_chain_.Add(context_chain_element);
     } else {
-      stop = true;
+      break;
     }
   }
-  if (innermost_context_.is_null()) {
-    innermost_context_ = outer_context;
+
+  for (int i = context_chain_.length() - 1; i >= 0; i--) {
+    evaluation_context_ = factory->NewDebugEvaluateContext(
+        evaluation_context_, context_chain_[i].materialized_object,
+        context_chain_[i].wrapped_context, context_chain_[i].whitelist);
   }
-  DCHECK(!innermost_context_.is_null());
 }
 
 
@@ -244,25 +210,11 @@ void DebugEvaluate::ContextBuilder::UpdateValues() {
   // TODO(yangguo): remove updating values.
   for (int i = 0; i < context_chain_.length(); i++) {
     ContextChainElement element = context_chain_[i];
-    if (!element.original_context.is_null() &&
-        !element.cloned_context.is_null()) {
-      Handle<Context> cloned_context = element.cloned_context;
-      cloned_context->CopyTo(
-          Context::MIN_CONTEXT_SLOTS, *element.original_context,
-          Context::MIN_CONTEXT_SLOTS,
-          cloned_context->length() - Context::MIN_CONTEXT_SLOTS);
-    }
     if (!element.materialized_object.is_null()) {
-      // Write back potential changes to materialized stack locals to the
-      // stack.
+      // Write back potential changes to materialized stack locals to the stack.
       FrameInspector(frame_, inlined_jsframe_index_, isolate_)
           .UpdateStackLocalsFromMaterializedObject(element.materialized_object,
                                                    element.scope_info);
-      if (element.scope_info->scope_type() == FUNCTION_SCOPE) {
-        DCHECK_EQ(context_chain_.length() - 1, i);
-        UpdateContextChainFromMaterializedObject(element.materialized_object,
-                                                 element.original_context);
-      }
     }
   }
 }
@@ -279,18 +231,6 @@ Handle<JSObject> DebugEvaluate::ContextBuilder::NewJSObjectWithNullProto() {
 }
 
 
-void DebugEvaluate::ContextBuilder::RecordContextsInChain(
-    Handle<Context>* inner_context, Handle<Context> first,
-    Handle<Context> last) {
-  if (!inner_context->is_null()) {
-    (*inner_context)->set_previous(*last);
-  } else {
-    innermost_context_ = last;
-  }
-  *inner_context = first;
-}
-
-
 void DebugEvaluate::ContextBuilder::MaterializeArgumentsObject(
     Handle<JSObject> target, Handle<JSFunction> function) {
   // Do not materialize the arguments object for eval or top-level code.
@@ -309,97 +249,19 @@ void DebugEvaluate::ContextBuilder::MaterializeArgumentsObject(
       .Check();
 }
 
-
-MaybeHandle<Object> DebugEvaluate::ContextBuilder::LoadFromContext(
-    Handle<Context> context, Handle<String> name, bool* global) {
-  static const ContextLookupFlags flags = FOLLOW_CONTEXT_CHAIN;
-  int index;
-  PropertyAttributes attributes;
-  BindingFlags binding;
-  Handle<Object> holder =
-      context->Lookup(name, flags, &index, &attributes, &binding);
-  if (holder.is_null()) return MaybeHandle<Object>();
-  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);
-  }
-}
-
-
-void DebugEvaluate::ContextBuilder::MaterializeContextChain(
-    Handle<JSObject> target, Handle<Context> context) {
-  for (const Handle<String>& name : non_locals_) {
-    HandleScope scope(isolate_);
-    Handle<Object> 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;
-    }
-    JSObject::SetOwnPropertyIgnoreAttributes(target, name, value, NONE).Check();
-  }
-}
-
-
-void DebugEvaluate::ContextBuilder::StoreToContext(Handle<Context> context,
-                                                   Handle<String> name,
-                                                   Handle<Object> value) {
-  static const ContextLookupFlags flags = FOLLOW_CONTEXT_CHAIN;
-  int index;
-  PropertyAttributes attributes;
-  BindingFlags binding;
-  Handle<Object> holder =
-      context->Lookup(name, flags, &index, &attributes, &binding);
-  if (holder.is_null()) return;
-  if (attributes & READ_ONLY) return;
-  if (index != Context::kNotFound) {  // Found on context.
-    Handle<Context> context = Handle<Context>::cast(holder);
-    context->set(index, *value);
-  } else {  // Found on object.
-    Handle<JSReceiver> object = Handle<JSReceiver>::cast(holder);
-    LookupIterator lookup(object, name);
-    if (lookup.state() != LookupIterator::DATA) return;
-    CHECK(JSReceiver::SetDataProperty(&lookup, value).FromJust());
-  }
-}
-
-
-void DebugEvaluate::ContextBuilder::UpdateContextChainFromMaterializedObject(
-    Handle<JSObject> source, Handle<Context> context) {
-  // TODO(yangguo): check whether overwriting context fields is actually safe
-  //                wrt fields we consider constant.
-  for (const Handle<String>& name : non_locals_) {
-    HandleScope scope(isolate_);
-    Handle<Object> value = JSReceiver::GetDataProperty(source, name);
-    StoreToContext(context, name, value);
-  }
-}
-
-
-Handle<Context> DebugEvaluate::ContextBuilder::MaterializeReceiver(
-    Handle<Context> parent_context, Handle<Context> lookup_context,
-    Handle<JSFunction> local_function, Handle<JSFunction> global_function,
-    bool this_is_non_local) {
-  Handle<Object> receiver = isolate_->factory()->undefined_value();
-  Handle<String> this_string = isolate_->factory()->this_string();
-  if (this_is_non_local) {
-    bool global;
-    LoadFromContext(lookup_context, this_string, &global).ToHandle(&receiver);
+void DebugEvaluate::ContextBuilder::MaterializeReceiver(
+    Handle<JSObject> target, Handle<Context> local_context,
+    Handle<JSFunction> local_function, Handle<StringSet> non_locals) {
+  Handle<Object> recv = isolate_->factory()->undefined_value();
+  Handle<String> name = isolate_->factory()->this_string();
+  if (non_locals->Has(name)) {
+    // 'this' is allocated in an outer context and is is already being
+    // referenced by the current function, so it can be correctly resolved.
+    return;
   } else if (local_function->shared()->scope_info()->HasReceiver()) {
-    receiver = handle(frame_->receiver(), isolate_);
+    recv = handle(frame_->receiver(), isolate_);
   }
-  return isolate_->factory()->NewCatchContext(global_function, parent_context,
-                                              this_string, receiver);
+  JSObject::SetOwnPropertyIgnoreAttributes(target, name, recv, NONE).Check();
 }
 
 }  // namespace internal