Serializer: fix false negative in hashmap lookups.

If we use HashMap::Lookup with insert=true, the returned entry may have
NULL as value. This could either mean that the value is 0, or that the
entry has just been inserted. This ambiguity can cause false negatives
in PartialCacheIndexMap::LookupOrInsert.

Also fix a TODO.

R=vogelheim@chromium.org

Review URL: https://codereview.chromium.org/974273002

Cr-Commit-Position: refs/heads/master@{#26994}
This commit is contained in:
yangguo 2015-03-04 07:36:02 -08:00 committed by Commit bot
parent 8d2e45669f
commit dbecf20d65
2 changed files with 10 additions and 12 deletions

View File

@ -480,16 +480,14 @@ ExternalReferenceDecoder::~ExternalReferenceDecoder() {
RootIndexMap::RootIndexMap(Isolate* isolate) {
map_ = new HashMap(HashMap::PointersMatch);
Object** root_array = isolate->heap()->roots_array_start();
for (int i = 0; i < Heap::kStrongRootListLength; i++) {
for (uint32_t i = 0; i < Heap::kStrongRootListLength; i++) {
Object* root = root_array[i];
if (root->IsHeapObject() && !isolate->heap()->InNewSpace(root)) {
HeapObject* heap_object = HeapObject::cast(root);
if (LookupEntry(map_, heap_object, false) != NULL) {
// Some root values are initialized to the empty FixedArray();
// Do not add them to the map.
// TODO(yangguo): This assert is not true. Some roots like
// instanceof_cache_answer can be e.g. null.
// DCHECK_EQ(isolate->heap()->empty_fixed_array(), heap_object);
HashMap::Entry* entry = LookupEntry(map_, heap_object, false);
if (entry != NULL) {
// Some are initialized to a previous value in the root list.
DCHECK_LT(GetValue(entry), i);
} else {
SetValue(LookupEntry(map_, heap_object, true), i);
}

View File

@ -150,7 +150,7 @@ class AddressMapBase {
return static_cast<uint32_t>(reinterpret_cast<intptr_t>(entry->value));
}
static HashMap::Entry* LookupEntry(HashMap* map, HeapObject* obj,
inline static HashMap::Entry* LookupEntry(HashMap* map, HeapObject* obj,
bool insert) {
return map->Lookup(Key(obj), Hash(obj), insert);
}
@ -195,9 +195,9 @@ class PartialCacheIndexMap : public AddressMapBase {
// Lookup object in the map. Return its index if found, or create
// a new entry with new_index as value, and return kInvalidIndex.
int LookupOrInsert(HeapObject* obj, int new_index) {
HashMap::Entry* entry = LookupEntry(&map_, obj, true);
if (entry->value != NULL) return GetValue(entry);
SetValue(entry, static_cast<uint32_t>(new_index));
HashMap::Entry* entry = LookupEntry(&map_, obj, false);
if (entry != NULL) return GetValue(entry);
SetValue(LookupEntry(&map_, obj, true), static_cast<uint32_t>(new_index));
return kInvalidIndex;
}