From 7e94553dea9679537da0860fe1d817085675aea7 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 14 Nov 2023 14:55:02 -0800 Subject: [PATCH 01/13] Fixup EEClassHash to support a hash function that includes enclosing class - Move namespace/name splitting to the Type.GetType code paths - Move exported type handling into the normal PopulateAvailableClass flow - Remove unnecessary work done to detect typedef name duplicates. We don't attempt to protect against invaild assemblies anymore - Unify path for insertion between ExportedType and TypeDef records, also unify the path for nested vs non-nested - Fix logic which implements inserts into the case insensitive table when dynamically adding entries to the ExportedType table (Previously it didn't work) - Update the ECMA 335 augments to capture the requirement that nested ExportedTypes must have a higher RID than the enclosing ExportedType - This requirement has actually always existed since .NET 1.0, but was never recorded --- docs/design/specs/Ecma-335-Augments.md | 6 + src/coreclr/inc/corhlprpriv.h | 4 +- src/coreclr/vm/assembly.cpp | 21 - src/coreclr/vm/assembly.hpp | 2 - src/coreclr/vm/assemblynative.cpp | 38 +- src/coreclr/vm/binder.cpp | 5 +- src/coreclr/vm/ceeload.cpp | 2 +- src/coreclr/vm/classhash.cpp | 763 +++++++++++++------------ src/coreclr/vm/classhash.h | 29 +- src/coreclr/vm/classhash.inl | 7 +- src/coreclr/vm/clsload.cpp | 548 ++++-------------- src/coreclr/vm/clsload.hpp | 26 +- src/coreclr/vm/readytoruninfo.cpp | 29 +- 13 files changed, 604 insertions(+), 876 deletions(-) diff --git a/docs/design/specs/Ecma-335-Augments.md b/docs/design/specs/Ecma-335-Augments.md index 6dd9a81099ccc4..886bb3cb3d711f 100644 --- a/docs/design/specs/Ecma-335-Augments.md +++ b/docs/design/specs/Ecma-335-Augments.md @@ -381,6 +381,12 @@ The text should be deleted: > Furthermore, ~~the InterfaceImpl table is sorted using the Interface column as a secondary key, and~~ the GenericParam table is sorted using the Number column as a secondary key. +In addition to the TypeDef table having a special ordering constraint, the ExportedTypes table ALSO has the same constraint. + +This line should be changed. + +> Finally, this TypeDef _and ExportedType_ ~~table has~~ _tables have_ a special ordering constraint: the definition of an enclosing class shall precede the definintion of all classes it encloses. + ## Module Initializer All modules may have a module initializer. A module initializer is defined as the type initializer (§ II.10.5.3) of the `` type (§ II.10.8). diff --git a/src/coreclr/inc/corhlprpriv.h b/src/coreclr/inc/corhlprpriv.h index ffa249543329e3..a043b08a4a4b07 100644 --- a/src/coreclr/inc/corhlprpriv.h +++ b/src/coreclr/inc/corhlprpriv.h @@ -294,9 +294,9 @@ class CQuickMemoryBase } // Copy single byte string and hold it - const char * SetStringNoThrow(const char * pStr, SIZE_T len) + const char * SetString(const char * pStr, SIZE_T len) { - LPSTR buffer = (LPSTR) AllocNoThrow(len + 1); + LPSTR buffer = (LPSTR) AllocThrows(len + 1); if (buffer != NULL) { diff --git a/src/coreclr/vm/assembly.cpp b/src/coreclr/vm/assembly.cpp index 8833aa45742b30..eff18ed372bcac 100644 --- a/src/coreclr/vm/assembly.cpp +++ b/src/coreclr/vm/assembly.cpp @@ -199,9 +199,6 @@ void Assembly::Init(AllocMemTracker *pamTracker, LoaderAllocator *pLoaderAllocat PrepareModuleForAssembly(m_pModule, pamTracker); - if (!m_pModule->IsReadyToRun()) - CacheManifestExportedTypes(pamTracker); - // We'll load the friend assembly information lazily. For the ngen case we should avoid // loading it entirely. //CacheFriendAssemblyInfo(); @@ -952,24 +949,6 @@ Module * Assembly::FindModuleByTypeRef( #ifndef DACCESS_COMPILE -void Assembly::CacheManifestExportedTypes(AllocMemTracker *pamTracker) -{ - STANDARD_VM_CONTRACT; - - mdToken mdExportedType; - - HENUMInternalHolder phEnum(GetMDImport()); - phEnum.EnumInit(mdtExportedType, - mdTokenNil); - - ClassLoader::AvailableClasses_LockHolder lh(m_pClassLoader); - - for(int i = 0; GetMDImport()->EnumNext(&phEnum, &mdExportedType); i++) - m_pClassLoader->AddExportedTypeHaveLock(GetModule(), - mdExportedType, - pamTracker); -} - void Assembly::PrepareModuleForAssembly(Module* module, AllocMemTracker *pamTracker) { STANDARD_VM_CONTRACT; diff --git a/src/coreclr/vm/assembly.hpp b/src/coreclr/vm/assembly.hpp index c70c85861b54fd..0166875b539c67 100644 --- a/src/coreclr/vm/assembly.hpp +++ b/src/coreclr/vm/assembly.hpp @@ -421,8 +421,6 @@ class Assembly //**************************************************************************************** - void CacheManifestExportedTypes(AllocMemTracker *pamTracker); - void CacheFriendAssemblyInfo(); #ifndef DACCESS_COMPILE ReleaseHolder GetFriendAssemblyInfo(); diff --git a/src/coreclr/vm/assemblynative.cpp b/src/coreclr/vm/assemblynative.cpp index 340096c919e989..79caef32d4eae0 100644 --- a/src/coreclr/vm/assemblynative.cpp +++ b/src/coreclr/vm/assemblynative.cpp @@ -362,10 +362,27 @@ extern "C" void QCALLTYPE AssemblyNative_GetTypeCore(QCall::AssemblyHandle assem ClassLoader* pClassLoader = pAssembly->GetLoader(); NameHandle typeName(pManifestModule, mdtBaseType); + CQuickBytes qbszNamespace; for (int32_t i = -1; i < cNestedTypeNamesLength; i++) { - typeName.SetName((i == -1) ? szTypeName : rgszNestedTypeNames[i]); + LPCUTF8 szFullyQualifiedName = (i == -1) ? szTypeName : rgszNestedTypeNames[i]; + + LPCUTF8 szNameSpace = ""; + LPCUTF8 szName = ""; + + if ((szName = ns::FindSep(szFullyQualifiedName)) != NULL) + { + SIZE_T d = szName - szFullyQualifiedName; + szNameSpace = qbszNamespace.SetString(szFullyQualifiedName, d); + szName++; + } + else + { + szName = szFullyQualifiedName; + } + + typeName.SetName(szNameSpace, szName); // typeName.m_pBucket gets set here if the type is found // it will be used in the next iteration to look up the nested type @@ -415,6 +432,7 @@ extern "C" void QCALLTYPE AssemblyNative_GetTypeCoreIgnoreCase(QCall::AssemblyHa ClassLoader* pClassLoader = pAssembly->GetLoader(); NameHandle typeName(pManifestModule, mdtBaseType); + CQuickBytes qbszNamespace; // Set up the name handle typeName.SetCaseInsensitive(); @@ -427,7 +445,23 @@ extern "C" void QCALLTYPE AssemblyNative_GetTypeCoreIgnoreCase(QCall::AssemblyHa // The type name is expected to be lower-cased by the caller for case-insensitive lookups name.LowerCase(); - typeName.SetName(name.GetUTF8()); + LPCUTF8 szFullyQualifiedName = name.GetUTF8(); + + LPCUTF8 szNameSpace = ""; + LPCUTF8 szName = ""; + + if ((szName = ns::FindSep(szFullyQualifiedName)) != NULL) + { + SIZE_T d = szName - szFullyQualifiedName; + szNameSpace = qbszNamespace.SetString(szFullyQualifiedName, d); + szName++; + } + else + { + szName = szFullyQualifiedName; + } + + typeName.SetName(szNameSpace, szName); // typeName.m_pBucket gets set here if the type is found // it will be used in the next iteration to look up the nested type diff --git a/src/coreclr/vm/binder.cpp b/src/coreclr/vm/binder.cpp index 23b14a34ee43b6..193cc3217c2271 100644 --- a/src/coreclr/vm/binder.cpp +++ b/src/coreclr/vm/binder.cpp @@ -83,10 +83,13 @@ PTR_MethodTable CoreLibBinder::LookupClassLocal(BinderClassID id) (void)ClassLoader::LoadTypeByNameThrowing(GetModule()->GetAssembly(), &nameHandle); // Now load the nested type. - nameHandle.SetName(NULL, nestedTypeMaybe + 1); + nameHandle.SetName("", nestedTypeMaybe + 1); // We don't support nested types in nested types. _ASSERTE(strchr(nameHandle.GetName(), '+') == NULL); + + // We don't support nested types with explicit namespaces + _ASSERTE(strchr(nameHandle.GetName(), '.') == NULL); pMT = ClassLoader::LoadTypeByNameThrowing(GetModule()->GetAssembly(), &nameHandle).AsMethodTable(); } diff --git a/src/coreclr/vm/ceeload.cpp b/src/coreclr/vm/ceeload.cpp index cbabf92302b798..9a7dbc2f41de33 100644 --- a/src/coreclr/vm/ceeload.cpp +++ b/src/coreclr/vm/ceeload.cpp @@ -460,7 +460,7 @@ void Module::Initialize(AllocMemTracker *pamTracker, LPCWSTR szName) { m_pAvailableClasses = EEClassHashTable::Create(this, GetAssembly()->IsCollectible() ? AVAILABLE_CLASSES_HASH_BUCKETS_COLLECTIBLE : AVAILABLE_CLASSES_HASH_BUCKETS, - FALSE /* bCaseInsensitive */, pamTracker); + NULL, pamTracker); } if (m_pAvailableParamTypes == NULL) diff --git a/src/coreclr/vm/classhash.cpp b/src/coreclr/vm/classhash.cpp index 8ea03a315154ce..ed8f5b896684a0 100644 --- a/src/coreclr/vm/classhash.cpp +++ b/src/coreclr/vm/classhash.cpp @@ -68,7 +68,7 @@ void EEClassHashEntry::SetEncloser(EEClassHashEntry *pEncloser) } /*static*/ -EEClassHashTable *EEClassHashTable::Create(Module *pModule, DWORD dwNumBuckets, BOOL bCaseInsensitive, AllocMemTracker *pamTracker) +EEClassHashTable *EEClassHashTable::Create(Module *pModule, DWORD dwNumBuckets, PTR_EEClassHashTable pCaseSensitiveTable, AllocMemTracker *pamTracker) { CONTRACTL { @@ -88,7 +88,7 @@ EEClassHashTable *EEClassHashTable::Create(Module *pModule, DWORD dwNumBuckets, // loader heap instead of new so use an in-place new to call the constructors now. new (pThis) EEClassHashTable(pModule, pHeap, dwNumBuckets); - pThis->m_bCaseInsensitive = bCaseInsensitive; + pThis->m_pCaseSensitiveTable = pCaseSensitiveTable != NULL ? pCaseSensitiveTable : pThis; return pThis; } @@ -223,17 +223,17 @@ VOID EEClassHashTable::ConstructKeyFromData(PTR_EEClassHashEntry pEntry, // IN THROWS; WRAPPER(MODE_ANY); WRAPPER(GC_TRIGGERS); - if (m_bCaseInsensitive) INJECT_FAULT(COMPlusThrowOM();); else WRAPPER(FORBID_FAULT); + if (IsCaseInsensitiveTable()) INJECT_FAULT(COMPlusThrowOM();); else WRAPPER(FORBID_FAULT); SUPPORTS_DAC; } CONTRACTL_END; { #ifdef _DEBUG_IMPL - _ASSERTE(!(m_bCaseInsensitive && FORBIDGC_LOADER_USE_ENABLED())); + _ASSERTE(!(IsCaseInsensitiveTable() && FORBIDGC_LOADER_USE_ENABLED())); #endif - // cqb - If m_bCaseInsensitive is true for the hash table, the bytes in Key will be allocated + // cqb - If IsCaseInsensitiveTable() is true for the hash table, the bytes in Key will be allocated // from cqb. This is to prevent wasting bytes in the Loader Heap. Thusly, it is important to note that // in this case, the lifetime of Key is bounded by the lifetime of cqb, which will free the memory // it allocated on destruction. @@ -244,7 +244,7 @@ VOID EEClassHashTable::ConstructKeyFromData(PTR_EEClassHashEntry pEntry, // IN IMDInternalImport *pInternalImport = NULL; PTR_VOID Data = NULL; - if (!m_bCaseInsensitive) + if (!IsCaseInsensitiveTable()) Data = pEntry->GetData(); else Data = (PTR_EEClassHashEntry(pEntry->GetData()))->GetData(); @@ -287,7 +287,7 @@ VOID EEClassHashTable::ConstructKeyFromData(PTR_EEClassHashEntry pEntry, // IN } } - if (!m_bCaseInsensitive) + if (!IsCaseInsensitiveTable()) { LPUTF8 Key[2]; @@ -311,54 +311,6 @@ VOID EEClassHashTable::ConstructKeyFromData(PTR_EEClassHashEntry pEntry, // IN #ifndef DACCESS_COMPILE -EEClassHashEntry_t *EEClassHashTable::InsertValue(LPCUTF8 pszNamespace, LPCUTF8 pszClassName, PTR_VOID Data, EEClassHashEntry_t *pEncloser, AllocMemTracker *pamTracker) -{ - CONTRACTL - { - THROWS; - GC_NOTRIGGER; - MODE_ANY; - - INJECT_FAULT(COMPlusThrowOM();); - PRECONDITION(!FORBIDGC_LOADER_USE_ENABLED()); - } - CONTRACTL_END; - - _ASSERTE(pszNamespace != NULL); - _ASSERTE(pszClassName != NULL); - _ASSERTE(m_pModule != NULL); - - EEClassHashEntry *pEntry = BaseAllocateEntry(pamTracker); - - pEntry->SetData(Data); - pEntry->SetEncloser(pEncloser); -#ifdef _DEBUG - pEntry->DebugKey[0] = pszNamespace; - pEntry->DebugKey[1] = pszClassName; -#endif - - BaseInsertEntry(Hash(pszNamespace, pszClassName), pEntry); - - return pEntry; -} - -#ifdef _DEBUG -class ConstructKeyCallbackValidate : public EEClassHashTable::ConstructKeyCallback -{ -public: - virtual void UseKeys(_In_reads_(2) LPUTF8 *Key) - { - LIMITED_METHOD_CONTRACT; - STATIC_CONTRACT_DEBUG_ONLY; - _ASSERTE (strcmp(pNewEntry->DebugKey[1], Key[1]) == 0); - _ASSERTE (strcmp(pNewEntry->DebugKey[0], Key[0]) == 0); - SUPPORTS_DAC; - } - - EEClassHashEntry_t *pNewEntry; - -}; -#endif // _DEBUG // This entrypoint lets the caller separate the allocation of the entrypoint from the actual insertion into the hashtable. (This lets us // do multiple insertions without having to worry about an OOM occurring inbetween.) @@ -380,309 +332,20 @@ EEClassHashEntry_t *EEClassHashTable::InsertValueUsingPreallocatedEntry(EEClassH pNewEntry->SetData(Data); pNewEntry->SetEncloser(pEncloser); + pNewEntry->SetHash(Hash(pszNamespace, pszClassName, pEncloser != NULL ? pEncloser->GetHash() : 0)); #ifdef _DEBUG pNewEntry->DebugKey[0] = pszNamespace; pNewEntry->DebugKey[1] = pszClassName; #endif - BaseInsertEntry(Hash(pszNamespace, pszClassName), pNewEntry); - - return pNewEntry; -} - -EEClassHashEntry_t *EEClassHashTable::InsertValueIfNotFound(LPCUTF8 pszNamespace, LPCUTF8 pszClassName, PTR_VOID *pData, EEClassHashEntry_t *pEncloser, BOOL IsNested, BOOL *pbFound, AllocMemTracker *pamTracker) -{ - CONTRACTL - { - THROWS; - GC_NOTRIGGER; - MODE_ANY; - INJECT_FAULT(COMPlusThrowOM();); - - PRECONDITION(!FORBIDGC_LOADER_USE_ENABLED()); - } - CONTRACTL_END; - - _ASSERTE(m_pModule != NULL); - _ASSERTE(pszNamespace != NULL); - _ASSERTE(pszClassName != NULL); - - EEClassHashEntry_t * pNewEntry = FindItem(pszNamespace, pszClassName, IsNested, NULL); - - if (pNewEntry) - { - *pData = pNewEntry->GetData(); - *pbFound = TRUE; - return pNewEntry; - } - - // Reached here implies that we didn't find the entry and need to insert it - *pbFound = FALSE; - - pNewEntry = BaseAllocateEntry(pamTracker); - - pNewEntry->SetData(*pData); - pNewEntry->SetEncloser(pEncloser); - -#ifdef _DEBUG - pNewEntry->DebugKey[0] = pszNamespace; - pNewEntry->DebugKey[1] = pszClassName; -#endif - - BaseInsertEntry(Hash(pszNamespace, pszClassName), pNewEntry); + BaseInsertEntry(pNewEntry->GetHash(), pNewEntry); return pNewEntry; } #endif // !DACCESS_COMPILE -EEClassHashEntry_t *EEClassHashTable::FindItem(LPCUTF8 pszNamespace, LPCUTF8 pszClassName, BOOL IsNested, LookupContext *pContext) -{ - CONTRACTL - { - if (m_bCaseInsensitive) THROWS; else NOTHROW; - if (m_bCaseInsensitive) GC_TRIGGERS; else GC_NOTRIGGER; - if (m_bCaseInsensitive) INJECT_FAULT(COMPlusThrowOM();); else FORBID_FAULT; - MODE_ANY; - SUPPORTS_DAC; - } - CONTRACTL_END; - - _ASSERTE(m_pModule != NULL); - _ASSERTE(pszNamespace != NULL); - _ASSERTE(pszClassName != NULL); - - // It's legal for the caller not to pass us a LookupContext (when the type being queried is not nested - // there will never be any need to iterate over the search results). But we might need to iterate - // internally (since we lookup via hash and hashes may collide). So substitute our own private context if - // one was not provided. - LookupContext sAltContext; - if (pContext == NULL) - pContext = &sAltContext; - - // The base class provides the ability to enumerate all entries with the same hash code. We call this and - // further check which of these entries actually match the full key (there can be multiple hits with - // nested types in the picture). - PTR_EEClassHashEntry pSearch = BaseFindFirstEntryByHash(Hash(pszNamespace, pszClassName), pContext); - - while (pSearch) - { - LPCUTF8 rgKey[] = { pszNamespace, pszClassName }; - - if (CompareKeys(pSearch, rgKey)) - { - // If (IsNested), then we're looking for a nested class - // If (pSearch->pEncloser), we've found a nested class - if ((IsNested != FALSE) == (pSearch->GetEncloser() != NULL)) - { - return pSearch; - } - } - - pSearch = BaseFindNextEntryByHash(pContext); - } - - return NULL; -} - -EEClassHashEntry_t *EEClassHashTable::FindNextNestedClass(const NameHandle* pName, PTR_VOID *pData, LookupContext *pContext) -{ - CONTRACTL - { - if (m_bCaseInsensitive) THROWS; else NOTHROW; - if (m_bCaseInsensitive) GC_TRIGGERS; else GC_NOTRIGGER; - if (m_bCaseInsensitive) INJECT_FAULT(COMPlusThrowOM();); else FORBID_FAULT; - MODE_ANY; - SUPPORTS_DAC; - } - CONTRACTL_END; - - _ASSERTE(m_pModule != NULL); - _ASSERTE(pName); - - if (pName->GetNameSpace()) - { - return FindNextNestedClass(pName->GetNameSpace(), pName->GetName(), pData, pContext); - } - else { -#ifndef DACCESS_COMPILE - return FindNextNestedClass(pName->GetName(), pData, pContext); // this won't support dac-- - // it allocates a new namespace string -#else - DacNotImpl(); - return NULL; -#endif - } -} - - -EEClassHashEntry_t *EEClassHashTable::FindNextNestedClass(LPCUTF8 pszNamespace, LPCUTF8 pszClassName, PTR_VOID *pData, LookupContext *pContext) -{ - CONTRACTL - { - if (m_bCaseInsensitive) THROWS; else NOTHROW; - if (m_bCaseInsensitive) GC_TRIGGERS; else GC_NOTRIGGER; - if (m_bCaseInsensitive) INJECT_FAULT(COMPlusThrowOM();); else FORBID_FAULT; - MODE_ANY; - SUPPORTS_DAC; - } - CONTRACTL_END; - - _ASSERTE(m_pModule != NULL); - - PTR_EEClassHashEntry pSearch = BaseFindNextEntryByHash(pContext); - - while (pSearch) - { - LPCUTF8 rgKey[] = { pszNamespace, pszClassName }; - - if (pSearch->GetEncloser() && CompareKeys(pSearch, rgKey)) - { - *pData = pSearch->GetData(); - return pSearch; - } - - pSearch = BaseFindNextEntryByHash(pContext); - } - - return NULL; -} - -const UTF8 Utf8Empty[] = { 0 }; - -EEClassHashEntry_t *EEClassHashTable::FindNextNestedClass(LPCUTF8 pszFullyQualifiedName, PTR_VOID *pData, LookupContext *pContext) -{ - CONTRACTL - { - if (m_bCaseInsensitive) THROWS; else NOTHROW; - if (m_bCaseInsensitive) GC_TRIGGERS; else GC_NOTRIGGER; - if (m_bCaseInsensitive) INJECT_FAULT(COMPlusThrowOM();); else FORBID_FAULT; - MODE_ANY; - } - CONTRACTL_END; - - _ASSERTE(m_pModule != NULL); - - CQuickBytes szNamespace; - - LPCUTF8 pNamespace = Utf8Empty; - LPCUTF8 p; - - if ((p = ns::FindSep(pszFullyQualifiedName)) != NULL) - { - SIZE_T d = p - pszFullyQualifiedName; - - FAULT_NOT_FATAL(); - pNamespace = szNamespace.SetStringNoThrow(pszFullyQualifiedName, d); - - if (NULL == pNamespace) - { - return NULL; - } - - p++; - } - else - { - p = pszFullyQualifiedName; - } - - return FindNextNestedClass(pNamespace, p, pData, pContext); -} - - -EEClassHashEntry_t * EEClassHashTable::GetValue(LPCUTF8 pszFullyQualifiedName, PTR_VOID *pData, BOOL IsNested, LookupContext *pContext) -{ - CONTRACTL - { - if (m_bCaseInsensitive) THROWS; else NOTHROW; - if (m_bCaseInsensitive) GC_TRIGGERS; else GC_NOTRIGGER; - if (m_bCaseInsensitive) INJECT_FAULT(COMPlusThrowOM();); else FORBID_FAULT; - MODE_ANY; - SUPPORTS_DAC; - } - CONTRACTL_END; - - _ASSERTE(m_pModule != NULL); - - CQuickBytes szNamespace; - - LPCUTF8 pNamespace = Utf8Empty; - - LPCUTF8 p = ns::FindSep(pszFullyQualifiedName); - - if (p != NULL) - { - SIZE_T d = p - pszFullyQualifiedName; - - FAULT_NOT_FATAL(); - pNamespace = szNamespace.SetStringNoThrow(pszFullyQualifiedName, d); - - if (NULL == pNamespace) - { - return NULL; - } - - p++; - } - else - { - p = pszFullyQualifiedName; - } - - EEClassHashEntry_t * ret = GetValue(pNamespace, p, pData, IsNested, pContext); - - return ret; -} - - -EEClassHashEntry_t * EEClassHashTable::GetValue(LPCUTF8 pszNamespace, LPCUTF8 pszClassName, PTR_VOID *pData, BOOL IsNested, LookupContext *pContext) -{ - CONTRACTL - { - if (m_bCaseInsensitive) THROWS; else NOTHROW; - if (m_bCaseInsensitive) GC_TRIGGERS; else GC_NOTRIGGER; - if (m_bCaseInsensitive) INJECT_FAULT(COMPlusThrowOM();); else FORBID_FAULT; - MODE_ANY; - SUPPORTS_DAC; - } - CONTRACTL_END; - - - _ASSERTE(m_pModule != NULL); - EEClassHashEntry_t *pItem = FindItem(pszNamespace, pszClassName, IsNested, pContext); - if (pItem) - *pData = pItem->GetData(); - - return pItem; -} - - -EEClassHashEntry_t * EEClassHashTable::GetValue(const NameHandle* pName, PTR_VOID *pData, BOOL IsNested, LookupContext *pContext) -{ - CONTRACTL - { - // for DAC builds m_bCaseInsensitive should be false - if (m_bCaseInsensitive) THROWS; else NOTHROW; - if (m_bCaseInsensitive) GC_TRIGGERS; else GC_NOTRIGGER; - if (m_bCaseInsensitive) INJECT_FAULT(COMPlusThrowOM();); else FORBID_FAULT; - MODE_ANY; - SUPPORTS_DAC; - } - CONTRACTL_END; - - - _ASSERTE(pName); - _ASSERTE(m_pModule != NULL); - if(pName->GetNameSpace() == NULL) { - return GetValue(pName->GetName(), pData, IsNested, pContext); - } - else { - return GetValue(pName->GetNameSpace(), pName->GetName(), pData, IsNested, pContext); - } -} - class ConstructKeyCallbackCompare : public EEClassHashTable::ConstructKeyCallback { public: @@ -708,9 +371,9 @@ BOOL EEClassHashTable::CompareKeys(PTR_EEClassHashEntry pEntry, LPCUTF8 * pKey2) { CONTRACTL { - if (m_bCaseInsensitive) THROWS; else NOTHROW; - if (m_bCaseInsensitive) GC_TRIGGERS; else GC_NOTRIGGER; - if (m_bCaseInsensitive) INJECT_FAULT(COMPlusThrowOM();); else FORBID_FAULT; + if (IsCaseInsensitiveTable()) THROWS; else NOTHROW; + if (IsCaseInsensitiveTable()) GC_TRIGGERS; else GC_NOTRIGGER; + if (IsCaseInsensitiveTable()) INJECT_FAULT(COMPlusThrowOM();); else FORBID_FAULT; MODE_ANY; SUPPORTS_DAC; } @@ -791,7 +454,7 @@ EEClassHashTable *EEClassHashTable::MakeCaseInsensitiveTable(Module *pModule, Al // Allocate the table and verify that we actually got one. EEClassHashTable * pCaseInsTable = EEClassHashTable::Create(pModule, max(BaseGetElementCount() / 2, 11), - TRUE /* bCaseInsensitive */, + this, pamTracker); // Walk all of the buckets and insert them into our new case insensitive table @@ -808,9 +471,407 @@ EEClassHashTable *EEClassHashTable::MakeCaseInsensitiveTable(Module *pModule, Al //Add the newly created name to our hash table. The hash datum is a pointer //to the entry associated with that name in this hashtable. - pCaseInsTable->InsertValue(pszLowerNameSpace, pszLowerClsName, (PTR_VOID)pTempEntry, pTempEntry->GetEncloser(), pamTracker); + pCaseInsTable->InsertValueUsingPreallocatedEntry(pCaseInsTable->BaseAllocateEntry(pamTracker), pszLowerNameSpace, pszLowerClsName, pTempEntry, pTempEntry->GetEncloser()); } return pCaseInsTable; } #endif // !DACCESS_COMPILE + +BOOL CompareNestedEntryWithExportedType(IMDInternalImport * pImport, + mdExportedType mdCurrent, + EEClassHashTable * pClassHash, + PTR_EEClassHashEntry pEntry) +{ + CONTRACTL + { + NOTHROW; + GC_NOTRIGGER; + MODE_ANY; + FORBID_FAULT; + SUPPORTS_DAC; + } + CONTRACTL_END; + + LPCUTF8 Key[2]; + + do + { + if (FAILED(pImport->GetExportedTypeProps( + mdCurrent, + &Key[0], + &Key[1], + &mdCurrent, + NULL, //binding (type def) + NULL))) //flags + { + return FALSE; + } + + if (pClassHash->CompareKeys(pEntry, Key)) + { + // Reached top level class for mdCurrent - return whether + // or not pEntry is a top level class + // (pEntry is a top level class if its pEncloser is NULL) + if ((TypeFromToken(mdCurrent) != mdtExportedType) || + (mdCurrent == mdExportedTypeNil)) + { + return pEntry->GetEncloser() == NULL; + } + } + else // Keys don't match - wrong entry + { + return FALSE; + } + } + while ((pEntry = pEntry->GetEncloser()) != NULL); + + // Reached the top level class for pEntry, but mdCurrent is nested + return FALSE; +} + +DWORD ComputeHashFunctionWithExportedType(EEClassHashTable * pClassHash, EEClassHashTable * pCaseSensitiveClassHash, IMDInternalImport *pImport, mdExportedType etCurrent, BOOL *pFailed) +{ + LPCSTR _namespace, name; + if (FAILED(pImport->GetExportedTypeProps( + etCurrent, + &_namespace, + &name, + &etCurrent, + NULL, //binding (type def) + NULL))) //flags + { + return FALSE; + } + DWORD hashEncloser = 0; + if (TypeFromToken(etCurrent) == mdtExportedType && etCurrent != mdExportedTypeNil) + { + // The enclosing hash is always based on the entry from the case sensitive table + hashEncloser = ComputeHashFunctionWithExportedType(pCaseSensitiveClassHash, pCaseSensitiveClassHash, pImport, etCurrent, pFailed); + } + return pClassHash->Hash(_namespace, name, hashEncloser); +} + +BOOL CompareNestedEntryWithTypeDef(IMDInternalImport * pImport, + mdTypeDef mdCurrent, + EEClassHashTable * pClassHash, + PTR_EEClassHashEntry pEntry) +{ + CONTRACTL + { + NOTHROW; + GC_NOTRIGGER; + MODE_ANY; + FORBID_FAULT; + SUPPORTS_DAC; + } + CONTRACTL_END; + + LPCUTF8 Key[2]; + + do { + if (FAILED(pImport->GetNameOfTypeDef(mdCurrent, &Key[1], &Key[0]))) + { + return FALSE; + } + + if (pClassHash->CompareKeys(pEntry, Key)) { + // Reached top level class for mdCurrent - return whether + // or not pEntry is a top level class + // (pEntry is a top level class if its pEncloser is NULL) + if (FAILED(pImport->GetNestedClassProps(mdCurrent, &mdCurrent))) + return pEntry->GetEncloser() == NULL; + } + else // Keys don't match - wrong entry + return FALSE; + } + while ((pEntry = pEntry->GetEncloser()) != NULL); + + // Reached the top level class for pEntry, but mdCurrent is nested + return FALSE; +} + +DWORD ComputeHashFunctionWithTypeDef(EEClassHashTable *pClassHash, EEClassHashTable *pCaseSensitiveClassHash, IMDInternalImport *pImport, mdTypeDef tdCurrent, BOOL *pFailed) +{ + LPCSTR _namespace, name; + if (FAILED(pImport->GetNameOfTypeDef(tdCurrent, &name, &_namespace))) + { + *pFailed = TRUE; + return 0; + } + DWORD hashEncloser = 0; + if (SUCCEEDED(pImport->GetNestedClassProps(tdCurrent, &tdCurrent))) + { + // The enclosing hash is always based on the entry from the case sensitive table + hashEncloser = ComputeHashFunctionWithTypeDef(pCaseSensitiveClassHash, pCaseSensitiveClassHash, pImport, tdCurrent, pFailed); + } + return pClassHash->Hash(_namespace, name, hashEncloser); +} + +BOOL CompareNestedEntryWithTypeRef(IMDInternalImport * pImport, + mdTypeRef mdCurrent, + EEClassHashTable * pClassHash, + PTR_EEClassHashEntry pEntry) +{ + CONTRACTL + { + NOTHROW; + GC_NOTRIGGER; + MODE_ANY; + FORBID_FAULT; + SUPPORTS_DAC; + } + CONTRACTL_END; + + LPCUTF8 Key[2]; + + do { + if (FAILED(pImport->GetNameOfTypeRef(mdCurrent, &Key[0], &Key[1]))) + { + return FALSE; + } + + if (pClassHash->CompareKeys(pEntry, Key)) + { + if (FAILED(pImport->GetResolutionScopeOfTypeRef(mdCurrent, &mdCurrent))) + { + return FALSE; + } + // Reached top level class for mdCurrent - return whether + // or not pEntry is a top level class + // (pEntry is a top level class if its pEncloser is NULL) + if ((TypeFromToken(mdCurrent) != mdtTypeRef) || + (mdCurrent == mdTypeRefNil)) + return pEntry->GetEncloser() == NULL; + } + else // Keys don't match - wrong entry + return FALSE; + } + while ((pEntry = pEntry->GetEncloser())!=NULL); + + // Reached the top level class for pEntry, but mdCurrent is nested + return FALSE; +} + + +DWORD ComputeHashFunctionWithTypeRef(EEClassHashTable *pClassHash, EEClassHashTable *pCaseSensitiveClassHash, IMDInternalImport *pImport, mdTypeRef trCurrent, BOOL *pFailed) +{ + LPCSTR _namespace, name; + if (FAILED(pImport->GetNameOfTypeRef(trCurrent, &_namespace, &name))) + { + *pFailed = TRUE; + return 0; + } + DWORD hashEncloser = 0; + if (SUCCEEDED(pImport->GetResolutionScopeOfTypeRef(trCurrent, &trCurrent)) && TypeFromToken(trCurrent) == mdtTypeRef) + { + // The enclosing hash is always based on the entry from the case sensitive table + hashEncloser = ComputeHashFunctionWithTypeRef(pCaseSensitiveClassHash, pCaseSensitiveClassHash, pImport, trCurrent, pFailed); + } + return pClassHash->Hash(_namespace, name, hashEncloser); +} + +/*static*/ +BOOL EEClassHashTable::IsNested(ModuleBase *pModule, mdToken token, mdToken *mdEncloser) +{ + CONTRACTL + { + if (FORBIDGC_LOADER_USE_ENABLED()) NOTHROW; else THROWS; + if (FORBIDGC_LOADER_USE_ENABLED()) GC_NOTRIGGER; else GC_TRIGGERS; + if (FORBIDGC_LOADER_USE_ENABLED()) FORBID_FAULT; else { INJECT_FAULT(COMPlusThrowOM()); } + MODE_ANY; + SUPPORTS_DAC; + } + CONTRACTL_END; + + switch(TypeFromToken(token)) { + case mdtTypeDef: + return (SUCCEEDED(pModule->GetMDImport()->GetNestedClassProps(token, mdEncloser))); + + case mdtTypeRef: + IfFailThrow(pModule->GetMDImport()->GetResolutionScopeOfTypeRef(token, mdEncloser)); + return ((TypeFromToken(*mdEncloser) == mdtTypeRef) && + (*mdEncloser != mdTypeRefNil)); + + case mdtExportedType: + _ASSERTE(pModule->IsFullModule()); + IfFailThrow(((Module*)pModule)->GetAssembly()->GetMDImport()->GetExportedTypeProps( + token, + NULL, // namespace + NULL, // name + mdEncloser, + NULL, //binding (type def) + NULL)); //flags + return ((TypeFromToken(*mdEncloser) == mdtExportedType) && + (*mdEncloser != mdExportedTypeNil)); + + default: + ThrowHR(COR_E_BADIMAGEFORMAT, BFA_INVALID_TOKEN_TYPE); + } +} + +BOOL EEClassHashTable::IsNested(const NameHandle* pName, mdToken *mdEncloser) +{ + CONTRACTL + { + if (FORBIDGC_LOADER_USE_ENABLED()) NOTHROW; else THROWS; + if (FORBIDGC_LOADER_USE_ENABLED()) GC_NOTRIGGER; else GC_TRIGGERS; + if (FORBIDGC_LOADER_USE_ENABLED()) FORBID_FAULT; else { INJECT_FAULT(COMPlusThrowOM()); } + MODE_ANY; + SUPPORTS_DAC; + } + CONTRACTL_END; + + if (pName->GetTypeModule()) { + if (TypeFromToken(pName->GetTypeToken()) == mdtBaseType) + { + if (!pName->GetBucket().IsNull()) + return TRUE; + return FALSE; + } + else + return IsNested(pName->GetTypeModule(), pName->GetTypeToken(), mdEncloser); + } + else + return FALSE; +} + +EEClassHashEntry_t * EEClassHashTable::FindByNameHandle(const NameHandle* pName) +{ + // TODO remove this pointless local + EEClassHashTable *pTable = this; + + PTR_EEClassHashEntry pBucket; + mdToken mdEncloser; + bool isNested = IsNested(pName, &mdEncloser); + LPCUTF8 pszName = NULL, pszNamespace = NULL; + DWORD hash; + BOOL failed; + + _ASSERTE(pName->GetNameSpace() != NULL); + _ASSERTE(pName->GetName() != NULL); + + pszName = pName->GetName(); + pszNamespace = pName->GetNameSpace(); + + mdToken typeToken = pName->GetTypeToken(); + ModuleBase *pNameModule = pName->GetTypeModule(); + PREFIX_ASSUME(pNameModule != NULL); + + switch (TypeFromToken(typeToken)) + { + case mdtTypeDef: + hash = ComputeHashFunctionWithTypeDef(pTable, m_pCaseSensitiveTable, pNameModule->GetMDImport(), typeToken, &failed); + break; + case mdtTypeRef: + hash = ComputeHashFunctionWithTypeRef(pTable, m_pCaseSensitiveTable, pNameModule->GetMDImport(), typeToken, &failed); + break; + case mdtExportedType: + hash = ComputeHashFunctionWithExportedType(pTable, m_pCaseSensitiveTable, pNameModule->GetMDImport(), typeToken, &failed); + break; + default: + DWORD enclosingHash; + if (pName->GetBucket().IsNull()) + { + enclosingHash = 0; + } + else + { + // The enclosing hash is always based on the entry from the case sensitive table + // A NameHandle bucket is always the entry in the CaseSensitive table + enclosingHash = pName->GetBucket().GetClassHashBasedEntryValue()->GetHash(); + } + hash = Hash(pszNamespace, pszName, pName->GetBucket().IsNull() ? 0 : pName->GetBucket().GetClassHashBasedEntryValue()->GetHash()); + break; + } + + EEClassHashTable::LookupContext lookupContext; + pBucket = pTable->BaseFindFirstEntryByHash(hash, &lookupContext); + LPCUTF8 key[] = {pszNamespace, pszName}; + while (pBucket != NULL) + { + if (pTable->CompareKeys(pBucket, key)) + { + if ((pBucket->GetEncloser() != NULL) == isNested) + { + if (isNested) + { + bool hasNameBucket = !pName->GetBucket().IsNull(); +#ifdef _DEBUG + bool expectedMatchCheck = !hasNameBucket || (pBucket->GetEncloser() == pName->GetBucket().GetClassHashBasedEntryValue()); + bool expectedNotMatchCheck = !hasNameBucket || (pBucket->GetEncloser() != pName->GetBucket().GetClassHashBasedEntryValue()); +#endif +#ifndef _DEBUG + // In non-debug builds, we can simply check the encloser in the name first. If it matches then we've found the right + // result, and if it doesn't match it also indicates that it shouldn't match. We only do this in non-debug builds + // as we want to validate via asserts that this code is correct. + if (!pName->GetBucket().IsNull()) + { + if (pBucket->GetEncloser() == pName->GetBucket().GetClassHashBasedEntryValue()) + { + // We found our result + break; + } + } else +#endif // !_DEBUG + if (TypeFromToken(typeToken) == mdtTypeDef) + { + if (CompareNestedEntryWithTypeDef(pNameModule->GetMDImport(), + mdEncloser, + this, + pBucket->GetEncloser())) + { + _ASSERTE(expectedMatchCheck); + // We found our result + break; + } + _ASSERTE(expectedNotMatchCheck); + } + else if (TypeFromToken(typeToken) == mdtTypeRef) + { + if (CompareNestedEntryWithTypeRef(pNameModule->GetMDImport(), + mdEncloser, + this, + pBucket->GetEncloser())) + { + _ASSERTE(expectedMatchCheck); + // We found our result + break; + } + _ASSERTE(expectedNotMatchCheck); + } + else if (TypeFromToken(typeToken) == mdtExportedType) + { + if (CompareNestedEntryWithExportedType(pNameModule->GetMDImport(), + mdEncloser, + this, + pBucket->GetEncloser())) + { + _ASSERTE(expectedMatchCheck); + // We found our result + break; + } + _ASSERTE(expectedNotMatchCheck); + } + else + { + // String based lookup always set the encloser bucket on the name. We will only + // hit this particular block in debug builds + if (pBucket->GetEncloser() == pName->GetBucket().GetClassHashBasedEntryValue()) + { + // We found our result + break; + } + } + } + else + { + // We found our result + break; + } + } + } + pBucket = pTable->BaseFindNextEntryByHash(&lookupContext); + } + return pBucket; +} diff --git a/src/coreclr/vm/classhash.h b/src/coreclr/vm/classhash.h index 4fd943165b4787..8cea7bd4eea36e 100644 --- a/src/coreclr/vm/classhash.h +++ b/src/coreclr/vm/classhash.h @@ -39,6 +39,9 @@ typedef struct EEClassHashEntry PTR_VOID GetData(); void SetData(PTR_VOID data) DAC_EMPTY(); + DWORD GetHash() { return m_hash; } // Get the case sensitive hash value for this hash entry + void SetHash(DWORD hash) { m_hash = hash; } + private: PTR_VOID m_Data; // Either the token (if EECLASSHASH_TYPEHANDLE_DISCR), or the type handle encoded // as a relative pointer @@ -48,6 +51,7 @@ typedef struct EEClassHashEntry // reference to the enclosing type // (which must be in this same // hash). + DWORD m_hash; // Hash of this entry. Used for computing nested type hashes. } EEClassHashEntry_t; // The hash type itself. All common logic is provided by the DacEnumerableHashTable templated base class. See @@ -61,27 +65,20 @@ class EEClassHashTable : public DacEnumerableHashTable::LookupContext LookupContext; - static EEClassHashTable *Create(Module *pModule, DWORD dwNumBuckets, BOOL bCaseInsensitive, AllocMemTracker *pamTracker); + static EEClassHashTable *Create(Module *pModule, DWORD dwNumBuckets, PTR_EEClassHashTable caseSensitiveTable, AllocMemTracker *pamTracker); //NOTICE: look at InsertValue() in ClassLoader, that may be the function you want to use. Use this only // when you are sure you want to insert the value in 'this' table. This function does not deal // with case (as often the class loader has to) - EEClassHashEntry_t *InsertValue(LPCUTF8 pszNamespace, LPCUTF8 pszClassName, PTR_VOID Data, EEClassHashEntry_t *pEncloser, AllocMemTracker *pamTracker); - EEClassHashEntry_t *InsertValueIfNotFound(LPCUTF8 pszNamespace, LPCUTF8 pszClassName, PTR_VOID *pData, EEClassHashEntry_t *pEncloser, BOOL IsNested, BOOL *pbFound, AllocMemTracker *pamTracker); EEClassHashEntry_t *InsertValueUsingPreallocatedEntry(EEClassHashEntry_t *pStorageForNewEntry, LPCUTF8 pszNamespace, LPCUTF8 pszClassName, PTR_VOID Data, EEClassHashEntry_t *pEncloser); - EEClassHashEntry_t *GetValue(LPCUTF8 pszNamespace, LPCUTF8 pszClassName, PTR_VOID *pData, BOOL IsNested, LookupContext *pContext); - EEClassHashEntry_t *GetValue(LPCUTF8 pszFullyQualifiedName, PTR_VOID *pData, BOOL IsNested, LookupContext *pContext); - EEClassHashEntry_t *GetValue(const NameHandle* pName, PTR_VOID *pData, BOOL IsNested, LookupContext *pContext); EEClassHashEntry_t *AllocNewEntry(AllocMemTracker *pamTracker); EEClassHashTable *MakeCaseInsensitiveTable(Module *pModule, AllocMemTracker *pamTracker); - EEClassHashEntry_t *FindItem(LPCUTF8 pszNamespace, LPCUTF8 pszClassName, BOOL IsNested, LookupContext *pContext); - EEClassHashEntry_t *FindNextNestedClass(const NameHandle* pName, PTR_VOID *pData, LookupContext *pContext); - EEClassHashEntry_t *FindNextNestedClass(LPCUTF8 pszNamespace, LPCUTF8 pszClassName, PTR_VOID *pData, LookupContext *pContext); - EEClassHashEntry_t *FindNextNestedClass(LPCUTF8 pszFullyQualifiedName, PTR_VOID *pData, LookupContext *pContext); + + EEClassHashEntry_t * FindByNameHandle(const NameHandle* pName); BOOL CompareKeys(PTR_EEClassHashEntry pEntry, LPCUTF8 * pKey2); - static DWORD Hash(LPCUTF8 pszNamespace, LPCUTF8 pszClassName); + static DWORD Hash(LPCUTF8 pszNamespace, LPCUTF8 pszClassName, DWORD enclosingHash); class ConstructKeyCallback { @@ -100,12 +97,18 @@ class EEClassHashTable : public DacEnumerableHashTable(pModule, pHeap, cInitialBuckets) {} + DacEnumerableHashTable(pModule, pHeap, cInitialBuckets), + m_pCaseSensitiveTable(NULL) {} #endif VOID ConstructKeyFromData(PTR_EEClassHashEntry pEntry, ConstructKeyCallback * pCallback); - BOOL m_bCaseInsensitive; // Default is true FALSE unless we call MakeCaseInsensitiveTable + bool IsCaseInsensitiveTable() { return m_pCaseSensitiveTable != this; } + + static BOOL IsNested(ModuleBase *pModule, mdToken token, mdToken *mdEncloser); + static BOOL IsNested(const NameHandle* pName, mdToken *mdEncloser); + + PTR_EEClassHashTable m_pCaseSensitiveTable; // This will be a recursive pointer for the case sensitive table }; #endif // !__CLASS_HASH_INCLUDED diff --git a/src/coreclr/vm/classhash.inl b/src/coreclr/vm/classhash.inl index 25caf51d579fbe..554c0c2005c463 100644 --- a/src/coreclr/vm/classhash.inl +++ b/src/coreclr/vm/classhash.inl @@ -38,7 +38,7 @@ inline PTR_VOID EEClassHashTable::CompressClassDef(mdToken cl) } } -inline DWORD EEClassHashTable::Hash(LPCUTF8 pszNamespace, LPCUTF8 pszClassName) +inline DWORD EEClassHashTable::Hash(LPCUTF8 pszNamespace, LPCUTF8 pszClassName, DWORD hashEncloser) { CONTRACTL { @@ -60,6 +60,11 @@ inline DWORD EEClassHashTable::Hash(LPCUTF8 pszNamespace, LPCUTF8 pszClassName) while ((dwChar = *pszClassName++) != 0) dwHash = ((dwHash << 5) + dwHash) ^ dwChar; + if (hashEncloser != 0) + { + dwHash = ((dwHash << 5) + dwHash) ^ hashEncloser; + } + return dwHash; } diff --git a/src/coreclr/vm/clsload.cpp b/src/coreclr/vm/clsload.cpp index 06775e004629d6..8edf41b5c1969b 100644 --- a/src/coreclr/vm/clsload.cpp +++ b/src/coreclr/vm/clsload.cpp @@ -436,213 +436,6 @@ EEClassHashEntry_t* ClassLoader::InsertValue(EEClassHashTable *pClassHash, EECla #endif // #ifndef DACCESS_COMPILE -BOOL ClassLoader::CompareNestedEntryWithExportedType(IMDInternalImport * pImport, - mdExportedType mdCurrent, - EEClassHashTable * pClassHash, - PTR_EEClassHashEntry pEntry) -{ - CONTRACTL - { - INSTANCE_CHECK; - NOTHROW; - GC_NOTRIGGER; - MODE_ANY; - FORBID_FAULT; - SUPPORTS_DAC; - } - CONTRACTL_END; - - LPCUTF8 Key[2]; - - do - { - if (FAILED(pImport->GetExportedTypeProps( - mdCurrent, - &Key[0], - &Key[1], - &mdCurrent, - NULL, //binding (type def) - NULL))) //flags - { - return FALSE; - } - - if (pClassHash->CompareKeys(pEntry, Key)) - { - // Reached top level class for mdCurrent - return whether - // or not pEntry is a top level class - // (pEntry is a top level class if its pEncloser is NULL) - if ((TypeFromToken(mdCurrent) != mdtExportedType) || - (mdCurrent == mdExportedTypeNil)) - { - return pEntry->GetEncloser() == NULL; - } - } - else // Keys don't match - wrong entry - { - return FALSE; - } - } - while ((pEntry = pEntry->GetEncloser()) != NULL); - - // Reached the top level class for pEntry, but mdCurrent is nested - return FALSE; -} - - -BOOL ClassLoader::CompareNestedEntryWithTypeDef(IMDInternalImport * pImport, - mdTypeDef mdCurrent, - EEClassHashTable * pClassHash, - PTR_EEClassHashEntry pEntry) -{ - CONTRACTL - { - INSTANCE_CHECK; - NOTHROW; - GC_NOTRIGGER; - MODE_ANY; - FORBID_FAULT; - SUPPORTS_DAC; - } - CONTRACTL_END; - - LPCUTF8 Key[2]; - - do { - if (FAILED(pImport->GetNameOfTypeDef(mdCurrent, &Key[1], &Key[0]))) - { - return FALSE; - } - - if (pClassHash->CompareKeys(pEntry, Key)) { - // Reached top level class for mdCurrent - return whether - // or not pEntry is a top level class - // (pEntry is a top level class if its pEncloser is NULL) - if (FAILED(pImport->GetNestedClassProps(mdCurrent, &mdCurrent))) - return pEntry->GetEncloser() == NULL; - } - else // Keys don't match - wrong entry - return FALSE; - } - while ((pEntry = pEntry->GetEncloser()) != NULL); - - // Reached the top level class for pEntry, but mdCurrent is nested - return FALSE; -} - - -BOOL ClassLoader::CompareNestedEntryWithTypeRef(IMDInternalImport * pImport, - mdTypeRef mdCurrent, - EEClassHashTable * pClassHash, - PTR_EEClassHashEntry pEntry) -{ - CONTRACTL - { - INSTANCE_CHECK; - NOTHROW; - GC_NOTRIGGER; - MODE_ANY; - FORBID_FAULT; - SUPPORTS_DAC; - } - CONTRACTL_END; - - LPCUTF8 Key[2]; - - do { - if (FAILED(pImport->GetNameOfTypeRef(mdCurrent, &Key[0], &Key[1]))) - { - return FALSE; - } - - if (pClassHash->CompareKeys(pEntry, Key)) - { - if (FAILED(pImport->GetResolutionScopeOfTypeRef(mdCurrent, &mdCurrent))) - { - return FALSE; - } - // Reached top level class for mdCurrent - return whether - // or not pEntry is a top level class - // (pEntry is a top level class if its pEncloser is NULL) - if ((TypeFromToken(mdCurrent) != mdtTypeRef) || - (mdCurrent == mdTypeRefNil)) - return pEntry->GetEncloser() == NULL; - } - else // Keys don't match - wrong entry - return FALSE; - } - while ((pEntry = pEntry->GetEncloser())!=NULL); - - // Reached the top level class for pEntry, but mdCurrent is nested - return FALSE; -} - - -/*static*/ -BOOL ClassLoader::IsNested(ModuleBase *pModule, mdToken token, mdToken *mdEncloser) -{ - CONTRACTL - { - if (FORBIDGC_LOADER_USE_ENABLED()) NOTHROW; else THROWS; - if (FORBIDGC_LOADER_USE_ENABLED()) GC_NOTRIGGER; else GC_TRIGGERS; - if (FORBIDGC_LOADER_USE_ENABLED()) FORBID_FAULT; else { INJECT_FAULT(COMPlusThrowOM()); } - MODE_ANY; - SUPPORTS_DAC; - } - CONTRACTL_END; - - switch(TypeFromToken(token)) { - case mdtTypeDef: - return (SUCCEEDED(pModule->GetMDImport()->GetNestedClassProps(token, mdEncloser))); - - case mdtTypeRef: - IfFailThrow(pModule->GetMDImport()->GetResolutionScopeOfTypeRef(token, mdEncloser)); - return ((TypeFromToken(*mdEncloser) == mdtTypeRef) && - (*mdEncloser != mdTypeRefNil)); - - case mdtExportedType: - _ASSERTE(pModule->IsFullModule()); - IfFailThrow(((Module*)pModule)->GetAssembly()->GetMDImport()->GetExportedTypeProps( - token, - NULL, // namespace - NULL, // name - mdEncloser, - NULL, //binding (type def) - NULL)); //flags - return ((TypeFromToken(*mdEncloser) == mdtExportedType) && - (*mdEncloser != mdExportedTypeNil)); - - default: - ThrowHR(COR_E_BADIMAGEFORMAT, BFA_INVALID_TOKEN_TYPE); - } -} - -BOOL ClassLoader::IsNested(const NameHandle* pName, mdToken *mdEncloser) -{ - CONTRACTL - { - INSTANCE_CHECK; - if (FORBIDGC_LOADER_USE_ENABLED()) NOTHROW; else THROWS; - if (FORBIDGC_LOADER_USE_ENABLED()) GC_NOTRIGGER; else GC_TRIGGERS; - if (FORBIDGC_LOADER_USE_ENABLED()) FORBID_FAULT; else { INJECT_FAULT(COMPlusThrowOM()); } - MODE_ANY; - SUPPORTS_DAC; - } - CONTRACTL_END; - - if (pName->GetTypeModule()) { - if (TypeFromToken(pName->GetTypeToken()) == mdtBaseType) - { - if (!pName->GetBucket().IsNull()) - return TRUE; - return FALSE; - } - else - return IsNested(pName->GetTypeModule(), pName->GetTypeToken(), mdEncloser); - } - else - return FALSE; -} void ClassLoader::GetClassValue(NameHandleTable nhTable, const NameHandle *pName, @@ -749,54 +542,11 @@ void ClassLoader::GetClassValue(NameHandleTable nhTable, } _ASSERTE(pTable); - mdToken mdEncloser; - BOOL isNested = IsNested(pName, &mdEncloser); - - if (isNested) - { - ModuleBase *pNameModule = pName->GetTypeModule(); - PREFIX_ASSUME(pNameModule != NULL); - - EEClassHashTable::LookupContext sContext; - if ((pBucket = pTable->GetValue(pName, pData, TRUE, &sContext)) != NULL) - { - switch (TypeFromToken(pName->GetTypeToken())) - { - case mdtTypeDef: - while ((!CompareNestedEntryWithTypeDef(pNameModule->GetMDImport(), - mdEncloser, - pCurrentClsModule->GetAvailableClassHash(), - pBucket->GetEncloser())) && - (pBucket = pTable->FindNextNestedClass(pName, pData, &sContext)) != NULL); - break; - case mdtTypeRef: - while ((!CompareNestedEntryWithTypeRef(pNameModule->GetMDImport(), - mdEncloser, - pCurrentClsModule->GetAvailableClassHash(), - pBucket->GetEncloser())) && - (pBucket = pTable->FindNextNestedClass(pName, pData, &sContext)) != NULL); - break; - case mdtExportedType: - _ASSERTE(pNameModule->IsFullModule()); - while ((!CompareNestedEntryWithExportedType(((Module*)pNameModule)->GetAssembly()->GetMDImport(), - mdEncloser, - pCurrentClsModule->GetAvailableClassHash(), - pBucket->GetEncloser())) && - (pBucket = pTable->FindNextNestedClass(pName, pData, &sContext)) != NULL); - break; - default: - while ((pBucket->GetEncloser() != pName->GetBucket().GetClassHashBasedEntryValue()) && - (pBucket = pTable->FindNextNestedClass(pName, pData, &sContext)) != NULL); - } - } - } - else - { - pBucket = pTable->GetValue(pName, pData, FALSE, NULL); - } + pBucket = pTable->FindByNameHandle(pName); if (pBucket) // Return on the first success { + *pData = pBucket->GetData(); pFoundEntry->SetClassHashBasedEntryValue(pBucket); return; } @@ -822,20 +572,45 @@ VOID ClassLoader::PopulateAvailableClassHashTable(Module* pModule, } CONTRACTL_END; - mdTypeDef td; - HENUMInternal hTypeDefEnum; + SArray entries; IMDInternalImport * pImport = pModule->GetMDImport(); + { + mdTypeDef td; + HENUMInternalHolder hTypeDefEnum(pImport); - IfFailThrow(pImport->EnumTypeDefInit(&hTypeDefEnum)); + hTypeDefEnum.EnumTypeDefInit(); - // Now loop through all the classdefs adding the CVID and scope to the hash - while(pImport->EnumNext(&hTypeDefEnum, &td)) { + DWORD cEntries = hTypeDefEnum.EnumGetCount() + 1; // Add 1 since this enum doesn't report the class + EEClassHashEntry_t** pBuffer = entries.OpenRawBuffer(cEntries); + memset(pBuffer, 0, sizeof(EEClassHashEntry_t*) * cEntries); + entries.CloseRawBuffer(); - AddAvailableClassHaveLock(pModule, - td, - pamTracker); + // Now loop through all the classdefs adding the CVID and scope to the hash + while(pImport->EnumNext(&hTypeDefEnum, &td)) + { + AddAvailableClassHaveLock(pModule, + td, + &entries, + pamTracker); + } + } + + { + // Add exported types to the hashtable + HENUMInternalHolder phEnum(pImport); + phEnum.EnumInit(mdtExportedType, mdTokenNil); + + DWORD cEntries = phEnum.EnumGetCount(); + EEClassHashEntry_t** pBuffer = entries.OpenRawBuffer(cEntries); + memset(pBuffer, 0, sizeof(EEClassHashEntry_t*) * cEntries); + entries.CloseRawBuffer(); + + mdToken mdExportedType; + while (pImport->EnumNext(&phEnum, &mdExportedType)) + { + AddExportedTypeHaveLock(pModule, mdExportedType, &entries, pamTracker); + } } - pImport->EnumClose(&hTypeDefEnum); } @@ -881,21 +656,12 @@ void ClassLoader::LazyPopulateCaseSensitiveHashTables() // (either images compiled with an old version of crossgen, or for case-insensitive type lookups in R2R modules) _ASSERT(pModule->IsReadyToRun()); - EEClassHashTable * pNewClassHash = EEClassHashTable::Create(pModule, AVAILABLE_CLASSES_HASH_BUCKETS, FALSE /* bCaseInsensitive */, &amTracker); + EEClassHashTable * pNewClassHash = EEClassHashTable::Create(pModule, AVAILABLE_CLASSES_HASH_BUCKETS, NULL, &amTracker); pModule->SetAvailableClassHash(pNewClassHash); PopulateAvailableClassHashTable(pModule, &amTracker); } - // Add exported types to the hashtable - IMDInternalImport * pManifestImport = GetAssembly()->GetMDImport(); - HENUMInternalHolder phEnum(pManifestImport); - phEnum.EnumInit(mdtExportedType, mdTokenNil); - - mdToken mdExportedType; - while (pManifestImport->EnumNext(&phEnum, &mdExportedType)) - AddExportedTypeHaveLock(GetAssembly()->GetModule(), mdExportedType, &amTracker); - amTracker.SuppressRelease(); } @@ -1547,16 +1313,7 @@ ClassLoader::LoadTypeHandleThrowing( const HashedTypeEntry& bucket = pName->GetBucket(); // Reset pName's bucket entry - if (bucket.GetEntryType() == HashedTypeEntry::IsHashedClassEntry && bucket.GetClassHashBasedEntryValue()->GetEncloser()) - { - // We will be searching for the type name again, so set the nesting/context type to the - // encloser of just found type - pName->SetBucket(HashedTypeEntry().SetClassHashBasedEntryValue(bucket.GetClassHashBasedEntryValue()->GetEncloser())); - } - else - { - pName->SetBucket(HashedTypeEntry()); - } + pName->SetBucket(HashedTypeEntry()); // Update the class loader for the new module/token pair. pClsLdr = pFoundModule->GetClassLoader(); @@ -3707,6 +3464,7 @@ VOID ClassLoader::AddAvailableClassDontHaveLock(Module *pModule, AddAvailableClassHaveLock( pModule, classdef, + NULL, pamTracker); } @@ -3722,6 +3480,7 @@ VOID ClassLoader::AddAvailableClassDontHaveLock(Module *pModule, VOID ClassLoader::AddAvailableClassHaveLock( Module * pModule, mdTypeDef classdef, + SArray* classEntries, AllocMemTracker * pamTracker) // Optimization for faster prefix comparison implementation { CONTRACTL @@ -3736,10 +3495,10 @@ VOID ClassLoader::AddAvailableClassHaveLock( EEClassHashTable *pClassHash = pModule->GetAvailableClassHash(); EEClassHashTable *pClassCaseInsHash = pModule->GetAvailableClassCaseInsHash(); + EEClassHashEntry_t * insertedEntry = NULL; LPCUTF8 pszName; LPCUTF8 pszNameSpace; - HashDatum ThrowawayData; IMDInternalImport *pMDImport = pModule->GetMDImport(); if (FAILED(pMDImport->GetNameOfTypeDef(classdef, &pszName, &pszNameSpace))) { @@ -3747,101 +3506,45 @@ VOID ClassLoader::AddAvailableClassHaveLock( pModule->GetAssembly()->ThrowBadImageException(pszNameSpace, pszName, BFA_INVALID_TOKEN); } - EEClassHashEntry_t *pBucket; mdTypeDef enclosing; + EEClassHashEntry_t *pEncloser = NULL; if (SUCCEEDED(pMDImport->GetNestedClassProps(classdef, &enclosing))) { // nested type - LPCUTF8 pszEnclosingName; - LPCUTF8 pszEnclosingNameSpace; - mdTypeDef enclEnclosing; - - // Find this type's encloser's entry in the available table. - // We'll save a pointer to it in the new hash entry for this type. - BOOL fNestedEncl = SUCCEEDED(pMDImport->GetNestedClassProps(enclosing, &enclEnclosing)); - - EEClassHashTable::LookupContext sContext; - if (FAILED(pMDImport->GetNameOfTypeDef(enclosing, &pszEnclosingName, &pszEnclosingNameSpace))) + COUNT_T classEntryIndex = RidFromToken(enclosing) - 1; + _ASSERTE(RidFromToken(enclosing) < RidFromToken(classdef)); + if (classEntries != NULL && classEntries->GetCount() > classEntryIndex) { - pszName = pszNameSpace = "Invalid TypeDef token"; - pModule->GetAssembly()->ThrowBadImageException(pszNameSpace, pszName, BFA_INVALID_TOKEN); + pEncloser = (*classEntries)[classEntryIndex]; } - if ((pBucket = pClassHash->GetValue(pszEnclosingNameSpace, - pszEnclosingName, - &ThrowawayData, - fNestedEncl, - &sContext)) != NULL) { - if (fNestedEncl) { - // Find entry for enclosing class - NOTE, this assumes that the - // enclosing class's TypeDef or ExportedType was inserted previously, - // which assumes that, when enuming TD's, we get the enclosing class first - while ((!CompareNestedEntryWithTypeDef(pMDImport, - enclEnclosing, - pClassHash, - pBucket->GetEncloser())) && - (pBucket = pClassHash->FindNextNestedClass(pszEnclosingNameSpace, - pszEnclosingName, - &ThrowawayData, - &sContext)) != NULL); + else + { + LPCUTF8 pszEnclosingName; + LPCUTF8 pszEnclosingNameSpace; + if (FAILED(pMDImport->GetNameOfTypeDef(enclosing, &pszEnclosingName, &pszEnclosingNameSpace))) + { + pszName = pszNameSpace = "Invalid TypeDef token"; + pModule->GetAssembly()->ThrowBadImageException(pszNameSpace, pszName, BFA_INVALID_TOKEN); } + NameHandle nameHandleEncloser(pModule, classdef); + nameHandleEncloser.SetName(pszEnclosingNameSpace, pszEnclosingName); - if (!pBucket) // Enclosing type not found in hash table - pModule->GetAssembly()->ThrowBadImageException(pszNameSpace, pszName, BFA_ENCLOSING_TYPE_NOT_FOUND); - - // In this hash table, if the lower bit is set, it means a Module, otherwise it means EEClass* - ThrowawayData = EEClassHashTable::CompressClassDef(classdef); - InsertValue(pClassHash, pClassCaseInsHash, pszNameSpace, pszName, ThrowawayData, pBucket, pamTracker); - } - } - else { - // Don't add duplicate top-level classes. Top-level classes are - // added to the beginning of the bucket, while nested classes are - // added to the end. So, a duplicate top-level class could hide - // the previous type's EEClass* entry in the hash table. - EEClassHashEntry_t *pCaseInsEntry = NULL; - LPUTF8 pszLowerCaseNS = NULL; - LPUTF8 pszLowerCaseName = NULL; - - if (pClassCaseInsHash) { - CreateCanonicallyCasedKey(pszNameSpace, pszName, &pszLowerCaseNS, &pszLowerCaseName); - pCaseInsEntry = pClassCaseInsHash->AllocNewEntry(pamTracker); + pEncloser = pClassHash->FindByNameHandle(&nameHandleEncloser); } - EEClassHashEntry_t *pEntry = pClassHash->FindItem(pszNameSpace, pszName, FALSE, NULL); - if (pEntry) { - HashDatum Data = pEntry->GetData(); - - if (((size_t)Data & EECLASSHASH_TYPEHANDLE_DISCR) && - ((size_t)Data & EECLASSHASH_MDEXPORT_DISCR)) { - - // it's an ExportedType - check the 'already seen' bit and if on, report a class loading exception - // otherwise, set it - if ((size_t)Data & EECLASSHASH_ALREADYSEEN) - pModule->GetAssembly()->ThrowBadImageException(pszNameSpace, pszName, BFA_MULT_TYPE_SAME_NAME); - else { - Data = (HashDatum)((size_t)Data | EECLASSHASH_ALREADYSEEN); - pEntry->SetData(Data); - } - } - else { - // We want to throw an exception for a duplicate typedef. - // However, this used to be allowed in 1.0/1.1, and some third-party DLLs have - // been obfuscated so that they have duplicate private typedefs. - // We must allow this for old assemblies for app compat reasons - pModule->GetAssembly()->ThrowBadImageException(pszNameSpace, pszName, BFA_MULT_TYPE_SAME_NAME); - } + if (pEncloser == NULL) + { + pModule->GetAssembly()->ThrowBadImageException(pszNameSpace, pszName, BFA_ENCLOSING_TYPE_NOT_FOUND); } - else { - pEntry = pClassHash->AllocNewEntry(pamTracker); - - CANNOTTHROWCOMPLUSEXCEPTION(); - FAULT_FORBID(); - - pClassHash->InsertValueUsingPreallocatedEntry(pEntry, pszNameSpace, pszName, EEClassHashTable::CompressClassDef(classdef), NULL); + } + insertedEntry = InsertValue(pClassHash, pClassCaseInsHash, pszNameSpace, pszName, EEClassHashTable::CompressClassDef(classdef), pEncloser, pamTracker); - if (pClassCaseInsHash) - pClassCaseInsHash->InsertValueUsingPreallocatedEntry(pCaseInsEntry, pszLowerCaseNS, pszLowerCaseName, pEntry, pEntry->GetEncloser()); - } + _ASSERTE(insertedEntry != NULL); + if (classEntries != NULL) + { + COUNT_T classEntryIndex = RidFromToken(classdef) - 1; + _ASSERTE(classEntryIndex < classEntries->GetCount()); + (*classEntries)[classEntryIndex] = insertedEntry; } } @@ -3864,11 +3567,13 @@ VOID ClassLoader::AddExportedTypeDontHaveLock(Module *pManifestModule, AddExportedTypeHaveLock( pManifestModule, cl, + NULL, pamTracker); } VOID ClassLoader::AddExportedTypeHaveLock(Module *pManifestModule, mdExportedType cl, + SArray* exportedEntries, AllocMemTracker *pamTracker) { CONTRACTL @@ -3881,6 +3586,10 @@ VOID ClassLoader::AddExportedTypeHaveLock(Module *pManifestModule, } CONTRACTL_END + EEClassHashTable *pClassHash = pManifestModule->GetAvailableClassHash(); + EEClassHashTable *pClassCaseInsHash = pManifestModule->GetAvailableClassCaseInsHash(); + EEClassHashEntry_t * insertedEntry = NULL; + EEClassHashEntry_t *pEncloser = NULL; mdToken mdImpl; LPCSTR pszName; @@ -3897,92 +3606,55 @@ VOID ClassLoader::AddExportedTypeHaveLock(Module *pManifestModule, pManifestModule->GetAssembly()->ThrowBadImageException(pszNameSpace, pszName, BFA_INVALID_TOKEN); } - HashDatum ThrowawayData; - if (TypeFromToken(mdImpl) == mdtExportedType) { - // nested class - LPCUTF8 pszEnclosingNameSpace; - LPCUTF8 pszEnclosingName; - mdToken nextImpl; - if (FAILED(pAsmImport->GetExportedTypeProps( - mdImpl, - &pszEnclosingNameSpace, - &pszEnclosingName, - &nextImpl, - NULL, // type def - NULL))) // flags + COUNT_T exportedEntryIndex = RidFromToken(mdImpl) - 1; + _ASSERTE(RidFromToken(mdImpl) < RidFromToken(cl)); + if (exportedEntries != NULL && exportedEntries->GetCount() > exportedEntryIndex) { - pManifestModule->GetAssembly()->ThrowBadImageException(pszNameSpace, pszName, BFA_INVALID_TOKEN); + pEncloser = (*exportedEntries)[exportedEntryIndex]; } + else + { + // nested class + LPCUTF8 pszEnclosingNameSpace; + LPCUTF8 pszEnclosingName; + mdToken nextImpl; + if (FAILED(pAsmImport->GetExportedTypeProps( + mdImpl, + &pszEnclosingNameSpace, + &pszEnclosingName, + &nextImpl, + NULL, // type def + NULL))) // flags + { + pManifestModule->GetAssembly()->ThrowBadImageException(pszNameSpace, pszName, BFA_INVALID_TOKEN); + } - // Find entry for enclosing class - NOTE, this assumes that the - // enclosing class's ExportedType was inserted previously, which assumes that, - // when enuming ExportedTypes, we get the enclosing class first - EEClassHashEntry_t *pBucket; - EEClassHashTable::LookupContext sContext; - if ((pBucket = pManifestModule->GetAvailableClassHash()->GetValue(pszEnclosingNameSpace, - pszEnclosingName, - &ThrowawayData, - TypeFromToken(nextImpl) == mdtExportedType, - &sContext)) != NULL) { - do { - // check to see if this is the correct class - if (EEClassHashTable::UncompressModuleAndClassDef(ThrowawayData) == mdImpl) { - ThrowawayData = EEClassHashTable::CompressClassDef(cl); - - // we explicitly don't check for the case insensitive hash table because we know it can't have been created yet - pManifestModule->GetAvailableClassHash()->InsertValue(pszNameSpace, pszName, ThrowawayData, pBucket, pamTracker); - } - pBucket = pManifestModule->GetAvailableClassHash()->FindNextNestedClass(pszEnclosingNameSpace, pszEnclosingName, &ThrowawayData, &sContext); - } while (pBucket); + NameHandle nameHandleEncloser(pManifestModule, mdImpl); + nameHandleEncloser.SetName(pszEnclosingNameSpace, pszEnclosingName); + pEncloser = pClassHash->FindByNameHandle(&nameHandleEncloser); } - // If the encloser is not in the hash table, this nested class - // was defined in the manifest module, so it doesn't need to be added - return; + if (pEncloser == NULL) + { + // This can happen if the enclosing type was defined in the manifest module, and was instead added by TypeDef instead. + return; + } } else { // Defined in the manifest module - add to the hash table by TypeDef instead if (mdImpl == mdFileNil) return; + } - // Don't add duplicate top-level classes - // In this hash table, if the lower bit is set, it means a Module, otherwise it means EEClass* - ThrowawayData = EEClassHashTable::CompressClassDef(cl); - // ThrowawayData is an IN OUT param. Going in its the pointer to the new value if the entry needs - // to be inserted. The OUT param points to the value stored in the hash table. - BOOL bFound; - pManifestModule->GetAvailableClassHash()->InsertValueIfNotFound(pszNameSpace, pszName, &ThrowawayData, NULL, FALSE, &bFound, pamTracker); - if (bFound) { - - // Check for duplicate ExportedTypes - // Let it slide if it's pointing to the same type - mdToken foundTypeImpl; - if ((size_t)ThrowawayData & EECLASSHASH_MDEXPORT_DISCR) - { - mdExportedType foundExportedType = EEClassHashTable::UncompressModuleAndClassDef(ThrowawayData); - if (FAILED(pAsmImport->GetExportedTypeProps( - foundExportedType, - NULL, // namespace - NULL, // name - &foundTypeImpl, - NULL, // TypeDef - NULL))) // flags - { - pManifestModule->GetAssembly()->ThrowBadImageException(pszNameSpace, pszName, BFA_INVALID_TOKEN); - } - } - else - { - foundTypeImpl = mdFileNil; - } - - if (mdImpl != foundTypeImpl) - { - pManifestModule->GetAssembly()->ThrowBadImageException(pszNameSpace, pszName, BFA_MULT_TYPE_SAME_NAME); - } - } + insertedEntry = InsertValue(pClassHash, pClassCaseInsHash, pszNameSpace, pszName, EEClassHashTable::CompressClassDef(cl), pEncloser, pamTracker); + _ASSERTE(insertedEntry != NULL); + if (exportedEntries != NULL) + { + COUNT_T exportedEntryIndex = RidFromToken(cl) - 1; + _ASSERTE(exportedEntryIndex < exportedEntries->GetCount()); + (*exportedEntries)[exportedEntryIndex] = insertedEntry; } } diff --git a/src/coreclr/vm/clsload.hpp b/src/coreclr/vm/clsload.hpp index fd2aabe3801ed1..df80bd9f69b8e2 100644 --- a/src/coreclr/vm/clsload.hpp +++ b/src/coreclr/vm/clsload.hpp @@ -59,7 +59,7 @@ class HashedTypeEntry IsNullEntry, // Uninitialized HashedTypeEntry IsHashedTokenEntry, // Entry is a token value in a R2R hashtable in from the R2R module IsHashedClassEntry // Entry is a EEClassHashEntry_t from the hashtable constructed at - // module load time (or from the hashtable loaded from the native image) + // module load time } EntryType; typedef struct @@ -138,18 +138,6 @@ class NameHandle memset((void*) this, NULL, sizeof(*this)); } - NameHandle(LPCUTF8 name) : - m_nameSpace(NULL), - m_name(name), - m_pTypeScope(PTR_NULL), - m_mdType(mdTokenNil), - m_mdTokenNotToLoad(tdNoTypes), - m_WhichTable(nhCaseSensitive), - m_Bucket() - { - LIMITED_METHOD_CONTRACT; - } - NameHandle(LPCUTF8 nameSpace, LPCUTF8 name) : m_nameSpace(nameSpace), m_name(name), @@ -161,6 +149,8 @@ class NameHandle { LIMITED_METHOD_CONTRACT; SUPPORTS_DAC; + _ASSERTE(nameSpace != NULL); + _ASSERTE(name != NULL); } NameHandle(ModuleBase* pModule, mdToken token); @@ -180,17 +170,13 @@ class NameHandle m_Bucket = p.m_Bucket; } - void SetName(LPCUTF8 pName) - { - LIMITED_METHOD_CONTRACT; - m_name = pName; - } - void SetName(LPCUTF8 pNameSpace, LPCUTF8 pName) { LIMITED_METHOD_CONTRACT; SUPPORTS_DAC_HOST_ONLY; + _ASSERTE(pNameSpace != NULL); + _ASSERTE(pName != NULL); m_nameSpace = pNameSpace; m_name = pName; } @@ -761,6 +747,7 @@ class ClassLoader VOID AddAvailableClassHaveLock(Module * pModule, mdTypeDef classdef, + SArray* classEntries, AllocMemTracker * pamTracker); VOID AddExportedTypeDontHaveLock(Module *pManifestModule, @@ -769,6 +756,7 @@ class ClassLoader VOID AddExportedTypeHaveLock(Module *pManifestModule, mdExportedType cl, + SArray* exportedEntries, AllocMemTracker *pamTracker); public: diff --git a/src/coreclr/vm/readytoruninfo.cpp b/src/coreclr/vm/readytoruninfo.cpp index e75373db8855aa..40ffa8e7d0eaca 100644 --- a/src/coreclr/vm/readytoruninfo.cpp +++ b/src/coreclr/vm/readytoruninfo.cpp @@ -94,42 +94,21 @@ BOOL ReadyToRunInfo::TryLookupTypeTokenFromName(const NameHandle *pName, mdToken LPCUTF8 pszName = NULL; LPCUTF8 pszNameSpace = NULL; - // Reserve stack space for parsing out the namespace in a name-based lookup - // at this scope so the stack space is in scope for all usages in this method. - CQuickBytes namespaceBuffer; // // Compute the hashcode of the type (hashcode based on type name and namespace name) // int dwHashCode = 0; + _ASSERTE(pName->GetName() != NULL); + _ASSERTE(pName->GetNameSpace() != NULL); + if (pName->GetTypeToken() == mdtBaseType || pName->GetTypeModule() == NULL) { // Name-based lookups (ex: Type.GetType()). pszName = pName->GetName(); - pszNameSpace = ""; - if (pName->GetNameSpace() != NULL) - { - pszNameSpace = pName->GetNameSpace(); - } - else - { - LPCUTF8 p; - - if ((p = ns::FindSep(pszName)) != NULL) - { - SIZE_T d = p - pszName; - - FAULT_NOT_FATAL(); - pszNameSpace = namespaceBuffer.SetStringNoThrow(pszName, d); - - if (pszNameSpace == NULL) - return FALSE; - - pszName = (p + 1); - } - } + pszNameSpace = pName->GetNameSpace(); _ASSERT(pszNameSpace != NULL); dwHashCode ^= ComputeNameHashCode(pszNameSpace, pszName); From e1790ea7c6792b0f4009444d2c819555b4657886 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 15 Nov 2023 17:15:52 -0800 Subject: [PATCH 02/13] Improve instance method hash table --- src/coreclr/vm/instmethhash.cpp | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/src/coreclr/vm/instmethhash.cpp b/src/coreclr/vm/instmethhash.cpp index 2cb27b86fa9df4..cdb8b8617dec88 100644 --- a/src/coreclr/vm/instmethhash.cpp +++ b/src/coreclr/vm/instmethhash.cpp @@ -96,30 +96,19 @@ static DWORD Hash(TypeHandle declaringType, mdMethodDef token, Instantiation ins DWORD dwHash = 0x87654321; #define INST_HASH_ADD(_value) dwHash = ((dwHash << 5) + dwHash) ^ (_value) +#ifdef TARGET_64BIT +#define INST_HASH_ADDPOINTER(_value) INST_HASH_ADD((uint32_t)(uintptr_t)_value); INST_HASH_ADD((uint32_t)(((uintptr_t)_value) >> 32)) +#else +#define INST_HASH_ADDPOINTER(_value) INST_HASH_ADD((uint32_t)(uintptr_t)_value); +#endif - INST_HASH_ADD(declaringType.GetCl()); + INST_HASH_ADDPOINTER(declaringType.AsPtr()); INST_HASH_ADD(token); for (DWORD i = 0; i < inst.GetNumArgs(); i++) { TypeHandle thArg = inst[i]; - - if (thArg.GetMethodTable()) - { - INST_HASH_ADD(thArg.GetCl()); - - Instantiation sArgInst = thArg.GetInstantiation(); - for (DWORD j = 0; j < sArgInst.GetNumArgs(); j++) - { - TypeHandle thSubArg = sArgInst[j]; - if (thSubArg.GetMethodTable()) - INST_HASH_ADD(thSubArg.GetCl()); - else - INST_HASH_ADD(thSubArg.GetSignatureCorElementType()); - } - } - else - INST_HASH_ADD(thArg.GetSignatureCorElementType()); + INST_HASH_ADDPOINTER(thArg.AsPtr()); } return dwHash; From 5b1b6f6d9fa1aa53a5aaadc4eb901294077a83c3 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 15 Nov 2023 18:10:06 -0800 Subject: [PATCH 03/13] Make GetData safe for lock free access, considering it is used in such a manner --- src/coreclr/vm/classhash.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/classhash.cpp b/src/coreclr/vm/classhash.cpp index ed8f5b896684a0..2d0ab7d2aef107 100644 --- a/src/coreclr/vm/classhash.cpp +++ b/src/coreclr/vm/classhash.cpp @@ -37,7 +37,7 @@ PTR_VOID EEClassHashEntry::GetData() } CONTRACTL_END; - return m_Data; + return VolatileLoadWithoutBarrier(&m_Data); } #ifndef DACCESS_COMPILE From f08041b7b69ee0b8b38939a171e121cf5324067e Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 15 Nov 2023 18:11:03 -0800 Subject: [PATCH 04/13] Remove use of FindTypeDef from its only common use pattern. This api is a linear scan of the entire typedef table, and we already have the right hash to make this cheap --- src/coreclr/vm/clsload.cpp | 92 ++++++++++++++++++++++++++- src/coreclr/vm/clsload.hpp | 1 + src/coreclr/vm/methodtablebuilder.cpp | 40 +----------- 3 files changed, 93 insertions(+), 40 deletions(-) diff --git a/src/coreclr/vm/clsload.cpp b/src/coreclr/vm/clsload.cpp index 8edf41b5c1969b..9695150830eebc 100644 --- a/src/coreclr/vm/clsload.cpp +++ b/src/coreclr/vm/clsload.cpp @@ -434,8 +434,98 @@ EEClassHashEntry_t* ClassLoader::InsertValue(EEClassHashTable *pClassHash, EECla } -#endif // #ifndef DACCESS_COMPILE +mdTypeDef ClassLoader::LookupTypeDefTokenThatMatchesTypeRef(mdTypeRef typeRef) +{ + CONTRACTL + { + INSTANCE_CHECK; + MODE_ANY; + THROWS; + GC_TRIGGERS; + SUPPORTS_DAC; + } + CONTRACTL_END + + _ASSERTE(TypeFromToken(typeRef) == mdtTypeRef); + mdToken mdFoundTypeToken; + + Module* pModule = GetAssembly()->GetModule(); + + NameHandle nhTypeRef(pModule, typeRef); + + LPCUTF8 pszNameSpace; + LPCUTF8 pszClassName; + if (FAILED(pModule->GetMDImport()->GetNameOfTypeRef(typeRef, &pszNameSpace, &pszClassName))) + { + ThrowHR(COR_E_BADIMAGEFORMAT); + } + + nhTypeRef.SetName(pszNameSpace, pszClassName); + +#ifdef FEATURE_READYTORUN + if (pModule->IsReadyToRun()) + { + if (pModule->GetReadyToRunInfo()->HasHashtableOfTypes() && pModule->GetAvailableClassHash() == NULL) + { + // For R2R modules, we only search the hashtable of token types stored in the module's image, and don't fallback + // to searching m_pAvailableClasses or m_pAvailableClassesCaseIns (in fact, we don't even allocate them for R2R modules). + // Also note that type lookups in R2R modules only support case sensitive lookups. + if (pModule->GetReadyToRunInfo()->TryLookupTypeTokenFromName(&nhTypeRef, &mdFoundTypeToken)) + { + if (TypeFromToken(mdFoundTypeToken) == mdtTypeDef) + { + return mdFoundTypeToken; + } + } + return mdTypeDefNil; + } + if (pModule->GetAvailableClassHash() == NULL) + { + // Old R2R image generated without the hashtable of types. + // We fallback to the slow path of creating the hashtable dynamically + // at execution time in that scenario. + // Take the lock. To make sure the table is not being built by another thread. + ClassLoader::AvailableClasses_LockHolder lh(this); + if (m_cUnhashedModules != 0) + { + // Note: This codepath is only valid for R2R scenarios + LazyPopulateCaseSensitiveHashTables(); + } + } + } +#endif + + EEClassHashEntry_t* entry = pModule->GetAvailableClassHash()->FindByNameHandle(&nhTypeRef); + if (entry != NULL) + { + HashDatum Data = entry->GetData(); + if ((dac_cast(Data) & EECLASSHASH_TYPEHANDLE_DISCR) == 0) + { + TypeHandle t = TypeHandle::FromPtr(Data); + if (!t.IsTypeDesc()) + { + MethodTable *pMT = t.AsMethodTable(); + if (pMT->GetModule() == pModule) + { + return pMT->GetCl(); + } + } + } + else + { + mdFoundTypeToken = pModule->GetAvailableClassHash()->UncompressModuleAndClassDef(Data); + if (TypeFromToken(mdFoundTypeToken) == mdtTypeDef) + { + return mdFoundTypeToken; + } + } + } + + return mdTypeDefNil; +} + +#endif // #ifndef DACCESS_COMPILE void ClassLoader::GetClassValue(NameHandleTable nhTable, const NameHandle *pName, diff --git a/src/coreclr/vm/clsload.hpp b/src/coreclr/vm/clsload.hpp index df80bd9f69b8e2..8a5305f92a1606 100644 --- a/src/coreclr/vm/clsload.hpp +++ b/src/coreclr/vm/clsload.hpp @@ -734,6 +734,7 @@ class ClassLoader TypeHandle LoadTypeHandleThrowIfFailed(NameHandle* pName, ClassLoadLevel level = CLASS_LOADED, Module* pLookInThisModuleOnly=NULL); + mdTypeDef LookupTypeDefTokenThatMatchesTypeRef(mdTypeRef typeRef); public: // Looks up class in the local module table, if it is there it succeeds, // Otherwise it fails, This is meant only for optimizations etc diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 44ddcf546064bb..0bf70b63889f71 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -4047,45 +4047,7 @@ VOID MethodTableBuilder::InitializeFieldDescs(FieldDesc *pFieldDescList, if (TypeFromToken(dwByValueClassToken) == mdtTypeRef) { // It's a typeref - check if it's a class that has a static field of itself - LPCUTF8 pszNameSpace; - LPCUTF8 pszClassName; - if (FAILED(pInternalImport->GetNameOfTypeRef(dwByValueClassToken, &pszNameSpace, &pszClassName))) - { - BuildMethodTableThrowException(IDS_CLASSLOAD_BADFORMAT); - } - - if (IsStrLongerThan((char *)pszClassName, MAX_CLASS_NAME) - || IsStrLongerThan((char *)pszNameSpace, MAX_CLASS_NAME) - || (strlen(pszClassName) + strlen(pszNameSpace) + 1 >= MAX_CLASS_NAME)) - { - BuildMethodTableThrowException(BFA_TYPEREG_NAME_TOO_LONG, mdMethodDefNil); - } - - mdToken tkRes; - if (FAILED(pInternalImport->GetResolutionScopeOfTypeRef(dwByValueClassToken, &tkRes))) - { - BuildMethodTableThrowException(BFA_BAD_TYPEREF_TOKEN, dwByValueClassToken); - } - - if (TypeFromToken(tkRes) == mdtTypeRef) - { - if (!pInternalImport->IsValidToken(tkRes)) - { - BuildMethodTableThrowException(BFA_BAD_TYPEREF_TOKEN, mdMethodDefNil); - } - } - else - { - tkRes = mdTokenNil; - } - - if (FAILED(pInternalImport->FindTypeDef(pszNameSpace, - pszClassName, - tkRes, - &dwByValueClassToken))) - { - dwByValueClassToken = mdTokenNil; - } + dwByValueClassToken = GetModule()->GetClassLoader()->LookupTypeDefTokenThatMatchesTypeRef(dwByValueClassToken); } // If field is static typeref BOOL selfref = IsSelfReferencingStaticValueTypeField(dwByValueClassToken, From a97426e638da4c920e9ec2ee6889da11e2b7fc3a Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 15 Nov 2023 18:39:23 -0800 Subject: [PATCH 05/13] Remove extra hash field we really didn't need on EEClassHashEntry - Also make the code more DAC correct (probably not completely correct, but this logic does not appear to actually be used within the DAC) --- src/coreclr/vm/classhash.cpp | 9 ++++----- src/coreclr/vm/classhash.h | 16 ++++++++++++---- src/coreclr/vm/clsload.cpp | 6 +++--- src/coreclr/vm/clsload.hpp | 6 +++--- src/coreclr/vm/dacenumerablehash.h | 2 ++ src/coreclr/vm/dacenumerablehash.inl | 10 ++++++++++ 6 files changed, 34 insertions(+), 15 deletions(-) diff --git a/src/coreclr/vm/classhash.cpp b/src/coreclr/vm/classhash.cpp index 2d0ab7d2aef107..58c67848eaf7b0 100644 --- a/src/coreclr/vm/classhash.cpp +++ b/src/coreclr/vm/classhash.cpp @@ -332,14 +332,13 @@ EEClassHashEntry_t *EEClassHashTable::InsertValueUsingPreallocatedEntry(EEClassH pNewEntry->SetData(Data); pNewEntry->SetEncloser(pEncloser); - pNewEntry->SetHash(Hash(pszNamespace, pszClassName, pEncloser != NULL ? pEncloser->GetHash() : 0)); #ifdef _DEBUG pNewEntry->DebugKey[0] = pszNamespace; pNewEntry->DebugKey[1] = pszClassName; #endif - BaseInsertEntry(pNewEntry->GetHash(), pNewEntry); + BaseInsertEntry(Hash(pszNamespace, pszClassName, pEncloser != NULL ? GetHash(pEncloser) : 0), pNewEntry); return pNewEntry; } @@ -736,7 +735,7 @@ BOOL EEClassHashTable::IsNested(const NameHandle* pName, mdToken *mdEncloser) return FALSE; } -EEClassHashEntry_t * EEClassHashTable::FindByNameHandle(const NameHandle* pName) +PTR_EEClassHashEntry EEClassHashTable::FindByNameHandle(const NameHandle* pName) { // TODO remove this pointless local EEClassHashTable *pTable = this; @@ -779,9 +778,9 @@ EEClassHashEntry_t * EEClassHashTable::FindByNameHandle(const NameHandle* pName) { // The enclosing hash is always based on the entry from the case sensitive table // A NameHandle bucket is always the entry in the CaseSensitive table - enclosingHash = pName->GetBucket().GetClassHashBasedEntryValue()->GetHash(); + enclosingHash = GetHash(pName->GetBucket().GetClassHashBasedEntryValue()); } - hash = Hash(pszNamespace, pszName, pName->GetBucket().IsNull() ? 0 : pName->GetBucket().GetClassHashBasedEntryValue()->GetHash()); + hash = Hash(pszNamespace, pszName, enclosingHash); break; } diff --git a/src/coreclr/vm/classhash.h b/src/coreclr/vm/classhash.h index 8cea7bd4eea36e..60c2650fff9ceb 100644 --- a/src/coreclr/vm/classhash.h +++ b/src/coreclr/vm/classhash.h @@ -39,8 +39,6 @@ typedef struct EEClassHashEntry PTR_VOID GetData(); void SetData(PTR_VOID data) DAC_EMPTY(); - DWORD GetHash() { return m_hash; } // Get the case sensitive hash value for this hash entry - void SetHash(DWORD hash) { m_hash = hash; } private: PTR_VOID m_Data; // Either the token (if EECLASSHASH_TYPEHANDLE_DISCR), or the type handle encoded @@ -51,7 +49,6 @@ typedef struct EEClassHashEntry // reference to the enclosing type // (which must be in this same // hash). - DWORD m_hash; // Hash of this entry. Used for computing nested type hashes. } EEClassHashEntry_t; // The hash type itself. All common logic is provided by the DacEnumerableHashTable templated base class. See @@ -74,7 +71,7 @@ class EEClassHashTable : public DacEnumerableHashTableGetTable() == nhCaseInsensitive) { _ASSERTE(Data); - pBucket = PTR_EEClassHashEntry(Data); + pBucket = dac_cast(dac_cast((Data))); Data = pBucket->GetData(); } diff --git a/src/coreclr/vm/clsload.hpp b/src/coreclr/vm/clsload.hpp index 8a5305f92a1606..01094883184d30 100644 --- a/src/coreclr/vm/clsload.hpp +++ b/src/coreclr/vm/clsload.hpp @@ -83,15 +83,15 @@ class HashedTypeEntry EntryType GetEntryType() const { return m_EntryType; } bool IsNull() const { return m_EntryType == EntryType::IsNullEntry; } - const HashedTypeEntry& SetClassHashBasedEntryValue(EEClassHashEntry_t * pClassHashEntry) + const HashedTypeEntry& SetClassHashBasedEntryValue(PTR_EEClassHashEntry pClassHashEntry) { LIMITED_METHOD_CONTRACT; m_EntryType = EntryType::IsHashedClassEntry; - m_pClassHashEntry = dac_cast(pClassHashEntry); + m_pClassHashEntry = pClassHashEntry; return *this; } - EEClassHashEntry_t * GetClassHashBasedEntryValue() const + PTR_EEClassHashEntry GetClassHashBasedEntryValue() const { LIMITED_METHOD_CONTRACT; diff --git a/src/coreclr/vm/dacenumerablehash.h b/src/coreclr/vm/dacenumerablehash.h index 1da48cfdeba533..8f5108b8528321 100644 --- a/src/coreclr/vm/dacenumerablehash.h +++ b/src/coreclr/vm/dacenumerablehash.h @@ -273,6 +273,8 @@ class DacEnumerableHashTable DPTR(VALUE) BaseFindFirstEntryByHash(DacEnumerableHashValue iHash, LookupContext *pContext); DPTR(VALUE) BaseFindNextEntryByHash(LookupContext *pContext); + static DacEnumerableHashValue BaseValuePtrToHash(DPTR(VALUE) pValue); + PTR_Module GetModule() { return m_pModule; diff --git a/src/coreclr/vm/dacenumerablehash.inl b/src/coreclr/vm/dacenumerablehash.inl index 6f78c53955c839..a6083e26fda4ea 100644 --- a/src/coreclr/vm/dacenumerablehash.inl +++ b/src/coreclr/vm/dacenumerablehash.inl @@ -14,6 +14,7 @@ // doing this in a DAC-friendly fashion involves some DAC gymnastics. The following couple of macros factor // those complexities out. #define VALUE_FROM_VOLATILE_ENTRY(_ptr) dac_cast(PTR_TO_MEMBER_TADDR(VolatileEntry, (_ptr), m_sValue)) +#define VALUE_TO_VOLATILE_ENTRY(_ptr) dac_cast(dac_cast(_ptr)) #ifndef DACCESS_COMPILE @@ -591,3 +592,12 @@ DPTR(VALUE) DacEnumerableHashTable::BaseIterator::Next() return NULL; } + +template +/*static*/ DacEnumerableHashValue DacEnumerableHashTable::BaseValuePtrToHash(DPTR(VALUE) pValue) +{ +#ifndef DACCESS_COMPILE + _ASSERTE(offsetof(VolatileEntry, m_sValue) == 0); +#endif + return VALUE_TO_VOLATILE_ENTRY(pValue)->m_iHashValue; +} From ff60cb926481960839dfd36639cd2d70b4ac0cc8 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Thu, 16 Nov 2023 12:53:15 -0800 Subject: [PATCH 06/13] Fix a couple of missed details --- src/coreclr/vm/classhash.cpp | 4 +++- src/coreclr/vm/clsload.cpp | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/classhash.cpp b/src/coreclr/vm/classhash.cpp index 58c67848eaf7b0..e49c13bf46e3c9 100644 --- a/src/coreclr/vm/classhash.cpp +++ b/src/coreclr/vm/classhash.cpp @@ -755,17 +755,19 @@ PTR_EEClassHashEntry EEClassHashTable::FindByNameHandle(const NameHandle* pName) mdToken typeToken = pName->GetTypeToken(); ModuleBase *pNameModule = pName->GetTypeModule(); - PREFIX_ASSUME(pNameModule != NULL); switch (TypeFromToken(typeToken)) { case mdtTypeDef: + PREFIX_ASSUME(pNameModule != NULL); hash = ComputeHashFunctionWithTypeDef(pTable, m_pCaseSensitiveTable, pNameModule->GetMDImport(), typeToken, &failed); break; case mdtTypeRef: + PREFIX_ASSUME(pNameModule != NULL); hash = ComputeHashFunctionWithTypeRef(pTable, m_pCaseSensitiveTable, pNameModule->GetMDImport(), typeToken, &failed); break; case mdtExportedType: + PREFIX_ASSUME(pNameModule != NULL); hash = ComputeHashFunctionWithExportedType(pTable, m_pCaseSensitiveTable, pNameModule->GetMDImport(), typeToken, &failed); break; default: diff --git a/src/coreclr/vm/clsload.cpp b/src/coreclr/vm/clsload.cpp index b02dcd948bd219..d86723cd21824e 100644 --- a/src/coreclr/vm/clsload.cpp +++ b/src/coreclr/vm/clsload.cpp @@ -284,6 +284,26 @@ TypeHandle ClassLoader::LoadTypeByNameThrowing(Assembly *pAssembly, ClassLoadLevel level) { WRAPPER_NO_CONTRACT; + + CQuickBytes qbszNamespace; + + if (nameSpace == NULL) + { + LPCUTF8 szFullyQualifiedName = name; + nameSpace = ""; + + if ((name = ns::FindSep(szFullyQualifiedName)) != NULL) + { + SIZE_T d = name - szFullyQualifiedName; + nameSpace = qbszNamespace.SetString(szFullyQualifiedName, d); + name++; + } + else + { + name = szFullyQualifiedName; + } + } + NameHandle nameHandle(nameSpace, name); return LoadTypeByNameThrowing(pAssembly, &nameHandle, fNotFound, fLoadTypes, level); } From 93723fc9f19a40cc4ac9c72257d15cb7d80519c6 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Thu, 16 Nov 2023 15:38:56 -0800 Subject: [PATCH 07/13] Use the right typedef when looking up enclosing types in the fallback add path --- src/coreclr/vm/clsload.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/clsload.cpp b/src/coreclr/vm/clsload.cpp index d86723cd21824e..1263c643a5ab06 100644 --- a/src/coreclr/vm/clsload.cpp +++ b/src/coreclr/vm/clsload.cpp @@ -3636,7 +3636,7 @@ VOID ClassLoader::AddAvailableClassHaveLock( pszName = pszNameSpace = "Invalid TypeDef token"; pModule->GetAssembly()->ThrowBadImageException(pszNameSpace, pszName, BFA_INVALID_TOKEN); } - NameHandle nameHandleEncloser(pModule, classdef); + NameHandle nameHandleEncloser(pModule, enclosing); nameHandleEncloser.SetName(pszEnclosingNameSpace, pszEnclosingName); pEncloser = pClassHash->FindByNameHandle(&nameHandleEncloser); From 81228fe4f7c0b3382d74b8cc97fefc20eb23d09d Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Mon, 27 Nov 2023 13:26:36 -0800 Subject: [PATCH 08/13] Spelling fix --- docs/design/specs/Ecma-335-Augments.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/design/specs/Ecma-335-Augments.md b/docs/design/specs/Ecma-335-Augments.md index 886bb3cb3d711f..e9722759ea80e6 100644 --- a/docs/design/specs/Ecma-335-Augments.md +++ b/docs/design/specs/Ecma-335-Augments.md @@ -385,7 +385,7 @@ In addition to the TypeDef table having a special ordering constraint, the Expor This line should be changed. -> Finally, this TypeDef _and ExportedType_ ~~table has~~ _tables have_ a special ordering constraint: the definition of an enclosing class shall precede the definintion of all classes it encloses. +> Finally, this TypeDef _and ExportedType_ ~~table has~~ _tables have_ a special ordering constraint: the definition of an enclosing class shall precede the definition of all classes it encloses. ## Module Initializer From 64e1282c3c7835fc92fbc9491e195617330af8a6 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Mon, 27 Nov 2023 13:47:53 -0800 Subject: [PATCH 09/13] Remove FindTypeDef optimization change --- src/coreclr/vm/clsload.cpp | 91 --------------------------- src/coreclr/vm/clsload.hpp | 1 - src/coreclr/vm/methodtablebuilder.cpp | 40 +++++++++++- 3 files changed, 39 insertions(+), 93 deletions(-) diff --git a/src/coreclr/vm/clsload.cpp b/src/coreclr/vm/clsload.cpp index 3040ef9d17ee53..e1dfc9c6e19661 100644 --- a/src/coreclr/vm/clsload.cpp +++ b/src/coreclr/vm/clsload.cpp @@ -454,97 +454,6 @@ EEClassHashEntry_t* ClassLoader::InsertValue(EEClassHashTable *pClassHash, EECla } -mdTypeDef ClassLoader::LookupTypeDefTokenThatMatchesTypeRef(mdTypeRef typeRef) -{ - CONTRACTL - { - INSTANCE_CHECK; - MODE_ANY; - THROWS; - GC_TRIGGERS; - SUPPORTS_DAC; - } - CONTRACTL_END - - _ASSERTE(TypeFromToken(typeRef) == mdtTypeRef); - mdToken mdFoundTypeToken; - - Module* pModule = GetAssembly()->GetModule(); - - NameHandle nhTypeRef(pModule, typeRef); - - LPCUTF8 pszNameSpace; - LPCUTF8 pszClassName; - if (FAILED(pModule->GetMDImport()->GetNameOfTypeRef(typeRef, &pszNameSpace, &pszClassName))) - { - ThrowHR(COR_E_BADIMAGEFORMAT); - } - - nhTypeRef.SetName(pszNameSpace, pszClassName); - -#ifdef FEATURE_READYTORUN - if (pModule->IsReadyToRun()) - { - if (pModule->GetReadyToRunInfo()->HasHashtableOfTypes() && pModule->GetAvailableClassHash() == NULL) - { - // For R2R modules, we only search the hashtable of token types stored in the module's image, and don't fallback - // to searching m_pAvailableClasses or m_pAvailableClassesCaseIns (in fact, we don't even allocate them for R2R modules). - // Also note that type lookups in R2R modules only support case sensitive lookups. - if (pModule->GetReadyToRunInfo()->TryLookupTypeTokenFromName(&nhTypeRef, &mdFoundTypeToken)) - { - if (TypeFromToken(mdFoundTypeToken) == mdtTypeDef) - { - return mdFoundTypeToken; - } - } - return mdTypeDefNil; - } - if (pModule->GetAvailableClassHash() == NULL) - { - // Old R2R image generated without the hashtable of types. - // We fallback to the slow path of creating the hashtable dynamically - // at execution time in that scenario. - // Take the lock. To make sure the table is not being built by another thread. - ClassLoader::AvailableClasses_LockHolder lh(this); - - if (m_cUnhashedModules != 0) - { - // Note: This codepath is only valid for R2R scenarios - LazyPopulateCaseSensitiveHashTables(); - } - } - } -#endif - - EEClassHashEntry_t* entry = pModule->GetAvailableClassHash()->FindByNameHandle(&nhTypeRef); - if (entry != NULL) - { - HashDatum Data = entry->GetData(); - if ((dac_cast(Data) & EECLASSHASH_TYPEHANDLE_DISCR) == 0) - { - TypeHandle t = TypeHandle::FromPtr(Data); - if (!t.IsTypeDesc()) - { - MethodTable *pMT = t.AsMethodTable(); - if (pMT->GetModule() == pModule) - { - return pMT->GetCl(); - } - } - } - else - { - mdFoundTypeToken = pModule->GetAvailableClassHash()->UncompressModuleAndClassDef(Data); - if (TypeFromToken(mdFoundTypeToken) == mdtTypeDef) - { - return mdFoundTypeToken; - } - } - } - - return mdTypeDefNil; -} - #endif // #ifndef DACCESS_COMPILE void ClassLoader::GetClassValue(NameHandleTable nhTable, diff --git a/src/coreclr/vm/clsload.hpp b/src/coreclr/vm/clsload.hpp index 01094883184d30..0c2cb86cc5e75d 100644 --- a/src/coreclr/vm/clsload.hpp +++ b/src/coreclr/vm/clsload.hpp @@ -734,7 +734,6 @@ class ClassLoader TypeHandle LoadTypeHandleThrowIfFailed(NameHandle* pName, ClassLoadLevel level = CLASS_LOADED, Module* pLookInThisModuleOnly=NULL); - mdTypeDef LookupTypeDefTokenThatMatchesTypeRef(mdTypeRef typeRef); public: // Looks up class in the local module table, if it is there it succeeds, // Otherwise it fails, This is meant only for optimizations etc diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 0bf70b63889f71..44ddcf546064bb 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -4047,7 +4047,45 @@ VOID MethodTableBuilder::InitializeFieldDescs(FieldDesc *pFieldDescList, if (TypeFromToken(dwByValueClassToken) == mdtTypeRef) { // It's a typeref - check if it's a class that has a static field of itself - dwByValueClassToken = GetModule()->GetClassLoader()->LookupTypeDefTokenThatMatchesTypeRef(dwByValueClassToken); + LPCUTF8 pszNameSpace; + LPCUTF8 pszClassName; + if (FAILED(pInternalImport->GetNameOfTypeRef(dwByValueClassToken, &pszNameSpace, &pszClassName))) + { + BuildMethodTableThrowException(IDS_CLASSLOAD_BADFORMAT); + } + + if (IsStrLongerThan((char *)pszClassName, MAX_CLASS_NAME) + || IsStrLongerThan((char *)pszNameSpace, MAX_CLASS_NAME) + || (strlen(pszClassName) + strlen(pszNameSpace) + 1 >= MAX_CLASS_NAME)) + { + BuildMethodTableThrowException(BFA_TYPEREG_NAME_TOO_LONG, mdMethodDefNil); + } + + mdToken tkRes; + if (FAILED(pInternalImport->GetResolutionScopeOfTypeRef(dwByValueClassToken, &tkRes))) + { + BuildMethodTableThrowException(BFA_BAD_TYPEREF_TOKEN, dwByValueClassToken); + } + + if (TypeFromToken(tkRes) == mdtTypeRef) + { + if (!pInternalImport->IsValidToken(tkRes)) + { + BuildMethodTableThrowException(BFA_BAD_TYPEREF_TOKEN, mdMethodDefNil); + } + } + else + { + tkRes = mdTokenNil; + } + + if (FAILED(pInternalImport->FindTypeDef(pszNameSpace, + pszClassName, + tkRes, + &dwByValueClassToken))) + { + dwByValueClassToken = mdTokenNil; + } } // If field is static typeref BOOL selfref = IsSelfReferencingStaticValueTypeField(dwByValueClassToken, From 6ceb5d0b378d5848533b95d694d8d98a21d4ce39 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 28 Nov 2023 14:22:17 -0800 Subject: [PATCH 10/13] Code review feedback --- src/coreclr/inc/corhlprpriv.h | 7 ++----- src/coreclr/vm/clsload.cpp | 10 +++++----- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/coreclr/inc/corhlprpriv.h b/src/coreclr/inc/corhlprpriv.h index a043b08a4a4b07..62298798d70178 100644 --- a/src/coreclr/inc/corhlprpriv.h +++ b/src/coreclr/inc/corhlprpriv.h @@ -298,11 +298,8 @@ class CQuickMemoryBase { LPSTR buffer = (LPSTR) AllocThrows(len + 1); - if (buffer != NULL) - { - memcpy(buffer, pStr, len); - buffer[len] = 0; - } + memcpy(buffer, pStr, len); + buffer[len] = 0; return buffer; } diff --git a/src/coreclr/vm/clsload.cpp b/src/coreclr/vm/clsload.cpp index e1dfc9c6e19661..e54348a2b3878e 100644 --- a/src/coreclr/vm/clsload.cpp +++ b/src/coreclr/vm/clsload.cpp @@ -3480,11 +3480,12 @@ VOID ClassLoader::AddAvailableClassDontHaveLock(Module *pModule, CONTRACTL_END CrstHolder ch(&m_AvailableClassLock); + SArray classEntries(); AddAvailableClassHaveLock( pModule, classdef, - NULL, + &classEntries, pamTracker); } @@ -3533,7 +3534,7 @@ VOID ClassLoader::AddAvailableClassHaveLock( COUNT_T classEntryIndex = RidFromToken(enclosing) - 1; _ASSERTE(RidFromToken(enclosing) < RidFromToken(classdef)); - if (classEntries != NULL && classEntries->GetCount() > classEntryIndex) + if (classEntries->GetCount() > classEntryIndex) { pEncloser = (*classEntries)[classEntryIndex]; } @@ -3560,10 +3561,9 @@ VOID ClassLoader::AddAvailableClassHaveLock( insertedEntry = InsertValue(pClassHash, pClassCaseInsHash, pszNameSpace, pszName, EEClassHashTable::CompressClassDef(classdef), pEncloser, pamTracker); _ASSERTE(insertedEntry != NULL); - if (classEntries != NULL) + COUNT_T classEntryIndex = RidFromToken(classdef) - 1; + if (classEntryIndex < classEntries->GetCount()) { - COUNT_T classEntryIndex = RidFromToken(classdef) - 1; - _ASSERTE(classEntryIndex < classEntries->GetCount()); (*classEntries)[classEntryIndex] = insertedEntry; } } From 551c8e91f2a57da7b51d6343116a3dd6ada6835c Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 29 Nov 2023 17:28:07 -0800 Subject: [PATCH 11/13] Fix oops --- src/coreclr/vm/clsload.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/clsload.cpp b/src/coreclr/vm/clsload.cpp index e54348a2b3878e..7b4e24dba6bd0c 100644 --- a/src/coreclr/vm/clsload.cpp +++ b/src/coreclr/vm/clsload.cpp @@ -3480,7 +3480,7 @@ VOID ClassLoader::AddAvailableClassDontHaveLock(Module *pModule, CONTRACTL_END CrstHolder ch(&m_AvailableClassLock); - SArray classEntries(); + SArray classEntries; AddAvailableClassHaveLock( pModule, From 67af66d7dbeac3bd0aaf86662cb417fcdad454b3 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Mon, 18 Dec 2023 16:29:04 -0800 Subject: [PATCH 12/13] Fix code review nits --- src/coreclr/vm/classhash.cpp | 8 ++++---- src/coreclr/vm/clsload.cpp | 11 ++++++----- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/coreclr/vm/classhash.cpp b/src/coreclr/vm/classhash.cpp index e49c13bf46e3c9..5d2be11c9b3280 100644 --- a/src/coreclr/vm/classhash.cpp +++ b/src/coreclr/vm/classhash.cpp @@ -233,10 +233,10 @@ VOID EEClassHashTable::ConstructKeyFromData(PTR_EEClassHashEntry pEntry, // IN _ASSERTE(!(IsCaseInsensitiveTable() && FORBIDGC_LOADER_USE_ENABLED())); #endif - // cqb - If IsCaseInsensitiveTable() is true for the hash table, the bytes in Key will be allocated - // from cqb. This is to prevent wasting bytes in the Loader Heap. Thusly, it is important to note that - // in this case, the lifetime of Key is bounded by the lifetime of cqb, which will free the memory - // it allocated on destruction. + // If IsCaseInsensitiveTable() is true for the hash table, strings passed to the ConstructKeyCallback instance + // will be dynamically allocated. This is to prevent wasting bytes in the Loader Heap. Thusly, it is important + // to note that in this case, the lifetime of Key is bounded by the lifetime of the single call to UseKeys, and + // will be freed when that function returns. _ASSERTE(m_pModule != NULL); LPSTR pszName = NULL; diff --git a/src/coreclr/vm/clsload.cpp b/src/coreclr/vm/clsload.cpp index 7b4e24dba6bd0c..cfcf79c2ffa22d 100644 --- a/src/coreclr/vm/clsload.cpp +++ b/src/coreclr/vm/clsload.cpp @@ -3583,11 +3583,12 @@ VOID ClassLoader::AddExportedTypeDontHaveLock(Module *pManifestModule, CONTRACTL_END CrstHolder ch(&m_AvailableClassLock); + SArray exportedEntries; AddExportedTypeHaveLock( pManifestModule, cl, - NULL, + &exportedEntries, pamTracker); } @@ -3630,7 +3631,7 @@ VOID ClassLoader::AddExportedTypeHaveLock(Module *pManifestModule, { COUNT_T exportedEntryIndex = RidFromToken(mdImpl) - 1; _ASSERTE(RidFromToken(mdImpl) < RidFromToken(cl)); - if (exportedEntries != NULL && exportedEntries->GetCount() > exportedEntryIndex) + if (exportedEntries->GetCount() > exportedEntryIndex) { pEncloser = (*exportedEntries)[exportedEntryIndex]; } @@ -3669,11 +3670,11 @@ VOID ClassLoader::AddExportedTypeHaveLock(Module *pManifestModule, } insertedEntry = InsertValue(pClassHash, pClassCaseInsHash, pszNameSpace, pszName, EEClassHashTable::CompressClassDef(cl), pEncloser, pamTracker); + _ASSERTE(insertedEntry != NULL); - if (exportedEntries != NULL) + COUNT_T exportedEntryIndex = RidFromToken(cl) - 1; + if (classEntryIndex < exportedEntries->GetCount()) { - COUNT_T exportedEntryIndex = RidFromToken(cl) - 1; - _ASSERTE(exportedEntryIndex < exportedEntries->GetCount()); (*exportedEntries)[exportedEntryIndex] = insertedEntry; } } From 3dc1e0a6f5c5fd27f5aae528869cc6d722529b95 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 19 Dec 2023 09:01:31 -0800 Subject: [PATCH 13/13] Fix oops --- src/coreclr/vm/clsload.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/clsload.cpp b/src/coreclr/vm/clsload.cpp index cfcf79c2ffa22d..630016d31398bd 100644 --- a/src/coreclr/vm/clsload.cpp +++ b/src/coreclr/vm/clsload.cpp @@ -3673,7 +3673,7 @@ VOID ClassLoader::AddExportedTypeHaveLock(Module *pManifestModule, _ASSERTE(insertedEntry != NULL); COUNT_T exportedEntryIndex = RidFromToken(cl) - 1; - if (classEntryIndex < exportedEntries->GetCount()) + if (exportedEntryIndex < exportedEntries->GetCount()) { (*exportedEntries)[exportedEntryIndex] = insertedEntry; }