Skip to content
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

Fixes memory leaks inside CoverageTool and improves style #324

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

Fixes memory leaks inside CoverageTool and improves style #324

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
9943ed9
[feat] adds assembly tracking to coverage tool
DanielELog Feb 3, 2024
4eb6f3b
[feat] fixes CoverageTool memory leaks
DanielELog May 27, 2024
993d716
[style] improves readability
DanielELog Jul 8, 2024
80de94c
[fix] use of private fields
DanielELog Jul 8, 2024
65ecc14
[fix] restores coverage output
DanielELog Jul 9, 2024
3b56026
[fix] incorrect thread loss upon filter leave
DanielELog Jul 10, 2024
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
308 changes: 11 additions & 297 deletions VSharp.CoverageInstrumenter/profiler/ILRewriter.cpp
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 5 additions & 3 deletions VSharp.CoverageInstrumenter/profiler/ILRewriter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@
#include "corprof.h"
#include <stdexcept>
#include "probes.h"
#include "memory.h"

#undef IfFailRet
#define IfFailRet(EXPR) do { HRESULT hr = (EXPR); if(FAILED(hr)) { return (hr); } } while (0)
#define IfFailRet(EXPR) do { if (std::atomic_load(&vsharp::shutdownInOrder)) { return S_OK; } HRESULT hr = (EXPR); if(FAILED(hr)) { return (hr); } } while (0)

#undef IfNullRet
#define IfNullRet(EXPR) do { if ((EXPR) == NULL) return E_OUTOFMEMORY; } while (0)
#define IfNullRet(EXPR) do { if (std::atomic_load(&vsharp::shutdownInOrder)) { return S_OK; } if ((EXPR) == NULL) return E_OUTOFMEMORY; } while (0)

struct ILInstr
{
Expand Down Expand Up @@ -115,7 +116,8 @@ HRESULT RewriteIL(
ModuleID moduleID,
mdMethodDef methodDef,
int methodId,
bool isMain);
bool isMain,
bool isTestRun);

bool NeedFullInstrumentation(const WCHAR *moduleName, int moduleSize, mdMethodDef method);

Expand Down
10 changes: 8 additions & 2 deletions VSharp.CoverageInstrumenter/profiler/api.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <vector>

using namespace vsharp;

