[turbofan] Fix reduction of global proxy access

This fixes a bug in the optimization concerning detached
or re-attached global proxies.

Bug: v8:7790
Change-Id: Ifd30b88361914430bb373d4b64a76e33ccde37e5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1809361Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64035}

src/compiler/heap-refs.h

@@ -588,7 +588,6 @@ class V8_EXPORT_PRIVATE MapRef : public HeapObjectRef {
   bool is_migration_target() const;
   bool supports_fast_array_iteration() const;
   bool supports_fast_array_resize() const;
-  bool IsMapOfTargetGlobalProxy() const;
   bool is_abandoned_prototype_map() const;
 
   OddballType oddball_type() const;
@@ -869,6 +868,8 @@ class JSGlobalObjectRef : public JSObjectRef {
   DEFINE_REF_CONSTRUCTOR(JSGlobalObject, JSObjectRef)
 
   Handle<JSGlobalObject> object() const;
+
+  bool IsDetached() const;
 };
 
 class JSGlobalProxyRef : public JSObjectRef {

src/compiler/js-heap-broker.cc

@@ -963,9 +963,6 @@ class MapData : public HeapObjectData {
   bool supports_fast_array_resize() const {
     return supports_fast_array_resize_;
   }
-  bool IsMapOfTargetGlobalProxy() const {
-    return is_map_of_target_global_proxy_;
-  }
   bool is_abandoned_prototype_map() const {
     return is_abandoned_prototype_map_;
   }
@@ -1028,7 +1025,6 @@ class MapData : public HeapObjectData {
   int const unused_property_fields_;
   bool const supports_fast_array_iteration_;
   bool const supports_fast_array_resize_;
-  bool const is_map_of_target_global_proxy_;
   bool const is_abandoned_prototype_map_;
 
   bool serialized_elements_kind_generalizations_ = false;
@@ -1155,8 +1151,6 @@ MapData::MapData(JSHeapBroker* broker, ObjectData** storage, Handle<Map> object)
           SupportsFastArrayIteration(broker->isolate(), object)),
       supports_fast_array_resize_(
           SupportsFastArrayResize(broker->isolate(), object)),
-      is_map_of_target_global_proxy_(
-          object->IsMapOfGlobalProxy(broker->target_native_context().object())),
       is_abandoned_prototype_map_(object->is_abandoned_prototype_map()),
       elements_kind_generalizations_(broker->zone()) {}
 
@@ -1817,12 +1811,17 @@ class JSGlobalObjectData : public JSObjectData {
  public:
   JSGlobalObjectData(JSHeapBroker* broker, ObjectData** storage,
                      Handle<JSGlobalObject> object);
+  bool IsDetached() const { return is_detached_; }
+
+ private:
+  bool const is_detached_;
 };
 
 JSGlobalObjectData::JSGlobalObjectData(JSHeapBroker* broker,
                                        ObjectData** storage,
                                        Handle<JSGlobalObject> object)
-    : JSObjectData(broker, storage, object) {}
+    : JSObjectData(broker, storage, object),
+      is_detached_(object->IsDetached()) {}
 
 class JSGlobalProxyData : public JSObjectData {
  public:
@@ -2709,16 +2708,6 @@ bool MapRef::supports_fast_array_resize() const {
   return data()->AsMap()->supports_fast_array_resize();
 }
 
-bool MapRef::IsMapOfTargetGlobalProxy() const {
-  if (broker()->mode() == JSHeapBroker::kDisabled) {
-    AllowHandleDereference allow_handle_dereference;
-    AllowHandleAllocation handle_allocation;
-    return object()->IsMapOfGlobalProxy(
-        broker()->target_native_context().object());
-  }
-  return data()->AsMap()->IsMapOfTargetGlobalProxy();
-}
-
 int JSFunctionRef::InitialMapInstanceSizeWithMinSlack() const {
   if (broker()->mode() == JSHeapBroker::kDisabled) {
     AllowHandleDereference allow_handle_dereference;
@@ -3169,6 +3158,8 @@ BIMODAL_ACCESSOR(JSFunction, Object, prototype)
 BIMODAL_ACCESSOR(JSFunction, SharedFunctionInfo, shared)
 BIMODAL_ACCESSOR(JSFunction, FeedbackVector, feedback_vector)
 
+BIMODAL_ACCESSOR_C(JSGlobalObject, bool, IsDetached)
+
 BIMODAL_ACCESSOR_C(JSTypedArray, bool, is_on_heap)
 BIMODAL_ACCESSOR_C(JSTypedArray, size_t, length)
 BIMODAL_ACCESSOR(JSTypedArray, HeapObject, buffer)

src/compiler/js-native-context-specialization.cc

@@ -1082,7 +1082,9 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess(
   // corresponding global object instead.
   if (receiver_maps.size() == 1) {
     MapRef receiver_map(broker(), receiver_maps[0]);
-    if (receiver_map.IsMapOfTargetGlobalProxy()) {
+    if (receiver_map.equals(
+            broker()->target_native_context().global_proxy_object().map()) &&
+        !broker()->target_native_context().global_object().IsDetached()) {
       return ReduceGlobalAccess(node, receiver, value, feedback.name(),
                                 access_mode, key);
     }

src/compiler/serializer-for-background-compilation.cc

@@ -2573,9 +2573,9 @@ SerializerForBackgroundCompilation::ProcessMapForNamedPropertyAccess(
   receiver_map.SerializeRootMap();
 
   // For JSNativeContextSpecialization::ReduceNamedAccess.
-  if (receiver_map.IsMapOfTargetGlobalProxy()) {
-    JSGlobalProxyRef global_proxy =
-        broker()->target_native_context().global_proxy_object();
+  JSGlobalProxyRef global_proxy =
+      broker()->target_native_context().global_proxy_object();
+  if (receiver_map.equals(global_proxy.map())) {
     base::Optional<PropertyCellRef> cell = global_proxy.GetPropertyCell(
         name, SerializationPolicy::kSerializeIfNeeded);
     if (access_mode == AccessMode::kLoad && cell.has_value()) {

src/init/bootstrapper.cc

@@ -1300,8 +1300,8 @@ Handle<JSGlobalObject> Genesis::CreateNewGlobals(
   DCHECK(native_context()
              ->get(Context::GLOBAL_PROXY_INDEX)
              .IsUndefined(isolate()) ||
-         native_context()->global_proxy() == *global_proxy);
-  native_context()->set_global_proxy(*global_proxy);
+         native_context()->global_proxy_object() == *global_proxy);
+  native_context()->set_global_proxy_object(*global_proxy);
 
   return global_object;
 }

src/objects/contexts.cc

@@ -131,10 +131,6 @@ JSGlobalProxy Context::global_proxy() {
   return native_context().global_proxy_object();
 }
 
-void Context::set_global_proxy(JSGlobalProxy object) {
-  native_context().set_global_proxy_object(object);
-}
-
 /**
  * Lookups a property in an object environment, taking the unscopables into
  * account. This is used For HasBinding spec algorithms for ObjectEnvironment.

src/objects/contexts.h

@@ -580,7 +580,6 @@ class Context : public HeapObject {
 
   // Returns a JSGlobalProxy object or null.
   V8_EXPORT_PRIVATE JSGlobalProxy global_proxy();
-  void set_global_proxy(JSGlobalProxy global);
 
   // Get the JSGlobalObject object.
   V8_EXPORT_PRIVATE JSGlobalObject global_object();

src/objects/map.cc

@@ -56,18 +56,6 @@ MaybeHandle<JSFunction> Map::GetConstructorFunction(
   return MaybeHandle<JSFunction>();
 }
 
-bool Map::IsMapOfGlobalProxy(Handle<NativeContext> native_context) const {
-  DisallowHeapAllocation no_gc;
-  if (IsJSGlobalProxyMap()) {
-    Object maybe_constructor = GetConstructor();
-    // Detached global proxies have |null| as their constructor.
-    return maybe_constructor.IsJSFunction() &&
-           JSFunction::cast(maybe_constructor).native_context() ==
-               *native_context;
-  }
-  return false;
-}
-
 void Map::PrintReconfiguration(Isolate* isolate, FILE* file, int modify_index,
                                PropertyKind kind,
                                PropertyAttributes attributes) {

src/objects/map.h

@@ -881,9 +881,6 @@ class Map : public HeapObject {
       InstanceType instance_type);
   inline bool CanHaveFastTransitionableElementsKind() const;
 
-  // Whether this is the map of the given native context's global proxy.
-  bool IsMapOfGlobalProxy(Handle<NativeContext> native_context) const;
-
  private:
   // This byte encodes either the instance size without the in-object slack or
   // the slack size in properties backing store.

test/mjsunit/global-proxy-globalThis.js

+// Copyright 2019 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: --allow-natives-syntax
+
+{
+  const realm = Realm.createAllowCrossRealmAccess();
+  const foo = Realm.eval(realm, "function foo() { return globalThis.foo }; foo");
+
+  %PrepareFunctionForOptimization(foo);
+  assertSame(foo(), foo);
+  %OptimizeFunctionOnNextCall(foo);
+  assertSame(foo(), foo);
+}
+
+// detachGlobal, old map
+{
+  const realm = Realm.createAllowCrossRealmAccess();
+  const foo = Realm.eval(realm, "function foo() { return globalThis.foo }; foo");
+
+  %PrepareFunctionForOptimization(foo);
+  assertSame(foo(), foo);
+  Realm.detachGlobal(realm);
+  %OptimizeFunctionOnNextCall(foo);
+  assertThrows(foo);
+}
+
+// navigate, old map
+{
+  const realm = Realm.createAllowCrossRealmAccess();
+  const foo = Realm.eval(realm, "function foo() { return globalThis.foo }; foo");
+
+  %PrepareFunctionForOptimization(foo);
+  assertSame(foo(), foo);
+  Realm.navigate(realm);
+  %OptimizeFunctionOnNextCall(foo);
+  assertThrows(foo);
+}
+
+// detachGlobal, new map
+{
+  const realm = Realm.createAllowCrossRealmAccess();
+  const foo = Realm.eval(realm, "function foo() { return globalThis.foo }; foo");
+
+  assertSame(foo(), foo);
+  Realm.detachGlobal(realm);
+  %PrepareFunctionForOptimization(foo);
+  assertThrows(foo);
+  %OptimizeFunctionOnNextCall(foo);
+  assertThrows(foo);
+}
+
+// navigate, new map
+{
+  const realm = Realm.createAllowCrossRealmAccess();
+  const foo = Realm.eval(realm, "function foo() { return globalThis.foo }; foo");
+
+  assertSame(foo(), foo);
+  Realm.navigate(realm);
+  %PrepareFunctionForOptimization(foo);
+  assertThrows(foo);
+  %OptimizeFunctionOnNextCall(foo);
+  assertThrows(foo);
+}
+
+// detachGlobal, old and new map
+{
+  const realm = Realm.createAllowCrossRealmAccess();
+  const foo = Realm.eval(realm, "function foo() { return globalThis.foo }; foo");
+
+  %PrepareFunctionForOptimization(foo);
+  assertSame(foo(), foo);
+  Realm.detachGlobal(realm);
+  assertThrows(foo);
+  %OptimizeFunctionOnNextCall(foo);
+  assertThrows(foo);
+}
+
+// navigate, old and new map
+{
+  const realm = Realm.createAllowCrossRealmAccess();
+  const foo = Realm.eval(realm, "function foo() { return globalThis.foo }; foo");
+
+  %PrepareFunctionForOptimization(foo);
+  assertSame(foo(), foo);
+  Realm.navigate(realm);
+  assertThrows(foo);
+  %OptimizeFunctionOnNextCall(foo);
+  assertThrows(foo);
+}

test/mjsunit/global-proxy-this.js

+// Copyright 2019 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: --allow-natives-syntax
+
+{
+  const realm = Realm.createAllowCrossRealmAccess();
+  const foo = Realm.eval(realm, "function foo() { return this.foo }; foo");
+
+  %PrepareFunctionForOptimization(foo);
+  assertSame(foo(), foo);
+  %OptimizeFunctionOnNextCall(foo);
+  assertSame(foo(), foo);
+}
+
+// detachGlobal, old map
+{
+  const realm = Realm.createAllowCrossRealmAccess();
+  const foo = Realm.eval(realm, "function foo() { return this.foo }; foo");
+
+  %PrepareFunctionForOptimization(foo);
+  assertSame(foo(), foo);
+  Realm.detachGlobal(realm);
+  %OptimizeFunctionOnNextCall(foo);
+  assertThrows(foo);
+}
+
+// navigate, old map
+{
+  const realm = Realm.createAllowCrossRealmAccess();
+  const foo = Realm.eval(realm, "function foo() { return this.foo }; foo");
+
+  %PrepareFunctionForOptimization(foo);
+  assertSame(foo(), foo);
+  Realm.navigate(realm);
+  %OptimizeFunctionOnNextCall(foo);
+  assertThrows(foo);
+}
+
+// detachGlobal, new map
+{
+  const realm = Realm.createAllowCrossRealmAccess();
+  const foo = Realm.eval(realm, "function foo() { return this.foo }; foo");
+
+  assertSame(foo(), foo);
+  Realm.detachGlobal(realm);
+  %PrepareFunctionForOptimization(foo);
+  assertThrows(foo);
+  %OptimizeFunctionOnNextCall(foo);
+  assertThrows(foo);
+}
+
+// navigate, new map
+{
+  const realm = Realm.createAllowCrossRealmAccess();
+  const foo = Realm.eval(realm, "function foo() { return this.foo }; foo");
+
+  assertSame(foo(), foo);
+  Realm.navigate(realm);
+  %PrepareFunctionForOptimization(foo);
+  assertThrows(foo);
+  %OptimizeFunctionOnNextCall(foo);
+  assertThrows(foo);
+}
+
+// detachGlobal, old and new map
+{
+  const realm = Realm.createAllowCrossRealmAccess();
+  const foo = Realm.eval(realm, "function foo() { return this.foo }; foo");
+
+  %PrepareFunctionForOptimization(foo);
+  assertSame(foo(), foo);
+  Realm.detachGlobal(realm);
+  assertThrows(foo);
+  %OptimizeFunctionOnNextCall(foo);
+  assertThrows(foo);
+}
+
+// navigate, old and new map
+{
+  const realm = Realm.createAllowCrossRealmAccess();
+  const foo = Realm.eval(realm, "function foo() { return this.foo }; foo");
+
+  %PrepareFunctionForOptimization(foo);
+  assertSame(foo(), foo);
+  Realm.navigate(realm);
+  assertThrows(foo);
+  %OptimizeFunctionOnNextCall(foo);
+  assertThrows(foo);
+}