[debugger] fix debug-evaluate wrt shadowed context var.

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 65bbc3fe50e134c06c2917e0014dc1afd8a80989..44ce86ad63edff140394095506068c541c831c9a 100644
--- a/src/debug/debug-evaluate.cc
+++ b/src/debug/debug-evaluate.cc
@@ -112,6 +112,16 @@ MaybeHandle<Object> DebugEvaluate::Evaluate(
 }
 
 
+Handle<Context> FindScriptOrNativeContext(Handle<Context> context) {
+  DisallowHeapAllocation no_gc;
+  Context* raw_context = *context;
+  while (!raw_context->IsScriptContext() && !raw_context->IsNativeContext()) {
+    raw_context = raw_context->previous();
+  }
+  return Handle<Context>(raw_context);
+}
+
+
 DebugEvaluate::ContextBuilder::ContextBuilder(Isolate* isolate,
                                               JavaScriptFrame* frame,
                                               int inlined_jsframe_index)
@@ -126,35 +136,57 @@ DebugEvaluate::ContextBuilder::ContextBuilder(Isolate* isolate,
   Handle<Context> inner_context;
 
   bool stop = false;
-  for (ScopeIterator it(isolate, &frame_inspector);
+  const ScopeIterator::Option option = ScopeIterator::COLLECT_NON_LOCALS;
+  for (ScopeIterator it(isolate, &frame_inspector, option);
        !it.Failed() && !it.Done() && !stop; it.Next()) {
     ScopeIterator::ScopeType scope_type = it.Type();
 
+    // Duplicate contexts up to the script context for debug evaluate.
+    // The content of the duplicated contexts are written back to the original
+    // at the end. This however means that any changes to the original will be
+    // overwritten.
     if (scope_type == ScopeIterator::ScopeTypeLocal) {
-      Handle<Context> parent_context =
+      // The context chain at this point looks like this:
+      // <native context> <script context> <outer contexts>
+      //     <function context> <inner contexts>
+      // We can only be sure to correctly resolve variables that our function
+      // itself already references. Other variables may be stack-allocated,
+      // and invisible to the context lookup.
+      // To make sure that other variables are not accessible, we collect the
+      // variables used by our function and replace the context chain between
+      // the current function and the script context by a with-context that
+      // materializes accessible context vars and stack vars. We also add a
+      // catch-context to resolve 'this' if necessary.
+      // After the replacement, it looks like this:
+      // <native context> <script context> <receiver context>
+      //     <accessible context vars and materialized stack> <inner contexts>
+      DCHECK_EQ(FUNCTION_SCOPE, it.CurrentScopeInfo()->scope_type());
+      it.GetNonLocals(&non_locals_);
+      Handle<Context> base_context = FindScriptOrNativeContext(outer_context);
+      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".
-      parent_context = MaterializeReceiver(parent_context, function);
+      Handle<Context> receiver_context = MaterializeReceiver(
+          base_context, local_context, function, it.ThisIsNonLocal());
 
       Handle<JSObject> materialized_function = NewJSObjectWithNullProto();
-
       frame_inspector.MaterializeStackLocals(materialized_function, function);
-
       MaterializeArgumentsObject(materialized_function, function);
+      MaterializeContextChain(materialized_function, local_context);
 
       Handle<Context> with_context = isolate->factory()->NewWithContext(
-          function, parent_context, materialized_function);
+          function, receiver_context, materialized_function);
 
       ContextChainElement context_chain_element;
-      context_chain_element.original_context = it.CurrentContext();
+      context_chain_element.original_context = local_context;
       context_chain_element.materialized_object = materialized_function;
       context_chain_element.scope_info = it.CurrentScopeInfo();
       context_chain_.Add(context_chain_element);
 
       stop = true;
-      RecordContextsInChain(&inner_context, with_context, with_context);
+      RecordContextsInChain(&inner_context, receiver_context, with_context);
     } else if (scope_type == ScopeIterator::ScopeTypeCatch ||
                scope_type == ScopeIterator::ScopeTypeWith) {
       Handle<Context> cloned_context = Handle<Context>::cast(
@@ -207,6 +239,7 @@ DebugEvaluate::ContextBuilder::ContextBuilder(Isolate* isolate,
 
 
 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() &&
@@ -223,6 +256,11 @@ void DebugEvaluate::ContextBuilder::UpdateValues() {
       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);
+      }
     }
   }
 }
@@ -271,41 +309,84 @@ void DebugEvaluate::ContextBuilder::MaterializeArgumentsObject(
 }
 
 
-Handle<Context> DebugEvaluate::ContextBuilder::MaterializeReceiver(
-    Handle<Context> target, Handle<JSFunction> function) {
-  Handle<SharedFunctionInfo> shared(function->shared());
-  Handle<ScopeInfo> scope_info(shared->scope_info());
-  Handle<Object> receiver;
-  switch (scope_info->scope_type()) {
-    case FUNCTION_SCOPE: {
-      VariableMode mode;
-      InitializationFlag init_flag;
-      MaybeAssignedFlag maybe_assigned_flag;
-
-      // Don't bother creating a fake context node if "this" is in the context
-      // already.
-      if (ScopeInfo::ContextSlotIndex(scope_info,
-                                      isolate_->factory()->this_string(), &mode,
-                                      &init_flag, &maybe_assigned_flag) >= 0) {
-        return target;
-      }
-      receiver = handle(frame_->receiver(), isolate_);
-      break;
-    }
-    case MODULE_SCOPE:
-      receiver = isolate_->factory()->undefined_value();
-      break;
-    case SCRIPT_SCOPE:
-      receiver = handle(function->global_proxy(), isolate_);
-      break;
-    default:
-      // For eval code, arrow functions, and the like, there's no "this" binding
-      // to materialize.
-      return target;
+MaybeHandle<Object> DebugEvaluate::ContextBuilder::LoadFromContext(
+    Handle<Context> context, Handle<String> name) {
+  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);
+    return Handle<Object>(context->get(index), isolate_);
+  } else {  // Found on object.
+    Handle<JSReceiver> object = Handle<JSReceiver>::cast(holder);
+    return JSReceiver::GetDataProperty(object, name);
   }
+}
+
 
-  return isolate_->factory()->NewCatchContext(
-      function, target, isolate_->factory()->this_string(), receiver);
+void DebugEvaluate::ContextBuilder::MaterializeContextChain(
+    Handle<JSObject> target, Handle<Context> context) {
+  for (const Handle<String>& name : non_locals_) {
+    HandleScope scope(isolate_);
+    Handle<Object> value;
+    if (!LoadFromContext(context, name).ToHandle(&value)) 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> 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) {
+    LoadFromContext(lookup_context, this_string).ToHandle(&receiver);
+  } else if (function->shared()->scope_info()->HasReceiver()) {
+    receiver = handle(frame_->receiver(), isolate_);
+  }
+  return isolate_->factory()->NewCatchContext(function, parent_context,
+                                              this_string, receiver);
 }
 
 }  // namespace internal