extern "C" void SetEntryMain(char* assemblyName, int assemblyNameLength, char* moduleName, int moduleNameLength, int methodToken) {
profilerState->setEntryMain(assemblyName, assemblyNameLength, moduleName, moduleNameLength, methodToken);
LOG(tout << "received entry main" << std::endl);
Expand All @@ -18,9 +19,9 @@ extern "C" void GetHistory(UINT_PTR size, UINT_PTR bytes) {

std::atomic_fetch_add(&shutdownBlockingRequestsCount, 1);
size_t tmpSize;
auto tmpBytes = profilerState->coverageTracker->serializeCoverageReport(&tmpSize);
historyBuffer = profilerState->coverageTracker->serializeCoverageReport(&tmpSize);
*(ULONG*)size = tmpSize;
*(char**)bytes = tmpBytes;
*(char**)bytes = historyBuffer;

profilerState->coverageTracker->clear();
profilerState->threadTracker->clear();
Expand All @@ -29,6 +30,11 @@ extern "C" void GetHistory(UINT_PTR size, UINT_PTR bytes) {
LOG(tout << "GetHistory request handled!");
}

extern "C" void ClearHistory() {
delete historyBuffer;
historyBuffer = nullptr;
}

extern "C" void SetCurrentThreadId(int mapId) {
LOG(tout << "Map current thread to: " << mapId);
vsharp::profilerState->threadTracker->mapCurrentThread(mapId);
Expand Down
3 changes: 3 additions & 0 deletions VSharp.CoverageInstrumenter/profiler/api.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,8 @@
extern "C" IMAGEHANDLER_API void SetEntryMain(char* assemblyName, int assemblyNameLength, char* moduleName, int moduleNameLength, int methodToken);
extern "C" IMAGEHANDLER_API void GetHistory(UINT_PTR size, UINT_PTR bytes);
extern "C" IMAGEHANDLER_API void SetCurrentThreadId(int mapId);
extern "C" IMAGEHANDLER_API void ClearHistory();

static char* historyBuffer = nullptr;

#endif //VSHARP_COVERAGEINSTRUMENTER_API_H
27 changes: 17 additions & 10 deletions VSharp.CoverageInstrumenter/profiler/corProfiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,9 @@ HRESULT STDMETHODCALLTYPE CorProfiler::Initialize(IUnknown *pICorProfilerInfoUnk

// TMP Windows fix
#undef IfFailRet
#define IfFailRet(EXPR) do { HRESULT hr = (EXPR); if(FAILED(hr)) { return (hr); } } while (0)
#define IfFailRet(EXPR) do { if (std::atomic_load(&shutdownInOrder)) return S_OK; HRESULT hr = (EXPR); if(FAILED(hr)) { return (hr); } } while (0)
IfFailRet(this->corProfilerInfo->SetEventMask(eventMask));


profilerState = new ProfilerState((ICorProfilerInfo8*)this);

LOG(tout << "Initialize finished" << std::endl);
Expand All @@ -65,27 +64,30 @@ HRESULT STDMETHODCALLTYPE CorProfiler::Initialize(IUnknown *pICorProfilerInfoUnk

HRESULT STDMETHODCALLTYPE CorProfiler::Shutdown()
{
profilerState->isFinished = true;
std::atomic_store(&shutdownInOrder, true);

// waiting until all current requests are resolved
while (std::atomic_load(&shutdownBlockingRequestsCount) > 0) {}

LOG(tout << "SHUTDOWN");
if (profilerState->isPassiveRun) {

size_t tmpSize;
auto tmpBytes = profilerState->coverageTracker->serializeCoverageReport(&tmpSize);;
auto tmpBytes = profilerState->coverageTracker->serializeCoverageReport(&tmpSize);

std::ofstream fout;
fout.open(profilerState->passiveResultPath, std::ios::out|std::ios::binary);
fout.write(tmpBytes, static_cast<long>(tmpSize));
fout.close();

delete tmpBytes;
}

#ifdef _LOGGING
close_log();
#endif

delete profilerState;

if (this->corProfilerInfo != nullptr)
{
this->corProfilerInfo->Release();
Expand Down Expand Up @@ -215,13 +217,14 @@ HRESULT STDMETHODCALLTYPE CorProfiler::FunctionUnloadStarted(FunctionID function
HRESULT STDMETHODCALLTYPE CorProfiler::JITCompilationStarted(FunctionID functionId, BOOL fIsSafeToBlock)
{
// the process was finished, ignoring all firther requests
if (profilerState->isFinished) return S_OK;
if (std::atomic_load(&shutdownInOrder)) return S_OK;

std::atomic_fetch_add(&shutdownBlockingRequestsCount, 1);

UNUSED(fIsSafeToBlock);
auto instrument = new Instrumenter(*corProfilerInfo);
HRESULT hr = instrument->instrument(functionId);
auto methodName = GetFunctionName(functionId);
HRESULT hr = instrument->instrument(functionId, methodName);
delete instrument;

std::atomic_fetch_sub(&shutdownBlockingRequestsCount, 1);
Expand Down Expand Up @@ -421,6 +424,7 @@ HRESULT STDMETHODCALLTYPE CorProfiler::RootReferences(ULONG cRootRefs, ObjectID

HRESULT STDMETHODCALLTYPE CorProfiler::ExceptionThrown(ObjectID thrownObjectId)
{
if (std::atomic_load(&shutdownInOrder)) return S_OK;
auto exceptionName = GetObjectTypeName(thrownObjectId);
LOG(
if(profilerState->threadTracker->hasMapping()) {
Expand Down Expand Up @@ -454,7 +458,7 @@ HRESULT STDMETHODCALLTYPE CorProfiler::ExceptionSearchFunctionLeave()

HRESULT STDMETHODCALLTYPE CorProfiler::ExceptionSearchFilterEnter(FunctionID functionId)
{
if (profilerState->isFinished || !profilerState->threadTracker->isCurrentThreadTracked()) return S_OK;
if (std::atomic_load(&shutdownInOrder) || !profilerState->threadTracker->isCurrentThreadTracked()) return S_OK;
LOG(tout << "EXCEPTION Search filter enter");
profilerState->threadTracker->filterEnter();
UNUSED(functionId);
Expand All @@ -463,7 +467,7 @@ HRESULT STDMETHODCALLTYPE CorProfiler::ExceptionSearchFilterEnter(FunctionID fun

HRESULT STDMETHODCALLTYPE CorProfiler::ExceptionSearchFilterLeave()
{
if (profilerState->isFinished || !profilerState->threadTracker->isCurrentThreadTracked()) return S_OK;
if (std::atomic_load(&shutdownInOrder) || !profilerState->threadTracker->isCurrentThreadTracked()) return S_OK;
LOG(tout << "EXCEPTION Search filter leave");
profilerState->threadTracker->filterLeave();
return S_OK;
Expand Down Expand Up @@ -502,7 +506,8 @@ HRESULT STDMETHODCALLTYPE CorProfiler::ExceptionUnwindFunctionLeave()
{
LOG(tout << "EXCEPTION UNWIND FUNCTION LEAVE");
// the process was finished, ignoring all further requests
if (profilerState->isFinished || !profilerState->threadTracker->isCurrentThreadTracked()) return S_OK;
if (std::atomic_load(&shutdownInOrder)
|| !profilerState->threadTracker->isCurrentThreadTracked()) return S_OK;
profilerState->threadTracker->unwindFunctionLeave();
return S_OK;
}
Expand Down Expand Up @@ -719,6 +724,7 @@ HRESULT STDMETHODCALLTYPE CorProfiler::DynamicMethodJITCompilationFinished(Funct
}

std::string CorProfiler::GetObjectTypeName(ObjectID objectId) {
if (std::atomic_load(&shutdownInOrder)) return "";
ClassID classId;
corProfilerInfo->GetClassFromObject(objectId, &classId);

Expand All @@ -739,6 +745,7 @@ std::string CorProfiler::GetObjectTypeName(ObjectID objectId) {
}

std::string CorProfiler::GetFunctionName(FunctionID functionId) {
if (std::atomic_load(&shutdownInOrder)) return "";
ClassID classId;
ModuleID moduleId;
mdToken token;
Expand Down
49 changes: 41 additions & 8 deletions VSharp.CoverageInstrumenter/profiler/coverageTracker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ void MethodInfo::serialize(std::vector<char>& buffer) const {
serializePrimitive(moduleNameLength, buffer);
serializePrimitiveArray(moduleName, moduleNameLength, buffer);
}

void MethodInfo::Dispose() {
delete moduleName;
delete assemblyName;
}
//endregion

//region CoverageRecord
Expand All @@ -21,27 +26,31 @@ void CoverageRecord::serialize(std::vector<char>& buffer) const {
serializePrimitive(event, buffer);
serializePrimitive(methodId, buffer);
serializePrimitive(thread, buffer);
serializePrimitive(timestamp, buffer);
}
//endregion

long long GetMicrosecondTime() {
using namespace std::chrono;
return duration_cast<microseconds>(system_clock::now().time_since_epoch()).count();
}

//region CoverageHistory
CoverageHistory::CoverageHistory(OFFSET offset, int methodId) {
auto insertResult = visitedMethods.insert(methodId);
LOG(if (insertResult.second) {
tout << "Visit method: " << methodId;
});
auto record = new CoverageRecord({offset, EnterMain, profilerState->threadInfo->getCurrentThread(), methodId});
if (insertResult.second) {
LOG(tout << "Visit method: " << methodId);
}
auto record = new CoverageRecord({offset, EnterMain, profilerState->threadInfo->getCurrentThread(), methodId, GetMicrosecondTime()});
records.push_back(record);
}

void CoverageHistory::addCoverage(OFFSET offset, CoverageEvent event, int methodId) {
auto insertResult = visitedMethods.insert(methodId);
LOG(
if (insertResult.second) {
tout << "Visit method: " << methodId;
LOG(tout << "Visit method: " << methodId);
}
);
auto record = new CoverageRecord({offset, event, profilerState->threadInfo->getCurrentThread(), methodId});
auto record = new CoverageRecord({offset, event, profilerState->threadInfo->getCurrentThread(), methodId, GetMicrosecondTime()});
records.push_back(record);
}

Expand All @@ -54,6 +63,8 @@ void CoverageHistory::serialize(std::vector<char>& buffer) const {
}

CoverageHistory::~CoverageHistory() {
for (auto r : records)
delete r;
records.clear();
}
//endregion
Expand Down Expand Up @@ -99,6 +110,7 @@ void CoverageTracker::invocationFinished() {
coverage->serialize(buffer);
}

delete coverage;
trackedCoverage->remove();

serializedCoverageMutex.lock();
Expand Down Expand Up @@ -148,6 +160,8 @@ char* CoverageTracker::serializeCoverageReport(size_t* size) {
serializePrimitiveArray(&serializedCoverage[i][0], serializedCoverage[i].size(), buffer);
}

for (auto cov : trackedCoverage->items())
delete cov.second;
trackedCoverage->clear();
serializedCoverage.clear();
methodsToSerialize.clear();
Expand All @@ -158,6 +172,7 @@ char* CoverageTracker::serializeCoverageReport(size_t* size) {
*size = buffer.size();
char* array = new char[*size];
std::memcpy(array, &buffer[0], *size);

return array;
}

Expand All @@ -179,11 +194,29 @@ void CoverageTracker::clear() {

CoverageTracker::~CoverageTracker(){
clear();
for (int i = 0; i < collectedMethods.size(); i++)
collectedMethods[i].Dispose();
collectedMethods.clear();
delete trackedCoverage;
}

void CoverageTracker::invocationAborted() {
auto abortedCov = trackedCoverage->load();
delete abortedCov;
trackedCoverage->update([](CoverageHistory *cov) {
return (CoverageHistory*) nullptr;
});
}

void CoverageTracker::printMethodDebug(int methodId)
{
LOG(
collectedMethodsMutex.lock();
auto method = collectedMethods[methodId];
auto wl = method.assemblyNameLength;
auto ws = method.assemblyName;
tout << ' ' << std::string(ws, ws + wl - 1) << '.' << method.methodName << std::endl;
collectedMethodsMutex.unlock();
);
}
//endregion
15 changes: 11 additions & 4 deletions VSharp.CoverageInstrumenter/profiler/coverageTracker.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ enum CoverageEvent {
Call,
Tailcall,
TrackCoverage,
StsfldHit
StsfldHit,
ThrowLeave,
};

struct MethodInfo {
Expand All @@ -28,15 +29,20 @@ struct MethodInfo {
WCHAR *assemblyName;
ULONG moduleNameLength;
WCHAR *moduleName;
std::string methodName;

void serialize(std::vector<char>& buffer) const;

// frees previously allocated resources for it; the object is not supposed to be used afterwards
void Dispose();
};

struct CoverageRecord {
OFFSET offset;
CoverageEvent event;
ThreadID thread;
int methodId;
long long timestamp;

void serialize(std::vector<char>& buffer) const;
};
Expand All @@ -57,15 +63,15 @@ class CoverageTracker {

private:
bool collectMainOnly;
std::mutex collectedMethodsMutex;
std::vector<MethodInfo> collectedMethods;
std::mutex visitedMethodsMutex;
std::set<int> visitedMethods;
ThreadStorage<CoverageHistory*>* trackedCoverage;
ThreadTracker* threadTracker;
std::mutex serializedCoverageMutex;
std::vector<std::vector<char>> serializedCoverage;
std::vector<int> serializedCoverageThreadIds;
std::vector<std::vector<char>> serializedCoverage;
std::mutex collectedMethodsMutex;
std::vector<MethodInfo> collectedMethods;
Comment on lines +72 to +74
Copy link
Contributor

Choose a reason for hiding this comment

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

private?

public:
explicit CoverageTracker(ThreadTracker* threadTracker, ThreadInfo* threadInfo, bool collectMainOnly);
bool isCollectMainOnly() const;
Expand All @@ -75,6 +81,7 @@ class CoverageTracker {
size_t collectMethod(MethodInfo info);
char* serializeCoverageReport(size_t* size);
void clear();
void printMethodDebug(int methodId);
~CoverageTracker();
};

Expand Down
Loading
Loading