From c6b4af37986ed165cd545a1f5869a10038b46c43 Mon Sep 17 00:00:00 2001 From: David Mason Date: Mon, 24 Aug 2020 14:53:07 -0700 Subject: [PATCH] Fix GetAppdomainStaticAddress for non-collectible assemblies (#41076) --- src/coreclr/src/vm/proftoeeinterfaceimpl.cpp | 8 ++-- .../getappdomainstaticaddress.cpp | 23 +++++++-- .../getappdomainstaticaddress.h | 2 + .../metadatagetdispenser/metadatagetdispenser.cpp | 26 ----------- src/tests/profiler/native/profilerstring.h | 54 +++++++++++++++++++++- .../native/rejitprofiler/rejitprofiler.cpp | 24 ---------- .../profiler/native/rejitprofiler/rejitprofiler.h | 1 - 7 files changed, 79 insertions(+), 59 deletions(-) diff --git a/src/coreclr/src/vm/proftoeeinterfaceimpl.cpp b/src/coreclr/src/vm/proftoeeinterfaceimpl.cpp index 487e123..2de646b 100644 --- a/src/coreclr/src/vm/proftoeeinterfaceimpl.cpp +++ b/src/coreclr/src/vm/proftoeeinterfaceimpl.cpp @@ -3085,7 +3085,7 @@ HRESULT ProfToEEInterfaceImpl::GetRVAStaticAddress(ClassID classId, * Returns: * S_OK on success, * E_INVALIDARG if not an app domain static, - * CORPROF_E_DATAINCOMPLETE if not yet initialized. + * CORPROF_E_DATAINCOMPLETE if not yet initialized or the module is being unloaded. * */ HRESULT ProfToEEInterfaceImpl::GetAppDomainStaticAddress(ClassID classId, @@ -3146,8 +3146,10 @@ HRESULT ProfToEEInterfaceImpl::GetAppDomainStaticAddress(ClassID classId, return CORPROF_E_DATAINCOMPLETE; } - if (typeHandle.GetModule()->GetLoaderAllocator() == NULL || - typeHandle.GetModule()->GetLoaderAllocator()->GetExposedObject() == NULL) + // We might have caught a collectible assembly in the middle of being collected + Module *pModule = typeHandle.GetModule(); + if (pModule->IsCollectible() && + (pModule->GetLoaderAllocator() == NULL || pModule->GetLoaderAllocator()->GetExposedObject() == NULL)) { return CORPROF_E_DATAINCOMPLETE; } diff --git a/src/tests/profiler/native/getappdomainstaticaddress/getappdomainstaticaddress.cpp b/src/tests/profiler/native/getappdomainstaticaddress/getappdomainstaticaddress.cpp index f62a92a..70fcc60 100644 --- a/src/tests/profiler/native/getappdomainstaticaddress/getappdomainstaticaddress.cpp +++ b/src/tests/profiler/native/getappdomainstaticaddress/getappdomainstaticaddress.cpp @@ -24,6 +24,8 @@ GetAppDomainStaticAddress::GetAppDomainStaticAddress() : refCount(0), failures(0), successes(0), + collectibleCount(0), + nonCollectibleCount(0), jitEventCount(0), gcTriggerThread(), gcWaitEvent(), @@ -111,13 +113,14 @@ HRESULT GetAppDomainStaticAddress::Shutdown() this->pCorProfilerInfo = nullptr; } - if(failures == 0 && successes > 0) + if(failures == 0 && successes > 0 && collectibleCount > 0 && nonCollectibleCount > 0) { printf("PROFILER TEST PASSES\n"); } else { - printf("Test failed number of failures=%d successes=%d\n", failures.load(), successes.load()); + printf("Test failed number of failures=%d successes=%d collectibleCount=%d nonCollectibleCount=%d\n", + failures.load(), successes.load(), collectibleCount.load(), nonCollectibleCount.load()); } fflush(stdout); @@ -403,7 +406,7 @@ HRESULT GetAppDomainStaticAddress::GarbageCollectionFinished() // Module is being unloaded... continue; } - if (FAILED(hr)) + else if (FAILED(hr)) { printf("GetModuleMetaData returned 0x%x for ModuleID 0x%" PRIxPTR "\n", hr, classModuleId); ++failures; @@ -434,7 +437,7 @@ HRESULT GetAppDomainStaticAddress::GarbageCollectionFinished() // Class load not complete. We can not inspect yet. continue; } - if (FAILED(hr)) + else if (FAILED(hr)) { printf("GetClassIDInfo2returned 0x%x\n", hr); ++failures; @@ -515,6 +518,18 @@ HRESULT GetAppDomainStaticAddress::GarbageCollectionFinished() ++failures; continue; } + else if (hr != CORPROF_E_DATAINCOMPLETE) + { + String moduleName = GetModuleIDName(classModuleId); + if (EndsWith(moduleName, WCHAR("unloadlibrary.dll"))) + { + ++collectibleCount; + } + else + { + ++nonCollectibleCount; + } + } } } } diff --git a/src/tests/profiler/native/getappdomainstaticaddress/getappdomainstaticaddress.h b/src/tests/profiler/native/getappdomainstaticaddress/getappdomainstaticaddress.h index 009fb55..519700e 100644 --- a/src/tests/profiler/native/getappdomainstaticaddress/getappdomainstaticaddress.h +++ b/src/tests/profiler/native/getappdomainstaticaddress/getappdomainstaticaddress.h @@ -88,6 +88,8 @@ private: std::atomic refCount; std::atomic failures; std::atomic successes; + std::atomic collectibleCount; + std::atomic nonCollectibleCount; std::atomic jitEventCount; std::thread gcTriggerThread; diff --git a/src/tests/profiler/native/metadatagetdispenser/metadatagetdispenser.cpp b/src/tests/profiler/native/metadatagetdispenser/metadatagetdispenser.cpp index 93375b0..8abeb3e 100644 --- a/src/tests/profiler/native/metadatagetdispenser/metadatagetdispenser.cpp +++ b/src/tests/profiler/native/metadatagetdispenser/metadatagetdispenser.cpp @@ -165,32 +165,6 @@ HRESULT MetaDataGetDispenser::GetDispenser(IMetaDataDispenserEx **disp) #else // WIN32 -bool EndsWith(const char *lhs, const char *rhs) -{ - size_t lhsLen = strlen(lhs); - size_t rhsLen = strlen(rhs); - if (lhsLen < rhsLen) - { - return false; - } - - size_t lhsPos = lhsLen - rhsLen; - size_t rhsPos = 0; - - while (rhsPos < rhsLen) - { - if (lhs[lhsPos] != rhs[rhsPos]) - { - return false; - } - - ++lhsPos; - ++rhsPos; - } - - return true; -} - #ifdef __APPLE__ string GetCoreCLRPath() { diff --git a/src/tests/profiler/native/profilerstring.h b/src/tests/profiler/native/profilerstring.h index d7b6375..16ac3ec 100644 --- a/src/tests/profiler/native/profilerstring.h +++ b/src/tests/profiler/native/profilerstring.h @@ -261,7 +261,7 @@ public: return temp; } - size_t Size() const + size_t Length() const { return wcslen(buffer); } @@ -281,3 +281,55 @@ inline std::wostream& operator<<(std::wostream& os, const String& obj) return os; } + +inline bool EndsWith(const char *lhs, const char *rhs) +{ + size_t lhsLen = strlen(lhs); + size_t rhsLen = strlen(rhs); + if (lhsLen < rhsLen) + { + return false; + } + + size_t lhsPos = lhsLen - rhsLen; + size_t rhsPos = 0; + + while (rhsPos < rhsLen) + { + if (lhs[lhsPos] != rhs[rhsPos]) + { + return false; + } + + ++lhsPos; + ++rhsPos; + } + + return true; +} + +inline bool EndsWith(const String &lhs, const String &rhs) +{ + size_t lhsLength = lhs.Length(); + size_t rhsLength = rhs.Length(); + if (lhsLength < rhsLength) + { + return false; + } + + size_t lhsPos = lhsLength - rhsLength; + size_t rhsPos = 0; + + while (rhsPos < rhsLength) + { + if (std::tolower(lhs[lhsPos]) != std::tolower(rhs[rhsPos])) + { + return false; + } + + ++lhsPos; + ++rhsPos; + } + + return true; +} diff --git a/src/tests/profiler/native/rejitprofiler/rejitprofiler.cpp b/src/tests/profiler/native/rejitprofiler/rejitprofiler.cpp index 4da457c..298109b 100644 --- a/src/tests/profiler/native/rejitprofiler/rejitprofiler.cpp +++ b/src/tests/profiler/native/rejitprofiler/rejitprofiler.cpp @@ -492,27 +492,3 @@ ModuleID ReJITProfiler::GetModuleIDForFunction(FunctionID functionId) typeArgs); return moduleId; } - -bool ReJITProfiler::EndsWith(const String &lhs, const String &rhs) -{ - if (lhs.Size() < rhs.Size()) - { - return false; - } - - size_t lhsPos = lhs.Size() - rhs.Size(); - size_t rhsPos = 0; - - while (rhsPos < rhs.Size()) - { - if (std::tolower(lhs[lhsPos]) != std::tolower(rhs[rhsPos])) - { - return false; - } - - ++lhsPos; - ++rhsPos; - } - - return true; -} diff --git a/src/tests/profiler/native/rejitprofiler/rejitprofiler.h b/src/tests/profiler/native/rejitprofiler/rejitprofiler.h index 88f26f8..f105945 100644 --- a/src/tests/profiler/native/rejitprofiler/rejitprofiler.h +++ b/src/tests/profiler/native/rejitprofiler/rejitprofiler.h @@ -51,7 +51,6 @@ private: FunctionID GetFunctionIDFromToken(ModuleID module, mdMethodDef token); mdMethodDef GetMethodDefForFunction(FunctionID functionId); ModuleID GetModuleIDForFunction(FunctionID functionId); - bool EndsWith(const String &lhs, const String &rhs); ICorProfilerInfo10 *_profInfo10; std::atomic _failures; -- 2.7.4