diff --git a/parser/htmlparser/src/nsHTMLEntities.cpp b/parser/htmlparser/src/nsHTMLEntities.cpp index 209b7e901cca..e597cd8abcca 100644 --- a/parser/htmlparser/src/nsHTMLEntities.cpp +++ b/parser/htmlparser/src/nsHTMLEntities.cpp @@ -127,8 +127,10 @@ nsHTMLEntities::AddRefTable(void) if (!entry->node) entry->node = node; } +#ifdef DEBUG PL_DHashMarkTableImmutable(&gUnicodeToEntity); PL_DHashMarkTableImmutable(&gEntityToUnicode); +#endif } ++gTableRefCnt; return NS_OK; diff --git a/toolkit/components/telemetry/Telemetry.cpp b/toolkit/components/telemetry/Telemetry.cpp index 78aa969e8a89..0d47e2226fd9 100644 --- a/toolkit/components/telemetry/Telemetry.cpp +++ b/toolkit/components/telemetry/Telemetry.cpp @@ -956,8 +956,10 @@ mFailedLockCount(0) for (size_t i = 0; i < ArrayLength(trackedDBs); i++) mTrackedDBs.PutEntry(nsDependentCString(trackedDBs[i])); +#ifdef DEBUG // Mark immutable to prevent asserts on simultaneous access from multiple threads mTrackedDBs.MarkImmutable(); +#endif mReporter = new TelemetryReporter(); NS_RegisterMemoryReporter(mReporter); } diff --git a/xpcom/ds/nsStaticNameTable.cpp b/xpcom/ds/nsStaticNameTable.cpp index 8ac97986364b..fdd0a4d70d1b 100644 --- a/xpcom/ds/nsStaticNameTable.cpp +++ b/xpcom/ds/nsStaticNameTable.cpp @@ -177,7 +177,9 @@ nsStaticCaseInsensitiveNameTable::Init(const char* const aNames[], int32_t Count entry->mString = strPtr; // not owned! entry->mIndex = index; } +#ifdef DEBUG PL_DHashMarkTableImmutable(&mNameTable); +#endif return true; } diff --git a/xpcom/glue/nsTHashtable.h b/xpcom/glue/nsTHashtable.h index 0ff65b386a59..bb123957bbdc 100644 --- a/xpcom/glue/nsTHashtable.h +++ b/xpcom/glue/nsTHashtable.h @@ -275,6 +275,7 @@ public: return PL_DHashTableSizeOfExcludingThis(&mTable, nullptr, mallocSizeOf); } +#ifdef DEBUG /** * Mark the table as constant after initialization. * @@ -287,6 +288,7 @@ public: PL_DHashMarkTableImmutable(&mTable); } +#endif protected: PLDHashTable mTable; diff --git a/xpcom/glue/pldhash.cpp b/xpcom/glue/pldhash.cpp index 4fe95c428f76..20a177138a8f 100644 --- a/xpcom/glue/pldhash.cpp +++ b/xpcom/glue/pldhash.cpp @@ -24,28 +24,21 @@ #endif /* - * The following code is used to ensure that calls to one of table->ops or to - * an enumerator do not cause re-entry into a call that can mutate the table. - * Any check that fails will cause an immediate abort, even in non-debug - * builds. + * The following DEBUG-only code is used to assert that calls to one of + * table->ops or to an enumerator do not cause re-entry into a call that + * can mutate the table. */ +#ifdef DEBUG /* - * Most callers that check the recursion level don't care about this magical - * value because they are checking that mutation is allowed (and therefore the - * level is 0 or 1, depending on whether they incremented it). + * Most callers that assert about the recursion level don't care about + * this magical value because they are asserting that mutation is + * allowed (and therefore the level is 0 or 1, depending on whether they + * incremented it). * * Only PL_DHashTableFinish needs to allow this special value. */ -#define IMMUTABLE_RECURSION_LEVEL ((uint32_t)-1) - -/* This aborts in all builds. */ -#define ABORT_UNLESS(condition) \ - do { \ - if (!(condition)) { \ - NS_RUNTIMEABORT("fatal error in pldhash"); \ - } \ - } while (0) +#define IMMUTABLE_RECURSION_LEVEL ((uint16_t)-1) #define RECURSION_LEVEL_SAFE_TO_FINISH(table_) \ (table_->recursionLevel == 0 || \ @@ -55,14 +48,21 @@ do { \ if (table_->recursionLevel != IMMUTABLE_RECURSION_LEVEL) \ ++table_->recursionLevel; \ - } while (0) + } while(0) #define DECREMENT_RECURSION_LEVEL(table_) \ do { \ if (table->recursionLevel != IMMUTABLE_RECURSION_LEVEL) { \ - ABORT_UNLESS(table->recursionLevel > 0); \ + MOZ_ASSERT(table->recursionLevel > 0); \ --table->recursionLevel; \ } \ - } while (0) + } while(0) + +#else + +#define INCREMENT_RECURSION_LEVEL(table_) do { } while(0) +#define DECREMENT_RECURSION_LEVEL(table_) do { } while(0) + +#endif /* defined(DEBUG) */ using namespace mozilla; @@ -213,7 +213,6 @@ PL_DHashTableInit(PLDHashTable *table, const PLDHashTableOps *ops, void *data, return false; table->hashShift = PL_DHASH_BITS - log2; table->entrySize = entrySize; - ABORT_UNLESS(uint32_t(table->entrySize) == entrySize); table->entryCount = table->removedCount = 0; table->generation = 0; uint32_t nbytes; @@ -226,7 +225,9 @@ PL_DHashTableInit(PLDHashTable *table, const PLDHashTableOps *ops, void *data, memset(table->entryStore, 0, nbytes); METER(memset(&table->stats, 0, sizeof table->stats)); +#ifdef DEBUG table->recursionLevel = 0; +#endif return true; } @@ -304,7 +305,7 @@ PL_DHashTableFinish(PLDHashTable *table) } DECREMENT_RECURSION_LEVEL(table); - ABORT_UNLESS(RECURSION_LEVEL_SAFE_TO_FINISH(table)); + MOZ_ASSERT(RECURSION_LEVEL_SAFE_TO_FINISH(table)); /* Free entry storage last. */ table->ops->freeTable(table, table->entryStore); @@ -447,7 +448,9 @@ ChangeTable(PLDHashTable *table, int deltaLog2) return false; /* We can't fail from here on, so update table parameters. */ +#ifdef DEBUG uint32_t recursionLevel = table->recursionLevel; +#endif table->hashShift = PL_DHASH_BITS - newLog2; table->removedCount = 0; table->generation++; @@ -458,7 +461,9 @@ ChangeTable(PLDHashTable *table, int deltaLog2) oldEntryAddr = oldEntryStore = table->entryStore; table->entryStore = newEntryStore; PLDHashMoveEntry moveEntry = table->ops->moveEntry; +#ifdef DEBUG table->recursionLevel = recursionLevel; +#endif /* Copy only live entries, leaving removed ones behind. */ uint32_t oldCapacity = 1u << oldLog2; @@ -484,7 +489,7 @@ PL_DHashTableOperate(PLDHashTable *table, const void *key, PLDHashOperator op) { PLDHashEntryHdr *entry; - ABORT_UNLESS(op == PL_DHASH_LOOKUP || table->recursionLevel == 0); + MOZ_ASSERT(op == PL_DHASH_LOOKUP || table->recursionLevel == 0); INCREMENT_RECURSION_LEVEL(table); PLDHashNumber keyHash = table->ops->hashKey(table, key); @@ -592,7 +597,7 @@ PL_DHashTableOperate(PLDHashTable *table, const void *key, PLDHashOperator op) void PL_DHashTableRawRemove(PLDHashTable *table, PLDHashEntryHdr *entry) { - ABORT_UNLESS(table->recursionLevel != IMMUTABLE_RECURSION_LEVEL); + MOZ_ASSERT(table->recursionLevel != IMMUTABLE_RECURSION_LEVEL); NS_ASSERTION(PL_DHASH_ENTRY_IS_LIVE(entry), "PL_DHASH_ENTRY_IS_LIVE(entry)"); @@ -636,7 +641,7 @@ PL_DHashTableEnumerate(PLDHashTable *table, PLDHashEnumerator etor, void *arg) entryAddr += entrySize; } - ABORT_UNLESS(!didRemove || table->recursionLevel == 1); + MOZ_ASSERT(!didRemove || table->recursionLevel == 1); /* * Shrink or compress if a quarter or more of all entries are removed, or @@ -711,11 +716,13 @@ PL_DHashTableSizeOfIncludingThis(const PLDHashTable *table, mallocSizeOf, arg); } +#ifdef DEBUG void PL_DHashMarkTableImmutable(PLDHashTable *table) { table->recursionLevel = IMMUTABLE_RECURSION_LEVEL; } +#endif #ifdef PL_DHASHMETER #include diff --git a/xpcom/glue/pldhash.h b/xpcom/glue/pldhash.h index 9ce4fe3527f4..047ca16a5587 100644 --- a/xpcom/glue/pldhash.h +++ b/xpcom/glue/pldhash.h @@ -8,7 +8,6 @@ /* * Double hashing, a la Knuth 6. */ -#include "mozilla/Atomics.h" #include "mozilla/MemoryReporting.h" #include "mozilla/Types.h" #include "nscore.h" @@ -183,8 +182,14 @@ struct PLDHashTable { const PLDHashTableOps *ops; /* virtual operations, see below */ void *data; /* ops- and instance-specific data */ int16_t hashShift; /* multiplicative hash shift */ - uint16_t entrySize; /* number of bytes in an entry */ - mozilla::Atomic recursionLevel; /* detects unsafe re-entry */ + /* + * |recursionLevel| is only used in debug builds, but is present in opt + * builds to avoid binary compatibility problems when mixing DEBUG and + * non-DEBUG components. (Actually, even if it were removed, + * sizeof(PLDHashTable) wouldn't change, due to struct padding.) + */ + uint16_t recursionLevel; /* used to detect unsafe re-entry */ + uint32_t entrySize; /* number of bytes in an entry */ uint32_t entryCount; /* number of entries in table */ uint32_t removedCount; /* removed entry sentinels in table */ uint32_t generation; /* entry storage generation number */ @@ -544,22 +549,24 @@ PL_DHashTableSizeOfIncludingThis(const PLDHashTable *table, mozilla::MallocSizeOf mallocSizeOf, void *arg = nullptr); +#ifdef DEBUG /** * Mark a table as immutable for the remainder of its lifetime. This - * changes the implementation from checking one set of invariants to - * checking a different set. + * changes the implementation from ASSERTing one set of invariants to + * ASSERTing a different set. * * When a table is NOT marked as immutable, the table implementation - * checks that the table is not mutated from its own callbacks. It + * asserts that the table is not mutated from its own callbacks. It * assumes the caller protects the table from being accessed on multiple * threads simultaneously. * - * When the table is marked as immutable, the re-entry checks will + * When the table is marked as immutable, the re-entry assertions will * no longer trigger erroneously due to multi-threaded access. Instead, - * mutations will cause checks to fail. + * mutations will cause assertions. */ NS_COM_GLUE void PL_DHashMarkTableImmutable(PLDHashTable *table); +#endif #ifdef PL_DHASHMETER #include