Skip to content

Commit

Permalink
Bug 948516 - Assert that js::HashTable pointers and enumerators are u…
Browse files Browse the repository at this point in the history 
…sed correctly; r=luke
  • Loading branch information
Terrence Cole committed Dec 3, 2013
1 parent b2cd320 commit 2e8c6de
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 26 deletions.
129 changes: 103 additions & 26 deletions js/public/HashTable.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -756,9 +756,19 @@ class HashTable : private AllocPolicy
void nonNull() {}

Entry *entry_;
#ifdef DEBUG
const HashTable *table_;
uint32_t generation;
#endif

protected:
Ptr(Entry &entry) : entry_(&entry) {}
Ptr(Entry &entry, const HashTable &tableArg)
: entry_(&entry)
#ifdef DEBUG
, table_(&tableArg)
, generation(tableArg.generation())
#endif
{}

public:
// Leaves Ptr uninitialized.
Expand All @@ -768,13 +778,34 @@ class HashTable : private AllocPolicy
#endif
}

bool found() const { return entry_->isLive(); }
operator ConvertibleToBool() const { return found() ? &Ptr::nonNull : 0; }
bool operator==(const Ptr &rhs) const { JS_ASSERT(found() && rhs.found()); return entry_ == rhs.entry_; }
bool operator!=(const Ptr &rhs) const { return !(*this == rhs); }
bool found() const {
JS_ASSERT(generation == table_->generation());
return entry_->isLive();
}

operator ConvertibleToBool() const {
return found() ? &Ptr::nonNull : 0;
}

bool operator==(const Ptr &rhs) const {
JS_ASSERT(found() && rhs.found());
return entry_ == rhs.entry_;
}

bool operator!=(const Ptr &rhs) const {
JS_ASSERT(generation == table_->generation());
return !(*this == rhs);
}

T &operator*() const { return entry_->get(); }
T *operator->() const { return &entry_->get(); }
T &operator*() const {
JS_ASSERT(generation == table_->generation());
return entry_->get();
}

T *operator->() const {
JS_ASSERT(generation == table_->generation());
return &entry_->get();
}
};

// A Ptr that can be used to add a key after a failed lookup.
Expand All @@ -784,7 +815,10 @@ class HashTable : private AllocPolicy
HashNumber keyHash;
mozilla::DebugOnly<uint64_t> mutationCount;

AddPtr(Entry &entry, HashNumber hn) : Ptr(entry), keyHash(hn) {}
AddPtr(Entry &entry, const HashTable &tableArg, HashNumber hn)
: Ptr(entry, tableArg), keyHash(hn), mutationCount(tableArg.mutationCount)
{}

public:
// Leaves AddPtr uninitialized.
AddPtr() {}
Expand All @@ -799,32 +833,63 @@ class HashTable : private AllocPolicy
protected:
friend class HashTable;

Range(Entry *c, Entry *e) : cur(c), end(e), validEntry(true) {
Range(const HashTable &tableArg, Entry *c, Entry *e)
: cur(c)
, end(e)
#ifdef DEBUG
, table_(&tableArg)
, mutationCount(tableArg.mutationCount)
, generation(tableArg.generation())
, validEntry(true)
#endif
{
while (cur < end && !cur->isLive())
++cur;
}

Entry *cur, *end;
mozilla::DebugOnly<bool> validEntry;
#ifdef DEBUG
const HashTable *table_;
uint64_t mutationCount;
uint32_t generation;
bool validEntry;
#endif

public:
Range() : cur(nullptr), end(nullptr), validEntry(false) {}
Range()
: cur(nullptr)
, end(nullptr)
#ifdef DEBUG
, table_(nullptr)
, mutationCount(0)
, generation(0)
, validEntry(false)
#endif
{}

bool empty() const {
JS_ASSERT(generation == table_->generation());
JS_ASSERT(mutationCount == table_->mutationCount);
return cur == end;
}

T &front() const {
JS_ASSERT(validEntry);
JS_ASSERT(!empty());
JS_ASSERT(generation == table_->generation());
JS_ASSERT(mutationCount == table_->mutationCount);
return cur->get();
}

void popFront() {
JS_ASSERT(!empty());
JS_ASSERT(generation == table_->generation());
JS_ASSERT(mutationCount == table_->mutationCount);
while (++cur < end && !cur->isLive())
continue;
#ifdef DEBUG
validEntry = true;
#endif
}
};

