From 6f4c01623b256d0786f1ad9a15dbcf202514c845 Mon Sep 17 00:00:00 2001 From: David Mason Date: Thu, 23 May 2019 00:28:09 -0700 Subject: [PATCH] Prevent duplicate class addition when a profiler adds a type (#24737) --- src/vm/ceeload.cpp | 37 +++++++++++++++++++++++++------------ src/vm/clsload.cpp | 35 ++++++++++++++++++----------------- src/vm/clsload.hpp | 1 + 3 files changed, 44 insertions(+), 29 deletions(-) diff --git a/src/vm/ceeload.cpp b/src/vm/ceeload.cpp index 3d26a0a..7f382b1 100644 --- a/src/vm/ceeload.cpp +++ b/src/vm/ceeload.cpp @@ -221,22 +221,35 @@ void Module::UpdateNewlyAddedTypes() DWORD countExportedTypesAfterProfilerUpdate = GetMDImport()->GetCountWithTokenKind(mdtExportedType); DWORD countCustomAttributeCount = GetMDImport()->GetCountWithTokenKind(mdtCustomAttribute); - // typeDefs rids 0 and 1 aren't included in the count, thus X typeDefs before means rid X+1 was valid and our incremental addition should start at X+2 - for (DWORD typeDefRid = m_dwTypeCount + 2; typeDefRid < countTypesAfterProfilerUpdate + 2; typeDefRid++) + // R2R pre-computes an export table and tries to avoid populating a class hash at runtime. However the profiler can + // still add new types on the fly by calling here. If that occurs we fallback to the slower path of creating the + // in memory hashtable as usual. + if (!IsResource() && GetAvailableClassHash() == NULL) { - GetAssembly()->AddType(this, TokenFromRid(typeDefRid, mdtTypeDef)); + // This call will populate the hash tables with anything that is in metadata already. + GetClassLoader()->LazyPopulateCaseSensitiveHashTablesDontHaveLock(); } - - // exportedType rid 0 isn't included in the count, thus X exportedTypes before means rid X was valid and our incremental addition should start at X+1 - for (DWORD exportedTypeDef = m_dwExportedTypeCount + 1; exportedTypeDef < countExportedTypesAfterProfilerUpdate + 1; exportedTypeDef++) + else { - GetAssembly()->AddExportedType(TokenFromRid(exportedTypeDef, mdtExportedType)); - } + // If the hash tables already exist (either R2R and we've previously populated the ) we need to manually add the types. - if ((countCustomAttributeCount != m_dwCustomAttributeCount) && IsReadyToRun()) - { - // Set of custom attributes has changed. Disable the cuckoo filter from ready to run, and do normal custom attribute parsing - GetReadyToRunInfo()->DisableCustomAttributeFilter(); + // typeDefs rids 0 and 1 aren't included in the count, thus X typeDefs before means rid X+1 was valid and our incremental addition should start at X+2 + for (DWORD typeDefRid = m_dwTypeCount + 2; typeDefRid < countTypesAfterProfilerUpdate + 2; typeDefRid++) + { + GetAssembly()->AddType(this, TokenFromRid(typeDefRid, mdtTypeDef)); + } + + // exportedType rid 0 isn't included in the count, thus X exportedTypes before means rid X was valid and our incremental addition should start at X+1 + for (DWORD exportedTypeDef = m_dwExportedTypeCount + 1; exportedTypeDef < countExportedTypesAfterProfilerUpdate + 1; exportedTypeDef++) + { + GetAssembly()->AddExportedType(TokenFromRid(exportedTypeDef, mdtExportedType)); + } + + if ((countCustomAttributeCount != m_dwCustomAttributeCount) && IsReadyToRun()) + { + // Set of custom attributes has changed. Disable the cuckoo filter from ready to run, and do normal custom attribute parsing + GetReadyToRunInfo()->DisableCustomAttributeFilter(); + } } m_dwTypeCount = countTypesAfterProfilerUpdate; diff --git a/src/vm/clsload.cpp b/src/vm/clsload.cpp index 479e124..ff4d0a0 100644 --- a/src/vm/clsload.cpp +++ b/src/vm/clsload.cpp @@ -1016,6 +1016,23 @@ VOID ClassLoader::PopulateAvailableClassHashTable(Module* pModule, } +void ClassLoader::LazyPopulateCaseSensitiveHashTablesDontHaveLock() +{ + CONTRACTL + { + INSTANCE_CHECK; + THROWS; + GC_TRIGGERS; + MODE_ANY; + INJECT_FAULT(COMPlusThrowOM()); + } + CONTRACTL_END; + + + CrstHolder ch(&m_AvailableClassLock); + LazyPopulateCaseSensitiveHashTables(); +} + void ClassLoader::LazyPopulateCaseSensitiveHashTables() { CONTRACTL @@ -1036,7 +1053,7 @@ void ClassLoader::LazyPopulateCaseSensitiveHashTables() { Module *pModule = i.GetModule(); PREFIX_ASSUME(pModule != NULL); - if (pModule->IsResource()) + if (pModule->IsResource() || pModule->GetAvailableClassHash() != NULL) continue; // Lazy construction of the case-sensitive hashtable of types is *only* a scenario for ReadyToRun images @@ -4089,14 +4106,6 @@ VOID ClassLoader::AddAvailableClassDontHaveLock(Module *pModule, CrstHolder ch(&m_AvailableClassLock); - // R2R pre-computes an export table and tries to avoid populating a class hash at runtime. However the profiler can - // still add new types on the fly by calling here. If that occurs we fallback to the slower path of creating the - // in memory hashtable as usual. - if (!pModule->IsResource() && pModule->GetAvailableClassHash() == NULL) - { - LazyPopulateCaseSensitiveHashTables(); - } - AddAvailableClassHaveLock( pModule, classdef, @@ -4293,14 +4302,6 @@ VOID ClassLoader::AddExportedTypeDontHaveLock(Module *pManifestModule, CONTRACTL_END CrstHolder ch(&m_AvailableClassLock); - - // R2R pre-computes an export table and tries to avoid populating a class hash at runtime. However the profiler can - // still add new types on the fly by calling here. If that occurs we fallback to the slower path of creating the - // in memory hashtable as usual. - if (!pManifestModule->IsResource() && pManifestModule->GetAvailableClassHash() == NULL) - { - LazyPopulateCaseSensitiveHashTables(); - } AddExportedTypeHaveLock( pManifestModule, diff --git a/src/vm/clsload.hpp b/src/vm/clsload.hpp index 85ee72c..920628b 100644 --- a/src/vm/clsload.hpp +++ b/src/vm/clsload.hpp @@ -581,6 +581,7 @@ private: VOID PopulateAvailableClassHashTable(Module *pModule, AllocMemTracker *pamTracker); + void LazyPopulateCaseSensitiveHashTablesDontHaveLock(); void LazyPopulateCaseSensitiveHashTables(); void LazyPopulateCaseInsensitiveHashTables(); -- 2.7.4