From 79344c33f9515ed78ccb8de6390c25db6aa3a890 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 17 Jul 2024 14:03:48 -0700 Subject: [PATCH 1/6] Remove unused parameter --- src/coreclr/vm/amd64/cgenamd64.cpp | 2 +- src/coreclr/vm/arm/stubs.cpp | 2 +- src/coreclr/vm/arm64/stubs.cpp | 2 +- src/coreclr/vm/fcall.h | 2 +- src/coreclr/vm/frames.cpp | 24 +++++++----------------- src/coreclr/vm/frames.h | 8 ++++---- src/coreclr/vm/i386/cgenx86.cpp | 6 +++--- src/coreclr/vm/jithelpers.cpp | 2 +- src/coreclr/vm/loongarch64/stubs.cpp | 2 +- src/coreclr/vm/proftoeeinterfaceimpl.cpp | 1 - src/coreclr/vm/riscv64/stubs.cpp | 2 +- 11 files changed, 21 insertions(+), 32 deletions(-) diff --git a/src/coreclr/vm/amd64/cgenamd64.cpp b/src/coreclr/vm/amd64/cgenamd64.cpp index 6959899e8b89cf..a7ec8d33f050b1 100644 --- a/src/coreclr/vm/amd64/cgenamd64.cpp +++ b/src/coreclr/vm/amd64/cgenamd64.cpp @@ -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; diff --git a/src/coreclr/vm/arm/stubs.cpp b/src/coreclr/vm/arm/stubs.cpp index fe58e072c1195b..9589cff63d4011 100644 --- a/src/coreclr/vm/arm/stubs.cpp +++ b/src/coreclr/vm/arm/stubs.cpp @@ -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; diff --git a/src/coreclr/vm/arm64/stubs.cpp b/src/coreclr/vm/arm64/stubs.cpp index f12caa85834886..25a62472d1e6b4 100644 --- a/src/coreclr/vm/arm64/stubs.cpp +++ b/src/coreclr/vm/arm64/stubs.cpp @@ -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; diff --git a/src/coreclr/vm/fcall.h b/src/coreclr/vm/fcall.h index 15c4d1e5ec3b69..c52a6d86872402 100644 --- a/src/coreclr/vm/fcall.h +++ b/src/coreclr/vm/fcall.h @@ -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( (__helperframe.InsureInit(false, NULL)), (__helperframe.MachineState()->GetRetAddr()) ) ) + ( static_cast( (__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!!! diff --git a/src/coreclr/vm/frames.cpp b/src/coreclr/vm/frames.cpp index 650d1ba485f589..e6afc8995ef8e1 100644 --- a/src/coreclr/vm/frames.cpp +++ b/src/coreclr/vm/frames.cpp @@ -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()) @@ -1806,10 +1806,6 @@ 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: @@ -1817,8 +1813,7 @@ MethodDesc* HelperMethodFrame::GetFunction() // // -BOOL HelperMethodFrame::InsureInit(bool initialInit, - MachState * unwindState) +BOOL HelperMethodFrame::InsureInit(MachState * unwindState) { CONTRACTL { NOTHROW; @@ -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., @@ -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( @@ -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); diff --git a/src/coreclr/vm/frames.h b/src/coreclr/vm/frames.h index 5bc064b39bc299..6d4535aa1ca22c 100644 --- a/src/coreclr/vm/frames.h +++ b/src/coreclr/vm/frames.h @@ -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."); @@ -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; @@ -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); } diff --git a/src/coreclr/vm/i386/cgenx86.cpp b/src/coreclr/vm/i386/cgenx86.cpp index 15ddb6f6d79524..1738fd04905046 100644 --- a/src/coreclr/vm/i386/cgenx86.cpp +++ b/src/coreclr/vm/i386/cgenx86.cpp @@ -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(pUnwoundState->pRetAddr()); pRD->pCurrentContext->Eip = pRD->ControlPC = pUnwoundState->GetRetAddr(); @@ -295,7 +295,7 @@ void HelperMethodFrame::UpdateRegDisplay(const PREGDISPLAY pRD, bool updateFloat { MachState unwindState; - InsureInit(false, &unwindState); + InsureInit(&unwindState); pRD->PCTAddr = dac_cast(unwindState.pRetAddr()); pRD->ControlPC = unwindState.GetRetAddr(); pRD->SP = unwindState._esp; @@ -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); diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index c5798f2b1605f0..2b88c1a26b5082 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -3534,7 +3534,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) diff --git a/src/coreclr/vm/loongarch64/stubs.cpp b/src/coreclr/vm/loongarch64/stubs.cpp index 8f9b46325db37c..3754da19813e4e 100644 --- a/src/coreclr/vm/loongarch64/stubs.cpp +++ b/src/coreclr/vm/loongarch64/stubs.cpp @@ -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; diff --git a/src/coreclr/vm/proftoeeinterfaceimpl.cpp b/src/coreclr/vm/proftoeeinterfaceimpl.cpp index 40cc7a5f3b0c15..e99273beb5d133 100644 --- a/src/coreclr/vm/proftoeeinterfaceimpl.cpp +++ b/src/coreclr/vm/proftoeeinterfaceimpl.cpp @@ -8240,7 +8240,6 @@ static BOOL EnsureFrameInitialized(Frame * pFrame) HelperMethodFrame * pHMF = (HelperMethodFrame *) pFrame; if (pHMF->InsureInit( - false, // initialInit NULL // unwindState ) != NULL) { diff --git a/src/coreclr/vm/riscv64/stubs.cpp b/src/coreclr/vm/riscv64/stubs.cpp index 596034b07e4326..efa3d2c1e030ce 100644 --- a/src/coreclr/vm/riscv64/stubs.cpp +++ b/src/coreclr/vm/riscv64/stubs.cpp @@ -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; From 91d55264a2943eee7ff2d858c232e63086622263 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 17 Jul 2024 14:20:26 -0700 Subject: [PATCH 2/6] Remove DbgRandomOnExe --- src/coreclr/inc/debugmacros.h | 11 ----------- src/coreclr/vm/amd64/jitinterfaceamd64.cpp | 6 +++--- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/src/coreclr/inc/debugmacros.h b/src/coreclr/inc/debugmacros.h index da88e15f12a07a..af2804af9951b9 100644 --- a/src/coreclr/inc/debugmacros.h +++ b/src/coreclr/inc/debugmacros.h @@ -186,20 +186,9 @@ do { hr = (EXPR); if(hr != ERROR_SUCCESS) { hr = HRESULT_FROM_WIN32(hr); goto LA // 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 diff --git a/src/coreclr/vm/amd64/jitinterfaceamd64.cpp b/src/coreclr/vm/amd64/jitinterfaceamd64.cpp index 08d5e9493f6c22..8d567406072030 100644 --- a/src/coreclr/vm/amd64/jitinterfaceamd64.cpp +++ b/src/coreclr/vm/amd64/jitinterfaceamd64.cpp @@ -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)) { + // Use the default slow write barrier some of the time in debug builds because of some good asserts + if ((g_pConfig->GetHeapVerifyLevel() & EEConfig::HEAPVERIFY_BARRIERCHECK)) { break; } #endif @@ -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 cardTableImmediateWriterHolder((UINT64*)m_pCardTableImmediate, sizeof(UINT64)); From 582c85eebd76bb7fc0e0f52d2cdd98eb55b5abd9 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 17 Jul 2024 14:54:37 -0700 Subject: [PATCH 3/6] Remove DbgGetEXETimeStamp --- src/coreclr/inc/debugmacros.h | 16 +-------------- src/coreclr/utilcode/debug.cpp | 27 ------------------------- src/coreclr/vm/i386/jitinterfacex86.cpp | 6 ++---- src/coreclr/vm/threads.cpp | 5 ----- 4 files changed, 3 insertions(+), 51 deletions(-) diff --git a/src/coreclr/inc/debugmacros.h b/src/coreclr/inc/debugmacros.h index af2804af9951b9..35592ea36b1214 100644 --- a/src/coreclr/inc/debugmacros.h +++ b/src/coreclr/inc/debugmacros.h @@ -178,18 +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(); - -#else - -#define DbgGetEXETimeStamp() 0 - -#endif // _DEBUG && !FEATUREPAL - -#endif +#endif // __DebugMacros_h__ diff --git a/src/coreclr/utilcode/debug.cpp b/src/coreclr/utilcode/debug.cpp index 00866d255d7fd1..2d1354fd6f149e 100644 --- a/src/coreclr/utilcode/debug.cpp +++ b/src/coreclr/utilcode/debug.cpp @@ -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; diff --git a/src/coreclr/vm/i386/jitinterfacex86.cpp b/src/coreclr/vm/i386/jitinterfacex86.cpp index 3807b00a8ca6e1..986ecaa1bebd10 100644 --- a/src/coreclr/vm/i386/jitinterfacex86.cpp +++ b/src/coreclr/vm/i386/jitinterfacex86.cpp @@ -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. @@ -915,7 +913,7 @@ void InitJITHelpers1() // 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) { + 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)); @@ -959,7 +957,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 diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index f98a5cf58a2251..1d47c16b226447 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -1176,11 +1176,6 @@ void InitThreadManager() // to update the contracts if you remove this flag. g_DeadlockAwareCrst.Init(CrstDeadlockDetection, CRST_UNSAFE_ANYMODE); -#ifdef _DEBUG - // Randomize OBJREF_HASH to handle hash collision. - Thread::OBJREF_HASH = OBJREF_TABSIZE - (DbgGetEXETimeStamp()%10); -#endif // _DEBUG - ThreadSuspend::Initialize(); } From 22baf2d6615938d25ec7a06cee4678a93e2b6ac0 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Wed, 17 Jul 2024 20:47:58 -0700 Subject: [PATCH 4/6] Apply suggestions from code review Co-authored-by: Jan Kotas --- src/coreclr/vm/amd64/jitinterfaceamd64.cpp | 2 +- src/coreclr/vm/i386/jitinterfacex86.cpp | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/amd64/jitinterfaceamd64.cpp b/src/coreclr/vm/amd64/jitinterfaceamd64.cpp index 8d567406072030..f9cd2a968a02a7 100644 --- a/src/coreclr/vm/amd64/jitinterfaceamd64.cpp +++ b/src/coreclr/vm/amd64/jitinterfaceamd64.cpp @@ -698,7 +698,7 @@ 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 some good asserts + // The default slow write barrier has some good asserts if ((g_pConfig->GetHeapVerifyLevel() & EEConfig::HEAPVERIFY_BARRIERCHECK)) { break; } diff --git a/src/coreclr/vm/i386/jitinterfacex86.cpp b/src/coreclr/vm/i386/jitinterfacex86.cpp index 986ecaa1bebd10..3417f04c53817e 100644 --- a/src/coreclr/vm/i386/jitinterfacex86.cpp +++ b/src/coreclr/vm/i386/jitinterfacex86.cpp @@ -911,8 +911,7 @@ 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 + // 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] From 9e05852cfc2455d7db89f1e71465684c8eb900b0 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 17 Jul 2024 20:58:05 -0700 Subject: [PATCH 5/6] Keep back the randomness for object hashing. --- src/coreclr/vm/threads.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index 1d47c16b226447..2044f2be83fbe0 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -1176,6 +1176,11 @@ void InitThreadManager() // to update the contracts if you remove this flag. g_DeadlockAwareCrst.Init(CrstDeadlockDetection, CRST_UNSAFE_ANYMODE); +#ifdef _DEBUG + // Randomize OBJREF_HASH to handle hash collision. + Thread::OBJREF_HASH = OBJREF_TABSIZE - GetRandomInt(10); +#endif // _DEBUG + ThreadSuspend::Initialize(); } From 02a915ab544ac0565390048dee090d2b96e81b77 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Wed, 17 Jul 2024 21:30:07 -0700 Subject: [PATCH 6/6] Apply suggestions from code review Co-authored-by: Jan Kotas --- src/coreclr/vm/i386/jitinterfacex86.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/i386/jitinterfacex86.cpp b/src/coreclr/vm/i386/jitinterfacex86.cpp index 3417f04c53817e..0b6607aff6d2a8 100644 --- a/src/coreclr/vm/i386/jitinterfacex86.cpp +++ b/src/coreclr/vm/i386/jitinterfacex86.cpp @@ -912,7 +912,7 @@ void InitJITHelpers1() #ifdef WRITE_BARRIER_CHECK // Don't do the fancy optimization just jump to the old one // Use the slow one for write barrier checks build because it has some good asserts - if ((g_pConfig->GetHeapVerifyLevel() & EEConfig::HEAPVERIFY_BARRIERCHECK)) { + 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)); @@ -956,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)) + if (g_pConfig->GetHeapVerifyLevel() & EEConfig::HEAPVERIFY_BARRIERCHECK) return; #endif // WRITE_BARRIER_CHECK