Expand All @@ -837,17 +902,17 @@ class HashTable : private AllocPolicy
{
friend class HashTable;

HashTable &table;
HashTable &table_;
bool rekeyed;
bool removed;

/* Not copyable. */
Enum(const Enum &);
void operator=(const Enum &);
Enum(const Enum &) MOZ_DELETE;
void operator=(const Enum &) MOZ_DELETE;

public:
template<class Map> explicit
Enum(Map &map) : Range(map.all()), table(map.impl), rekeyed(false), removed(false) {}
Enum(Map &map) : Range(map.all()), table_(map.impl), rekeyed(false), removed(false) {}

// Removes the |front()| element from the table, leaving |front()|
// invalid until the next call to |popFront()|. For example:
Expand All @@ -857,18 +922,25 @@ class HashTable : private AllocPolicy
// if (e.front() == 42)
// e.removeFront();
void removeFront() {
table.remove(*this->cur);
table_.remove(*this->cur);
removed = true;
#ifdef DEBUG
this->validEntry = false;
this->mutationCount = table_.mutationCount;
#endif
}

// Removes the |front()| element and re-inserts it into the table with
// a new key at the new Lookup position. |front()| is invalid after
// this operation until the next call to |popFront()|.
void rekeyFront(const Lookup &l, const Key &k) {
table.rekeyWithoutRehash(*this->cur, l, k);
Ptr p(*this->cur, table_);
table_.rekeyWithoutRehash(p, l, k);
rekeyed = true;
#ifdef DEBUG
this->validEntry = false;
this->mutationCount = table_.mutationCount;
#endif
}

void rekeyFront(const Key &k) {
Expand All @@ -878,12 +950,12 @@ class HashTable : private AllocPolicy
// Potentially rehashes the table.
~Enum() {
if (rekeyed) {
table.gen++;
table.checkOverRemoved();
table_.gen++;
table_.checkOverRemoved();
}

if (removed)
table.compactIfUnderloaded();
table_.compactIfUnderloaded();
}
};

Expand Down Expand Up @@ -1392,7 +1464,7 @@ class HashTable : private AllocPolicy
Range all() const
{
JS_ASSERT(table);
return Range(table, table + capacity());
return Range(*this, table, table + capacity());
}

bool empty() const
Expand Down Expand Up @@ -1433,30 +1505,28 @@ class HashTable : private AllocPolicy
{
mozilla::ReentrancyGuard g(*this);
HashNumber keyHash = prepareHash(l);
return Ptr(lookup(l, keyHash, 0));
return Ptr(lookup(l, keyHash, 0), *this);
}

Ptr readonlyThreadsafeLookup(const Lookup &l) const
{
HashNumber keyHash = prepareHash(l);
return Ptr(lookup(l, keyHash, 0));
return Ptr(lookup(l, keyHash, 0), *this);
}

AddPtr lookupForAdd(const Lookup &l) const
{
mozilla::ReentrancyGuard g(*this);
HashNumber keyHash = prepareHash(l);
Entry &entry = lookup(l, keyHash, sCollisionBit);
AddPtr p(entry, keyHash);
p.mutationCount = mutationCount;
AddPtr p(entry, *this, keyHash);
return p;
}

template <class U>
bool add(AddPtr &p, U &&u)
{
mozilla::ReentrancyGuard g(*this);
JS_ASSERT(mutationCount == p.mutationCount);
JS_ASSERT(table);
JS_ASSERT(!p.found());
JS_ASSERT(!(p.keyHash & sCollisionBit));
Expand All @@ -1479,6 +1549,10 @@ class HashTable : private AllocPolicy
p.entry_->setLive(p.keyHash, mozilla::Forward<U>(u));
entryCount++;
mutationCount++;
#ifdef DEBUG
p.generation = generation();
p.mutationCount = mutationCount;
#endif
return true;
}

Expand Down Expand Up @@ -1520,7 +1594,10 @@ class HashTable : private AllocPolicy
template <class U>
bool relookupOrAdd(AddPtr& p, const Lookup &l, U &&u)
{
#ifdef DEBUG
p.generation = generation();
p.mutationCount = mutationCount;
#endif
{
mozilla::ReentrancyGuard g(*this);
JS_ASSERT(prepareHash(l) == p.keyHash); // l has not been destroyed
Expand Down
1 change: 1 addition & 0 deletions mfbt/DebugOnly.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class DebugOnly
operator const T&() const { return value; }

T& operator->() { return value; }
const T& operator->() const { return value; }

#else
DebugOnly() { }
Expand Down

0 comments on commit 2e8c6de

Please sign in to comment.