Skip to content

Commit

Permalink
SPMI: Fix a few memory leaks (#104691)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jakobbotsch
jakobbotsch authored Jul 16, 2024
1 parent f1d9579 commit 92abb8c
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 28 deletions.
14 changes: 10 additions & 4 deletions src/coreclr/tools/superpmi/superpmi-shared/compileresult.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,18 @@ bool CompileResult::IsEmpty()
return isEmpty;
}

// Allocate memory associated with this CompileResult. Keep track of it in a list so we can free it all later.
void* CompileResult::allocateMemory(size_t sizeInBytes)
MemoryTracker* CompileResult::getOrCreateMemoryTracker()
{
if (memoryTracker == nullptr)
memoryTracker = new MemoryTracker();
return memoryTracker->allocate(sizeInBytes);

return memoryTracker;
}

// Allocate memory associated with this CompileResult. Keep track of it in a list so we can free it all later.
void* CompileResult::allocateMemory(size_t sizeInBytes)
{
return getOrCreateMemoryTracker()->allocate(sizeInBytes);
}

void CompileResult::recAssert(const char* assertText)
Expand Down Expand Up @@ -1259,7 +1265,7 @@ bool CompileResult::fndRecordCallSiteSigInfo(ULONG instrOffset, CORINFO_SIG_INFO
if (value.callSig.callConv == (DWORD)-1)
return false;

*pCallSig = SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(value.callSig, RecordCallSiteWithSignature, CrSigInstHandleMap);
*pCallSig = SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(value.callSig, RecordCallSiteWithSignature, CrSigInstHandleMap, getOrCreateMemoryTracker());

return true;
}
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/tools/superpmi/superpmi-shared/compileresult.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ class CompileResult
// not persisted to disk.
public:
LightWeightMap<DWORDLONG, DWORD>* CallTargetTypes;
MemoryTracker* getOrCreateMemoryTracker();

