From f6c07a0abb6172a84c843d8b578603fd96bd9494 Mon Sep 17 00:00:00 2001 From: Mitchell Hwang Date: Fri, 6 Feb 2026 13:00:49 -0500 Subject: [PATCH 1/5] Add null check for AppDomain in early-startup PerfMap::Enable AppDomain is created in SystemDomain::Attach, which runs after DiagnosticServerAdapter::Initialize. A PerfMapEnable IPC command received during PauseForDiagnosticsMonitor would crash calling IterateAssembliesEx on null. Safe to skip: no assemblies are loaded before SystemDomain::Attach. --- src/coreclr/vm/perfmap.cpp | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/src/coreclr/vm/perfmap.cpp b/src/coreclr/vm/perfmap.cpp index f2cc1212595891..75df7b4c4c9708 100644 --- a/src/coreclr/vm/perfmap.cpp +++ b/src/coreclr/vm/perfmap.cpp @@ -121,26 +121,30 @@ void PerfMap::Enable(PerfMapType type, bool sendExisting) if (sendExisting) { - AppDomain::AssemblyIterator assemblyIterator = GetAppDomain()->IterateAssembliesEx( - (AssemblyIterationFlags)(kIncludeLoaded | kIncludeExecution)); - CollectibleAssemblyHolder pAssembly; - while (assemblyIterator.Next(pAssembly.This())) + AppDomain *pAppDomain = GetAppDomain(); + if (pAppDomain != nullptr) { - // PerfMap does not log R2R methods so only proceed if we are emitting jitdumps - if (type == PerfMapType::ALL || type == PerfMapType::JITDUMP) + AppDomain::AssemblyIterator assemblyIterator = pAppDomain->IterateAssembliesEx( + (AssemblyIterationFlags)(kIncludeLoaded | kIncludeExecution)); + CollectibleAssemblyHolder pAssembly; + while (assemblyIterator.Next(pAssembly.This())) { - Module *pModule = pAssembly->GetModule(); - if (pModule->IsReadyToRun()) + // PerfMap does not log R2R methods so only proceed if we are emitting jitdumps + if (type == PerfMapType::ALL || type == PerfMapType::JITDUMP) { - ReadyToRunInfo::MethodIterator mi(pModule->GetReadyToRunInfo()); - - while (mi.Next()) + Module *pModule = pAssembly->GetModule(); + if (pModule->IsReadyToRun()) { - // Call GetMethodDesc_NoRestore instead of GetMethodDesc to avoid restoring methods. - MethodDesc *hotDesc = (MethodDesc *)mi.GetMethodDesc_NoRestore(); - if (hotDesc != nullptr && hotDesc->GetNativeCode() != (PCODE)NULL) + ReadyToRunInfo::MethodIterator mi(pModule->GetReadyToRunInfo()); + + while (mi.Next()) { - PerfMap::LogPreCompiledMethod(hotDesc, hotDesc->GetNativeCode()); + // Call GetMethodDesc_NoRestore instead of GetMethodDesc to avoid restoring methods. + MethodDesc *hotDesc = (MethodDesc *)mi.GetMethodDesc_NoRestore(); + if (hotDesc != nullptr && hotDesc->GetNativeCode() != (PCODE)NULL) + { + PerfMap::LogPreCompiledMethod(hotDesc, hotDesc->GetNativeCode()); + } } } } From 62d36a5e028271625f6532f636e70a1383d29d4c Mon Sep 17 00:00:00 2001 From: Mitchell Hwang Date: Fri, 6 Feb 2026 13:01:16 -0500 Subject: [PATCH 2/5] Add null check for EEJitManager in early-startup PerfMap::Enable EEJitManager is created in ExecutionManager::Init, which runs after DiagnosticServerAdapter::Initialize. A PerfMapEnable IPC command received during PauseForDiagnosticsMonitor would crash on null dereference. Safe to skip: no JIT'd code exists before ExecutionManager::Init. CodeVersionManager::LockHolder remains inside the null check intentionally - the lock is only needed while iterating code versions, not for the null check. The lock is already initialized in CodeVersionManager::StaticInitialize before DiagnosticServerAdapter::Initialize. --- src/coreclr/vm/perfmap.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/perfmap.cpp b/src/coreclr/vm/perfmap.cpp index 75df7b4c4c9708..3a3901dc601af6 100644 --- a/src/coreclr/vm/perfmap.cpp +++ b/src/coreclr/vm/perfmap.cpp @@ -121,6 +121,9 @@ void PerfMap::Enable(PerfMapType type, bool sendExisting) if (sendExisting) { + // When Enable is called very early in startup (e.g., via DiagnosticServer IPC before + // SystemDomain::Attach), the AppDomain may not exist yet. In this case, skip assembly + // iteration since no assemblies are loaded anyway. Future JIT'd methods will still be logged. AppDomain *pAppDomain = GetAppDomain(); if (pAppDomain != nullptr) { @@ -151,11 +154,15 @@ void PerfMap::Enable(PerfMapType type, bool sendExisting) } } + // Similarly, the EEJitManager may not exist yet if called before ExecutionManager::Init(). + // In this case, there's no JIT'd code to log anyway. + EEJitManager *pJitManager = ExecutionManager::GetEEJitManager(); + if (pJitManager != nullptr) { #ifdef FEATURE_CODE_VERSIONING CodeVersionManager::LockHolder codeVersioningLockHolder; #endif // FEATURE_CODE_VERSIONING - CodeHeapIterator heapIterator = ExecutionManager::GetEEJitManager()->GetCodeHeapIterator(); + CodeHeapIterator heapIterator = pJitManager->GetCodeHeapIterator(); while (heapIterator.Next()) { MethodDesc * pMethod = heapIterator.GetMethod(); From c2ebd886748a00ef08532fead919bf51b76dcffe Mon Sep 17 00:00:00 2001 From: Mitchell Hwang Date: Sat, 7 Feb 2026 12:43:04 -0500 Subject: [PATCH 3/5] Use Volatile flag instead of null checks for early-startup guard The previous null checks on GetAppDomain() and GetEEJitManager() have race conditions because statics like m_pEEJitManager are not Volatile. Use a Volatile s_sendExistingReady flag instead. The flag is set by PerfMap::SignalSendExistingReady, called from EEStartup after ExecutionManager::Init. When Enable is called before the flag is set, sendExisting iteration is skipped since no assemblies are loaded and no code is JIT'd anyway. --- src/coreclr/vm/ceemain.cpp | 4 +++ src/coreclr/vm/perfmap.cpp | 62 ++++++++++++++++++++++---------------- src/coreclr/vm/perfmap.h | 6 ++++ 3 files changed, 46 insertions(+), 26 deletions(-) diff --git a/src/coreclr/vm/ceemain.cpp b/src/coreclr/vm/ceemain.cpp index 372222dce0ffe7..fcb6581f831b38 100644 --- a/src/coreclr/vm/ceemain.cpp +++ b/src/coreclr/vm/ceemain.cpp @@ -824,6 +824,10 @@ void EEStartupHelper() ExecutionManager::Init(); +#ifdef FEATURE_PERFMAP + PerfMap::SignalSendExistingReady(); +#endif + JitHost::Init(); #ifndef TARGET_UNIX diff --git a/src/coreclr/vm/perfmap.cpp b/src/coreclr/vm/perfmap.cpp index 3a3901dc601af6..ac6a0bb866204a 100644 --- a/src/coreclr/vm/perfmap.cpp +++ b/src/coreclr/vm/perfmap.cpp @@ -28,6 +28,7 @@ #endif Volatile PerfMap::s_enabled = false; +Volatile PerfMap::s_sendExistingReady = false; PerfMap * PerfMap::s_Current = nullptr; bool PerfMap::s_ShowOptimizationTiers = false; bool PerfMap::s_GroupStubsOfSameType = false; @@ -122,47 +123,47 @@ void PerfMap::Enable(PerfMapType type, bool sendExisting) if (sendExisting) { // When Enable is called very early in startup (e.g., via DiagnosticServer IPC before - // SystemDomain::Attach), the AppDomain may not exist yet. In this case, skip assembly - // iteration since no assemblies are loaded anyway. Future JIT'd methods will still be logged. - AppDomain *pAppDomain = GetAppDomain(); - if (pAppDomain != nullptr) + // SystemDomain::Attach and ExecutionManager::Init), the AppDomain and EEJitManager + // may not exist yet. We use s_sendExistingReady (a Volatile) to guard against + // this, rather than null-checking individual pointers which would have race conditions + // due to non-Volatile statics like m_pEEJitManager. + // Safe to skip: no assemblies are loaded and no code is JIT'd at that point. + if (!s_sendExistingReady) { - AppDomain::AssemblyIterator assemblyIterator = pAppDomain->IterateAssembliesEx( - (AssemblyIterationFlags)(kIncludeLoaded | kIncludeExecution)); - CollectibleAssemblyHolder pAssembly; - while (assemblyIterator.Next(pAssembly.This())) + return; + } + + AppDomain::AssemblyIterator assemblyIterator = GetAppDomain()->IterateAssembliesEx( + (AssemblyIterationFlags)(kIncludeLoaded | kIncludeExecution)); + CollectibleAssemblyHolder pAssembly; + while (assemblyIterator.Next(pAssembly.This())) + { + // PerfMap does not log R2R methods so only proceed if we are emitting jitdumps + if (type == PerfMapType::ALL || type == PerfMapType::JITDUMP) { - // PerfMap does not log R2R methods so only proceed if we are emitting jitdumps - if (type == PerfMapType::ALL || type == PerfMapType::JITDUMP) + Module *pModule = pAssembly->GetModule(); + if (pModule->IsReadyToRun()) { - Module *pModule = pAssembly->GetModule(); - if (pModule->IsReadyToRun()) - { - ReadyToRunInfo::MethodIterator mi(pModule->GetReadyToRunInfo()); + ReadyToRunInfo::MethodIterator mi(pModule->GetReadyToRunInfo()); - while (mi.Next()) + while (mi.Next()) + { + // Call GetMethodDesc_NoRestore instead of GetMethodDesc to avoid restoring methods. + MethodDesc *hotDesc = (MethodDesc *)mi.GetMethodDesc_NoRestore(); + if (hotDesc != nullptr && hotDesc->GetNativeCode() != (PCODE)NULL) { - // Call GetMethodDesc_NoRestore instead of GetMethodDesc to avoid restoring methods. - MethodDesc *hotDesc = (MethodDesc *)mi.GetMethodDesc_NoRestore(); - if (hotDesc != nullptr && hotDesc->GetNativeCode() != (PCODE)NULL) - { - PerfMap::LogPreCompiledMethod(hotDesc, hotDesc->GetNativeCode()); - } + PerfMap::LogPreCompiledMethod(hotDesc, hotDesc->GetNativeCode()); } } } } } - // Similarly, the EEJitManager may not exist yet if called before ExecutionManager::Init(). - // In this case, there's no JIT'd code to log anyway. - EEJitManager *pJitManager = ExecutionManager::GetEEJitManager(); - if (pJitManager != nullptr) { #ifdef FEATURE_CODE_VERSIONING CodeVersionManager::LockHolder codeVersioningLockHolder; #endif // FEATURE_CODE_VERSIONING - CodeHeapIterator heapIterator = pJitManager->GetCodeHeapIterator(); + CodeHeapIterator heapIterator = ExecutionManager::GetEEJitManager()->GetCodeHeapIterator(); while (heapIterator.Next()) { MethodDesc * pMethod = heapIterator.GetMethod(); @@ -219,6 +220,15 @@ void PerfMap::Disable() } } +// Signal that all dependencies (AppDomain, ExecutionManager) are ready. +// Called from EEStartup after ExecutionManager::Init. +void PerfMap::SignalSendExistingReady() +{ + LIMITED_METHOD_CONTRACT; + + s_sendExistingReady = true; +} + // Construct a new map for the process. PerfMap::PerfMap() { diff --git a/src/coreclr/vm/perfmap.h b/src/coreclr/vm/perfmap.h index 5268c4b0d56aef..b99dd1fa619ef2 100644 --- a/src/coreclr/vm/perfmap.h +++ b/src/coreclr/vm/perfmap.h @@ -24,6 +24,9 @@ class PerfMap private: static Volatile s_enabled; + // Set to true after all dependencies for sendExisting are initialized. + static Volatile s_sendExistingReady; + // The one and only PerfMap for the process. static PerfMap * s_Current; @@ -104,6 +107,9 @@ class PerfMap // Close the map and flush any remaining data. static void Disable(); + // Signal that all dependencies are ready for sendExisting to work. + static void SignalSendExistingReady(); + static bool LowGranularityStubs() { return !s_IndividualAllocationStubReporting; } }; #endif // PERFPID_H From 113ca67b62ade2e354a18a176f881cf53e2c3e0f Mon Sep 17 00:00:00 2001 From: Mitchell Hwang Date: Mon, 9 Feb 2026 10:50:35 -0500 Subject: [PATCH 4/5] Address review feedback: rename SignalSendExistingReady to SignalDependenciesReady - Rename method to better capture intent - Update comment to clarify it must be called before any code is JITed or restored from R2R - Remove stale comment about call site location --- src/coreclr/vm/ceemain.cpp | 2 +- src/coreclr/vm/perfmap.cpp | 4 ++-- src/coreclr/vm/perfmap.h | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/coreclr/vm/ceemain.cpp b/src/coreclr/vm/ceemain.cpp index fcb6581f831b38..11a9b8b5e00923 100644 --- a/src/coreclr/vm/ceemain.cpp +++ b/src/coreclr/vm/ceemain.cpp @@ -825,7 +825,7 @@ void EEStartupHelper() ExecutionManager::Init(); #ifdef FEATURE_PERFMAP - PerfMap::SignalSendExistingReady(); + PerfMap::SignalDependenciesReady(); #endif JitHost::Init(); diff --git a/src/coreclr/vm/perfmap.cpp b/src/coreclr/vm/perfmap.cpp index ac6a0bb866204a..743abffe1dfea8 100644 --- a/src/coreclr/vm/perfmap.cpp +++ b/src/coreclr/vm/perfmap.cpp @@ -221,8 +221,8 @@ void PerfMap::Disable() } // Signal that all dependencies (AppDomain, ExecutionManager) are ready. -// Called from EEStartup after ExecutionManager::Init. -void PerfMap::SignalSendExistingReady() +// This method must be called before any code is JITed or restored from R2R image. +void PerfMap::SignalDependenciesReady() { LIMITED_METHOD_CONTRACT; diff --git a/src/coreclr/vm/perfmap.h b/src/coreclr/vm/perfmap.h index b99dd1fa619ef2..b02d7e1e29085c 100644 --- a/src/coreclr/vm/perfmap.h +++ b/src/coreclr/vm/perfmap.h @@ -107,8 +107,8 @@ class PerfMap // Close the map and flush any remaining data. static void Disable(); - // Signal that all dependencies are ready for sendExisting to work. - static void SignalSendExistingReady(); + // Signal that all dependencies (AppDomain, ExecutionManager) are ready. + static void SignalDependenciesReady(); static bool LowGranularityStubs() { return !s_IndividualAllocationStubReporting; } }; From 31e4f806d80d286cfa204a7b07a542f13abbb647 Mon Sep 17 00:00:00 2001 From: Mitchell Hwang Date: Mon, 9 Feb 2026 12:44:39 -0500 Subject: [PATCH 5/5] Rename s_sendExistingReady to s_dependenciesReady --- src/coreclr/vm/perfmap.cpp | 8 ++++---- src/coreclr/vm/perfmap.h | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/coreclr/vm/perfmap.cpp b/src/coreclr/vm/perfmap.cpp index 743abffe1dfea8..c4c12604433c2a 100644 --- a/src/coreclr/vm/perfmap.cpp +++ b/src/coreclr/vm/perfmap.cpp @@ -28,7 +28,7 @@ #endif Volatile PerfMap::s_enabled = false; -Volatile PerfMap::s_sendExistingReady = false; +Volatile PerfMap::s_dependenciesReady = false; PerfMap * PerfMap::s_Current = nullptr; bool PerfMap::s_ShowOptimizationTiers = false; bool PerfMap::s_GroupStubsOfSameType = false; @@ -124,11 +124,11 @@ void PerfMap::Enable(PerfMapType type, bool sendExisting) { // When Enable is called very early in startup (e.g., via DiagnosticServer IPC before // SystemDomain::Attach and ExecutionManager::Init), the AppDomain and EEJitManager - // may not exist yet. We use s_sendExistingReady (a Volatile) to guard against + // may not exist yet. We use s_dependenciesReady (a Volatile) to guard against // this, rather than null-checking individual pointers which would have race conditions // due to non-Volatile statics like m_pEEJitManager. // Safe to skip: no assemblies are loaded and no code is JIT'd at that point. - if (!s_sendExistingReady) + if (!s_dependenciesReady) { return; } @@ -226,7 +226,7 @@ void PerfMap::SignalDependenciesReady() { LIMITED_METHOD_CONTRACT; - s_sendExistingReady = true; + s_dependenciesReady = true; } // Construct a new map for the process. diff --git a/src/coreclr/vm/perfmap.h b/src/coreclr/vm/perfmap.h index b02d7e1e29085c..4dc23f65e107db 100644 --- a/src/coreclr/vm/perfmap.h +++ b/src/coreclr/vm/perfmap.h @@ -24,8 +24,8 @@ class PerfMap private: static Volatile s_enabled; - // Set to true after all dependencies for sendExisting are initialized. - static Volatile s_sendExistingReady; + // Set to true after all dependencies (AppDomain, ExecutionManager) are initialized. + static Volatile s_dependenciesReady; // The one and only PerfMap for the process. static PerfMap * s_Current;