[turbofan] Properly guard keyed stores wrt. setters in the prototype chain.

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

Index: src/compiler/js-native-context-specialization.cc
diff --git a/src/compiler/js-native-context-specialization.cc b/src/compiler/js-native-context-specialization.cc
index 26793cc85185ea395bf134c0a6395df17f475b7e..eb71da5e0b46f52fb767413f0241fa8f9972f068 100644
--- a/src/compiler/js-native-context-specialization.cc
+++ b/src/compiler/js-native-context-specialization.cc
@@ -472,6 +472,41 @@ Reduction JSNativeContextSpecialization::ReduceElementAccess(
           DeoptimizeReason::kInsufficientTypeFeedbackForGenericKeyedAccess);
     }
 
+    // For holey stores or growing stores, we need to check that the prototype
+    // chain contains no setters for elements, and we need to guard those checks
+    // via code dependencies on the relevant prototype maps.
+    if (access_mode == AccessMode::kStore) {
+      ZoneVector<Handle<Map>> prototype_maps(zone());
+      for (ElementAccessInfo const& access_info : access_infos) {
+        for (Handle<Map> receiver_map : access_info.receiver_maps()) {
+          // If the {receiver_map} has a prototype and it's elements backing
+          // store is either holey, or we have a potentially growing store,
+          // then we need to check that all prototypes have stable maps with
+          // fast elements (and we need to guard against changes to that below).
+          if (!receiver_map->prototype()->IsNull(isolate()) &&

[Toon Verwaest, 2016/08/10 05:38:36]

drop this null check, already covered in the loop below.

[Benedikt Meurer, 2016/08/10 05:41:46]

Done.

+              (IsHoleyElementsKind(receiver_map->elements_kind()) ||
+               IsGrowStoreMode(store_mode))) {
+            // Make sure all prototypes are stable and have fast elements.
+            for (Handle<Map> map = receiver_map;;) {
+              Handle<Object> map_prototype(map->prototype(), isolate());
+              if (map_prototype->IsNull(isolate())) break;
+              if (!map_prototype->IsJSObject()) return NoChange();
+              map = handle(Handle<JSObject>::cast(map_prototype)->map(),

[Toon Verwaest, 2016/08/10 05:38:36]

JSObject::cast(*map_prototype)->map() is shorter :)

[Benedikt Meurer, 2016/08/10 05:41:46]

Acknowledged.

+                           isolate());
+              if (!map->is_stable()) return NoChange();
+              if (!IsFastElementsKind(map->elements_kind())) return NoChange();
+              prototype_maps.push_back(map);

[Toon Verwaest, 2016/08/10 05:38:36]

Most of the time this will only cover array_prototype, object_prototype for
different receiver maps. Don't we want to optimize for that case (and avoid
later overhead for registering, and avoiding zone allocation)?

[Benedikt Meurer, 2016/08/10 05:41:46]

Added a TODO.

+            }
+          }
+        }
+      }
+
+      // Install dependencies on the relevant prototype maps.
+      for (Handle<Map> prototype_map : prototype_maps) {
+        dependencies()->AssumeMapStable(prototype_map);
+      }
+    }
+
     // Ensure that {receiver} is a heap object.
     effect = BuildCheckTaggedPointer(receiver, effect, control);