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.

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

Enable GC double reporting detection #107986

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions src/coreclr/inc/clrconfigvalues.h
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ CONFIG_STRING_INFO(INTERNAL_SkipGCCoverage, W("SkipGcCoverage"), "Specify a list
RETAIL_CONFIG_DWORD_INFO(UNSUPPORTED_StatsUpdatePeriod, W("StatsUpdatePeriod"), 60, "Specifies the interval, in seconds, at which to update the statistics")
RETAIL_CONFIG_DWORD_INFO(UNSUPPORTED_GCRetainVM, W("GCRetainVM"), 0, "When set we put the segments that should be deleted on a standby list (instead of releasing them back to the OS) which will be considered to satisfy new segment requests (note that the same thing can be specified via API which is the supported way)")
RETAIL_CONFIG_DWORD_INFO(EXTERNAL_gcAllowVeryLargeObjects, W("gcAllowVeryLargeObjects"), 1, "Allow allocation of 2GB+ objects on GC heap")
RETAIL_CONFIG_DWORD_INFO(UNSUPPORTED_EnableGCHoleMonitoring, W("EnableGCHoleMonitoring"), 0, "Enable checks to proactively watch for possible GC holes")
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we can enable it some of the GC tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have just added a change that enables it also when GC stress is enabled, as @jkotas has suggested. So all the CI runs with GC stress will automatically execute this stuff.

RETAIL_CONFIG_DWORD_INFO(EXTERNAL_GCStress, W("GCStress"), 0, "Trigger GCs at regular intervals")
CONFIG_DWORD_INFO(INTERNAL_GcStressOnDirectCalls, W("GcStressOnDirectCalls"), 0, "Whether to trigger a GC on direct calls")
RETAIL_CONFIG_DWORD_INFO(UNSUPPORTED_HeapVerify, W("HeapVerify"), 0, "When set verifies the integrity of the managed heap on entry and exit of each GC")
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/vm/eeconfig.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,8 @@ HRESULT EEConfig::sync()
iGCConservative = (CLRConfig::GetConfigValue(CLRConfig::UNSUPPORTED_gcConservative) != 0);
#endif // FEATURE_CONSERVATIVE_GC

fEnableGCHoleMonitoring = (CLRConfig::GetConfigValue(CLRConfig::UNSUPPORTED_EnableGCHoleMonitoring) != 0);

#ifdef HOST_64BIT
iGCAllowVeryLargeObjects = (CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_gcAllowVeryLargeObjects) != 0);
#endif
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/vm/eeconfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ class EEConfig
#ifdef FEATURE_CONSERVATIVE_GC
bool GetGCConservative() const {LIMITED_METHOD_CONTRACT; return iGCConservative;}
#endif
bool GetEnableGCHoleMonitoring() const {LIMITED_METHOD_CONTRACT; return fEnableGCHoleMonitoring; }
#ifdef HOST_64BIT
bool GetGCAllowVeryLargeObjects() const {LIMITED_METHOD_CONTRACT; return iGCAllowVeryLargeObjects;}
#endif
Expand Down Expand Up @@ -572,6 +573,8 @@ class EEConfig
bool iGCAllowVeryLargeObjects;
#endif // HOST_64BIT

bool fEnableGCHoleMonitoring;

bool fGCBreakOnOOM;

#ifdef _DEBUG
Expand Down
21 changes: 21 additions & 0 deletions src/coreclr/vm/gcenv.ee.common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,11 @@ inline bool SafeToReportGenericParamContext(CrawlFrame* pCF)
return true;
}

thread_local void **doubleReportTracking = NULL;
janvorli marked this conversation as resolved.
Show resolved Hide resolved
thread_local int doubleReportTrackingSize = 0;
thread_local int doubleReportTrackingIndex = 0;
static const int DoubleReportTrackingInitialSize = 256;

/*
* GcEnumObject()
*
Expand All @@ -149,6 +154,22 @@ void GcEnumObject(LPVOID pData, OBJECTREF *pObj, uint32_t flags)
Object ** ppObj = (Object **)pObj;
GCCONTEXT * pCtx = (GCCONTEXT *) pData;

if (g_pConfig->GetEnableGCHoleMonitoring() && ((flags & GC_CALL_PINNED) == 0) && pCtx->sc->promotion)
{
if (doubleReportTrackingIndex == doubleReportTrackingSize)
janvorli marked this conversation as resolved.
Show resolved Hide resolved
{
doubleReportTrackingSize = (doubleReportTrackingSize == 0) ? DoubleReportTrackingInitialSize : doubleReportTrackingSize * 2;
doubleReportTracking = (void**)realloc(doubleReportTracking, doubleReportTrackingSize * sizeof(void*));
}
for (int i = 0; i < doubleReportTrackingIndex; i++)
janvorli marked this conversation as resolved.
Show resolved Hide resolved
{
// Double reporting of the same slot detected
_ASSERTE_ALL_BUILDS(doubleReportTracking[i] != pObj);
}

doubleReportTracking[doubleReportTrackingIndex++] = pObj;
}

// Since we may be asynchronously walking another thread's stack,
// check (frequently) for stack-buffer-overrun corruptions after
// any long operation
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/vm/gcenv.ee.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ extern void FinalizeWeakReference(Object* obj);
extern GCHeapHardLimitInfo g_gcHeapHardLimitInfo;
extern bool g_gcHeapHardLimitInfoSpecified;

extern thread_local int doubleReportTrackingIndex;

#include <generatedumpflags.h>
#include "gcrefmap.h"

Expand Down Expand Up @@ -201,6 +203,7 @@ static void ScanStackRoots(Thread * pThread, promote_func* fn, ScanContext* sc)
#if defined(FEATURE_EH_FUNCLETS)
flagsStackWalk |= GC_FUNCLET_REFERENCE_REPORTING;
#endif // defined(FEATURE_EH_FUNCLETS)
doubleReportTrackingIndex = 0;
pThread->StackWalkFrames( GcStackCrawlCallBack, &gcctx, flagsStackWalk);
}

Expand Down