[ic] Add OOB support to KeyedLoadIC.

This adds support to the KeyedLoadIC to ignore out of bounds accesses
for Strings and return undefined instead. We add a dedicated bit to the
Smi handler to encode the OOB state and have TurboFan generate appropriate
code for that case as well. This is mostly useful when programs
accidentially access past the length of a string, which was observed and
fixed for example in Babel recently, see

  https://github.com/babel/babel/pull/6589

for details. The idea is to also extend this mechanism to Arrays and
maybe other receivers, as reading beyond the length is also often used
in jQuery and other popular libraries.

Note that this is considered a mitigation for a performance cliff and
not a general optimization of OOB accesses. These should still be
avoided and handled properly instead.

This seems to further improve the babel test on the web-tooling-benchmark
by around 1%, because the OOB access no longer turns the otherwise
MONOMORPHIC access into MEGAMORPHIC state.

Bug: v8:6936, v8:7014
Change-Id: I9df03304e056d7001a65da8e9621119f8e9bb55b
Reviewed-on: https://chromium-review.googlesource.com/744022
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49049}

src/bootstrapper.cc

@@ -1947,6 +1947,7 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
         Handle<JSValue>::cast(factory->NewJSObject(string_fun, TENURED));
     prototype->set_value(isolate->heap()->empty_string());
     JSFunction::SetPrototype(string_fun, prototype);
