[ic] Do access checks when storing via JSGlobalProxy.

Bug: chromium:764219
Change-Id: I99d1192c5c0f2b8bf47e0f193a0c4d9c00477466
Reviewed-on: https://chromium-review.googlesource.com/712454
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48492}

src/ic/accessor-assembler.h

@@ -115,6 +115,8 @@ class AccessorAssembler : public CodeStubAssembler {
   void KeyedLoadIC(const LoadICParameters* p);
   void KeyedLoadICGeneric(const LoadICParameters* p);
   void StoreIC(const StoreICParameters* p);
+  void StoreGlobalIC_PropertyCellCase(Node* property_cell, Node* value,
+                                      ExitPoint* exit_point, Label* miss);
   void KeyedStoreIC(const StoreICParameters* p);
 
   // IC dispatcher behavior.
@@ -150,6 +152,9 @@ class AccessorAssembler : public CodeStubAssembler {
                        Variable* var_double_value, Label* rebox_double,
                        ExitPoint* exit_point);
 
+  void EmitAccessCheck(Node* expected_native_context, Node* context,
+                       Node* receiver, Label* can_access, Label* miss);
+
   Node* EmitLoadICProtoArrayCheck(const LoadICParameters* p, Node* handler,
                                   Node* handler_length, Node* handler_flags,
                                   Label* miss);

src/ic/handler-configuration-inl.h

@@ -108,6 +108,11 @@ Handle<Smi> LoadHandler::LoadElement(Isolate* isolate,
   return handle(Smi::FromInt(config), isolate);
 }
 
+Handle<Smi> StoreHandler::StoreGlobalProxy(Isolate* isolate) {
+  int config = KindBits::encode(kStoreGlobalProxy);
+  return handle(Smi::FromInt(config), isolate);
+}
+
 Handle<Smi> StoreHandler::StoreNormal(Isolate* isolate) {
   int config = KindBits::encode(kStoreNormal);
   return handle(Smi::FromInt(config), isolate);
@@ -118,6 +123,13 @@ Handle<Smi> StoreHandler::StoreProxy(Isolate* isolate) {
   return handle(Smi::FromInt(config), isolate);
 }
 
+Handle<Smi> StoreHandler::EnableAccessCheckOnReceiver(Isolate* isolate,
+                                                      Handle<Smi> smi_handler) {
+  int config = smi_handler->value();
+  config = DoAccessCheckOnReceiverBits::update(config, true);
+  return handle(Smi::FromInt(config), isolate);
+}
+
 Handle<Smi> StoreHandler::StoreField(Isolate* isolate, Kind kind,
                                      int descriptor, FieldIndex field_index,
                                      Representation representation,

src/ic/handler-configuration.cc

@@ -232,41 +232,59 @@ Handle<Object> StoreHandler::StoreElementTransition(
 Handle<Object> StoreHandler::StoreTransition(Isolate* isolate,
                                              Handle<Map> receiver_map,
                                              Handle<JSObject> holder,
-                                             Handle<Map> transition,
+                                             Handle<HeapObject> transition,
                                              Handle<Name> name) {
-  Handle<Object> smi_handler;
-  if (transition->is_dictionary_map()) {
-    smi_handler = StoreNormal(isolate);
-  } else {
-    int descriptor = transition->LastAdded();
-    Handle<DescriptorArray> descriptors(transition->instance_descriptors());
-    PropertyDetails details = descriptors->GetDetails(descriptor);
-    Representation representation = details.representation();
-    DCHECK(!representation.IsNone());
+  Handle<Smi> smi_handler;
+  Handle<WeakCell> transition_cell;
+
+  if (transition->IsMap()) {
+    Handle<Map> transition_map = Handle<Map>::cast(transition);
+    if (transition_map->is_dictionary_map()) {
+      smi_handler = StoreNormal(isolate);
+    } else {
+      int descriptor = transition_map->LastAdded();
+      Handle<DescriptorArray> descriptors(
+          transition_map->instance_descriptors());
+      PropertyDetails details = descriptors->GetDetails(descriptor);
+      Representation representation = details.representation();
+      DCHECK(!representation.IsNone());
 
-    // Declarative handlers don't support access checks.
-    DCHECK(!transition->is_access_check_needed());
+      // Declarative handlers don't support access checks.
+      DCHECK(!transition_map->is_access_check_needed());
 
-    DCHECK_EQ(kData, details.kind());
-    if (details.location() == kDescriptor) {
-      smi_handler = TransitionToConstant(isolate, descriptor);
+      DCHECK_EQ(kData, details.kind());
+      if (details.location() == kDescriptor) {
+        smi_handler = TransitionToConstant(isolate, descriptor);
 
+      } else {
+        DCHECK_EQ(kField, details.location());
+        bool extend_storage = Map::cast(transition_map->GetBackPointer())
+                                  ->unused_property_fields() == 0;
+
+        FieldIndex index =
+            FieldIndex::ForDescriptor(*transition_map, descriptor);
+        smi_handler = TransitionToField(isolate, descriptor, index,
+                                        representation, extend_storage);
+      }
+    }
+    // |holder| is either a receiver if the property is non-existent or
+    // one of the prototypes.
+    DCHECK(!holder.is_null());
+    bool is_nonexistent = holder->map() == transition_map->GetBackPointer();
+    if (is_nonexistent) holder = Handle<JSObject>::null();
+    transition_cell = Map::WeakCellForMap(transition_map);
+
+  } else {
+    DCHECK(transition->IsPropertyCell());
+    if (receiver_map->IsJSGlobalObjectMap()) {
+      // TODO(ishell): this must be handled by StoreGlobalIC once it's finished.
+      return StoreGlobal(isolate, Handle<PropertyCell>::cast(transition));
     } else {
-      DCHECK_EQ(kField, details.location());
-      bool extend_storage =
-          Map::cast(transition->GetBackPointer())->unused_property_fields() ==
-          0;
-
-      FieldIndex index = FieldIndex::ForDescriptor(*transition, descriptor);
-      smi_handler = TransitionToField(isolate, descriptor, index,
-                                      representation, extend_storage);
+      DCHECK(receiver_map->IsJSGlobalProxyMap());
+      smi_handler = StoreGlobalProxy(isolate);
+      transition_cell = isolate->factory()->NewWeakCell(transition);
     }
   }
-  // |holder| is either a receiver if the property is non-existent or
-  // one of the prototypes.
-  DCHECK(!holder.is_null());
-  bool is_nonexistent = holder->map() == transition->GetBackPointer();
-  if (is_nonexistent) holder = Handle<JSObject>::null();
 
   int checks_count =
       GetPrototypeCheckCount(isolate, receiver_map, holder, name);
@@ -274,6 +292,12 @@ Handle<Object> StoreHandler::StoreTransition(Isolate* isolate,
   DCHECK_LE(0, checks_count);
   DCHECK(!receiver_map->IsJSGlobalObjectMap());
 
+  if (receiver_map->is_access_check_needed()) {
+    DCHECK(!receiver_map->is_dictionary_map());
+    DCHECK_LE(1, checks_count);  // For native context.
+    smi_handler = EnableAccessCheckOnReceiver(isolate, smi_handler);
+  }
+
   Handle<Object> validity_cell =
       Map::GetOrCreatePrototypeChainValidityCell(receiver_map, isolate);
   if (validity_cell.is_null()) {
@@ -281,8 +305,6 @@ Handle<Object> StoreHandler::StoreTransition(Isolate* isolate,
     validity_cell = handle(Smi::kZero, isolate);
   }
 
-  Handle<WeakCell> transition_cell = Map::WeakCellForMap(transition);
-
   Factory* factory = isolate->factory();
   if (checks_count == 0) {
     return factory->NewTuple3(transition_cell, smi_handler, validity_cell,
@@ -298,13 +320,19 @@ Handle<Object> StoreHandler::StoreTransition(Isolate* isolate,
   return handler_array;
 }
 
+// static
+Handle<Object> StoreHandler::StoreGlobal(Isolate* isolate,
+                                         Handle<PropertyCell> cell) {
+  return isolate->factory()->NewWeakCell(cell);
+}
+
 // static
 Handle<Object> StoreHandler::StoreProxy(Isolate* isolate,
                                         Handle<Map> receiver_map,
                                         Handle<JSProxy> proxy,
                                         Handle<JSReceiver> receiver,
                                         Handle<Name> name) {
-  Handle<Object> smi_handler = StoreProxy(isolate);
+  Handle<Smi> smi_handler = StoreProxy(isolate);
 
   if (receiver.is_identical_to(proxy)) return smi_handler;
 
@@ -312,6 +340,12 @@ Handle<Object> StoreHandler::StoreProxy(Isolate* isolate,
 
   DCHECK_LE(0, checks_count);
 
+  if (receiver_map->is_access_check_needed()) {
+    DCHECK(!receiver_map->is_dictionary_map());
+    DCHECK_LE(1, checks_count);  // For native context.
+    smi_handler = EnableAccessCheckOnReceiver(isolate, smi_handler);
+  }
+
   Handle<Object> validity_cell =
       Map::GetOrCreatePrototypeChainValidityCell(receiver_map, isolate);
   if (validity_cell.is_null()) {

src/ic/handler-configuration.h

@@ -178,12 +178,13 @@ class StoreHandler {
     kStoreElement,
     kStoreField,
     kStoreConstField,
+    // TODO(ishell): remove once constant field tracking is done.
+    kTransitionToConstant = kStoreConstField,
     kTransitionToField,
+    kStoreGlobalProxy,
     kStoreNormal,
     kProxy,
-    kKindsNumber,  // Keep last
-    // TODO(ishell): remove once constant field tracking is done.
-    kTransitionToConstant = kStoreConstField
+    kKindsNumber  // Keep last
   };
   class KindBits : public BitField<Kind, 0, 3> {};
 
@@ -191,12 +192,19 @@ class StoreHandler {
 
   static inline bool IsHandler(Object* maybe_handler);
 
+  // Applicable to kStoreGlobalProxy, kProxy kinds.
+
+  // Defines whether access rights check should be done on receiver object.
+  class DoAccessCheckOnReceiverBits
+      : public BitField<bool, KindBits::kNext, 1> {};
+
   // Applicable to kStoreField, kTransitionToField and kTransitionToConstant
   // kinds.
 
   // Index of a value entry in the descriptor array.
   class DescriptorBits
-      : public BitField<unsigned, KindBits::kNext, kDescriptorIndexBitCount> {};
+      : public BitField<unsigned, DoAccessCheckOnReceiverBits::kNext,
+                        kDescriptorIndexBitCount> {};
   //
   // Encoding when KindBits contains kTransitionToConstant.
   //
@@ -245,7 +253,7 @@ class StoreHandler {
   static Handle<Object> StoreTransition(Isolate* isolate,
                                         Handle<Map> receiver_map,
                                         Handle<JSObject> holder,
-                                        Handle<Map> transition,
+                                        Handle<HeapObject> transition,
                                         Handle<Name> name);
 
   static Handle<Object> StoreElementTransition(Isolate* isolate,
@@ -258,6 +266,14 @@ class StoreHandler {
                                    Handle<JSReceiver> receiver,
                                    Handle<Name> name);
 
+  // Creates a handler for storing a property to the property cell of a global
+  // object.
+  static Handle<Object> StoreGlobal(Isolate* isolate,
+                                    Handle<PropertyCell> cell);
+
+  // Creates a Smi-handler for storing a property to a global proxy object.
+  static inline Handle<Smi> StoreGlobalProxy(Isolate* isolate);
+
   // Creates a Smi-handler for storing a property to a slow object.
   static inline Handle<Smi> StoreNormal(Isolate* isolate);
 
@@ -265,6 +281,11 @@ class StoreHandler {
   static inline Handle<Smi> StoreProxy(Isolate* isolate);
 
  private:
+  // Sets DoAccessCheckOnReceiverBits in given Smi-handler. The receiver
+  // check is a part of a prototype chain check.
+  static inline Handle<Smi> EnableAccessCheckOnReceiver(
+      Isolate* isolate, Handle<Smi> smi_handler);
+
   static inline Handle<Smi> StoreField(Isolate* isolate, Kind kind,
                                        int descriptor, FieldIndex field_index,
                                        Representation representation,

src/ic/ic.cc

@@ -1366,23 +1366,19 @@ void StoreIC::UpdateCaches(LookupIterator* lookup, Handle<Object> value,
   TRACE_IC("StoreIC", lookup->name());
 }
 
-namespace {
-
-Handle<Object> StoreGlobal(Isolate* isolate, Handle<PropertyCell> cell) {
-  return isolate->factory()->NewWeakCell(cell);
-}
-
-}  // namespace
-
 Handle<Object> StoreIC::GetMapIndependentHandler(LookupIterator* lookup) {
   switch (lookup->state()) {
     case LookupIterator::TRANSITION: {
       Handle<JSObject> holder = lookup->GetHolder<JSObject>();
 
-      auto store_target = lookup->GetStoreTarget();
+      Handle<JSObject> store_target = lookup->GetStoreTarget();
       if (store_target->IsJSGlobalObject()) {
         TRACE_HANDLER_STATS(isolate(), StoreIC_StoreGlobalTransitionDH);
-        return StoreGlobal(isolate(), lookup->transition_cell());
+
+        Handle<Object> handler = StoreHandler::StoreTransition(
+            isolate(), receiver_map(), store_target, lookup->transition_cell(),
+            lookup->name());
+        return handler;
       }
       // Currently not handled by CompileStoreTransition.
       if (!holder->HasFastProperties()) {
@@ -1477,7 +1473,8 @@ Handle<Object> StoreIC::GetMapIndependentHandler(LookupIterator* lookup) {
       if (lookup->is_dictionary_holder()) {
         if (holder->IsJSGlobalObject()) {
           TRACE_HANDLER_STATS(isolate(), StoreIC_StoreGlobalDH);
-          return StoreGlobal(isolate(), lookup->GetPropertyCell());
+          return StoreHandler::StoreGlobal(isolate(),
+                                           lookup->GetPropertyCell());
         }
         TRACE_HANDLER_STATS(isolate(), StoreIC_StoreNormalDH);
         DCHECK(holder.is_identical_to(receiver));

test/mjsunit/regress/regress-crbug-764219.js

+// Copyright 2017 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.
+
+(function() {
+  function f(o) {
+    o.x = 42;
+  };
+
+  f({});
+  f(this);
+  f(this);
+})();
+
+(function() {
+  function f(o) {
+    o.y = 153;
+  };
+
+  Object.setPrototypeOf(this, new Proxy({}, {}));
+  f({});
+  f(this);
+  f(this);
+})();