From e2d749df91ce854b203c121900398cb78c94f96f Mon Sep 17 00:00:00 2001 From: Max Charlamb Date: Thu, 11 Jun 2026 12:50:33 -0400 Subject: [PATCH] Make NativeAOT StressLog binary-compatible with CoreCLR Align the NativeAOT StressLog struct layout to match CoreCLR's, so that diagnostic tools reading raw memory (SOS !DumpLog, clrmd) can read stress logs from both runtimes without layout-specific code paths. Changes: - Add padding field (0xFFFFFFFF sentinel) between logs and deadCount - Change lock from inline minipal_mutex to heap-allocated CrstStatic* (pointer-sized, matching CoreCLR's CRITSEC_COOKIE) - Add modules[MAX_MODULES=5] array with ModuleDesc struct - Update StressLog::LogMsg signature to (level, facility, cArgs, fmt) matching CoreCLR's convention - Update STRESS_LOG_WRITE macro and LogMsgOL overloads to pass level - Add static_asserts verifying field offsets match CoreCLR layout - Add null guard for lock in CreateThreadStressLog The lock change uses CrstStatic which wraps the same minipal_mutex primitives, so runtime behavior is identical. In Debug builds, CrstStatic adds thread ownership assertions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Runtime/datadescriptor/datadescriptor.inc | 11 +++- src/coreclr/nativeaot/Runtime/inc/stressLog.h | 64 +++++++++++++++---- src/coreclr/nativeaot/Runtime/stressLog.cpp | 29 +++++++-- 3 files changed, 86 insertions(+), 18 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/datadescriptor/datadescriptor.inc b/src/coreclr/nativeaot/Runtime/datadescriptor/datadescriptor.inc index 365abce8061d29..5b0f0de396a898 100644 --- a/src/coreclr/nativeaot/Runtime/datadescriptor/datadescriptor.inc +++ b/src/coreclr/nativeaot/Runtime/datadescriptor/datadescriptor.inc @@ -94,8 +94,16 @@ CDAC_TYPE_FIELD(StressLog, T_INT32, TotalChunks, offsetof(StressLog, totalChunk) CDAC_TYPE_FIELD(StressLog, T_POINTER, Logs, offsetof(StressLog, logs)) CDAC_TYPE_FIELD(StressLog, T_UINT64, TickFrequency, offsetof(StressLog, tickFrequency)) CDAC_TYPE_FIELD(StressLog, T_UINT64, StartTimestamp, offsetof(StressLog, startTimeStamp)) +CDAC_TYPE_FIELD(StressLog, T_NUINT, ModuleOffset, offsetof(StressLog, moduleOffset)) +CDAC_TYPE_FIELD(StressLog, T_ARRAY(TYPE(StressLogModuleDesc)), Modules, offsetof(StressLog, modules)) CDAC_TYPE_END(StressLog) +CDAC_TYPE_BEGIN(StressLogModuleDesc) +CDAC_TYPE_SIZE(sizeof(StressLog::ModuleDesc)) +CDAC_TYPE_FIELD(StressLogModuleDesc, T_POINTER, BaseAddress, offsetof(StressLog::ModuleDesc, baseAddress)) +CDAC_TYPE_FIELD(StressLogModuleDesc, T_NUINT, Size, offsetof(StressLog::ModuleDesc, size)) +CDAC_TYPE_END(StressLogModuleDesc) + CDAC_TYPE_BEGIN(ThreadStressLog) CDAC_TYPE_INDETERMINATE(ThreadStressLog) CDAC_TYPE_FIELD(ThreadStressLog, T_POINTER, Next, cdac_data::Next) @@ -152,10 +160,11 @@ CDAC_GLOBAL(ObjectToMethodTableUnmask, T_UINT8, 7) CDAC_GLOBAL(ObjectToMethodTableUnmask, T_UINT8, 3) #endif -// StressLog globals (no module table in NativeAOT) +// StressLog globals CDAC_GLOBAL(StressLogEnabled, T_UINT8, 1) CDAC_GLOBAL_POINTER(StressLog, &StressLog::theLog) CDAC_GLOBAL(StressLogHasModuleTable, T_UINT8, 0) +CDAC_GLOBAL(StressLogMaxModules, T_UINT64, StressLog::MAX_MODULES) CDAC_GLOBAL(StressLogChunkSize, T_UINT32, STRESSLOG_CHUNK_SIZE) CDAC_GLOBAL(StressLogValidChunkSig, T_UINT32, 0xCFCFCFCF) CDAC_GLOBAL(StressLogMaxMessageSize, T_UINT64, (uint64_t)StressMsg::maxMsgSize) diff --git a/src/coreclr/nativeaot/Runtime/inc/stressLog.h b/src/coreclr/nativeaot/Runtime/inc/stressLog.h index 91c72b0fac58d8..619fcb1ad85aa9 100644 --- a/src/coreclr/nativeaot/Runtime/inc/stressLog.h +++ b/src/coreclr/nativeaot/Runtime/inc/stressLog.h @@ -49,6 +49,8 @@ #include "cdacdata.h" +class CrstStatic; + // // Logging levels and facilities // @@ -113,12 +115,12 @@ enum LogFacilitiesEnum: unsigned int { #define STRESS_LOG_WRITE(facility, level, msg, ...) do { \ if (StressLog::StressLogOn(facility, level)) \ - StressLog::LogMsgOL(facility, msg, __VA_ARGS__); \ + StressLog::LogMsgOL(facility, level, msg, __VA_ARGS__); \ } WHILE_0 #define STRESS_LOG0(facility, level, msg) do { \ if (StressLog::StressLogOn(facility, level)) \ - StressLog::LogMsg(facility, 0, msg); \ + StressLog::LogMsg(level, facility, 0, msg); \ } WHILE_0 \ #define STRESS_LOG1(facility, level, msg, data1) \ @@ -153,32 +155,32 @@ enum LogFacilitiesEnum: unsigned int { #define STRESS_LOG_PLUG_MOVE(plug_start, plug_end, plug_delta) do { \ if (StressLog::StressLogOn(LF_GC, LL_INFO1000)) \ - StressLog::LogMsg(LF_GC, 3, ThreadStressLog::gcPlugMoveMsg(), \ + StressLog::LogMsg(LL_INFO1000, LF_GC, 3, ThreadStressLog::gcPlugMoveMsg(), \ (void*)(size_t)(plug_start), (void*)(size_t)(plug_end), (void*)(size_t)(plug_delta)); \ } WHILE_0 #define STRESS_LOG_ROOT_PROMOTE(root_addr, objPtr, methodTable) do { \ if (StressLog::StressLogOn(LF_GC|LF_GCROOTS, LL_INFO1000)) \ - StressLog::LogMsg(LF_GC|LF_GCROOTS, 3, ThreadStressLog::gcRootPromoteMsg(), \ + StressLog::LogMsg(LL_INFO1000, LF_GC|LF_GCROOTS, 3, ThreadStressLog::gcRootPromoteMsg(), \ (void*)(size_t)(root_addr), (void*)(size_t)(objPtr), (void*)(size_t)(methodTable)); \ } WHILE_0 #define STRESS_LOG_ROOT_RELOCATE(root_addr, old_value, new_value, methodTable) do { \ if (StressLog::StressLogOn(LF_GC|LF_GCROOTS, LL_INFO1000) && ((size_t)(old_value) != (size_t)(new_value))) \ - StressLog::LogMsg(LF_GC|LF_GCROOTS, 4, ThreadStressLog::gcRootMsg(), \ + StressLog::LogMsg(LL_INFO1000, LF_GC|LF_GCROOTS, 4, ThreadStressLog::gcRootMsg(), \ (void*)(size_t)(root_addr), (void*)(size_t)(old_value), \ (void*)(size_t)(new_value), (void*)(size_t)(methodTable)); \ } WHILE_0 #define STRESS_LOG_GC_START(gcCount, Gen, collectClasses) do { \ if (StressLog::StressLogOn(LF_GCROOTS|LF_GC|LF_GCALLOC, LL_INFO10)) \ - StressLog::LogMsg(LF_GCROOTS|LF_GC|LF_GCALLOC, 3, ThreadStressLog::gcStartMsg(), \ + StressLog::LogMsg(LL_INFO10, LF_GCROOTS|LF_GC|LF_GCALLOC, 3, ThreadStressLog::gcStartMsg(), \ (void*)(size_t)(gcCount), (void*)(size_t)(Gen), (void*)(size_t)(collectClasses)); \ } WHILE_0 #define STRESS_LOG_GC_END(gcCount, Gen, collectClasses) do { \ if (StressLog::StressLogOn(LF_GCROOTS|LF_GC|LF_GCALLOC, LL_INFO10)) \ - StressLog::LogMsg(LF_GCROOTS|LF_GC|LF_GCALLOC, 3, ThreadStressLog::gcEndMsg(),\ + StressLog::LogMsg(LL_INFO10, LF_GCROOTS|LF_GC|LF_GCALLOC, 3, ThreadStressLog::gcEndMsg(),\ (void*)(size_t)(gcCount), (void*)(size_t)(Gen), (void*)(size_t)(collectClasses), 0);\ } WHILE_0 @@ -233,12 +235,20 @@ class StressLog { unsigned MaxSizeTotal; // maximum memory allowed for stress log int32_t totalChunk; // current number of total chunks allocated PTR_ThreadStressLog logs; // the list of logs for every thread. + unsigned padding; // Preserve the layout for SOS int32_t deadCount; // count of dead threads in the log - minipal_mutex lock; // lock + CrstStatic* lock; // lock (heap-allocated, pointer-sized for CoreCLR layout compat) uint64_t tickFrequency; // number of ticks per second uint64_t startTimeStamp; // start time from when tick counter started uint64_t startTime; // time the application started in Windows FILETIME precision (100ns since 01 Jan 1601) size_t moduleOffset; // Used to compute format strings. + struct ModuleDesc + { + uint8_t* baseAddress; + size_t size; + }; + static const size_t MAX_MODULES = 5; + ModuleDesc modules[MAX_MODULES]; // descriptor of the modules images #ifndef DACCESS_COMPILE public: @@ -300,7 +310,7 @@ class StressLog { #endif } - static void LogMsg(unsigned facility, int cArgs, const char* format, ... ); + static void LogMsg(unsigned level, unsigned facility, int cArgs, const char* format, ... ); // Support functions for STRESS_LOG_VA // We disable the warning "conversion from 'type' to 'type' of greater size" since everything will @@ -320,13 +330,13 @@ class StressLog { template static void LogMsgOL(const char* format, Ts... args) { - LogMsg(LF_GC, sizeof...(args), format, ConvertArgument(args)...); + LogMsg(LL_ALWAYS, LF_GC, sizeof...(args), format, ConvertArgument(args)...); } template - static void LogMsgOL(unsigned facility, const char* format, Ts... args) + static void LogMsgOL(unsigned facility, unsigned level, const char* format, Ts... args) { - LogMsg(facility, sizeof...(args), format, ConvertArgument(args)...); + LogMsg(level, facility, sizeof...(args), format, ConvertArgument(args)...); } #ifdef _MSC_VER @@ -344,6 +354,36 @@ class StressLog { static StressLog theLog; // We only have one log, and this is it }; +// Verify that the NativeAOT StressLog layout is binary-compatible with CoreCLR. +// The lock field must be pointer-sized (matching CoreCLR's CRITSEC_COOKIE), and +// all fields that diagnostic tools read must be at the same offsets. +static_assert(sizeof(StressLog::theLog.lock) == sizeof(void*), + "StressLog::lock must be pointer-sized for CoreCLR layout compatibility"); +static_assert(offsetof(StressLog, facilitiesToLog) == 0, "facilitiesToLog offset mismatch"); +static_assert(offsetof(StressLog, levelToLog) == 4, "levelToLog offset mismatch"); +static_assert(offsetof(StressLog, MaxSizePerThread) == 8, "MaxSizePerThread offset mismatch"); +static_assert(offsetof(StressLog, MaxSizeTotal) == 12, "MaxSizeTotal offset mismatch"); +static_assert(offsetof(StressLog, totalChunk) == 16, "totalChunk offset mismatch"); +static_assert(offsetof(StressLog, padding) == offsetof(StressLog, logs) + sizeof(void*), + "padding must follow logs"); +static_assert(offsetof(StressLog, deadCount) == offsetof(StressLog, padding) + sizeof(unsigned), + "deadCount must follow padding"); +static_assert(offsetof(StressLog, lock) == offsetof(StressLog, deadCount) + sizeof(int32_t), + "lock must follow deadCount"); +// Note: on 32-bit platforms the compiler may insert alignment padding between +// lock (pointer-sized) and tickFrequency (uint64_t). This matches CoreCLR's +// layout since CRITSEC_COOKIE is also pointer-sized. +static_assert(offsetof(StressLog, tickFrequency) > offsetof(StressLog, lock), + "tickFrequency must be after lock"); +static_assert(offsetof(StressLog, startTimeStamp) == offsetof(StressLog, tickFrequency) + sizeof(uint64_t), + "startTimeStamp must follow tickFrequency"); +static_assert(offsetof(StressLog, startTime) == offsetof(StressLog, startTimeStamp) + sizeof(uint64_t), + "startTime must follow startTimeStamp"); +static_assert(offsetof(StressLog, moduleOffset) == offsetof(StressLog, startTime) + sizeof(uint64_t), + "moduleOffset must follow startTime"); +static_assert(offsetof(StressLog, modules) == offsetof(StressLog, moduleOffset) + sizeof(size_t), + "modules must follow moduleOffset"); + #if TARGET_64BIT template<> diff --git a/src/coreclr/nativeaot/Runtime/stressLog.cpp b/src/coreclr/nativeaot/Runtime/stressLog.cpp index be3d69b30d1e56..1fcba12547ce32 100644 --- a/src/coreclr/nativeaot/Runtime/stressLog.cpp +++ b/src/coreclr/nativeaot/Runtime/stressLog.cpp @@ -17,6 +17,7 @@ #include "Pal.h" #include "daccess.h" #include "stressLog.h" +#include "Crst.h" #include "holder.h" #include "rhassert.h" #include "slist.h" @@ -79,7 +80,7 @@ uint64_t getTickFrequency() #endif // DACCESS_COMPILE -StressLog StressLog::theLog = { 0, 0, 0, 0, 0, 0 }; +StressLog StressLog::theLog = { 0, 0, 0, 0, 0, 0, 0xFFFFFFFF, 0 }; const static uint64_t RECYCLE_AGE = 0x40000000L; // after a billion cycles, we can discard old threads /*********************************************************************************/ @@ -95,8 +96,11 @@ void StressLog::Initialize(unsigned facilities, unsigned level, unsigned maxByt return; } - bool success = minipal_mutex_init(&theLog.lock); - _ASSERTE(success); + theLog.lock = new (nothrow) CrstStatic(); + if (theLog.lock != NULL) + { + theLog.lock->Init(CrstStressLog); + } g_pStressLog = &theLog; if (maxBytesPerThread < STRESSLOG_CHUNK_SIZE) @@ -114,6 +118,7 @@ void StressLog::Initialize(unsigned facilities, unsigned level, unsigned maxByt theLog.facilitiesToLog = facilities | LF_ALWAYS; theLog.levelToLog = level; theLog.deadCount = 0; + theLog.padding = 0xFFFFFFFF; theLog.tickFrequency = getTickFrequency(); @@ -121,6 +126,15 @@ void StressLog::Initialize(unsigned facilities, unsigned level, unsigned maxByt theLog.startTimeStamp = getTimeStamp(); theLog.moduleOffset = (size_t)hMod; // HMODULES are base addresses. + + // Initialize module table (single module for NativeAOT) + theLog.modules[0].baseAddress = (uint8_t*)hMod; + theLog.modules[0].size = 0; // Size will be set if needed + for (size_t i = 1; i < MAX_MODULES; i++) + { + theLog.modules[i].baseAddress = nullptr; + theLog.modules[i].size = 0; + } } /*********************************************************************************/ @@ -145,7 +159,12 @@ ThreadStressLog* StressLog::CreateThreadStressLog(Thread * pThread) { return NULL; } - minipal::MutexHolder holder(theLog.lock); + if (theLog.lock == NULL) + { + return NULL; + } + + CrstHolder holder(theLog.lock); msgs = CreateThreadStressLogHelper(pThread); return msgs; @@ -352,7 +371,7 @@ void ThreadStressLog::Activate (Thread * pThread) } /* static */ -void StressLog::LogMsg (unsigned facility, int cArgs, const char* format, ... ) +void StressLog::LogMsg (unsigned level, unsigned facility, int cArgs, const char* format, ... ) { _ASSERTE ( cArgs >= 0 && (size_t)cArgs <= StressMsg::maxArgCnt );