[ic] Rely on prototype validity cell in Load/StoreGlobalIC.

... instead of checking if the property cell is still empty when loading/storing
through JSGlobalObject prototype.

Also invalidate the validity cell when new global lexical variables appear in the
script.

Bug: v8:5561
Change-Id: Iaf122dffe76d57b32e2b69291dee079e772b271c
Reviewed-on: https://chromium-review.googlesource.com/819230Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50100}

src/ic/accessor-assembler.cc

@@ -682,13 +682,9 @@ Node* AccessorAssembler::EmitLoadICProtoArrayCheck(const LoadICParameters* p,
   }
   BIND(&can_access);
 
-  BuildFastLoop(var_start_index.value(), handler_length,
-                [=](Node* current) {
-                  Node* prototype_cell =
-                      LoadFixedArrayElement(handler, current);
-                  CheckPrototype(prototype_cell, p->name, miss);
-                },
-                1, INTPTR_PARAMETERS, IndexAdvanceMode::kPost);
+  // TODO(ishell): Use LoadHandler with data2 field instead of FixedArray
+  // handlers.
+  CSA_ASSERT(this, WordEqual(var_start_index.value(), handler_length));
 
   Node* maybe_holder_cell =
       LoadFixedArrayElement(handler, LoadHandler::kDataIndex);
@@ -973,14 +969,11 @@ void AccessorAssembler::HandleStoreICProtoHandler(
     }
     BIND(&can_access);
 
+    // TODO(ishell): Use StoreHandler with data2 field instead of FixedArray
+    // handlers.
     Node* length = SmiUntag(maybe_transition_cell);
-    BuildFastLoop(var_start_index.value(), length,
-                  [=](Node* current) {
-                    Node* prototype_cell =
-                        LoadFixedArrayElement(handler, current);
-                    CheckPrototype(prototype_cell, p->name, miss);
-                  },
-                  1, INTPTR_PARAMETERS, IndexAdvanceMode::kPost);
+    CSA_ASSERT(this, WordEqual(var_start_index.value(), length));
+    USE(length);
 
     Node* maybe_transition_cell =
         LoadFixedArrayElement(handler, StoreHandler::kDataIndex);
@@ -1749,35 +1742,6 @@ void AccessorAssembler::EmitElementLoad(
   }
 }
 
-void AccessorAssembler::CheckPrototype(Node* prototype_cell, Node* name,
-                                       Label* miss) {
-  Node* maybe_prototype = LoadWeakCellValue(prototype_cell, miss);
-
-  Label done(this);
-  Label if_property_cell(this), if_dictionary_object(this);
-
-  // |maybe_prototype| is either a PropertyCell or a slow-mode prototype.
-  Branch(IsPropertyCell(maybe_prototype), &if_property_cell,
-         &if_dictionary_object);
-
-  BIND(&if_dictionary_object);
-  {
-    CSA_ASSERT(this, IsDictionaryMap(LoadMap(maybe_prototype)));
-    NameDictionaryNegativeLookup(maybe_prototype, name, miss);
-    Goto(&done);
-  }
-
-  BIND(&if_property_cell);
-  {
-    // Ensure the property cell still contains the hole.
-    Node* value = LoadObjectField(maybe_prototype, PropertyCell::kValueOffset);
-    GotoIfNot(IsTheHole(value), miss);
-    Goto(&done);
-  }
-
-  BIND(&done);
-}
-
 void AccessorAssembler::NameDictionaryNegativeLookup(Node* object, Node* name,
                                                      Label* miss) {
   CSA_ASSERT(this, IsDictionaryMap(LoadMap(object)));
@@ -1833,11 +1797,18 @@ void AccessorAssembler::InvalidateValidityCellIfPrototype(Node* map,
          &cont);
 
   BIND(&is_prototype);
-  Node* function = ExternalConstant(
-      ExternalReference::invalidate_prototype_chains_function(isolate()));
-  CallCFunction1(MachineType::AnyTagged(), MachineType::AnyTagged(), function,
-                 map);
-  Goto(&cont);
+  {
+    Node* maybe_prototype_info =
+        LoadObjectField(map, Map::kTransitionsOrPrototypeInfoOffset);
+    // If there's no prototype info then there's nothing to invalidate.
+    GotoIf(TaggedIsSmi(maybe_prototype_info), &cont);
+
+    Node* function = ExternalConstant(
+        ExternalReference::invalidate_prototype_chains_function(isolate()));
+    CallCFunction1(MachineType::AnyTagged(), MachineType::AnyTagged(), function,
+                   map);
+    Goto(&cont);
+  }
   BIND(&cont);
 }
 

src/ic/accessor-assembler.h

@@ -234,7 +234,6 @@ class AccessorAssembler : public CodeStubAssembler {
                        Label* rebox_double, Variable* var_double_value,
                        Label* unimplemented_elements_kind, Label* out_of_bounds,
                        Label* miss, ExitPoint* exit_point);
-  void CheckPrototype(Node* prototype_cell, Node* name, Label* miss);
   void NameDictionaryNegativeLookup(Node* object, Node* name, Label* miss);
 
   // Stub cache access helpers.

src/ic/handler-configuration.cc

@@ -34,19 +34,6 @@ int InitPrototypeChecks(Isolate* isolate, Handle<Map> receiver_map,
       array->set(first_index + checks_count, native_context->self_weak_cell());
     }
     checks_count++;
