Correct handling of arrays with callbacks in the prototype chain.

src/objects.cc

 // Copyright 2013 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 2930 matching lines @@
   bool has_pending_exception;
   Handle<Object> argv[] = { value };
   Execution::Call(
       isolate, setter, object, ARRAY_SIZE(argv), argv, &has_pending_exception);
   // Check for pending exception and return the result.
   if (has_pending_exception) return Handle<Object>();
   return value;
 }
 
 
+bool Map::MayHaveIndexedCallbacksInPrototypeChain() {
+  Heap* heap = GetHeap();
+
+  if (has_element_callbacks()) {
+    return true;
+  }
+
+  for (Object* pt = prototype();

[danno, 2013/10/30 12:17:45]

don't use the abbreviation, use the variable name prototype and call the
function by specifying this->prototype()

[mvstanton, 2013/10/30 18:22:28]

Done.

+       pt != heap->null_value();
+       pt = pt->GetPrototype(GetIsolate())) {
+    if (pt->IsJSProxy()) {
+      // Be conservative, don't walk into proxies.
+      return true;
+    }
+
+    if (JSObject::cast(pt)->map()->has_element_callbacks()) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
+
 MaybeObject* JSObject::SetElementWithCallbackSetterInPrototypes(
     uint32_t index,
     Object* value,
     bool* found,
     StrictModeFlag strict_mode) {
   Heap* heap = GetHeap();
   for (Object* pt = GetPrototype();
        pt != heap->null_value();
        pt = pt->GetPrototype(GetIsolate())) {
     if (pt->IsJSProxy()) {
@@ skipping 1510 matching lines @@
 
 
 Handle<Map> NormalizedMapCache::Get(Handle<NormalizedMapCache> cache,
                                     Handle<JSObject> obj,
                                     PropertyNormalizationMode mode) {
   int index = obj->map()->Hash() % kEntries;
   Handle<Object> result = handle(cache->get(index), cache->GetIsolate());
   if (result->IsMap() &&
       Handle<Map>::cast(result)->EquivalentToForNormalization(obj->map(),
                                                               mode)) {
+    Handle<Map> result_map = Handle<Map>::cast(result);
 #ifdef VERIFY_HEAP
     if (FLAG_verify_heap) {
-      Handle<Map>::cast(result)->SharedMapVerify();
+      result_map->SharedMapVerify();
     }
 #endif
 #ifdef ENABLE_SLOW_ASSERTS
     if (FLAG_enable_slow_asserts) {
       // The cached map should match newly created normalized map bit-by-bit,
       // except for the code cache, which can contain some ics which can be
       // applied to the shared map.
       Handle<Map> fresh = Map::CopyNormalized(handle(obj->map()), mode,
                                               SHARED_NORMALIZED_MAP);
 
       ASSERT(memcmp(fresh->address(),
-                    Handle<Map>::cast(result)->address(),
-                    Map::kCodeCacheOffset) == 0);
+                    result_map->address(),
+                    Map::kTransitionsOrBackPointerOffset) == 0);
       STATIC_ASSERT(Map::kDependentCodeOffset ==
-                    Map::kCodeCacheOffset + kPointerSize);
-      int offset = Map::kDependentCodeOffset + kPointerSize;
-      ASSERT(memcmp(fresh->address() + offset,
-                    Handle<Map>::cast(result)->address() + offset,
-                    Map::kSize - offset) == 0);
+                    Map::kTransitionsOrBackPointerOffset + 3 * kPointerSize);
+
+      // The final fields to look at are flag fields 3 and 4. Field 3

[danno, 2013/10/30 12:17:45]

Is the logic needed? I thought the conclusion was other than the transitions
array, everything in this slow assert logic is OK. In that case, it's probably
better/safer to just use the original memcmp, since that will automatically
catch problems with new fields that are added to the Map without having to
explicitly add them in the code here.

+      // needs special attention.
+      STATIC_ASSERT(Map::kDependentCodeOffset + kPointerSize ==
+                    Map::kBitField3Offset);
+      STATIC_ASSERT(Map::kSize - (2 * kPointerSize) == Map::kBitField3Offset);
+      STATIC_ASSERT(Map::kSize - kPointerSize == Map::kBitField4Offset);
+
+      int mask = Map::EnumLengthBits::kMask |
+          Map::NumberOfOwnDescriptorsBits::kMask |
+          Map::IsShared::kMask |
+          Map::FunctionWithPrototype::kMask |
+          Map::DictionaryMap::kMask |
+          // OwnsDescriptors::kMask |
+          Map::IsObserved::kMask |
+          Map::Deprecated::kMask |
+          Map::IsFrozen::kMask |
+          Map::IsUnstable::kMask |
+          Map::IsMigrationTarget::kMask;
+      int fresh_field3 = fresh->bit_field3() & mask;
+      int result_field3 = result_map->bit_field3() & mask;
+      ASSERT(fresh_field3 == result_field3);
+      ASSERT(fresh->bit_field4() == result_map->bit_field4());
     }
 #endif
-    return Handle<Map>::cast(result);
+    return result_map;
   }
 
   Isolate* isolate = cache->GetIsolate();
   Handle<Map> map = Map::CopyNormalized(handle(obj->map()), mode,
                                         SHARED_NORMALIZED_MAP);
   ASSERT(map->is_dictionary_map());
   cache->set(index, *map);
   isolate->counters()->normalized_maps()->Increment();
 
   return map;
@@ skipping 180 matching lines @@
 
 Handle<SeededNumberDictionary> JSObject::NormalizeElements(
     Handle<JSObject> object) {
   CALL_HEAP_FUNCTION(object->GetIsolate(),
                      object->NormalizeElements(),
                      SeededNumberDictionary);
 }
 
 
 MaybeObject* JSObject::NormalizeElements() {
-  ASSERT(!HasExternalArrayElements());
-
-  // Find the backing store.
-  FixedArrayBase* array = FixedArrayBase::cast(elements());
-  Map* old_map = array->map();
-  bool is_arguments =
-      (old_map == old_map->GetHeap()->non_strict_arguments_elements_map());
-  if (is_arguments) {
-    array = FixedArrayBase::cast(FixedArray::cast(array)->get(1));
-  }
+  SeededNumberDictionary* dictionary;
+  FixedArrayBase* array = GetElements();

[danno, 2013/10/30 12:17:45]

I'd call this GetElementBackingStore(), and you might want to find the places in
the code that can use it (look for "get(1)" in this file).

   if (array->IsDictionary()) return array;
 
-  ASSERT(HasFastSmiOrObjectElements() ||
-         HasFastDoubleElements() ||
-         HasFastArgumentsElements());
-  // Compute the effective length and allocate a new backing store.
-  int length = IsJSArray()
-      ? Smi::cast(JSArray::cast(this)->length())->value()
-      : array->length();
-  int old_capacity = 0;
-  int used_elements = 0;
-  GetElementsCapacityAndUsage(&old_capacity, &used_elements);
-  SeededNumberDictionary* dictionary;
-  MaybeObject* maybe_dictionary =
-      SeededNumberDictionary::Allocate(GetHeap(), used_elements);
-  if (!maybe_dictionary->To(&dictionary)) return maybe_dictionary;
-
-  maybe_dictionary = CopyFastElementsToDictionary(
-      GetIsolate(), array, length, dictionary);
+  MaybeObject* maybe_dictionary = CreateNormalizedElements();
   if (!maybe_dictionary->To(&dictionary)) return maybe_dictionary;
 
   // Switch to using the dictionary as the backing storage for elements.
-  if (is_arguments) {
+  if (HasNonStrictArgumentsElementsMap()) {
     FixedArray::cast(elements())->set(1, dictionary);
   } else {
     // Set the new map first to satify the elements type assert in
     // set_elements().
     Map* new_map;
     MaybeObject* maybe = GetElementsTransitionMap(GetIsolate(),
                                                   DICTIONARY_ELEMENTS);
     if (!maybe->To(&new_map)) return maybe;
     set_map(new_map);
     set_elements(dictionary);
   }
 
-  old_map->GetHeap()->isolate()->counters()->elements_to_dictionary()->
-      Increment();
+  GetHeap()->isolate()->counters()->elements_to_dictionary()->Increment();
 
 #ifdef DEBUG
   if (FLAG_trace_normalization) {
     PrintF("Object elements have been normalized:\n");
     Print();
   }
 #endif
 
   ASSERT(HasDictionaryElements() || HasDictionaryArgumentsElements());
   return dictionary;
 }
 
 
+Handle<SeededNumberDictionary> JSObject::CreateNormalizedElements(
+    Handle<JSObject> object) {
+  CALL_HEAP_FUNCTION(object->GetIsolate(),
+                     object->CreateNormalizedElements(),
+                     SeededNumberDictionary);
+}
+
+
+MaybeObject* JSObject::CreateNormalizedElements() {
+  ASSERT(!HasExternalArrayElements());
+
+  // Find the backing store.
+  FixedArrayBase* array = GetElements();
+  ASSERT(!array->IsDictionary());
+
+  ASSERT(HasFastSmiOrObjectElements() ||
+         HasFastDoubleElements() ||
+         HasFastArgumentsElements());
+  // Compute the effective length and allocate a new backing store.
+  int length = IsJSArray()
+      ? Smi::cast(JSArray::cast(this)->length())->value()
+      : array->length();
+  int old_capacity = 0;
+  int used_elements = 0;
+  GetElementsCapacityAndUsage(&old_capacity, &used_elements);
+  SeededNumberDictionary* dictionary;
+  MaybeObject* maybe_dictionary =
+      SeededNumberDictionary::Allocate(GetHeap(), used_elements);
+  if (!maybe_dictionary->To(&dictionary)) return maybe_dictionary;
+
+  maybe_dictionary = CopyFastElementsToDictionary(
+      GetIsolate(), array, length, dictionary);
+  if (!maybe_dictionary->To(&dictionary)) return maybe_dictionary;
+
+  return dictionary;
+}
+
+
 Smi* JSReceiver::GenerateIdentityHash() {
   Isolate* isolate = GetIsolate();
 
   int hash_value;
   int attempts = 0;
   do {
     // Generate a random 32-bit hash value but limit range to fit
     // within a smi.
     hash_value = isolate->random_number_generator()->NextInt() & Smi::kMaxValue;
     attempts++;
@@ skipping 782 matching lines @@
     }
   }
 
   LookupResult result(isolate);
   Handle<Map> old_map(object->map());
   old_map->LookupTransition(*object, isolate->heap()->frozen_symbol(), &result);
   if (result.IsTransition()) {
     Map* transition_map = result.GetTransitionTarget();
     ASSERT(transition_map->has_dictionary_elements());
     ASSERT(transition_map->is_frozen());
-    ASSERT(!transition_map->is_extensible());
+     ASSERT(!transition_map->is_extensible());

[danno, 2013/10/30 12:17:45]

nit: remove stray whitespace change.

[mvstanton, 2013/10/30 18:22:28]

Done.

     object->set_map(transition_map);
   } else if (object->HasFastProperties() && old_map->CanHaveMoreTransitions()) {
     // Create a new descriptor array with fully-frozen properties
     int num_descriptors = old_map->NumberOfOwnDescriptors();
     Handle<DescriptorArray> new_descriptors =
         DescriptorArray::CopyUpToAddAttributes(
             handle(old_map->instance_descriptors()), num_descriptors, FROZEN);
     Handle<Map> new_map = Map::CopyReplaceDescriptors(
         old_map, new_descriptors, INSERT_TRANSITION,
         isolate->factory()->frozen_symbol());
@@ skipping 640 matching lines @@
     }
   }
   return true;
 }
 
 
 void JSObject::SetElementCallback(Handle<JSObject> object,
                                   uint32_t index,
                                   Handle<Object> structure,
                                   PropertyAttributes attributes) {
+  Isolate* isolate = object->GetIsolate();
   Heap* heap = object->GetHeap();
   PropertyDetails details = PropertyDetails(attributes, CALLBACKS, 0);
+  bool is_arguments = object->HasNonStrictArgumentsElements();
 
-  // Normalize elements to make this operation simple.
-  Handle<SeededNumberDictionary> dictionary = NormalizeElements(object);
+  // Do we need to create a new dictionary?
+  Handle<SeededNumberDictionary> new_element_dictionary;

[danno, 2013/10/30 12:17:45]

I think NormalizeElements will do the right thing without all of this extra
logic, including gcreating a transition from the fast elements kind map to the
new dictionary map if you need one, or just leaving the object in dictionary
mode and not changing the map if you are already here.

[mvstanton, 2013/10/30 18:22:28]

Yep, I thought combining the move to dictionary and setting the
HasElementCallbacks bit in one transition would be better, but it sure does
simplify things if I let go of that!

+  FixedArrayBase* array = object->GetElements();
+  if (!array->IsDictionary()) {
+    new_element_dictionary = CreateNormalizedElements(object);
+  }
+
+  if (!object->map()->has_element_callbacks()) {
+    LookupResult result(isolate);
+    Handle<Map> old_map(object->map());
+    old_map->LookupTransition(*object,

[danno, 2013/10/30 12:17:45]

It looks like JSObject::SetObserve does almost exactly the same logic as you
have here. Maybe it makes sense to have a SetHasElementsCallback or
PrepareForSetElementCallback that works just like SetObserve (including the
logic to to ignore external array elements, e.g. TypedArrays). Sorry, I should
have found that code for your sooner.

[mvstanton, 2013/10/30 18:22:28]

addressed.

+                              heap->element_callbacks_symbol(),
+                              &result);
+    if (result.IsTransition()) {
+      object->set_map(result.GetTransitionTarget());
+    } else {
+      Handle<Map> new_map = Map::CopyAsElementCallbacksTransition(old_map,
+                                                             !is_arguments);
+      object->set_map(*new_map);
+    }
+
+    ASSERT(object->map()->has_element_callbacks());
+
+    // Install the dictionary if required without doing an extra transition.
+    if (!new_element_dictionary.is_null()) {
+      if (is_arguments) {
+        FixedArray::cast(object->elements())->set(1, *new_element_dictionary);
+      } else {
+        object->set_elements(*new_element_dictionary);
+      }
+    }
+
+    // KeyedStoreICs (at least the non-generic ones) need a reset.
+    // I *could* look at the maps and only reset the poly/mono ones that

[danno, 2013/10/30 12:17:45]

nit: we try to avoid using "I" or "we" in comments. Passive voice is your enemy
in writing a novel, but your friend here :-p

[mvstanton, 2013/10/30 18:22:28]

:) done.

+    // depend on a map in this prototype chain?
+    heap->ClearAllICsByKind(Code::KEYED_STORE_IC);
+  } else {
+    // If our map is already flagged for element callbacks, then we should
+    // not have had to create a new element dictionary.
+    ASSERT(new_element_dictionary.is_null());
+  }
+
+  Handle<SeededNumberDictionary> dictionary = Handle<SeededNumberDictionary>(
+      SeededNumberDictionary::cast(object->GetElements()), isolate);
+
   ASSERT(object->HasDictionaryElements() ||
          object->HasDictionaryArgumentsElements());
 
   // Update the dictionary with the new CALLBACKS property.
   dictionary = SeededNumberDictionary::Set(dictionary, index, structure,
                                            details);
   dictionary->set_requires_slow_elements();
 
   // Update the dictionary backing store on the object.
-  if (object->elements()->map() == heap->non_strict_arguments_elements_map()) {
+  if (is_arguments) {
     // Also delete any parameter alias.
     //
     // TODO(kmillikin): when deleting the last parameter alias we could
     // switch to a direct backing store without the parameter map.  This
     // would allow GC of the context.
     FixedArray* parameter_map = FixedArray::cast(object->elements());
     if (index < static_cast<uint32_t>(parameter_map->length()) - 2) {
       parameter_map->set(index + 2, heap->the_hole_value());
     }
     parameter_map->set(1, *dictionary);
@@ skipping 411 matching lines @@
   result->set_constructor(constructor());
   result->set_bit_field(bit_field());
   result->set_bit_field2(bit_field2());
   int new_bit_field3 = bit_field3();
   new_bit_field3 = OwnsDescriptors::update(new_bit_field3, true);
   new_bit_field3 = NumberOfOwnDescriptorsBits::update(new_bit_field3, 0);
   new_bit_field3 = EnumLengthBits::update(new_bit_field3, kInvalidEnumCache);
   new_bit_field3 = Deprecated::update(new_bit_field3, false);
   new_bit_field3 = IsUnstable::update(new_bit_field3, false);
   result->set_bit_field3(new_bit_field3);
+  result->set_bit_field4(bit_field4());
   return result;
 }
 
 
 Handle<Map> Map::CopyNormalized(Handle<Map> map,
                                 PropertyNormalizationMode mode,
                                 NormalizedMapSharingMode sharing) {
   int new_instance_size = map->instance_size();
   if (mode == CLEAR_INOBJECT_PROPERTIES) {
     new_instance_size -= map->inobject_properties() * kPointerSize;
@@ skipping 231 matching lines @@
   if (insert_transition) {
     MaybeObject* added_elements = set_elements_transition_map(new_map);
     if (added_elements->IsFailure()) return added_elements;
     new_map->SetBackPointer(this);
   }
 
   return new_map;
 }
 
 
+MaybeObject* Map::CopyAsElementCallbacksTransition(

[danno, 2013/10/30 12:17:45]

I think with a little work, you can 100% share this code with CopyForObserved,
refactoring to call it CopyForTransitionedMap, and just pustting the logic to
set either "set_is_observed" or "set_has_elements_callback" either in wrapper
functions CopyForObserved/CopyForElementsCallback, or just directly in
SetObserved/SetElementCallback.

[mvstanton, 2013/10/30 18:22:28]

I this this in a basic way, with a protected function that does a "meaningless"
transition, then the caller can set a map bit on the new map. Let me know if it
should be fancier (there could be a macro with a code snippet, for example).

+    bool with_dictionary_elements) {
+  ASSERT(!HasElementCallbacksTransition());
+
+  if (owns_descriptors()) {
+    // In case the map owned its own descriptors, share the descriptors and
+    // transfer ownership to the new map.
+    Map* new_map;
+    MaybeObject* maybe_new_map = CopyDropDescriptors();
+    if (!maybe_new_map->To(&new_map)) return maybe_new_map;
+
+    MaybeObject* added_elements = set_element_callbacks_map(new_map);
+    if (added_elements->IsFailure()) return added_elements;
+
+    new_map->InitializeDescriptors(instance_descriptors());
+    new_map->set_has_element_callbacks(true);
+    if (with_dictionary_elements) {
+      new_map->set_elements_kind(DICTIONARY_ELEMENTS);
+    }
+    new_map->SetBackPointer(this);
+    set_owns_descriptors(false);
+    return new_map;
+  }
+
+  // In case the map did not own its own descriptors, a split is forced by
+  // copying the map; creating a new descriptor array cell.
+  // Create a new free-floating map only if we are not allowed to store it.
+  Map* new_map;
+  MaybeObject* maybe_new_map = Copy();
+  if (!maybe_new_map->To(&new_map)) return maybe_new_map;
+  new_map->set_has_element_callbacks(true);
+  if (with_dictionary_elements) {
+    new_map->set_elements_kind(DICTIONARY_ELEMENTS);
+  }
+
+  MaybeObject* added_elements = set_element_callbacks_map(new_map);
+  if (added_elements->IsFailure()) return added_elements;
+  new_map->SetBackPointer(this);
+
+  return new_map;
+}
+
+
+Handle<Map> Map::CopyAsElementCallbacksTransition(
+    Handle<Map> map,
+    bool with_dictionary_elements) {
+  Isolate* isolate = map->GetIsolate();
+  CALL_HEAP_FUNCTION(isolate,
+                     map->CopyAsElementCallbacksTransition(
+                         with_dictionary_elements),
+                     Map);
+}
+
+
 Handle<Map> Map::CopyForObserved(Handle<Map> map) {
   ASSERT(!map->is_observed());
 
   Isolate* isolate = map->GetIsolate();
 
   // In case the map owned its own descriptors, share the descriptors and
   // transfer ownership to the new map.
   Handle<Map> new_map;
   if (map->owns_descriptors()) {
     new_map = Map::CopyDropDescriptors(map);
@@ skipping 2488 matching lines @@
 }
 
 
 static bool CheckEquivalent(Map* first, Map* second) {
   return
     first->constructor() == second->constructor() &&
     first->prototype() == second->prototype() &&
     first->instance_type() == second->instance_type() &&
     first->bit_field() == second->bit_field() &&
     first->bit_field2() == second->bit_field2() &&
+    first->bit_field4() == second->bit_field4() &&
     first->is_observed() == second->is_observed() &&
     first->function_with_prototype() == second->function_with_prototype();
 }
 
 
 bool Map::EquivalentToForTransition(Map* other) {
   return CheckEquivalent(this, other);
 }
 
 
@@ skipping 1114 matching lines @@
     if (--n == 0) {
       info->set_target_cell(replace_with);
       return;
     }
   }
   UNREACHABLE();
 }
 
 
 void Code::ClearInlineCaches() {
+  ClearInlineCaches(NULL);
+}
+
+
+void Code::ClearInlineCaches(Code::Kind kind) {
+  ClearInlineCaches(&kind);
+}
+
+
+void Code::ClearInlineCaches(Code::Kind* kind) {
   int mask = RelocInfo::ModeMask(RelocInfo::CODE_TARGET) |
              RelocInfo::ModeMask(RelocInfo::CONSTRUCT_CALL) |
              RelocInfo::ModeMask(RelocInfo::CODE_TARGET_WITH_ID) |
              RelocInfo::ModeMask(RelocInfo::CODE_TARGET_CONTEXT);
   for (RelocIterator it(this, mask); !it.done(); it.next()) {
     RelocInfo* info = it.rinfo();
     Code* target(Code::GetCodeFromTargetAddress(info->target_address()));
     if (target->is_inline_cache_stub()) {
-      IC::Clear(this->GetIsolate(), info->pc());
+      if (kind == NULL || *kind == target->kind()) {
+        IC::Clear(this->GetIsolate(), info->pc());
+      }
     }
   }
 }
 
 
 void Code::ClearTypeFeedbackCells(Heap* heap) {
   if (kind() != FUNCTION) return;
   Object* raw_info = type_feedback_info();
   if (raw_info->IsTypeFeedbackInfo()) {
     TypeFeedbackCells* type_feedback_cells =
@@ skipping 1152 matching lines @@
        pt = pt->GetPrototype(isolate)) {
     if (JSReceiver::cast(pt) == *object) {
       // Cycle detected.
       Handle<Object> error = isolate->factory()->NewError(
           "cyclic_proto", HandleVector<Object>(NULL, 0));
       isolate->Throw(*error);
       return Handle<Object>();
     }
   }
 
+  bool has_element_callbacks_in_chain =
+      object->map()->MayHaveIndexedCallbacksInPrototypeChain();
   Handle<JSObject> real_receiver = object;
 
   if (skip_hidden_prototypes) {
     // Find the first object in the chain whose prototype object is not
     // hidden and set the new prototype on that object.
     Object* current_proto = real_receiver->GetPrototype();
     while (current_proto->IsJSObject() &&
           JSObject::cast(current_proto)->map()->is_hidden_prototype()) {
       real_receiver = handle(JSObject::cast(current_proto), isolate);
       current_proto = current_proto->GetPrototype(isolate);
@@ skipping 12 matching lines @@
 
   Handle<Map> new_map = Map::GetPrototypeTransition(map, value);
   if (new_map.is_null()) {
     new_map = Map::Copy(map);
     Map::PutPrototypeTransition(map, value, new_map);
     new_map->set_prototype(*value);
   }
   ASSERT(new_map->prototype() == *value);
   real_receiver->set_map(*new_map);
 
+  if (!has_element_callbacks_in_chain &&
+      new_map->MayHaveIndexedCallbacksInPrototypeChain()) {
+    // If our prototype chain didn't have element callbacks, and now we do
+    // have them, then we need to clear KeyedStoreICs.
+    object->GetHeap()->ClearAllICsByKind(Code::KEYED_STORE_IC);
+  }
+
   heap->ClearInstanceofCache();
   ASSERT(size == object->Size());
   return value;
 }
 
 
 MaybeObject* JSObject::EnsureCanContainElements(Arguments* args,
                                                 uint32_t first_arg,
                                                 uint32_t arg_count,
                                                 EnsureElementsMode mode) {
@@ skipping 969 matching lines @@
 MaybeObject* JSObject::TransitionElementsKind(ElementsKind to_kind) {
   ASSERT(!map()->is_observed());
   ElementsKind from_kind = map()->elements_kind();
 
   if (IsFastHoleyElementsKind(from_kind)) {
     to_kind = GetHoleyElementsKind(to_kind);
   }
 
   if (from_kind == to_kind) return this;
 
-  MaybeObject* maybe_failure = UpdateAllocationSite(to_kind);
-  if (maybe_failure->IsFailure()) return maybe_failure;
+  // Don't update the site if to_kind isn't fast
+  if (IsFastElementsKind(to_kind)) {
+    MaybeObject* maybe_failure = UpdateAllocationSite(to_kind);
+    if (maybe_failure->IsFailure()) return maybe_failure;
+  }
 
   Isolate* isolate = GetIsolate();
   if (elements() == isolate->heap()->empty_fixed_array() ||
       (IsFastSmiOrObjectElementsKind(from_kind) &&
        IsFastSmiOrObjectElementsKind(to_kind)) ||
       (from_kind == FAST_DOUBLE_ELEMENTS &&
        to_kind == FAST_HOLEY_DOUBLE_ELEMENTS)) {
     ASSERT(from_kind != TERMINAL_FAST_ELEMENTS_KIND);
     // No change is needed to the elements() buffer, the transition
     // only requires a map change.
@@ skipping 3616 matching lines @@
 #define ERROR_MESSAGES_TEXTS(C, T) T,
   static const char* error_messages_[] = {
       ERROR_MESSAGES_LIST(ERROR_MESSAGES_TEXTS)
   };
 #undef ERROR_MESSAGES_TEXTS
   return error_messages_[reason];
 }
 
 
 } }  // namespace v8::internal