Skip to content

Commit

Permalink
Deallocate resources for DacDbiArrayList with a matching deallocator (#…
Browse files Browse the repository at this point in the history
…58791)

Deleted InfoStoreForDbiNew as there were no users.

Added DeleteDbiMemory to deal with destruction of items and proper usage of the corresponding allocator. However, using this custom new here across dll boundaries is risky as the T type will get its destructor called then the array list is destructed. The old implementation that used the CLR allocators didn't call the destructors on the inner elements (we had a potential leak, in practice it wasn't very noticeable - most objects are never deleted). After #55945, the standard array delete expression - as well as this change - call the destructor on every object. Currently, the instantiations of this type are all devoid of special destructors and are largely blittable structs, so this remains working as it was:

- ICorDebugInfo::NativeVarInfo
- GUID
- FieldData
- DebuggerIPCE_TypeArgData
- DebuggerIPCE_ExpandedTypeData
- DebuggerIPCE_BasicTypeData
- DebuggerILToNativeMap
- DacExceptionCallStackData
- CordbType *
- CORDB_ADDRESS
- COR_SEGMENT
- COR_MEMORY_RA
  • Loading branch information
hoyosjs committed Sep 9, 2021
1 parent f7b441d commit 7d42fe2
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 10 deletions.
27 changes: 20 additions & 7 deletions src/coreclr/debug/daccess/dacdbiimpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ IDacDbiInterface::IAllocator * g_pAllocator = NULL;
// return; // DBI will then free this memory.
// }
// ...
// DeleteDbiMemory(p);
// DeleteDbiMemory(p); // DeleteDbiMemory(p, len); if it was an array allocation.
// }
//
// Be very careful when using this on classes since Dbi and DAC may be in
Expand Down Expand Up @@ -155,6 +155,25 @@ template<class T> void DeleteDbiMemory(T *p)
g_pAllocator->Free((BYTE*) p);
}

// Delete memory and invoke dtor for memory allocated with 'operator (forDbi) new[]'
// There's an inherent risk here - where each element's destructor will get called within
// the context of the DAC. If the destructor tries to use the CRT allocator logic expecting
// to hit the DBI's, we could be in trouble. Those objects need to use an export allocator like this.
template<class T> void DeleteDbiArrayMemory(T *p, int count)
{
if (p == NULL)
{
return;
}

for (T *cur = p; cur < p + count; cur++)
{
cur->~T();
}

_ASSERTE(g_pAllocator != NULL);
g_pAllocator->Free((BYTE*) p);
}

//---------------------------------------------------------------------------------------
// Creates the DacDbiInterface object, used by Dbi.
Expand Down Expand Up @@ -818,12 +837,6 @@ SIZE_T DacDbiInterfaceImpl::GetArgCount(MethodDesc * pMD)
return NumArguments;
} //GetArgCount

// Allocator to pass to DebugInfoStores, allocating forDBI
BYTE* InfoStoreForDbiNew(void * pData, size_t cBytes)
{
return new(forDbi) BYTE[cBytes];
}

// Allocator to pass to the debug-info-stores...
BYTE* InfoStoreNew(void * pData, size_t cBytes)
{
Expand Down
13 changes: 11 additions & 2 deletions src/coreclr/debug/di/rspriv.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,16 @@ struct MachineInfo
USHORT m_usPort;
};

#ifdef DACCESS_COMPILE
#error This header cannot be used in the DAC
#endif

extern forDbiWorker forDbi;

// for dbi we just default to new, but we need to have these defined for both dac and dbi
inline void * operator new(size_t lenBytes, const forDbiWorker &)
{
void * result = new BYTE[lenBytes];
void * result = new (nothrow) BYTE[lenBytes];
if (result == NULL)
{
ThrowOutOfMemory();
Expand All @@ -183,7 +187,7 @@ inline void * operator new(size_t lenBytes, const forDbiWorker &)

inline void * operator new[](size_t lenBytes, const forDbiWorker &)
{
void * result = new BYTE[lenBytes];
void * result = new (nothrow) BYTE[lenBytes];
if (result == NULL)
{
ThrowOutOfMemory();
Expand All @@ -198,6 +202,11 @@ void DeleteDbiMemory(T *p)
delete p;
}

template<class T> inline
void DeleteDbiArrayMemory(T *p, int)
{
delete[] p;
}


//---------------------------------------------------------------------------------------
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/debug/inc/dacdbiinterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
// In the DBI implementation, this can directly call delete (assuming the IAllocator::Free
// directly called new).
template<class T> void DeleteDbiMemory(T *p);
template<class T> void DeleteDbiArrayMemory(T *p, int count);
// Need a class to serve as a tag that we can use to overload New/Delete.
class forDbiWorker {};
extern forDbiWorker forDbi;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/debug/inc/dacdbistructures.inl
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ void DacDbiArrayList<T>::Dealloc()

if (m_pList != NULL)
{
delete [] m_pList;
DeleteDbiArrayMemory(m_pList, m_nEntries);
m_pList = NULL;
}
m_nEntries = 0;
Expand Down

0 comments on commit 7d42fe2

Please sign in to comment.