diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp index c0e25f52e3a639..42196705413479 100644 --- a/src/coreclr/debug/daccess/daccess.cpp +++ b/src/coreclr/debug/daccess/daccess.cpp @@ -1487,21 +1487,19 @@ DAC_INSTANCE* DacInstanceManager::Add(DAC_INSTANCE* inst) { _ASSERTE(inst != NULL); - const KeyValuePair* pEntry = m_hash.LookupPtr(inst->addr); - if (pEntry != NULL) - { - DAC_INSTANCE* existing = pEntry->Value(); + if (!m_hash.Update(inst->addr, [&](KeyValuePair &entry) + { + DAC_INSTANCE* existing = entry.Value(); - //This is necessary to preserve the semantics of Supersede, however, it - //is more or less dead code. - inst->next = existing; + //This is necessary to preserve the semantics of Supersede, however, it + //is more or less dead code. + inst->next = existing; - //verify descending order - _ASSERTE(inst->size >= existing->size); + //verify descending order + _ASSERTE(inst->size >= existing->size); - m_hash.ReplacePtr(pEntry, KeyValuePair(inst->addr, inst)); - } - else + entry = KeyValuePair(inst->addr, inst); + })) { inst->next = NULL; m_hash.Add(KeyValuePair(inst->addr, inst)); @@ -1747,7 +1745,11 @@ DacInstanceManager::Supersede(DAC_INSTANCE* inst) // Removing the head of the chain. if (inst->next != NULL) { - m_hash.ReplacePtr(pEntry, KeyValuePair(inst->addr, inst->next)); + bool updated = m_hash.Update(inst->addr, [&](KeyValuePair &entry) + { + entry = KeyValuePair(inst->addr, inst->next); + }); + _ASSERTE(updated); } else { diff --git a/src/coreclr/debug/daccess/dacfn.cpp b/src/coreclr/debug/daccess/dacfn.cpp index 822d99080a6386..d31729d074a4b8 100644 --- a/src/coreclr/debug/daccess/dacfn.cpp +++ b/src/coreclr/debug/daccess/dacfn.cpp @@ -1397,10 +1397,10 @@ void DacPatchCache::Populate() // Clear any stale entries from previous failed population attempts m_entries.Clear(); - HASHFIND info; + ULONG patchIndex; DebuggerPatchTable * pTable = DebuggerController::GetPatchTable(); - DebuggerControllerPatch * pPatch = pTable->GetFirstPatch(&info); + DebuggerControllerPatch * pPatch = pTable->GetFirstPatch(&patchIndex); while (pPatch != NULL) { @@ -1414,7 +1414,7 @@ void DacPatchCache::Populate() m_entries.Append(entry); } - pPatch = pTable->GetNextPatch(&info); + pPatch = pTable->GetNextPatch(&patchIndex); } m_isPopulated = true; diff --git a/src/coreclr/debug/di/divalue.cpp b/src/coreclr/debug/di/divalue.cpp index 46a549880899f3..cf86f8ec884813 100644 --- a/src/coreclr/debug/di/divalue.cpp +++ b/src/coreclr/debug/di/divalue.cpp @@ -2093,7 +2093,7 @@ HRESULT CordbObjectValue::GetFieldValueForType(ICorDebugType * pType, ThrowHR(E_INVALIDARG); } - FieldData * pFieldData; + const FieldData * pFieldData; #ifdef _DEBUG pFieldData = NULL; @@ -3130,7 +3130,7 @@ HRESULT CordbVCObjectValue::GetFieldValueForType(ICorDebugType * pType, pCordbType = static_cast (pType); } - FieldData * pFieldData; + const FieldData * pFieldData; #ifdef _DEBUG pFieldData = NULL; diff --git a/src/coreclr/debug/di/hash.cpp b/src/coreclr/debug/di/hash.cpp index 911e0742814bb2..d42e05e03e560b 100644 --- a/src/coreclr/debug/di/hash.cpp +++ b/src/coreclr/debug/di/hash.cpp @@ -12,92 +12,48 @@ * Hash Table class * ------------------------------------------------------------------------- */ -CordbHashTable::~CordbHashTable() -{ - HASHFIND find; - - for (CordbHashEntry *entry = (CordbHashEntry *) FindFirstEntry(&find); - entry != NULL; - entry = (CordbHashEntry *) FindNextEntry(&find)) - entry->pBase->InternalRelease(); -} - HRESULT CordbHashTable::UnsafeAddBase(CordbBase *pBase) { AssertIsProtected(); - DbgIncChangeCount(); - - HRESULT hr = S_OK; - - if (!m_initialized) - { - HRESULT res = NewInit(m_iBuckets, sizeof(CordbHashEntry), 0xffff); - if (res != S_OK) - { - return res; - } - - m_initialized = true; - } + if (!m_table.AddNoThrow(pBase)) + return E_OUTOFMEMORY; - CordbHashEntry *entry = (CordbHashEntry *) Add(HASH((ULONG_PTR)pBase->m_id)); - - if (entry == NULL) - { - hr = E_FAIL; - } - else - { - entry->pBase = pBase; - m_count++; - pBase->InternalAddRef(); - } - return hr; + pBase->InternalAddRef(); + return S_OK; } bool CordbHashTable::IsInitialized() { - return m_initialized; + return m_table.GetCapacity() != 0; } CordbBase *CordbHashTable::UnsafeGetBase(ULONG_PTR id, BOOL fFab) { AssertIsProtected(); - CordbHashEntry *entry = NULL; - - if (!m_initialized) - return (NULL); - - entry = (CordbHashEntry *) Find(HASH((ULONG_PTR)id), KEY(id)); - return (entry ? entry->pBase : NULL); + return m_table.Lookup(id); } HRESULT CordbHashTable::UnsafeSwapBase(CordbBase *pOldBase, CordbBase *pNewBase) { - if (!m_initialized) - return E_FAIL; - AssertIsProtected(); - DbgIncChangeCount(); ULONG_PTR id = (ULONG_PTR)pOldBase->m_id; - CordbHashEntry *entry - = (CordbHashEntry *) Find(HASH((ULONG_PTR)id), KEY(id)); - - if (entry == NULL) + if (!m_table.Update(id, [&](CordbBase *&entry) + { + _ASSERTE(entry == pOldBase); + _ASSERTE(pOldBase->m_id == pNewBase->m_id); + pNewBase->InternalAddRef(); + entry = pNewBase; + })) { return E_FAIL; } - _ASSERTE(entry->pBase == pOldBase); - entry->pBase = pNewBase; - // release the hash table's reference to the old base and transfer it // to the new one. pOldBase->InternalRelease(); - pNewBase->InternalAddRef(); return S_OK; } @@ -107,59 +63,28 @@ CordbBase *CordbHashTable::UnsafeRemoveBase(ULONG_PTR id) { AssertIsProtected(); - if (!m_initialized) - return NULL; - - DbgIncChangeCount(); - - - CordbHashEntry *entry - = (CordbHashEntry *) Find(HASH((ULONG_PTR)id), KEY(id)); - - if (entry == NULL) + CordbBase *base = m_table.Lookup(id); + if (base == NULL) { return NULL; } - CordbBase *base = entry->pBase; - - Delete(HASH((ULONG_PTR)id), (HASHENTRY *) entry); - m_count--; + m_table.Remove(id); base->InternalRelease(); return base; } -CordbBase *CordbHashTable::UnsafeFindFirst(HASHFIND *find) +CordbHashTable::Iterator CordbHashTable::Begin() { AssertIsProtected(); - return UnsafeUnlockedFindFirst(find); + return m_table.Begin(); } -CordbBase *CordbHashTable::UnsafeUnlockedFindFirst(HASHFIND *find) -{ - CordbHashEntry *entry = (CordbHashEntry *) FindFirstEntry(find); - - if (entry == NULL) - return NULL; - else - return entry->pBase; -} - -CordbBase *CordbHashTable::UnsafeFindNext(HASHFIND *find) +CordbHashTable::Iterator CordbHashTable::End() { AssertIsProtected(); - return UnsafeUnlockedFindNext(find); -} - -CordbBase *CordbHashTable::UnsafeUnlockedFindNext(HASHFIND *find) -{ - CordbHashEntry *entry = (CordbHashEntry *) FindNextEntry(find); - - if (entry == NULL) - return NULL; - else - return entry->pBase; + return m_table.End(); } /* ------------------------------------------------------------------------- * @@ -170,20 +95,15 @@ CordbBase *CordbHashTable::UnsafeUnlockedFindNext(HASHFIND *find) // Use the BuildOrThrow method to instantiate. CordbHashTableEnum::CordbHashTableEnum( CordbBase * pOwnerObj, NeuterList * pOwnerList, - CordbHashTable *table, REFIID guid ) : CordbBase(pOwnerObj->GetProcess(), 0, enumCordbHashTableEnum), m_pOwnerObj(pOwnerObj), m_pOwnerNeuterList(pOwnerList), - m_table(table), - m_started(false), - m_done(false), m_guid(guid), - m_iCurElt(0), - m_count(0), - m_fCountInit(FALSE) + m_iCurElt(0) { + m_items.EnableAutoClear(); } //--------------------------------------------------------------------------------------- @@ -204,9 +124,23 @@ void CordbHashTableEnum::BuildOrThrow( const GUID &id, RSInitHolder * pHolder) { - CordbHashTableEnum * pEnum = new CordbHashTableEnum(pOwnerObj, pOwnerList, pTable, id); + CordbProcess *pProcess = pOwnerObj->GetProcess(); + RSLock *pLock = pProcess == nullptr ? nullptr : pProcess->GetProcessLock(); + RSLockHolder lockHolder(pLock, FALSE); + if (pLock != nullptr) + lockHolder.Acquire(); + + CordbHashTableEnum * pEnum = new CordbHashTableEnum(pOwnerObj, pOwnerList, id); pHolder->Assign(pEnum); + pEnum->m_items.AllocOrThrow(pTable->GetCount()); + ULONG index = 0; + for (CordbHashTable::Iterator iterator = pTable->Begin(), end = pTable->End(); iterator != end; iterator++) + { + pEnum->m_items.Assign(index++, *iterator); + } + _ASSERTE(index == pEnum->m_items.Length()); + // If no neuter-list supplied, then our owner is manually managing us. // It also means we can't be cloned. if (pOwnerList != NULL) @@ -214,9 +148,6 @@ void CordbHashTableEnum::BuildOrThrow( pOwnerList->Add(pOwnerObj->GetProcess(), pEnum); } -#ifdef _DEBUG - pEnum->m_DbgChangeCount = pEnum->m_table->GetChangeCount(); -#endif } // Only for cloning. @@ -225,20 +156,15 @@ CordbHashTableEnum::CordbHashTableEnum(CordbHashTableEnum *cloneSrc) : CordbBase(cloneSrc->m_pOwnerObj->GetProcess(), 0, enumCordbHashTableEnum), m_pOwnerObj(cloneSrc->m_pOwnerObj), m_pOwnerNeuterList(cloneSrc->m_pOwnerNeuterList), - m_started(cloneSrc->m_started), - m_done(cloneSrc->m_done), - m_hashfind(cloneSrc->m_hashfind), m_guid(cloneSrc->m_guid), - m_iCurElt(cloneSrc->m_iCurElt), - m_count(cloneSrc->m_count), - m_fCountInit(cloneSrc->m_fCountInit) + m_iCurElt(cloneSrc->m_iCurElt) { + m_items.EnableAutoClear(); + // We can get cloned at any time, so our owner can't manually control us, // so we need explicit access to a neuter list. _ASSERTE(m_pOwnerNeuterList != NULL); - m_table = cloneSrc->m_table; - HRESULT hr = S_OK; EX_TRY { @@ -259,23 +185,19 @@ CordbHashTableEnum::CordbHashTableEnum(CordbHashTableEnum *cloneSrc) EX_CATCH_HRESULT(hr); SetUnrecoverableIfFailed(GetProcess(), hr); -#ifdef _DEBUG - m_DbgChangeCount = cloneSrc->m_DbgChangeCount; -#endif } CordbHashTableEnum::~CordbHashTableEnum() { _ASSERTE(this->IsNeutered()); - _ASSERTE(m_table == NULL); _ASSERTE(m_pOwnerObj == NULL); _ASSERTE(m_pOwnerNeuterList == NULL); } void CordbHashTableEnum::Neuter() { - m_table = NULL; + m_items.Clear(); m_pOwnerObj = NULL; m_pOwnerNeuterList = NULL; @@ -287,8 +209,7 @@ HRESULT CordbHashTableEnum::Reset() { HRESULT hr = S_OK; - m_started = false; - m_done = false; + m_iCurElt = 0; return hr; } @@ -304,36 +225,34 @@ HRESULT CordbHashTableEnum::Clone(ICorDebugEnum **ppEnum) HRESULT hr; hr = S_OK; - CordbHashTableEnum *e = NULL; - CordbProcess * pProc = GetProcess(); + *ppEnum = nullptr; - if (pProc != NULL) - { - // @todo - this is really ugly. This macro sets up stack-based dtor cleanup - // objects, and so it has to be in the same block of code as the code - // it protectes. Eg, we couldn't say: 'if (...) { ATT_ } { common code }' - // because the ATT_ stack based object would get destructed before the 'common code' - // was executed. - ATT_REQUIRE_STOPPED_MAY_FAIL(pProc); - e = new (nothrow) CordbHashTableEnum(this); - } - else - { - e = new (nothrow) CordbHashTableEnum(this); - - } - - if (e == NULL) + EX_TRY { - (*ppEnum) = NULL; - hr = E_OUTOFMEMORY; - goto LExit; + auto cloneSnapshot = [&]() + { + RSInitHolder clone; + clone.Assign(new CordbHashTableEnum(this)); + clone->m_items.AllocOrThrow(m_items.Length()); + for (ULONG index = 0; index < m_items.Length(); index++) + clone->m_items.Assign(index, m_items[index]); + IfFailThrow(clone->QueryInterface(m_guid, reinterpret_cast(ppEnum))); + clone.ClearAndMarkDontNeuter(); + }; + + if (pProc != NULL) + { + ATT_REQUIRE_STOPPED_MAY_FAIL(pProc); + cloneSnapshot(); + } + else + { + cloneSnapshot(); + } } + EX_CATCH_HRESULT(hr); - e->QueryInterface(m_guid, (void **) ppEnum); - -LExit: return hr; } @@ -345,57 +264,7 @@ HRESULT CordbHashTableEnum::GetCount(ULONG *pcelt) VALIDATE_POINTER_TO_OBJECT(pcelt, ULONG *); - *pcelt = m_table->GetCount(); - - return S_OK; -} - -HRESULT CordbHashTableEnum::PrepForEnum(CordbBase **pBase) -{ - HRESULT hr = S_OK; - - if (!m_started) - { - (*pBase) = m_table->UnsafeUnlockedFindFirst(&m_hashfind); - m_started = true; - } - else - { - (*pBase) = m_table->UnsafeUnlockedFindNext(&m_hashfind); - } - - return hr; -} - -HRESULT CordbHashTableEnum::SetupModuleEnum(void) -{ - return S_OK; -} - - -HRESULT CordbHashTableEnum::AdvancePreAssign(CordbBase **pBase) -{ - HRESULT hr = S_OK; - return hr; -} - -HRESULT CordbHashTableEnum::AdvancePostAssign(CordbBase **pBase, - CordbBase **b, - CordbBase **bEnd) -{ - CordbBase *base; - - if (pBase == NULL) - pBase = &base; - - // If we're looping like normal, or we're in skip - if ( ((b < bEnd) || ((b ==bEnd)&&(b==NULL))) - ) - { - (*pBase) = m_table->UnsafeUnlockedFindNext(&m_hashfind); - if (*pBase == NULL) - m_done = true; - } + *pcelt = m_items.Length(); return S_OK; } @@ -432,79 +301,50 @@ HRESULT CordbHashTableEnum::Next(ULONG celt, } HRESULT hr = S_OK; - CordbBase *base = NULL; CordbBase **b = bases; CordbBase **bEnd = bases + celt; - hr = PrepForEnum(&base); - if (FAILED(hr)) - { - goto LError; - } - - while (b < bEnd && !m_done) + while (b < bEnd && m_iCurElt < m_items.Length()) { - hr = AdvancePreAssign(&base); - if (FAILED(hr)) + CordbBase *base = m_items[m_iCurElt++]; + if (m_guid == IID_ICorDebugProcessEnum) { - goto LError; + *b = (CordbBase*)(ICorDebugProcess*)(CordbProcess*)base; } - - if (base == NULL) + else if (m_guid == IID_ICorDebugBreakpointEnum) + { + *b = (CordbBase*)(ICorDebugBreakpoint*)(CordbBreakpoint*)base; + } + else if (m_guid == IID_ICorDebugStepperEnum) + { + *b = (CordbBase*)(ICorDebugStepper*)(CordbStepper*)base; + } + else if (m_guid == IID_ICorDebugModuleEnum) + { + *b = (CordbBase*)(ICorDebugModule*)(CordbModule*)base; + } + else if (m_guid == IID_ICorDebugThreadEnum) + { + *b = (CordbBase*)(ICorDebugThread*)(CordbThread*)base; + } + else if (m_guid == IID_ICorDebugAppDomainEnum) + { + *b = (CordbBase*)(ICorDebugAppDomain*)(CordbAppDomain*)base; + } + else if (m_guid == IID_ICorDebugAssemblyEnum) { - m_done = true; + *b = (CordbBase*)(ICorDebugAssembly*)(CordbAssembly*)base; } else { - if (m_guid == IID_ICorDebugProcessEnum) - { - *b = (CordbBase*)(ICorDebugProcess*)(CordbProcess*)base; - } - else if (m_guid == IID_ICorDebugBreakpointEnum) - { - *b = (CordbBase*)(ICorDebugBreakpoint*)(CordbBreakpoint*)base; - } - else if (m_guid == IID_ICorDebugStepperEnum) - { - *b = (CordbBase*)(ICorDebugStepper*)(CordbStepper*)base; - } - else if (m_guid == IID_ICorDebugModuleEnum) - { - *b = (CordbBase*)(ICorDebugModule*)(CordbModule*)base; - } - else if (m_guid == IID_ICorDebugThreadEnum) - { - *b = (CordbBase*)(ICorDebugThread*)(CordbThread*)base; - } - else if (m_guid == IID_ICorDebugAppDomainEnum) - { - *b = (CordbBase*)(ICorDebugAppDomain*)(CordbAppDomain*)base; - } - else if (m_guid == IID_ICorDebugAssemblyEnum) - { - *b = (CordbBase*)(ICorDebugAssembly*)(CordbAssembly*)base; - } - else - { - *b = (CordbBase*)(IUnknown*)base; - } - - if (*b) - { - // 'b' is not a valid CordbBase ptr. - base->ExternalAddRef(); - b++; - } - - hr = AdvancePostAssign(&base, b, bEnd); - if (FAILED(hr)) - { - goto LError; - } + *b = (CordbBase*)(IUnknown*)base; } + + // 'b' is not a valid CordbBase ptr. + base->ExternalAddRef(); + b++; } -LError: // // If celt == 1, then the pceltFetched parameter is optional. // @@ -517,7 +357,7 @@ HRESULT CordbHashTableEnum::Next(ULONG celt, // If we reached the end of the enumeration, but not the end // of the number of requested items, we return S_FALSE. // - if (!FAILED(hr) && m_done && (b != bEnd)) + if (b != bEnd) { hr = S_FALSE; } @@ -531,36 +371,15 @@ HRESULT CordbHashTableEnum::Skip(ULONG celt) FAIL_IF_NEUTERED(this); AssertValid(); - HRESULT hr = S_OK; - - CordbBase *base; - - if (celt > 0) + ULONG remaining = m_items.Length() - m_iCurElt; + if (celt > remaining) { - if (!m_started) - { - base = m_table->UnsafeUnlockedFindFirst(&m_hashfind); - - if (base == NULL) - m_done = true; - else - celt--; - - m_started = true; - } - - while (celt > 0 && !m_done) - { - base = m_table->UnsafeUnlockedFindNext(&m_hashfind); - - if (base == NULL) - m_done = true; - else - celt--; - } + m_iCurElt = m_items.Length(); + return S_OK; } - return hr; + m_iCurElt += celt; + return S_OK; } HRESULT CordbHashTableEnum::QueryInterface(REFIID id, void **pInterface) @@ -607,16 +426,7 @@ HRESULT CordbHashTableEnum::QueryInterface(REFIID id, void **pInterface) #ifdef _DEBUG void CordbHashTableEnum::AssertValid() { - // @todo - Our behavior is undefined when enumerating a collection that changes underneath us. - // We'd love to just call this situation illegal, but clients could have very reasonably taken a dependency - // on it. Various APIs (eg, ICDStepper::Deactivate) may remove items from the hash. - // So we need to figure out what the behavior is here, spec it, and then enforce that and enable the - // strongest asserts possible there. - // - // Specifically, we cannot check that the hash hasn't change from underneath us: - // CONSISTENCY_CHECK_MSGF(m_DbgChangeCount == m_table->GetChangeCount(), - // ("Underlying hashtable has changed while enumerating.\nOriginal stamp=%d\nNew stamp=%d\nThis enum=0x%p", - // m_DbgChangeCount, m_table->GetChangeCount(), this)); + _ASSERTE(m_iCurElt <= m_items.Length()); } diff --git a/src/coreclr/debug/di/module.cpp b/src/coreclr/debug/di/module.cpp index 4c5c08c5f69835..47c8cbf94b2c0c 100644 --- a/src/coreclr/debug/di/module.cpp +++ b/src/coreclr/debug/di/module.cpp @@ -30,13 +30,13 @@ CordbModule::CordbModule( : CordbBase(pProcess, VmPtrToCookie(vmModule), enumCordbModule), m_pAssembly(0), m_pAppDomain(0), - m_classes(11), - m_functions(101), + m_classes(), + m_functions(), m_vmAssembly(vmAssembly), m_vmModule(vmModule), m_EnCCount(0), m_fForceMetaDataSerialize(FALSE), - m_nativeCodeTable(101) + m_nativeCodeTable() { _ASSERTE(pProcess->GetProcessLock()->HasLock()); diff --git a/src/coreclr/debug/di/process.cpp b/src/coreclr/debug/di/process.cpp index f5359a8873f6d0..cf04c3e1b7e818 100644 --- a/src/coreclr/debug/di/process.cpp +++ b/src/coreclr/debug/di/process.cpp @@ -353,15 +353,14 @@ IMDInternalImport * CordbProcess::LookupMetaData(VMPTR_PEAssembly vmPEAssembly) { INTERNAL_DAC_CALLBACK(this); - HASHFIND hashFindModule; IMDInternalImport * pMDII = NULL; if (m_pAppDomain != NULL) { - for (CordbModule * pModule = m_pAppDomain->m_modules.FindFirst(&hashFindModule); - pModule != NULL; - pModule = m_pAppDomain->m_modules.FindNext(&hashFindModule)) + for (CordbHashTable::Iterator iterator = m_pAppDomain->m_modules.Begin(), end = m_pAppDomain->m_modules.End(); + iterator != end; iterator++) { + CordbModule *pModule = static_cast(*iterator); if (pModule->GetPEFile() == vmPEAssembly) { pMDII = NULL; @@ -385,10 +384,10 @@ IMDInternalImport * CordbProcess::LookupMetaData(VMPTR_PEAssembly vmPEAssembly) { m_pAppDomain->PrepopulateModules(); - for (CordbModule * pModule = m_pAppDomain->m_modules.FindFirst(&hashFindModule); - pModule != NULL; - pModule = m_pAppDomain->m_modules.FindNext(&hashFindModule)) + for (CordbHashTable::Iterator iterator = m_pAppDomain->m_modules.Begin(), end = m_pAppDomain->m_modules.End(); + iterator != end; iterator++) { + CordbModule *pModule = static_cast(*iterator); if (pModule->GetPEFile() == vmPEAssembly) { pMDII = NULL; @@ -840,13 +839,13 @@ CordbProcess::CordbProcess(ULONG64 clrInstanceId, m_synchronized(false), m_syncCompleteReceived(false), m_pShim(pShim), - m_userThreads(11), + m_userThreads(), m_oddSync(false), #ifdef FEATURE_INTEROP_DEBUGGING - m_unmanagedThreads(11), + m_unmanagedThreads(), #endif m_pAppDomain(NULL), - m_steppers(11), + m_steppers(), m_continueCounter(1), m_flushCounter(0), m_leftSideEventAvailable(NULL), @@ -1939,11 +1938,9 @@ CordbThread * CordbProcess::TryLookupOrCreateThreadByVolatileOSId(DWORD dwThread // CordbThread * CordbProcess::TryLookupThreadByVolatileOSId(DWORD dwThreadId) { - HASHFIND find; - for (CordbThread * pThread = m_userThreads.FindFirst(&find); - pThread != NULL; - pThread = m_userThreads.FindNext(&find)) + for (CordbHashTable::Iterator iterator = m_userThreads.Begin(), end = m_userThreads.End(); iterator != end; iterator++) { + CordbThread *pThread = static_cast(*iterator); _ASSERTE(pThread != NULL); // Get the OS tid. This returns 0 if the thread is switched out. @@ -3548,14 +3545,10 @@ void CordbProcess::MarkAllThreadsDirty() INTERNAL_API_ENTRY(this); _ASSERTE(ThreadHoldsProcessLock()); - CordbThread * pThread; - HASHFIND find; - // We don't need to prepopulate here (to collect LS state) because we're just updating RS state. - for (pThread = m_userThreads.FindFirst(&find); - pThread != NULL; - pThread = m_userThreads.FindNext(&find)) + for (CordbHashTable::Iterator iterator = m_userThreads.Begin(), end = m_userThreads.End(); iterator != end; iterator++) { + CordbThread *pThread = static_cast(*iterator); _ASSERTE(pThread != NULL); pThread->MarkStackFramesDirty(); } @@ -5695,14 +5688,11 @@ HRESULT CordbProcess::SetAllThreadsDebugState(CorDebugThreadState state, if (SUCCEEDED(hr)) { RSLockHolder lockHolder(GetProcessLock()); - HASHFIND hashFind; - CordbThread * pThread; - // We don't need to prepopulate here (to collect LS state) because we're just updating RS state. - for (pThread = m_userThreads.FindFirst(&hashFind); - pThread != NULL; - pThread = m_userThreads.FindNext(&hashFind)) + for (CordbHashTable::Iterator iterator = m_userThreads.Begin(), end = m_userThreads.End(); + iterator != end; iterator++) { + CordbThread *pThread = static_cast(*iterator); if (pThread != pCordbExceptThread) { pThread->m_debugState = state; @@ -7479,11 +7469,10 @@ HRESULT CordbProcess::SuspendUnmanagedThreads() _ASSERTE(ThreadHoldsProcessLock()); // Iterate over all unmanaged threads... - CordbUnmanagedThread* ut; - HASHFIND find; - - for (ut = m_unmanagedThreads.FindFirst(&find); ut != NULL; ut = m_unmanagedThreads.FindNext(&find)) + for (CordbHashTable::Iterator iterator = m_unmanagedThreads.Begin(), end = m_unmanagedThreads.End(); + iterator != end; iterator++) { + CordbUnmanagedThread *ut = static_cast(*iterator); // Don't suspend any thread in a can't stop region. This includes cooperative mode threads & preemptive // threads that haven't pushed a NativeTransitionFrame. The ultimate problem here is that a thread @@ -7527,11 +7516,10 @@ HRESULT CordbProcess::ResumeUnmanagedThreads() FAIL_IF_NEUTERED(this); // Iterate over all unmanaged threads... - CordbUnmanagedThread* ut; - HASHFIND find; - - for (ut = m_unmanagedThreads.FindFirst(&find); ut != NULL; ut = m_unmanagedThreads.FindNext(&find)) + for (CordbHashTable::Iterator iterator = m_unmanagedThreads.Begin(), end = m_unmanagedThreads.End(); + iterator != end; iterator++) { + CordbUnmanagedThread *ut = static_cast(*iterator); // Only resume those unmanaged threads that were suspended by us. if (ut->IsSuspended()) { @@ -8247,16 +8235,14 @@ HRESULT CordbProcess::GetThreadForTaskID(TASKID taskId, ICorDebugThread2 ** ppTh PrepopulateThreadsOrThrow(); // now find the ICorDebugThread corresponding to it - CordbThread * pThread; - HASHFIND hashFind; - - - for (pThread = m_userThreads.FindFirst(&hashFind); - pThread != NULL; - pThread = m_userThreads.FindNext(&hashFind)) + CordbThread *pThread = nullptr; + for (CordbHashTable::Iterator iterator = m_userThreads.Begin(), end = m_userThreads.End(); + iterator != end; iterator++) { - if (pThread->GetTaskID() == taskId) + CordbThread *pCandidate = static_cast(*iterator); + if (pCandidate->GetTaskID() == taskId) { + pThread = pCandidate; break; } } @@ -9642,11 +9628,10 @@ void CordbRCEventThread::ThreadProc() waitCount = 1; CordbSafeHashTable * pHashTable = m_cordb->GetProcessList(); - HASHFIND hashFind; - CordbProcess * pProcess; - - for (pProcess = pHashTable->FindFirst(&hashFind); pProcess != NULL; pProcess = pHashTable->FindNext(&hashFind)) + for (CordbHashTable::Iterator iterator = pHashTable->Begin(), end = pHashTable->End(); + iterator != end; iterator++) { + CordbProcess *pProcess = static_cast(*iterator); _ASSERTE(waitCount < MAXIMUM_WAIT_OBJECTS); if( waitCount >= MAXIMUM_WAIT_OBJECTS ) @@ -9682,7 +9667,7 @@ void CordbRCEventThread::ThreadProc() while(i < waitCount) { - pProcess = rgProcessSet[i]; + CordbProcess *pProcess = rgProcessSet[i]; // Take the process lock so we can check the queue safely pProcess->Lock(); @@ -14384,15 +14369,14 @@ HRESULT CordbProcess::IsReadyForDetach() } // Check for any outstanding EnC modules. - HASHFIND foundModule; - CordbModule * pModule = m_pAppDomain->m_modules.FindFirst(&foundModule); - while (pModule != NULL) + for (CordbHashTable::Iterator iterator = m_pAppDomain->m_modules.Begin(), end = m_pAppDomain->m_modules.End(); + iterator != end; iterator++) { + CordbModule *pModule = static_cast(*iterator); if (pModule->m_EnCCount > 0) { return CORDBG_E_DETACH_FAILED_ON_ENC; } - pModule = m_pAppDomain->m_modules.FindNext(&foundModule); } } diff --git a/src/coreclr/debug/di/rsappdomain.cpp b/src/coreclr/debug/di/rsappdomain.cpp index 2b07c1649afd93..ba15debd4295a9 100644 --- a/src/coreclr/debug/di/rsappdomain.cpp +++ b/src/coreclr/debug/di/rsappdomain.cpp @@ -42,10 +42,10 @@ CordbAppDomain::CordbAppDomain(CordbProcess * pProcess, VMPTR_AppDomain vmAppDomain) : CordbBase(pProcess, LsPtrToCookie(vmAppDomain.ToLsPtr()), enumCordbAppDomain), m_AppDomainId(0), - m_breakpoints(17), - m_sharedtypes(3), - m_modules(17), - m_assemblies(9), + m_breakpoints(), + m_sharedtypes(), + m_modules(), + m_assemblies(), m_vmAppDomain(vmAppDomain) { // This may throw out of the Ctor on error. @@ -108,13 +108,10 @@ void CordbAppDomain::Neuter() // Disconnect any active breakpoints // { - CordbBreakpoint* entry; - HASHFIND find; - - for (entry = m_breakpoints.FindFirst(&find); - entry != NULL; - entry = m_breakpoints.FindNext(&find)) + for (CordbHashTable::Iterator iterator = m_breakpoints.Begin(), end = m_breakpoints.End(); + iterator != end; iterator++) { + CordbBreakpoint *entry = static_cast(*iterator); entry->Disconnect(); } } @@ -534,15 +531,11 @@ CordbModule * CordbAppDomain::GetModuleFromMetaDataInterface(IUnknown *pIMetaDat hr = pImport->GetScopeProps(NULL, 0, 0, &matchMVID); IfFailThrow(hr); - CordbModule* pModule; - HASHFIND findmodule; - PrepopulateModules(); - for (pModule = m_modules.FindFirst(&findmodule); - pModule != NULL; - pModule = m_modules.FindNext(&findmodule)) + for (CordbHashTable::Iterator iterator = m_modules.Begin(), end = m_modules.End(); iterator != end; iterator++) { + CordbModule *pModule = static_cast(*iterator); IMetaDataImport * pImportCurrent = pModule->GetMetaDataImporter(); // throws _ASSERTE(pImportCurrent != NULL); @@ -848,12 +841,9 @@ void CordbAppDomain::PrepopulateModules() PrepopulateAssembliesOrThrow(); - HASHFIND hashfind; - - for (CordbAssembly * pAssembly = m_assemblies.FindFirst(&hashfind); - pAssembly != NULL; - pAssembly = m_assemblies.FindNext(&hashfind)) + for (CordbHashTable::Iterator iterator = m_assemblies.Begin(), end = m_assemblies.End(); iterator != end; iterator++) { + CordbAssembly *pAssembly = static_cast(*iterator); VMPTR_Assembly vmAssembly = pAssembly->GetAssemblyPtr(); VMPTR_Module vmModule = VMPTR_Module::NullPtr(); BOOL isModuleLoaded = FALSE; diff --git a/src/coreclr/debug/di/rsclass.cpp b/src/coreclr/debug/di/rsclass.cpp index 8e30f5f759982c..78020d982194ed 100644 --- a/src/coreclr/debug/di/rsclass.cpp +++ b/src/coreclr/debug/di/rsclass.cpp @@ -46,7 +46,7 @@ CordbClass::CordbClass(CordbModule *m, mdTypeDef classMetadataToken) HANDLED: CordbModule* m_module; // Assigned w/o AddRef() FieldData *m_fields; // Deleted in ~CordbClass - CordbHangingFieldTable m_hangingFieldsStatic; // by value, ~CHashTableAndData frees + CordbHangingFieldTable m_hangingFieldsStatic; // by value, owns its storage */ @@ -154,7 +154,7 @@ HRESULT CordbClass::GetStaticFieldValue(mdFieldDef fieldDef, // Lookup the field given its metadata token. - FieldData *pFieldData; + const FieldData *pFieldData; hr = GetFieldInfo(fieldDef, &pFieldData); @@ -220,7 +220,7 @@ HRESULT CordbClass::GetStaticFieldValue(mdFieldDef fieldDef, // Else some other failure. /* static */ HRESULT CordbClass::GetStaticFieldValue2(CordbModule * pModule, - FieldData * pFieldData, + const FieldData * pFieldData, BOOL fEnCHangingField, const Instantiation * pInst, ICorDebugFrame * pFrame, @@ -847,7 +847,7 @@ BOOL CordbClass::GotUnallocatedStatic(DacDbiArrayList * pFieldList) * */ HRESULT FieldData::GetFieldSignature(CordbModule *pModule, - SigParser *pSigParser) + SigParser *pSigParser) const { CONTRACTL { @@ -989,14 +989,14 @@ void CordbClass::InitEnCFieldInfo(EnCHangingFieldInfo * pEncField, // fieldToken - token for the EnC field // output: pointer to an initialized instance of FieldData that has been added to the appropriate table // in our cache -FieldData * CordbClass::GetEnCFieldFromDac(BOOL fStatic, +const FieldData * CordbClass::GetEnCFieldFromDac(BOOL fStatic, CordbObjectValue * pObject, mdFieldDef fieldToken) { EnCHangingFieldInfo encField; mdTypeDef metadataToken; - FieldData fieldData, - * pInfo = NULL; + FieldData fieldData; + const FieldData * pInfo = NULL; CordbProcess * pProcess = GetModule()->GetProcess(); _ASSERTE(pProcess != NULL); @@ -1007,7 +1007,7 @@ FieldData * CordbClass::GetEnCFieldFromDac(BOOL fStatic, IfFailThrow(pProcess->GetDAC()->GetEnCHangingFieldInfo(&encField, &fieldData)); _ASSERTE((fStatic != 0) == fieldData.m_fFldIsStatic); - // Save the field results in our cache and get a stable pointer to the data + // Cache the field data and return a borrowed pointer that may be invalidated by subsequent cache mutations. if (fStatic) { pInfo = m_hangingFieldsStatic.AddFieldInfo(&fieldData); @@ -1046,7 +1046,7 @@ FieldData * CordbClass::GetEnCFieldFromDac(BOOL fStatic, // S_OK on success, failure code otherwise. //----------------------------------------------------------------------------- HRESULT CordbClass::GetEnCHangingField(mdFieldDef fldToken, - FieldData **ppFieldData, + const FieldData **ppFieldData, CordbObjectValue * pObject) { FAIL_IF_NEUTERED(this); @@ -1065,7 +1065,7 @@ HRESULT CordbClass::GetEnCHangingField(mdFieldDef fldToken, BOOL fStatic = (pObject == NULL); // Look for cached field information - FieldData *pInfo = NULL; + const FieldData *pInfo = NULL; if (fStatic) { // Static fields should _NOT_ be cleared, since they stick around. Thus @@ -1114,7 +1114,7 @@ HRESULT CordbClass::GetEnCHangingField(mdFieldDef fldToken, // Returns: // S_OK on success. else failure. //----------------------------------------------------------------------------- -HRESULT CordbClass::GetFieldInfo(mdFieldDef fldToken, FieldData **ppFieldData) +HRESULT CordbClass::GetFieldInfo(mdFieldDef fldToken, const FieldData **ppFieldData) { INTERNAL_SYNC_API_ENTRY(GetProcess()); @@ -1150,7 +1150,7 @@ HRESULT CordbClass::SearchFieldInfo( DacDbiArrayList * pFieldList, mdTypeDef classToken, mdFieldDef fldToken, - FieldData **ppFieldData + const FieldData **ppFieldData ) { unsigned int i; diff --git a/src/coreclr/debug/di/rsfunction.cpp b/src/coreclr/debug/di/rsfunction.cpp index 92c73b9aba6847..f18450402bd41e 100644 --- a/src/coreclr/debug/di/rsfunction.cpp +++ b/src/coreclr/debug/di/rsfunction.cpp @@ -44,7 +44,7 @@ CordbFunction::CordbFunction(CordbModule * m, m_argCountCached(0), m_fIsStaticCached(FALSE) #ifdef FEATURE_CODE_VERSIONING - , m_reJitILCodes(1) + , m_reJitILCodes() #endif // FEATURE_CODE_VERSIONING { m_methodSigParserCached = SigParser(NULL, 0); diff --git a/src/coreclr/debug/di/rsmain.cpp b/src/coreclr/debug/di/rsmain.cpp index b0199c36356183..a629d477307cde 100644 --- a/src/coreclr/debug/di/rsmain.cpp +++ b/src/coreclr/debug/di/rsmain.cpp @@ -927,7 +927,7 @@ Cordb::Cordb(CorDebugInterfaceVersion iDebuggerVersion) Cordb::Cordb(CorDebugInterfaceVersion iDebuggerVersion, const ProcessDescriptor& pd, LPCWSTR dacModulePath) : CordbBase(NULL, 0, enumCordb), - m_processes(11), + m_processes(), m_initialized(false), m_debuggerSpecifiedVersion(iDebuggerVersion), m_pd(pd), @@ -1056,13 +1056,9 @@ HRESULT Cordb::Terminate() { RSLockHolder ch(&m_processListMutex); - HASHFIND hfDT; - CordbProcess * pProcess; - - for (pProcess= (CordbProcess*) m_processes.FindFirst(&hfDT); - pProcess != NULL; - pProcess = (CordbProcess*) m_processes.FindNext(&hfDT)) + for (CordbHashTable::Iterator iterator = m_processes.Begin(), end = m_processes.End(); iterator != end; iterator++) { + CordbProcess *pProcess = static_cast(*iterator); _ASSERTE(pProcess->IsSafeToSendEvents() || pProcess->m_unrecoverableError); if (pProcess->IsSafeToSendEvents() && !pProcess->m_unrecoverableError) { @@ -1108,8 +1104,7 @@ HRESULT Cordb::Terminate() #ifdef _DEBUG { - HASHFIND find; - _ASSERTE(m_processes.FindFirst(&find) == NULL); // should be emptied by neuter + _ASSERTE(m_processes.GetCount() == 0); // should be emptied by neuter } #endif //_DEBUG diff --git a/src/coreclr/debug/di/rspriv.h b/src/coreclr/debug/di/rspriv.h index 9e68d3c1abd5ea..81d457812cf821 100644 --- a/src/coreclr/debug/di/rspriv.h +++ b/src/coreclr/debug/di/rspriv.h @@ -1709,48 +1709,42 @@ typedef CordbEnumerator, // explicitly associate w/ an optional RSLock and assert that lock is held on access. // ---------------------------------------------------------------------------- -struct CordbHashEntry +class EMPTY_BASES CordbHashTraits : public NonDacAwareSHashTraits> { - FREEHASHENTRY entry; - CordbBase *pBase; +public: + typedef ULONG_PTR key_t; + static const bool s_DestructPerEntryCleanupAction = true; + static void OnDestructPerEntryCleanupAction(CordbBase *element) { element->InternalRelease(); } + static key_t GetKey(CordbBase *const &element) { return element->m_id; } + static BOOL Equals(key_t left, key_t right) { return left == right; } + static count_t Hash(key_t key) + { + count_t hash = static_cast(key); +#ifdef HOST_64BIT + hash = HashCOUNT_T(hash, static_cast(key >> 32)); +#endif + return hash; + } }; -class CordbHashTable : private CHashTableAndData +class CordbHashTable { private: - bool m_initialized; - SIZE_T m_count; - - BOOL Cmp(SIZE_T k1, const HASHENTRY * pc2) - { - LIMITED_METHOD_CONTRACT; - - return ((ULONG_PTR)k1) != (reinterpret_cast(pc2))->pBase->m_id; - } - - ULONG HASH(ULONG_PTR id) - { - return (ULONG)(id); - } - - SIZE_T KEY(UINT_PTR id) - { - return (SIZE_T)id; - } + SHash m_table; public: + typedef SHash::Iterator Iterator; + bool IsInitialized(); #ifndef DACCESS_COMPILE - CordbHashTable(ULONG size) - : CHashTableAndData(size), m_initialized(false), m_count(0) + CordbHashTable() { #ifdef _DEBUG m_pDbgLock = NULL; - m_dbgChangeCount = 0; #endif } - virtual ~CordbHashTable(); + virtual ~CordbHashTable() = default; #ifdef _DEBUG // CordbHashTables may be protected by a lock. For debug-builds, we can associate @@ -1759,19 +1753,13 @@ class CordbHashTable : private CHashTableAndData { m_pDbgLock = pLock; } - int GetChangeCount() { return m_dbgChangeCount; } private: void AssertIsProtected(); - // Increment the Change count. This can be used to check if the hashtable changes while being enumerated. - void DbgIncChangeCount() { m_dbgChangeCount++; } - - int m_dbgChangeCount; RSLock * m_pDbgLock; #else // RSLock association is a no-op on free builds. void AssertIsProtected() { }; - void DbgIncChangeCount() { }; #endif // _DEBUG public: @@ -1781,7 +1769,7 @@ class CordbHashTable : private CHashTableAndData ULONG32 GetCount() { - return ((ULONG32)m_count); + return m_table.GetCount(); } // These operators are unsafe b/c they have no typesafety. @@ -1791,13 +1779,8 @@ class CordbHashTable : private CHashTableAndData CordbBase *UnsafeGetBase(ULONG_PTR id, BOOL fFab = TRUE); CordbBase *UnsafeRemoveBase(ULONG_PTR id); - CordbBase *UnsafeFindFirst(HASHFIND *find); - CordbBase *UnsafeFindNext(HASHFIND *find); - - // Unlocked versions don't assert that the lock us held. - CordbBase *UnsafeUnlockedFindFirst(HASHFIND *find); - CordbBase *UnsafeUnlockedFindNext(HASHFIND *find); - + Iterator Begin(); + Iterator End(); }; @@ -1811,9 +1794,7 @@ class CordbSafeHashTable : public CordbHashTable { public: #ifndef DACCESS_COMPILE - CordbSafeHashTable(ULONG size) : CordbHashTable(size) - { - } + CordbSafeHashTable() = default; #endif // Typesafe wrappers HRESULT AddBase(T * pBase) { return UnsafeAddBase(pBase); } @@ -1831,9 +1812,6 @@ class CordbSafeHashTable : public CordbHashTable T* RemoveBase(ULONG_PTR id) { return static_cast(UnsafeRemoveBase(id)); } - T* FindFirst(HASHFIND *find) { return static_cast(UnsafeFindFirst(find)); } - T* FindNext(HASHFIND *find) { return static_cast(UnsafeFindNext(find)); } - // Neuter all items and clear void NeuterAndClear(RSLock * pLock); @@ -1855,7 +1833,6 @@ public ICorDebugAssemblyEnum CordbHashTableEnum( CordbBase * pOwnerObj, NeuterList * pOwnerList, - CordbHashTable *table, const GUID &id); public: @@ -2007,39 +1984,16 @@ public ICorDebugAssemblyEnum NeuterList * m_pOwnerNeuterList; - CordbHashTable *m_table; - bool m_started; - bool m_done; - HASHFIND m_hashfind; + RSPtrArray m_items; REFIID m_guid; ULONG m_iCurElt; - ULONG m_count; - BOOL m_fCountInit; #ifdef _DEBUG - // timestampt of hashtable when we start enumerating it. Useful for detecting if the table - // changes underneath us. - int m_DbgChangeCount; void AssertValid(); #else void AssertValid() { } #endif -private: - //These factor code between Next & Skip - HRESULT PrepForEnum(CordbBase **pBase); - - // Note that the set of types advanced by Pre & by Post are disjoint, and - // that the union of these two sets are all possible types enuerated by - // the CordbHashTableEnum. - HRESULT AdvancePreAssign(CordbBase **pBase); - HRESULT AdvancePostAssign(CordbBase **pBase, - CordbBase **b, - CordbBase **bEnd); - - // This factors some code that initializes the module enumerator. - HRESULT SetupModuleEnum(); - }; @@ -3975,7 +3929,6 @@ class CordbProcess : CORDB_ADDRESS m_minPatchAddr; //smallest patch in table CORDB_ADDRESS m_maxPatchAddr; - // @todo port : if slots of CHashTable change, so should these #define DPT_TERMINATING_INDEX (UINT32_MAX) // Index into m_pPatchTable of the first patch (first used entry). ULONG m_iFirstPatch; @@ -4433,69 +4386,57 @@ class CordbModule : public CordbBase, }; -struct CordbHangingField +class EMPTY_BASES CordbHangingFieldTraits : public NonDacAwareSHashTraits> { - FREEHASHENTRY entry; - FieldData data; +public: + typedef mdFieldDef key_t; + static key_t GetKey(const FieldData &element) { return element.m_fldMetadataToken; } + static BOOL Equals(key_t left, key_t right) { return left == right; } + static count_t Hash(key_t key) { return key; } + static FieldData Null() { return {}; } + static FieldData Deleted() + { + FieldData element = {}; + element.m_fldMetadataToken = static_cast(-1); + return element; + } + static bool IsNull(const FieldData &element) { return element.m_fldMetadataToken == 0; } + static bool IsDeleted(const FieldData &element) { return element.m_fldMetadataToken == static_cast(-1); } }; // A hashtable for storing EnC hanging field information // FieldData.m_fldMetadataToken is the key -class CordbHangingFieldTable : public CHashTableAndData +class CordbHangingFieldTable { - private: - - BOOL Cmp(SIZE_T k1, const HASHENTRY *pc2) - { - LIMITED_METHOD_CONTRACT; - return (ULONG)(UINT_PTR)(k1) != - (reinterpret_cast(pc2))->data.m_fldMetadataToken; - } - - ULONG HASH(mdFieldDef fldToken) - { - LIMITED_METHOD_CONTRACT; - return fldToken; - } - - SIZE_T KEY(mdFieldDef fldToken) - { - return (SIZE_T)fldToken; - } + SHash m_table; public: #ifndef DACCESS_COMPILE - CordbHangingFieldTable() : CHashTableAndData(11) + void Clear() { - NewInit(11, sizeof(CordbHangingField), 11); + m_table.RemoveAll(); } - FieldData * AddFieldInfo(FieldData * pInfo) + const FieldData * AddFieldInfo(const FieldData * pInfo) { _ASSERTE(pInfo != NULL); - CordbHangingField *pEntry = (CordbHangingField *)Add(HASH(pInfo->m_fldMetadataToken)); - pEntry->data = *pInfo; // copy everything over - - // Return a pointer to the data - return &(pEntry->data); + m_table.Add(*pInfo); + return GetFieldInfo(pInfo->m_fldMetadataToken); } void RemoveFieldInfo(mdFieldDef fldToken) { - CordbHangingField *entry = (CordbHangingField*)Find(HASH(fldToken), KEY(fldToken)); - _ASSERTE(entry != NULL); - Delete(HASH(fldToken), (HASHENTRY*)entry); + m_table.Remove(fldToken); } #endif // #ifndef DACCESS_COMPILE - FieldData * GetFieldInfo(mdFieldDef fldToken) + const FieldData * GetFieldInfo(mdFieldDef fldToken) const { - CordbHangingField * entry = (CordbHangingField *)Find(HASH(fldToken), KEY(fldToken)); - return (entry!=NULL?&(entry->data):NULL); + return m_table.LookupPtr(fldToken); } }; @@ -4765,7 +4706,7 @@ class CordbType : public CordbBase, public ICorDebugType, public ICorDebugType2 // These are available after Init() has been called.... HRESULT GetUnboxedObjectSize(ULONG32 *res); - HRESULT GetFieldInfo(mdFieldDef fldToken, FieldData ** ppFieldData); + HRESULT GetFieldInfo(mdFieldDef fldToken, const FieldData ** ppFieldData); CordbAppDomain *GetAppDomain() { return m_appdomain; } @@ -4995,10 +4936,10 @@ class CordbClass : public CordbBase, public ICorDebugClass, public ICorDebugClas DacDbiArrayList * pFieldList, mdTypeDef classToken, mdFieldDef fldToken, - FieldData ** ppFieldData); + const FieldData ** ppFieldData); static HRESULT GetStaticFieldValue2(CordbModule * pModule, - FieldData * pFieldData, + const FieldData * pFieldData, BOOL fEnCHangingField, const Instantiation * pInst, ICorDebugFrame * pFrame, @@ -5010,12 +4951,12 @@ class CordbClass : public CordbBase, public ICorDebugClass, public ICorDebugClas // Get information about a field that was added by EnC HRESULT GetEnCHangingField(mdFieldDef fldToken, - FieldData ** ppFieldData, + const FieldData ** ppFieldData, CordbObjectValue * pObject); private: // Get information via the DAC about a field added with Edit and Continue. - FieldData * GetEnCFieldFromDac(BOOL fStatic, + const FieldData * GetEnCFieldFromDac(BOOL fStatic, CordbObjectValue * pObject, mdFieldDef fieldToken); @@ -5029,7 +4970,7 @@ class CordbClass : public CordbBase, public ICorDebugClass, public ICorDebugClas public: - HRESULT GetFieldInfo(mdFieldDef fldToken, FieldData ** ppFieldData); + HRESULT GetFieldInfo(mdFieldDef fldToken, const FieldData ** ppFieldData); // If you want to force the init to happen even if we think the class // is up to date, set fForceInit to TRUE diff --git a/src/coreclr/debug/di/rspriv.inl b/src/coreclr/debug/di/rspriv.inl index 755de8d2bc7a64..42f118e0b9a344 100644 --- a/src/coreclr/debug/di/rspriv.inl +++ b/src/coreclr/debug/di/rspriv.inl @@ -92,7 +92,10 @@ RSInitHolder::~RSInitHolder() if (m_pObject != NULL) { CordbProcess * pProcess = m_pObject->GetProcess(); - RSLockHolder lockHolder(pProcess->GetProcessLock()); + RSLock *pLock = pProcess == nullptr ? nullptr : pProcess->GetProcessLock(); + RSLockHolder lockHolder(pLock, FALSE); + if (pLock != nullptr) + lockHolder.Acquire(); m_pObject->Neuter(); @@ -630,19 +633,14 @@ inline void CordbSafeHashTable::CopyToArray(RSPtrArray * pArray) pArray->AllocOrThrow(count); - HASHFIND find; UINT32 idx = 0; - T * pCordbBase = FindFirst(&find); - while(idx < count) + for (Iterator iterator = Begin(), end = End(); iterator != end; iterator++) { - pArray->Assign(idx, pCordbBase); - idx++; - pCordbBase = FindNext(&find); + pArray->Assign(idx++, static_cast(*iterator)); } - // Assert is at end. - _ASSERTE(pCordbBase == NULL); + _ASSERTE(idx == count); } // Empty the contents of the hash to an array. Array gets ownersship. @@ -660,25 +658,21 @@ inline void CordbSafeHashTable::TransferToArray(RSPtrArray * pArray) { // Assumes caller has necessary locks - HASHFIND find; UINT32 count = GetCount(); UINT32 idx = 0; pArray->AllocOrThrow(count); - while(idx < count) + for (Iterator iterator = Begin(), end = End(); iterator != end; iterator++) { - T * pCordbBase = FindFirst(&find); - _ASSERTE(pCordbBase != NULL); - pArray->Assign(idx, pCordbBase); + T *pCordbBase = static_cast(*iterator); + pArray->Assign(idx++, pCordbBase); - idx++; - // We're removing while iterating the collection. - // But we reset the iteration each time by calling FindFirst. RemoveBase((ULONG_PTR)pCordbBase->m_id); // this will call release, adjust GetCount() } // Assert is at end. + _ASSERTE(idx == count); _ASSERTE(GetCount() == 0); } @@ -697,19 +691,11 @@ inline void CordbSafeHashTable::NeuterAndClear(RSLock * pLock) { _ASSERTE(pLock->HasLock()); - HASHFIND find; - UINT32 count = GetCount(); - UINT32 idx = 0; - - while(idx < count) + for (Iterator iterator = Begin(), end = End(); iterator != end; iterator++) { - T * pCordbBase = FindFirst(&find); - _ASSERTE(pCordbBase != NULL); + T *pCordbBase = static_cast(*iterator); pCordbBase->Neuter(); - idx++; - // We're removing while iterating the collection. - // But we reset the iteration each time by calling FindFirst. RemoveBase((ULONG_PTR)pCordbBase->m_id); // this will call release, adjust GetCount() } diff --git a/src/coreclr/debug/di/rstype.cpp b/src/coreclr/debug/di/rstype.cpp index b5585105ea754f..3320304a42ae9a 100644 --- a/src/coreclr/debug/di/rstype.cpp +++ b/src/coreclr/debug/di/rstype.cpp @@ -78,7 +78,7 @@ HRESULT CordbType::GetStaticFieldValue(mdFieldDef fieldDef, IfFailThrow(hr); // Lookup the field given its metadata token. - FieldData * pFieldData; + const FieldData * pFieldData; hr = GetFieldInfo(fieldDef, &pFieldData); @@ -123,7 +123,7 @@ CordbType::CordbType(CordbAppDomain *appdomain, CorElementType et, unsigned int m_appdomain(appdomain), m_pClass(NULL), m_rank(rank), - m_spinetypes(2), + m_spinetypes(), m_objectSize(0), m_fieldInfoNeedsInit(TRUE) { @@ -152,7 +152,7 @@ CordbType::CordbType(CordbAppDomain *appdomain, CorElementType et, CordbClass *c m_appdomain(appdomain), m_pClass(cls), m_rank(0), - m_spinetypes(2), + m_spinetypes(), m_objectSize(0), m_fieldInfoNeedsInit(TRUE) { @@ -180,7 +180,7 @@ CordbType::CordbType(CordbType *tycon, CordbType *tyarg) m_appdomain(tycon->m_appdomain), m_pClass(tycon->m_pClass), m_rank(tycon->m_rank), - m_spinetypes(2), + m_spinetypes(), m_objectSize(0), m_fieldInfoNeedsInit(TRUE) // tyarg is added as part of instantiation -see below... @@ -2488,7 +2488,7 @@ HRESULT CordbType::GetTypeID(COR_TYPEID *pId) // S_OK on success. CORDBG_E_ENC_HANGING_FIELD for EnC fields (common case) // Other errors on failure case. //----------------------------------------------------------------------------- -HRESULT CordbType::GetFieldInfo(mdFieldDef fldToken, FieldData ** ppFieldData) +HRESULT CordbType::GetFieldInfo(mdFieldDef fldToken, const FieldData ** ppFieldData) { INTERNAL_SYNC_API_ENTRY(GetProcess()); // HRESULT hr = S_OK; diff --git a/src/coreclr/debug/ee/controller.cpp b/src/coreclr/debug/ee/controller.cpp index 589b513883a2c6..0b82e2bc1a7a7d 100644 --- a/src/coreclr/debug/ee/controller.cpp +++ b/src/coreclr/debug/ee/controller.cpp @@ -18,6 +18,8 @@ #include "../../vm/methoditer.h" #include "../../vm/tailcallhelp.h" +#include + const char *GetTType( TraceType tt); #define IsSingleStep(exception) ((exception) == EXCEPTION_SINGLE_STEP) @@ -276,7 +278,88 @@ SharedPatchBypassBuffer* DebuggerControllerPatch::GetOrCreateSharedPatchBypassBu #endif // !FEATURE_EMULATE_SINGLESTEP #endif // !DACCESS_COMPILE -// @todo - remove all this splicing trash +#ifndef DACCESS_COMPILE +static_assert(std::is_trivially_copyable::value); + +DebuggerPatchTable::~DebuggerPatchTable() +{ + if (m_pcEntries != 0) + CNewZeroData::Free(reinterpret_cast(m_pcEntries), 0); +} + +void DebuggerPatchTable::EnsureList(DebuggerPatchKey key) +{ + if (m_patchLists.LookupPtr(key) == nullptr) + { + DebuggerPatchListEntry entry = {}; + entry.key = key; + entry.first = DPT_INVALID_SLOT; + m_patchLists.Add(entry); + } +} + +DebuggerControllerPatch *DebuggerPatchTable::AddPatch(DebuggerPatchKey key) +{ + if (m_iFree == DPT_INVALID_SLOT) + { + S_UINT32 count = S_UINT32(m_iEntries) + S_UINT32(m_iEntries / 2) + S_UINT32(17); + S_UINT32 size = count * S_UINT32(sizeof(DebuggerControllerPatch)); + if (count.IsOverflow() || size.IsOverflow() || size.Value() > INT_MAX) + ThrowOutOfMemory(); + + BYTE *memory = CNewZeroData::Alloc(static_cast(size.Value()), 0); + if (memory == nullptr) + ThrowOutOfMemory(); + + if (m_pcEntries != 0) + { + memcpy(memory, reinterpret_cast(m_pcEntries), m_iEntries * sizeof(DebuggerControllerPatch)); + CNewZeroData::Free(reinterpret_cast(m_pcEntries), 0); + } + m_pcEntries = reinterpret_cast(memory); + for (ULONG index = m_iEntries; index < count.Value(); index++) + { + DebuggerControllerPatch *patch = EntryPtr(index); + patch->iFree = index + 1; + } + EntryPtr(count.Value() - 1)->iFree = DPT_INVALID_SLOT; + m_iFree = m_iEntries; + m_iEntries = count.Value(); + } + + // Allocate the index before taking a slot so OOM leaves the table unchanged. + EnsureList(key); + ULONG index = m_iFree; + DebuggerControllerPatch *patch = EntryPtr(index); + m_iFree = patch->iFree; + *patch = {}; + patch->iPrev = DPT_INVALID_SLOT; + bool updated = m_patchLists.Update(key, [&](DebuggerPatchListEntry &list) + { + patch->iNext = list.first; + if (list.first != DPT_INVALID_SLOT) + EntryPtr(list.first)->iPrev = index; + list.first = index; + }); + _ASSERTE(updated); + return patch; +} + +void DebuggerPatchTable::LinkPatch(DebuggerControllerPatch *patch) +{ + ULONG index = ItemIndex(patch); + patch->iPrev = DPT_INVALID_SLOT; + bool updated = m_patchLists.Update(Key(patch), [&](DebuggerPatchListEntry &list) + { + patch->iNext = list.first; + if (list.first != DPT_INVALID_SLOT) + EntryPtr(list.first)->iPrev = index; + list.first = index; + }); + _ASSERTE(updated); +} +#endif // DACCESS_COMPILE + // This Sort/Splice stuff just reorders the patches within a particular chain such // that when we iterate through by calling GetPatch() and GetNextPatch(DebuggerControllerPatch), // we'll get patches in increasing order of DebuggerControllerTypes. @@ -288,7 +371,7 @@ void DebuggerPatchTable::SortPatchIntoPatchList(DebuggerControllerPatch **ppPatc LOG((LF_CORDB, LL_EVERYTHING, "DPT::SPIPL ppPatch: %p, pPatch: %p \n", ppPatch, (*ppPatch))); #ifdef _DEBUG DebuggerControllerPatch *patchFirst - = (DebuggerControllerPatch *) Find(Hash((*ppPatch)), Key((*ppPatch))); + = FindPatch(Key(*ppPatch)); _ASSERTE(patchFirst == (*ppPatch)); _ASSERTE((*ppPatch)->controller->GetDCType() != DEBUGGER_CONTROLLER_STATIC); #endif //_DEBUG @@ -301,14 +384,6 @@ void DebuggerPatchTable::SortPatchIntoPatchList(DebuggerControllerPatch **ppPatc return; } - // If we decide to reorder the list, we'll need to keep the element - // indexed by the hash function as the (sorted)first item. Everything else - // chains off this element, can thus stay put. - // Thus, either the element we just added is already sorted, or else we'll - // have to move it elsewhere in the list, meaning that we'll have to swap - // the second item & the new item, so that the index points to the proper - // first item in the list. - //use Cur ptr for case where patch gets appended to list DebuggerControllerPatch *patchCur = patchNext; @@ -348,56 +423,56 @@ void DebuggerPatchTable::SortPatchIntoPatchList(DebuggerControllerPatch **ppPatc // the patch back somewhere else. void DebuggerPatchTable::SpliceOutOfList(DebuggerControllerPatch *patch) { - // We need to get iHash, the index of the ptr within - // m_piBuckets, ie it's entry in the hashtable. - ULONG iHash = Hash(patch) % m_iBuckets; - ULONG iElement = m_piBuckets[iHash]; - DebuggerControllerPatch *patchFirst - = (DebuggerControllerPatch *) EntryPtr(iElement); + DebuggerPatchKey key = Key(patch); + _ASSERTE(m_patchLists.LookupPtr(key) != nullptr); // Fix up pointers to chain - if (patchFirst == patch) + if (patch->iPrev == DPT_INVALID_SLOT) { // The first patch shouldn't have anything behind it. - _ASSERTE(patch->entry.iPrev == DPT_INVALID_SLOT); + _ASSERTE(patch->iPrev == DPT_INVALID_SLOT); - if (patch->entry.iNext != DPT_INVALID_SLOT) + if (patch->iNext == DPT_INVALID_SLOT) { - m_piBuckets[iHash] = patch->entry.iNext; + m_patchLists.Remove(key); } else { - m_piBuckets[iHash] = DPT_INVALID_SLOT; + bool updated = m_patchLists.Update(key, [&](DebuggerPatchListEntry &entry) + { + entry.first = patch->iNext; + }); + _ASSERTE(updated); } } - if (patch->entry.iNext != DPT_INVALID_SLOT) + if (patch->iNext != DPT_INVALID_SLOT) { - EntryPtr(patch->entry.iNext)->iPrev = patch->entry.iPrev; + EntryPtr(patch->iNext)->iPrev = patch->iPrev; } - if (patch->entry.iPrev != DPT_INVALID_SLOT) + if (patch->iPrev != DPT_INVALID_SLOT) { - EntryPtr(patch->entry.iNext)->iNext = patch->entry.iNext; + EntryPtr(patch->iPrev)->iNext = patch->iNext; } - patch->entry.iNext = DPT_INVALID_SLOT; - patch->entry.iPrev = DPT_INVALID_SLOT; + patch->iNext = DPT_INVALID_SLOT; + patch->iPrev = DPT_INVALID_SLOT; } void DebuggerPatchTable::SpliceInBackOf(DebuggerControllerPatch *patchAppend, DebuggerControllerPatch *patchEnd) { - ULONG iAppend = ItemIndex((HASHENTRY*)patchAppend); - ULONG iEnd = ItemIndex((HASHENTRY*)patchEnd); + ULONG iAppend = ItemIndex(patchAppend); + ULONG iEnd = ItemIndex(patchEnd); - patchAppend->entry.iPrev = iEnd; - patchAppend->entry.iNext = patchEnd->entry.iNext; + patchAppend->iPrev = iEnd; + patchAppend->iNext = patchEnd->iNext; - if (patchAppend->entry.iNext != DPT_INVALID_SLOT) - EntryPtr(patchAppend->entry.iNext)->iPrev = iAppend; + if (patchAppend->iNext != DPT_INVALID_SLOT) + EntryPtr(patchAppend->iNext)->iPrev = iAppend; - patchEnd->entry.iNext = iAppend; + patchEnd->iNext = iAppend; } #endif @@ -634,13 +709,8 @@ DebuggerControllerPatch *DebuggerPatchTable::AddPatchForMethodDef(DebuggerContro LOG( (LF_CORDB,LL_INFO10000,"DPT:APFMD 0x%x with dji %p, %s offset 0x%zx controller:%p AD:%p\n", md, dji, (offsetIsIL ? "IL" : "native"), offset, controller, pAppDomain)); - DebuggerFunctionKey key; - - key.module = module; - key.md = md; - // Get a new uninitialized patch object - DebuggerControllerPatch *patch = (DebuggerControllerPatch *) Add(HashKey(&key)); + DebuggerControllerPatch *patch = AddPatch({0, dac_cast(module), md}); if (patch == NULL) ThrowOutOfMemory(); @@ -747,7 +817,7 @@ DebuggerControllerPatch *DebuggerPatchTable::AddPatchForAddress(DebuggerControll // get new uninitialized patch object DebuggerControllerPatch *patch = - (DebuggerControllerPatch *) Add(HashAddress(address)); + AddPatch({dac_cast(address), 0, 0}); if (patch == NULL) { @@ -826,11 +896,12 @@ void DebuggerPatchTable::BindPatch(DebuggerControllerPatch *patch, CORDB_ADDRESS //zeroing out the opcode field (see lengthy comment above) // Since the patch is double-hashed based off Address, if we change the address, // we must remove and reinsert the patch. - CHashTable::Delete(HashKey(&patch->key), ItemIndex((HASHENTRY*)patch)); + EnsureList({dac_cast(address), 0, 0}); + SpliceOutOfList(patch); patch->address = address; - CHashTable::Add(HashAddress(address), ItemIndex((HASHENTRY*)patch)); + LinkPatch(patch); SortPatchIntoPatchList(&patch); @@ -861,13 +932,12 @@ void DebuggerPatchTable::UnbindPatch(DebuggerControllerPatch *patch) // Update it's index entry in the table to use it's unbound key // Since the patch is double-hashed based off Address, if we change the address, // we must remove and reinsert the patch. - CHashTable::Delete( HashAddress(patch->address), - ItemIndex((HASHENTRY*)patch)); + EnsureList({0, dac_cast(patch->key.module), patch->key.md}); + SpliceOutOfList(patch); patch->address = NULL; // we're no longer bound to this address - CHashTable::Add( HashKey(&patch->key), - ItemIndex((HASHENTRY*)patch)); + LinkPatch(patch); _ASSERTE(!patch->IsBound() ); @@ -881,47 +951,17 @@ void DebuggerPatchTable::RemovePatch(DebuggerControllerPatch *patch) patch->DoCleanup(); #endif // !FEATURE_EMULATE_SINGLESTEP - // - // Because of the implementation of CHashTable, we can safely - // delete elements while iterating through the table. This - // behavior is relied upon - do not change to a different - // implementation without considering this fact. - // - Delete(Hash(patch), (HASHENTRY *) patch); + SpliceOutOfList(patch); + patch->refCount = 0; + patch->iFree = m_iFree; + m_iFree = ItemIndex(patch); } DebuggerControllerPatch *DebuggerPatchTable::GetNextPatch(DebuggerControllerPatch *prev) { - ULONG iNext; - HASHENTRY *psEntry; - - // Start at the next entry in the chain. - // @todo - note that: EntryPtr(ItemIndex(x)) == x - iNext = EntryPtr(ItemIndex((HASHENTRY*)prev))->iNext; - - // Search until we hit the end. - while (iNext != UINT32_MAX) - { - // Compare the keys. - psEntry = EntryPtr(iNext); - - // Careful here... we can hash the entries in this table - // by two types of keys. In this type of search, the type - // of the second key (psEntry) does not necessarily - // indicate the type of the first key (prev), so we have - // to check for sure. - DebuggerControllerPatch *pc2 = (DebuggerControllerPatch*)psEntry; - - if (((pc2->address == NULL) && (prev->address == NULL)) || - ((pc2->address != NULL) && (prev->address != NULL))) - if (!Cmp(Key(prev), psEntry)) - return pc2; - - // Advance to the next item in the chain. - iNext = psEntry->iNext; - } - - return NULL; + ULONG next = prev->iNext; + return next == DPT_INVALID_SLOT ? nullptr + : EntryPtr(next); } // Count how many patches are in the table. @@ -933,9 +973,7 @@ int DebuggerPatchTable::GetNumberOfPatches() if ((TADDR)NULL != m_pcEntries) { DebuggerControllerPatch *dcp; - ULONG i = 0; - - while (i++ GetFirstPatch(&f); + ULONG patchIndex; + for (DebuggerControllerPatch *patch = g_patches->GetFirstPatch(&patchIndex); patch != NULL; - patch = g_patches->GetNextPatch(&f)) + patch = g_patches->GetNextPatch(&patchIndex)) { if (patch->controller == this) { @@ -2132,7 +2170,7 @@ BOOL DebuggerController::AddILPatch(AppDomain * pAppDomain, Module *module, // We have to keep the index here instead of the pointer. The loop below adds more patches, // which may cause the patch table to grow and move. - ULONG primaryIndex = g_patches->GetItemIndex((HASHENTRY*)primary); + ULONG primaryIndex = g_patches->GetItemIndex(primary); // Iterate through every existing NativeCodeBlob (with the same EnC version). // This includes generics + prejitted code. @@ -2171,7 +2209,7 @@ BOOL DebuggerController::AddILPatch(AppDomain * pAppDomain, Module *module, { fVersionMatch = TRUE; - primary = (DebuggerControllerPatch *)g_patches->GetEntryPtr(primaryIndex); + primary = g_patches->GetEntryPtr(primaryIndex); // If we're missing JIT info for any then // we won't have applied the bp to every instantiation. That should probably be reported @@ -2357,10 +2395,10 @@ void DebuggerController::RemovePatchesFromModule(Module *pModule, AppDomain *pAp // First find all patches of interest DebuggerController::ControllerLockHolder ch; - HASHFIND f; - for (DebuggerControllerPatch *patch = g_patches->GetFirstPatch(&f); + ULONG patchIndex; + for (DebuggerControllerPatch *patch = g_patches->GetFirstPatch(&patchIndex); patch != NULL; - patch = g_patches->GetNextPatch(&f)) + patch = g_patches->GetNextPatch(&patchIndex)) { // Skip patches not in the specified domain if ((pAppDomain != NULL) && (patch->pAppDomain != pAppDomain)) @@ -2415,10 +2453,10 @@ bool DebuggerController::ModuleHasPatches( Module* pModule ) } // First find all patches of interest - HASHFIND f; - for (DebuggerControllerPatch *patch = g_patches->GetFirstPatch(&f); + ULONG patchIndex; + for (DebuggerControllerPatch *patch = g_patches->GetFirstPatch(&patchIndex); patch != NULL; - patch = g_patches->GetNextPatch(&f)) + patch = g_patches->GetNextPatch(&patchIndex)) { // // This mirrors logic in code:DebuggerController::RemovePatchesFromModule @@ -2865,11 +2903,11 @@ DPOSS_ACTION DebuggerController::ScanForTriggers(CORDB_ADDRESS_TYPE *address, // Annoyingly, TriggerPatch may add patches, which may cause // the patch table to move, which may, in turn, invalidate // the patch (and patchNext) pointers. Store indices, instead. - iEvent = g_patches->GetItemIndex( (HASHENTRY *)patch ); + iEvent = g_patches->GetItemIndex(patch); if (patchNext != NULL) { - iEventNext = g_patches->GetItemIndex((HASHENTRY *)patchNext); + iEventNext = g_patches->GetItemIndex(patchNext); } if (MatchPatch(thread, context, patch)) @@ -2904,16 +2942,14 @@ DPOSS_ACTION DebuggerController::ScanForTriggers(CORDB_ADDRESS_TYPE *address, tpr == TPR_TRIGGER_ONLY_THIS_AND_LOOP) { // Make sure we've still got a valid pointer. - patch = (DebuggerControllerPatch *) - DebuggerController::g_patches->GetEntryPtr( iEvent ); + patch = DebuggerController::g_patches->GetEntryPtr(iEvent); pDcq->dcqEnqueue(patch->controller, TRUE); // @todo Return value } // Make sure we've got a valid pointer in case TriggerPatch // returned false but still caused the table to move. - patch = (DebuggerControllerPatch *) - g_patches->GetEntryPtr( iEvent ); + patch = g_patches->GetEntryPtr(iEvent); // A patch can be deleted as a result of it's being triggered. // The actual deletion of the patch is delayed until after the @@ -2924,7 +2960,7 @@ DPOSS_ACTION DebuggerController::ScanForTriggers(CORDB_ADDRESS_TYPE *address, // "deleted" flag didn't get set when we iterate the patches in DisableAll(). patchNext = g_patches->GetNextPatch(patch); if (patchNext != NULL) - iEventNext = g_patches->GetItemIndex((HASHENTRY *)patchNext); + iEventNext = g_patches->GetItemIndex(patchNext); // Note that ReleasePatch() actually removes the patch if its ref count // reaches 0 after the release. @@ -2945,8 +2981,7 @@ DPOSS_ACTION DebuggerController::ScanForTriggers(CORDB_ADDRESS_TYPE *address, } else if (patchNext != NULL) { - patch = (DebuggerControllerPatch *) - g_patches->GetEntryPtr(iEventNext); + patch = g_patches->GetEntryPtr(iEventNext); } else { diff --git a/src/coreclr/debug/ee/controller.h b/src/coreclr/debug/ee/controller.h index b0f510cf9de162..34233c35236a4c 100644 --- a/src/coreclr/debug/ee/controller.h +++ b/src/coreclr/debug/ee/controller.h @@ -363,8 +363,7 @@ enum DebuggerPatchKind { PATCH_KIND_IL_PRIMARY, PATCH_KIND_IL_REPLICA, PATCH_KIN // this should contain all the info that's needed over the course of a // patch's lifetime. // -// FREEHASHENTRY entry: Three ULONGs, this is required -// by the underlying hashtable implementation +// ULONG iPrev, iNext, iFree: Slot indices for the patch chains and free list. // DWORD opcode: A nonzero opcode && address field means that // the patch has been applied to something. This may not be a full // opcode, it is possible it is a partial opcode. @@ -419,7 +418,9 @@ struct DebuggerControllerPatch friend class DebuggerPatchTable; friend class DebuggerController; - FREEHASHENTRY entry; + ULONG iPrev; + ULONG iNext; + ULONG iFree; DebuggerController *controller; DebuggerFunctionKey key; SIZE_T offset; @@ -653,100 +654,102 @@ bool ShouldBindPatchToMethodDesc(MethodDesc* pMD, MethodDesc* pMethodDescFilter) #define DPT_INVALID_SLOT (UINT32_MAX) #define DPT_DEFAULT_TRACE_TYPE TRACE_OTHER -/* Although CHashTableAndData can grow, we always use a fixed number of buckets. - * This is problematic for tables like the patch table which are usually small, but - * can become huge. When the number of entries far exceeds the number of buckets, - * lookup and addition basically degrade into linear searches. There is a trade-off - * here between wasting memory for unused buckets, and performance of large tables. - * Also note that the number of buckets should be a prime number. -*/ -#define DPT_HASH_BUCKETS 1103 +struct DebuggerPatchKey +{ + TADDR address; + TADDR module; + mdMethodDef token; +}; + +struct DebuggerPatchListEntry : DebuggerHeapHashEntry +{ + DebuggerPatchKey key; + ULONG first; +}; -class DebuggerPatchTable : private CHashTableAndData +class EMPTY_BASES DebuggerPatchListTraits : public DefaultSHashTraits +{ +public: + typedef DebuggerPatchKey key_t; + static key_t GetKey(const element_t &element) { return element.key; } + static BOOL Equals(key_t left, key_t right) + { + return left.address == right.address && left.module == right.module && left.token == right.token; + } + static count_t Hash(key_t key) + { + TADDR value = key.address != 0 ? key.address : key.module; + return HashCOUNT_T(key.token, DebuggerHashPointer(value)); + } + static element_t Null() { return {}; } + static element_t Deleted() + { + element_t element = {}; + element.key.address = static_cast(-1); + return element; + } + static bool IsNull(const element_t &element) { return element.key.address == 0 && element.key.module == 0; } + static bool IsDeleted(const element_t &element) { return element.key.address == static_cast(-1); } +}; + +class DebuggerPatchTable { VPTR_BASE_CONCRETE_VTABLE_CLASS(DebuggerPatchTable); friend struct ::cdac_data; public: +#ifndef DACCESS_COMPILE + virtual ~DebuggerPatchTable(); +#else virtual ~DebuggerPatchTable() = default; +#endif friend class DebuggerRCThread; private: //incremented so that we can get DPT-wide unique PIDs. // pid = Patch ID. SIZE_T m_patchId; - // Given a patch, retrieves the correct key. The return value of this function is passed to Cmp(), Find(), etc. - SIZE_T Key(DebuggerControllerPatch *patch) + // Slots retain their indices when the SHash index grows. Readers in another process + // consume only this zero-initialized, contiguous storage and its published capacity. + TADDR m_pcEntries; + ULONG m_iEntries; + ULONG m_iFree; + SHash m_patchLists; + + static DebuggerPatchKey Key(DebuggerControllerPatch *patch) { LIMITED_METHOD_DAC_CONTRACT; - - // Most clients of CHashTable pass a host pointer as the key. However, the key really could be - // anything. In our case, the key can either be a host pointer of type DebuggerFunctionKey or - // the address of the patch. - if (patch->address == NULL) - { - return (SIZE_T)(&patch->key); - } - else - { - return (SIZE_T)(dac_cast(patch->address)); - } + return patch->address == nullptr + ? DebuggerPatchKey{0, dac_cast(patch->key.module), patch->key.md} + : DebuggerPatchKey{dac_cast(patch->address), 0, 0}; } - // Given two DebuggerControllerPatches, tells - // whether they are equal or not. Does this by comparing the correct - // key. - // BYTE* pc1: If pc2 is hashed by address, - // pc1 is an address. If - // pc2 is hashed by DebuggerFunctionKey, - // pc1 is a DebuggerFunctionKey - //Returns true if the two patches are equal, false otherwise - BOOL Cmp(SIZE_T k1, const HASHENTRY * pc2) + DebuggerControllerPatch *FindPatch(DebuggerPatchKey key) { LIMITED_METHOD_DAC_CONTRACT; - - DebuggerControllerPatch * pPatch2 = dac_cast(const_cast(pc2)); - - if (pPatch2->address == NULL) - { - // k1 is a host pointer of type DebuggerFunctionKey. - DebuggerFunctionKey * pKey1 = reinterpret_cast(k1); - - return ((pKey1->module != pPatch2->key.module) || (pKey1->md != pPatch2->key.md)); - } - else - { - return ((SIZE_T)(dac_cast(pPatch2->address)) != k1); - } + const DebuggerPatchListEntry *entry = m_patchLists.LookupPtr(key); + return entry == nullptr || entry->first == DPT_INVALID_SLOT ? nullptr + : EntryPtr(entry->first); } - //Computes a hash value based on an address - ULONG HashAddress(PTR_CORDB_ADDRESS_TYPE address) + DebuggerControllerPatch *EntryPtr(ULONG index) { LIMITED_METHOD_DAC_CONTRACT; - return (ULONG)(SIZE_T)(dac_cast(address)); + return dac_cast(m_pcEntries + index * sizeof(DebuggerControllerPatch)); } - //Computes a hash value based on a DebuggerFunctionKey - ULONG HashKey(DebuggerFunctionKey * pKey) + ULONG ItemIndex(DebuggerControllerPatch *patch) { - SUPPORTS_DAC; - return HashPtr(pKey->md, pKey->module); + LIMITED_METHOD_DAC_CONTRACT; + return static_cast((dac_cast(patch) - m_pcEntries) / sizeof(DebuggerControllerPatch)); } - //Computes a hash value from a patch, using the address field - // if the patch is hashed by address, using the DebuggerFunctionKey - // otherwise - ULONG Hash(DebuggerControllerPatch * pPatch) - { - SUPPORTS_DAC; - - if (pPatch->address == NULL) - return HashKey(&(pPatch->key)); - else - return HashAddress(pPatch->address); - } +#ifndef DACCESS_COMPILE + DebuggerControllerPatch *AddPatch(DebuggerPatchKey key); + void EnsureList(DebuggerPatchKey key); + void LinkPatch(DebuggerControllerPatch *patch); +#endif //Public Members public: enum { @@ -756,7 +759,8 @@ class DebuggerPatchTable : private CHashTableAndData #ifndef DACCESS_COMPILE - DebuggerPatchTable() : CHashTableAndData(DPT_HASH_BUCKETS) { } + DebuggerPatchTable() : m_patchId(DCP_PATCHID_FIRST_VALID), m_pcEntries(0), + m_iEntries(0), m_iFree(DPT_INVALID_SLOT) { } HRESULT Init() { @@ -764,8 +768,7 @@ class DebuggerPatchTable : private CHashTableAndData m_patchId = DCP_PATCHID_FIRST_VALID; - SUPPRESS_ALLOCATION_ASSERTS_IN_THIS_SCOPE; - return NewInit(17, sizeof(DebuggerControllerPatch), 101); + return S_OK; } // Assuming that the chain of patches (as defined by all the @@ -818,16 +821,12 @@ class DebuggerPatchTable : private CHashTableAndData // We can hopefully remove them. static SIZE_T GetOffsetOfEntries() { - // assert that we the offsets of these fields in the base class is - // the same as the offset of this field in this class. - _ASSERTE((void*)(DebuggerPatchTable*)NULL == (void*)(CHashTableAndData*)NULL); - return helper_GetOffsetOfEntries(); + return offsetof(DebuggerPatchTable, m_pcEntries); } static SIZE_T GetOffsetOfCount() { - _ASSERTE((void*)(DebuggerPatchTable*)NULL == (void*)(CHashTableAndData*)NULL); - return helper_GetOffsetOfCount(); + return offsetof(DebuggerPatchTable, m_iEntries); } // GetPatch find the first patch in the hash table @@ -839,12 +838,7 @@ class DebuggerPatchTable : private CHashTableAndData // this Module,mdMethodDef pair DebuggerControllerPatch *GetPatch(Module *module, mdToken md) { - DebuggerFunctionKey key; - - key.module = module; - key.md = md; - - return reinterpret_cast(Find(HashKey(&key), (SIZE_T)&key)); + return FindPatch({0, dac_cast(module), md}); } #endif // #ifndef DACCESS_COMPILE @@ -856,44 +850,47 @@ class DebuggerPatchTable : private CHashTableAndData SUPPORTS_DAC; ARM_ONLY(_ASSERTE(dac_cast(address) & THUMB_CODE)); - DebuggerControllerPatch * pPatch = - dac_cast(Find(HashAddress(address), (SIZE_T)(dac_cast(address)))); - - return pPatch; + return FindPatch({dac_cast(address), 0, 0}); } DebuggerControllerPatch *GetNextPatch(DebuggerControllerPatch *prev); // Find the first patch in the patch table, and store - // index info in info. Along with GetNextPatch, this can - // iterate through the whole patch table. Note that since the - // hashtable operates via iterating through all the contents - // of all the buckets, if you add an entry while iterating - // through the table, you may or may not iterate across + // the next slot index in pIndex. Along with GetNextPatch, this can + // iterate through the whole patch table by scanning its slots. + // If you add an entry while iterating + // through the table, you may or may not iterate across // the new entries. You will iterate through all the entries // that were present at the beginning of the run. You // safely delete anything you've already iterated by, anything // else is kinda risky. - DebuggerControllerPatch * GetFirstPatch(HASHFIND * pInfo) + DebuggerControllerPatch *GetFirstPatch(ULONG *pIndex) { SUPPORTS_DAC; - return dac_cast(FindFirstEntry(pInfo)); + *pIndex = 0; + return GetNextPatch(pIndex); } // Along with GetFirstPatch, this can iterate through // the whole patch table. See GetFirstPatch for more info // on the rules of iterating through the table. - DebuggerControllerPatch * GetNextPatch(HASHFIND * pInfo) + DebuggerControllerPatch *GetNextPatch(ULONG *pIndex) { SUPPORTS_DAC; - return dac_cast(FindNextEntry(pInfo)); + while (*pIndex < m_iEntries) + { + DebuggerControllerPatch *patch = EntryPtr((*pIndex)++); + if (!patch->IsFree()) + return patch; + } + return nullptr; } // Used by DebuggerController to translate an index // of a patch into a direct pointer. - inline HASHENTRY * GetEntryPtr(ULONG iEntry) + inline DebuggerControllerPatch *GetEntryPtr(ULONG iEntry) { SUPPORTS_DAC; @@ -902,11 +899,11 @@ class DebuggerPatchTable : private CHashTableAndData // Used by DebuggerController to grab indices of patches // rather than holding direct pointers to them. - inline ULONG GetItemIndex(HASHENTRY * p) + inline ULONG GetItemIndex(DebuggerControllerPatch *patch) { SUPPORTS_DAC; - return ItemIndex(p); + return ItemIndex(patch); } // Count how many patches are in the table. diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index 745b9bdbcb2cdf..5d2fe628cbe23b 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -65,6 +65,31 @@ InteropSafe interopsafe; #ifndef DACCESS_COMPILE +void *DebuggerHeapHashEntry::operator new[](size_t size, const std::nothrow_t &) noexcept +{ + if (size > INT_MAX) + return nullptr; + return CNewZeroData::Alloc(static_cast(size), 0); +} + +void *DebuggerHeapHashEntry::operator new[](size_t size) +{ + void *memory = operator new[](size, std::nothrow); + if (memory == nullptr) + ThrowOutOfMemory(); + return memory; +} + +void DebuggerHeapHashEntry::operator delete[](void *memory) +{ + CNewZeroData::Free(static_cast(memory), 0); +} + +void DebuggerHeapHashEntry::operator delete[](void *memory, const std::nothrow_t &) noexcept +{ + operator delete[](memory); +} + DebuggerRCThread *g_pRCThread = NULL; // Do some compile time checking on the events in DbgIpcEventTypes.h @@ -1062,7 +1087,69 @@ HRESULT Debugger::CheckInitModuleTable() return S_OK; } -// Checks if the m_pModules table has been allocated, and if not does so. +#ifndef DACCESS_COMPILE +HRESULT DebuggerPendingFuncEvalTable::AddPendingEval(Thread *pThread, DebuggerEval *pDE) +{ + CONTRACTL + { + NOTHROW; + GC_NOTRIGGER; + } + CONTRACTL_END; + + _ASSERTE(pThread != nullptr && pDE != nullptr); + _ASSERTE(pDE->m_nextPendingEval == nullptr); + + if (m_table.Update(pThread, [&](DebuggerPendingFuncEval &entry) + { + // Keep newest-first order independently of SHash rehashing. + pDE->m_nextPendingEval = entry.pDE; + entry.pDE = pDE; + })) + { + return S_OK; + } + + DebuggerPendingFuncEval newEntry = {}; + newEntry.pThread = pThread; + newEntry.pDE = pDE; + return m_table.AddNoThrow(newEntry) ? S_OK : E_OUTOFMEMORY; +} + +void DebuggerPendingFuncEvalTable::RemovePendingEval(Thread *pThread) +{ + CONTRACTL + { + NOTHROW; + GC_NOTRIGGER; + } + CONTRACTL_END; + + _ASSERTE(pThread != nullptr); + + DebuggerEval *pDE = nullptr; + bool removeEntry = false; + if (!m_table.Update(pThread, [&](DebuggerPendingFuncEval &entry) + { + pDE = entry.pDE; + if (pDE->m_nextPendingEval == nullptr) + removeEntry = true; + else + entry.pDE = pDE->m_nextPendingEval; + })) + { + return; + } + + if (removeEntry) + { + m_table.Remove(pThread); + } + pDE->m_nextPendingEval = nullptr; +} +#endif // DACCESS_COMPILE + +// Checks if the pending-evaluation table has been allocated, and if not does so. HRESULT Debugger::CheckInitPendingFuncEvalTable() { CONTRACTL @@ -1102,83 +1189,14 @@ ULONG DebuggerMethodInfoTable::CheckDmiTable(void) { LIMITED_METHOD_CONTRACT; - ULONG cApparent = 0; - ULONG cOfficial = 0; - - if (NULL != m_pcEntries) + ULONG count = 0; + for (Iterator iterator = m_table.Begin(), end = m_table.End(); iterator != end; iterator++) { - DebuggerMethodInfoEntry *dcp; - int i = 0; - while (i++ pFD != 0 && - dcp->pFD != (MethodDesc*)0xcdcdcdcd && - dcp->mi != NULL) - { - cApparent++; - - _ASSERTE( dcp->pFD == dcp->mi->m_fd ); - LOG((LF_CORDB, LL_INFO1000, "DMIT::CDT:Entry:0x%p mi:0x%p\nPrevs:\n", - dcp, dcp->mi)); - DebuggerMethodInfo *dmi = dcp->mi->m_prevMethodInfo; - - while(dmi != NULL) - { - LOG((LF_CORDB, LL_INFO1000, "\t0x%p\n", dmi)); - dmi = dmi->m_prevMethodInfo; - } - dmi = dcp->mi->m_nextMethodInfo; - - LOG((LF_CORDB, LL_INFO1000, "Nexts:\n", dmi)); - while(dmi != NULL) - { - LOG((LF_CORDB, LL_INFO1000, "\t0x%p\n", dmi)); - dmi = dmi->m_nextMethodInfo; - } - - LOG((LF_CORDB, LL_INFO1000, "DMIT::CDT:DONE\n", - dcp, dcp->mi)); - } - } - - if (m_piBuckets == 0) - { - LOG((LF_CORDB, LL_INFO1000, "DMIT::CDT: The table is officially empty!\n")); - return cOfficial; - } - - LOG((LF_CORDB, LL_INFO1000, "DMIT::CDT:Looking for official entries:\n")); - - ULONG iNext = m_piBuckets[0]; - ULONG iBucket = 1; - HASHENTRY *psEntry = NULL; - while (TRUE) - { - while (iNext != UINT32_MAX) - { - cOfficial++; - - psEntry = EntryPtr(iNext); - dcp = ((DebuggerMethodInfoEntry *)psEntry); - - LOG((LF_CORDB, LL_INFO1000, "\tEntry:0x%p mi:0x%p @idx:0x%x @bucket:0x%x\n", - dcp, dcp->mi, iNext, iBucket)); - - iNext = psEntry->iNext; - } - - // Advance to the next bucket. - if (iBucket < m_iBuckets) - iNext = m_piBuckets[iBucket++]; - else - break; - } - - LOG((LF_CORDB, LL_INFO1000, "DMIT::CDT:Finished official entries: ****************")); + _ASSERTE(iterator->key.pModule != nullptr); + count++; } - - return cOfficial; + _ASSERTE(count == m_table.GetCount()); + return count; } #endif // _DEBUG_DMI_TABLE @@ -1225,6 +1243,7 @@ DebuggerEval::DebuggerEval(CONTEXT * pContext, DebuggerIPCE_FuncEvalInfo * pEval } m_thread = pEvalInfo->vmThreadToken.GetRawPtr(); + m_nextPendingEval = nullptr; m_evalType = pEvalInfo->funcEvalType; m_methodToken = pEvalInfo->funcMetadataToken; m_classToken = pEvalInfo->funcClassMetadataToken; @@ -4632,7 +4651,7 @@ HRESULT Debugger::MapAndBindFunctionPatches(DebuggerJitInfo *djiNew, CONTRACTL_END; HRESULT hr = S_OK; - HASHFIND hf; + ULONG patchIndex; SIZE_T *pidTableEntry = NULL; SIZE_T pidInCaseTableMoves; Module *pModule = g_pEEInterface->MethodDescGetModule(fd); @@ -4657,9 +4676,9 @@ HRESULT Debugger::MapAndBindFunctionPatches(DebuggerJitInfo *djiNew, DebuggerPatchTable *pPatchTable = DebuggerController::GetPatchTable(); GetBPMappingDuplicates()->Clear(); //dups are tracked per-version - for (DebuggerControllerPatch *dcp = pPatchTable->GetFirstPatch(&hf); + for (DebuggerControllerPatch *dcp = pPatchTable->GetFirstPatch(&patchIndex); dcp != NULL; - dcp = pPatchTable->GetNextPatch( &hf )) + dcp = pPatchTable->GetNextPatch(&patchIndex)) { LOG((LF_CORDB, LL_INFO10000, "D::MABFP: Got patch %p, patchId:0x%zx\n", dcp, dcp->patchId)); @@ -5514,11 +5533,10 @@ void Debugger::UpdateAllModuleJMCFlag(bool fStatus) // and so there is nothing to update. if (m_pModules != NULL) { - HASHFIND f; - for (DebuggerModule * m = m_pModules->GetFirstModule(&f); - m != NULL; - m = m_pModules->GetNextModule(&f)) + for (DebuggerModuleTable::Iterator iterator = m_pModules->Begin(), end = m_pModules->End(); + iterator != end; iterator++) { + DebuggerModule *m = iterator->module; // the primary module may get called multiple times, but that's ok. UpdateModuleJMCFlag(m->GetRuntimeModule(), fStatus); } // end for all modules. @@ -5585,13 +5603,11 @@ void Debugger::SetModuleDefaultJMCStatus(Module * pRuntimeModule, bool fStatus) if (pTable != NULL) { Debugger::DebuggerDataLockHolder debuggerDataLockHolder(g_pDebugger); - HASHFIND info; - - for (DebuggerMethodInfo *dmi = pTable->GetFirstMethodInfo(&info); - dmi != NULL; - dmi = pTable->GetNextMethodInfo(&info)) + for (DebuggerMethodInfoTable::Iterator iterator = pTable->Begin(), end = pTable->End(); + iterator != end; iterator++) { - if (dmi->GetRuntimeModule() == pRuntimeModule) + DebuggerMethodInfo *dmi = iterator->mi; + if (dmi != nullptr && dmi->GetRuntimeModule() == pRuntimeModule) { // This DMI is in this module, so update its status dmi->SetJMCStatus(fStatus); @@ -7560,7 +7576,7 @@ void Debugger::ProcessAnyPendingEvals(Thread *pThread) // FuncEvalComplete event, so if the user asks for another func eval then there will be a new pending eval when we // loop and check again. // - DebuggerPendingFuncEval *pfe; + const DebuggerPendingFuncEval *pfe; while (GetPendingEvals() != NULL && (pfe = GetPendingEvals()->GetPendingEval(pThread)) != NULL) { @@ -9401,13 +9417,11 @@ void Debugger::UnloadModule(Module* pRuntimeModule) DebuggerMethodInfoTable * pTable = GetMethodInfoTable(); if (pTable != NULL) { - HASHFIND info; - - for (DebuggerMethodInfo *dmi = pTable->GetFirstMethodInfo(&info); - dmi != NULL; - dmi = pTable->GetNextMethodInfo(&info)) + for (DebuggerMethodInfoTable::Iterator iterator = pTable->Begin(), end = pTable->End(); + iterator != end; iterator++) { - if (dmi->m_module == pRuntimeModule) + DebuggerMethodInfo *dmi = iterator->mi; + if (dmi != nullptr && dmi->m_module == pRuntimeModule) { dmi->SetJMCStatus(false); } @@ -10656,14 +10670,14 @@ bool Debugger::HandleIPCEvent(DebuggerIPCEvent * pEvent) if (pMethodInfoTable != NULL) { - HASHFIND hashFind; DebuggerDataLockHolder debuggerDataLockHolder(this); - for (DebuggerMethodInfo * pMethodInfo = pMethodInfoTable->GetFirstMethodInfo(&hashFind); - pMethodInfo != NULL; - pMethodInfo = pMethodInfoTable->GetNextMethodInfo(&hashFind)) + for (DebuggerMethodInfoTable::Iterator iterator = pMethodInfoTable->Begin(), end = pMethodInfoTable->End(); + iterator != end; iterator++) { - pMethodInfo->SetJMCStatus(false); + DebuggerMethodInfo *pMethodInfo = iterator->mi; + if (pMethodInfo != nullptr) + pMethodInfo->SetJMCStatus(false); } } LOG((LF_CORDB, LL_EVERYTHING, "Done clearing JMC methods!\n")); @@ -14144,12 +14158,9 @@ HRESULT Debugger::FuncEvalSetup(DebuggerIPCE_FuncEvalInfo *pEvalInfo, if (pDE->m_argData == NULL) { - DeleteInteropSafeExecutable(pDE); + DeleteInteropSafe(pDE); return E_OUTOFMEMORY; } - - // Pass back the address of the argument data area so the right side can write to it for us. - *argDataArea = pDE->m_argData; } // Hijacked evals rewrite the thread's native context to enter FuncEvalHijack when execution resumes. @@ -14199,7 +14210,7 @@ HRESULT Debugger::FuncEvalSetup(DebuggerIPCE_FuncEvalInfo *pEvalInfo, if (FAILED(hr)) { - DeleteInteropSafeExecutable(pDE); // Note this runs the destructor for DebuggerEval, which releases its internal buffers + DeleteInteropSafe(pDE); return hr; } @@ -14207,7 +14218,12 @@ HRESULT Debugger::FuncEvalSetup(DebuggerIPCE_FuncEvalInfo *pEvalInfo, // the process continues. Interpreter evals run from the INTOP_BREAKPOINT handler after // the debugger callback returns — no context modification and no IncThreadsAtUnsafePlaces // needed because the stack remains walkable. - GetPendingEvals()->AddPendingEval(pDE->m_thread, pDE); + hr = GetPendingEvals()->AddPendingEval(pDE->m_thread, pDE); + if (FAILED(hr)) + { + DeleteInteropSafe(pDE); + return hr; + } LOG((LF_CORDB, LL_INFO1000, "D::FES: Non-hijack func eval setup for pDE:%p on thread %p (fInException=%d)\n", pDE, pThread, fInException)); @@ -14216,6 +14232,7 @@ HRESULT Debugger::FuncEvalSetup(DebuggerIPCE_FuncEvalInfo *pEvalInfo, // Return that all went well. Tracing the stack at this point should not show that the func eval is setup, but it // will show a wrong IP, so it shouldn't be done. + *argDataArea = pDE->m_argData; *debuggerEvalKey = pDE; LOG((LF_CORDB, LL_INFO100000, "D:FES for pDE:%p evalType:%d on thread %p, id=0x%x\n", diff --git a/src/coreclr/debug/ee/debugger.h b/src/coreclr/debug/ee/debugger.h index f2461a16864c74..d50ee2bb56301c 100644 --- a/src/coreclr/debug/ee/debugger.h +++ b/src/coreclr/debug/ee/debugger.h @@ -455,9 +455,20 @@ class DebuggerModule * Hash to hold pending func evals by thread id * ------------------------------------------------------------------------ */ -struct DebuggerPendingFuncEval +// Array allocation for SHash elements must not take a heap lock held by a suspended thread. +class DebuggerHeapHashEntry +{ +public: +#ifndef DACCESS_COMPILE + static void *operator new[](size_t size); + static void *operator new[](size_t size, const std::nothrow_t &) noexcept; + static void operator delete[](void *memory); + static void operator delete[](void *memory, const std::nothrow_t &) noexcept; +#endif +}; + +struct DebuggerPendingFuncEval : DebuggerHeapHashEntry { - FREEHASHENTRY entry; PTR_Thread pThread; PTR_DebuggerEval pDE; }; @@ -2938,8 +2949,7 @@ void ApcActivationCallbackStubEnd(); }; -// CNewZeroData is the allocator used by the all the hash tables that the helper thread could possibly alter. It uses -// the interop safe allocator. +// Zero-initialized allocations on the debugger's interop-safe heap. class CNewZeroData { public: @@ -2985,193 +2995,93 @@ class CNewZeroData pHeap->Free(pPtr); } - static BYTE *Grow(BYTE *&pPtr, int iCurSize) - { - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - PRECONDITION(g_pDebugger != NULL); - } - CONTRACTL_END; - - void *p; - - DebuggerHeap* pHeap = g_pDebugger->GetInteropSafeHeap_NoThrow(); - _ASSERTE(pHeap != NULL); // should already exist - - _ASSERTE( iCurSize >= 0 ); - S_UINT32 iNewSize = S_UINT32( iCurSize ) + S_UINT32( GrowSize(iCurSize) ); - if( iNewSize.IsOverflow() ) - { - return NULL; - } - p = pHeap->Realloc(pPtr, iNewSize.Value(), iCurSize); - if (p == NULL) - { - return NULL; - } - - memset((BYTE*)p+iCurSize, 0, GrowSize(iCurSize)); - return (pPtr = (BYTE *)p); - } - - // A hashtable may recycle memory. We need to zero it out again. - static void Clean(BYTE * pData, int iSize) - { - LIMITED_METHOD_CONTRACT; - - memset(pData, 0, iSize); - } #endif // DACCESS_COMPILE - - static int RoundSize(int iSize) - { - LIMITED_METHOD_CONTRACT; - - return (iSize); - } - static int GrowSize(int iCurSize) - { - LIMITED_METHOD_CONTRACT; - int newSize = (3 * iCurSize) / 2; - return (newSize < 256) ? 256 : newSize; - } }; -class DebuggerPendingFuncEvalTable : private CHashTableAndData +inline COUNT_T DebuggerHashPointer(TADDR pointer) { - public: - virtual ~DebuggerPendingFuncEvalTable() = default; - - private: + LIMITED_METHOD_DAC_CONTRACT; - BOOL Cmp(SIZE_T k1, const HASHENTRY * pc2) - { - LIMITED_METHOD_DAC_CONTRACT; - -#if defined(DACCESS_COMPILE) - // This function hasn't been tested yet in the DAC build. Make sure the DACization is correct. - DacNotImpl(); -#endif // DACCESS_COMPILE - - Thread * pThread1 = reinterpret_cast(k1); - Thread * pThread2 = dac_cast(const_cast(pc2))->pThread; - - return (pThread1 != pThread2); - } - - ULONG HASH(Thread* pThread) - { - LIMITED_METHOD_CONTRACT; - return (ULONG)((SIZE_T)pThread); // only use low 32-bits if 64-bit - } - - - SIZE_T KEY(Thread * pThread) - { - LIMITED_METHOD_CONTRACT; - return (SIZE_T)pThread; - } + COUNT_T hash = static_cast(pointer); +#ifdef HOST_64BIT + hash = HashCOUNT_T(hash, static_cast(pointer >> 32)); +#endif + return hash; +} - public: +class EMPTY_BASES DebuggerPendingFuncEvalTraits : public DefaultSHashTraits +{ +public: + typedef PTR_Thread key_t; + static key_t GetKey(const element_t &element) { return element.pThread; } + static BOOL Equals(key_t left, key_t right) { return left == right; } + static count_t Hash(key_t key) { return DebuggerHashPointer(dac_cast(key)); } + static element_t Null() { return {}; } + static element_t Deleted() + { + element_t element = {}; + element.pThread = dac_cast(static_cast(-1)); + return element; + } + static bool IsNull(const element_t &element) { return element.pThread == nullptr; } + static bool IsDeleted(const element_t &element) { return element.pThread == dac_cast(static_cast(-1)); } +}; +class DebuggerPendingFuncEvalTable +{ + SHash m_table; +public: #ifndef DACCESS_COMPILE - DebuggerPendingFuncEvalTable() : CHashTableAndData(11) - { - WRAPPER_NO_CONTRACT; - - SUPPRESS_ALLOCATION_ASSERTS_IN_THIS_SCOPE; - NewInit(11, sizeof(DebuggerPendingFuncEval), 11); - } - - void AddPendingEval(Thread *pThread, DebuggerEval *pDE) - { - WRAPPER_NO_CONTRACT; - - _ASSERTE((pThread != NULL) && (pDE != NULL)); - - DebuggerPendingFuncEval *pfe = (DebuggerPendingFuncEval*)Add(HASH(pThread)); - pfe->pThread = pThread; - pfe->pDE = pDE; - } - - void RemovePendingEval(Thread* pThread) - { - WRAPPER_NO_CONTRACT; - - _ASSERTE(pThread != NULL); - - DebuggerPendingFuncEval *entry = (DebuggerPendingFuncEval*)Find(HASH(pThread), KEY(pThread)); - Delete(HASH(pThread), (HASHENTRY*)entry); - } - + HRESULT AddPendingEval(Thread *pThread, DebuggerEval *pDE); + void RemovePendingEval(Thread *pThread); #endif // #ifndef DACCESS_COMPILE - DebuggerPendingFuncEval *GetPendingEval(Thread* pThread) + const DebuggerPendingFuncEval *GetPendingEval(Thread* pThread) const { WRAPPER_NO_CONTRACT; - DebuggerPendingFuncEval *entry = (DebuggerPendingFuncEval*)Find(HASH(pThread), KEY(pThread)); - return entry; + return m_table.LookupPtr(dac_cast(pThread)); } }; -struct DebuggerModuleEntry +struct DebuggerModuleEntry : DebuggerHeapHashEntry { - FREEHASHENTRY entry; PTR_DebuggerModule module; }; typedef DPTR(struct DebuggerModuleEntry) PTR_DebuggerModuleEntry; -class DebuggerModuleTable : private CHashTableAndData +class EMPTY_BASES DebuggerModuleTraits : public DefaultSHashTraits { -#ifdef DACCESS_COMPILE - public: - virtual ~DebuggerModuleTable() = default; -#endif - - private: - - BOOL Cmp(SIZE_T k1, const HASHENTRY * pc2) - { - LIMITED_METHOD_DAC_CONTRACT; - -#if defined(DACCESS_COMPILE) - // This function hasn't been tested yet in the DAC build. Make sure the DACization is correct. - DacNotImpl(); -#endif // DACCESS_COMPILE - - Module * pModule1 = reinterpret_cast(k1); - Module * pModule2 = - dac_cast(const_cast(pc2))->module->GetRuntimeModule(); - - return (pModule1 != pModule2); - } - - ULONG HASH(Module* module) - { - LIMITED_METHOD_CONTRACT; - return (ULONG)((SIZE_T)module); // only use low 32-bits if 64-bit - } - - SIZE_T KEY(Module * pModule) - { - LIMITED_METHOD_CONTRACT; - return (SIZE_T)pModule; - } +public: + typedef PTR_Module key_t; + static key_t GetKey(const element_t &element) { return dac_cast(element.module->GetRuntimeModule()); } + static BOOL Equals(key_t left, key_t right) { return left == right; } + static count_t Hash(key_t key) { return DebuggerHashPointer(dac_cast(key)); } + static element_t Null() { return {}; } + static element_t Deleted() + { + element_t element = {}; + element.module = dac_cast(static_cast(-1)); + return element; + } + static bool IsNull(const element_t &element) { return element.module == nullptr; } + static bool IsDeleted(const element_t &element) { return element.module == dac_cast(static_cast(-1)); } +}; +class DebuggerModuleTable +{ + SHash m_table; #ifdef _DEBUG bool ThreadHoldsLock(); #endif public: + typedef SHash::Iterator Iterator; #ifndef DACCESS_COMPILE - DebuggerModuleTable(); - virtual ~DebuggerModuleTable(); + ~DebuggerModuleTable(); void AddModule(DebuggerModule *module); @@ -3183,9 +3093,8 @@ class DebuggerModuleTable : private CHashTableAndData DebuggerModule *GetModule(Module* module); - // We should never look for a NULL Module * - DebuggerModule *GetFirstModule(HASHFIND *info); - DebuggerModule *GetNextModule(HASHFIND *info); + Iterator Begin(); + Iterator End(); }; // struct DebuggerMethodInfoKey: Key for each of the method info hash table entries. @@ -3201,20 +3110,18 @@ struct DebuggerMethodInfoKey } ; // struct DebuggerMethodInfoEntry: Entry for the JIT info hash table. -// FREEHASHENTRY entry: Needed for use by the hash table // DebuggerMethodInfo * ji: The actual DebuggerMethodInfo to // hash. Note that DMI's will be hashed by MethodDesc. typedef DPTR(struct DebuggerMethodInfoEntry) PTR_DebuggerMethodInfoEntry; -struct DebuggerMethodInfoEntry +struct DebuggerMethodInfoEntry : DebuggerHeapHashEntry { - FREEHASHENTRY entry; DebuggerMethodInfoKey key; SIZE_T nVersion; SIZE_T nVersionLastRemapped; PTR_DebuggerMethodInfo mi; #ifdef DACCESS_COMPILE - void EnumMemoryRegions(CLRDataEnumMemoryFlags flags); + void EnumMemoryRegions(CLRDataEnumMemoryFlags flags) const; #endif }; @@ -3226,7 +3133,28 @@ struct DebuggerMethodInfoEntry // INVARIANT: There is only one DebuggerMethodInfo per method // in the table. Note that DMI's will be hashed by MethodDesc. // -class DebuggerMethodInfoTable : private CHashTableAndData +class EMPTY_BASES DebuggerMethodInfoTraits : public DefaultSHashTraits +{ +public: + typedef DebuggerMethodInfoKey key_t; + static key_t GetKey(const element_t &element) { return element.key; } + static BOOL Equals(key_t left, key_t right) + { + return left.pModule == right.pModule && left.token == right.token; + } + static count_t Hash(key_t key) { return HashCOUNT_T(key.token, DebuggerHashPointer(dac_cast(key.pModule))); } + static element_t Null() { return {}; } + static element_t Deleted() + { + element_t element = {}; + element.key.pModule = dac_cast(static_cast(-1)); + return element; + } + static bool IsNull(const element_t &element) { return element.key.pModule == nullptr; } + static bool IsDeleted(const element_t &element) { return element.key.pModule == dac_cast(static_cast(-1)); } +}; + +class DebuggerMethodInfoTable { VPTR_BASE_CONCRETE_VTABLE_CLASS(DebuggerMethodInfoTable); @@ -3234,32 +3162,7 @@ class DebuggerMethodInfoTable : private CHashTableAndData virtual ~DebuggerMethodInfoTable() = default; private: - BOOL Cmp(SIZE_T k1, const HASHENTRY * pc2) - { - LIMITED_METHOD_DAC_CONTRACT; - - // This is the inverse of the KEY() function. - DebuggerMethodInfoKey * pDjik = reinterpret_cast(k1); - - DebuggerMethodInfoEntry * pDjie = dac_cast(const_cast(pc2)); - - return (pDjik->pModule != pDjie->key.pModule) || - (pDjik->token != pDjie->key.token); - } - - ULONG HASH(DebuggerMethodInfoKey* pDjik) - { - LIMITED_METHOD_DAC_CONTRACT; - return HashPtr( pDjik->token, pDjik->pModule ); - } - - SIZE_T KEY(DebuggerMethodInfoKey * pDjik) - { - // This is casting a host pointer to a SIZE_T. So that key is restricted to the host address space. - // This key is just passed to Cmp(), which will cast it back to a DebuggerMethodInfoKey*. - LIMITED_METHOD_DAC_CONTRACT; - return (SIZE_T)pDjik; - } + SHash m_table; //#define _DEBUG_DMI_TABLE @@ -3278,10 +3181,11 @@ class DebuggerMethodInfoTable : private CHashTableAndData #endif // _DEBUG_DMI_TABLE public: + typedef SHash::Iterator Iterator; #ifndef DACCESS_COMPILE - DebuggerMethodInfoTable(); + DebuggerMethodInfoTable() = default; HRESULT AddMethodInfo(Module *pModule, mdMethodDef token, @@ -3298,13 +3202,13 @@ class DebuggerMethodInfoTable : private CHashTableAndData // {Module*,mdMethodDef} // (b) Perf: don't waste the memory! void ClearMethodsOfModule(Module *pModule); - void DeleteEntryDMI(DebuggerMethodInfoEntry *entry); + void DeleteEntryDMI(DebuggerMethodInfoEntry &entry); #endif // #ifndef DACCESS_COMPILE DebuggerMethodInfo *GetMethodInfo(Module *pModule, mdMethodDef token); - DebuggerMethodInfo *GetFirstMethodInfo(HASHFIND *info); - DebuggerMethodInfo *GetNextMethodInfo(HASHFIND *info); + Iterator Begin(); + Iterator End(); #ifdef DACCESS_COMPILE void EnumMemoryRegions(CLRDataEnumMemoryFlags flags); @@ -3387,6 +3291,7 @@ class DebuggerEval T_CONTEXT m_context; Thread *m_thread; + DebuggerEval *m_nextPendingEval; DebuggerIPCE_FuncEvalType m_evalType; mdMethodDef m_methodToken; mdTypeDef m_classToken; @@ -3458,21 +3363,21 @@ class DebuggerEval DeleteInteropSafe(m_externalMemoryOwner); } - // Clean up any temporary buffers used to send the argument type information. These were allocated - // in response to a GET_BUFFER message. - DebuggerIPCE_FuncEvalArgData *argData = GetArgData(); - for (unsigned int i = 0; i < m_argCount; i++) + if (m_argData) { - if (argData[i].fullArgType != (CORDB_ADDRESS)0) + // Clean up any temporary buffers used to send the argument type information. These were allocated + // in response to a GET_BUFFER message. + DebuggerIPCE_FuncEvalArgData *argData = GetArgData(); + for (unsigned int i = 0; i < m_argCount; i++) { - _ASSERTE(g_pDebugger != NULL); - g_pDebugger->ReleaseRemoteBuffer((BYTE*)CORDB_ADDRESS_TO_PTR(argData[i].fullArgType), true); + if (argData[i].fullArgType != (CORDB_ADDRESS)0) + { + _ASSERTE(g_pDebugger != NULL); + g_pDebugger->ReleaseRemoteBuffer((BYTE*)CORDB_ADDRESS_TO_PTR(argData[i].fullArgType), true); + } } - } - // Clean up the array of argument information. This was allocated as part of Func Eval setup. - if (m_argData) - { + // Clean up the array of argument information. This was allocated as part of Func Eval setup. DeleteInteropSafe(m_argData); } diff --git a/src/coreclr/debug/ee/debuggermodule.cpp b/src/coreclr/debug/ee/debuggermodule.cpp index f8f495aa85adf3..042a0627e448c7 100644 --- a/src/coreclr/debug/ee/debuggermodule.cpp +++ b/src/coreclr/debug/ee/debuggermodule.cpp @@ -30,14 +30,6 @@ void DebuggerModule::SetCanChangeJitFlags(bool fCanChangeJitFlags) #ifndef DACCESS_COMPILE -DebuggerModuleTable::DebuggerModuleTable() : CHashTableAndData(101) -{ - WRAPPER_NO_CONTRACT; - - SUPPRESS_ALLOCATION_ASSERTS_IN_THIS_SCOPE; - NewInit(101, sizeof(DebuggerModuleEntry), 101); -} - DebuggerModuleTable::~DebuggerModuleTable() { WRAPPER_NO_CONTRACT; @@ -66,24 +58,15 @@ void DebuggerModuleTable::Clear() _ASSERTE(ThreadHoldsLock()); - HASHFIND hf; - DebuggerModuleEntry *pDME; - - pDME = (DebuggerModuleEntry *) FindFirstEntry(&hf); - - while (pDME) + for (Iterator iterator = m_table.Begin(), end = m_table.End(); iterator != end; iterator++) { - DebuggerModule *pDM = pDME->module; - Module *pEEM = pDM->GetRuntimeModule(); - - TRACE_FREE(pDME->module); + DebuggerModule *pDM = iterator->module; + TRACE_FREE(pDM); + m_table.Remove(pDM->GetRuntimeModule()); DeleteInteropSafe(pDM); - Delete(HASH(pEEM), (HASHENTRY *) pDME); - - pDME = (DebuggerModuleEntry *) FindFirstEntry(&hf); } - CHashTableAndData::Clear(); + m_table.RemoveAll(); } void DebuggerModuleTable::AddModule(DebuggerModule *pModule) @@ -102,13 +85,9 @@ void DebuggerModuleTable::AddModule(DebuggerModule *pModule) LOG((LF_CORDB, LL_EVERYTHING, "DMT::AM: DebuggerMod:%p Module:%p\n", pModule, pModule->GetRuntimeModule())); - DebuggerModuleEntry * pEntry = (DebuggerModuleEntry *) Add(HASH(pModule->GetRuntimeModule())); - if (pEntry == NULL) - { - ThrowOutOfMemory(); - } - - pEntry->module = pModule; + DebuggerModuleEntry entry = {}; + entry.module = pModule; + m_table.Add(entry); } //----------------------------------------------------------------------------- @@ -129,25 +108,17 @@ void DebuggerModuleTable::RemoveModule(Module* pModule) _ASSERTE(ThreadHoldsLock()); _ASSERTE(pModule != NULL); - HASHFIND hf; - - for (DebuggerModuleEntry *pDME = (DebuggerModuleEntry *) FindFirstEntry(&hf); - pDME != NULL; - pDME = (DebuggerModuleEntry*) FindNextEntry(&hf)) + const DebuggerModuleEntry *entry = m_table.LookupPtr(pModule); + if (entry != nullptr) { - DebuggerModule *pDM = pDME->module; - Module* pRuntimeModule = pDM->GetRuntimeModule(); - - if (pRuntimeModule == pModule) - { - LOG((LF_CORDB, LL_INFO1000, "DMT::RM Removing DebuggerMod:%p - Module:%p Asm:%p\n", - pDM, pModule, pDM->GetAssembly())); - TRACE_FREE(pDM); - DeleteInteropSafe(pDM); - Delete(HASH(pRuntimeModule), (HASHENTRY *) pDME); - _ASSERTE(GetModule(pModule) == NULL); - return; - } + DebuggerModule *pDM = entry->module; + LOG((LF_CORDB, LL_INFO1000, "DMT::RM Removing DebuggerMod:%p - Module:%p Asm:%p\n", + pDM, pModule, pDM->GetAssembly())); + TRACE_FREE(pDM); + m_table.Remove(pModule); + DeleteInteropSafe(pDM); + _ASSERTE(GetModule(pModule) == NULL); + return; } LOG((LF_CORDB, LL_INFO1000, "DMT::RM No debugger module found for Module:%p\n", pModule)); @@ -168,15 +139,14 @@ DebuggerModule *DebuggerModuleTable::GetModule(Module* module) _ASSERTE(module != NULL); _ASSERTE(ThreadHoldsLock()); - DebuggerModuleEntry *entry - = (DebuggerModuleEntry *) Find(HASH(module), KEY(module)); + const DebuggerModuleEntry *entry = m_table.LookupPtr(dac_cast(module)); if (entry == NULL) return NULL; else return entry->module; } -DebuggerModule *DebuggerModuleTable::GetFirstModule(HASHFIND *info) +DebuggerModuleTable::Iterator DebuggerModuleTable::Begin() { CONTRACTL { @@ -187,14 +157,10 @@ DebuggerModule *DebuggerModuleTable::GetFirstModule(HASHFIND *info) _ASSERTE(ThreadHoldsLock()); - DebuggerModuleEntry *entry = (DebuggerModuleEntry *) FindFirstEntry(info); - if (entry == NULL) - return NULL; - else - return entry->module; + return m_table.Begin(); } -DebuggerModule *DebuggerModuleTable::GetNextModule(HASHFIND *info) +DebuggerModuleTable::Iterator DebuggerModuleTable::End() { CONTRACTL { @@ -205,9 +171,5 @@ DebuggerModule *DebuggerModuleTable::GetNextModule(HASHFIND *info) _ASSERTE(ThreadHoldsLock()); - DebuggerModuleEntry *entry = (DebuggerModuleEntry *) FindNextEntry(info); - if (entry == NULL) - return NULL; - else - return entry->module; + return m_table.End(); } diff --git a/src/coreclr/debug/ee/functioninfo.cpp b/src/coreclr/debug/ee/functioninfo.cpp index 6c0ac028cb6c91..383731117c1619 100644 --- a/src/coreclr/debug/ee/functioninfo.cpp +++ b/src/coreclr/debug/ee/functioninfo.cpp @@ -2095,25 +2095,6 @@ DebuggerJitInfo* DebuggerMethodInfo::GetLatestJitInfo_NoCreate() } -DebuggerMethodInfoTable::DebuggerMethodInfoTable() : CHashTableAndData(101) -{ - CONTRACTL - { - WRAPPER(THROWS); - GC_NOTRIGGER; - - } - CONTRACTL_END; - - SUPPRESS_ALLOCATION_ASSERTS_IN_THIS_SCOPE; - HRESULT hr = NewInit(101, sizeof(DebuggerMethodInfoEntry), 101); - - if (FAILED(hr)) - { - ThrowWin32(hr); - } -} - HRESULT DebuggerMethodInfoTable::AddMethodInfo(Module *pModule, mdMethodDef token, DebuggerMethodInfo *mi) @@ -2144,18 +2125,10 @@ HRESULT DebuggerMethodInfoTable::AddMethodInfo(Module *pModule, dmik.pModule = pModule; dmik.token = token; - DebuggerMethodInfoEntry *dmie = - (DebuggerMethodInfoEntry *) Add(HASH(&dmik)); - - if (dmie == NULL) - { - ThrowOutOfMemory(); - return E_OUTOFMEMORY; - } - - dmie->key.pModule = pModule; - dmie->key.token = token; - dmie->mi = mi; + DebuggerMethodInfoEntry entry = {}; + entry.key = dmik; + entry.mi = mi; + m_table.Add(entry); LOG((LF_CORDB, LL_INFO1000, "DMIT::AMI: mod:%p tok:0x%08x\n", pModule, token)); @@ -2186,22 +2159,18 @@ HRESULT DebuggerMethodInfoTable::OverwriteMethodInfo(Module *pModule, dmik.pModule = pModule; dmik.token = token; - DebuggerMethodInfoEntry *entry - = (DebuggerMethodInfoEntry *) Find(HASH(&dmik), KEY(&dmik)); - if (entry != NULL) + HRESULT hr = E_FAIL; + m_table.Update(dmik, [&](DebuggerMethodInfoEntry &entry) { - if ( (fOnlyIfNull && entry->mi == NULL) - || !fOnlyIfNull) - { - entry->mi = mi; - - LOG((LF_CORDB, LL_INFO1000, "DMIT::OMI: mod:%p tok:0x%08x remap nVer:0x%zx\n", - pModule, token, entry->nVersionLastRemapped)); - return S_OK; - } - } + if (fOnlyIfNull && entry.mi != nullptr) + return; - return E_FAIL; + entry.mi = mi; + hr = S_OK; + LOG((LF_CORDB, LL_INFO1000, "DMIT::OMI: mod:%p tok:0x%08x remap nVer:0x%zx\n", + pModule, token, entry.nVersionLastRemapped)); + }); + return hr; } // pModule is being destroyed - remove any entries that belong to it. Why? @@ -2218,23 +2187,24 @@ void DebuggerMethodInfoTable::ClearMethodsOfModule(Module *pModule) LOG((LF_CORDB, LL_INFO1000000, "CMOM:mod:0x%p (%s)\n", pModule ,pModule->GetDebugName())); - HASHFIND info; - - DebuggerMethodInfoEntry *entry - = (DebuggerMethodInfoEntry *) FindFirstEntry(&info); - while(entry != NULL) + for (Iterator iterator = m_table.Begin(), end = m_table.End(); iterator != end; iterator++) { + const DebuggerMethodInfoEntry *entry = &*iterator; Module *pMod = entry->key.pModule ; if (pMod == pModule) { - // This method actually got mitted, at least + // This method actually got emitted, at least // once - remove all version info. - while(entry->mi != NULL) + bool updated = m_table.Update(entry->key, [&](DebuggerMethodInfoEntry ¤t) { - DeleteEntryDMI(entry); - } + while (current.mi != NULL) + { + DeleteEntryDMI(current); + } + }); + _ASSERTE(updated); - Delete(HASH(&(entry->key)), (HASHENTRY*)entry); + m_table.Remove(entry->key); } else { @@ -2258,12 +2228,10 @@ void DebuggerMethodInfoTable::ClearMethodsOfModule(Module *pModule) dmi = dmi->m_prevMethodInfo; } } - - entry = (DebuggerMethodInfoEntry *) FindNextEntry(&info); } } -void DebuggerMethodInfoTable::DeleteEntryDMI(DebuggerMethodInfoEntry *entry) +void DebuggerMethodInfoTable::DeleteEntryDMI(DebuggerMethodInfoEntry &entry) { CONTRACTL { @@ -2275,12 +2243,12 @@ void DebuggerMethodInfoTable::DeleteEntryDMI(DebuggerMethodInfoEntry *entry) } CONTRACTL_END; - DebuggerMethodInfo *dmiPrev = entry->mi->m_prevMethodInfo; - TRACE_FREE(entry->mi); - DeleteInteropSafe(entry->mi); - entry->mi = dmiPrev; - if ( dmiPrev != NULL ) - dmiPrev->m_nextMethodInfo = NULL; + DebuggerMethodInfo *dmiPrev = entry.mi->m_prevMethodInfo; + TRACE_FREE(entry.mi); + DeleteInteropSafe(entry.mi); + entry.mi = dmiPrev; + if (dmiPrev != nullptr) + dmiPrev->m_nextMethodInfo = nullptr; } #endif // #ifndef DACCESS_COMPILE @@ -2321,7 +2289,7 @@ DebuggerMethodInfo *DebuggerMethodInfoTable::GetMethodInfo(Module *pModule, mdMe dmik.pModule = dac_cast(pModule); dmik.token = token; - DebuggerMethodInfoEntry *entry = dac_cast(Find(HASH(&dmik), KEY(&dmik))); + const DebuggerMethodInfoEntry *entry = m_table.LookupPtr(dmik); if (entry == NULL ) { @@ -2336,74 +2304,43 @@ DebuggerMethodInfo *DebuggerMethodInfoTable::GetMethodInfo(Module *pModule, mdMe } -DebuggerMethodInfo *DebuggerMethodInfoTable::GetFirstMethodInfo(HASHFIND *info) +DebuggerMethodInfoTable::Iterator DebuggerMethodInfoTable::Begin() { CONTRACTL { NOTHROW; GC_NOTRIGGER; - - PRECONDITION(CheckPointer(info)); } CONTRACTL_END; _ASSERTE(g_pDebugger->HasDebuggerDataLock()); - DebuggerMethodInfoEntry *entry = PTR_DebuggerMethodInfoEntry - (PTR_HOST_TO_TADDR(FindFirstEntry(info))); - if (entry == NULL) - return NULL; - else - { - return entry->mi; - } + return m_table.Begin(); } -DebuggerMethodInfo *DebuggerMethodInfoTable::GetNextMethodInfo(HASHFIND *info) +DebuggerMethodInfoTable::Iterator DebuggerMethodInfoTable::End() { CONTRACTL { NOTHROW; GC_NOTRIGGER; - - PRECONDITION(CheckPointer(info)); } CONTRACTL_END; _ASSERTE(g_pDebugger->HasDebuggerDataLock()); - DebuggerMethodInfoEntry *entry = PTR_DebuggerMethodInfoEntry - (PTR_HOST_TO_TADDR(FindNextEntry(info))); - - // We may have incremented the version number - // for methods that never got JITted, so we should - // pretend like they don't exist here. - while (entry != NULL && - entry->mi == NULL) - { - entry = PTR_DebuggerMethodInfoEntry - (PTR_HOST_TO_TADDR(FindNextEntry(info))); - } - - if (entry == NULL) - return NULL; - else - { - return entry->mi; - } + return m_table.End(); } #ifdef DACCESS_COMPILE void -DebuggerMethodInfoEntry::EnumMemoryRegions(CLRDataEnumMemoryFlags flags) +DebuggerMethodInfoEntry::EnumMemoryRegions(CLRDataEnumMemoryFlags flags) const { SUPPORTS_DAC; - // This structure is in an array in the hash - // so the 'this' is implicitly enumerated by the - // array enum in CHashTable. + // The containing SHash enumerates the entry storage. // For a MiniDumpNormal, what is needed for modules is already enumerated elsewhere. // Don't waste time doing it here an extra time. Also, this will add many MB extra into the dump. @@ -2412,10 +2349,11 @@ DebuggerMethodInfoEntry::EnumMemoryRegions(CLRDataEnumMemoryFlags flags) key.pModule->EnumMemoryRegions(flags, true); } - while (mi.IsValid()) + PTR_DebuggerMethodInfo methodInfo = mi; + while (methodInfo.IsValid()) { - mi->EnumMemoryRegions(flags); - mi = mi->m_prevMethodInfo; + methodInfo->EnumMemoryRegions(flags); + methodInfo = methodInfo->m_prevMethodInfo; } } @@ -2452,13 +2390,11 @@ void DebuggerMethodInfoTable::EnumMemoryRegions(CLRDataEnumMemoryFlags flags) WRAPPER_NO_CONTRACT; DAC_ENUM_VTHIS(); - CHashTableAndData::EnumMemoryRegions(flags); + m_table.EnumMemoryRegions(); - for (ULONG i = 0; i < m_iEntries; i++) + for (Iterator iterator = m_table.Begin(), end = m_table.End(); iterator != end; iterator++) { - DebuggerMethodInfoEntry* entry = - PTR_DebuggerMethodInfoEntry(PTR_HOST_TO_TADDR(EntryPtr(i))); - entry->EnumMemoryRegions(flags); + iterator->EnumMemoryRegions(flags); } } #endif // #ifdef DACCESS_COMPILE diff --git a/src/coreclr/debug/inc/dacdbistructures.h b/src/coreclr/debug/inc/dacdbistructures.h index 621fa84503e113..8b860e3256dc94 100644 --- a/src/coreclr/debug/inc/dacdbistructures.h +++ b/src/coreclr/debug/inc/dacdbistructures.h @@ -653,17 +653,17 @@ class MSLAYOUT FieldData // initialize various fields of an instance of FieldData from information in a FieldDesc void Initialize(BOOL fIsStatic, BOOL fIsPrimitive, mdFieldDef mdToken); #else - HRESULT GetFieldSignature(class CordbModule * pModule, /*OUT*/ SigParser * pSigParser); + HRESULT GetFieldSignature(class CordbModule * pModule, /*OUT*/ SigParser * pSigParser) const; #endif // clear various fields for a new instance of FieldData void ClearFields(); // Make sure it's okay to get or set an instance field offset. - BOOL OkToGetOrSetInstanceOffset(); + BOOL OkToGetOrSetInstanceOffset() const; // Make sure it's okay to get or set a static field address. - BOOL OkToGetOrSetStaticAddress(); + BOOL OkToGetOrSetStaticAddress() const; // If this is an instance field, store its offset void SetInstanceOffset( ULONG64 offset ); @@ -674,11 +674,11 @@ class MSLAYOUT FieldData // If this is an instance field, return its offset // Note that this offset is always a real offset (possibly larger than 22 bits), which isn't // necessarily the same as the overloaded FieldDesc.dwOffset field. - CORDB_ADDRESS GetInstanceOffset(); + CORDB_ADDRESS GetInstanceOffset() const; // If this is a "normal" static, get its absolute address // TLS and context-specific statics are "special". - CORDB_ADDRESS GetStaticAddress(); + CORDB_ADDRESS GetStaticAddress() const; // // Data members @@ -704,10 +704,10 @@ class MSLAYOUT FieldData // the object. CORDB_ADDRESS m_pFldStaticAddress; // The absolute target address of a static field - PCCOR_SIGNATURE m_fldSignatureCache; // This is passed across as null. It is a RS-only cache, and SHOULD - // NEVER BE ACCESSED DIRECTLY! - ULONG m_fldSignatureCacheSize; // This is passed across as 0. It is a RS-only cache, and SHOULD - // NEVER BE ACCESSED DIRECTLY! + mutable PCCOR_SIGNATURE m_fldSignatureCache; // This is passed across as null. It is a RS-only cache, and SHOULD + // NEVER BE ACCESSED DIRECTLY! + mutable ULONG m_fldSignatureCacheSize; // This is passed across as 0. It is a RS-only cache, and SHOULD + // NEVER BE ACCESSED DIRECTLY! public: VMPTR_FieldDesc m_vmFieldDesc; diff --git a/src/coreclr/debug/inc/dacdbistructures.inl b/src/coreclr/debug/inc/dacdbistructures.inl index 5ee266f329d166..d075c88eb2d7fb 100644 --- a/src/coreclr/debug/inc/dacdbistructures.inl +++ b/src/coreclr/debug/inc/dacdbistructures.inl @@ -643,7 +643,7 @@ void FieldData::ClearFields() } inline -BOOL FieldData::OkToGetOrSetInstanceOffset() +BOOL FieldData::OkToGetOrSetInstanceOffset() const { return (!m_fFldIsStatic && !m_fFldIsRVA && !m_fFldIsTLS && m_fFldStorageAvailable && (m_pFldStaticAddress == (CORDB_ADDRESS)NULL)); @@ -662,7 +662,7 @@ void FieldData::SetInstanceOffset(ULONG64 offset) } inline -BOOL FieldData::OkToGetOrSetStaticAddress() +BOOL FieldData::OkToGetOrSetStaticAddress() const { return (m_fFldIsStatic && !m_fFldIsTLS && m_fFldStorageAvailable && (m_fldInstanceOffset == 0)); @@ -681,7 +681,7 @@ void FieldData::SetStaticAddress(CORDB_ADDRESS addr) // Get the offset of a field inline -CORDB_ADDRESS FieldData::GetInstanceOffset() +CORDB_ADDRESS FieldData::GetInstanceOffset() const { _ASSERTE(!m_fFldIsStatic); _ASSERTE(!m_fFldIsRVA); @@ -693,7 +693,7 @@ CORDB_ADDRESS FieldData::GetInstanceOffset() // Get the static address for a field inline -CORDB_ADDRESS FieldData::GetStaticAddress() +CORDB_ADDRESS FieldData::GetStaticAddress() const { _ASSERTE(m_fFldIsStatic); _ASSERTE(!m_fFldIsTLS); diff --git a/src/coreclr/inc/shash.h b/src/coreclr/inc/shash.h index 8621acff0674c6..6cdb51e620d896 100644 --- a/src/coreclr/inc/shash.h +++ b/src/coreclr/inc/shash.h @@ -22,6 +22,7 @@ // (@todo: this can be remedied by delaying destruction of old tables during reallocation, e.g. during GC) // - Remove operations may be asynchronous with Lookup/Add, unless elements are also deallocated. (In which // case full synchronization is required) +// - Replacing elements must be synchronized with other accesses to those elements. // Common "gotchas": // - The Add method never replaces an element. The new element will be added even if an element with the same @@ -78,8 +79,8 @@ // static void OnDestructPerEntryCleanupAction(ELEMENT& e) Called on every element when in hashtable destructor. // s_DestructPerEntryCleanupAction must be set to true if implemented. // static void OnRemovePerEntryCleanupAction(ELEMENT& e) Called when an element is removed from the hashtable, including when -// the hashtable is destructed. s_RemovePerEntryCleanupAction must be set to true -// if implemented. +// the hashtable is destructed. Set s_RemovePerEntryCleanupAction to true for +// automatic cleanup. Update does not invoke this action. // // s_growth_factor_numerator // s_growth_factor_denominator Factor to grow allocation (numerator/denominator). @@ -194,13 +195,19 @@ class EMPTY_BASES SHash : public TRAITS element_t Lookup(key_t key) const; - // Pointer-based flavor of Lookup (allows efficient access to tables of structures) - + // Read-only pointer-based lookup. Use Update to modify an existing element. + // The returned pointer refers to table storage and is invalidated by reallocation, + // removal of the element, or clearing the table. const element_t* LookupPtr(key_t key) const; - // Pointer-based flavor to replace an existing element (allows efficient access to tables of structures) - - void ReplacePtr(const element_t *elementPtr, const element_t &newElement, bool invokeCleanupAction = true); + // Update the first matching element in place, returning false if no match exists. + // The callback must preserve the key and sentinel state, and must not structurally modify the table. + // It must not retain a mutable pointer or reference to the element. + // Callers must synchronize updates with other access to the affected element. + // No removal cleanup is performed; the callback manages ownership of any replaced value. + // Callback exceptions propagate; changes already made are not rolled back. + template + bool Update(key_t key, Functor &&functor); // Add an element to the hash table. This will never replace an element; multiple // elements may be stored with the same key. @@ -231,15 +238,19 @@ class EMPTY_BASES SHash : public TRAITS void RemovePtr(element_t * element); - // Remove all elements in the hashtable + // Remove all elements, optionally retaining the backing array for reuse. - void RemoveAll(); + void RemoveAll(bool retainCapacity = false); // Begin and End pointers for iteration over entire table. Iterator Begin() const; Iterator End() const; +#ifdef DACCESS_COMPILE + void EnumMemoryRegions(); +#endif + // Begin and End pointers for iteration over all elements with a given key. KeyIterator Begin(key_t key) const; @@ -333,7 +344,7 @@ class EMPTY_BASES SHash : public TRAITS // Utility function to find the first element with the given key in // the hash table. - const element_t* Lookup(PTR_element_t table, count_t tableSize, key_t key) const; + element_t* Lookup(PTR_element_t table, count_t tableSize, key_t key) const; // Utility function to remove the first element with the given key // in the hash table. diff --git a/src/coreclr/inc/shash.inl b/src/coreclr/inc/shash.inl index eb1129bb58bb4a..fe7d118d1c17c0 100644 --- a/src/coreclr/inc/shash.inl +++ b/src/coreclr/inc/shash.inl @@ -52,6 +52,16 @@ SHash::~SHash() delete [] m_table; } +#ifdef DACCESS_COMPILE +template +void SHash::EnumMemoryRegions() +{ + SUPPORTS_DAC; + if (m_table != nullptr) + DacEnumMemoryRegion(dac_cast(m_table), m_tableSize * sizeof(element_t)); +} +#endif + template typename SHash::count_t SHash::GetCount() const { @@ -99,29 +109,27 @@ const typename SHash::element_t * SHash::LookupPtr(key_t key) co } template -void SHash::ReplacePtr(const element_t *elementPtr, const element_t &newElement, bool invokeCleanupAction) +template +bool SHash::Update(key_t key, Functor &&functor) { - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - INSTANCE_CHECK; - PRECONDITION(m_table <= elementPtr); - PRECONDITION(elementPtr < m_table + m_tableSize); - PRECONDITION(!TRAITS::IsNull(*elementPtr)); - PRECONDITION(!TRAITS::IsDeleted(*elementPtr)); - PRECONDITION(!TRAITS::IsNull(newElement)); - PRECONDITION(!TRAITS::IsDeleted(newElement)); - PRECONDITION(TRAITS::Equals(TRAITS::GetKey(newElement), TRAITS::GetKey(*elementPtr))); - } - CONTRACTL_END; + WRAPPER_NO_CONTRACT; - if (TRAITS::s_RemovePerEntryCleanupAction && invokeCleanupAction) - { - TRAITS::OnRemovePerEntryCleanupAction(*elementPtr); - } + element_t *element = Lookup(m_table, m_tableSize, key); + if (element == nullptr) + return false; - *const_cast(elementPtr) = newElement; +#ifdef _DEBUG + PTR_element_t table = m_table; + count_t tableSize = m_tableSize; + count_t count = m_tableCount; +#endif // _DEBUG + + functor(*element); + + _ASSERTE(m_table == table && m_tableSize == tableSize && m_tableCount == count); + _ASSERTE(!TRAITS::IsNull(*element) && !TRAITS::IsDeleted(*element)); + _ASSERTE(TRAITS::Equals(TRAITS::GetKey(*element), key)); + return true; } template @@ -282,7 +290,7 @@ void SHash::RemovePtr(element_t * p) } template -void SHash::RemoveAll() +void SHash::RemoveAll(bool retainCapacity) { CONTRACTL { @@ -300,13 +308,20 @@ void SHash::RemoveAll() } } - delete [] m_table; - - m_table = NULL; - m_tableSize = 0; + if (retainCapacity) + { + for (count_t i = 0; i < m_tableSize; i++) + m_table[i] = TRAITS::Null(); + } + else + { + delete [] m_table; + m_table = NULL; + m_tableSize = 0; + m_tableMax = 0; + } m_tableCount = 0; m_tableOccupied = 0; - m_tableMax = 0; } template @@ -635,7 +650,7 @@ SHash::DeleteOldTable(element_t * oldTable) } template -const typename SHash::element_t * SHash::Lookup(PTR_element_t table, count_t tableSize, key_t key) const +typename SHash::element_t * SHash::Lookup(PTR_element_t table, count_t tableSize, key_t key) const { CONTRACTL { diff --git a/src/coreclr/inc/utilcode.h b/src/coreclr/inc/utilcode.h index 159aac8908e729..33c46d83653bc5 100644 --- a/src/coreclr/inc/utilcode.h +++ b/src/coreclr/inc/utilcode.h @@ -1336,576 +1336,6 @@ template class CBinarySearch } }; -//***************************************************************************** -// The information that the hash table implementation stores at the beginning -// of every record that can be but in the hash table. -//***************************************************************************** -typedef DPTR(struct HASHENTRY) PTR_HASHENTRY; -struct HASHENTRY -{ - ULONG iPrev; // Previous bucket in the chain. - ULONG iNext; // Next bucket in the chain. -}; - -typedef DPTR(struct FREEHASHENTRY) PTR_FREEHASHENTRY; -struct FREEHASHENTRY : HASHENTRY -{ - ULONG iFree; -}; - -//***************************************************************************** -// Used by the FindFirst/FindNextEntry functions. These api's allow you to -// do a sequential scan of all entries. -//***************************************************************************** -struct HASHFIND -{ - ULONG iBucket; // The next bucket to look in. - ULONG iNext; -}; - - -//***************************************************************************** -// IMPORTANT: This data structure is deprecated, please do not add any new uses. -// The hashtable implementation that should be used instead is code:SHash. -// If code:SHash does not work for you, talk to mailto:clrdeag. -//***************************************************************************** -// This is a class that implements a chain and bucket hash table. -// -// The data is actually supplied as an array of structures by the user of this class. -// This allows the buckets to use small indices to point to the chain, instead of pointers. -// -// Each entry in the array contains a HASHENTRY structure immediately -// followed by the key used to hash the structure. -// -// The HASHENTRY part of every structure is used to implement the chain of -// entries in a single bucket. -// -// This implementation does not support rehashing the buckets if the table grows -// to big. -// @TODO: Fix this by adding an abstract function Hash() which must be implemented -// by all clients. -// -//***************************************************************************** -class CHashTable -{ - friend class DebuggerRCThread; //RCthread actually needs access to - //fields of derrived class DebuggerPatchTable - -protected: - TADDR m_pcEntries; // Pointer to the array of structs. - ULONG m_iEntrySize; // Size of the structs. - - ULONG m_iBuckets; // # of chains we are hashing into. - PTR_ULONG m_piBuckets; // Ptr to the array of bucket chains. - - INDEBUG(unsigned m_maxSearch;) // For evaluating perf characteristics - - HASHENTRY *EntryPtr(ULONG iEntry) - { - LIMITED_METHOD_DAC_CONTRACT; - return (PTR_HASHENTRY(m_pcEntries + (iEntry * (size_t)m_iEntrySize))); - } - - ULONG ItemIndex(HASHENTRY *p) - { - SUPPORTS_DAC; - LIMITED_METHOD_CONTRACT; - return (ULONG)((dac_cast(p) - m_pcEntries) / m_iEntrySize); - } - - -public: - - CHashTable( - ULONG iBuckets) : // # of chains we are hashing into. - m_pcEntries((TADDR)NULL), - m_iBuckets(iBuckets) - { - LIMITED_METHOD_CONTRACT; - - m_piBuckets = NULL; - - INDEBUG(m_maxSearch = 0;) - } - - CHashTable() : // # of chains we are hashing into. - m_pcEntries((TADDR)NULL), - m_iBuckets(5) - { - LIMITED_METHOD_CONTRACT; - - m_piBuckets = NULL; - - INDEBUG(m_maxSearch = 0;) - } - -#ifndef DACCESS_COMPILE - - ~CHashTable() - { - LIMITED_METHOD_CONTRACT; - if (m_piBuckets != NULL) - { - delete [] m_piBuckets; - m_piBuckets = NULL; - } - } - -//***************************************************************************** -// This is the second part of construction where we do all of the work that -// can fail. We also take the array of structs here because the calling class -// presumably needs to allocate it in its NewInit. -//***************************************************************************** - HRESULT NewInit( // Return status. - BYTE *pcEntries, // Array of structs we are managing. - ULONG iEntrySize); // Size of the entries. - -//***************************************************************************** -// This can be called to change the pointer to the table that the hash table -// is managing. You might call this if (for example) you realloc the size -// of the table and its pointer is different. -//***************************************************************************** - void SetTable( - BYTE *pcEntries) // Array of structs we are managing. - { - LIMITED_METHOD_CONTRACT; - m_pcEntries = (TADDR)pcEntries; - } - -//***************************************************************************** -// Clear the hash table as if there were nothing in it. -//***************************************************************************** - void Clear() - { - LIMITED_METHOD_CONTRACT; - _ASSERTE(m_piBuckets != NULL); - memset(m_piBuckets, 0xff, m_iBuckets * sizeof(ULONG)); - } - -//***************************************************************************** -// Add the struct at the specified index in m_pcEntries to the hash chains. -//***************************************************************************** - BYTE *Add( // New entry. - ULONG iHash, // Hash value of entry to add. - ULONG iIndex); // Index of struct in m_pcEntries. - -//***************************************************************************** -// Delete the struct at the specified index in m_pcEntries from the hash chains. -//***************************************************************************** - void Delete( - ULONG iHash, // Hash value of entry to delete. - ULONG iIndex); // Index of struct in m_pcEntries. - - void Delete( - ULONG iHash, // Hash value of entry to delete. - HASHENTRY *psEntry); // The struct to delete. - -//***************************************************************************** -// The item at the specified index has been moved, update the previous and -// next item. -//***************************************************************************** - void Move( - ULONG iHash, // Hash value for the item. - ULONG iNew); // New location. - -#endif // #ifndef DACCESS_COMPILE - -//***************************************************************************** -// Return a boolean indicating whether or not this hash table has been inited. -//***************************************************************************** - int IsInited() - { - LIMITED_METHOD_CONTRACT; - return (m_piBuckets != NULL); - } - -//***************************************************************************** -// Search the hash table for an entry with the specified key value. -//***************************************************************************** - BYTE *Find( // Index of struct in m_pcEntries. - ULONG iHash, // Hash value of the item. - SIZE_T key); // The key to match. - -//***************************************************************************** -// Search the hash table for the next entry with the specified key value. -//***************************************************************************** - ULONG FindNext( // Index of struct in m_pcEntries. - SIZE_T key, // The key to match. - ULONG iIndex); // Index of previous match. - -//***************************************************************************** -// Returns the first entry in the first hash bucket and inits the search -// struct. Use the FindNextEntry function to continue walking the list. The -// return order is not guaranteed. -//***************************************************************************** - BYTE *FindFirstEntry( // First entry found, or 0. - HASHFIND *psSrch) // Search object. - { - WRAPPER_NO_CONTRACT; - if (m_piBuckets == nullptr) - return (0); - psSrch->iBucket = 1; - psSrch->iNext = m_piBuckets[0]; - return (FindNextEntry(psSrch)); - } - -//***************************************************************************** -// Returns the next entry in the list. -//***************************************************************************** - BYTE *FindNextEntry( // The next entry, or0 for end of list. - HASHFIND *psSrch); // Search object. - -#ifdef DACCESS_COMPILE - void EnumMemoryRegions(CLRDataEnumMemoryFlags flags, - ULONG numEntries); -#endif - -protected: - virtual BOOL Cmp(SIZE_T key1, const HASHENTRY * pc2) = 0; -}; - - -class CNewData -{ -public: - static BYTE *Alloc(int iSize, int iMaxSize) - { - WRAPPER_NO_CONTRACT; - return (new BYTE[iSize]); - } - static void Free(BYTE *pPtr, int iSize) - { - LIMITED_METHOD_CONTRACT; - delete [] pPtr; - } - static BYTE *Grow(BYTE *&pPtr, int iCurSize) - { - WRAPPER_NO_CONTRACT; - BYTE *p; - S_SIZE_T newSize = S_SIZE_T(iCurSize) + S_SIZE_T(GrowSize(iCurSize)); - //check for overflow - if(newSize.IsOverflow()) - p = NULL; - else - p = new (nothrow) BYTE[newSize.Value()]; - if (p == 0) return (0); - memcpy (p, pPtr, iCurSize); - delete [] pPtr; - pPtr = p; - return pPtr; - } - static void Clean(BYTE * pData, int iSize) - { - } - static int RoundSize(int iSize) - { - LIMITED_METHOD_CONTRACT; - return (iSize); - } - static int GrowSize(int iCurSize) - { - LIMITED_METHOD_CONTRACT; - int newSize = (3 * iCurSize) / 2; - return (newSize < 256) ? 256 : newSize; - } -}; - -class CNewDataNoThrow -{ -public: - static BYTE *Alloc(int iSize, int iMaxSize) - { - WRAPPER_NO_CONTRACT; - return (new (nothrow) BYTE[iSize]); - } - static void Free(BYTE *pPtr, int iSize) - { - LIMITED_METHOD_CONTRACT; - delete [] pPtr; - } - static BYTE *Grow(BYTE *&pPtr, int iCurSize) - { - WRAPPER_NO_CONTRACT; - BYTE *p; - S_SIZE_T newSize = S_SIZE_T(iCurSize) + S_SIZE_T(GrowSize(iCurSize)); - //check for overflow - if(newSize.IsOverflow()) - p = NULL; - else - p = new (nothrow) BYTE[newSize.Value()]; - if (p == 0) return (0); - memcpy (p, pPtr, iCurSize); - delete [] pPtr; - pPtr = p; - return pPtr; - } - static void Clean(BYTE * pData, int iSize) - { - } - static int RoundSize(int iSize) - { - LIMITED_METHOD_CONTRACT; - return (iSize); - } - static int GrowSize(int iCurSize) - { - LIMITED_METHOD_CONTRACT; - int newSize = (3 * iCurSize) / 2; - return (newSize < 256) ? 256 : newSize; - } -}; - - -//***************************************************************************** -// IMPORTANT: This data structure is deprecated, please do not add any new uses. -// The hashtable implementation that should be used instead is code:SHash. -// If code:SHash does not work for you, talk to mailto:clrdeag. -//***************************************************************************** -// CHashTable expects the data to be in a single array - this is provided by -// CHashTableAndData. -// The array is allocated using the MemMgr type. CNewData and -// CNewDataNoThrow can be used for this. -//***************************************************************************** -template -class CHashTableAndData : public CHashTable -{ -public: - DAC_ALIGNAS(CHashTable) - ULONG m_iFree; // Index into m_pcEntries[] of next available slot - ULONG m_iEntries; // size of m_pcEntries[] - -public: - - CHashTableAndData() : - CHashTable() - { - LIMITED_METHOD_CONTRACT; - } - - CHashTableAndData( - ULONG iBuckets) : // # of chains we are hashing into. - CHashTable(iBuckets) - { - LIMITED_METHOD_CONTRACT; - } - -#ifndef DACCESS_COMPILE - - ~CHashTableAndData() - { - WRAPPER_NO_CONTRACT; - if (m_pcEntries != (TADDR)NULL) - MemMgr::Free((BYTE*)m_pcEntries, MemMgr::RoundSize(m_iEntries * m_iEntrySize)); - } - -//***************************************************************************** -// This is the second part of construction where we do all of the work that -// can fail. We also take the array of structs here because the calling class -// presumably needs to allocate it in its NewInit. -//***************************************************************************** - HRESULT NewInit( // Return status. - ULONG iEntries, // # of entries. - ULONG iEntrySize, // Size of the entries. - int iMaxSize); // Max size of data. - -//***************************************************************************** -// Clear the hash table as if there were nothing in it. -//***************************************************************************** - void Clear() - { - WRAPPER_NO_CONTRACT; - m_iFree = 0; - InitFreeChain(0, m_iEntries); - CHashTable::Clear(); - } - -//***************************************************************************** -// Grabs a slot for the new entry to be added. -// The caller should fill in the non-HASHENTRY part of the returned slot -//***************************************************************************** - BYTE *Add( - ULONG iHash) // Hash value of entry to add. - { - WRAPPER_NO_CONTRACT; - FREEHASHENTRY *psEntry; - - // Make the table bigger if necessary. - if (m_iFree == UINT32_MAX && !Grow()) - return (NULL); - - // Add the first entry from the free list to the hash chain. - psEntry = (FREEHASHENTRY *) CHashTable::Add(iHash, m_iFree); - m_iFree = psEntry->iFree; - - // If we're recycling memory, give our memory-allocator a chance to re-init it. - - // Each entry is prefixed with a header - we don't want to trash that. - SIZE_T cbHeader = sizeof(FREEHASHENTRY); - MemMgr::Clean((BYTE*) psEntry + cbHeader, (int) (m_iEntrySize - cbHeader)); - - return ((BYTE *) psEntry); - } - -//***************************************************************************** -// Delete the struct at the specified index in m_pcEntries from the hash chains. -//***************************************************************************** - void Delete( - ULONG iHash, // Hash value of entry to delete. - ULONG iIndex) // Index of struct in m_pcEntries. - { - WRAPPER_NO_CONTRACT; - CHashTable::Delete(iHash, iIndex); - ((FREEHASHENTRY *) EntryPtr(iIndex))->iFree = m_iFree; - m_iFree = iIndex; - } - - void Delete( - ULONG iHash, // Hash value of entry to delete. - HASHENTRY *psEntry) // The struct to delete. - { - WRAPPER_NO_CONTRACT; - CHashTable::Delete(iHash, psEntry); - ((FREEHASHENTRY *) psEntry)->iFree = m_iFree; - m_iFree = ItemIndex(psEntry); - } - -#endif // #ifndef DACCESS_COMPILE - - // This is a sad legacy workaround. The debugger's patch table (implemented as this - // class) is shared across process. We publish the runtime offsets of - // some key fields. Since those fields are private, we have to provide - // accessors here. So if you're not using these functions, don't start. - // We can hopefully remove them. - // Note that we can't just make RCThread a friend of this class (we tried - // originally) because the inheritance chain has a private modifier, - // so DebuggerPatchTable::m_pcEntries is illegal. - static SIZE_T helper_GetOffsetOfEntries() - { - LIMITED_METHOD_CONTRACT; - return offsetof(CHashTableAndData, m_pcEntries); - } - - static SIZE_T helper_GetOffsetOfCount() - { - LIMITED_METHOD_CONTRACT; - return offsetof(CHashTableAndData, m_iEntries); - } - -#ifdef DACCESS_COMPILE - void EnumMemoryRegions(CLRDataEnumMemoryFlags flags) - { - SUPPORTS_DAC; - CHashTable::EnumMemoryRegions(flags, m_iEntries); - } -#endif - -private: - void InitFreeChain(ULONG iStart,ULONG iEnd); - int Grow(); -}; - -#ifndef DACCESS_COMPILE - -//***************************************************************************** -// This is the second part of construction where we do all of the work that -// can fail. We also take the array of structs here because the calling class -// presumably needs to allocate it in its NewInit. -//***************************************************************************** -template -HRESULT CHashTableAndData::NewInit(// Return status. - ULONG iEntries, // # of entries. - ULONG iEntrySize, // Size of the entries. - int iMaxSize) // Max size of data. -{ - WRAPPER_NO_CONTRACT; - BYTE *pcEntries; - HRESULT hr; - - - // note that this function can throw because it depends on the ::Alloc - - // Allocate the memory for the entries. - if ((pcEntries = MemMgr::Alloc(MemMgr::RoundSize(iEntries * iEntrySize), - MemMgr::RoundSize(iMaxSize))) == 0) - return (E_OUTOFMEMORY); - m_iEntries = iEntries; - - // Init the base table. - if (FAILED(hr = CHashTable::NewInit(pcEntries, iEntrySize))) - MemMgr::Free(pcEntries, MemMgr::RoundSize(iEntries * iEntrySize)); - else - { - // Init the free chain. - m_iFree = 0; - InitFreeChain(0, iEntries); - } - return (hr); -} - -//***************************************************************************** -// Initialize a range of records such that they are linked together to be put -// on the free chain. -//***************************************************************************** -template -void CHashTableAndData::InitFreeChain( - ULONG iStart, // Index to start initializing. - ULONG iEnd) // Index to stop initializing -{ - LIMITED_METHOD_CONTRACT; - BYTE* pcPtr; - _ASSERTE(iEnd > iStart); - - pcPtr = (BYTE*)m_pcEntries + iStart * (size_t)m_iEntrySize; - for (++iStart; iStart < iEnd; ++iStart) - { - ((FREEHASHENTRY *) pcPtr)->iFree = iStart; - pcPtr += m_iEntrySize; - } - ((FREEHASHENTRY *) pcPtr)->iFree = UINT32_MAX; -} - -//***************************************************************************** -// Attempt to increase the amount of space available for the record heap. -//***************************************************************************** -template -int CHashTableAndData::Grow() // 1 if successful, 0 if not. -{ - WRAPPER_NO_CONTRACT; - int iCurSize; // Current size in bytes. - int iEntries; // New # of entries. - - _ASSERTE(m_pcEntries != (TADDR)NULL); - _ASSERTE(m_iFree == UINT32_MAX); - - // Compute the current size and new # of entries. - S_UINT32 iTotEntrySize = S_UINT32(m_iEntries) * S_UINT32(m_iEntrySize); - if( iTotEntrySize.IsOverflow() ) - { - _ASSERTE( !"CHashTableAndData overflow!" ); - return (0); - } - iCurSize = MemMgr::RoundSize( iTotEntrySize.Value() ); - iEntries = (iCurSize + MemMgr::GrowSize(iCurSize)) / m_iEntrySize; - - if ( (iEntries < 0) || ((ULONG)iEntries <= m_iEntries) ) - { - _ASSERTE( !"CHashTableAndData overflow!" ); - return (0); - } - - // Try to expand the array. - if (MemMgr::Grow(*(BYTE**)&m_pcEntries, iCurSize) == 0) - return (0); - - // Init the newly allocated space. - InitFreeChain(m_iEntries, iEntries); - m_iFree = m_iEntries; - m_iEntries = iEntries; - return (1); -} - -#endif // #ifndef DACCESS_COMPILE - //***************************************************************************** //***************************************************************************** diff --git a/src/coreclr/nativeaot/Runtime/GcStressControl.cpp b/src/coreclr/nativeaot/Runtime/GcStressControl.cpp index ec1922eaab0c48..25e968f043113d 100644 --- a/src/coreclr/nativeaot/Runtime/GcStressControl.cpp +++ b/src/coreclr/nativeaot/Runtime/GcStressControl.cpp @@ -95,25 +95,21 @@ class GcStressControl CrstHolder lh(&s_lock); - const CallsiteCountEntry * pEntry = s_callsites.LookupPtr(CallsiteIP); - size_t hits; - - if (pEntry == NULL) - { - hits = 1; - CallsiteCountEntry e = {CallsiteIP, 1, 1, ht}; - s_callsites.AddOrReplace(e); - } - else + bool found = s_callsites.Update(CallsiteIP, [&](CallsiteCountEntry &entry) { - hits = ++(const_cast(pEntry)->countHit); + entry.countHit++; if (bForceGC) { - ++(const_cast(pEntry)->countForced); + entry.countForced++; } + }); + if (!found) + { + CallsiteCountEntry e = {CallsiteIP, 1, 1, ht}; + s_callsites.Add(e); } - return pEntry == NULL; + return !found; } static bool GcStressTriggerFirstHit(uintptr_t CallsiteIP, HijackType ht) @@ -177,4 +173,3 @@ bool ShouldHijackForGcStress(uintptr_t CallsiteIP, HijackType ht) #endif // FEATURE_GC_STRESS & !DACCESS_COMPILE - diff --git a/src/coreclr/nativeaot/Runtime/shash.h b/src/coreclr/nativeaot/Runtime/shash.h index 27b90157250a4f..1cb09082d67d25 100644 --- a/src/coreclr/nativeaot/Runtime/shash.h +++ b/src/coreclr/nativeaot/Runtime/shash.h @@ -151,10 +151,18 @@ class SHash : public TRAITS element_t Lookup(key_t key) const; - // Pointer-based flavor of Lookup (allows efficient access to tables of structures) - + // Read-only pointer-based lookup. Use Update to modify an existing element. + // The returned pointer refers to table storage and is invalidated by reallocation, + // removal of the element, or clearing the table. const element_t* LookupPtr(key_t key) const; + // Update the first matching element in place, returning false if no match exists. + // The callback must preserve the key and sentinel state, and must not structurally modify the table. + // It must not retain a mutable pointer or reference to the element. + // Callers must synchronize updates with other access to the affected element. + template + bool Update(key_t key, Functor &&functor); + // Add an element to the hash table. This will never replace an element; multiple // elements may be stored with the same key. // @@ -252,7 +260,7 @@ class SHash : public TRAITS // Utility function to find the first element with the given key in // the hash table. - static const element_t* Lookup(PTR_element_t table, count_t tableSize, key_t key); + static element_t* Lookup(PTR_element_t table, count_t tableSize, key_t key); // Utility function to remove the first element with the given key // in the hash table. diff --git a/src/coreclr/nativeaot/Runtime/shash.inl b/src/coreclr/nativeaot/Runtime/shash.inl index 8e2d5af49b593f..6bf17972163057 100644 --- a/src/coreclr/nativeaot/Runtime/shash.inl +++ b/src/coreclr/nativeaot/Runtime/shash.inl @@ -48,6 +48,28 @@ const typename SHash< TRAITS>::element_t* SHash::LookupPtr(key_t key) co return Lookup(m_table, m_tableSize, key); } +template +template +bool SHash::Update(key_t key, Functor &&functor) +{ + element_t *element = Lookup(m_table, m_tableSize, key); + if (element == nullptr) + return false; + +#ifdef _DEBUG + PTR_element_t table = m_table; + count_t tableSize = m_tableSize; + count_t count = m_tableCount; +#endif // _DEBUG + + functor(*element); + + ASSERT(m_table == table && m_tableSize == tableSize && m_tableCount == count); + ASSERT(!TRAITS::IsNull(*element) && !TRAITS::IsDeleted(*element)); + ASSERT(TRAITS::Equals(TRAITS::GetKey(*element), key)); + return true; +} + template bool SHash::Add(const element_t &element) { @@ -255,7 +277,7 @@ bool SHash::Reallocate(count_t newTableSize) } template -const typename SHash::element_t * SHash::Lookup(PTR_element_t table, count_t tableSize, key_t key) +typename SHash::element_t * SHash::Lookup(PTR_element_t table, count_t tableSize, key_t key) { if (tableSize == 0) return NULL; @@ -467,4 +489,3 @@ typename SHash::count_t SHash::NextPrime(typename SHash: // restore "Conditional expression is constant" warning to default value #pragma warning(default:4127) - diff --git a/src/coreclr/utilcode/collections.cpp b/src/coreclr/utilcode/collections.cpp index e6a165efbb6a3d..a6560a555a3b64 100644 --- a/src/coreclr/utilcode/collections.cpp +++ b/src/coreclr/utilcode/collections.cpp @@ -11,304 +11,6 @@ #include "utilcode.h" #include "ex.h" -// -// -// CHashTable -// -// - -#ifndef DACCESS_COMPILE - -//***************************************************************************** -// This is the second part of construction where we do all of the work that -// can fail. We also take the array of structs here because the calling class -// presumably needs to allocate it in its NewInit. -//***************************************************************************** -HRESULT CHashTable::NewInit( // Return status. - BYTE *pcEntries, // Array of structs we are managing. - ULONG iEntrySize) // Size of the entries. -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - } - CONTRACTL_END; - - _ASSERTE(iEntrySize >= sizeof(FREEHASHENTRY)); - - // Allocate the bucket chain array and init it. - if ((m_piBuckets = new (nothrow) ULONG [m_iBuckets]) == NULL) - return (OutOfMemory()); - memset(m_piBuckets, 0xff, m_iBuckets * sizeof(ULONG)); - - // Save the array of structs we are managing. - m_pcEntries = (TADDR)pcEntries; - m_iEntrySize = iEntrySize; - return (S_OK); -} - -//***************************************************************************** -// Add the struct at the specified index in m_pcEntries to the hash chains. -//***************************************************************************** -BYTE *CHashTable::Add( // New entry. - ULONG iHash, // Hash value of entry to add. - ULONG iIndex) // Index of struct in m_pcEntries. -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - } - CONTRACTL_END; - - HASHENTRY *psEntry; // The struct we are adding. - - // Get a pointer to the entry we are adding. - psEntry = EntryPtr(iIndex); - - // Compute the hash value for the entry. - iHash %= m_iBuckets; - - _ASSERTE(m_piBuckets[iHash] != iIndex && - (m_piBuckets[iHash] == UINT32_MAX || EntryPtr(m_piBuckets[iHash])->iPrev != iIndex)); - - // Setup this entry. - psEntry->iPrev = UINT32_MAX; - psEntry->iNext = m_piBuckets[iHash]; - - // Link it into the hash chain. - if (m_piBuckets[iHash] != UINT32_MAX) - EntryPtr(m_piBuckets[iHash])->iPrev = iIndex; - m_piBuckets[iHash] = iIndex; - return ((BYTE *) psEntry); -} - -//***************************************************************************** -// Delete the struct at the specified index in m_pcEntries from the hash chains. -//***************************************************************************** -void CHashTable::Delete( - ULONG iHash, // Hash value of entry to delete. - ULONG iIndex) // Index of struct in m_pcEntries. -{ - WRAPPER_NO_CONTRACT; - - HASHENTRY *psEntry; // Struct to delete. - - // Get a pointer to the entry we are deleting. - psEntry = EntryPtr(iIndex); - Delete(iHash, psEntry); -} - -//***************************************************************************** -// Delete the struct at the specified index in m_pcEntries from the hash chains. -//***************************************************************************** -void CHashTable::Delete( - ULONG iHash, // Hash value of entry to delete. - HASHENTRY *psEntry) // The struct to delete. -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - } - CONTRACTL_END; - - // Compute the hash value for the entry. - iHash %= m_iBuckets; - - _ASSERTE(psEntry->iPrev != psEntry->iNext || psEntry->iPrev == UINT32_MAX); - - // Fix the predecessor. - if (psEntry->iPrev == UINT32_MAX) - m_piBuckets[iHash] = psEntry->iNext; - else - EntryPtr(psEntry->iPrev)->iNext = psEntry->iNext; - - // Fix the successor. - if (psEntry->iNext != UINT32_MAX) - EntryPtr(psEntry->iNext)->iPrev = psEntry->iPrev; -} - -//***************************************************************************** -// The item at the specified index has been moved, update the previous and -// next item. -//***************************************************************************** -void CHashTable::Move( - ULONG iHash, // Hash value for the item. - ULONG iNew) // New location. -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - } - CONTRACTL_END; - - HASHENTRY *psEntry; // The struct we are deleting. - - psEntry = EntryPtr(iNew); - _ASSERTE(psEntry->iPrev != iNew && psEntry->iNext != iNew); - - if (psEntry->iPrev != UINT32_MAX) - EntryPtr(psEntry->iPrev)->iNext = iNew; - else - m_piBuckets[iHash % m_iBuckets] = iNew; - if (psEntry->iNext != UINT32_MAX) - EntryPtr(psEntry->iNext)->iPrev = iNew; -} - -#endif // !DACCESS_COMPILE - -//***************************************************************************** -// Search the hash table for an entry with the specified key value. -//***************************************************************************** -BYTE *CHashTable::Find( // Index of struct in m_pcEntries. - ULONG iHash, // Hash value of the item. - SIZE_T key) // The key to match. -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - SUPPORTS_DAC; - } - CONTRACTL_END; - - ULONG iNext; // Used to traverse the chains. - HASHENTRY *psEntry; // Used to traverse the chains. - - // Start at the top of the chain. - iNext = m_piBuckets[iHash % m_iBuckets]; - - // Search until we hit the end. - -#ifdef _DEBUG - unsigned count = 0; -#endif - - while (iNext != UINT32_MAX) - { - // Compare the keys. - psEntry = EntryPtr(iNext); - -#ifdef _DEBUG - count++; -#endif - if (!Cmp(key, psEntry)) - { -#ifdef _DEBUG - if (count > m_maxSearch) - m_maxSearch = count; -#endif - - return ((BYTE *) psEntry); - } - - // Advance to the next item in the chain. - iNext = psEntry->iNext; - } - - // We couldn't find it. - return (0); -} - -//***************************************************************************** -// Search the hash table for the next entry with the specified key value. -//***************************************************************************** -ULONG CHashTable::FindNext( // Index of struct in m_pcEntries. - SIZE_T key, // The key to match. - ULONG iIndex) // Index of previous match. -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - } - CONTRACTL_END; - - ULONG iNext; // Used to traverse the chains. - HASHENTRY *psEntry; // Used to traverse the chains. - - // Start at the next entry in the chain. - iNext = EntryPtr(iIndex)->iNext; - - // Search until we hit the end. - while (iNext != UINT32_MAX) - { - // Compare the keys. - psEntry = EntryPtr(iNext); - if (!Cmp(key, psEntry)) - return (iNext); - - // Advance to the next item in the chain. - iNext = psEntry->iNext; - } - - // We couldn't find it. - return (UINT32_MAX); -} - -//***************************************************************************** -// Returns the next entry in the list. -//***************************************************************************** -BYTE *CHashTable::FindNextEntry( // The next entry, or0 for end of list. - HASHFIND *psSrch) // Search object. -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - SUPPORTS_DAC; - } - CONTRACTL_END; - - HASHENTRY *psEntry; // Used to traverse the chains. - - for (;;) - { - // See if we already have one to use and if so, use it. - if (psSrch->iNext != UINT32_MAX) - { - psEntry = EntryPtr(psSrch->iNext); -#if DACCESS_COMPILE - // If there is a simple infinite loop in the linked list - // If more complex forms of infinite loops are present, this code may need to be adjusted to handle an arbitrary cycle. - if (psEntry->iNext == psSrch->iNext) - return NULL; -#endif - psSrch->iNext = psEntry->iNext; - return ((BYTE *) psEntry); - } - - // Advance to the next bucket. - if (psSrch->iBucket < m_iBuckets) - psSrch->iNext = m_piBuckets[psSrch->iBucket++]; - else - break; - } - - // There were no more entries to be found. - return (0); -} - -#ifdef DACCESS_COMPILE - -void -CHashTable::EnumMemoryRegions(CLRDataEnumMemoryFlags flags, - ULONG numEntries) -{ - SUPPORTS_DAC; - - // This class may be embedded so do not enum 'this'. - DacEnumMemoryRegion(m_pcEntries, - (ULONG)numEntries * m_iEntrySize); - DacEnumMemoryRegion(dac_cast(m_piBuckets), - (ULONG)m_iBuckets * sizeof(ULONG)); -} - -#endif // #ifdef DACCESS_COMPILE - // // // CClosedHashBase diff --git a/src/coreclr/vm/assemblybinder.cpp b/src/coreclr/vm/assemblybinder.cpp index 26ebe890fe3404..6591db69283de5 100644 --- a/src/coreclr/vm/assemblybinder.cpp +++ b/src/coreclr/vm/assemblybinder.cpp @@ -82,29 +82,27 @@ void AssemblyBinder::DeclareDependencyOnMvid(LPCUTF8 simpleName, GUID mvid, bool addAllLoadedModules = true; } - SimpleNameToExpectedMVIDAndRequiringAssembly* foundElem = (SimpleNameToExpectedMVIDAndRequiringAssembly*)m_assemblySimpleNameMvidCheckHash.LookupPtr(simpleName); - if (foundElem == NULL) - { - SimpleNameToExpectedMVIDAndRequiringAssembly newElem(simpleName, mvid, compositeComponent, imageName); - m_assemblySimpleNameMvidCheckHash.Add(newElem); - } - else - { - // Elem already exists. Determine if the existing elem is another one with the same mvid, in which case just record that a dependency is in play. - // If the existing elem has a different mvid, fail. - if (IsEqualGUID(mvid, foundElem->Mvid)) + if (!m_assemblySimpleNameMvidCheckHash.Update(simpleName, [&](SimpleNameToExpectedMVIDAndRequiringAssembly &entry) { - // Mvid matches exactly. - if (foundElem->AssemblyRequirementName == NULL) + // Elem already exists. Determine if the existing elem is another one with the same mvid, in which case just record that a dependency is in play. + // If the existing elem has a different mvid, fail. + if (IsEqualGUID(mvid, entry.Mvid)) { - foundElem->AssemblyRequirementName = imageName; - foundElem->CompositeComponent = compositeComponent; + // Mvid matches exactly. + if (entry.AssemblyRequirementName == NULL) + { + entry.AssemblyRequirementName = imageName; + entry.CompositeComponent = compositeComponent; + } } - } - else - { - MvidMismatchFatalError(foundElem->Mvid, mvid, simpleName, compositeComponent, imageName); - } + else + { + MvidMismatchFatalError(entry.Mvid, mvid, simpleName, compositeComponent, imageName); + } + })) + { + SimpleNameToExpectedMVIDAndRequiringAssembly newElem(simpleName, mvid, compositeComponent, imageName); + m_assemblySimpleNameMvidCheckHash.Add(newElem); } if (addAllLoadedModules) @@ -127,31 +125,29 @@ void AssemblyBinder::DeclareLoadedAssembly(Assembly* loadedAssembly) LPCUTF8 simpleName = loadedAssembly->GetSimpleName(); - SimpleNameToExpectedMVIDAndRequiringAssembly* foundElem = (SimpleNameToExpectedMVIDAndRequiringAssembly*)m_assemblySimpleNameMvidCheckHash.LookupPtr(simpleName); - if (foundElem == NULL) + if (!m_assemblySimpleNameMvidCheckHash.Update(simpleName, [&](SimpleNameToExpectedMVIDAndRequiringAssembly &entry) + { + // Elem already exists. Determine if the existing elem is another one with the same mvid, in which case do nothing. Everything is fine here. + // If the existing elem has a different mvid, but isn't a dependency on exact mvid elem, then set the mvid to all 0. + // If the existing elem has a different mvid, and is a dependency on exact mvid elem, then we've hit a fatal error. + if (IsEqualGUID(mvid, entry.Mvid)) + { + // Mvid matches exactly. + } + else if (entry.AssemblyRequirementName == NULL) + { + // Another loaded assembly, set the stored Mvid to all zeroes to indicate that it isn't a unique mvid + memset(&entry.Mvid, 0, sizeof(GUID)); + } + else + { + MvidMismatchFatalError(mvid, entry.Mvid, simpleName, entry.CompositeComponent, entry.AssemblyRequirementName); + } + })) { SimpleNameToExpectedMVIDAndRequiringAssembly newElem(simpleName, mvid, false, NULL); m_assemblySimpleNameMvidCheckHash.Add(newElem); } - else - { - // Elem already exists. Determine if the existing elem is another one with the same mvid, in which case do nothing. Everything is fine here. - // If the existing elem has a different mvid, but isn't a dependency on exact mvid elem, then set the mvid to all 0. - // If the existing elem has a different mvid, and is a dependency on exact mvid elem, then we've hit a fatal error. - if (IsEqualGUID(mvid, foundElem->Mvid)) - { - // Mvid matches exactly. - } - else if (foundElem->AssemblyRequirementName == NULL) - { - // Another loaded assembly, set the stored Mvid to all zeroes to indicate that it isn't a unique mvid - memset(&foundElem->Mvid, 0, sizeof(GUID)); - } - else - { - MvidMismatchFatalError(mvid, foundElem->Mvid, simpleName, foundElem->CompositeComponent, foundElem->AssemblyRequirementName); - } - } } #endif // FEATURE_READYTORUN diff --git a/src/coreclr/vm/callcounting.cpp b/src/coreclr/vm/callcounting.cpp index ada6cf272930ad..09947dc99b50a1 100644 --- a/src/coreclr/vm/callcounting.cpp +++ b/src/coreclr/vm/callcounting.cpp @@ -1018,11 +1018,11 @@ void CallCountingManager::DeleteAllCallCountingStubs() // Currently, tier 0 is the last code version that is counted, and the method is typically not counted anymore. // Remove the forwarder stub if one exists, a new one will be created if necessary, for example, if a profiler adds // an IL code version for the method. - Precode *const *forwarderStubPtr = - methodDescForwarderStubHash.LookupPtr(callCountingInfo->GetCodeVersion().GetMethodDesc()); - if (forwarderStubPtr != nullptr) + MethodDesc *methodDesc = callCountingInfo->GetCodeVersion().GetMethodDesc(); + MethodDescForwarderStubHash::KeyIterator forwarderStub = methodDescForwarderStubHash.Begin(methodDesc); + if (forwarderStub != methodDescForwarderStubHash.End(methodDesc)) { - methodDescForwarderStubHash.RemovePtr(const_cast(forwarderStubPtr)); + methodDescForwarderStubHash.Remove(forwarderStub); } callCountingInfoByCodeVersionHash.Remove(it); diff --git a/src/coreclr/vm/crossloaderallocatorhash.inl b/src/coreclr/vm/crossloaderallocatorhash.inl index 35344cfba9ce72..0b468a1c509755 100644 --- a/src/coreclr/vm/crossloaderallocatorhash.inl +++ b/src/coreclr/vm/crossloaderallocatorhash.inl @@ -428,7 +428,12 @@ void CrossLoaderAllocatorHash::Add(TKey key, TValue value, LoaderAllocat else { hashKeyEntry = keyValueStore; - keyToTrackersHash.ReplacePtr(hashKeyEntryPtr, hashKeyEntry); + bool updated = keyToTrackersHash.Update(key, [&](KeyValueStoreOrLAHashKeyToTrackers *&entry) + { + KeyToValuesHashTraits::OnRemovePerEntryCleanupAction(entry); + entry = hashKeyEntry; + }); + _ASSERTE(updated); } keyValueStoreHolder.SuppressRelease(); @@ -447,7 +452,11 @@ void CrossLoaderAllocatorHash::Add(TKey key, TValue value, LoaderAllocat // LAHashKeyToTrackers, so replace the element without cleanup. hashKeyToTrackers = new LAHashKeyToTrackers(keyValueStore); hashKeyEntry = hashKeyToTrackers; - keyToTrackersHash.ReplacePtr(hashKeyEntryPtr, hashKeyEntry, false /* invokeCleanupAction */); + bool updated = keyToTrackersHash.Update(key, [&](KeyValueStoreOrLAHashKeyToTrackers *&entry) + { + entry = hashKeyEntry; + }); + _ASSERTE(updated); } // Must add it to the cross LA structure @@ -462,7 +471,12 @@ void CrossLoaderAllocatorHash::Add(TKey key, TValue value, LoaderAllocat if (KeyToValuesHashTraits::AddToValuesInHeapMemory(&keyValueStore, keyValueStoreHolder, key, value)) { - keyToValuePerLAHash->ReplacePtr(hashKeyEntryInPerLAHashPtr, keyValueStore); + bool updated = keyToValuePerLAHash->Update(key, [&](KeyValueStoreOrLAHashKeyToTrackers *&entry) + { + KeyToValuesHashTraits::OnRemovePerEntryCleanupAction(entry); + entry = keyValueStore; + }); + _ASSERTE(updated); keyValueStoreHolder.SuppressRelease(); } } @@ -656,14 +670,14 @@ void CrossLoaderAllocatorHash::RemoveAll(TKey key) _ASSERTE(TRAITS::GetLoaderAllocator(key) == m_pLoaderAllocator); KeyToValuesHash &keyToTrackersHash = m_keyToDependentTrackersHash; - KeyValueStoreOrLAHashKeyToTrackers *const *hashKeyEntryPtr = keyToTrackersHash.LookupPtr(key); - if (hashKeyEntryPtr == NULL) + typename KeyToValuesHash::KeyIterator hashKeyEntryIterator = keyToTrackersHash.Begin(key); + if (hashKeyEntryIterator == keyToTrackersHash.End(key)) { return; } // We have an entry in the hashtable for the key/dependenthandle. - KeyValueStoreOrLAHashKeyToTrackers *hashKeyEntry = *hashKeyEntryPtr; + KeyValueStoreOrLAHashKeyToTrackers *hashKeyEntry = *hashKeyEntryIterator; LAHashKeyToTrackers *hashKeyToTrackers = NULL; KeyValueStore *keyValueStore; @@ -705,7 +719,7 @@ void CrossLoaderAllocatorHash::RemoveAll(TKey key) } // Remove entry from key to tracker hash - keyToTrackersHash.RemovePtr(const_cast(hashKeyEntryPtr)); + keyToTrackersHash.Remove(hashKeyEntryIterator); } #endif // !DACCESS_COMPILE diff --git a/src/coreclr/vm/eetoprofinterfaceimpl.cpp b/src/coreclr/vm/eetoprofinterfaceimpl.cpp index df0a0e6723ffa2..85680528c9e741 100644 --- a/src/coreclr/vm/eetoprofinterfaceimpl.cpp +++ b/src/coreclr/vm/eetoprofinterfaceimpl.cpp @@ -377,46 +377,6 @@ static HRESULT CoCreateProfiler( } -//--------------------------------------------------------------------------------------- -// -// Implementation of CHashTableImpl functions. This class a simple implementation of -// CHashTable to provide a very simple implementation of the Cmp pure virtual function -// - -EEToProfInterfaceImpl::CHashTableImpl::CHashTableImpl(ULONG iBuckets) - : CHashTable(iBuckets) -{ - WRAPPER_NO_CONTRACT; -} - -EEToProfInterfaceImpl::CHashTableImpl::~CHashTableImpl() -{ - WRAPPER_NO_CONTRACT; -} - -//--------------------------------------------------------------------------------------- -// -// Comparison function for hash table of ClassIDs -// -// Arguments: -// pc1 - hash key to compare -// pc2 - hash value to compare -// -// Return Value: -// TRUE if the key & value refer to the same ClassID; otherwise FALSE -// - -BOOL EEToProfInterfaceImpl::CHashTableImpl::Cmp(SIZE_T k1, const HASHENTRY * pc2) -{ - LIMITED_METHOD_CONTRACT; - - ClassID key = (ClassID) k1; - ClassID val = ((CLASSHASHENTRY *)pc2)->m_clsId; - - return (key != val); -} - - //--------------------------------------------------------------------------------------- // // EEToProfInterfaceImpl ctor just sets initial values @@ -985,11 +945,6 @@ EEToProfInterfaceImpl::~EEToProfInterfaceImpl() _ASSERTE((UINT_PTR)m_pSavedAllocDataBlock != 0xFFFFFFFF); #endif - _ASSERTE(m_pSavedAllocDataBlock->pHashTable != NULL); - // Get rid of the hash table - if (m_pSavedAllocDataBlock->pHashTable) - delete m_pSavedAllocDataBlock->pHashTable; - // Get rid of the two arrays used to hold class<->numinstance info if (m_pSavedAllocDataBlock->cLength != 0) { @@ -1000,12 +955,6 @@ EEToProfInterfaceImpl::~EEToProfInterfaceImpl() delete [] m_pSavedAllocDataBlock->arrcObjects; } - // Get rid of the hash array used by the hash table - if (m_pSavedAllocDataBlock->arrHash) - { - delete [] m_pSavedAllocDataBlock->arrHash; - } - delete m_pSavedAllocDataBlock; m_pSavedAllocDataBlock = NULL; } @@ -1408,11 +1357,6 @@ HRESULT EEToProfInterfaceImpl::EndMovedReferences(void * pHeapId) } -#define HASH_ARRAY_SIZE_INITIAL 1024 -#define HASH_ARRAY_SIZE_INC 256 -#define HASH_NUM_BUCKETS 32 -#define HASH(x) ( (ULONG) ((SIZE_T)x) ) // A simple hash function - //--------------------------------------------------------------------------------------- // // Callback used by the GC when walking the heap (via AllocByClassHelper in @@ -1473,96 +1417,25 @@ HRESULT EEToProfInterfaceImpl::AllocByClass(ObjectID objId, ClassID clsId, void if (pData == NULL) return (E_OUTOFMEMORY); - // Create a new hash table - pData->pHashTable = new (nothrow) CHashTableImpl(HASH_NUM_BUCKETS); - if (!pData->pHashTable) - { - delete pData; - return (E_OUTOFMEMORY); - } - - // Get the memory for the array that the hash table is going to use - pData->arrHash = new (nothrow) CLASSHASHENTRY[HASH_ARRAY_SIZE_INITIAL]; - if (pData->arrHash == NULL) - { - delete pData->pHashTable; - delete pData; - return (E_OUTOFMEMORY); - } - - // Save the number of elements in the array - pData->cHash = HASH_ARRAY_SIZE_INITIAL; - - // Now initialize the hash table - HRESULT hr = pData->pHashTable->NewInit((BYTE *)pData->arrHash, sizeof(CLASSHASHENTRY)); - if (hr == E_OUTOFMEMORY) - { - delete [] pData->arrHash; - delete pData->pHashTable; - delete pData; - return (E_OUTOFMEMORY); - } - _ASSERTE(pData->pHashTable->IsInited()); - - // Null some entries - pData->arrClsId = NULL; - pData->arrcObjects = NULL; - pData->cLength = 0; - // Hold on to the structure m_pSavedAllocDataBlock = pData; } - // Got some memory and hash table to store entries, yay! - *((size_t *)pHeapId) = (size_t)pData; - // Initialize the data - pData->iHash = 0; - pData->pHashTable->Clear(); + pData->hashTable.RemoveAll(true); } - _ASSERTE(pData->iHash <= pData->cHash); _ASSERTE(dwProcId == GetCurrentProcessId()); - // Lookup to see if this class already has an entry - CLASSHASHENTRY * pEntry = - reinterpret_cast(pData->pHashTable->Find(HASH(clsId), (SIZE_T)clsId)); - - // If this class has already been encountered, just increment the counter. - if (pEntry) - pEntry->m_count++; - - // Otherwise, need to add this one as a new entry in the hash table - else + if (!pData->hashTable.Update(clsId, [](AllocByClassEntry &entry) { entry.m_count++; })) { - // If we're full, we need to realloc - if (pData->iHash == pData->cHash) - { - // Try to realloc the memory - CLASSHASHENTRY *tmp = new (nothrow) CLASSHASHENTRY[pData->cHash + HASH_ARRAY_SIZE_INC]; - if (!tmp) - { - return (E_OUTOFMEMORY); - } - - _ASSERTE(pData->arrHash); - memcpy (tmp, pData->arrHash, pData->cHash*sizeof(CLASSHASHENTRY)); - delete [] pData->arrHash; - pData->arrHash = tmp; - // Tell the hash table that the memory location of the array has changed - pData->pHashTable->SetTable((BYTE *)pData->arrHash); - - // Save the new size of the array - pData->cHash += HASH_ARRAY_SIZE_INC; - } - - // Now add the new entry - CLASSHASHENTRY *pNewEntry = (CLASSHASHENTRY *) pData->pHashTable->Add(HASH(clsId), pData->iHash++); - - pNewEntry->m_clsId = clsId; - pNewEntry->m_count = 1; + AllocByClassEntry newEntry = { clsId, 1 }; + if (!pData->hashTable.AddNoThrow(newEntry)) + return E_OUTOFMEMORY; } + *((size_t *)pHeapId) = (size_t)pData; + // Indicate success return (S_OK); } @@ -5370,10 +5243,11 @@ HRESULT EEToProfInterfaceImpl::NotifyAllocByClass(AllocByClassData *pData) "**PROF: ObjectsAllocatedByClass.\n")); _ASSERTE(pData != NULL); - _ASSERTE(pData->iHash > 0); + ULONG classCount = pData->hashTable.GetCount(); + _ASSERTE(classCount > 0); // If the arrays are not long enough, get rid of them. - if (pData->cLength != 0 && pData->iHash > pData->cLength) + if (pData->cLength != 0 && classCount > pData->cLength) { _ASSERTE(pData->arrClsId != NULL && pData->arrcObjects != NULL); delete [] pData->arrClsId; @@ -5384,13 +5258,13 @@ HRESULT EEToProfInterfaceImpl::NotifyAllocByClass(AllocByClassData *pData) // If there are no arrays, must allocate them. if (pData->cLength == 0) { - pData->arrClsId = new (nothrow) ClassID[pData->iHash]; + pData->arrClsId = new (nothrow) ClassID[classCount]; if (pData->arrClsId == NULL) { return E_OUTOFMEMORY; } - pData->arrcObjects = new (nothrow) ULONG[pData->iHash]; + pData->arrcObjects = new (nothrow) ULONG[classCount]; if (pData->arrcObjects == NULL) { delete [] pData->arrClsId; @@ -5400,34 +5274,31 @@ HRESULT EEToProfInterfaceImpl::NotifyAllocByClass(AllocByClassData *pData) } // Indicate that the memory was successfully allocated - pData->cLength = pData->iHash; + pData->cLength = classCount; } // Now copy all the data - HASHFIND hFind; - CLASSHASHENTRY * pCur = (CLASSHASHENTRY *) pData->pHashTable->FindFirstEntry(&hFind); size_t iCur = 0; // current index for arrays - while (pCur != NULL) + for (AllocByClassTable::Iterator entry = pData->hashTable.Begin(); entry != pData->hashTable.End(); entry++) { - _ASSERTE(iCur < pData->iHash); + _ASSERTE(iCur < classCount); - pData->arrClsId[iCur] = pCur->m_clsId; - pData->arrcObjects[iCur] = (DWORD) pCur->m_count; + pData->arrClsId[iCur] = entry->m_clsId; + pData->arrcObjects[iCur] = (DWORD) entry->m_count; // Move to the next entry iCur++; - pCur = (CLASSHASHENTRY *) pData->pHashTable->FindNextEntry(&hFind); } - _ASSERTE(iCur == pData->iHash); + _ASSERTE(iCur == classCount); // Now communicate the results to the profiler { // All callbacks are really NOTHROW, but that's enforced partially by the profiler, // whose try/catch blocks aren't visible to the contract system PERMANENT_CONTRACT_VIOLATION(ThrowsViolation, ReasonProfilerCallout); - return m_pCallback2->ObjectsAllocatedByClass((ULONG)pData->iHash, pData->arrClsId, pData->arrcObjects); + return m_pCallback2->ObjectsAllocatedByClass(classCount, pData->arrClsId, pData->arrcObjects); } } diff --git a/src/coreclr/vm/eetoprofinterfaceimpl.h b/src/coreclr/vm/eetoprofinterfaceimpl.h index 89a76cfe853d83..b97ea59b7df0c7 100644 --- a/src/coreclr/vm/eetoprofinterfaceimpl.h +++ b/src/coreclr/vm/eetoprofinterfaceimpl.h @@ -491,35 +491,40 @@ class EEToProfInterfaceImpl // // This is for a hashing of ClassID values - struct CLASSHASHENTRY : HASHENTRY + struct AllocByClassEntry { ClassID m_clsId; // The class ID (also the key) size_t m_count; // How many of this class have been counted }; - // This is a simple implementation of CHashTable to provide a very simple - // implementation of the Cmp pure virtual function - class CHashTableImpl : public CHashTable + class AllocByClassTraits : public NoRemoveSHashTraits>> { public: - CHashTableImpl(ULONG iBuckets); - virtual ~CHashTableImpl(); - - protected: - virtual BOOL Cmp(SIZE_T k1, const HASHENTRY * pc2); + typedef ClassID key_t; + static key_t GetKey(const element_t &entry) { return entry.m_clsId; } + static BOOL Equals(key_t left, key_t right) { return left == right; } + static count_t Hash(key_t key) + { + count_t hash = static_cast(key); +#ifdef HOST_64BIT + hash = HashCOUNT_T(hash, static_cast(key >> 32)); +#endif + return hash; + } + static element_t Null() { return {}; } + static bool IsNull(const element_t &entry) { return entry.m_clsId == 0; } }; + typedef SHash AllocByClassTable; + // This contains the data for storing allocation information // in terms of numbers of objects sorted by class. struct AllocByClassData { - CHashTableImpl * pHashTable; // The hash table - CLASSHASHENTRY * arrHash; // Array that the hashtable uses for linking - ULONG cHash; // The total number of elements in arrHash - ULONG iHash; // Next empty entry in the hash array - ClassID * arrClsId; // Array of ClassIDs for the call to ObjectsAllocatedByClass - ULONG * arrcObjects; // Array of counts for the call to ObjectsAllocatedByClass - size_t cLength; // Length of the above two parallel arrays + AllocByClassTable hashTable; + ClassID * arrClsId = nullptr; // Array of ClassIDs for ObjectsAllocatedByClass + ULONG * arrcObjects = nullptr; // Array of counts for ObjectsAllocatedByClass + size_t cLength = 0; // Length of the above two parallel arrays }; static const UINT kcReferencesMax = 512; diff --git a/src/coreclr/vm/inlinetracking.cpp b/src/coreclr/vm/inlinetracking.cpp index 4b8e8a9467798c..4fd0e8b9de1e94 100644 --- a/src/coreclr/vm/inlinetracking.cpp +++ b/src/coreclr/vm/inlinetracking.cpp @@ -144,13 +144,7 @@ void InlineTrackingMap::AddInlining(MethodDesc *inliner, MethodDesc *inlinee) } CrstHolder lock(&m_mapCrst); - InlineTrackingEntry *existingEntry = const_cast(LookupPtr(inlineeMnM)); - if (existingEntry) - { - // We saw this inlinee before, just add one more inliner - existingEntry->Add(inliner); - } - else + if (!Update(inlineeMnM, [&](InlineTrackingEntry &entry) { entry.Add(inliner); })) { // We haven't seen this inlinee before, create a new record in the hashtable // and add a first inliner to it. diff --git a/src/tests/baseservices/exceptions/exceptionstacktrace/exceptionstacktrace.cs b/src/tests/baseservices/exceptions/exceptionstacktrace/exceptionstacktrace.cs index e6d5789a4fc47f..53473a567941a1 100644 --- a/src/tests/baseservices/exceptions/exceptionstacktrace/exceptionstacktrace.cs +++ b/src/tests/baseservices/exceptions/exceptionstacktrace/exceptionstacktrace.cs @@ -17,6 +17,54 @@ public static class Program static Exception theException = new Exception("My exception"); static int recursion; + [ConditionalTheory(typeof(Utilities), nameof(Utilities.IsReflectionEmitSupported))] + [InlineData(17)] + [InlineData(257)] + public static void StackTracesAfterCollectibleMethodTableGrowth(int methodCount) + { + for (int generation = 0; generation < 3; generation++) + { + WeakReference type = CreateAndTraceCollectibleMethods(methodCount); + for (int attempt = 0; type.IsAlive && attempt < 10; attempt++) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + } + + Assert.False(type.IsAlive); + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static WeakReference CreateAndTraceCollectibleMethods(int methodCount) + { + AssemblyBuilder assembly = AssemblyBuilder.DefineDynamicAssembly( + new AssemblyName("CollectibleStackTraces"), AssemblyBuilderAccess.RunAndCollect); + TypeBuilder builder = assembly.DefineDynamicModule("Methods").DefineType("ThrowingMethods"); + ConstructorInfo constructor = typeof(InvalidOperationException).GetConstructor(Type.EmptyTypes); + for (int index = 0; index < methodCount; index++) + { + MethodBuilder method = builder.DefineMethod( + "Throw" + index, MethodAttributes.Public | MethodAttributes.Static, typeof(void), Type.EmptyTypes); + method.SetImplementationFlags(MethodImplAttributes.NoInlining); + ILGenerator il = method.GetILGenerator(); + il.Emit(OpCodes.Newobj, constructor); + il.Emit(OpCodes.Throw); + } + + Type type = builder.CreateType(); + for (int index = 0; index < methodCount; index++) + { + string name = "Throw" + index; + Action action = type.GetMethod(name).CreateDelegate(); + InvalidOperationException exception = Assert.Throws(action); + Assert.Contains("ThrowingMethods." + name + "()", exception.ToString()); + } + + return new WeakReference(type); + } + private static void VerifyStackTrace(Exception e, string[] templateStackTrace) { string[] stackTrace = e.StackTrace.Split(Environment.NewLine, StringSplitOptions.RemoveEmptyEntries); diff --git a/src/tests/profiler/native/gcprofiler/gcprofiler.cpp b/src/tests/profiler/native/gcprofiler/gcprofiler.cpp index 45d810bbf21f03..55dd1cd31627f2 100644 --- a/src/tests/profiler/native/gcprofiler/gcprofiler.cpp +++ b/src/tests/profiler/native/gcprofiler/gcprofiler.cpp @@ -104,6 +104,16 @@ HRESULT GCProfiler::ObjectsAllocatedByClass(ULONG cClassCount, ClassID classIds[ printf("GCProfiler::ObjectsAllocatedByClass: FAIL: Expected ObjectsAllocatedByClass Calls == GCStart. AllocatedByClassCalls=%d, GCStart=%d\n", (int)_allocatedByClassCalls, (int)_gcStarts); } + std::unordered_set classes; + for (ULONG i = 0; i < cClassCount; i++) + { + if (classIds[i] == 0 || cObjects[i] == 0 || !classes.insert(classIds[i]).second) + { + _failures++; + printf("GCProfiler::ObjectsAllocatedByClass: FAIL: Invalid or duplicate class entry at %u\n", i); + } + } + return S_OK; }