Allow name collision when materializing scope object

When materializing a scope object, we previously assumed that we will
not have any name collisions. This is not correct e.g. when eval
introduces an aliased local variable.

This CL resolves this wrong assumption. The test case should not crash.
It however fails as there is a bug in how debug-evaluate should resolve
variables defined in eval.

R=verwaest@chromium.org

Fixed: chromium:1240962
Bug: chromium:1264852
Change-Id: I0e41e7905589735e25eff221376d09997ea99117
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3250911
Auto-Submit: Yang Guo <yangguo@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77649}

src/debug/debug-scopes.cc

@@ -569,7 +569,11 @@ Handle<JSObject> ScopeIterator::ScopeObject(Mode mode) {
       }
       value = isolate_->factory()->undefined_value();
     }
-    JSObject::AddProperty(isolate_, scope, name, value, NONE);
+    // Overwrite properties. Sometimes names in the same scope can collide, e.g.
+    // with extension objects introduced via local eval.
+    JSObject::SetPropertyOrElement(isolate_, scope, name, value,
+                                   Just(ShouldThrow::kDontThrow))
+        .Check();
     return false;
   };
 

src/heap/factory.cc

@@ -2303,11 +2303,8 @@ Handle<JSObject> Factory::NewJSObject(Handle<JSFunction> constructor,
 }
 
 Handle<JSObject> Factory::NewJSObjectWithNullProto() {
-  Handle<JSObject> result = NewJSObject(isolate()->object_function());
-  Handle<Map> new_map = Map::Copy(
-      isolate(), Handle<Map>(result->map(), isolate()), "ObjectWithNullProto");
-  Map::SetPrototype(isolate(), new_map, null_value());
-  JSObject::MigrateToMap(isolate(), result, new_map);
+  Handle<JSObject> result =
+      NewSlowJSObjectFromMap(isolate()->slow_object_with_null_prototype_map());
   return result;
 }
 

test/debugger/bugs/bug-1264852.js

+// Copyright 2011 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.
+//
+Debug = debug.Debug
+
+let listenerComplete = false;
+let exceptionThrown = false;
+
+function listener(event, exec_state, event_data, data) {
+  try {
+    if (event == Debug.DebugEvent.Break) {
+      assertEquals(42, exec_state.frame(0).evaluate("42").value());
+      // Indicate that all was processed.
+      listenerComplete = true;
+    }
+  } catch (e) {
+   exceptionThrown = true;
+  };
+};
+
+// Add the debug event listener.
+Debug.setListener(listener);
+
+assertEquals(
+  42,
+  (function f() {
+     eval("var f = 42");
+     debugger;
+     return f;
+   })()
+);
+
+Debug.setListener(null);
+
+assertFalse(exceptionThrown, "exception in listener");
+// Make sure that the debug event listener vas invoked.
+assertTrue(listenerComplete, "listener did not run to completion");