Skip to content

Commit

Permalink
Fix some SOS commands after SharedDomain removal (dotnet/coreclr#21401)
Browse files Browse the repository at this point in the history
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 dotnet/coreclr@af46c51
  • Loading branch information
janvorli committed Dec 7, 2018
1 parent b717e2f commit 4fec8ad
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 26 deletions.
64 changes: 44 additions & 20 deletions src/coreclr/src/ToolBox/SOS/Strike/strike.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<CLRDATA_ADDRESS> pArray = new NOTHROW CLRDATA_ADDRESS[adsData.DomainCount];

Expand Down Expand Up @@ -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<CLRDATA_ADDRESS> pArray = new NOTHROW CLRDATA_ADDRESS[adsData.DomainCount];
if (pArray==NULL)
Expand Down Expand Up @@ -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)
{
}

Expand Down Expand Up @@ -10772,19 +10775,28 @@ class GCHandleStatsForDomains
if (adsData.Request(g_sos) != S_OK)
return FALSE;

m_numDomains = adsData.DomainCount + 2;
ArrayHolder<CLRDATA_ADDRESS> pArray = new NOTHROW CLRDATA_ADDRESS[adsData.DomainCount + 2];
LONG numSpecialDomains = (adsData.sharedDomain != NULL) ? 2 : 1;
m_numDomains = adsData.DomainCount + numSpecialDomains;
ArrayHolder<CLRDATA_ADDRESS> 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;
}
Expand Down Expand Up @@ -10829,12 +10841,24 @@ class GCHandleStatsForDomains
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
Expand Down Expand Up @@ -10905,9 +10929,9 @@ class GCHandlesImpl
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");
Expand Down
20 changes: 14 additions & 6 deletions src/coreclr/src/ToolBox/SOS/Strike/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2679,7 +2679,8 @@ DWORD_PTR *ModuleFromName(__in_opt LPSTR mName, int *numModule)
ArrayHolder<CLRDATA_ADDRESS> pAssemblyArray = NULL;
ArrayHolder<CLRDATA_ADDRESS> pModules = NULL;
int arrayLength = 0;
if (!ClrSafeInt<int>::addition(adsData.DomainCount, 2, arrayLength))
int numSpecialDomains = (adsData.sharedDomain != NULL) ? 2 : 1;
if (!ClrSafeInt<int>::addition(adsData.DomainCount, numSpecialDomains, arrayLength))
{
ExtOut("<integer overflow>\n");
return NULL;
Expand All @@ -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;
Expand All @@ -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())
{
Expand Down Expand Up @@ -3458,15 +3462,19 @@ void GetDomainList (DWORD_PTR *&domainList, int &numDomain)
// Do prefast integer checks before the malloc.
size_t AllocSize;
LONG DomainAllocCount;
if (!ClrSafeInt<LONG>::addition(adsData.DomainCount, 2, DomainAllocCount) ||
LONG NumExtraDomains = (adsData.sharedDomain != NULL) ? 2 : 1;
if (!ClrSafeInt<LONG>::addition(adsData.DomainCount, NumExtraDomains, DomainAllocCount) ||
!ClrSafeInt<size_t>::multiply(DomainAllocCount, sizeof(PVOID), AllocSize) ||
(domainList = new DWORD_PTR[DomainAllocCount]) == NULL)
{
return;
}

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)
Expand Down

0 comments on commit 4fec8ad

Please sign in to comment.