From 4fec8ad045ff4a1f04892caa042fc3ba5c31b5a3 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Fri, 7 Dec 2018 20:13:43 +0100 Subject: [PATCH] Fix some SOS commands after SharedDomain removal (dotnet/coreclr#21401) The recent removal of SharedDomain has broken some SOS commands, like Name2EE or bpmd. There was a code that was enumerating domains and obtaining some information on them. And the shared domain pointer from DacpAppDomainStoreData was being included in the list of domains. As it is NULL now, we have failed to get the information and the domain iteration loop was exited prematurely. I have made SOS resilient to the possibility of missing shared domain. On older runtimes, the shared domain is still being reported. Commit migrated from https://github.com/dotnet/coreclr/commit/af46c514824aacd4d439eb1891d3d392c6894476 --- src/coreclr/src/ToolBox/SOS/Strike/strike.cpp | 64 ++++++++++++++++++--------- src/coreclr/src/ToolBox/SOS/Strike/util.cpp | 20 ++++++--- 2 files changed, 58 insertions(+), 26 deletions(-) diff --git a/src/coreclr/src/ToolBox/SOS/Strike/strike.cpp b/src/coreclr/src/ToolBox/SOS/Strike/strike.cpp index c808b40..8ce5be2 100644 --- a/src/coreclr/src/ToolBox/SOS/Strike/strike.cpp +++ b/src/coreclr/src/ToolBox/SOS/Strike/strike.cpp @@ -3386,7 +3386,10 @@ DECLARE_API(EEHeap) // The first one is the system domain. ExtOut("Loader Heap:\n"); IfFailRet(PrintDomainHeapInfo("System Domain", adsData.systemDomain, &allHeapSize, &wasted)); - IfFailRet(PrintDomainHeapInfo("Shared Domain", adsData.sharedDomain, &allHeapSize, &wasted)); + if (adsData.sharedDomain != NULL) + { + IfFailRet(PrintDomainHeapInfo("Shared Domain", adsData.sharedDomain, &allHeapSize, &wasted)); + } ArrayHolder pArray = new NOTHROW CLRDATA_ADDRESS[adsData.DomainCount]; @@ -6086,14 +6089,17 @@ DECLARE_API(DumpDomain) } DomainInfo(&appDomain); - ExtOut("--------------------------------------\n"); - DMLOut("Shared Domain: %s\n", DMLDomain(adsData.sharedDomain)); - if ((Status=appDomain.Request(g_sos, adsData.sharedDomain))!=S_OK) + if (adsData.sharedDomain != NULL) { - ExtOut("Unable to get shared domain info\n"); - return Status; + ExtOut("--------------------------------------\n"); + DMLOut("Shared Domain: %s\n", DMLDomain(adsData.sharedDomain)); + if ((Status=appDomain.Request(g_sos, adsData.sharedDomain))!=S_OK) + { + ExtOut("Unable to get shared domain info\n"); + return Status; + } + DomainInfo(&appDomain); } - DomainInfo(&appDomain); ArrayHolder pArray = new NOTHROW CLRDATA_ADDRESS[adsData.DomainCount]; if (pArray==NULL) @@ -10734,11 +10740,8 @@ DECLARE_API(FindRoots) class GCHandleStatsForDomains { public: - const static int SHARED_DOMAIN_INDEX = 0; - const static int SYSTEM_DOMAIN_INDEX = 1; - GCHandleStatsForDomains() - : m_singleDomainMode(FALSE), m_numDomains(0), m_pStatistics(NULL), m_pDomainPointers(NULL) + : m_singleDomainMode(FALSE), m_numDomains(0), m_pStatistics(NULL), m_pDomainPointers(NULL), m_sharedDomainIndex(-1), m_systemDomainIndex(-1) { } @@ -10772,19 +10775,28 @@ public: if (adsData.Request(g_sos) != S_OK) return FALSE; - m_numDomains = adsData.DomainCount + 2; - ArrayHolder pArray = new NOTHROW CLRDATA_ADDRESS[adsData.DomainCount + 2]; + LONG numSpecialDomains = (adsData.sharedDomain != NULL) ? 2 : 1; + m_numDomains = adsData.DomainCount + numSpecialDomains; + ArrayHolder pArray = new NOTHROW CLRDATA_ADDRESS[m_numDomains]; if (pArray == NULL) return FALSE; - pArray[SHARED_DOMAIN_INDEX] = adsData.sharedDomain; - pArray[SYSTEM_DOMAIN_INDEX] = adsData.systemDomain; + int i = 0; + if (adsData.sharedDomain != NULL) + { + pArray[i++] = adsData.sharedDomain; + } + + pArray[i] = adsData.systemDomain; + + m_sharedDomainIndex = i - 1; // The m_sharedDomainIndex is set to -1 if there is no shared domain + m_systemDomainIndex = i; - if (g_sos->GetAppDomainList(adsData.DomainCount, pArray+2, NULL) != S_OK) + if (g_sos->GetAppDomainList(adsData.DomainCount, pArray+numSpecialDomains, NULL) != S_OK) return FALSE; m_pDomainPointers = pArray.Detach(); - m_pStatistics = new NOTHROW GCHandleStatistics[adsData.DomainCount + 2]; + m_pStatistics = new NOTHROW GCHandleStatistics[m_numDomains]; if (m_pStatistics == NULL) return FALSE; } @@ -10829,12 +10841,24 @@ public: SOS_Assert(index < m_numDomains); return m_pDomainPointers[index]; } - + + int GetSharedDomainIndex() + { + return m_sharedDomainIndex; + } + + int GetSystemDomainIndex() + { + return m_systemDomainIndex; + } + private: BOOL m_singleDomainMode; int m_numDomains; GCHandleStatistics *m_pStatistics; CLRDATA_ADDRESS *m_pDomainPointers; + int m_sharedDomainIndex; + int m_systemDomainIndex; }; class GCHandlesImpl @@ -10905,9 +10929,9 @@ public: Print( "------------------------------------------------------------------------------\n"); Print("GC Handle Statistics for AppDomain ", AppDomainPtr(mHandleStat.GetDomain(i))); - if (i == GCHandleStatsForDomains::SHARED_DOMAIN_INDEX) + if (i == mHandleStat.GetSharedDomainIndex()) Print(" (Shared Domain)\n"); - else if (i == GCHandleStatsForDomains::SYSTEM_DOMAIN_INDEX) + else if (i == mHandleStat.GetSystemDomainIndex()) Print(" (System Domain)\n"); else Print("\n"); diff --git a/src/coreclr/src/ToolBox/SOS/Strike/util.cpp b/src/coreclr/src/ToolBox/SOS/Strike/util.cpp index 5533017..7098cc8 100644 --- a/src/coreclr/src/ToolBox/SOS/Strike/util.cpp +++ b/src/coreclr/src/ToolBox/SOS/Strike/util.cpp @@ -2679,7 +2679,8 @@ DWORD_PTR *ModuleFromName(__in_opt LPSTR mName, int *numModule) ArrayHolder pAssemblyArray = NULL; ArrayHolder pModules = NULL; int arrayLength = 0; - if (!ClrSafeInt::addition(adsData.DomainCount, 2, arrayLength)) + int numSpecialDomains = (adsData.sharedDomain != NULL) ? 2 : 1; + if (!ClrSafeInt::addition(adsData.DomainCount, numSpecialDomains, arrayLength)) { ExtOut("\n"); return NULL; @@ -2693,8 +2694,11 @@ DWORD_PTR *ModuleFromName(__in_opt LPSTR mName, int *numModule) } pArray[0] = adsData.systemDomain; - pArray[1] = adsData.sharedDomain; - if (g_sos->GetAppDomainList(adsData.DomainCount, pArray.GetPtr()+2, NULL)!=S_OK) + if (adsData.sharedDomain != NULL) + { + pArray[1] = adsData.sharedDomain; + } + if (g_sos->GetAppDomainList(adsData.DomainCount, pArray.GetPtr()+numSpecialDomains, NULL)!=S_OK) { ExtOut("Unable to get array of AppDomains\n"); return NULL; @@ -2720,7 +2724,7 @@ DWORD_PTR *ModuleFromName(__in_opt LPSTR mName, int *numModule) char fileName[sizeof(StringData)/2]; // Search all domains to find a module - for (int n = 0; n < adsData.DomainCount+2; n++) + for (int n = 0; n < adsData.DomainCount+numSpecialDomains; n++) { if (IsInterrupt()) { @@ -3458,7 +3462,8 @@ void GetDomainList (DWORD_PTR *&domainList, int &numDomain) // Do prefast integer checks before the malloc. size_t AllocSize; LONG DomainAllocCount; - if (!ClrSafeInt::addition(adsData.DomainCount, 2, DomainAllocCount) || + LONG NumExtraDomains = (adsData.sharedDomain != NULL) ? 2 : 1; + if (!ClrSafeInt::addition(adsData.DomainCount, NumExtraDomains, DomainAllocCount) || !ClrSafeInt::multiply(DomainAllocCount, sizeof(PVOID), AllocSize) || (domainList = new DWORD_PTR[DomainAllocCount]) == NULL) { @@ -3466,7 +3471,10 @@ void GetDomainList (DWORD_PTR *&domainList, int &numDomain) } domainList[numDomain++] = (DWORD_PTR) adsData.systemDomain; - domainList[numDomain++] = (DWORD_PTR) adsData.sharedDomain; + if (adsData.sharedDomain != NULL) + { + domainList[numDomain++] = (DWORD_PTR) adsData.sharedDomain; + } CLRDATA_ADDRESS *pArray = new CLRDATA_ADDRESS[adsData.DomainCount]; if (pArray==NULL) -- 2.7.4