Skip to content

Replace Win32 PAL implementation with minipal implementation #115858

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

Open
wants to merge 38 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
27fe11e
Reduce critsect.h header usage
AaronRobinsonMSFT May 18, 2025
9f60e1f
Remove typedef options for CRITICAL_SECTION
AaronRobinsonMSFT May 18, 2025
8bca213
Create PAL critical section
AaronRobinsonMSFT May 18, 2025
751ad38
Convert C++ API to C
AaronRobinsonMSFT May 19, 2025
1f2185e
Remove unused ThreadStore::CanAcquireLock().
AaronRobinsonMSFT May 19, 2025
4afc04f
Remove critsec API from coreclr/minipal
AaronRobinsonMSFT May 19, 2025
0cbab28
Add critical section API to native/minipal.
AaronRobinsonMSFT May 19, 2025
6bd8aa6
WIP
AaronRobinsonMSFT May 20, 2025
3a76f62
Rename critsec.h to critsect.h
AaronRobinsonMSFT May 20, 2025
9720e8a
Consistent naming
AaronRobinsonMSFT May 20, 2025
79aa481
Build on Windows
AaronRobinsonMSFT May 20, 2025
1494bcb
Fix macOS build
AaronRobinsonMSFT May 20, 2025
2dbedab
Changes to PAL
AaronRobinsonMSFT May 20, 2025
3d687c0
Non-windows WIP
AaronRobinsonMSFT May 20, 2025
a416750
WIP
AaronRobinsonMSFT May 20, 2025
22cee67
Fix build on macos
AaronRobinsonMSFT May 21, 2025
76b9ad8
WIP
AaronRobinsonMSFT May 22, 2025
0d385dc
DAC fixes
AaronRobinsonMSFT May 22, 2025
568c374
Merge remote-tracking branch 'upstream/main' into replace_win32_critsec
AaronRobinsonMSFT May 22, 2025
6153476
Unused local
AaronRobinsonMSFT May 22, 2025
585cd0e
Remove PAL tests for CRITICAL_SECTION
AaronRobinsonMSFT May 22, 2025
5fe82e0
Feedback and jit format
AaronRobinsonMSFT May 22, 2025
35343bf
Increase linux 64-bit max size.
AaronRobinsonMSFT May 22, 2025
e9b03d6
Feedback and build breaks.
AaronRobinsonMSFT May 22, 2025
2cc6108
Merge remote-tracking branch 'upstream/main' into replace_win32_critsec
AaronRobinsonMSFT May 22, 2025
090b690
Update Windows max size.
AaronRobinsonMSFT May 22, 2025
2429a6a
Fix up native AOT PAL for unix
AaronRobinsonMSFT May 22, 2025
3b51431
FIx enter call that was accidentally made leave.
AaronRobinsonMSFT May 23, 2025
111fd38
Feedback
AaronRobinsonMSFT May 23, 2025
ca603d3
Merge remote-tracking branch 'upstream/main' into replace_win32_critsec
AaronRobinsonMSFT May 23, 2025
522fcc6
Merge remote-tracking branch 'upstream/main' into replace_win32_critsec
AaronRobinsonMSFT May 27, 2025
58d5368
Feedback - first pass
AaronRobinsonMSFT May 27, 2025
0f79bf9
Add back __stdcall
AaronRobinsonMSFT May 27, 2025
7195c0f
Remove __stdcall usage
AaronRobinsonMSFT May 27, 2025
df8beda
Feedback - second pass.
AaronRobinsonMSFT May 27, 2025
3b20072
Fix cross build
AaronRobinsonMSFT May 27, 2025
9c5779a
Fix redefinition error.
AaronRobinsonMSFT May 27, 2025
df2a347
Remove unnecessary define
AaronRobinsonMSFT May 27, 2025
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
6 changes: 3 additions & 3 deletions src/coreclr/debug/daccess/daccess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ extern TADDR g_ClrModuleBase;
// To include definition of IsThrowableThreadAbortException
// #include <exstatecommon.h>

CRITICAL_SECTION g_dacCritSec;
minipal_critsect g_dacCritSec;
ClrDataAccess* g_dacImpl;

EXTERN_C BOOL WINAPI DllMain2(HANDLE instance, DWORD reason, LPVOID reserved)
Expand Down Expand Up @@ -72,7 +72,7 @@ EXTERN_C BOOL WINAPI DllMain2(HANDLE instance, DWORD reason, LPVOID reserved)
return FALSE;
}
#endif
InitializeCriticalSection(&g_dacCritSec);
minipal_critsect_init(&g_dacCritSec);