private:
MemoryTracker* memoryTracker;
Expand Down
16 changes: 8 additions & 8 deletions src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -696,8 +696,8 @@ void MethodContext::repCompileMethod(CORINFO_METHOD_INFO* info, unsigned* flags,
info->options = (CorInfoOptions)value.info.options;
info->regionKind = (CorInfoRegionKind)value.info.regionKind;

info->args = SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(value.info.args, CompileMethod, SigInstHandleMap);
info->locals = SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(value.info.locals, CompileMethod, SigInstHandleMap);
info->args = SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(value.info.args, CompileMethod, SigInstHandleMap, cr->getOrCreateMemoryTracker());
info->locals = SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(value.info.locals, CompileMethod, SigInstHandleMap, cr->getOrCreateMemoryTracker());

*flags = (unsigned)value.flags;
*os = (CORINFO_OS)value.os;
Expand Down Expand Up @@ -1572,7 +1572,7 @@ void MethodContext::repGetCallInfo(CORINFO_RESOLVED_TOKEN* pResolvedToken,
pResult->methodFlags |= CORINFO_FLG_DONT_INLINE;

pResult->classFlags = (unsigned)value.classFlags;
pResult->sig = SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(value.sig, GetCallInfo, SigInstHandleMap);
pResult->sig = SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(value.sig, GetCallInfo, SigInstHandleMap, cr->getOrCreateMemoryTracker());
pResult->accessAllowed = (CorInfoIsAccessAllowedResult)value.accessAllowed;
pResult->callsiteCalloutHelper.helperNum = (CorInfoHelpFunc)value.callsiteCalloutHelper.helperNum;
pResult->callsiteCalloutHelper.numArgs = (unsigned)value.callsiteCalloutHelper.numArgs;
Expand Down Expand Up @@ -2905,7 +2905,7 @@ void MethodContext::repGetMethodSig(CORINFO_METHOD_HANDLE ftn, CORINFO_SIG_INFO*

DEBUG_REP(dmpGetMethodSig(key, value));

*sig = SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(value, GetMethodSig, SigInstHandleMap);
*sig = SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(value, GetMethodSig, SigInstHandleMap, cr->getOrCreateMemoryTracker());
}

void MethodContext::recGetArgClass(CORINFO_SIG_INFO* sig,
Expand Down Expand Up @@ -3060,8 +3060,8 @@ bool MethodContext::repGetMethodInfo(CORINFO_METHOD_HANDLE ftn, CORINFO_METHOD_I
info->options = (CorInfoOptions)value.info.options;
info->regionKind = (CorInfoRegionKind)value.info.regionKind;

info->args = SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(value.info.args, GetMethodInfo, SigInstHandleMap);
info->locals = SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(value.info.locals, GetMethodInfo, SigInstHandleMap);
info->args = SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(value.info.args, GetMethodInfo, SigInstHandleMap, cr->getOrCreateMemoryTracker());
info->locals = SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(value.info.locals, GetMethodInfo, SigInstHandleMap, cr->getOrCreateMemoryTracker());
}
bool result = value.result;
*exceptionCode = (DWORD)value.exceptionCode;
Expand Down Expand Up @@ -4320,7 +4320,7 @@ void MethodContext::repFindSig(CORINFO_MODULE_HANDLE moduleHandle,

DEBUG_REP(dmpFindSig(key, value));

*sig = SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(value, FindSig, SigInstHandleMap);
*sig = SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(value, FindSig, SigInstHandleMap, cr->getOrCreateMemoryTracker());
}

void MethodContext::recGetEEInfo(CORINFO_EE_INFO* pEEInfoOut)
Expand Down Expand Up @@ -5388,7 +5388,7 @@ void MethodContext::repFindCallSiteSig(CORINFO_MODULE_HANDLE module,

DEBUG_REP(dmpFindCallSiteSig(key, value));

*sig = SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(value, FindCallSiteSig, SigInstHandleMap);
*sig = SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(value, FindCallSiteSig, SigInstHandleMap, cr->getOrCreateMemoryTracker());
}

void MethodContext::recGetMethodSync(CORINFO_METHOD_HANDLE ftn, void** ppIndirection, void* result)
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -920,7 +920,6 @@ class MethodContext
}

CompileResult* cr;
CompileResult* originalCR;
int index;
bool ignoreStoredConfig;

Expand Down
21 changes: 13 additions & 8 deletions src/coreclr/tools/superpmi/superpmi-shared/spmirecordhelper.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,19 +77,22 @@ class SpmiRecordsHelper
DWORD handleInstCount,
DWORD handleInstIndex,
const DenseLightWeightMap<DWORDLONG>* handleMap,
MemoryTracker* memoryTracker,
/* OUT */ unsigned* handleInstCountOut,
/* OUT */ CORINFO_CLASS_HANDLE** handleInstArrayOut);

static void DeserializeCORINFO_SIG_INST(
CORINFO_SIG_INFO& sigInfoOut,
const Agnostic_CORINFO_SIG_INFO& sigInfo,
const DenseLightWeightMap<DWORDLONG>* handleMap);
const DenseLightWeightMap<DWORDLONG>* handleMap,
MemoryTracker* memoryTracker);

template <typename key, typename value>
static CORINFO_SIG_INFO Restore_CORINFO_SIG_INFO(
const Agnostic_CORINFO_SIG_INFO& sigInfo,
LightWeightMap<key, value>* buffers,
const DenseLightWeightMap<DWORDLONG>* handleMap);
const DenseLightWeightMap<DWORDLONG>* handleMap,
MemoryTracker* memoryTracker);

