[debugger] ScopeIterator should unwrap debug-evaluate contexts.

If we use ScopeIterator inside a debug-evaluate call, we may iterate
over a debug-evaluate context that we created for the debug-evaluate
call. This may trigger assertions.

The solution is to have the ScopeIterator hide debug-evaluate contexts
by unwrapping it if it comes across any.

R=cbruni@chromium.org
BUG=chromium:599662
LOG=N

Review URL: https://codereview.chromium.org/1859033002

Cr-Commit-Position: refs/heads/master@{#35258}

src/debug/debug-evaluate.cc

@@ -154,7 +154,7 @@ DebugEvaluate::ContextBuilder::ContextBuilder(Isolate* isolate,
     ScopeIterator::ScopeType scope_type = it.Type();
     if (scope_type == ScopeIterator::ScopeTypeLocal) {
       DCHECK_EQ(FUNCTION_SCOPE, it.CurrentScopeInfo()->scope_type());
-      Handle<JSObject> materialized = NewJSObjectWithNullProto();
+      Handle<JSObject> materialized = factory->NewJSObjectWithNullProto();
       Handle<Context> local_context =
           it.HasContext() ? it.CurrentContext() : outer_context;
       Handle<StringSet> non_locals = it.GetNonLocals();
@@ -183,7 +183,7 @@ DebugEvaluate::ContextBuilder::ContextBuilder(Isolate* isolate,
       }
       context_chain_.Add(context_chain_element);
     } else if (scope_type == ScopeIterator::ScopeTypeBlock) {
-      Handle<JSObject> materialized = NewJSObjectWithNullProto();
+      Handle<JSObject> materialized = factory->NewJSObjectWithNullProto();
       frame_inspector.MaterializeStackLocals(materialized,
                                              it.CurrentScopeInfo());
       ContextChainElement context_chain_element;
@@ -220,17 +220,6 @@ void DebugEvaluate::ContextBuilder::UpdateValues() {
 }
 
 
-Handle<JSObject> DebugEvaluate::ContextBuilder::NewJSObjectWithNullProto() {
-  Handle<JSObject> result =
-      isolate_->factory()->NewJSObject(isolate_->object_function());
-  Handle<Map> new_map =
-      Map::Copy(Handle<Map>(result->map()), "ObjectWithNullProto");
-  Map::SetPrototype(new_map, isolate_->factory()->null_value());
-  JSObject::MigrateToMap(result, new_map);
-  return result;
-}
-
-
 void DebugEvaluate::ContextBuilder::MaterializeArgumentsObject(
     Handle<JSObject> target, Handle<JSFunction> function) {
   // Do not materialize the arguments object for eval or top-level code.

src/debug/debug-evaluate.h

@@ -64,8 +64,6 @@ class DebugEvaluate : public AllStatic {
       Handle<StringSet> whitelist;
     };
 
-    Handle<JSObject> NewJSObjectWithNullProto();
-
     // Helper function to find or create the arguments object for
     // Runtime_DebugEvaluate.
     void MaterializeArgumentsObject(Handle<JSObject> target,

src/debug/debug-scopes.cc

@@ -108,6 +108,7 @@ ScopeIterator::ScopeIterator(Isolate* isolate, FrameInspector* frame_inspector,
     if (!ignore_nested_scopes) RetrieveScopeChain(scope);
     if (collect_non_locals) CollectNonLocals(scope);
   }
+  UnwrapEvaluationContext();
 }
 
 
@@ -118,6 +119,23 @@ ScopeIterator::ScopeIterator(Isolate* isolate, Handle<JSFunction> function)
       seen_script_scope_(false),
       failed_(false) {
   if (!function->shared()->IsSubjectToDebugging()) context_ = Handle<Context>();
+  UnwrapEvaluationContext();
+}
+
+void ScopeIterator::UnwrapEvaluationContext() {
+  while (true) {
+    if (context_.is_null()) return;
+    if (!context_->IsDebugEvaluateContext()) return;
+    // An existing debug-evaluate context can only be outside the local scope.
+    DCHECK(nested_scope_chain_.is_empty());
+    Handle<Object> wrapped(context_->get(Context::WRAPPED_CONTEXT_INDEX),
+                           isolate_);
+    if (wrapped->IsContext()) {
+      context_ = Handle<Context>::cast(wrapped);
+    } else {
+      context_ = Handle<Context>(context_->previous(), isolate_);
+    }
+  }
 }
 
 
@@ -168,9 +186,7 @@ void ScopeIterator::Next() {
     // The global scope is always the last in the chain.
     DCHECK(context_->IsNativeContext());
     context_ = Handle<Context>();
-    return;
-  }
-  if (scope_type == ScopeTypeScript) {
+  } else if (scope_type == ScopeTypeScript) {
     seen_script_scope_ = true;
     if (context_->IsScriptContext()) {
       context_ = Handle<Context>(context_->previous(), isolate_);
@@ -182,9 +198,7 @@ void ScopeIterator::Next() {
       DCHECK(nested_scope_chain_.is_empty());
     }
     CHECK(context_->IsNativeContext());
-    return;
-  }
-  if (nested_scope_chain_.is_empty()) {
+  } else if (nested_scope_chain_.is_empty()) {
     context_ = Handle<Context>(context_->previous(), isolate_);
   } else {
     if (nested_scope_chain_.last().scope_info->HasContext()) {
@@ -193,6 +207,7 @@ void ScopeIterator::Next() {
     }
     nested_scope_chain_.RemoveLast();
   }
+  UnwrapEvaluationContext();
 }
 
 
@@ -262,9 +277,7 @@ MaybeHandle<JSObject> ScopeIterator::ScopeObject() {
       DCHECK(nested_scope_chain_.length() == 1);
       return MaterializeLocalScope();
     case ScopeIterator::ScopeTypeWith:
-      // Return the with object.
-      // TODO(neis): This breaks for proxies.
-      return handle(JSObject::cast(CurrentContext()->extension_receiver()));
+      return WithContextExtension();
     case ScopeIterator::ScopeTypeCatch:
       return MaterializeCatchScope();
     case ScopeIterator::ScopeTypeClosure:
@@ -534,6 +547,16 @@ Handle<JSObject> ScopeIterator::MaterializeCatchScope() {
   return catch_scope;
 }
 
+// Retrieve the with-context extension object. If the extension object is
+// a proxy, return an empty object.
+Handle<JSObject> ScopeIterator::WithContextExtension() {
+  Handle<Context> context = CurrentContext();
+  DCHECK(context->IsWithContext());
+  if (context->extension_receiver()->IsJSProxy()) {
+    return isolate_->factory()->NewJSObjectWithNullProto();
+  }
+  return handle(JSObject::cast(context->extension_receiver()));
+}
 
 // Create a plain JSObject which materializes the block scope for the specified
 // block context.

src/debug/debug-scopes.h

@@ -110,12 +110,15 @@ class ScopeIterator {
 
   void CollectNonLocals(Scope* scope);
 
+  void UnwrapEvaluationContext();
+
   MUST_USE_RESULT MaybeHandle<JSObject> MaterializeScriptScope();
   MUST_USE_RESULT MaybeHandle<JSObject> MaterializeLocalScope();
   MUST_USE_RESULT MaybeHandle<JSObject> MaterializeModuleScope();
   Handle<JSObject> MaterializeClosure();
   Handle<JSObject> MaterializeCatchScope();
   Handle<JSObject> MaterializeBlockScope();
+  Handle<JSObject> WithContextExtension();
 
   bool SetLocalVariableValue(Handle<String> variable_name,
                              Handle<Object> new_value);

src/factory.cc

@@ -1498,6 +1498,14 @@ Handle<JSObject> Factory::NewJSObjectWithMemento(
       JSObject);
 }
 
+Handle<JSObject> Factory::NewJSObjectWithNullProto() {
+  Handle<JSObject> result = NewJSObject(isolate()->object_function());
+  Handle<Map> new_map =
+      Map::Copy(Handle<Map>(result->map()), "ObjectWithNullProto");
+  Map::SetPrototype(new_map, null_value());
+  JSObject::MigrateToMap(result, new_map);
+  return result;
+}
 
 Handle<JSModule> Factory::NewJSModule(Handle<Context> context,
                                       Handle<ScopeInfo> scope_info) {

src/factory.h

@@ -394,6 +394,8 @@ class Factory final {
   // JSObject that should have a memento pointing to the allocation site.
   Handle<JSObject> NewJSObjectWithMemento(Handle<JSFunction> constructor,
                                           Handle<AllocationSite> site);
+  // JSObject without a prototype.
+  Handle<JSObject> NewJSObjectWithNullProto();
 
   // Global objects are pretenured and initialized based on a constructor.
   Handle<JSGlobalObject> NewJSGlobalObject(Handle<JSFunction> constructor);

test/mjsunit/debug-evaluate-nested.js

+// Copyright 2016 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;
+ScopeType = debug.ScopeType;
+var exception = null;
+var nested = false;
+
+function bar() {
+  let a = 1;
+  (function foo() {
+    let b = a;
+    with (new Proxy({}, {})) {
+      debugger;
+    }
+  })();
+}
+
+function checkScopes(scopes, expectation) {
+  assertEquals(scopes.map(s => s.scopeType()), expectation);
+}
+
+function listener(event, exec_state, event_data, data) {
+  if (event != Debug.DebugEvent.Break) return;
+  try {
+    if (!nested) {
+      nested = true;
+      checkScopes(exec_state.frame(0).allScopes(),
+                  [ ScopeType.With, ScopeType.Local, ScopeType.Closure,
+                    ScopeType.Script, ScopeType.Global ]);
+      exec_state.frame(0).evaluate("debugger;");
+    } else {
+      checkScopes(exec_state.frame(0).allScopes(),
+                  [ ScopeType.With, ScopeType.Closure,
+                    ScopeType.Script, ScopeType.Global ]);
+    }
+  } catch (e) {
+    exception = e;
+    print(e + e.stack);
+  }
+}
+
+Debug.setListener(listener);
+bar();
+Debug.setListener(null);
+assertNull(exception);