Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make DAC and ProfToEEInterfaceImpl stop using BaseDomain #107570

Merged
merged 3 commits into from
Sep 10, 2024
Merged

Make DAC and ProfToEEInterfaceImpl stop using BaseDomain #107570

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ad6ed14
Make DAC stop using BaseDomain
elinor-fung Sep 6, 2024
dd17e32
Make ProfToEEInterfaceImpl stop using BaseDomain
elinor-fung Sep 9, 2024
ebf4fb2
Apply suggestions from code review
elinor-fung Sep 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/coreclr/debug/daccess/dacimpl.h
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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())
jkotas marked this conversation as resolved.
Show resolved Hide resolved
{
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