From 8a84a1c62a451858415402df6f20ba9b25102ff9 Mon Sep 17 00:00:00 2001 From: Mike McLaughlin Date: Thu, 13 Feb 2025 14:06:20 -0800 Subject: [PATCH] Fix VS div-by-0 in DacEnumerableHashTable code --- src/coreclr/vm/dacenumerablehash.inl | 66 ++++++++++++++-------------- 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/src/coreclr/vm/dacenumerablehash.inl b/src/coreclr/vm/dacenumerablehash.inl index c2522e156647e4..e41056b3f4f7b1 100644 --- a/src/coreclr/vm/dacenumerablehash.inl +++ b/src/coreclr/vm/dacenumerablehash.inl @@ -345,45 +345,47 @@ DPTR(VALUE) DacEnumerableHashTable::BaseFindFirstEntryByHash do { DWORD cBuckets = GetLength(curBuckets); + if (cBuckets > 0) + { + // Compute which bucket the entry belongs in based on the hash. + // +2 to skip "length" and "next" slots + DWORD dwBucket = iHash % cBuckets + SKIP_SPECIAL_SLOTS; - // Compute which bucket the entry belongs in based on the hash. - // +2 to skip "length" and "next" slots - DWORD dwBucket = iHash % cBuckets + SKIP_SPECIAL_SLOTS; - - // Point at the first entry in the bucket chain that stores entries with the given hash code. - PTR_VolatileEntry pEntry = VolatileLoadWithoutBarrier(&curBuckets[dwBucket]); - TADDR expectedEndSentinel = ComputeEndSentinel(BaseEndSentinel(curBuckets), dwBucket); + // Point at the first entry in the bucket chain that stores entries with the given hash code. + PTR_VolatileEntry pEntry = VolatileLoadWithoutBarrier(&curBuckets[dwBucket]); + TADDR expectedEndSentinel = ComputeEndSentinel(BaseEndSentinel(curBuckets), dwBucket); - // Walk the bucket chain one entry at a time. - while (!IsEndSentinel(pEntry)) - { - if (pEntry->m_iHashValue == iHash) + // Walk the bucket chain one entry at a time. + while (!IsEndSentinel(pEntry)) { - // We've found our match. + if (pEntry->m_iHashValue == iHash) + { + // We've found our match. - // Record our current search state into the provided context so that a subsequent call to - // BaseFindNextEntryByHash can pick up the search where it left off. - pContext->m_pEntry = dac_cast(pEntry); - pContext->m_curBuckets = curBuckets; - pContext->m_expectedEndSentinel = dac_cast(expectedEndSentinel); + // Record our current search state into the provided context so that a subsequent call to + // BaseFindNextEntryByHash can pick up the search where it left off. + pContext->m_pEntry = dac_cast(pEntry); + pContext->m_curBuckets = curBuckets; + pContext->m_expectedEndSentinel = dac_cast(expectedEndSentinel); - // Return the address of the sub-classes' embedded entry structure. - return VALUE_FROM_VOLATILE_ENTRY(pEntry); - } + // Return the address of the sub-classes' embedded entry structure. + return VALUE_FROM_VOLATILE_ENTRY(pEntry); + } - // Move to the next entry in the chain. - pEntry = VolatileLoadWithoutBarrier(&pEntry->m_pNextEntry); - } + // Move to the next entry in the chain. + pEntry = VolatileLoadWithoutBarrier(&pEntry->m_pNextEntry); + } - if (!AcceptableEndSentinel(pEntry, expectedEndSentinel)) - { - // If we hit this logic, we've managed to hit a case where the linked list was in the process of being - // moved to a new set of buckets while we were walking the list, and we walked part of the list of the - // bucket in the old hash table (which is fine), and part of the list in the new table, which may not - // be the correct bucket to walk. Most notably, the situation that can cause this will cause the list in - // the old bucket to be missing items. Restart the lookup, as the linked list is unlikely to still be under - // edit a second time. - continue; + if (!AcceptableEndSentinel(pEntry, expectedEndSentinel)) + { + // If we hit this logic, we've managed to hit a case where the linked list was in the process of being + // moved to a new set of buckets while we were walking the list, and we walked part of the list of the + // bucket in the old hash table (which is fine), and part of the list in the new table, which may not + // be the correct bucket to walk. Most notably, the situation that can cause this will cause the list in + // the old bucket to be missing items. Restart the lookup, as the linked list is unlikely to still be under + // edit a second time. + continue; + } } // in a rare case if resize is in progress, look in the new table as well.