Skip to content

Replace EnumerateModulesInAssembly with combined GetModuleForAssembly; implement in cDAC #128294

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

Open
Copilot wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Replace EnumerateModulesInAssembly with combined GetModuleForAssembly; implement in cDAC #128294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1d5f765
Combine module loaded check into GetModuleForAssembly; implement in cDAC
Copilot May 17, 2026
cf3f3d1
Apply suggestions from code review
rcj1 May 17, 2026
836abfe
Update dacdbiimpl.cpp
rcj1 May 17, 2026
dd6d8b9
Apply suggestions from code review
rcj1 May 17, 2026
4861b24
Address unresolved Copilot review comments
Copilot May 17, 2026
7e626b9
clean up code
rcj1 May 19, 2026
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
50 changes: 21 additions & 29 deletions src/coreclr/debug/daccess/dacdbiimpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4192,7 +4192,7 @@ HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::GetSymbolsBuffer(VMPTR_Module vmM



HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::GetModuleForAssembly(VMPTR_Assembly vmAssembly, OUT VMPTR_Module * pModule)
HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::GetModuleForAssembly(VMPTR_Assembly vmAssembly, OUT VMPTR_Module * pModule, OUT BOOL * pIsModuleLoaded)
{
DD_ENTER_MAY_THROW;

Expand All @@ -4202,8 +4202,26 @@ HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::GetModuleForAssembly(VMPTR_Assemb

_ASSERTE(pModule != NULL);

// Initialize out params up front so callers don't observe stale/uninitialized
// values if we throw below.
*pModule = VMPTR_Module::NullPtr();
if (pIsModuleLoaded != NULL)
{
*pIsModuleLoaded = FALSE;
}

Assembly * pAssembly = vmAssembly.GetDacPtr();
if (pAssembly == NULL)
{
ThrowHR(E_INVALIDARG);
}

pModule->SetHostPtr(pAssembly->GetModule());

if (pIsModuleLoaded != NULL)
{
*pIsModuleLoaded = pAssembly->IsLoaded() ? TRUE : FALSE;
}
Comment thread
rcj1 marked this conversation as resolved.
}
EX_CATCH_HRESULT(hr);
return hr;
Expand Down Expand Up @@ -4256,7 +4274,8 @@ HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::GetModuleData(VMPTR_Module vmModu
}


// Enumerate all Assemblies in an appdomain.
// Implementation of IDacDbiInterface::EnumerateAssembliesInAppDomain.
// Enumerate all the assemblies in the appdomain.
HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::EnumerateAssembliesInAppDomain(VMPTR_AppDomain vmAppDomain, FP_ASSEMBLY_ENUMERATION_CALLBACK fpCallback, CALLBACK_DATA pUserData)
{
DD_ENTER_MAY_THROW;
Expand All @@ -4270,9 +4289,6 @@ HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::EnumerateAssembliesInAppDomain(VM
// Iterate through all Assemblies (including shared) in the appdomain.
AppDomain::AssemblyIterator iterator;

// If the containing appdomain is unloading, then don't enumerate any assemblies
// in the domain. This is to enforce rules at code:IDacDbiInterface#Enumeration.
// See comment in code:DacDbiInterfaceImpl::EnumerateModulesInAssembly code for details.
AppDomain * pAppDomain = vmAppDomain.GetDacPtr();

if (pAppDomain == nullptr)
Expand All @@ -4296,30 +4312,6 @@ HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::EnumerateAssembliesInAppDomain(VM
return hr;
}

// Implementation of IDacDbiInterface::EnumerateModulesInAssembly,
// Enumerate all the modules (non-resource) in an assembly.
HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::EnumerateModulesInAssembly(VMPTR_Assembly vmAssembly, FP_MODULE_ENUMERATION_CALLBACK fpCallback, CALLBACK_DATA pUserData)
{
DD_ENTER_MAY_THROW;

HRESULT hr = S_OK;
EX_TRY
{

_ASSERTE(fpCallback != NULL);

Assembly * pAssembly = vmAssembly.GetDacPtr();

// If assembly isn't yet loaded, just return
if (!pAssembly->IsLoaded())
return hr;

fpCallback(vmAssembly, pUserData);
}
EX_CATCH_HRESULT(hr);
return hr;
}

// Implementation of IDacDbiInterface::ResolveAssembly
// Returns NULL if not found.
HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::ResolveAssembly(VMPTR_Assembly vmScope, mdToken tkAssemblyRef, OUT VMPTR_Assembly * pRetVal)
Expand Down
5 changes: 1 addition & 4 deletions src/coreclr/debug/daccess/dacdbiimpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ class DacDbiInterfaceImpl :
// Gets properties for a module
HRESULT STDMETHODCALLTYPE GetModuleData(VMPTR_Module vmModule, OUT ModuleInfo * pData);

HRESULT STDMETHODCALLTYPE GetModuleForAssembly(VMPTR_Assembly vmAssembly, OUT VMPTR_Module * pModule);
HRESULT STDMETHODCALLTYPE GetModuleForAssembly(VMPTR_Assembly vmAssembly, OUT VMPTR_Module * pModule, OUT BOOL * pIsModuleLoaded);

// Get the "type" of address.
HRESULT STDMETHODCALLTYPE GetAddressType(CORDB_ADDRESS address, OUT AddressType * pRetVal);
Expand All @@ -612,9 +612,6 @@ class DacDbiInterfaceImpl :
// Enumerate the assemblies in the appdomain.
HRESULT STDMETHODCALLTYPE EnumerateAssembliesInAppDomain(VMPTR_AppDomain vmAppDomain, FP_ASSEMBLY_ENUMERATION_CALLBACK fpCallback, CALLBACK_DATA pUserData);

// Enumerate the moduels in the given assembly.
HRESULT STDMETHODCALLTYPE EnumerateModulesInAssembly(VMPTR_Assembly vmAssembly, FP_MODULE_ENUMERATION_CALLBACK fpCallback, CALLBACK_DATA pUserData);

// When stopped at an event, request a synchronization.
HRESULT STDMETHODCALLTYPE RequestSyncAtEvent();

Expand Down
152 changes: 1 addition & 151 deletions src/coreclr/debug/di/process.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4377,156 +4377,6 @@ void CordbProcess::GetAssembliesInLoadOrder(
// pAssemblies array has now been updated.
}

// Callback data for code:CordbProcess::GetModulesInLoadOrder
class ShimModuleCallbackData
{
public:
// Ctor to initialize callback data
//
// Arguments:
// pAssembly - assembly that the Modules are in.
// pModules - preallocated array of smart pointers to hold Modules
// countModules - size of pModules in elements.
ShimModuleCallbackData(
CordbAssembly * pAssembly,
RSExtSmartPtr<ICorDebugModule>* pModules,
ULONG countModules)
{
_ASSERTE(pAssembly != NULL);
_ASSERTE(pModules != NULL);

m_pProcess = pAssembly->GetAppDomain()->GetProcess();
m_pAssembly = pAssembly;
m_pModules = pModules;
m_countElements = countModules;
m_index = 0;

// Just to be safe, clear them all out
for(ULONG i = 0; i < countModules; i++)
{
pModules[i].Clear();
}
}

// Dtor
//
// Notes:
// This can assert end-of-enumeration invariants.
~ShimModuleCallbackData()
{
// Ensure that we went through all Modules.
_ASSERTE(m_index == m_countElements);
}

// Callback invoked from DAC enumeration.
//
// arguments:
// vmAssembly - VMPTR for Assembly
// pData - a 'this' pointer
//
static void Callback(VMPTR_Assembly vmAssembly, void * pData)
{
ShimModuleCallbackData * pThis = static_cast<ShimModuleCallbackData *> (pData);
INTERNAL_DAC_CALLBACK(pThis->m_pProcess);

CordbModule * pModule = pThis->m_pAssembly->GetAppDomain()->LookupOrCreateModule(vmAssembly);

pThis->SetAndMoveNext(pModule);
}

// Set the current index in the table and increment the cursor.
//
// Arguments:
// pModule - Module from DAC enumerator
void SetAndMoveNext(CordbModule * pModule)
{
_ASSERTE(pModule != NULL);

if (m_index >= m_countElements)
{
// Enumerating the Modules in the target should be fixed since
// the target is not running.
// We should never get here unless the target is unstable.
// The caller (the shim) pre-allocated the table of Modules.
m_pProcess->TargetConsistencyCheck(!"Target changed Module count");
return;
}

m_pModules[m_index].Assign(pModule);
m_index++;
}

protected:
CordbProcess * m_pProcess;
CordbAssembly * m_pAssembly;
RSExtSmartPtr<ICorDebugModule>* m_pModules;
ULONG m_countElements;
ULONG m_index;
};

//---------------------------------------------------------------------------------------
// Shim Helper to enumerate the Modules in the load-order
//
// Arguments:
// pAppdomain - non-null appdomain to enumerate Modules.
// pModules - caller pre-allocated array to hold Modules
// countModules - size of the array.
//
// Notes:
// Caller preallocated array (likely from ICorDebugModuleEnum::GetCount),
// and now this function fills in the Modules in the order they were
// loaded.
//
// The target should be stable, such that the number of Modules in the
// target is stable, and therefore countModules as determined by the
// shim via ICorDebugModuleEnum::GetCount should match the number of
// Modules enumerated here.
//
// Called by code:ShimProcess::QueueFakeAssemblyAndModuleEvent.
// This provides the Modules in load-order. In contrast,
// ICorDebugAssembly::EnumerateModules is a random order. The shim needs
// load-order to match Whidbey semantics for dispatching fake load-Module
// callbacks on attach. The most important thing is that the manifest module
// gets a LodModule callback before any secondary modules. For dynamic
// modules, this is necessary for operations on the secondary module
// that rely on manifest metadata (eg. GetSimpleName).
//
// @dbgtodo : This is almost identical to GetAssembliesInLoadOrder, and
// (together wih the CallbackData classes) seems a HUGE amount of code and
// complexity for such a simple thing. We also have extra code to order
// AppDomains and Threads. We should try and rip all of this extra complexity
// out, and replace it with better data structures for storing these items.
// Eg., if we used std::map, we could have efficient lookups and ordered
// enumerations. However, we do need to be careful about exposing new invariants
// through ICorDebug that customers may depend on, which could place a long-term
// compatibility burden on us. We could have a simple generic data structure
// (eg. built on std::hash_map and std::list) which provided efficient look-up
// and both in-order and random enumeration.
//
void CordbProcess::GetModulesInLoadOrder(
ICorDebugAssembly * pAssembly,
RSExtSmartPtr<ICorDebugModule>* pModules,
ULONG countModules)
{
PUBLIC_API_ENTRY_FOR_SHIM(this);
RSLockHolder lockHolder(GetProcessLock());

_ASSERTE(GetShim() != NULL);

CordbAssembly * pAssemblyInternal = static_cast<CordbAssembly *> (pAssembly);

ShimModuleCallbackData data(pAssemblyInternal, pModules, countModules);

// Enumerate through and fill out pModules table.
IfFailThrow(GetDAC()->EnumerateModulesInAssembly(
pAssemblyInternal->GetAssemblyPtr(),
ShimModuleCallbackData::Callback,
&data)); // user data

// pModules array has now been updated.
}


//---------------------------------------------------------------------------------------
// Callback to count the number of enumerations in a process.
//
Expand Down Expand Up @@ -14768,7 +14618,7 @@ CordbClass * CordbProcess::LookupClass(ICorDebugAppDomain * pAppDomain, VMPTR_As
if (pAppDomain != NULL)
{
VMPTR_Module vmModule = VMPTR_Module::NullPtr();
IfFailThrow(GetProcess()->GetDAC()->GetModuleForAssembly(vmAssembly, &vmModule));
IfFailThrow(GetProcess()->GetDAC()->GetModuleForAssembly(vmAssembly, &vmModule, NULL));
_ASSERTE(!vmModule.IsNull());
CordbModule * pModule = ((CordbAppDomain *)pAppDomain)->m_modules.GetBase(VmPtrToCookie(vmModule));
if (pModule != NULL)
Expand Down
43 changes: 10 additions & 33 deletions src/coreclr/debug/di/rsappdomain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,7 @@ CordbModule* CordbAppDomain::LookupOrCreateModule(VMPTR_Assembly vmAssembly, VMP

if (vmModule.IsNull())
{
IfFailThrow(GetProcess()->GetDAC()->GetModuleForAssembly(vmAssembly, &vmModule));
IfFailThrow(GetProcess()->GetDAC()->GetModuleForAssembly(vmAssembly, &vmModule, NULL));
}

else if (vmAssembly.IsNull())
Expand All @@ -825,33 +825,6 @@ CordbModule* CordbAppDomain::LookupOrCreateModule(VMPTR_Assembly vmAssembly, VMP



//---------------------------------------------------------------------------------------
// Callback invoked by DAC for each module in an assembly. Used to populate RS module cache.
//
// Arguments:
// vmModule - module from enumeration
// pUserData - user data, a 'this' pointer to the CordbAssembly to add to.
//
// Notes:
// This is called from code:CordbAppDomain::PrepopulateModules invoking DAC, which
// invokes this callback.

// static
void CordbAppDomain::ModuleEnumerationCallback(VMPTR_Assembly vmAssembly, void * pUserData)
{
CONTRACTL
{
THROWS;
}
CONTRACTL_END;

CordbAppDomain * pAppDomain = static_cast<CordbAppDomain *> (pUserData);
INTERNAL_DAC_CALLBACK(pAppDomain->GetProcess());

pAppDomain->LookupOrCreateModule(vmAssembly);
}


//
// Use DAC to preopulate the list of modules for this assembly
//
Expand Down Expand Up @@ -881,13 +854,17 @@ void CordbAppDomain::PrepopulateModules()
pAssembly != NULL;
pAssembly = m_assemblies.FindNext(&hashfind))
{
VMPTR_Assembly vmAssembly = pAssembly->GetAssemblyPtr();
VMPTR_Module vmModule = VMPTR_Module::NullPtr();
BOOL isModuleLoaded = FALSE;

// DD-primitive that invokes a callback.
IfFailThrow(GetProcess()->GetDAC()->EnumerateModulesInAssembly(
pAssembly->GetAssemblyPtr(),
CordbAppDomain::ModuleEnumerationCallback,
this)); // user data
IfFailThrow(GetProcess()->GetDAC()->GetModuleForAssembly(vmAssembly, &vmModule, &isModuleLoaded));

if (isModuleLoaded)
{
Comment thread
rcj1 marked this conversation as resolved.
_ASSERTE(!vmModule.IsNull());
LookupOrCreateModule(vmAssembly, vmModule);
}
}
}

Expand Down
18 changes: 0 additions & 18 deletions src/coreclr/debug/di/rspriv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2457,9 +2457,6 @@ class CordbAppDomain : public CordbBase,
// Lookup a module from the cache. Create and to the cache if needed.
CordbModule * LookupOrCreateModule(VMPTR_Assembly vmAssemblyToken, VMPTR_Module vmModuleToken = VMPTR_Module::NullPtr());

// Callback from DAC for module enumeration
static void ModuleEnumerationCallback(VMPTR_Assembly vmAssembly, void * pUserData);

// Use DAC to add any modules for this assembly.
void PrepopulateModules();

Expand Down Expand Up @@ -2785,15 +2782,6 @@ class IProcessShimHooks
virtual void HandleDebugEventForInteropDebugging(const DEBUG_EVENT * pEvent) = 0;
#endif // FEATURE_INTEROP_DEBUGGING

// Get the modules in the order that they were loaded. This is needed to send the fake-attach events
// for module load in the right order.
//
// This can be removed once ICorDebug's enumerations are ordered.
virtual void GetModulesInLoadOrder(
ICorDebugAssembly * pAssembly,
RSExtSmartPtr<ICorDebugModule>* pModules,
ULONG countModules) = 0;

// Get the assemblies in the order that they were loaded. This is needed to send the fake-attach events
// for assembly load in the right order.
//
Expand Down Expand Up @@ -3285,12 +3273,6 @@ class CordbProcess :
RSExtSmartPtr<ICorDebugAssembly>* pAssemblies,
ULONG countAssemblies);

// Callback for Shim to get the modules in load order
void GetModulesInLoadOrder(
ICorDebugAssembly * pAssembly,
RSExtSmartPtr<ICorDebugModule>* pModules,
ULONG countModules);

// Functions to queue fake Connection events on attach.
static void CountConnectionsCallback(DWORD id, LPCWSTR pName, void * pUserData);
static void EnumerateConnectionsCallback(DWORD id, LPCWSTR pName, void * pUserData);
Expand Down
Loading
Loading