Skip to content

Commit

Permalink
Remove DbgGet*Random* macros and constant arguments (#105051)
Browse files Browse the repository at this point in the history
* Remove unused parameter
* Remove DbgRandomOnExe
* Remove DbgGetEXETimeStamp
---------
Co-authored-by: Jan Kotas <[email protected]>
  • Loading branch information
AaronRobinsonMSFT authored Jul 18, 2024
1 parent d79cd32 commit bc39839
Show file tree
Hide file tree
Showing 16 changed files with 29 additions and 95 deletions.
27 changes: 1 addition & 26 deletions src/coreclr/inc/debugmacros.h
Original file line number Diff line number Diff line change
Expand Up @@ -178,29 +178,4 @@ do { hr = (EXPR); if(hr != ERROR_SUCCESS) { hr = HRESULT_FROM_WIN32(hr); goto LA
#undef _ASSERT
#define _ASSERT _ASSERTE


#if defined(_DEBUG) && defined(HOST_WINDOWS)

// This function returns the EXE time stamp (effectively a random number)
// Under retail it always returns 0. This is meant to be used in the
// RandomOnExe macro
unsigned DbgGetEXETimeStamp();

// returns true 'fractionOn' amount of the time using the EXE timestamp
// as the random number seed. For example DbgRandomOnExe(.1) returns true 1/10
// of the time. We use the line number so that different uses of DbgRandomOnExe
// will not be coorelated with each other (9973 is prime). Returns false on a retail build
#define DbgRandomOnHashAndExe(hash, fractionOn) \
(((DbgGetEXETimeStamp() * __LINE__ * ((hash) ? (hash) : 1)) % 9973) < \
unsigned((fractionOn) * 9973))
#define DbgRandomOnExe(fractionOn) DbgRandomOnHashAndExe(0, fractionOn)

#else

#define DbgGetEXETimeStamp() 0
#define DbgRandomOnHashAndExe(hash, fractionOn) 0
#define DbgRandomOnExe(fractionOn) 0

#endif // _DEBUG && !FEATUREPAL

#endif
#endif // __DebugMacros_h__
27 changes: 0 additions & 27 deletions src/coreclr/utilcode/debug.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -322,33 +322,6 @@ bool _DbgBreakCheckNoThrow(
return result;
}

#ifndef TARGET_UNIX
// Get the timestamp from the PE file header. This is useful
unsigned DbgGetEXETimeStamp()
{
STATIC_CONTRACT_NOTHROW;
STATIC_CONTRACT_GC_NOTRIGGER;
STATIC_CONTRACT_DEBUG_ONLY;

static ULONG cache = 0;
if (cache == 0) {
// Use GetModuleHandleA to avoid contracts - this results in a recursive loop initializing the
// debug allocator.
BYTE* imageBase = (BYTE*) GetModuleHandleA(NULL);
if (imageBase == 0)
return(0);
IMAGE_DOS_HEADER *pDOS = (IMAGE_DOS_HEADER*) imageBase;
if ((pDOS->e_magic != VAL16(IMAGE_DOS_SIGNATURE)) || (pDOS->e_lfanew == 0))
return(0);

IMAGE_NT_HEADERS *pNT = (IMAGE_NT_HEADERS*) (VAL32(pDOS->e_lfanew) + imageBase);
cache = VAL32(pNT->FileHeader.TimeDateStamp);
}

return cache;
}
#endif // TARGET_UNIX

VOID DebBreakHr(HRESULT hr)
{
STATIC_CONTRACT_LEAF;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/amd64/cgenamd64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ void HelperMethodFrame::UpdateRegDisplay(const PREGDISPLAY pRD, bool updateFloat
// This allocation throws on OOM.
MachState* pUnwoundState = (MachState*)DacAllocHostOnlyInstance(sizeof(*pUnwoundState), true);

InsureInit(false, pUnwoundState);
InsureInit(pUnwoundState);

pRD->pCurrentContext->Rip = pRD->ControlPC = pUnwoundState->m_Rip;
pRD->pCurrentContext->Rsp = pRD->SP = pUnwoundState->m_Rsp;
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/vm/amd64/jitinterfaceamd64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -698,8 +698,8 @@ bool WriteBarrierManager::NeedDifferentWriteBarrier(bool bReqUpperBoundsCheck, b
{
case WRITE_BARRIER_UNINITIALIZED:
#ifdef _DEBUG
// Use the default slow write barrier some of the time in debug builds because of of contains some good asserts
if ((g_pConfig->GetHeapVerifyLevel() & EEConfig::HEAPVERIFY_BARRIERCHECK) || DbgRandomOnExe(0.5)) {
// The default slow write barrier has some good asserts
if ((g_pConfig->GetHeapVerifyLevel() & EEConfig::HEAPVERIFY_BARRIERCHECK)) {
break;
}
#endif
Expand Down Expand Up @@ -901,7 +901,7 @@ int WriteBarrierManager::UpdateWriteWatchAndCardTableLocations(bool isRuntimeSus
default:
break; // clang seems to require all enum values to be covered for some reason
}

if (*(UINT64*)m_pCardTableImmediate != (size_t)g_card_table)
{
ExecutableWriterHolder<UINT64> cardTableImmediateWriterHolder((UINT64*)m_pCardTableImmediate, sizeof(UINT64));
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/arm/stubs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ void HelperMethodFrame::UpdateRegDisplay(const PREGDISPLAY pRD, bool updateFloat
// This allocation throws on OOM.
MachState* pUnwoundState = (MachState*)DacAllocHostOnlyInstance(sizeof(*pUnwoundState), true);

InsureInit(false, pUnwoundState);
InsureInit(pUnwoundState);

pRD->pCurrentContext->Pc = pRD->ControlPC = pUnwoundState->_pc;
pRD->pCurrentContext->Sp = pRD->SP = pUnwoundState->_sp;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/arm64/stubs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ void HelperMethodFrame::UpdateRegDisplay(const PREGDISPLAY pRD, bool updateFloat
// This allocation throws on OOM.
MachState* pUnwoundState = (MachState*)DacAllocHostOnlyInstance(sizeof(*pUnwoundState), true);

InsureInit(false, pUnwoundState);
InsureInit(pUnwoundState);

pRD->pCurrentContext->Pc = pRD->ControlPC = pUnwoundState->_pc;
pRD->pCurrentContext->Sp = pRD->SP = pUnwoundState->_sp;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/fcall.h
Original file line number Diff line number Diff line change
Expand Up @@ -797,7 +797,7 @@ LPVOID __FCThrowArgument(LPVOID me, enum RuntimeExceptionKind reKind, LPCWSTR ar
// The HelperMethodFrame knows how to get its return address. Let other code get at it, too.
// (Uses comma operator to call InsureInit & discard result.
#define HELPER_METHOD_FRAME_GET_RETURN_ADDRESS() \
( static_cast<UINT_PTR>( (__helperframe.InsureInit(false, NULL)), (__helperframe.MachineState()->GetRetAddr()) ) )
( static_cast<UINT_PTR>( (__helperframe.InsureInit(NULL)), (__helperframe.MachineState()->GetRetAddr()) ) )

// Very short routines, or routines that are guaranteed to force GC or EH
// don't need to poll the GC. USE VERY SPARINGLY!!!
Expand Down
24 changes: 7 additions & 17 deletions src/coreclr/vm/frames.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1787,7 +1787,7 @@ MethodDesc* HelperMethodFrame::GetFunction()
WRAPPER_NO_CONTRACT;

#ifndef DACCESS_COMPILE
InsureInit(false, NULL);
InsureInit(NULL);
return m_pMD;
#else
if (m_MachState.isValid())
Expand All @@ -1806,19 +1806,14 @@ MethodDesc* HelperMethodFrame::GetFunction()
// Ensures the HelperMethodFrame gets initialized, if not already.
//
// Arguments:
// * initialInit -
// * true: ensure the simple, first stage of initialization has been completed.
// This is used when the HelperMethodFrame is first created.
// * false: complete any initialization that was left to do, if any.
// * unwindState - [out] DAC builds use this to return the unwound machine state.
//
// Return Value:
// Normally, the function always returns TRUE meaning the initialization succeeded.
//
//

BOOL HelperMethodFrame::InsureInit(bool initialInit,
MachState * unwindState)
BOOL HelperMethodFrame::InsureInit(MachState * unwindState)
{
CONTRACTL {
NOTHROW;
Expand All @@ -1834,13 +1829,10 @@ BOOL HelperMethodFrame::InsureInit(bool initialInit,
_ASSERTE(m_Attribs != 0xCCCCCCCC);

#ifndef DACCESS_COMPILE
if (!initialInit)
{
m_pMD = ECall::MapTargetBackToMethod(m_FCallEntry);
m_pMD = ECall::MapTargetBackToMethod(m_FCallEntry);

// if this is an FCall, we should find it
_ASSERTE(m_FCallEntry == 0 || m_pMD != 0);
}
// if this is an FCall, we should find it
_ASSERTE(m_FCallEntry == 0 || m_pMD != 0);
#endif

// Because TRUE FCalls can be called from via reflection, com-interop, etc.,
Expand All @@ -1855,8 +1847,7 @@ BOOL HelperMethodFrame::InsureInit(bool initialInit,
DWORD threadId = m_pThread->GetOSThreadId();
MachState unwound;

if (!initialInit &&
m_FCallEntry == 0 &&
if (m_FCallEntry == 0 &&
!(m_Attribs & Frame::FRAME_ATTR_EXACT_DEPTH)) // Jit Helper
{
LazyMachState::unwindLazyState(
Expand Down Expand Up @@ -1884,8 +1875,7 @@ BOOL HelperMethodFrame::InsureInit(bool initialInit,
}
#endif // !defined(DACCESS_COMPILE)
}
else if (!initialInit &&
(m_Attribs & Frame::FRAME_ATTR_CAPTURE_DEPTH_2) != 0)
else if ((m_Attribs & Frame::FRAME_ATTR_CAPTURE_DEPTH_2) != 0)
{
// explicitly told depth
LazyMachState::unwindLazyState(lazy, &unwound, threadId, 2);
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/vm/frames.h
Original file line number Diff line number Diff line change
Expand Up @@ -1259,7 +1259,7 @@ class HelperMethodFrame : public Frame
{
#if defined(DACCESS_COMPILE)
MachState unwoundState;
InsureInit(false, &unwoundState);
InsureInit(&unwoundState);
return unwoundState.GetRetAddr();
#else // !DACCESS_COMPILE
_ASSERTE(!"HMF's should always be initialized in the non-DAC world.");
Expand Down Expand Up @@ -1342,7 +1342,7 @@ class HelperMethodFrame : public Frame
}
#endif // DACCESS_COMPILE

BOOL InsureInit(bool initialInit, struct MachState* unwindState);
BOOL InsureInit(struct MachState* unwindState);

LazyMachState * MachineState() {
LIMITED_METHOD_CONTRACT;
Expand Down Expand Up @@ -3188,8 +3188,8 @@ class FrameWithCookie
FrameType* operator&() { LIMITED_METHOD_CONTRACT; return &m_frame; }
LazyMachState * MachineState() { WRAPPER_NO_CONTRACT; return m_frame.MachineState(); }
Thread * GetThread() { WRAPPER_NO_CONTRACT; return m_frame.GetThread(); }
BOOL InsureInit(bool initialInit, struct MachState* unwindState)
{ WRAPPER_NO_CONTRACT; return m_frame.InsureInit(initialInit, unwindState); }
BOOL InsureInit(struct MachState* unwindState)
{ WRAPPER_NO_CONTRACT; return m_frame.InsureInit(unwindState); }
void Poll() { WRAPPER_NO_CONTRACT; m_frame.Poll(); }
void SetStackPointerPtr(TADDR sp) { WRAPPER_NO_CONTRACT; m_frame.SetStackPointerPtr(sp); }
void InitAndLink(T_CONTEXT *pContext) { WRAPPER_NO_CONTRACT; m_frame.InitAndLink(pContext); }
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/vm/i386/cgenx86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ void HelperMethodFrame::UpdateRegDisplay(const PREGDISPLAY pRD, bool updateFloat
// This allocation throws on OOM.
MachState* pUnwoundState = (MachState*)DacAllocHostOnlyInstance(sizeof(*pUnwoundState), true);

InsureInit(false, pUnwoundState);
InsureInit(pUnwoundState);

pRD->PCTAddr = dac_cast<TADDR>(pUnwoundState->pRetAddr());
pRD->pCurrentContext->Eip = pRD->ControlPC = pUnwoundState->GetRetAddr();
Expand Down Expand Up @@ -295,7 +295,7 @@ void HelperMethodFrame::UpdateRegDisplay(const PREGDISPLAY pRD, bool updateFloat
{
MachState unwindState;

InsureInit(false, &unwindState);
InsureInit(&unwindState);
pRD->PCTAddr = dac_cast<TADDR>(unwindState.pRetAddr());
pRD->ControlPC = unwindState.GetRetAddr();
pRD->SP = unwindState._esp;
Expand Down Expand Up @@ -371,7 +371,7 @@ EXTERN_C MachState* STDCALL HelperMethodFrameConfirmState(HelperMethodFrame* fra
BEGIN_DEBUG_ONLY_CODE;
if (!state->isValid())
{
frame->InsureInit(false, NULL);
frame->InsureInit(NULL);
_ASSERTE(state->_pEsi != &state->_esi || state->_esi == (TADDR)esiVal);
_ASSERTE(state->_pEdi != &state->_edi || state->_edi == (TADDR)ediVal);
_ASSERTE(state->_pEbx != &state->_ebx || state->_ebx == (TADDR)ebxVal);
Expand Down
9 changes: 3 additions & 6 deletions src/coreclr/vm/i386/jitinterfacex86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -802,8 +802,6 @@ static const void * const c_rgDebugWriteBarriers[NUM_WRITE_BARRIERS] = {
};
#endif // WRITE_BARRIER_CHECK

#define DEBUG_RANDOM_BARRIER_CHECK DbgGetEXETimeStamp() % 7 == 4

/*********************************************************************/
// Initialize the part of the JIT helpers that require very little of
// EE infrastructure to be in place.
Expand Down Expand Up @@ -913,9 +911,8 @@ void InitJITHelpers1()

#ifdef WRITE_BARRIER_CHECK
// Don't do the fancy optimization just jump to the old one
// Use the slow one from time to time in a debug build because
// there are some good asserts in the unoptimized one
if ((g_pConfig->GetHeapVerifyLevel() & EEConfig::HEAPVERIFY_BARRIERCHECK) || DEBUG_RANDOM_BARRIER_CHECK) {
// Use the slow one for write barrier checks build because it has some good asserts
if (g_pConfig->GetHeapVerifyLevel() & EEConfig::HEAPVERIFY_BARRIERCHECK) {
pfunc = &pBufRW[0];
*pfunc++ = 0xE9; // JMP c_rgDebugWriteBarriers[iBarrier]
*((DWORD*) pfunc) = (BYTE*) c_rgDebugWriteBarriers[iBarrier] - (&pBuf[1] + sizeof(DWORD));
Expand Down Expand Up @@ -959,7 +956,7 @@ void ValidateWriteBarrierHelpers()

#ifdef WRITE_BARRIER_CHECK
// write barrier checking uses the slower helpers that we don't bash so there is no need for validation
if ((g_pConfig->GetHeapVerifyLevel() & EEConfig::HEAPVERIFY_BARRIERCHECK) || DEBUG_RANDOM_BARRIER_CHECK)
if (g_pConfig->GetHeapVerifyLevel() & EEConfig::HEAPVERIFY_BARRIERCHECK)
return;
#endif // WRITE_BARRIER_CHECK

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/jithelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3078,7 +3078,7 @@ HCIMPL1(void, IL_Throw, Object* obj)
OBJECTREF oref = ObjectToOBJECTREF(obj);

#if defined(_DEBUG) && defined(TARGET_X86)
__helperframe.InsureInit(false, NULL);
__helperframe.InsureInit(NULL);
g_ExceptionEIP = (LPVOID)__helperframe.GetReturnAddress();
#endif // defined(_DEBUG) && defined(TARGET_X86)

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/loongarch64/stubs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ void HelperMethodFrame::UpdateRegDisplay(const PREGDISPLAY pRD, bool updateFloat
// This allocation throws on OOM.
MachState* pUnwoundState = (MachState*)DacAllocHostOnlyInstance(sizeof(*pUnwoundState), true);

InsureInit(false, pUnwoundState);
InsureInit(pUnwoundState);

pRD->pCurrentContext->Pc = pRD->ControlPC = pUnwoundState->_pc;
pRD->pCurrentContext->Sp = pRD->SP = pUnwoundState->_sp;
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/vm/proftoeeinterfaceimpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8240,7 +8240,6 @@ static BOOL EnsureFrameInitialized(Frame * pFrame)
HelperMethodFrame * pHMF = (HelperMethodFrame *) pFrame;

if (pHMF->InsureInit(
false, // initialInit
NULL // unwindState
) != NULL)
{
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/riscv64/stubs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ void HelperMethodFrame::UpdateRegDisplay(const PREGDISPLAY pRD, bool updateFloat
// This allocation throws on OOM.
MachState* pUnwoundState = (MachState*)DacAllocHostOnlyInstance(sizeof(*pUnwoundState), true);

InsureInit(false, pUnwoundState);
InsureInit(pUnwoundState);

pRD->pCurrentContext->Pc = pRD->ControlPC = pUnwoundState->_pc;
pRD->pCurrentContext->Sp = pRD->SP = pUnwoundState->_sp;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/threads.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1178,7 +1178,7 @@ void InitThreadManager()

#ifdef _DEBUG
// Randomize OBJREF_HASH to handle hash collision.
Thread::OBJREF_HASH = OBJREF_TABSIZE - (DbgGetEXETimeStamp()%10);
Thread::OBJREF_HASH = OBJREF_TABSIZE - GetRandomInt(10);
#endif // _DEBUG

ThreadSuspend::Initialize();
Expand Down

0 comments on commit bc39839

Please sign in to comment.