Currently, changes to HashTable::generation can only happen after bumping HashTable::mutationCount. With generational GC enabled, however, a MinorGC can change a table's generation through rekey without changing its mutationCount. Since AddPtr only checks mutationCount, this meant that we were sometimes not catching a MinorGC across the lifetime of an AddPtr, later accessing freed memory. We could "fix" this by making generation changes also bump mutationCount, but I was worried there might be other holes I wasn't seeing. Instead, since this is all debug code, I just added full correctness assertions to all the things. With this patch we assert the correctness of mutationCount and generation in Ptr, AddPtr, Range, and Enum, hopefully preventing any further instances of this class of errors.

Comment on attachment 8345378 [details] [diff] [review] add_hashtable_assertions-v0.diff Review of attachment 8345378 [details] [diff] [review]: ----------------------------------------------------------------- Nice! Can you wrap all the DebugOnly member variable declarations in #ifdef DEBUG? Otherwise they'll consume real space (even if it's dead) hash tables are relatively hot enough for this to matter.

https://hg.mozilla.org/integration/mozilla-inbound/rev/70e76314c8a7 You are right, it's probably worth it to make sure Ptr fits in a word even if it is a bit uglier.

https://hg.mozilla.org/integration/mozilla-inbound/rev/1ce15dd9df2e And backed out because apparently layout is constructing their own Ptrs(!)?

Was not able to reproduce the build bustage locally with linux-clang. Clang on mac seems to be complaining that |table| in HashTable::Ptr(HashTable &table) masks HashTable::table. O_o Likewise for the constructors of AddPtr and Range. AFAICT, clang is off it's meds. I've renamed the argument to |tableArg| and the nested |table| to |table_| in the vague hope that clang doesn't flip out again. https://tbpl.mozilla.org/?tree=Try&rev=14968651197b

This time with more qreffing. https://tbpl.mozilla.org/?tree=Try&rev=5ace6ce24832

Looks green on the builds that failed before, relanding: https://hg.mozilla.org/integration/mozilla-inbound/rev/517312577287