Fix identity hash code function to respect flag.

The flag passed to JSObject::GetIdentityHash() was not respected so far
and an indentity hash code was generated even when the flag requested
not to do so. This could lead to a rare corner cases (for which a test
case was added) where a GC request would have been dropped.

R=rossberg@chromium.org
TEST=cctest/test-dictionary

Review URL: http://codereview.chromium.org/8390047

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@9801 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/objects.cc

@@ -3589,6 +3589,9 @@ MaybeObject* JSObject::GetIdentityHash(CreationFlag flag) {
   Object* stored_value = GetHiddenProperty(GetHeap()->identity_hash_symbol());
   if (stored_value->IsSmi()) return stored_value;
 
+  // Do not generate permanent identity hash code if not requested.
+  if (flag == OMIT_CREATION) return GetHeap()->undefined_value();
+
   Smi* hash = GenerateIdentityHash();
   MaybeObject* result = SetHiddenProperty(GetHeap()->identity_hash_symbol(),
                                           hash);
@@ -12390,7 +12393,7 @@ MaybeObject* StringDictionary::TransformPropertiesToFastFor(
 bool ObjectHashSet::Contains(Object* key) {
   // If the object does not have an identity hash, it was never used as a key.
   { MaybeObject* maybe_hash = key->GetHash(OMIT_CREATION);
-    if (maybe_hash->IsFailure()) return false;
+    if (maybe_hash->ToObjectUnchecked()->IsUndefined()) return false;
   }
   return (FindEntry(key) != kNotFound);
 }
@@ -12424,7 +12427,7 @@ MaybeObject* ObjectHashSet::Add(Object* key) {
 MaybeObject* ObjectHashSet::Remove(Object* key) {
   // If the object does not have an identity hash, it was never used as a key.
   { MaybeObject* maybe_hash = key->GetHash(OMIT_CREATION);
-    if (maybe_hash->IsFailure()) return this;
+    if (maybe_hash->ToObjectUnchecked()->IsUndefined()) return this;
   }
   int entry = FindEntry(key);
 
@@ -12441,7 +12444,9 @@ MaybeObject* ObjectHashSet::Remove(Object* key) {
 Object* ObjectHashTable::Lookup(Object* key) {
   // If the object does not have an identity hash, it was never used as a key.
   { MaybeObject* maybe_hash = key->GetHash(OMIT_CREATION);
-    if (maybe_hash->IsFailure()) GetHeap()->undefined_value();
+    if (maybe_hash->ToObjectUnchecked()->IsUndefined()) {
+      return GetHeap()->undefined_value();
+    }
   }
   int entry = FindEntry(key);
   if (entry == kNotFound) return GetHeap()->undefined_value();

test/cctest/test-dictionary.cc

@@ -38,6 +38,7 @@
 
 using namespace v8::internal;
 
+
 TEST(ObjectHashTable) {
   v8::HandleScope scope;
   LocalContext context;
@@ -66,7 +67,8 @@ TEST(ObjectHashTable) {
   CHECK_EQ(table->NumberOfDeletedElements(), 1);
   CHECK_EQ(table->Lookup(*a), HEAP->undefined_value());
 
-  // Keys should map back to their respective values.
+  // Keys should map back to their respective values and also should get
+  // an identity hash code generated.
   for (int i = 0; i < 100; i++) {
     Handle<JSObject> key = FACTORY->NewJSArray(7);
     Handle<JSObject> value = FACTORY->NewJSArray(11);
@@ -74,12 +76,67 @@ TEST(ObjectHashTable) {
     CHECK_EQ(table->NumberOfElements(), i + 1);
     CHECK_NE(table->FindEntry(*key), ObjectHashTable::kNotFound);
     CHECK_EQ(table->Lookup(*key), *value);
+    CHECK(key->GetIdentityHash(OMIT_CREATION)->ToObjectChecked()->IsSmi());
+  }
+
+  // Keys never added to the map which already have an identity hash
+  // code should not be found.
+  for (int i = 0; i < 100; i++) {
+    Handle<JSObject> key = FACTORY->NewJSArray(7);
+    CHECK(key->GetIdentityHash(ALLOW_CREATION)->ToObjectChecked()->IsSmi());
+    CHECK_EQ(table->FindEntry(*key), ObjectHashTable::kNotFound);
+    CHECK_EQ(table->Lookup(*key), HEAP->undefined_value());
+    CHECK(key->GetIdentityHash(OMIT_CREATION)->ToObjectChecked()->IsSmi());
   }
 
-  // Keys never added to the map should not be found.
-  for (int i = 0; i < 1000; i++) {
-    Handle<JSObject> o = FACTORY->NewJSArray(100);
-    CHECK_EQ(table->FindEntry(*o), ObjectHashTable::kNotFound);
-    CHECK_EQ(table->Lookup(*o), HEAP->undefined_value());
+  // Keys that don't have an identity hash should not be found and also
+  // should not get an identity hash code generated.
+  for (int i = 0; i < 100; i++) {
+    Handle<JSObject> key = FACTORY->NewJSArray(7);
+    CHECK_EQ(table->Lookup(*key), HEAP->undefined_value());
+    CHECK_EQ(key->GetIdentityHash(OMIT_CREATION), HEAP->undefined_value());
   }
 }
+
+
+#ifdef DEBUG
+TEST(ObjectHashSetCausesGC) {
+  v8::HandleScope scope;
+  LocalContext context;
+  Handle<ObjectHashSet> table = FACTORY->NewObjectHashSet(1);
+  Handle<JSObject> key = FACTORY->NewJSArray(0);
+
+  // Simulate a full heap so that generating an identity hash code
+  // in subsequent calls will request GC.
+  FLAG_gc_interval = 0;
+
+  // Calling Contains() should not cause GC ever.
+  CHECK(!table->Contains(*key));
+
+  // Calling Remove() should not cause GC ever.
+  CHECK(!table->Remove(*key)->IsFailure());
+
+  // Calling Add() should request GC by returning a failure.
+  CHECK(table->Add(*key)->IsRetryAfterGC());
+}
+#endif
+
+
+#ifdef DEBUG
+TEST(ObjectHashTableCausesGC) {
+  v8::HandleScope scope;
+  LocalContext context;
+  Handle<ObjectHashTable> table = FACTORY->NewObjectHashTable(1);
+  Handle<JSObject> key = FACTORY->NewJSArray(0);
+
+  // Simulate a full heap so that generating an identity hash code
+  // in subsequent calls will request GC.
+  FLAG_gc_interval = 0;
+
+  // Calling Lookup() should not cause GC ever.
+  CHECK(table->Lookup(*key)->IsUndefined());
+
+  // Calling Put() should request GC by returning a failure.
+  CHECK(table->Put(*key, *key)->IsRetryAfterGC());
+}
+#endif