Fix deletion of hidden property with inline-stored hash.

src/objects.cc

 // Copyright 2012 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 3601 matching lines @@
   ASSERT(key->IsSymbol());
   if (IsJSGlobalProxy()) {
     // For a proxy, use the prototype as target object.
     Object* proxy_parent = GetPrototype();
     // If the proxy is detached, return undefined.
     if (proxy_parent->IsNull()) return GetHeap()->undefined_value();
     ASSERT(proxy_parent->IsJSGlobalObject());
     return JSObject::cast(proxy_parent)->SetHiddenProperty(key, value);
   }
   ASSERT(!IsJSGlobalProxy());
+  MaybeObject* hidden_lookup =
+      GetHiddenPropertiesHashTable(ONLY_RETURN_INLINE_VALUE);
+  ASSERT(!hidden_lookup->IsFailure());  // No failure when passing false as arg.

[Yang, 2012/10/22 12:35:23]

Not your fault, but what I think what I was trying to say (lost during
refactoring I guess) is that "No failure when merely fetching inline value".

[Michael Starzinger, 2012/10/22 12:45:52]

Done. As discussed offline, the whole assertion can be removed because
ToObjectUnchecked() asserts the same condition.

+  Object* inline_value = hidden_lookup->ToObjectUnchecked();
 
   // If there is no backing store yet, store the identity hash inline.
-  MaybeObject* hidden_lookup =
-      GetHiddenPropertiesHashTable(ONLY_RETURN_INLINE_VALUE);
-  ASSERT(!hidden_lookup->IsFailure());
-  Object* inline_value = hidden_lookup->ToObjectUnchecked();
-
   if (value->IsSmi() &&
       key == GetHeap()->identity_hash_symbol() &&
       (inline_value->IsUndefined() || inline_value->IsSmi())) {
     return SetHiddenPropertiesHashTable(value);
   }
 
   hidden_lookup = GetHiddenPropertiesHashTable(CREATE_NEW_IF_ABSENT);
   ObjectHashTable* hashtable;
   if (!hidden_lookup->To(&hashtable)) return hidden_lookup;
 
@@ skipping 16 matching lines @@
   ASSERT(key->IsSymbol());
   if (IsJSGlobalProxy()) {
     // For a proxy, use the prototype as target object.
     Object* proxy_parent = GetPrototype();
     // If the proxy is detached, return immediately.
     if (proxy_parent->IsNull()) return;
     ASSERT(proxy_parent->IsJSGlobalObject());
     JSObject::cast(proxy_parent)->DeleteHiddenProperty(key);
     return;
   }
+  ASSERT(!IsJSGlobalProxy());
   MaybeObject* hidden_lookup =
       GetHiddenPropertiesHashTable(ONLY_RETURN_INLINE_VALUE);
   ASSERT(!hidden_lookup->IsFailure());  // No failure when passing false as arg.

[Yang, 2012/10/22 12:35:23]

Ditto.

[Michael Starzinger, 2012/10/22 12:45:52]

Done. Likewise.

-  if (hidden_lookup->ToObjectUnchecked()->IsUndefined()) return;
+  Object* inline_value = hidden_lookup->ToObjectUnchecked();
+
   // We never delete (inline-stored) identity hashes.
-  ASSERT(!hidden_lookup->ToObjectUnchecked()->IsSmi());
+  ASSERT(key != GetHeap()->identity_hash_symbol());
+  if (inline_value->IsUndefined() || inline_value->IsSmi()) return;
 
-  ObjectHashTable* hashtable =
-      ObjectHashTable::cast(hidden_lookup->ToObjectUnchecked());
+  ObjectHashTable* hashtable = ObjectHashTable::cast(inline_value);
   MaybeObject* delete_result = hashtable->Put(key, GetHeap()->the_hole_value());
   USE(delete_result);
   ASSERT(!delete_result->IsFailure());  // Delete does not cause GC.
 }
 
 
 bool JSObject::HasHiddenProperties() {
   return GetPropertyAttributePostInterceptor(this,
                                              GetHeap()->hidden_symbol(),
                                              false) != ABSENT;
@@ skipping 9791 matching lines @@
   set_year(Smi::FromInt(year), SKIP_WRITE_BARRIER);
   set_month(Smi::FromInt(month), SKIP_WRITE_BARRIER);
   set_day(Smi::FromInt(day), SKIP_WRITE_BARRIER);
   set_weekday(Smi::FromInt(weekday), SKIP_WRITE_BARRIER);
   set_hour(Smi::FromInt(hour), SKIP_WRITE_BARRIER);
   set_min(Smi::FromInt(min), SKIP_WRITE_BARRIER);
   set_sec(Smi::FromInt(sec), SKIP_WRITE_BARRIER);
 }
 
 } }  // namespace v8::internal