Skip to content

Commit 9d4ceae

Browse files
Copilotrcj1Copilot
authored
Replace EnumerateModulesInAssembly with combined GetModuleForAssembly; implement in cDAC (#128294)
* Implement GetAssemblyFromModule in cDAC * Fold EnumerateModulesInAssembly into an IsLoaded check in GetModuleForAssembly --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com> Co-authored-by: Rachel Jarvi <rachel.jarvi@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent b6e38ad commit 9d4ceae

11 files changed

Lines changed: 154 additions & 321 deletions

File tree

src/coreclr/debug/daccess/dacdbiimpl.cpp

Lines changed: 21 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -4162,7 +4162,7 @@ HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::GetSymbolsBuffer(VMPTR_Module vmM
41624162

41634163

41644164

4165-
HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::GetModuleForAssembly(VMPTR_Assembly vmAssembly, OUT VMPTR_Module * pModule)
4165+
HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::GetModuleForAssembly(VMPTR_Assembly vmAssembly, OUT VMPTR_Module * pModule, OUT BOOL * pIsModuleLoaded)
41664166
{
41674167
DD_ENTER_MAY_THROW;
41684168

@@ -4172,8 +4172,26 @@ HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::GetModuleForAssembly(VMPTR_Assemb
41724172

41734173
_ASSERTE(pModule != NULL);
41744174

4175+
// Initialize out params up front so callers don't observe stale/uninitialized
4176+
// values if we throw below.
4177+
*pModule = VMPTR_Module::NullPtr();
4178+
if (pIsModuleLoaded != NULL)
4179+
{
4180+
*pIsModuleLoaded = FALSE;
4181+
}
4182+
41754183
Assembly * pAssembly = vmAssembly.GetDacPtr();
4184+
if (pAssembly == NULL)
4185+
{
4186+
ThrowHR(E_INVALIDARG);
4187+
}
4188+
41764189
pModule->SetHostPtr(pAssembly->GetModule());
4190+
4191+
if (pIsModuleLoaded != NULL)
4192+
{
4193+
*pIsModuleLoaded = pAssembly->IsLoaded() ? TRUE : FALSE;
4194+
}
41774195
}
41784196
EX_CATCH_HRESULT(hr);
41794197
return hr;
@@ -4226,7 +4244,8 @@ HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::GetModuleData(VMPTR_Module vmModu
42264244
}
42274245

42284246

4229-
// Enumerate all Assemblies in an appdomain.
4247+
// Implementation of IDacDbiInterface::EnumerateAssembliesInAppDomain.
4248+
// Enumerate all the assemblies in the appdomain.
42304249
HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::EnumerateAssembliesInAppDomain(VMPTR_AppDomain vmAppDomain, FP_ASSEMBLY_ENUMERATION_CALLBACK fpCallback, CALLBACK_DATA pUserData)
42314250
{
42324251
DD_ENTER_MAY_THROW;
@@ -4240,9 +4259,6 @@ HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::EnumerateAssembliesInAppDomain(VM
42404259
// Iterate through all Assemblies (including shared) in the appdomain.
42414260
AppDomain::AssemblyIterator iterator;
42424261

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

42484264
if (pAppDomain == nullptr)
@@ -4266,30 +4282,6 @@ HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::EnumerateAssembliesInAppDomain(VM
42664282
return hr;
42674283
}
42684284

4269-
// Implementation of IDacDbiInterface::EnumerateModulesInAssembly,
4270-
// Enumerate all the modules (non-resource) in an assembly.
4271-
HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::EnumerateModulesInAssembly(VMPTR_Assembly vmAssembly, FP_MODULE_ENUMERATION_CALLBACK fpCallback, CALLBACK_DATA pUserData)
4272-
{
4273-
DD_ENTER_MAY_THROW;
4274-
4275-
HRESULT hr = S_OK;
4276-
EX_TRY
4277-
{
4278-
4279-
_ASSERTE(fpCallback != NULL);
4280-
4281-
Assembly * pAssembly = vmAssembly.GetDacPtr();
4282-
4283-
// If assembly isn't yet loaded, just return
4284-
if (!pAssembly->IsLoaded())
4285-
return hr;
4286-
4287-
fpCallback(vmAssembly, pUserData);
4288-
}
4289-
EX_CATCH_HRESULT(hr);
4290-
return hr;
4291-
}
4292-
42934285
// Implementation of IDacDbiInterface::ResolveAssembly
42944286
// Returns NULL if not found.
42954287
HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::ResolveAssembly(VMPTR_Assembly vmScope, mdToken tkAssemblyRef, OUT VMPTR_Assembly * pRetVal)

src/coreclr/debug/daccess/dacdbiimpl.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -598,7 +598,7 @@ class DacDbiInterfaceImpl :
598598
// Gets properties for a module
599599
HRESULT STDMETHODCALLTYPE GetModuleData(VMPTR_Module vmModule, OUT ModuleInfo * pData);
600600

601-
HRESULT STDMETHODCALLTYPE GetModuleForAssembly(VMPTR_Assembly vmAssembly, OUT VMPTR_Module * pModule);
601+
HRESULT STDMETHODCALLTYPE GetModuleForAssembly(VMPTR_Assembly vmAssembly, OUT VMPTR_Module * pModule, OUT BOOL * pIsModuleLoaded);
602602

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

610-
// Enumerate the moduels in the given assembly.
611-
HRESULT STDMETHODCALLTYPE EnumerateModulesInAssembly(VMPTR_Assembly vmAssembly, FP_MODULE_ENUMERATION_CALLBACK fpCallback, CALLBACK_DATA pUserData);
612-
613610
// When stopped at an event, request a synchronization.
614611
HRESULT STDMETHODCALLTYPE RequestSyncAtEvent();
615612

src/coreclr/debug/di/process.cpp

Lines changed: 1 addition & 151 deletions
Original file line numberDiff line numberDiff line change
@@ -4357,156 +4357,6 @@ void CordbProcess::GetAssembliesInLoadOrder(
43574357
// pAssemblies array has now been updated.
43584358
}
43594359

4360-
// Callback data for code:CordbProcess::GetModulesInLoadOrder
4361-
class ShimModuleCallbackData
4362-
{
4363-
public:
4364-
// Ctor to initialize callback data
4365-
//
4366-
// Arguments:
4367-
// pAssembly - assembly that the Modules are in.
4368-
// pModules - preallocated array of smart pointers to hold Modules
4369-
// countModules - size of pModules in elements.
4370-
ShimModuleCallbackData(
4371-
CordbAssembly * pAssembly,
4372-
RSExtSmartPtr<ICorDebugModule>* pModules,
4373-
ULONG countModules)
4374-
{
4375-
_ASSERTE(pAssembly != NULL);
4376-
_ASSERTE(pModules != NULL);
4377-
4378-
m_pProcess = pAssembly->GetAppDomain()->GetProcess();
4379-
m_pAssembly = pAssembly;
4380-
m_pModules = pModules;
4381-
m_countElements = countModules;
4382-
m_index = 0;
4383-
4384-
// Just to be safe, clear them all out
4385-
for(ULONG i = 0; i < countModules; i++)
4386-
{
4387-
pModules[i].Clear();
4388-
}
4389-
}
4390-
4391-
// Dtor
4392-
//
4393-
// Notes:
4394-
// This can assert end-of-enumeration invariants.
4395-
~ShimModuleCallbackData()
4396-
{
4397-
// Ensure that we went through all Modules.
4398-
_ASSERTE(m_index == m_countElements);
4399-
}
4400-
4401-
// Callback invoked from DAC enumeration.
4402-
//
4403-
// arguments:
4404-
// vmAssembly - VMPTR for Assembly
4405-
// pData - a 'this' pointer
4406-
//
4407-
static void Callback(VMPTR_Assembly vmAssembly, void * pData)
4408-
{
4409-
ShimModuleCallbackData * pThis = static_cast<ShimModuleCallbackData *> (pData);
4410-
INTERNAL_DAC_CALLBACK(pThis->m_pProcess);
4411-
4412-
CordbModule * pModule = pThis->m_pAssembly->GetAppDomain()->LookupOrCreateModule(vmAssembly);
4413-
4414-
pThis->SetAndMoveNext(pModule);
4415-
}
4416-
4417-
// Set the current index in the table and increment the cursor.
4418-
//
4419-
// Arguments:
4420-
// pModule - Module from DAC enumerator
4421-
void SetAndMoveNext(CordbModule * pModule)
4422-
{
4423-
_ASSERTE(pModule != NULL);
4424-
4425-
if (m_index >= m_countElements)
4426-
{
4427-
// Enumerating the Modules in the target should be fixed since
4428-
// the target is not running.
4429-
// We should never get here unless the target is unstable.
4430-
// The caller (the shim) pre-allocated the table of Modules.
4431-
m_pProcess->TargetConsistencyCheck(!"Target changed Module count");
4432-
return;
4433-
}
4434-
4435-
m_pModules[m_index].Assign(pModule);
4436-
m_index++;
4437-
}
4438-
4439-
protected:
4440-
CordbProcess * m_pProcess;
4441-
CordbAssembly * m_pAssembly;
4442-
RSExtSmartPtr<ICorDebugModule>* m_pModules;
4443-
ULONG m_countElements;
4444-
ULONG m_index;
4445-
};
4446-
4447-
//---------------------------------------------------------------------------------------
4448-
// Shim Helper to enumerate the Modules in the load-order
4449-
//
4450-
// Arguments:
4451-
// pAppdomain - non-null appdomain to enumerate Modules.
4452-
// pModules - caller pre-allocated array to hold Modules
4453-
// countModules - size of the array.
4454-
//
4455-
// Notes:
4456-
// Caller preallocated array (likely from ICorDebugModuleEnum::GetCount),
4457-
// and now this function fills in the Modules in the order they were
4458-
// loaded.
4459-
//
4460-
// The target should be stable, such that the number of Modules in the
4461-
// target is stable, and therefore countModules as determined by the
4462-
// shim via ICorDebugModuleEnum::GetCount should match the number of
4463-
// Modules enumerated here.
4464-
//
4465-
// Called by code:ShimProcess::QueueFakeAssemblyAndModuleEvent.
4466-
// This provides the Modules in load-order. In contrast,
4467-
// ICorDebugAssembly::EnumerateModules is a random order. The shim needs
4468-
// load-order to match Whidbey semantics for dispatching fake load-Module
4469-
// callbacks on attach. The most important thing is that the manifest module
4470-
// gets a LodModule callback before any secondary modules. For dynamic
4471-
// modules, this is necessary for operations on the secondary module
4472-
// that rely on manifest metadata (eg. GetSimpleName).
4473-
//
4474-
// @dbgtodo : This is almost identical to GetAssembliesInLoadOrder, and
4475-
// (together wih the CallbackData classes) seems a HUGE amount of code and
4476-
// complexity for such a simple thing. We also have extra code to order
4477-
// AppDomains and Threads. We should try and rip all of this extra complexity
4478-
// out, and replace it with better data structures for storing these items.
4479-
// Eg., if we used std::map, we could have efficient lookups and ordered
4480-
// enumerations. However, we do need to be careful about exposing new invariants
4481-
// through ICorDebug that customers may depend on, which could place a long-term
4482-
// compatibility burden on us. We could have a simple generic data structure
4483-
// (eg. built on std::hash_map and std::list) which provided efficient look-up
4484-
// and both in-order and random enumeration.
4485-
//
4486-
void CordbProcess::GetModulesInLoadOrder(
4487-
ICorDebugAssembly * pAssembly,
4488-
RSExtSmartPtr<ICorDebugModule>* pModules,
4489-
ULONG countModules)
4490-
{
4491-
PUBLIC_API_ENTRY_FOR_SHIM(this);
4492-
RSLockHolder lockHolder(GetProcessLock());
4493-
4494-
_ASSERTE(GetShim() != NULL);
4495-
4496-
CordbAssembly * pAssemblyInternal = static_cast<CordbAssembly *> (pAssembly);
4497-
4498-
ShimModuleCallbackData data(pAssemblyInternal, pModules, countModules);
4499-
4500-
// Enumerate through and fill out pModules table.
4501-
IfFailThrow(GetDAC()->EnumerateModulesInAssembly(
4502-
pAssemblyInternal->GetAssemblyPtr(),
4503-
ShimModuleCallbackData::Callback,
4504-
&data)); // user data
4505-
4506-
// pModules array has now been updated.
4507-
}
4508-
4509-
45104360
//---------------------------------------------------------------------------------------
45114361
// Callback to count the number of enumerations in a process.
45124362
//
@@ -14748,7 +14598,7 @@ CordbClass * CordbProcess::LookupClass(ICorDebugAppDomain * pAppDomain, VMPTR_As
1474814598
if (pAppDomain != NULL)
1474914599
{
1475014600
VMPTR_Module vmModule = VMPTR_Module::NullPtr();
14751-
IfFailThrow(GetProcess()->GetDAC()->GetModuleForAssembly(vmAssembly, &vmModule));
14601+
IfFailThrow(GetProcess()->GetDAC()->GetModuleForAssembly(vmAssembly, &vmModule, NULL));
1475214602
_ASSERTE(!vmModule.IsNull());
1475314603
CordbModule * pModule = ((CordbAppDomain *)pAppDomain)->m_modules.GetBase(VmPtrToCookie(vmModule));
1475414604
if (pModule != NULL)

src/coreclr/debug/di/rsappdomain.cpp

Lines changed: 10 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -799,7 +799,7 @@ CordbModule* CordbAppDomain::LookupOrCreateModule(VMPTR_Assembly vmAssembly, VMP
799799

800800
if (vmModule.IsNull())
801801
{
802-
IfFailThrow(GetProcess()->GetDAC()->GetModuleForAssembly(vmAssembly, &vmModule));
802+
IfFailThrow(GetProcess()->GetDAC()->GetModuleForAssembly(vmAssembly, &vmModule, NULL));
803803
}
804804

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

826826

827827

828-
//---------------------------------------------------------------------------------------
829-
// Callback invoked by DAC for each module in an assembly. Used to populate RS module cache.
830-
//
831-
// Arguments:
832-
// vmModule - module from enumeration
833-
// pUserData - user data, a 'this' pointer to the CordbAssembly to add to.
834-
//
835-
// Notes:
836-
// This is called from code:CordbAppDomain::PrepopulateModules invoking DAC, which
837-
// invokes this callback.
838-
839-
// static
840-
void CordbAppDomain::ModuleEnumerationCallback(VMPTR_Assembly vmAssembly, void * pUserData)
841-
{
842-
CONTRACTL
843-
{
844-
THROWS;
845-
}
846-
CONTRACTL_END;
847-
848-
CordbAppDomain * pAppDomain = static_cast<CordbAppDomain *> (pUserData);
849-
INTERNAL_DAC_CALLBACK(pAppDomain->GetProcess());
850-
851-
pAppDomain->LookupOrCreateModule(vmAssembly);
852-
}
853-
854-
855828
//
856829
// Use DAC to preopulate the list of modules for this assembly
857830
//
@@ -881,13 +854,17 @@ void CordbAppDomain::PrepopulateModules()
881854
pAssembly != NULL;
882855
pAssembly = m_assemblies.FindNext(&hashfind))
883856
{
857+
VMPTR_Assembly vmAssembly = pAssembly->GetAssemblyPtr();
858+
VMPTR_Module vmModule = VMPTR_Module::NullPtr();
859+
BOOL isModuleLoaded = FALSE;
884860

885-
// DD-primitive that invokes a callback.
886-
IfFailThrow(GetProcess()->GetDAC()->EnumerateModulesInAssembly(
887-
pAssembly->GetAssemblyPtr(),
888-
CordbAppDomain::ModuleEnumerationCallback,
889-
this)); // user data
861+
IfFailThrow(GetProcess()->GetDAC()->GetModuleForAssembly(vmAssembly, &vmModule, &isModuleLoaded));
890862

863+
if (isModuleLoaded)
864+
{
865+
_ASSERTE(!vmModule.IsNull());
866+
LookupOrCreateModule(vmAssembly, vmModule);
867+
}
891868
}
892869
}
893870

src/coreclr/debug/di/rspriv.h

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2504,9 +2504,6 @@ class CordbAppDomain : public CordbBase,
25042504
// Lookup a module from the cache. Create and to the cache if needed.
25052505
CordbModule * LookupOrCreateModule(VMPTR_Assembly vmAssemblyToken, VMPTR_Module vmModuleToken = VMPTR_Module::NullPtr());
25062506

2507-
// Callback from DAC for module enumeration
2508-
static void ModuleEnumerationCallback(VMPTR_Assembly vmAssembly, void * pUserData);
2509-
25102507
// Use DAC to add any modules for this assembly.
25112508
void PrepopulateModules();
25122509

@@ -2832,15 +2829,6 @@ class IProcessShimHooks
28322829
virtual void HandleDebugEventForInteropDebugging(const DEBUG_EVENT * pEvent) = 0;
28332830
#endif // FEATURE_INTEROP_DEBUGGING
28342831

2835-
// Get the modules in the order that they were loaded. This is needed to send the fake-attach events
2836-
// for module load in the right order.
2837-
//
2838-
// This can be removed once ICorDebug's enumerations are ordered.
2839-
virtual void GetModulesInLoadOrder(
2840-
ICorDebugAssembly * pAssembly,
2841-
RSExtSmartPtr<ICorDebugModule>* pModules,
2842-
ULONG countModules) = 0;
2843-
28442832
// Get the assemblies in the order that they were loaded. This is needed to send the fake-attach events
28452833
// for assembly load in the right order.
28462834
//
@@ -3332,12 +3320,6 @@ class CordbProcess :
33323320
RSExtSmartPtr<ICorDebugAssembly>* pAssemblies,
33333321
ULONG countAssemblies);
33343322

3335-
// Callback for Shim to get the modules in load order
3336-
void GetModulesInLoadOrder(
3337-
ICorDebugAssembly * pAssembly,
3338-
RSExtSmartPtr<ICorDebugModule>* pModules,
3339-
ULONG countModules);
3340-
33413323
// Functions to queue fake Connection events on attach.
33423324
static void CountConnectionsCallback(DWORD id, LPCWSTR pName, void * pUserData);
33433325
static void EnumerateConnectionsCallback(DWORD id, LPCWSTR pName, void * pUserData);

0 commit comments

Comments
 (0)