-
-  } else if (receiver_map->IsJSGlobalObjectMap()) {
-    // If we are creating a handler for [Load/Store]GlobalIC then we need to
-    // check that the property did not appear in the global object.
-    if (fill_array) {
-      Handle<JSGlobalObject> global = isolate->global_object();
-      Handle<PropertyCell> cell = JSGlobalObject::EnsureEmptyPropertyCell(
-          global, name, PropertyCellType::kInvalidated);
-      DCHECK(cell->value()->IsTheHole(isolate));
-      Handle<WeakCell> weak_cell = isolate->factory()->NewWeakCell(cell);
-      array->set(first_index + checks_count, *weak_cell);
-    }
-    checks_count++;
   }
   return checks_count;
 }

src/objects-debug.cc

@@ -448,6 +448,10 @@ void Map::MapVerify() {
   CHECK_IMPLIES(IsJSObjectMap() && !CanHaveFastTransitionableElementsKind(),
                 IsDictionaryElementsKind(elements_kind()) ||
                     IsTerminalElementsKind(elements_kind()));
+  if (is_prototype_map()) {
+    DCHECK(prototype_info() == Smi::kZero ||
+           prototype_info()->IsPrototypeInfo());
+  }
 }
 
 

src/objects.cc

@@ -12478,15 +12478,19 @@ bool JSObject::UnregisterPrototypeUser(Handle<Map> user, Isolate* isolate) {
   return true;
 }
 
+namespace {
 
-static void InvalidatePrototypeChainsInternal(Map* map) {
+// This function must be kept in sync with
+// AccessorAssembler::InvalidateValidityCellIfPrototype() which does pre-checks
+// before jumping here.
+PrototypeInfo* InvalidateOnePrototypeValidityCellInternal(Map* map) {
   DCHECK(map->is_prototype_map());
   if (FLAG_trace_prototype_users) {
     PrintF("Invalidating prototype map %p 's cell\n",
            reinterpret_cast<void*>(map));
   }
   Object* maybe_proto_info = map->prototype_info();
-  if (!maybe_proto_info->IsPrototypeInfo()) return;
+  if (!maybe_proto_info->IsPrototypeInfo()) return nullptr;
   PrototypeInfo* proto_info = PrototypeInfo::cast(maybe_proto_info);
   Object* maybe_cell = proto_info->validity_cell();
   if (maybe_cell->IsCell()) {
@@ -12494,6 +12498,12 @@ static void InvalidatePrototypeChainsInternal(Map* map) {
     Cell* cell = Cell::cast(maybe_cell);
     cell->set_value(Smi::FromInt(Map::kPrototypeChainInvalid));
   }
+  return proto_info;
+}
+
+void InvalidatePrototypeChainsInternal(Map* map) {
+  PrototypeInfo* proto_info = InvalidateOnePrototypeValidityCellInternal(map);
+  if (proto_info == nullptr) return;
 
   WeakFixedArray::Iterator iterator(proto_info->prototype_users());
   // For now, only maps register themselves as users.
@@ -12504,6 +12514,7 @@ static void InvalidatePrototypeChainsInternal(Map* map) {
   }
 }
 
+}  // namespace
 
 // static
 Map* JSObject::InvalidatePrototypeChains(Map* map) {
@@ -12512,6 +12523,18 @@ Map* JSObject::InvalidatePrototypeChains(Map* map) {
   return map;
 }
 
+// We also invalidate global objects validity cell when a new lexical
+// environment variable is added. This is necessary to ensure that
+// Load/StoreGlobalIC handlers that load/store from global object's prototype
+// get properly invalidated.
+// Note, that the normal Load/StoreICs that load/store through the global object
+// in the prototype chain are not affected by appearance of a new lexical
+// variable and therefore we don't propagate invalidation down.
+// static
+void JSObject::InvalidatePrototypeValidityCell(JSGlobalObject* global) {
+  DisallowHeapAllocation no_gc;
+  InvalidateOnePrototypeValidityCellInternal(global->map());
+}
 
 // static
 Handle<PrototypeInfo> Map::GetOrCreatePrototypeInfo(Handle<JSObject> prototype,
@@ -16636,6 +16659,10 @@ MaybeHandle<JSTypedArray> JSTypedArray::SpeciesCreate(
 
 void JSGlobalObject::InvalidatePropertyCell(Handle<JSGlobalObject> global,
                                             Handle<Name> name) {
+  // Regardless of whether the property is there or not invalidate
+  // Load/StoreGlobalICs that load/store through global object's prototype.
+  JSObject::InvalidatePrototypeValidityCell(*global);
+
   DCHECK(!global->HasFastProperties());
   auto dictionary = handle(global->global_dictionary());
   int entry = dictionary->FindEntry(name);

src/objects.h

@@ -2355,6 +2355,7 @@ class JSObject: public JSReceiver {
                                               Isolate* isolate);
   static bool UnregisterPrototypeUser(Handle<Map> user, Isolate* isolate);
   static Map* InvalidatePrototypeChains(Map* map);
+  static void InvalidatePrototypeValidityCell(JSGlobalObject* global);
 
   // Updates prototype chain tracking information when an object changes its
   // map from |old_map| to |new_map|.