+    native_context()->set_initial_string_prototype(*prototype);
 
     // Install the "constructor" property on the {prototype}.
     JSObject::AddProperty(prototype, factory->constructor_string(), string_fun,

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

@@ -1047,7 +1047,8 @@ Reduction JSNativeContextSpecialization::ReduceJSStoreNamedOwn(Node* node) {
 
 Reduction JSNativeContextSpecialization::ReduceElementAccess(
     Node* node, Node* index, Node* value, MapHandles const& receiver_maps,
-    AccessMode access_mode, KeyedAccessStoreMode store_mode) {
+    AccessMode access_mode, KeyedAccessLoadMode load_mode,
+    KeyedAccessStoreMode store_mode) {
   DCHECK(node->opcode() == IrOpcode::kJSLoadProperty ||
          node->opcode() == IrOpcode::kJSStoreProperty);
   Node* receiver = NodeProperties::GetValueInput(node, 0);
@@ -1069,13 +1070,10 @@ Reduction JSNativeContextSpecialization::ReduceElementAccess(
         simplified()->LoadField(AccessBuilder::ForStringLength()), receiver,
         effect, control);
 
-    // Ensure that {index} is less than {receiver} length.
-    index = effect = graph()->NewNode(simplified()->CheckBounds(), index,
-                                      length, effect, control);
-
-    // Return the character from the {receiver} as single character string.
-    value = graph()->NewNode(simplified()->StringCharAt(), receiver, index,
-                             control);
+    // Load the single character string from {receiver} or yield undefined
+    // if the {index} is out of bounds (depending on the {load_mode}).
+    value = BuildIndexedStringLoad(receiver, index, length, &effect, &control,
+                                   load_mode);
   } else {
     // Retrieve the native context from the given {node}.
     // Compute element access infos for the receiver maps.
@@ -1271,7 +1269,8 @@ Reduction JSNativeContextSpecialization::ReduceElementAccess(
 template <typename KeyedICNexus>
 Reduction JSNativeContextSpecialization::ReduceKeyedAccess(
     Node* node, Node* index, Node* value, KeyedICNexus const& nexus,
-    AccessMode access_mode, KeyedAccessStoreMode store_mode) {
+    AccessMode access_mode, KeyedAccessLoadMode load_mode,
+    KeyedAccessStoreMode store_mode) {
   DCHECK(node->opcode() == IrOpcode::kJSLoadProperty ||
          node->opcode() == IrOpcode::kJSStoreProperty);
   Node* receiver = NodeProperties::GetValueInput(node, 0);
@@ -1345,13 +1344,11 @@ Reduction JSNativeContextSpecialization::ReduceKeyedAccess(
         if (nexus.ic_state() != MEGAMORPHIC && nexus.GetKeyType() == ELEMENT) {
           // Ensure that {index} is less than {receiver} length.
           Node* length = jsgraph()->Constant(string->length());
-          index = effect = graph()->NewNode(simplified()->CheckBounds(), index,
-                                            length, effect, control);
 
-          // Return the character from the {receiver} as single character
-          // string.
-          value = graph()->NewNode(simplified()->StringCharAt(), receiver,
-                                   index, control);
+          // Load the single character string from {receiver} or yield undefined
+          // if the {index} is out of bounds (depending on the {load_mode}).
+          value = BuildIndexedStringLoad(receiver, index, length, &effect,
+                                         &control, load_mode);
           ReplaceWithValue(node, value, effect, control);
           return Replace(value);
         }
@@ -1421,7 +1418,7 @@ Reduction JSNativeContextSpecialization::ReduceKeyedAccess(
 
   // Try to lower the element access based on the {receiver_maps}.
   return ReduceElementAccess(node, index, value, receiver_maps, access_mode,
-                             store_mode);
+                             load_mode, store_mode);
 }
 
 Reduction JSNativeContextSpecialization::ReduceSoftDeoptimize(
@@ -1552,9 +1549,12 @@ Reduction JSNativeContextSpecialization::ReduceJSLoadProperty(Node* node) {
   if (!p.feedback().IsValid()) return NoChange();
   KeyedLoadICNexus nexus(p.feedback().vector(), p.feedback().slot());
 
+  // Extract the keyed access load mode from the keyed load IC.
+  KeyedAccessLoadMode load_mode = nexus.GetKeyedAccessLoadMode();
+
   // Try to lower the keyed access based on the {nexus}.
   return ReduceKeyedAccess(node, name, value, nexus, AccessMode::kLoad,
-                           STANDARD_STORE);
+                           load_mode, STANDARD_STORE);
 }
 
 Reduction JSNativeContextSpecialization::ReduceJSStoreProperty(Node* node) {
@@ -1572,7 +1572,7 @@ Reduction JSNativeContextSpecialization::ReduceJSStoreProperty(Node* node) {
 
   // Try to lower the keyed access based on the {nexus}.
   return ReduceKeyedAccess(node, index, value, nexus, AccessMode::kStore,
-                           store_mode);
+                           STANDARD_LOAD, store_mode);
 }
 
 Node* JSNativeContextSpecialization::InlinePropertyGetterCall(
@@ -2414,6 +2414,47 @@ JSNativeContextSpecialization::BuildElementAccess(
   return ValueEffectControl(value, effect, control);
 }
 
+Node* JSNativeContextSpecialization::BuildIndexedStringLoad(
+    Node* receiver, Node* index, Node* length, Node** effect, Node** control,
+    KeyedAccessLoadMode load_mode) {
+  if (load_mode == LOAD_IGNORE_OUT_OF_BOUNDS &&
+      isolate()->IsFastArrayConstructorPrototypeChainIntact()) {
+    // Add a code dependency on the "no elements" protector.
+    dependencies()->AssumePropertyCell(factory()->array_protector());
+
+    // Ensure that the {index} is a valid String length.
+    index = *effect = graph()->NewNode(simplified()->CheckBounds(), index,
+                                       jsgraph()->Constant(String::kMaxLength),
+                                       *effect, *control);
+
+    // Load the single character string from {receiver} or yield
+    // undefined if the {index} is not within the valid bounds.
+    Node* check =
+        graph()->NewNode(simplified()->NumberLessThan(), index, length);
+    Node* branch =
+        graph()->NewNode(common()->Branch(BranchHint::kTrue), check, *control);
+
+    Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+    Node* vtrue = graph()->NewNode(simplified()->StringCharAt(), receiver,
+                                   index, if_true);
+
+    Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+    Node* vfalse = jsgraph()->UndefinedConstant();
+
+    *control = graph()->NewNode(common()->Merge(2), if_true, if_false);
+    return graph()->NewNode(common()->Phi(MachineRepresentation::kTagged, 2),
+                            vtrue, vfalse, *control);
+  } else {
+    // Ensure that {index} is less than {receiver} length.
+    index = *effect = graph()->NewNode(simplified()->CheckBounds(), index,
+                                       length, *effect, *control);
+
+    // Return the character from the {receiver} as single character string.
+    return graph()->NewNode(simplified()->StringCharAt(), receiver, index,
+                            *control);
+  }
+}
+
 Node* JSNativeContextSpecialization::BuildExtendPropertiesBackingStore(
     Handle<Map> map, Node* properties, Node* effect, Node* control) {
   // TODO(bmeurer/jkummerow): Property deletions can undo map transitions

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

@@ -75,10 +75,12 @@ class JSNativeContextSpecialization final : public AdvancedReducer {
   Reduction ReduceElementAccess(Node* node, Node* index, Node* value,
                                 MapHandles const& receiver_maps,
                                 AccessMode access_mode,
+                                KeyedAccessLoadMode load_mode,
                                 KeyedAccessStoreMode store_mode);
   template <typename KeyedICNexus>
   Reduction ReduceKeyedAccess(Node* node, Node* index, Node* value,
                               KeyedICNexus const& nexus, AccessMode access_mode,
+                              KeyedAccessLoadMode load_mode,
                               KeyedAccessStoreMode store_mode);
   Reduction ReduceNamedAccessFromNexus(Node* node, Node* value,
                                        FeedbackNexus const& nexus,
@@ -158,6 +160,11 @@ class JSNativeContextSpecialization final : public AdvancedReducer {
                                         AccessMode access_mode,
                                         KeyedAccessStoreMode store_mode);
 
+  // Construct appropriate subgraph to load from a String.
+  Node* BuildIndexedStringLoad(Node* receiver, Node* index, Node* length,
+                               Node** effect, Node** control,
+                               KeyedAccessLoadMode load_mode);
+
   // Construct appropriate subgraph to extend properties backing store.
   Node* BuildExtendPropertiesBackingStore(Handle<Map> map, Node* properties,
                                           Node* effect, Node* control);

src/contexts.h

@@ -266,6 +266,7 @@ enum ContextLookupFlags {
     initial_async_generator_prototype)                                         \
   V(INITIAL_ITERATOR_PROTOTYPE_INDEX, JSObject, initial_iterator_prototype)    \
   V(INITIAL_OBJECT_PROTOTYPE_INDEX, JSObject, initial_object_prototype)        \
+  V(INITIAL_STRING_PROTOTYPE_INDEX, JSObject, initial_string_prototype)        \
   V(INT16_ARRAY_FUN_INDEX, JSFunction, int16_array_fun)                        \
   V(INT32_ARRAY_FUN_INDEX, JSFunction, int32_array_fun)                        \
   V(INT8_ARRAY_FUN_INDEX, JSFunction, int8_array_fun)                          \

src/elements.cc

@@ -1887,7 +1887,8 @@ class FastElementsAccessor : public ElementsAccessorBase<Subclass, KindTraits> {
 
     // Ensure that notifications fire if the array or object prototypes are
     // normalizing.
-    if (IsSmiOrObjectElementsKind(kind)) {
+    if (IsSmiOrObjectElementsKind(kind) ||
+        kind == FAST_STRING_WRAPPER_ELEMENTS) {
       isolate->UpdateArrayProtectorOnNormalizeElements(object);
     }
 
@@ -4151,6 +4152,13 @@ class StringWrapperElementsAccessor
                                          uint32_t capacity) {
     Handle<FixedArrayBase> old_elements(object->elements());
     ElementsKind from_kind = object->GetElementsKind();
+    if (from_kind == FAST_STRING_WRAPPER_ELEMENTS) {
+      // The optimizing compiler relies on the prototype lookups of String
+      // objects always returning undefined. If there's a store to the
+      // initial String.prototype object, make sure all the optimizations
+      // are invalidated.
+      object->GetIsolate()->UpdateArrayProtectorOnSetLength(object);
+    }
     // This method should only be called if there's a reason to update the
     // elements.
     DCHECK(from_kind == SLOW_STRING_WRAPPER_ELEMENTS ||

src/feedback-vector.cc

@@ -858,6 +858,30 @@ Name* KeyedStoreICNexus::FindFirstName() const {
   return nullptr;
 }
 
+KeyedAccessLoadMode KeyedLoadICNexus::GetKeyedAccessLoadMode() const {
+  KeyedAccessLoadMode mode = STANDARD_LOAD;
+  MapHandles maps;
+  ObjectHandles handlers;
+
+  if (GetKeyType() == PROPERTY) return mode;
+
+  ExtractMaps(&maps);
+  FindHandlers(&handlers, static_cast<int>(maps.size()));
+  for (Handle<Object> const& handler : handlers) {
+    if (handler->IsSmi()) {
+      int const raw_handler = Handle<Smi>::cast(handler)->value();
+      if (LoadHandler::KindBits::decode(raw_handler) ==
+              LoadHandler::kIndexedString &&
+          LoadHandler::AllowOutOfBoundsBits::decode(raw_handler)) {
+        mode = LOAD_IGNORE_OUT_OF_BOUNDS;
+        break;
+      }
+    }
+  }
+
+  return mode;
+}
+
 KeyedAccessStoreMode KeyedStoreICNexus::GetKeyedAccessStoreMode() const {
   KeyedAccessStoreMode mode = STANDARD_STORE;
   MapHandles maps;

src/feedback-vector.h

@@ -709,6 +709,7 @@ class KeyedLoadICNexus : public FeedbackNexus {
 
   void Clear() override { ConfigurePremonomorphic(); }
 
+  KeyedAccessLoadMode GetKeyedAccessLoadMode() const;
   IcCheckType GetKeyType() const;
   InlineCacheState StateFromFeedback() const override;
   Name* FindFirstName() const override;

src/ic/accessor-assembler.cc

@@ -255,13 +255,24 @@ void AccessorAssembler::HandleLoadICSmiHandlerCase(
 
     BIND(&if_indexed_string);
     {
+      Label if_oob(this, Label::kDeferred);
+
       Comment("indexed string");
       Node* intptr_index = TryToIntptr(p->name, miss);
       Node* length = SmiUntag(LoadStringLength(holder));
-      GotoIf(UintPtrGreaterThanOrEqual(intptr_index, length), miss);
+      GotoIf(UintPtrGreaterThanOrEqual(intptr_index, length), &if_oob);
       Node* code = StringCharCodeAt(holder, intptr_index, INTPTR_PARAMETERS);
       Node* result = StringFromCharCode(code);
       Return(result);
+
+      BIND(&if_oob);
+      Node* allow_out_of_bounds =
+          IsSetWord<LoadHandler::AllowOutOfBoundsBits>(handler_word);
+      GotoIfNot(allow_out_of_bounds, miss);
+      // TODO(bmeurer): This is going to be renamed to NoElementsProtector
+      // in a follow-up CL.
+      GotoIf(IsArrayProtectorCellInvalid(), miss);
+      Return(UndefinedConstant());
     }
 
     BIND(&if_property);

src/ic/handler-configuration-inl.h

@@ -108,8 +108,10 @@ Handle<Smi> LoadHandler::LoadElement(Isolate* isolate,
   return handle(Smi::FromInt(config), isolate);
 }
 
-Handle<Smi> LoadHandler::LoadIndexedString(Isolate* isolate) {
-  int config = KindBits::encode(kIndexedString);
+Handle<Smi> LoadHandler::LoadIndexedString(Isolate* isolate,
+                                           bool allow_out_of_bounds) {
+  int config = KindBits::encode(kIndexedString) |
+               AllowOutOfBoundsBits::encode(allow_out_of_bounds);
   return handle(Smi::FromInt(config), isolate);
 }
 

src/ic/handler-configuration.h

@@ -79,6 +79,11 @@ class LoadHandler {
   // Make sure we don't overflow the smi.
   STATIC_ASSERT(ElementsKindBits::kNext <= kSmiValueSize);
 
+  //
+  // Encoding when KindBits contains kIndexedString.
+  //
+  class AllowOutOfBoundsBits : public BitField<bool, KindBits::kNext, 1> {};
+
   //
   // Encoding when KindBits contains kModuleExport.
   //
@@ -161,7 +166,8 @@ class LoadHandler {
                                         bool is_js_array);
 
   // Creates a Smi-handler for loading from a String.
-  static inline Handle<Smi> LoadIndexedString(Isolate* isolate);
+  static inline Handle<Smi> LoadIndexedString(Isolate* isolate,
+                                              bool allow_out_of_bounds);
 
  private:
   // Sets DoAccessCheckOnReceiverBits in given Smi-handler. The receiver

src/ic/ic.cc

@@ -51,8 +51,14 @@ char IC::TransitionMarkFromState(IC::State state) {
   UNREACHABLE();
 }
 
+namespace {
 
-const char* GetTransitionMarkModifier(KeyedAccessStoreMode mode) {
+const char* GetModifier(KeyedAccessLoadMode mode) {
+  if (mode == LOAD_IGNORE_OUT_OF_BOUNDS) return ".IGNORE_OOB";
+  return "";
+}
+
+const char* GetModifier(KeyedAccessStoreMode mode) {
   if (mode == STORE_NO_TRANSITION_HANDLE_COW) return ".COW";
   if (mode == STORE_NO_TRANSITION_IGNORE_OUT_OF_BOUNDS) {
     return ".IGNORE_OOB";
@@ -61,6 +67,8 @@ const char* GetTransitionMarkModifier(KeyedAccessStoreMode mode) {
   return "";
 }
 
+}  // namespace
+
 #define TRACE_GENERIC_IC(reason) set_slow_stub_reason(reason);
 
 void IC::TraceIC(const char* type, Handle<Object> name) {
@@ -81,10 +89,14 @@ void IC::TraceIC(const char* type, Handle<Object> name, State old_state,
   }
 
   const char* modifier = "";
-  if (IsKeyedStoreIC()) {
+  if (IsKeyedLoadIC()) {
+    KeyedAccessLoadMode mode =
+        casted_nexus<KeyedLoadICNexus>()->GetKeyedAccessLoadMode();
+    modifier = GetModifier(mode);
+  } else if (IsKeyedStoreIC()) {
     KeyedAccessStoreMode mode =
         casted_nexus<KeyedStoreICNexus>()->GetKeyedAccessStoreMode();
-    modifier = GetTransitionMarkModifier(mode);
+    modifier = GetModifier(mode);
   }
 
   if (!(FLAG_ic_stats &
@@ -984,14 +996,15 @@ static Handle<Object> TryConvertKey(Handle<Object> key, Isolate* isolate) {
   return key;
 }
 
-void KeyedLoadIC::UpdateLoadElement(Handle<HeapObject> receiver) {
+void KeyedLoadIC::UpdateLoadElement(Handle<HeapObject> receiver,
+                                    KeyedAccessLoadMode load_mode) {
   Handle<Map> receiver_map(receiver->map(), isolate());
   DCHECK(receiver_map->instance_type() != JS_VALUE_TYPE);  // Checked by caller.
   MapHandles target_receiver_maps;
   TargetMaps(&target_receiver_maps);
 
   if (target_receiver_maps.empty()) {
-    Handle<Object> handler = LoadElementHandler(receiver_map);
+    Handle<Object> handler = LoadElementHandler(receiver_map, load_mode);
     return ConfigureVectorState(Handle<Name>(), receiver_map, handler);
   }
 
@@ -1019,7 +1032,7 @@ void KeyedLoadIC::UpdateLoadElement(Handle<HeapObject> receiver) {
       IsMoreGeneralElementsKindTransition(
           target_receiver_maps.at(0)->elements_kind(),
           Handle<JSObject>::cast(receiver)->GetElementsKind())) {
-    Handle<Object> handler = LoadElementHandler(receiver_map);
+    Handle<Object> handler = LoadElementHandler(receiver_map, load_mode);
     return ConfigureVectorState(Handle<Name>(), receiver_map, handler);
   }
 
@@ -1028,11 +1041,18 @@ void KeyedLoadIC::UpdateLoadElement(Handle<HeapObject> receiver) {
   // Determine the list of receiver maps that this call site has seen,
   // adding the map that was just encountered.
   if (!AddOneReceiverMapIfMissing(&target_receiver_maps, receiver_map)) {
+    // If the {receiver_map} is a primitive String map, we can only get
+    // here if the access was out of bounds (and the IC was not in that
+    // state already). In that case just go on and update the handler
+    // appropriately below.
+    if (!receiver_map->IsStringMap() ||
+        load_mode != LOAD_IGNORE_OUT_OF_BOUNDS) {
       // If the miss wasn't due to an unseen map, a polymorphic stub
       // won't help, use the generic stub.
       TRACE_GENERIC_IC("same map added twice");
       return;
     }
+  }
 
   // If the maximum number of receiver maps has been exceeded, use the generic
   // version of the IC.
@@ -1043,7 +1063,7 @@ void KeyedLoadIC::UpdateLoadElement(Handle<HeapObject> receiver) {
 
   ObjectHandles handlers;
   handlers.reserve(target_receiver_maps.size());
-  LoadElementPolymorphicHandlers(&target_receiver_maps, &handlers);
+  LoadElementPolymorphicHandlers(&target_receiver_maps, &handlers, load_mode);
   DCHECK_LE(1, target_receiver_maps.size());
   if (target_receiver_maps.size() == 1) {
     ConfigureVectorState(Handle<Name>(), target_receiver_maps[0], handlers[0]);
@@ -1052,7 +1072,8 @@ void KeyedLoadIC::UpdateLoadElement(Handle<HeapObject> receiver) {
   }
 }
 
-Handle<Object> KeyedLoadIC::LoadElementHandler(Handle<Map> receiver_map) {
+Handle<Object> KeyedLoadIC::LoadElementHandler(Handle<Map> receiver_map,
+                                               KeyedAccessLoadMode load_mode) {
   if (receiver_map->has_indexed_interceptor() &&
       !receiver_map->GetIndexedInterceptor()->getter()->IsUndefined(
           isolate()) &&
@@ -1063,7 +1084,7 @@ Handle<Object> KeyedLoadIC::LoadElementHandler(Handle<Map> receiver_map) {
   InstanceType instance_type = receiver_map->instance_type();
   if (instance_type < FIRST_NONSTRING_TYPE) {
     TRACE_HANDLER_STATS(isolate(), KeyedLoadIC_LoadIndexedStringDH);
-    return LoadHandler::LoadIndexedString(isolate());
+    return LoadHandler::LoadIndexedString(isolate(), load_mode);
   }
   if (instance_type < FIRST_JS_RECEIVER_TYPE) {
     TRACE_HANDLER_STATS(isolate(), KeyedLoadIC_SlowStub);
@@ -1096,8 +1117,9 @@ Handle<Object> KeyedLoadIC::LoadElementHandler(Handle<Map> receiver_map) {
                                   convert_hole_to_undefined, is_js_array);
 }
 
-void KeyedLoadIC::LoadElementPolymorphicHandlers(MapHandles* receiver_maps,
-                                                 ObjectHandles* handlers) {
+void KeyedLoadIC::LoadElementPolymorphicHandlers(
+    MapHandles* receiver_maps, ObjectHandles* handlers,
+    KeyedAccessLoadMode load_mode) {
   // Filter out deprecated maps to ensure their instances get migrated.
   receiver_maps->erase(
       std::remove_if(
@@ -1115,10 +1137,36 @@ void KeyedLoadIC::LoadElementPolymorphicHandlers(MapHandles* receiver_maps,
         receiver_map->NotifyLeafMapLayoutChange();
       }
     }
-    handlers->push_back(LoadElementHandler(receiver_map));
+    handlers->push_back(LoadElementHandler(receiver_map, load_mode));
   }
 }
 
+namespace {
+
+bool IsOutOfBoundsAccess(Handle<Object> receiver, uint32_t index) {
+  uint32_t length = 0;
+  if (receiver->IsJSArray()) {
+    JSArray::cast(*receiver)->length()->ToArrayLength(&length);
+  } else if (receiver->IsString()) {
+    length = String::cast(*receiver)->length();
+  } else {
+    length = JSObject::cast(*receiver)->elements()->length();
+  }
+  return index >= length;
+}
+
+KeyedAccessLoadMode GetLoadMode(Handle<Object> receiver, uint32_t index) {
+  if (receiver->IsString() && IsOutOfBoundsAccess(receiver, index)) {
+    Isolate* isolate = Handle<String>::cast(receiver)->GetIsolate();
+    if (isolate->IsFastArrayConstructorPrototypeChainIntact()) {
+      return LOAD_IGNORE_OUT_OF_BOUNDS;
+    }
+  }
+  return STANDARD_LOAD;
+}
+
+}  // namespace
+
 MaybeHandle<Object> KeyedLoadIC::Load(Handle<Object> object,
                                       Handle<Object> key) {
   if (MigrateDeprecated(object)) {
@@ -1144,9 +1192,10 @@ MaybeHandle<Object> KeyedLoadIC::Load(Handle<Object> object,
                                Object);
   } else if (FLAG_use_ic && !object->IsAccessCheckNeeded() &&
              !object->IsJSValue()) {
-    if ((object->IsJSReceiver() && key->IsSmi()) ||
-        (object->IsString() && key->IsNumber())) {
-      UpdateLoadElement(Handle<HeapObject>::cast(object));
+    if ((object->IsJSReceiver() || object->IsString()) &&
+        key->ToArrayIndex(&index)) {
+      KeyedAccessLoadMode load_mode = GetLoadMode(object, index);
+      UpdateLoadElement(Handle<HeapObject>::cast(object), load_mode);
       if (is_vector_set()) {
         TRACE_IC("LoadIC", key);
       }
@@ -1812,16 +1861,6 @@ void KeyedStoreIC::StoreElementPolymorphicHandlers(
   }
 }
 
-bool IsOutOfBoundsAccess(Handle<JSObject> receiver, uint32_t index) {
-  uint32_t length = 0;
-  if (receiver->IsJSArray()) {
-    JSArray::cast(*receiver)->length()->ToArrayLength(&length);
-  } else {
-    length = static_cast<uint32_t>(receiver->elements()->length());
-  }
-  return index >= length;
-}
-
 
 static KeyedAccessStoreMode GetStoreMode(Handle<JSObject> receiver,
                                          uint32_t index, Handle<Object> value) {

src/ic/ic.h

@@ -288,15 +288,18 @@ class KeyedLoadIC : public LoadIC {
 
  protected:
   // receiver is HeapObject because it could be a String or a JSObject
-  void UpdateLoadElement(Handle<HeapObject> receiver);
+  void UpdateLoadElement(Handle<HeapObject> receiver,
+                         KeyedAccessLoadMode load_mode);
 
  private:
   friend class IC;
 
-  Handle<Object> LoadElementHandler(Handle<Map> receiver_map);
+  Handle<Object> LoadElementHandler(Handle<Map> receiver_map,
+                                    KeyedAccessLoadMode load_mode);
 
   void LoadElementPolymorphicHandlers(MapHandles* receiver_maps,
-                                      ObjectHandles* handlers);
+                                      ObjectHandles* handlers,
+                                      KeyedAccessLoadMode load_mode);
 };
 
 

src/isolate.cc

@@ -3091,12 +3091,13 @@ void Isolate::InitializeVectorListFromHeap() {
   SetFeedbackVectorsForProfilingTools(*list);
 }
 
-bool Isolate::IsArrayOrObjectPrototype(Object* object) {
+bool Isolate::IsArrayOrObjectOrStringPrototype(Object* object) {
   Object* context = heap()->native_contexts_list();
   while (!context->IsUndefined(this)) {
     Context* current_context = Context::cast(context);
     if (current_context->initial_object_prototype() == object ||
-        current_context->initial_array_prototype() == object) {
+        current_context->initial_array_prototype() == object ||
+        current_context->initial_string_prototype() == object) {
       return true;
     }
     context = current_context->next_context_link();
@@ -3131,6 +3132,8 @@ bool Isolate::IsFastArrayConstructorPrototypeChainIntact() {
       native_context->get(Context::INITIAL_ARRAY_PROTOTYPE_INDEX));
   JSObject* initial_object_proto = JSObject::cast(
       native_context->get(Context::INITIAL_OBJECT_PROTOTYPE_INDEX));
+  JSObject* initial_string_proto = JSObject::cast(
+      native_context->get(Context::INITIAL_STRING_PROTOTYPE_INDEX));
 
   if (root_array_map == nullptr ||
       initial_array_proto == initial_object_proto) {
@@ -3152,22 +3155,38 @@ bool Isolate::IsFastArrayConstructorPrototypeChainIntact() {
     return cell_reports_intact;
   }
 
-  // Check that the object prototype hasn't been altered WRT empty elements.
+  // Check that the Object.prototype hasn't been altered WRT empty elements.
+  elements = initial_object_proto->elements();
+  if (elements != heap()->empty_fixed_array() &&
+      elements != heap()->empty_slow_element_dictionary()) {
+    DCHECK_EQ(false, cell_reports_intact);
+    return cell_reports_intact;
+  }
+
+  // Check that the Array.prototype has the Object.prototype as its
+  // [[Prototype]] and that the Object.prototype has a null [[Prototype]].
   PrototypeIterator iter(this, initial_array_proto);
   if (iter.IsAtEnd() || iter.GetCurrent() != initial_object_proto) {
     DCHECK_EQ(false, cell_reports_intact);
     return cell_reports_intact;
   }
+  iter.Advance();
+  if (!iter.IsAtEnd()) {
+    DCHECK_EQ(false, cell_reports_intact);
+    return cell_reports_intact;
+  }
 
-  elements = initial_object_proto->elements();
+  // Check that the String.prototype hasn't been altered WRT empty elements.
+  elements = initial_string_proto->elements();
   if (elements != heap()->empty_fixed_array() &&
       elements != heap()->empty_slow_element_dictionary()) {
     DCHECK_EQ(false, cell_reports_intact);
     return cell_reports_intact;
   }
 
-  iter.Advance();
-  if (!iter.IsAtEnd()) {
+  // Check that the String.prototype has the Object.prototype
+  // as its [[Prototype]] still.
+  if (initial_string_proto->map()->prototype() != initial_object_proto) {
     DCHECK_EQ(false, cell_reports_intact);
     return cell_reports_intact;
   }
@@ -3213,7 +3232,7 @@ void Isolate::UpdateArrayProtectorOnSetElement(Handle<JSObject> object) {
   DisallowHeapAllocation no_gc;
   if (!object->map()->is_prototype_map()) return;
   if (!IsFastArrayConstructorPrototypeChainIntact()) return;
-  if (!IsArrayOrObjectPrototype(*object)) return;
+  if (!IsArrayOrObjectOrStringPrototype(*object)) return;
   PropertyCell::SetValueWithInvalidation(
       factory()->array_protector(),
       handle(Smi::FromInt(kProtectorInvalid), this));

src/isolate.h

@@ -1352,7 +1352,7 @@ class Isolate {
 
  protected:
   explicit Isolate(bool enable_serializer);
-  bool IsArrayOrObjectPrototype(Object* object);
+  bool IsArrayOrObjectOrStringPrototype(Object* object);
 
  private:
   friend struct GlobalState;

src/objects.h

@@ -175,6 +175,11 @@ namespace internal {
 struct InliningPosition;
 class PropertyDescriptorObject;
 
+enum KeyedAccessLoadMode {
+  STANDARD_LOAD,
+  LOAD_IGNORE_OUT_OF_BOUNDS,
+};
+
 enum KeyedAccessStoreMode {
   STANDARD_STORE,
   STORE_TRANSITION_TO_OBJECT,

test/mjsunit/compiler/deopt-string-outofbounds.js

@@ -29,3 +29,27 @@ var s = "12345";
   foo(5);
   assertOptimized(foo);
 })();
+
+(function() {
+  function foo(s) { return s[5]; }
+
+  foo(s);
+  foo(s);
+  %OptimizeFunctionOnNextCall(foo);
+  foo(s);
+  %OptimizeFunctionOnNextCall(foo);
+  foo(s);
+  assertOptimized(foo);
+})();
+
+(function() {
+  function foo(s, i) { return s[i]; }
+
+  foo(s, 0);
+  foo(s, 1);
+  %OptimizeFunctionOnNextCall(foo);
+  foo(s, 5);
+  %OptimizeFunctionOnNextCall(foo);
+  foo(s, 5);
+  assertOptimized(foo);
+})();

test/mjsunit/regress/regress-7014-1.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.
+
+// Flags: --allow-natives-syntax --opt --no-always-opt
+
+function foo(s) {
+  return s[5];
+}
+
+assertEquals("f", foo("abcdef"));
+assertEquals(undefined, foo("a"));
+%OptimizeFunctionOnNextCall(foo);
+assertEquals("f", foo("abcdef"));
+assertEquals(undefined, foo("a"));
+assertOptimized(foo);
+
+// Now mess with the String.prototype.
+String.prototype[5] = "5";
+
+assertEquals("f", foo("abcdef"));
+assertEquals("5", foo("a"));
+%OptimizeFunctionOnNextCall(foo);
+assertEquals("f", foo("abcdef"));
+assertEquals("5", foo("a"));
+assertOptimized(foo);

test/mjsunit/regress/regress-7014-2.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.
+
+// Flags: --allow-natives-syntax --opt --no-always-opt
+
+function foo(s) {
+  return s[5];
+}
+
+assertEquals("f", foo("abcdef"));
+assertEquals(undefined, foo("a"));
+%OptimizeFunctionOnNextCall(foo);
+assertEquals("f", foo("abcdef"));
+assertEquals(undefined, foo("a"));
+assertOptimized(foo);
+
+// Now mess with the String.prototype.
+String.prototype.__proto__ = new Proxy(String.prototype.__proto__, {
+  get(target, property) {
+    return "5";
+  }
+});
+
+assertEquals("f", foo("abcdef"));
+assertEquals("5", foo("a"));
+%OptimizeFunctionOnNextCall(foo);
+assertEquals("f", foo("abcdef"));
+assertEquals("5", foo("a"));
+assertOptimized(foo);