Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
106 changes: 66 additions & 40 deletions src/coreclr/debug/daccess/dacdbiimpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ template<class T> void DeleteDbiArrayMemory(T *p, int count)
// pAllocator - pointer to client allocator object. This lets DD allocate objects and
// pass them out back to the client, which can then delete them.
// DD takes a weak ref to this, so client must keep it alive until it
// calls Destroy.
// calls Release.
// pMetadataLookup - callback interface to do internal metadata lookup. This is because
// metadata is not dac-ized.
// ppInterface - mandatory out-parameter
Expand All @@ -238,7 +238,7 @@ template<class T> void DeleteDbiArrayMemory(T *p, int count)
// This will yield an IDacDbiInterface to provide structured access to the
// data-target.
//
// Must call Destroy to on interface to free its resources.
// Must call Release on interface to free its resources.
//
//---------------------------------------------------------------------------------------
STDAPI
Expand Down Expand Up @@ -295,7 +295,7 @@ DacDbiInterfaceInstance(
// pAllocator - pointer to client allocator object. This lets DD allocate objects and
// pass them out back to the client, which can then delete them.
// DD takes a weak ref to this, so client must keep it alive until it
// calls Destroy.
// calls Release.
// pMetadataLookup - callback interface to do internal metadata lookup. This is because
// metadata is not dac-ized.
//
Expand Down Expand Up @@ -333,7 +333,7 @@ DacDbiInterfaceImpl::DacDbiInterfaceImpl(
// Destructor.
//
// Notes:
// This gets invoked after Destroy().
// This gets invoked when the ref count drops to 0 via Release().
//-----------------------------------------------------------------------------
DacDbiInterfaceImpl::~DacDbiInterfaceImpl()
{
Expand Down Expand Up @@ -441,19 +441,30 @@ interface IMDInternalImport* DacDbiInterfaceImpl::GetMDImport(
// See DacDbiInterface.h for full descriptions of all of these functions
//-----------------------------------------------------------------------------

// Destroy the connection, freeing up any resources.
HRESULT DacDbiInterfaceImpl::Destroy()
// IUnknown implementation for DacDbiInterfaceImpl.
// Delegates to ClrDataAccess's ref-counting and adds support for IDacDbiInterface IID.
STDMETHODIMP
DacDbiInterfaceImpl::QueryInterface(THIS_ IN REFIID interfaceId, OUT PVOID* iface)
{
HRESULT hr = S_OK;
EX_TRY
if (IsEqualIID(interfaceId, __uuidof(IDacDbiInterface)))
{
m_pAllocator = NULL;

this->Release();
// Memory is deleted, don't access this object any more
AddRef();
*iface = static_cast<IDacDbiInterface*>(this);
return S_OK;
}
EX_CATCH_HRESULT(hr);
return hr;
return ClrDataAccess::QueryInterface(interfaceId, iface);
Comment on lines +446 to +455
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DacDbiInterfaceImpl::QueryInterface dereferences iface without a null check. If a caller passes a null out-pointer (or uses it incorrectly), this will AV before you can return an HRESULT. Consider matching COM expectations here by returning E_POINTER when iface is null and (optionally) setting *iface = nullptr before any IID checks.

Copilot uses AI. Check for mistakes.
}

STDMETHODIMP_(ULONG)
DacDbiInterfaceImpl::AddRef(THIS)
{
return ClrDataAccess::AddRef();
}

STDMETHODIMP_(ULONG)
DacDbiInterfaceImpl::Release(THIS)
{
return ClrDataAccess::Release();
}

// Check whether the version of the DBI matches the version of the runtime.
Expand Down Expand Up @@ -495,7 +506,7 @@ HRESULT DacDbiInterfaceImpl::FlushCache()
}

// enable or disable DAC target consistency checks
HRESULT DacDbiInterfaceImpl::DacSetTargetConsistencyChecks(bool fEnableAsserts)
HRESULT DacDbiInterfaceImpl::DacSetTargetConsistencyChecks(BOOL fEnableAsserts)
{
HRESULT hr = S_OK;
EX_TRY
Expand Down Expand Up @@ -1172,8 +1183,11 @@ mdSignature DacDbiInterfaceImpl::GetILCodeAndSigHelper(Module * pModule,
}


HRESULT DacDbiInterfaceImpl::GetMetaDataFileInfoFromPEFile(VMPTR_PEAssembly vmPEAssembly, DWORD & dwTimeStamp, DWORD & dwImageSize, IStringHolder* pStrFilename, OUT bool * pResult)
HRESULT DacDbiInterfaceImpl::GetMetaDataFileInfoFromPEFile(VMPTR_PEAssembly vmPEAssembly, DWORD * pTimeStamp, DWORD * pImageSize, IStringHolder* pStrFilename, OUT BOOL * pResult)
{
if (pTimeStamp == NULL || pImageSize == NULL || pStrFilename == NULL || pResult == NULL)
return E_POINTER;

DD_ENTER_MAY_THROW;

HRESULT hr = S_OK;
Expand All @@ -1186,15 +1200,15 @@ HRESULT DacDbiInterfaceImpl::GetMetaDataFileInfoFromPEFile(VMPTR_PEAssembly vmPE
_ASSERTE(pPEAssembly != NULL);
if (pPEAssembly == NULL)
{
*pResult = false;
*pResult = FALSE;
}
else
{
WCHAR wszFilePath[MAX_LONGPATH] = {0};
DWORD cchFilePath = MAX_LONGPATH;
bool ret = ClrDataAccess::GetMetaDataFileInfoFromPEFile(pPEAssembly,
dwTimeStamp,
dwImageSize,
*pTimeStamp,
*pImageSize,
dwDataSize,
dwRvaHint,
wszFilePath,
Expand Down Expand Up @@ -2743,8 +2757,11 @@ HRESULT DacDbiInterfaceImpl::GetApproxTypeHandle(TypeInfoList * pTypeData, OUT V
// DacDbiInterface API: Get the exact type handle from type data
HRESULT DacDbiInterfaceImpl::GetExactTypeHandle(DebuggerIPCE_ExpandedTypeData * pTypeData,
ArgInfoList * pArgInfo,
VMPTR_TypeHandle& vmTypeHandle)
VMPTR_TypeHandle * pVmTypeHandle)
{
if (pVmTypeHandle == NULL)
return E_POINTER;

DD_ENTER_MAY_THROW;

LOG((LF_CORDB, LL_INFO10000, "D::GETH: getting info.\n"));
Expand All @@ -2753,12 +2770,12 @@ HRESULT DacDbiInterfaceImpl::GetExactTypeHandle(DebuggerIPCE_ExpandedTypeData *

EX_TRY
{
vmTypeHandle = vmTypeHandle.NullPtr();
*pVmTypeHandle = VMPTR_TypeHandle::NullPtr();

Comment on lines 2758 to 2774
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetExactTypeHandle switched from an out reference to an out pointer (pVmTypeHandle) but the implementation immediately dereferences it without validating. To match the rest of DacDbiInterfaceImpl’s COM-style behavior, please add a null check for pVmTypeHandle and return E_POINTER before writing through it.

Copilot uses AI. Check for mistakes.
// convert the type information to a type handle
TypeHandle typeHandle = ExpandedTypeInfoToTypeHandle(pTypeData, pArgInfo);
_ASSERTE(!typeHandle.IsNull());
vmTypeHandle.SetDacTargetPtr(typeHandle.AsTAddr());
pVmTypeHandle->SetDacTargetPtr(typeHandle.AsTAddr());
}
EX_CATCH_HRESULT(hr);

Expand Down Expand Up @@ -3732,8 +3749,11 @@ HRESULT DacDbiInterfaceImpl::GetLoaderHeapMemoryRanges(DacDbiArrayList<COR_MEMOR
return hr;
}

HRESULT DacDbiInterfaceImpl::GetStackFramesFromException(VMPTR_Object vmObject, DacDbiArrayList<DacExceptionCallStackData>& dacStackFrames)
HRESULT DacDbiInterfaceImpl::GetStackFramesFromException(VMPTR_Object vmObject, DacDbiArrayList<DacExceptionCallStackData>* pDacStackFrames)
{
if (pDacStackFrames == NULL)
return E_POINTER;

DD_ENTER_MAY_THROW;

HRESULT hr = S_OK;
Expand Down Expand Up @@ -3761,12 +3781,12 @@ HRESULT DacDbiInterfaceImpl::GetStackFramesFromException(VMPTR_Object vmObject,

if (dacStackFramesLength > 0)
{
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetStackFramesFromException changed its DacDbiArrayList parameter from a reference to a pointer, but the implementation dereferences pDacStackFrames unconditionally (Alloc / operator[]). This introduces a null-deref crash path; validate pDacStackFrames (and any other required pointer params) and return E_POINTER when null.

Suggested change
{
{
if (pDacStackFrames == NULL)
{
ThrowHR(E_POINTER);
}

Copilot uses AI. Check for mistakes.
dacStackFrames.Alloc(dacStackFramesLength);
pDacStackFrames->Alloc(dacStackFramesLength);

for (INT32 index = 0; index < dacStackFramesLength; ++index)
{
DebugStackTrace::Element const& currentElement = stackFramesData.pElements[index];
DacExceptionCallStackData& currentFrame = dacStackFrames[index];
DacExceptionCallStackData& currentFrame = (*pDacStackFrames)[index];

AppDomain* pDomain = AppDomain::GetCurrentDomain();
_ASSERTE(pDomain != NULL);
Expand Down Expand Up @@ -3882,8 +3902,11 @@ HRESULT DacDbiInterfaceImpl::GetRcwCachedInterfacePointers(VMPTR_Object vmObject
#endif // FEATURE_COMINTEROP
}

HRESULT DacDbiInterfaceImpl::GetCachedWinRTTypesForIIDs(VMPTR_AppDomain vmAppDomain, DacDbiArrayList<GUID> & iids, OUT DacDbiArrayList<DebuggerIPCE_ExpandedTypeData> * pTypes)
HRESULT DacDbiInterfaceImpl::GetCachedWinRTTypesForIIDs(VMPTR_AppDomain vmAppDomain, DacDbiArrayList<GUID> * pIids, OUT DacDbiArrayList<DebuggerIPCE_ExpandedTypeData> * pTypes)
{
if (pIids == NULL || pTypes == NULL)
return E_POINTER;

HRESULT hr = S_OK;
EX_TRY
{
Expand Down Expand Up @@ -4319,7 +4342,7 @@ HRESULT DacDbiInterfaceImpl::IsModuleMapped(VMPTR_Module pModule, OUT BOOL *isMo
return hr;
}

HRESULT DacDbiInterfaceImpl::MetadataUpdatesApplied(OUT bool * pResult)
HRESULT DacDbiInterfaceImpl::MetadataUpdatesApplied(OUT BOOL * pResult)
{
DD_ENTER_MAY_THROW;

Expand All @@ -4329,7 +4352,7 @@ HRESULT DacDbiInterfaceImpl::MetadataUpdatesApplied(OUT bool * pResult)
#ifdef FEATURE_METADATA_UPDATER
*pResult = g_metadataUpdatesApplied;
#else
*pResult = false;
*pResult = FALSE;
#endif
}
EX_CATCH_HRESULT(hr);
Expand Down Expand Up @@ -4843,7 +4866,7 @@ HRESULT DacDbiInterfaceImpl::EnumerateThreads(FP_THREAD_ENUMERATION_CALLBACK fpC
}

// public implementation of IsThreadMarkedDead
HRESULT DacDbiInterfaceImpl::IsThreadMarkedDead(VMPTR_Thread vmThread, OUT bool * pResult)
HRESULT DacDbiInterfaceImpl::IsThreadMarkedDead(VMPTR_Thread vmThread, OUT BOOL * pResult)
{
DD_ENTER_MAY_THROW;

Expand Down Expand Up @@ -6237,12 +6260,15 @@ HRESULT DacDbiInterfaceImpl::IsVmObjectHandleValid(VMPTR_OBJECTHANDLE vmHandle,
}

// determines if the specified module is a WinRT module
HRESULT DacDbiInterfaceImpl::IsWinRTModule(VMPTR_Module vmModule, BOOL& isWinRT)
HRESULT DacDbiInterfaceImpl::IsWinRTModule(VMPTR_Module vmModule, BOOL * pIsWinRT)
{
if (pIsWinRT == NULL)
return E_POINTER;

DD_ENTER_MAY_THROW;

HRESULT hr = S_OK;
isWinRT = FALSE;
*pIsWinRT = FALSE;

return hr;
Comment on lines +6263 to 6273
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IsWinRTModule changed from an out reference to an out pointer (pIsWinRT) but still writes to it unconditionally. Add a null check and return E_POINTER when pIsWinRT is null (consistent with other APIs in this layer that validate out pointers).

Copilot uses AI. Check for mistakes.
}
Expand Down Expand Up @@ -6723,12 +6749,12 @@ HRESULT DacDbiInterfaceImpl::EnumerateMonitorEventWaitList(VMPTR_Object vmObject
}


HRESULT DacDbiInterfaceImpl::AreGCStructuresValid(OUT bool * pResult)
HRESULT DacDbiInterfaceImpl::AreGCStructuresValid(OUT BOOL * pResult)
{
HRESULT hr = S_OK;
EX_TRY
{
*pResult = true;
*pResult = TRUE;
}
EX_CATCH_HRESULT(hr);
return hr;
Expand Down Expand Up @@ -7447,15 +7473,15 @@ HRESULT DacDbiInterfaceImpl::GetHeapSegments(OUT DacDbiArrayList<COR_SEGMENT> *p
return hr;
}

HRESULT DacDbiInterfaceImpl::IsValidObject(CORDB_ADDRESS obj, OUT bool * pResult)
HRESULT DacDbiInterfaceImpl::IsValidObject(CORDB_ADDRESS obj, OUT BOOL * pResult)
{
DD_ENTER_MAY_THROW;

HRESULT hr = S_OK;
EX_TRY
{

bool isValid = false;
BOOL isValid = FALSE;

if (obj != 0 && obj != (CORDB_ADDRESS)-1)
{
Expand All @@ -7467,13 +7493,13 @@ HRESULT DacDbiInterfaceImpl::IsValidObject(CORDB_ADDRESS obj, OUT bool * pResult
PTR_EEClass cls = mt->GetClass();

if (mt == cls->GetMethodTable())
isValid = true;
isValid = TRUE;
else if (!mt->IsCanonicalMethodTable() || mt->IsContinuation())
isValid = cls->GetMethodTable()->GetClass() == cls;
}
EX_CATCH
{
isValid = false;
isValid = FALSE;
}
EX_END_CATCH
}
Expand All @@ -7484,7 +7510,7 @@ HRESULT DacDbiInterfaceImpl::IsValidObject(CORDB_ADDRESS obj, OUT bool * pResult
return hr;
}

HRESULT DacDbiInterfaceImpl::GetAppDomainForObject(CORDB_ADDRESS obj, OUT VMPTR_AppDomain * pApp, OUT VMPTR_Module * pModule, OUT VMPTR_DomainAssembly * pDomainAssembly, OUT bool * pResult)
HRESULT DacDbiInterfaceImpl::GetAppDomainForObject(CORDB_ADDRESS obj, OUT VMPTR_AppDomain * pApp, OUT VMPTR_Module * pModule, OUT VMPTR_DomainAssembly * pDomainAssembly, OUT BOOL * pResult)
{
DD_ENTER_MAY_THROW;

Expand All @@ -7494,7 +7520,7 @@ HRESULT DacDbiInterfaceImpl::GetAppDomainForObject(CORDB_ADDRESS obj, OUT VMPTR_

if (obj == 0 || obj == (CORDB_ADDRESS)-1)
{
*pResult = false;
*pResult = FALSE;
}
else
{
Expand All @@ -7506,7 +7532,7 @@ HRESULT DacDbiInterfaceImpl::GetAppDomainForObject(CORDB_ADDRESS obj, OUT VMPTR_
pModule->SetDacTargetPtr(PTR_HOST_TO_TADDR(module));
pDomainAssembly->SetDacTargetPtr(PTR_HOST_TO_TADDR(module->GetDomainAssembly()));

*pResult = true;
*pResult = TRUE;
}
}
EX_CATCH_HRESULT(hr);
Expand Down
Loading
Loading