g_procInitialized = true;
break;
Expand All @@ -82,7 +82,7 @@ EXTERN_C BOOL WINAPI DllMain2(HANDLE instance, DWORD reason, LPVOID reserved)
// It's possible for this to be called without ATTACH completing (eg. if it failed)
if (g_procInitialized)
{
DeleteCriticalSection(&g_dacCritSec);
minipal_critsect_destroy(&g_dacCritSec);
}
g_procInitialized = false;
break;
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/debug/daccess/dacdbiimpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -1116,7 +1116,7 @@ class DDHolder
public:
DDHolder(DacDbiInterfaceImpl* pContainer, bool fAllowReentrant)
{
EnterCriticalSection(&g_dacCritSec);
minipal_critsect_enter(&g_dacCritSec);

// If we're not re-entrant, then assert.
if (!fAllowReentrant)
Expand All @@ -1139,7 +1139,7 @@ class DDHolder
g_dacImpl = m_pOldContainer;
g_pAllocator = m_pOldAllocator;

LeaveCriticalSection(&g_dacCritSec);
minipal_critsect_leave(&g_dacCritSec);
}

protected:
Expand Down
11 changes: 6 additions & 5 deletions src/coreclr/debug/daccess/dacimpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#ifndef __DACIMPL_H__
#define __DACIMPL_H__

#include <minipal/critsect.h>
#include "gcinterface.dac.h"
//---------------------------------------------------------------------------------------
// Setting DAC_HASHTABLE tells the DAC to use the hand rolled hashtable for
Expand All @@ -26,7 +27,7 @@
#include <unordered_map>
#pragma pop_macro("return")
#endif //DAC_HASHTABLE
extern CRITICAL_SECTION g_dacCritSec;
extern minipal_critsect g_dacCritSec;

// Convert between CLRDATA_ADDRESS and TADDR.
// Note that CLRDATA_ADDRESS is sign-extended (for compat with Windbg and OS conventions). Converting
Expand Down Expand Up @@ -3809,26 +3810,26 @@ class EnumMethodInstances
//----------------------------------------------------------------------------

#define DAC_ENTER() \
EnterCriticalSection(&g_dacCritSec); \
minipal_critsect_enter(&g_dacCritSec); \
ClrDataAccess* __prevDacImpl = g_dacImpl; \
g_dacImpl = this;

// When entering a child object we validate that
// the process's host instance cache hasn't been flushed
// since the child was created.
#define DAC_ENTER_SUB(dac) \
EnterCriticalSection(&g_dacCritSec); \
minipal_critsect_enter(&g_dacCritSec); \
if (dac->m_instanceAge != m_instanceAge) \
{ \
LeaveCriticalSection(&g_dacCritSec); \
minipal_critsect_leave(&g_dacCritSec); \
return E_INVALIDARG; \
} \
ClrDataAccess* __prevDacImpl = g_dacImpl; \
g_dacImpl = (dac)

#define DAC_LEAVE() \
g_dacImpl = __prevDacImpl; \
LeaveCriticalSection(&g_dacCritSec)
minipal_critsect_leave(&g_dacCritSec)


#define SOSHelperEnter() \
Expand Down
9 changes: 4 additions & 5 deletions src/coreclr/debug/di/rspriv.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
#include <windows.h>

#include <utilcode.h>

#include <minipal/critsect.h>

#ifdef _DEBUG
#define LOGGING
Expand Down Expand Up @@ -798,7 +798,7 @@ class RSLock
}


CRITICAL_SECTION m_lock;
minipal_critsect m_lock;

#ifdef _DEBUG
public:
Expand Down Expand Up @@ -840,8 +840,7 @@ typedef RSLock::RSLockHolder RSLockHolder;
typedef RSLock::RSInverseLockHolder RSInverseLockHolder;

// In the RS, we should be using RSLocks instead of raw critical sections.
#define CRITICAL_SECTION USE_RSLOCK_INSTEAD_OF_CRITICAL_SECTION

#define minipal_critsect USE_RSLOCK_INSTEAD_OF_DN_CRIT_SEC

