Skip to content

Commit

Permalink
Make DAC and ProfToEEInterfaceImpl stop using BaseDomain (dotnet#107570)
Browse files Browse the repository at this point in the history 
`BaseDomain` should no longer be needed now that we only have one `AppDomain` and the `SystemDomain` can be treated as separate. This makes the DAC and ProfToEEInterfaceImpl use `AppDomain` directly and check against `SystemDomain::System()` to determine if a pointer is the system domain.
  • Loading branch information
@elinor-fung
elinor-fung committed Sep 10, 2024
1 parent 76dbb27 commit 61de5df
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 68 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/debug/daccess/dacimpl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1061,7 +1061,7 @@ class ClrDataAccess
virtual HRESULT STDMETHODCALLTYPE GetAppDomainData(CLRDATA_ADDRESS addr, struct DacpAppDomainData *data);
virtual HRESULT STDMETHODCALLTYPE GetAppDomainName(CLRDATA_ADDRESS addr, unsigned int count, _Inout_updates_z_(count) WCHAR *name, unsigned int *pNeeded);
virtual HRESULT STDMETHODCALLTYPE GetAssemblyList(CLRDATA_ADDRESS appDomain, int count, CLRDATA_ADDRESS values[], int *fetched);
virtual HRESULT STDMETHODCALLTYPE GetAssemblyData(CLRDATA_ADDRESS baseDomainPtr, CLRDATA_ADDRESS assembly, struct DacpAssemblyData *data);
virtual HRESULT STDMETHODCALLTYPE GetAssemblyData(CLRDATA_ADDRESS domainPtr, CLRDATA_ADDRESS assembly, struct DacpAssemblyData *data);
virtual HRESULT STDMETHODCALLTYPE GetAssemblyName(CLRDATA_ADDRESS assembly, unsigned int count, _Inout_updates_z_(count) WCHAR *name, unsigned int *pNeeded);
virtual HRESULT STDMETHODCALLTYPE GetThreadData(CLRDATA_ADDRESS thread, struct DacpThreadData *data);
virtual HRESULT STDMETHODCALLTYPE GetThreadFromThinlockID(UINT thinLockId, CLRDATA_ADDRESS *pThread);
Expand Down
50 changes: 19 additions & 31 deletions src/coreclr/debug/daccess/request.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2834,19 +2834,17 @@ ClrDataAccess::GetAppDomainData(CLRDATA_ADDRESS addr, struct DacpAppDomainData *
}
else
{
PTR_BaseDomain pBaseDomain = PTR_BaseDomain(TO_TADDR(addr));

ZeroMemory(appdomainData, sizeof(DacpAppDomainData));
appdomainData->AppDomainPtr = PTR_CDADDR(pBaseDomain);
appdomainData->AppDomainPtr = addr;
PTR_LoaderAllocator pLoaderAllocator = SystemDomain::GetGlobalLoaderAllocator();
appdomainData->pHighFrequencyHeap = HOST_CDADDR(pLoaderAllocator->GetHighFrequencyHeap());
appdomainData->pLowFrequencyHeap = HOST_CDADDR(pLoaderAllocator->GetLowFrequencyHeap());
appdomainData->pStubHeap = HOST_CDADDR(pLoaderAllocator->GetStubHeap());
appdomainData->appDomainStage = STAGE_OPEN;

if (pBaseDomain->IsAppDomain())
if (addr != HOST_CDADDR(SystemDomain::System()))
{
AppDomain * pAppDomain = pBaseDomain->AsAppDomain();
PTR_AppDomain pAppDomain = PTR_AppDomain(TO_TADDR(addr));
appdomainData->DomainLocalBlock = 0;
appdomainData->pDomainLocalModules = 0;

Expand Down Expand Up @@ -2963,15 +2961,19 @@ ClrDataAccess::GetAssemblyList(CLRDATA_ADDRESS addr, int count, CLRDATA_ADDRESS

SOSDacEnter();

BaseDomain* pBaseDomain = PTR_BaseDomain(TO_TADDR(addr));

int n=0;
if (pBaseDomain->IsAppDomain())
if (addr == HOST_CDADDR(SystemDomain::System()))
{
// We shouldn't be asking for the assemblies in SystemDomain
hr = E_INVALIDARG;
}
else
{
AppDomain::AssemblyIterator i = pBaseDomain->AsAppDomain()->IterateAssembliesEx(
PTR_AppDomain pAppDomain = PTR_AppDomain(TO_TADDR(addr));
AppDomain::AssemblyIterator i = pAppDomain->IterateAssembliesEx(
(AssemblyIterationFlags)(kIncludeLoading | kIncludeLoaded | kIncludeExecution));
CollectibleAssemblyHolder<DomainAssembly *> pDomainAssembly;

int n = 0;
if (values)
{
while (i.Next(pDomainAssembly.This()) && (n < count))
Expand All @@ -2994,13 +2996,6 @@ ClrDataAccess::GetAssemblyList(CLRDATA_ADDRESS addr, int count, CLRDATA_ADDRESS
if (pNeeded)
*pNeeded = n;
}
else
{
// The only other type of BaseDomain is the SystemDomain, and we shouldn't be asking
// for the assemblies in it.
_ASSERTE(false);
hr = E_INVALIDARG;
}

SOSDacLeave();
return hr;
Expand Down Expand Up @@ -3040,19 +3035,17 @@ ClrDataAccess::GetAppDomainName(CLRDATA_ADDRESS addr, unsigned int count, _Inout
{
SOSDacEnter();

PTR_BaseDomain pBaseDomain = PTR_BaseDomain(TO_TADDR(addr));
if (!pBaseDomain->IsAppDomain())
if (addr == HOST_CDADDR(SystemDomain::System()))
{
// Shared domain and SystemDomain don't have this field.
// SystemDomain doesn't have this field.
if (pNeeded)
*pNeeded = 1;
if (name)
name[0] = 0;
}
else
{
AppDomain* pAppDomain = pBaseDomain->AsAppDomain();

PTR_AppDomain pAppDomain = PTR_AppDomain(TO_TADDR(addr));
if (!pAppDomain->m_friendlyName.IsEmpty())
{
if (!pAppDomain->m_friendlyName.DacGetUnicode(count, name, pNeeded))
Expand Down Expand Up @@ -3103,9 +3096,9 @@ ClrDataAccess::GetAppDomainConfigFile(CLRDATA_ADDRESS appDomain, int count,
}

HRESULT
ClrDataAccess::GetAssemblyData(CLRDATA_ADDRESS cdBaseDomainPtr, CLRDATA_ADDRESS assembly, struct DacpAssemblyData *assemblyData)
ClrDataAccess::GetAssemblyData(CLRDATA_ADDRESS domain, CLRDATA_ADDRESS assembly, struct DacpAssemblyData *assemblyData)
{
if (assembly == (CLRDATA_ADDRESS)NULL && cdBaseDomainPtr == (CLRDATA_ADDRESS)NULL)
if (assembly == (CLRDATA_ADDRESS)NULL && domain == (CLRDATA_ADDRESS)NULL)
{
return E_INVALIDARG;
}
Expand All @@ -3117,14 +3110,9 @@ ClrDataAccess::GetAssemblyData(CLRDATA_ADDRESS cdBaseDomainPtr, CLRDATA_ADDRESS
// Make sure conditionally-assigned fields like AssemblySecDesc, LoadContext, etc. are zeroed
ZeroMemory(assemblyData, sizeof(DacpAssemblyData));

// If the specified BaseDomain is an AppDomain, get a pointer to it
AppDomain * pDomain = NULL;
if (cdBaseDomainPtr != (CLRDATA_ADDRESS)NULL)
if (domain != (CLRDATA_ADDRESS)NULL)
{
assemblyData->BaseDomainPtr = cdBaseDomainPtr;
PTR_BaseDomain baseDomain = PTR_BaseDomain(TO_TADDR(cdBaseDomainPtr));
if( baseDomain->IsAppDomain() )
pDomain = baseDomain->AsAppDomain();
assemblyData->DomainPtr = domain;
}

assemblyData->AssemblyPtr = HOST_CDADDR(pAssembly);
Expand Down
10 changes: 5 additions & 5 deletions src/coreclr/inc/dacprivate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -425,11 +425,11 @@ enum DacpAppDomainDataStage {
STAGE_CLOSED
};

// Information about a BaseDomain (AppDomain, SharedDomain or SystemDomain).
// Information about an AppDomain or SystemDomain.
// For types other than AppDomain, some fields (like dwID, DomainLocalBlock, etc.) will be 0/null.
struct MSLAYOUT DacpAppDomainData
{
// The pointer to the BaseDomain (not necessarily an AppDomain).
// The pointer to the AppDomain or SystemDomain.
// It's useful to keep this around in the structure
CLRDATA_ADDRESS AppDomainPtr = 0;
CLRDATA_ADDRESS AppSecDesc = 0;
Expand All @@ -455,17 +455,17 @@ struct MSLAYOUT DacpAssemblyData
CLRDATA_ADDRESS AssemblyPtr = 0; //useful to have
CLRDATA_ADDRESS ClassLoader = 0;
CLRDATA_ADDRESS ParentDomain = 0;
CLRDATA_ADDRESS BaseDomainPtr = 0;
CLRDATA_ADDRESS DomainPtr = 0;
CLRDATA_ADDRESS AssemblySecDesc = 0;
BOOL isDynamic = FALSE;
UINT ModuleCount = FALSE;
UINT LoadContext = FALSE;
BOOL isDomainNeutral = FALSE; // Always false, preserved for backward compatibility
DWORD dwLocationFlags = 0;

HRESULT Request(ISOSDacInterface *sos, CLRDATA_ADDRESS addr, CLRDATA_ADDRESS baseDomainPtr)
HRESULT Request(ISOSDacInterface *sos, CLRDATA_ADDRESS addr, CLRDATA_ADDRESS domainPtr)
{
return sos->GetAssemblyData(baseDomainPtr, addr, this);
return sos->GetAssemblyData(domainPtr, addr, this);
}

HRESULT Request(ISOSDacInterface *sos, CLRDATA_ADDRESS addr)
Expand Down
16 changes: 8 additions & 8 deletions src/coreclr/vm/appdomain.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1546,15 +1546,15 @@ void SystemDomain::NotifyProfilerStartup()

{
BEGIN_PROFILER_CALLBACK(CORProfilerTrackAppDomainLoads());
_ASSERTE(System()->DefaultDomain());
(&g_profControlBlock)->AppDomainCreationStarted((AppDomainID) System()->DefaultDomain());
_ASSERTE(AppDomain::GetCurrentDomain());
(&g_profControlBlock)->AppDomainCreationStarted((AppDomainID) AppDomain::GetCurrentDomain());
END_PROFILER_CALLBACK();
}

{
BEGIN_PROFILER_CALLBACK(CORProfilerTrackAppDomainLoads());
_ASSERTE(System()->DefaultDomain());
(&g_profControlBlock)->AppDomainCreationFinished((AppDomainID) System()->DefaultDomain(), S_OK);
_ASSERTE(AppDomain::GetCurrentDomain());
(&g_profControlBlock)->AppDomainCreationFinished((AppDomainID) AppDomain::GetCurrentDomain(), S_OK);
END_PROFILER_CALLBACK();
}
}
Expand Down Expand Up @@ -1585,15 +1585,15 @@ HRESULT SystemDomain::NotifyProfilerShutdown()

{
BEGIN_PROFILER_CALLBACK(CORProfilerTrackAppDomainLoads());
_ASSERTE(System()->DefaultDomain());
(&g_profControlBlock)->AppDomainShutdownStarted((AppDomainID) System()->DefaultDomain());
_ASSERTE(AppDomain::GetCurrentDomain());
(&g_profControlBlock)->AppDomainShutdownStarted((AppDomainID) AppDomain::GetCurrentDomain());
END_PROFILER_CALLBACK();
}

{
BEGIN_PROFILER_CALLBACK(CORProfilerTrackAppDomainLoads());
_ASSERTE(System()->DefaultDomain());
(&g_profControlBlock)->AppDomainShutdownFinished((AppDomainID) System()->DefaultDomain(), S_OK);
_ASSERTE(AppDomain::GetCurrentDomain());
(&g_profControlBlock)->AppDomainShutdownFinished((AppDomainID) AppDomain::GetCurrentDomain(), S_OK);
END_PROFILER_CALLBACK();
}
return (S_OK);
Expand Down
29 changes: 6 additions & 23 deletions src/coreclr/vm/proftoeeinterfaceimpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3148,9 +3148,9 @@ HRESULT ProfToEEInterfaceImpl::GetAppDomainStaticAddress(ClassID classId,
return E_INVALIDARG;
}

// Some domains, like the system domain, aren't APP domains, and thus don't contain any
// The system domain isn't an APP domain and thus doesn't contain any
// statics. See if the profiler is trying to be naughty.
if (!((BaseDomain*) appDomainId)->IsAppDomain())
if (appDomainId == (AppDomainID)SystemDomain::System())
{
return E_INVALIDARG;
}
Expand Down Expand Up @@ -3382,9 +3382,9 @@ HRESULT ProfToEEInterfaceImpl::GetThreadStaticAddress2(ClassID classId,
return E_INVALIDARG;
}

// Some domains, like the system domain, aren't APP domains, and thus don't contain any
// The system domain isn't an APP domain and thus doesn't contain any
// statics. See if the profiler is trying to be naughty.
if (!((BaseDomain*) appDomainId)->IsAppDomain())
if (appDomainId == (AppDomainID)SystemDomain::System())
{
return E_INVALIDARG;
}
Expand Down Expand Up @@ -5479,25 +5479,8 @@ HRESULT ProfToEEInterfaceImpl::GetAppDomainInfo(AppDomainID appDomainId,
return E_INVALIDARG;
}

BaseDomain *pDomain; // Internal data structure.
HRESULT hr = S_OK;

// <TODO>@todo:
// Right now, this ID is not a true AppDomain, since we use the old
// AppDomain/SystemDomain model in the profiling API. This means that
// the profiler exposes the SharedDomain and the SystemDomain to the
// outside world. It's not clear whether this is actually the right thing
// to do or not. - seantrow
//
// Postponed to V2.
// </TODO>

pDomain = (BaseDomain *) appDomainId;

// Make sure they've passed in a valid appDomainId
if (pDomain == NULL)
return (E_INVALIDARG);

// Pick sensible defaults.
if (pcchName)
*pcchName = 0;
Expand All @@ -5507,10 +5490,10 @@ HRESULT ProfToEEInterfaceImpl::GetAppDomainInfo(AppDomainID appDomainId,
*pProcessId = 0;

LPCWSTR szFriendlyName;
if (pDomain == SystemDomain::System())
if (appDomainId == (AppDomainID)SystemDomain::System())
szFriendlyName = g_pwBaseLibrary;
else
szFriendlyName = ((AppDomain*)pDomain)->GetFriendlyNameForDebugger();
szFriendlyName = ((AppDomain*)appDomainId)->GetFriendlyNameForDebugger();

if (szFriendlyName != NULL)
{
Expand Down

0 comments on commit 61de5df

Please sign in to comment.