static Agnostic_CORINFO_LOOKUP_KIND CreateAgnostic_CORINFO_LOOKUP_KIND(
const CORINFO_LOOKUP_KIND* pGenericLookupKind);
Expand Down Expand Up @@ -381,14 +384,15 @@ inline void SpmiRecordsHelper::DeserializeCORINFO_SIG_INST_HandleArray(
DWORD handleInstCount,
DWORD handleInstIndex,
const DenseLightWeightMap<DWORDLONG>* handleMap,
MemoryTracker* memoryTracker,
/* OUT */ unsigned* handleInstCountOut,
/* OUT */ CORINFO_CLASS_HANDLE** handleInstArrayOut)
{
CORINFO_CLASS_HANDLE* handleInstArray;

if (handleInstCount > 0)
{
handleInstArray = new CORINFO_CLASS_HANDLE[handleInstCount]; // memory leak?
handleInstArray = (CORINFO_CLASS_HANDLE*)memoryTracker->allocate(handleInstCount * sizeof(CORINFO_CLASS_HANDLE));
for (unsigned int i = 0; i < handleInstCount; i++)
{
DWORD key = handleInstIndex + i;
Expand All @@ -405,16 +409,17 @@ inline void SpmiRecordsHelper::DeserializeCORINFO_SIG_INST_HandleArray(
}

inline void SpmiRecordsHelper::DeserializeCORINFO_SIG_INST(
CORINFO_SIG_INFO& sigInfoOut, const Agnostic_CORINFO_SIG_INFO& sigInfo, const DenseLightWeightMap<DWORDLONG>* handleMap)
CORINFO_SIG_INFO& sigInfoOut, const Agnostic_CORINFO_SIG_INFO& sigInfo, const DenseLightWeightMap<DWORDLONG>* handleMap, MemoryTracker* memoryTracker)
{
DeserializeCORINFO_SIG_INST_HandleArray(sigInfo.sigInst_classInstCount, sigInfo.sigInst_classInst_Index, handleMap, &sigInfoOut.sigInst.classInstCount, &sigInfoOut.sigInst.classInst);
DeserializeCORINFO_SIG_INST_HandleArray(sigInfo.sigInst_methInstCount, sigInfo.sigInst_methInst_Index, handleMap, &sigInfoOut.sigInst.methInstCount, &sigInfoOut.sigInst.methInst);
DeserializeCORINFO_SIG_INST_HandleArray(sigInfo.sigInst_classInstCount, sigInfo.sigInst_classInst_Index, handleMap, memoryTracker, &sigInfoOut.sigInst.classInstCount, &sigInfoOut.sigInst.classInst);
DeserializeCORINFO_SIG_INST_HandleArray(sigInfo.sigInst_methInstCount, sigInfo.sigInst_methInst_Index, handleMap, memoryTracker, &sigInfoOut.sigInst.methInstCount, &sigInfoOut.sigInst.methInst);
}

template <typename key, typename value>
inline CORINFO_SIG_INFO SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(const Agnostic_CORINFO_SIG_INFO& sigInfo,
LightWeightMap<key, value>* buffers,
const DenseLightWeightMap<DWORDLONG>* handleMap)
const DenseLightWeightMap<DWORDLONG>* handleMap,
MemoryTracker* memoryTracker)
{
CORINFO_SIG_INFO sig;
sig.callConv = (CorInfoCallConv)sigInfo.callConv;
Expand All @@ -430,7 +435,7 @@ inline CORINFO_SIG_INFO SpmiRecordsHelper::Restore_CORINFO_SIG_INFO(const Agnost
sig.scope = (CORINFO_MODULE_HANDLE)sigInfo.scope;
sig.token = (mdToken)sigInfo.token;

DeserializeCORINFO_SIG_INST(sig, sigInfo, handleMap);
DeserializeCORINFO_SIG_INST(sig, sigInfo, handleMap, memoryTracker);

return sig;
}
Expand Down
12 changes: 5 additions & 7 deletions src/coreclr/tools/superpmi/superpmi/superpmi.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -417,9 +417,6 @@ int __cdecl main(int argc, char* argv[])
continue;
}

// Save the stored CompileResult
mc->originalCR = mc->cr;

// Compile this method context as many times as we have been asked to (default is once).
for (int iter = 0; iter < o.repeatCount; iter++)
{
Expand All @@ -446,10 +443,6 @@ int __cdecl main(int argc, char* argv[])
}
}

// Create a new CompileResult for this compilation (the CompileResult from the stored file is
// in originalCR if necessary).
mc->cr = new CompileResult();

// For asm diffs, we need to store away the CompileResult generated by the first JIT when compiling
// with the 2nd JIT.
CompileResult* crl = nullptr;
Expand Down Expand Up @@ -776,6 +769,11 @@ int __cdecl main(int argc, char* argv[])
}

delete crl;

if (iter + 1 < o.repeatCount)
{
mc->Reset();
}
}

delete mc;
Expand Down

0 comments on commit 92abb8c

Please sign in to comment.