/* ------------------------------------------------------------------------- *
* Helper macros. Use the ATT_* macros below instead of these.
Expand Down Expand Up @@ -11200,7 +11199,7 @@ inline CordbEval * UnwrapCookieCordbEval(CordbProcess *pProc, UINT cookie)


// We defined this at the top of the file - undef it now so that we don't pollute other files.
#undef CRITICAL_SECTION
#undef minipal_critsect


#ifdef RSCONTRACTS
Expand Down
10 changes: 6 additions & 4 deletions src/coreclr/debug/di/rspriv.inl
Original file line number Diff line number Diff line change
Expand Up @@ -528,14 +528,15 @@ inline void RSLock::Init(const char * szTag, int eAttr, ERSLockLevel level)

_ASSERTE(IsInit());

InitializeCriticalSection(&m_lock);
bool init = minipal_critsect_init(&m_lock);
_ASSERTE(init);
}

// Cleanup a lock.
inline void RSLock::Destroy()
{
CONSISTENCY_CHECK_MSGF(IsInit(), ("RSLock '%s' not inited", m_szTag));
DeleteCriticalSection(&m_lock);
minipal_critsect_destroy(&m_lock);

#ifdef _DEBUG
m_eAttr = cLockUninit; // No longer initialized.
Expand All @@ -549,10 +550,11 @@ inline void RSLock::Lock()

#ifdef RSCONTRACTS
DbgRSThread * pThread = DbgRSThread::GetThread();

pThread->NotifyTakeLock(this);
#endif

EnterCriticalSection(&m_lock);
minipal_critsect_enter(&m_lock);
#ifdef _DEBUG
m_tidOwner = ::GetCurrentThreadId();
m_count++;
Expand Down Expand Up @@ -583,7 +585,7 @@ inline void RSLock::Unlock()
pThread->NotifyReleaseLock(this);
#endif

LeaveCriticalSection(&m_lock);
minipal_critsect_leave(&m_lock);
}

template <class T>
Expand Down
46 changes: 23 additions & 23 deletions src/coreclr/debug/inc/dbgtransportsession.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
#ifndef RIGHT_SIDE_COMPILE
#include <utilcode.h>
#include <crst.h>

#endif // !RIGHT_SIDE_COMPILE

#include <minipal/critsect.h>
#include <minipal/guid.h>

#if defined(FEATURE_DBGIPC_TRANSPORT_VM) || defined(FEATURE_DBGIPC_TRANSPORT_DI)
Expand Down Expand Up @@ -271,7 +271,7 @@ inline UINT32 DBGIPC_HTONL(UINT32 x)

// Lock abstraction (we can't use the same lock implementation on LS and RS since we really want a Crst on the
// LS and this isn't available in the RS environment).
class DbgTransportLock
class DbgTransportLock final
{
public:
void Init();
Expand All @@ -281,12 +281,32 @@ class DbgTransportLock

private:
#ifdef RIGHT_SIDE_COMPILE
CRITICAL_SECTION m_sLock;
minipal_critsect m_sLock;
#else // RIGHT_SIDE_COMPILE
CrstExplicitInit m_sLock;
#endif // RIGHT_SIDE_COMPILE
};

class TransportLockHolder final
{
DbgTransportLock& _lock;
public:
TransportLockHolder(DbgTransportLock& lock)
: _lock(lock)
{
_lock.Enter();
}
~TransportLockHolder()
{
_lock.Leave();
}

TransportLockHolder(TransportLockHolder const&) = delete;
TransportLockHolder& operator=(TransportLockHolder const&) = delete;
TransportLockHolder(TransportLockHolder&& other) = delete;
TransportLockHolder&& operator=(TransportLockHolder&&) = delete;
};

// The transport has only one queue for IPC events, but each IPC event can be marked as one of two types.
// The transport will signal the handle corresponding to the type of each IPC event. (See
// code:DbgTransportSession::GetIPCEventReadyEvent and code:DbgTransportSession::GetDebugEventReadyEvent.)
Expand Down Expand Up @@ -555,26 +575,6 @@ class DbgTransportSession
}
};

// Holder class used to take a transport lock in a given scope and automatically release it once that
// scope is exited.
class TransportLockHolder
{
public:
TransportLockHolder(DbgTransportLock *pLock)
{
m_pLock = pLock;
m_pLock->Enter();
}

~TransportLockHolder()
{
m_pLock->Leave();
}

private:
DbgTransportLock *m_pLock;
};

#ifdef _DEBUG
// Store statistics for various session activities that will be useful for performance analysis and tracking
// down bugs.
Expand Down
Loading
Loading