From 7358ca63cd34dea1fad836cd95e8f0776bfe9b55 Mon Sep 17 00:00:00 2001 From: Eduardo Velarde Date: Mon, 16 Mar 2026 19:05:42 -0700 Subject: [PATCH 01/24] Batch unwind info table entries --- src/coreclr/vm/codeman.cpp | 149 +++++++++++++++++++++++++++---------- src/coreclr/vm/codeman.h | 9 +++ 2 files changed, 119 insertions(+), 39 deletions(-) diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index 3b57576853be3e..ba97327418a3cf 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -146,6 +146,8 @@ UnwindInfoTable::UnwindInfoTable(ULONG_PTR rangeStart, ULONG_PTR rangeEnd, ULONG iRangeEnd = rangeEnd; hHandle = NULL; pTable = new T_RUNTIME_FUNCTION[cTableMaxCount]; + cPendingCount = 0; + pPendingTable = new T_RUNTIME_FUNCTION[cPendingMaxCount]; } /****************************************************************************/ @@ -161,6 +163,7 @@ UnwindInfoTable::~UnwindInfoTable() // It would be cleaner if we could take the lock (we did not have to be GC_NOTRIGGER) UnRegister(); delete[] pTable; + delete[] pPendingTable; } /*****************************************************************************/ @@ -236,7 +239,7 @@ void UnwindInfoTable::AddToUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, PT_R ULONG size = (ULONG) ((rangeEnd - rangeStart) / 128) + 1; - // To ensure the test the growing logic in debug code make the size much smaller. + // To ensure we test the growing logic in debug builds, make the size much smaller. INDEBUG(size = size / 4 + 1); unwindInfo = (PTR_UnwindInfoTable)new UnwindInfoTable(rangeStart, rangeEnd, size); unwindInfo->Register(); @@ -252,7 +255,9 @@ void UnwindInfoTable::AddToUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, PT_R return; // Check for the fast path: we are adding the end of an UnwindInfoTable with space - if (unwindInfo->cTableCurCount < unwindInfo->cTableMaxCount) + // and there are no pending entries waiting to be published. + if (unwindInfo->cTableCurCount < unwindInfo->cTableMaxCount + && unwindInfo->cPendingCount == 0) { if (unwindInfo->cTableCurCount == 0 || unwindInfo->pTable[unwindInfo->cTableCurCount-1].BeginAddress < data->BeginAddress) @@ -271,57 +276,108 @@ void UnwindInfoTable::AddToUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, PT_R } } - // OK we need to rellocate the table and reregister. First figure out our 'desiredSpace' - // We could imagine being much more efficient for 'bulk' updates, but we don't try - // because we assume that this is rare and we want to keep the code simple + // The entry is out-of-order or the main table is full. + // Buffer the entry and flush when the buffer is full. + _ASSERTE(unwindInfo->cPendingCount < UnwindInfoTable::cPendingMaxCount); + unwindInfo->pPendingTable[unwindInfo->cPendingCount++] = *data; - ULONG usedSpace = unwindInfo->cTableCurCount - unwindInfo->cDeletedEntries; - ULONG desiredSpace = usedSpace * 5 / 4 + 1; // Increase by 20% - // Be more aggressive if we used all of our space; - if (usedSpace == unwindInfo->cTableMaxCount) - desiredSpace = usedSpace * 3 / 2 + 1; // Increase by 50% - - STRESS_LOG7(LF_JIT, LL_INFO100, "AddToUnwindTable Handle: %p [%p, %p] SLOW Realloc Cnt 0x%x Max 0x%x NewMax 0x%x, Adding %x\n", + STRESS_LOG5(LF_JIT, LL_INFO1000, "AddToUnwindTable Handle: %p [%p, %p] BUFFERED 0x%p, pending 0x%x\n", unwindInfo->hHandle, unwindInfo->iRangeStart, unwindInfo->iRangeEnd, - unwindInfo->cTableCurCount, unwindInfo->cTableMaxCount, desiredSpace, data->BeginAddress); + data->BeginAddress, unwindInfo->cPendingCount); - UnwindInfoTable* newTab = new UnwindInfoTable(unwindInfo->iRangeStart, unwindInfo->iRangeEnd, desiredSpace); + if (unwindInfo->cPendingCount == UnwindInfoTable::cPendingMaxCount) + unwindInfo->FlushPendingEntries(); +} - // Copy in the entries, removing deleted entries and adding the new entry wherever it belongs - int toIdx = 0; - bool inserted = false; // Have we inserted 'data' into the table - for(ULONG fromIdx = 0; fromIdx < unwindInfo->cTableCurCount; fromIdx++) +/*****************************************************************************/ +void UnwindInfoTable::FlushPendingEntries() +{ + CONTRACTL { - if (!inserted && data->BeginAddress < unwindInfo->pTable[fromIdx].BeginAddress) + THROWS; + GC_TRIGGERS; + } + CONTRACTL_END; + + _ASSERTE(s_pUnwindInfoTableLock->OwnedByCurrentThread()); + + // Sort the pending entries by BeginAddress. + // Use a simple insertion sort since cPendingMaxCount is small (32). + for (ULONG i = 1; i < cPendingCount; i++) + { + T_RUNTIME_FUNCTION key = pPendingTable[i]; + ULONG j = i; + while (j > 0 && pPendingTable[j - 1].BeginAddress > key.BeginAddress) + { + pPendingTable[j] = pPendingTable[j - 1]; + j--; + } + pPendingTable[j] = key; + } + + // Calculate the new table size: live entries from main table + all pending entries + ULONG liveCount = cTableCurCount - cDeletedEntries; + ULONG newCount = liveCount + cPendingCount; + ULONG desiredSpace = newCount * 5 / 4 + 1; // Increase by 20% + + STRESS_LOG7(LF_JIT, LL_INFO100, "FlushPendingEntries Handle: %p [%p, %p] Merging 0x%x live + 0x%x pending into 0x%x max, from 0x%x\n", + hHandle, iRangeStart, iRangeEnd, liveCount, cPendingCount, desiredSpace, cTableMaxCount); + + T_RUNTIME_FUNCTION* newPTable = new T_RUNTIME_FUNCTION[desiredSpace]; + + // Merge-sort the main table and pending buffer into newPTable. + ULONG mainIdx = 0; + ULONG pendIdx = 0; + ULONG toIdx = 0; + + while (mainIdx < cTableCurCount && pendIdx < cPendingCount) + { + // Skip deleted entries in main table + if (pTable[mainIdx].UnwindData == 0) + { + mainIdx++; + continue; + } + + if (pPendingTable[pendIdx].BeginAddress < pTable[mainIdx].BeginAddress) + { + newPTable[toIdx++] = pPendingTable[pendIdx++]; + } + else { - STRESS_LOG1(LF_JIT, LL_INFO100, "AddToUnwindTable Inserted at MID position 0x%x\n", toIdx); - newTab->pTable[toIdx++] = *data; - inserted = true; + newPTable[toIdx++] = pTable[mainIdx++]; } - if (unwindInfo->pTable[fromIdx].UnwindData != 0) // A 'non-deleted' entry - newTab->pTable[toIdx++] = unwindInfo->pTable[fromIdx]; } - if (!inserted) + + while (mainIdx < cTableCurCount) + { + if (pTable[mainIdx].UnwindData != 0) + newPTable[toIdx++] = pTable[mainIdx]; + mainIdx++; + } + + while (pendIdx < cPendingCount) { - STRESS_LOG1(LF_JIT, LL_INFO100, "AddToUnwindTable Inserted at END position 0x%x\n", toIdx); - newTab->pTable[toIdx++] = *data; + newPTable[toIdx++] = pPendingTable[pendIdx++]; } - newTab->cTableCurCount = toIdx; - STRESS_LOG2(LF_JIT, LL_INFO100, "AddToUnwindTable New size 0x%x max 0x%x\n", - newTab->cTableCurCount, newTab->cTableMaxCount); - _ASSERTE(newTab->cTableCurCount <= newTab->cTableMaxCount); - // Unregister the old table - *unwindInfoPtr = 0; - unwindInfo->UnRegister(); + _ASSERTE(toIdx == newCount); + _ASSERTE(toIdx <= desiredSpace); - // Note that there is a short time when we are not publishing... + T_RUNTIME_FUNCTION* oldPTable = pTable; - // Register the new table - newTab->Register(); - *unwindInfoPtr = newTab; + UnRegister(); - delete unwindInfo; + pTable = newPTable; + cTableCurCount = toIdx; + cTableMaxCount = desiredSpace; + cDeletedEntries = 0; + cPendingCount = 0; + + // Note that there is a short time when we are not publishing... + Register(); + + delete[] oldPTable; } /*****************************************************************************/ @@ -345,6 +401,21 @@ void UnwindInfoTable::AddToUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, PT_R DWORD relativeEntryPoint = (DWORD)(entryPoint - baseAddress); STRESS_LOG3(LF_JIT, LL_INFO100, "RemoveFromUnwindInfoTable Removing %p BaseAddress %p rel %x\n", entryPoint, baseAddress, relativeEntryPoint); + + // First check the pending buffer (not yet published to the OS). + // Use swap-remove since the buffer doesn't need to stay sorted. + for (ULONG i = 0; i < unwindInfo->cPendingCount; i++) + { + if (unwindInfo->pPendingTable[i].BeginAddress <= relativeEntryPoint && + relativeEntryPoint < RUNTIME_FUNCTION__EndAddress(&unwindInfo->pPendingTable[i], unwindInfo->iRangeStart)) + { + unwindInfo->pPendingTable[i] = unwindInfo->pPendingTable[--unwindInfo->cPendingCount]; + STRESS_LOG1(LF_JIT, LL_INFO100, "RemoveFromUnwindInfoTable Removed pending entry 0x%x\n", i); + return; + } + } + + // Then check the main (published) table. for(ULONG i = 0; i < unwindInfo->cTableCurCount; i++) { if (unwindInfo->pTable[i].BeginAddress <= relativeEntryPoint && diff --git a/src/coreclr/vm/codeman.h b/src/coreclr/vm/codeman.h index 4a663b9df29e68..f80666ce0d8935 100644 --- a/src/coreclr/vm/codeman.h +++ b/src/coreclr/vm/codeman.h @@ -618,6 +618,8 @@ class UnwindInfoTable final UnwindInfoTable(ULONG_PTR rangeStart, ULONG_PTR rangeEnd, ULONG size); private: + void FlushPendingEntries(); + PVOID hHandle; // OS handle for a published RUNTIME_FUNCTION table ULONG_PTR iRangeStart; // Start of memory described by this table ULONG_PTR iRangeEnd; // End of memory described by this table @@ -625,6 +627,13 @@ class UnwindInfoTable final ULONG cTableCurCount; ULONG cTableMaxCount; int cDeletedEntries; // Number of slots we removed. + + // Pending buffer for out-of-order entries that haven't been published to the OS yet. + // These entries are accumulated and batch-merged into pTable to amortize the cost of + // RtlDeleteGrowableFunctionTable + RtlAddGrowableFunctionTable. + T_RUNTIME_FUNCTION* pPendingTable; + ULONG cPendingCount; + static const ULONG cPendingMaxCount = 32; #endif // defined(TARGET_AMD64) && defined(TARGET_WINDOWS) }; From 23fc617440f7f9878ecca8e565005008ddffdaa0 Mon Sep 17 00:00:00 2001 From: Eduardo Velarde Date: Tue, 17 Mar 2026 11:03:40 -0700 Subject: [PATCH 02/24] Flush before method is done JITing --- src/coreclr/vm/codeman.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index ba97327418a3cf..05823c43c3363f 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -450,6 +450,16 @@ void UnwindInfoTable::FlushPendingEntries() { for(int i = 0; i < unwindInfoCount; i++) AddToUnwindInfoTable(&pRS->_pUnwindInfoTable, &unwindInfo[i], pRS->_range.RangeStart(), pRS->_range.RangeEndOpen()); + + // Flush any entries that were buffered above to the OS can unwind this + // method immediately. Otherwise, we may end up with broken stack traces + // for recently JITed methods. + CrstHolder ch(s_pUnwindInfoTableLock); + UnwindInfoTable* unwindInfoTable = pRS->_pUnwindInfoTable; + if (unwindInfoTable != NULL && unwindInfoTable->cPendingCount > 0) + { + unwindInfoTable->FlushPendingEntries(); + } } } From 2d1341c148cbca199cbb60f6b9f1b8c9d1742478 Mon Sep 17 00:00:00 2001 From: Eduardo Velarde Date: Tue, 17 Mar 2026 16:57:30 -0700 Subject: [PATCH 03/24] Use two locks --- src/coreclr/inc/CrstTypes.def | 7 +- src/coreclr/inc/crsttypes_generated.h | 15 ++- src/coreclr/vm/codeman.cpp | 177 +++++++++++++++----------- 3 files changed, 116 insertions(+), 83 deletions(-) diff --git a/src/coreclr/inc/CrstTypes.def b/src/coreclr/inc/CrstTypes.def index c9798ea5ebe084..5b44da943170be 100644 --- a/src/coreclr/inc/CrstTypes.def +++ b/src/coreclr/inc/CrstTypes.def @@ -392,8 +392,13 @@ Crst SingleUseLock AcquiredBefore LoaderHeap UniqueStack DebuggerJitInfo End -Crst UnwindInfoTableLock +Crst UnwindInfoTablePublishLock AcquiredAfter SingleUseLock + AcquiredBefore UnwindInfoTablePendingLock +End + +Crst UnwindInfoTablePendingLock + AcquiredAfter UnwindInfoTablePublishLock AcquiredBefore StressLog End diff --git a/src/coreclr/inc/crsttypes_generated.h b/src/coreclr/inc/crsttypes_generated.h index afab5353f7d97a..a5cc09100c6a63 100644 --- a/src/coreclr/inc/crsttypes_generated.h +++ b/src/coreclr/inc/crsttypes_generated.h @@ -116,10 +116,11 @@ enum CrstType CrstUMEntryThunkFreeListLock = 98, CrstUniqueStack = 99, CrstUnresolvedClassLock = 100, - CrstUnwindInfoTableLock = 101, - CrstVSDIndirectionCellLock = 102, - CrstWrapperTemplate = 103, - kNumberOfCrstTypes = 104 + CrstUnwindInfoTablePendingLock = 101, + CrstUnwindInfoTablePublishLock = 102, + CrstVSDIndirectionCellLock = 103, + CrstWrapperTemplate = 104, + kNumberOfCrstTypes = 105 }; #endif // __CRST_TYPES_INCLUDED @@ -231,7 +232,8 @@ int g_rgCrstLevelMap[] = 2, // CrstUMEntryThunkFreeListLock 3, // CrstUniqueStack 6, // CrstUnresolvedClassLock - 2, // CrstUnwindInfoTableLock + 2, // CrstUnwindInfoTablePendingLock + 3, // CrstUnwindInfoTablePublishLock 3, // CrstVSDIndirectionCellLock 2, // CrstWrapperTemplate }; @@ -340,7 +342,8 @@ LPCSTR g_rgCrstNameMap[] = "CrstUMEntryThunkFreeListLock", "CrstUniqueStack", "CrstUnresolvedClassLock", - "CrstUnwindInfoTableLock", + "CrstUnwindInfoTablePendingLock", + "CrstUnwindInfoTablePublishLock", "CrstVSDIndirectionCellLock", "CrstWrapperTemplate", }; diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index 05823c43c3363f..8abad9a5bec75f 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -95,8 +95,9 @@ static RtlAddGrowableFunctionTableFnPtr pRtlAddGrowableFunctionTable; static RtlGrowFunctionTableFnPtr pRtlGrowFunctionTable; static RtlDeleteGrowableFunctionTableFnPtr pRtlDeleteGrowableFunctionTable; -static bool s_publishingActive; // Publishing to ETW is turned on -static Crst* s_pUnwindInfoTableLock; // lock protects all public UnwindInfoTable functions +static bool s_publishingActive; // Publishing to ETW is turned on +static Crst* s_pUnwindInfoTablePublishLock; // Protects main table, OS registration, and lazy init +static Crst* s_pUnwindInfoTablePendingLock; // Protects pending buffer only /****************************************************************************/ // initialize the entry points for new win8 unwind info publishing functions. @@ -136,7 +137,7 @@ bool InitUnwindFtns() UnwindInfoTable::UnwindInfoTable(ULONG_PTR rangeStart, ULONG_PTR rangeEnd, ULONG size) { STANDARD_VM_CONTRACT; - _ASSERTE(s_pUnwindInfoTableLock->OwnedByCurrentThread()); + _ASSERTE(s_pUnwindInfoTablePublishLock->OwnedByCurrentThread()); _ASSERTE((rangeEnd - rangeStart) <= 0x7FFFFFFF); cTableCurCount = 0; @@ -169,7 +170,7 @@ UnwindInfoTable::~UnwindInfoTable() /*****************************************************************************/ void UnwindInfoTable::Register() { - _ASSERTE(s_pUnwindInfoTableLock->OwnedByCurrentThread()); + _ASSERTE(s_pUnwindInfoTablePublishLock->OwnedByCurrentThread()); EX_TRY { hHandle = NULL; @@ -208,7 +209,8 @@ void UnwindInfoTable::UnRegister() } /*****************************************************************************/ -// Add 'data' to the linked list whose head is pointed at by 'unwindInfoPtr' +// Add 'data' to the pending buffer for later publication to the OS. +// When the buffer is full, entries are flushed under s_pUnwindInfoTablePublishLock. // /* static */ void UnwindInfoTable::AddToUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, PT_RUNTIME_FUNCTION data, @@ -227,26 +229,28 @@ void UnwindInfoTable::AddToUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, PT_R if (!s_publishingActive) return; - CrstHolder ch(s_pUnwindInfoTableLock); - - UnwindInfoTable* unwindInfo = *unwindInfoPtr; + UnwindInfoTable* unwindInfo = VolatileLoad(unwindInfoPtr); // was the original list null, If so lazy initialize. if (unwindInfo == NULL) { - // We can choose the average method size estimate dynamically based on past experience - // 128 is the estimated size of an average method, so we can accurately predict - // how many RUNTIME_FUNCTION entries are in each chunk we allocate. + CrstHolder publishLock(s_pUnwindInfoTablePublishLock); + unwindInfo = *unwindInfoPtr; + if (unwindInfo == NULL) + { + // We can choose the average method size estimate dynamically based on past experience + // 128 is the estimated size of an average method, so we can accurately predict + // how many RUNTIME_FUNCTION entries are in each chunk we allocate. - ULONG size = (ULONG) ((rangeEnd - rangeStart) / 128) + 1; + ULONG size = (ULONG) ((rangeEnd - rangeStart) / 128) + 1; - // To ensure we test the growing logic in debug builds, make the size much smaller. - INDEBUG(size = size / 4 + 1); - unwindInfo = (PTR_UnwindInfoTable)new UnwindInfoTable(rangeStart, rangeEnd, size); - unwindInfo->Register(); - *unwindInfoPtr = unwindInfo; + // To ensure we test the growing logic in debug builds, make the size much smaller. + INDEBUG(size = size / 4 + 1); + unwindInfo = (PTR_UnwindInfoTable)new UnwindInfoTable(rangeStart, rangeEnd, size); + unwindInfo->Register(); + VolatileStore(unwindInfoPtr, unwindInfo); + } } _ASSERTE(unwindInfo != NULL); // If new had failed, we would have thrown OOM - _ASSERTE(unwindInfo->cTableCurCount <= unwindInfo->cTableMaxCount); _ASSERTE(unwindInfo->iRangeStart == rangeStart); _ASSERTE(unwindInfo->iRangeEnd == rangeEnd); @@ -254,39 +258,24 @@ void UnwindInfoTable::AddToUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, PT_R if (unwindInfo->hHandle == NULL) return; - // Check for the fast path: we are adding the end of an UnwindInfoTable with space - // and there are no pending entries waiting to be published. - if (unwindInfo->cTableCurCount < unwindInfo->cTableMaxCount - && unwindInfo->cPendingCount == 0) + // Add to the pending buffer. If the buffer is full, flush it first and retry. + while (true) { - if (unwindInfo->cTableCurCount == 0 || - unwindInfo->pTable[unwindInfo->cTableCurCount-1].BeginAddress < data->BeginAddress) { - // Yeah, we can simply add to the end of table and we are done! - unwindInfo->pTable[unwindInfo->cTableCurCount] = *data; - unwindInfo->cTableCurCount++; - - // Add to the function table - pRtlGrowFunctionTable(unwindInfo->hHandle, unwindInfo->cTableCurCount); + CrstHolder pendingLock(s_pUnwindInfoTablePendingLock); + if (unwindInfo->cPendingCount < UnwindInfoTable::cPendingMaxCount) + { + unwindInfo->pPendingTable[unwindInfo->cPendingCount++] = *data; - STRESS_LOG5(LF_JIT, LL_INFO1000, "AddToUnwindTable Handle: %p [%p, %p] ADDING 0x%p TO END, now 0x%x entries\n", - unwindInfo->hHandle, unwindInfo->iRangeStart, unwindInfo->iRangeEnd, - data->BeginAddress, unwindInfo->cTableCurCount); - return; + STRESS_LOG5(LF_JIT, LL_INFO1000, "AddToUnwindTable Handle: %p [%p, %p] BUFFERED 0x%p, pending 0x%x\n", + unwindInfo->hHandle, unwindInfo->iRangeStart, unwindInfo->iRangeEnd, + data->BeginAddress, unwindInfo->cPendingCount); + return; + } } - } - - // The entry is out-of-order or the main table is full. - // Buffer the entry and flush when the buffer is full. - _ASSERTE(unwindInfo->cPendingCount < UnwindInfoTable::cPendingMaxCount); - unwindInfo->pPendingTable[unwindInfo->cPendingCount++] = *data; - - STRESS_LOG5(LF_JIT, LL_INFO1000, "AddToUnwindTable Handle: %p [%p, %p] BUFFERED 0x%p, pending 0x%x\n", - unwindInfo->hHandle, unwindInfo->iRangeStart, unwindInfo->iRangeEnd, - data->BeginAddress, unwindInfo->cPendingCount); - - if (unwindInfo->cPendingCount == UnwindInfoTable::cPendingMaxCount) + CrstHolder publishLock(s_pUnwindInfoTablePublishLock); unwindInfo->FlushPendingEntries(); + } } /*****************************************************************************/ @@ -299,29 +288,59 @@ void UnwindInfoTable::FlushPendingEntries() } CONTRACTL_END; - _ASSERTE(s_pUnwindInfoTableLock->OwnedByCurrentThread()); + _ASSERTE(s_pUnwindInfoTablePublishLock->OwnedByCurrentThread()); + + // Grab the pending entries under the pending lock, then release it so + // other threads can keep accumulating new entries while we flush. + T_RUNTIME_FUNCTION localPending[cPendingMaxCount]; + ULONG localPendingCount; + { + CrstHolder pendingLock(s_pUnwindInfoTablePendingLock); + localPendingCount = cPendingCount; + memcpy(localPending, pPendingTable, cPendingCount * sizeof(T_RUNTIME_FUNCTION)); + cPendingCount = 0; + } + + if (localPendingCount == 0) + return; // Sort the pending entries by BeginAddress. // Use a simple insertion sort since cPendingMaxCount is small (32). - for (ULONG i = 1; i < cPendingCount; i++) + for (ULONG i = 1; i < localPendingCount; i++) { - T_RUNTIME_FUNCTION key = pPendingTable[i]; + T_RUNTIME_FUNCTION key = localPending[i]; ULONG j = i; - while (j > 0 && pPendingTable[j - 1].BeginAddress > key.BeginAddress) + while (j > 0 && localPending[j - 1].BeginAddress > key.BeginAddress) { - pPendingTable[j] = pPendingTable[j - 1]; + localPending[j] = localPending[j - 1]; j--; } - pPendingTable[j] = key; + localPending[j] = key; + } + + // Fast path: if all pending entries can be appended in order with room to spare, + // we can just append and call RtlGrowFunctionTable. + if (cDeletedEntries == 0 + && cTableCurCount + localPendingCount <= cTableMaxCount + && (cTableCurCount == 0 || pTable[cTableCurCount - 1].BeginAddress < localPending[0].BeginAddress)) + { + memcpy(&pTable[cTableCurCount], localPending, localPendingCount * sizeof(T_RUNTIME_FUNCTION)); + cTableCurCount += localPendingCount; + pRtlGrowFunctionTable(hHandle, cTableCurCount); + + STRESS_LOG5(LF_JIT, LL_INFO1000, "FlushPendingEntries Handle: %p [%p, %p] APPENDED 0x%x entries, now 0x%x\n", + hHandle, iRangeStart, iRangeEnd, localPendingCount, cTableCurCount); + return; } + // Merge main table and pending entries. // Calculate the new table size: live entries from main table + all pending entries ULONG liveCount = cTableCurCount - cDeletedEntries; - ULONG newCount = liveCount + cPendingCount; + ULONG newCount = liveCount + localPendingCount; ULONG desiredSpace = newCount * 5 / 4 + 1; // Increase by 20% STRESS_LOG7(LF_JIT, LL_INFO100, "FlushPendingEntries Handle: %p [%p, %p] Merging 0x%x live + 0x%x pending into 0x%x max, from 0x%x\n", - hHandle, iRangeStart, iRangeEnd, liveCount, cPendingCount, desiredSpace, cTableMaxCount); + hHandle, iRangeStart, iRangeEnd, liveCount, localPendingCount, desiredSpace, cTableMaxCount); T_RUNTIME_FUNCTION* newPTable = new T_RUNTIME_FUNCTION[desiredSpace]; @@ -330,7 +349,7 @@ void UnwindInfoTable::FlushPendingEntries() ULONG pendIdx = 0; ULONG toIdx = 0; - while (mainIdx < cTableCurCount && pendIdx < cPendingCount) + while (mainIdx < cTableCurCount && pendIdx < localPendingCount) { // Skip deleted entries in main table if (pTable[mainIdx].UnwindData == 0) @@ -339,9 +358,9 @@ void UnwindInfoTable::FlushPendingEntries() continue; } - if (pPendingTable[pendIdx].BeginAddress < pTable[mainIdx].BeginAddress) + if (localPending[pendIdx].BeginAddress < pTable[mainIdx].BeginAddress) { - newPTable[toIdx++] = pPendingTable[pendIdx++]; + newPTable[toIdx++] = localPending[pendIdx++]; } else { @@ -356,9 +375,9 @@ void UnwindInfoTable::FlushPendingEntries() mainIdx++; } - while (pendIdx < cPendingCount) + while (pendIdx < localPendingCount) { - newPTable[toIdx++] = pPendingTable[pendIdx++]; + newPTable[toIdx++] = localPending[pendIdx++]; } _ASSERTE(toIdx == newCount); @@ -372,7 +391,6 @@ void UnwindInfoTable::FlushPendingEntries() cTableCurCount = toIdx; cTableMaxCount = desiredSpace; cDeletedEntries = 0; - cPendingCount = 0; // Note that there is a short time when we are not publishing... Register(); @@ -393,17 +411,19 @@ void UnwindInfoTable::FlushPendingEntries() if (!s_publishingActive) return; - CrstHolder ch(s_pUnwindInfoTableLock); - UnwindInfoTable* unwindInfo = *unwindInfoPtr; - if (unwindInfo != NULL) - { - DWORD relativeEntryPoint = (DWORD)(entryPoint - baseAddress); - STRESS_LOG3(LF_JIT, LL_INFO100, "RemoveFromUnwindInfoTable Removing %p BaseAddress %p rel %x\n", - entryPoint, baseAddress, relativeEntryPoint); + UnwindInfoTable* unwindInfo = VolatileLoad(unwindInfoPtr); + if (unwindInfo == NULL) + return; + + DWORD relativeEntryPoint = (DWORD)(entryPoint - baseAddress); + STRESS_LOG3(LF_JIT, LL_INFO100, "RemoveFromUnwindInfoTable Removing %p BaseAddress %p rel %x\n", + entryPoint, baseAddress, relativeEntryPoint); - // First check the pending buffer (not yet published to the OS). - // Use swap-remove since the buffer doesn't need to stay sorted. + // First check the pending buffer under the pending lock. + // Use swap-remove since the buffer doesn't need to stay sorted. + { + CrstHolder pendingLock(s_pUnwindInfoTablePendingLock); for (ULONG i = 0; i < unwindInfo->cPendingCount; i++) { if (unwindInfo->pPendingTable[i].BeginAddress <= relativeEntryPoint && @@ -414,9 +434,12 @@ void UnwindInfoTable::FlushPendingEntries() return; } } + } - // Then check the main (published) table. - for(ULONG i = 0; i < unwindInfo->cTableCurCount; i++) + // Not found in pending buffer — check the main (published) table under publish lock. + { + CrstHolder publishLock(s_pUnwindInfoTablePublishLock); + for (ULONG i = 0; i < unwindInfo->cTableCurCount; i++) { if (unwindInfo->pTable[i].BeginAddress <= relativeEntryPoint && relativeEntryPoint < RUNTIME_FUNCTION__EndAddress(&unwindInfo->pTable[i], unwindInfo->iRangeStart)) @@ -429,6 +452,7 @@ void UnwindInfoTable::FlushPendingEntries() } } } + STRESS_LOG2(LF_JIT, LL_WARNING, "RemoveFromUnwindInfoTable COULD NOT FIND %p BaseAddress %p\n", entryPoint, baseAddress); } @@ -451,12 +475,12 @@ void UnwindInfoTable::FlushPendingEntries() for(int i = 0; i < unwindInfoCount; i++) AddToUnwindInfoTable(&pRS->_pUnwindInfoTable, &unwindInfo[i], pRS->_range.RangeStart(), pRS->_range.RangeEndOpen()); - // Flush any entries that were buffered above to the OS can unwind this + // Flush any entries that were buffered above so the OS can unwind this // method immediately. Otherwise, we may end up with broken stack traces // for recently JITed methods. - CrstHolder ch(s_pUnwindInfoTableLock); + CrstHolder publishLock(s_pUnwindInfoTablePublishLock); UnwindInfoTable* unwindInfoTable = pRS->_pUnwindInfoTable; - if (unwindInfoTable != NULL && unwindInfoTable->cPendingCount > 0) + if (unwindInfoTable != NULL) { unwindInfoTable->FlushPendingEntries(); } @@ -511,8 +535,9 @@ void UnwindInfoTable::FlushPendingEntries() if (!InitUnwindFtns()) return; - // Create the lock - s_pUnwindInfoTableLock = new Crst(CrstUnwindInfoTableLock); + // Create the locks + s_pUnwindInfoTablePublishLock = new Crst(CrstUnwindInfoTablePublishLock); + s_pUnwindInfoTablePendingLock = new Crst(CrstUnwindInfoTablePendingLock); s_publishingActive = true; } From ed6120084fa918967debf31afce1eab0a1b6f1b9 Mon Sep 17 00:00:00 2001 From: Eduardo Velarde Date: Tue, 17 Mar 2026 18:18:26 -0700 Subject: [PATCH 04/24] Copilot code review --- src/coreclr/vm/codeman.cpp | 7 +++++-- src/coreclr/vm/codeman.h | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index 8abad9a5bec75f..c45da00c5cb86b 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -143,6 +143,7 @@ UnwindInfoTable::UnwindInfoTable(ULONG_PTR rangeStart, ULONG_PTR rangeEnd, ULONG cTableCurCount = 0; cTableMaxCount = size; cDeletedEntries = 0; + bRegistrationFailed = false; iRangeStart = rangeStart; iRangeEnd = rangeEnd; hHandle = NULL; @@ -247,6 +248,8 @@ void UnwindInfoTable::AddToUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, PT_R INDEBUG(size = size / 4 + 1); unwindInfo = (PTR_UnwindInfoTable)new UnwindInfoTable(rangeStart, rangeEnd, size); unwindInfo->Register(); + if (unwindInfo->hHandle == NULL) + unwindInfo->bRegistrationFailed = true; VolatileStore(unwindInfoPtr, unwindInfo); } } @@ -255,7 +258,7 @@ void UnwindInfoTable::AddToUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, PT_R _ASSERTE(unwindInfo->iRangeEnd == rangeEnd); // Means we had a failure publishing to the OS, in this case we give up - if (unwindInfo->hHandle == NULL) + if (unwindInfo->bRegistrationFailed) return; // Add to the pending buffer. If the buffer is full, flush it first and retry. @@ -267,7 +270,7 @@ void UnwindInfoTable::AddToUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, PT_R { unwindInfo->pPendingTable[unwindInfo->cPendingCount++] = *data; - STRESS_LOG5(LF_JIT, LL_INFO1000, "AddToUnwindTable Handle: %p [%p, %p] BUFFERED 0x%p, pending 0x%x\n", + STRESS_LOG5(LF_JIT, LL_INFO1000, "AddToUnwindTable Handle: %p [%p, %p] BUFFERED 0x%x, pending 0x%x\n", unwindInfo->hHandle, unwindInfo->iRangeStart, unwindInfo->iRangeEnd, data->BeginAddress, unwindInfo->cPendingCount); return; diff --git a/src/coreclr/vm/codeman.h b/src/coreclr/vm/codeman.h index f80666ce0d8935..3564cc03a3e0ff 100644 --- a/src/coreclr/vm/codeman.h +++ b/src/coreclr/vm/codeman.h @@ -627,6 +627,7 @@ class UnwindInfoTable final ULONG cTableCurCount; ULONG cTableMaxCount; int cDeletedEntries; // Number of slots we removed. + bool bRegistrationFailed; // Set once if initial OS registration failed; never cleared. // Pending buffer for out-of-order entries that haven't been published to the OS yet. // These entries are accumulated and batch-merged into pTable to amortize the cost of From 58cbb758175d8380926328dcc61fa407a7062790 Mon Sep 17 00:00:00 2001 From: Eduardo Velarde Date: Tue, 17 Mar 2026 18:40:01 -0700 Subject: [PATCH 05/24] Revert "Copilot code review" This reverts commit ed6120084fa918967debf31afce1eab0a1b6f1b9. --- src/coreclr/vm/codeman.cpp | 7 ++----- src/coreclr/vm/codeman.h | 1 - 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index c45da00c5cb86b..8abad9a5bec75f 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -143,7 +143,6 @@ UnwindInfoTable::UnwindInfoTable(ULONG_PTR rangeStart, ULONG_PTR rangeEnd, ULONG cTableCurCount = 0; cTableMaxCount = size; cDeletedEntries = 0; - bRegistrationFailed = false; iRangeStart = rangeStart; iRangeEnd = rangeEnd; hHandle = NULL; @@ -248,8 +247,6 @@ void UnwindInfoTable::AddToUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, PT_R INDEBUG(size = size / 4 + 1); unwindInfo = (PTR_UnwindInfoTable)new UnwindInfoTable(rangeStart, rangeEnd, size); unwindInfo->Register(); - if (unwindInfo->hHandle == NULL) - unwindInfo->bRegistrationFailed = true; VolatileStore(unwindInfoPtr, unwindInfo); } } @@ -258,7 +255,7 @@ void UnwindInfoTable::AddToUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, PT_R _ASSERTE(unwindInfo->iRangeEnd == rangeEnd); // Means we had a failure publishing to the OS, in this case we give up - if (unwindInfo->bRegistrationFailed) + if (unwindInfo->hHandle == NULL) return; // Add to the pending buffer. If the buffer is full, flush it first and retry. @@ -270,7 +267,7 @@ void UnwindInfoTable::AddToUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, PT_R { unwindInfo->pPendingTable[unwindInfo->cPendingCount++] = *data; - STRESS_LOG5(LF_JIT, LL_INFO1000, "AddToUnwindTable Handle: %p [%p, %p] BUFFERED 0x%x, pending 0x%x\n", + STRESS_LOG5(LF_JIT, LL_INFO1000, "AddToUnwindTable Handle: %p [%p, %p] BUFFERED 0x%p, pending 0x%x\n", unwindInfo->hHandle, unwindInfo->iRangeStart, unwindInfo->iRangeEnd, data->BeginAddress, unwindInfo->cPendingCount); return; diff --git a/src/coreclr/vm/codeman.h b/src/coreclr/vm/codeman.h index 3564cc03a3e0ff..f80666ce0d8935 100644 --- a/src/coreclr/vm/codeman.h +++ b/src/coreclr/vm/codeman.h @@ -627,7 +627,6 @@ class UnwindInfoTable final ULONG cTableCurCount; ULONG cTableMaxCount; int cDeletedEntries; // Number of slots we removed. - bool bRegistrationFailed; // Set once if initial OS registration failed; never cleared. // Pending buffer for out-of-order entries that haven't been published to the OS yet. // These entries are accumulated and batch-merged into pTable to amortize the cost of From 00c8e3db8c134c053eac623f6bbee00dc69baa8a Mon Sep 17 00:00:00 2001 From: Eduardo Velarde Date: Tue, 17 Mar 2026 18:42:27 -0700 Subject: [PATCH 06/24] Read the table under the publish lock for early return --- src/coreclr/vm/codeman.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index 8abad9a5bec75f..cc80096b4b31b1 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -256,7 +256,14 @@ void UnwindInfoTable::AddToUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, PT_R // Means we had a failure publishing to the OS, in this case we give up if (unwindInfo->hHandle == NULL) - return; + { + CrstHolder publishLock(s_pUnwindInfoTablePublishLock); + // Re-read the table under the publish lock in case a flush/unregister/register + // sequence was in progress when we first inspected hHandle. + unwindInfo = *unwindInfoPtr; + if (unwindInfo == NULL || unwindInfo->hHandle == NULL) + return; + } // Add to the pending buffer. If the buffer is full, flush it first and retry. while (true) @@ -267,7 +274,7 @@ void UnwindInfoTable::AddToUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, PT_R { unwindInfo->pPendingTable[unwindInfo->cPendingCount++] = *data; - STRESS_LOG5(LF_JIT, LL_INFO1000, "AddToUnwindTable Handle: %p [%p, %p] BUFFERED 0x%p, pending 0x%x\n", + STRESS_LOG5(LF_JIT, LL_INFO1000, "AddToUnwindTable Handle: %p [%p, %p] BUFFERED 0x%x, pending 0x%x\n", unwindInfo->hHandle, unwindInfo->iRangeStart, unwindInfo->iRangeEnd, data->BeginAddress, unwindInfo->cPendingCount); return; From fc63e44aa8d036f4a1b09b9477ebb8b6b0697889 Mon Sep 17 00:00:00 2001 From: Eduardo Velarde Date: Tue, 17 Mar 2026 19:27:59 -0700 Subject: [PATCH 07/24] Check for null hHandle in FlushPendingEntries --- src/coreclr/vm/codeman.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index cc80096b4b31b1..3745e90b820283 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -311,6 +311,11 @@ void UnwindInfoTable::FlushPendingEntries() if (localPendingCount == 0) return; + // If a previous Register() call failed, hHandle is NULL. Skip OS updates + // to avoid calling RtlGrowFunctionTable or re-registering with a null handle. + if (hHandle == NULL) + return; + // Sort the pending entries by BeginAddress. // Use a simple insertion sort since cPendingMaxCount is small (32). for (ULONG i = 1; i < localPendingCount; i++) From e94cfd3747b3ef34e0c29110220304e09d799288 Mon Sep 17 00:00:00 2001 From: Eduardo Velarde Date: Wed, 18 Mar 2026 14:26:00 -0700 Subject: [PATCH 08/24] PR feedback --- src/coreclr/vm/codeman.cpp | 37 +++++++++++++++++-------------------- src/coreclr/vm/codeman.h | 4 ++-- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index 3745e90b820283..f89fd456f23cfc 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -148,7 +148,6 @@ UnwindInfoTable::UnwindInfoTable(ULONG_PTR rangeStart, ULONG_PTR rangeEnd, ULONG hHandle = NULL; pTable = new T_RUNTIME_FUNCTION[cTableMaxCount]; cPendingCount = 0; - pPendingTable = new T_RUNTIME_FUNCTION[cPendingMaxCount]; } /****************************************************************************/ @@ -164,7 +163,6 @@ UnwindInfoTable::~UnwindInfoTable() // It would be cleaner if we could take the lock (we did not have to be GC_NOTRIGGER) UnRegister(); delete[] pTable; - delete[] pPendingTable; } /*****************************************************************************/ @@ -332,8 +330,7 @@ void UnwindInfoTable::FlushPendingEntries() // Fast path: if all pending entries can be appended in order with room to spare, // we can just append and call RtlGrowFunctionTable. - if (cDeletedEntries == 0 - && cTableCurCount + localPendingCount <= cTableMaxCount + if (cTableCurCount + localPendingCount <= cTableMaxCount && (cTableCurCount == 0 || pTable[cTableCurCount - 1].BeginAddress < localPending[0].BeginAddress)) { memcpy(&pTable[cTableCurCount], localPending, localPendingCount * sizeof(T_RUNTIME_FUNCTION)); @@ -432,22 +429,6 @@ void UnwindInfoTable::FlushPendingEntries() STRESS_LOG3(LF_JIT, LL_INFO100, "RemoveFromUnwindInfoTable Removing %p BaseAddress %p rel %x\n", entryPoint, baseAddress, relativeEntryPoint); - // First check the pending buffer under the pending lock. - // Use swap-remove since the buffer doesn't need to stay sorted. - { - CrstHolder pendingLock(s_pUnwindInfoTablePendingLock); - for (ULONG i = 0; i < unwindInfo->cPendingCount; i++) - { - if (unwindInfo->pPendingTable[i].BeginAddress <= relativeEntryPoint && - relativeEntryPoint < RUNTIME_FUNCTION__EndAddress(&unwindInfo->pPendingTable[i], unwindInfo->iRangeStart)) - { - unwindInfo->pPendingTable[i] = unwindInfo->pPendingTable[--unwindInfo->cPendingCount]; - STRESS_LOG1(LF_JIT, LL_INFO100, "RemoveFromUnwindInfoTable Removed pending entry 0x%x\n", i); - return; - } - } - } - // Not found in pending buffer — check the main (published) table under publish lock. { CrstHolder publishLock(s_pUnwindInfoTablePublishLock); @@ -465,6 +446,22 @@ void UnwindInfoTable::FlushPendingEntries() } } + // Check the pending buffer under the pending lock. + // Use swap-remove since the buffer doesn't need to stay sorted. + { + CrstHolder pendingLock(s_pUnwindInfoTablePendingLock); + for (ULONG i = 0; i < unwindInfo->cPendingCount; i++) + { + if (unwindInfo->pPendingTable[i].BeginAddress <= relativeEntryPoint && + relativeEntryPoint < RUNTIME_FUNCTION__EndAddress(&unwindInfo->pPendingTable[i], unwindInfo->iRangeStart)) + { + unwindInfo->pPendingTable[i] = unwindInfo->pPendingTable[--unwindInfo->cPendingCount]; + STRESS_LOG1(LF_JIT, LL_INFO100, "RemoveFromUnwindInfoTable Removed pending entry 0x%x\n", i); + return; + } + } + } + STRESS_LOG2(LF_JIT, LL_WARNING, "RemoveFromUnwindInfoTable COULD NOT FIND %p BaseAddress %p\n", entryPoint, baseAddress); } diff --git a/src/coreclr/vm/codeman.h b/src/coreclr/vm/codeman.h index f80666ce0d8935..dee41c09bf95b7 100644 --- a/src/coreclr/vm/codeman.h +++ b/src/coreclr/vm/codeman.h @@ -631,9 +631,9 @@ class UnwindInfoTable final // Pending buffer for out-of-order entries that haven't been published to the OS yet. // These entries are accumulated and batch-merged into pTable to amortize the cost of // RtlDeleteGrowableFunctionTable + RtlAddGrowableFunctionTable. - T_RUNTIME_FUNCTION* pPendingTable; - ULONG cPendingCount; static const ULONG cPendingMaxCount = 32; + T_RUNTIME_FUNCTION pPendingTable[cPendingMaxCount]; + ULONG cPendingCount; #endif // defined(TARGET_AMD64) && defined(TARGET_WINDOWS) }; From d0fa5bc02fd7b1f716e1b1781393161aa0ea3bda Mon Sep 17 00:00:00 2001 From: Eduardo Velarde <32459232+eduardo-vp@users.noreply.github.com> Date: Wed, 18 Mar 2026 15:12:40 -0700 Subject: [PATCH 09/24] Update commet Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/coreclr/vm/codeman.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index f89fd456f23cfc..193b12cab84f2e 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -429,7 +429,7 @@ void UnwindInfoTable::FlushPendingEntries() STRESS_LOG3(LF_JIT, LL_INFO100, "RemoveFromUnwindInfoTable Removing %p BaseAddress %p rel %x\n", entryPoint, baseAddress, relativeEntryPoint); - // Not found in pending buffer — check the main (published) table under publish lock. + // First check the main (published) table under the publish lock. { CrstHolder publishLock(s_pUnwindInfoTablePublishLock); for (ULONG i = 0; i < unwindInfo->cTableCurCount; i++) From 2aaa4767ea4d87bede1e44200d678c2c5055c0a2 Mon Sep 17 00:00:00 2001 From: Eduardo Velarde Date: Wed, 18 Mar 2026 16:45:19 -0700 Subject: [PATCH 10/24] PR feedback --- src/coreclr/vm/codeman.cpp | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index 193b12cab84f2e..6bd545fbf5a493 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -295,6 +295,15 @@ void UnwindInfoTable::FlushPendingEntries() _ASSERTE(s_pUnwindInfoTablePublishLock->OwnedByCurrentThread()); + if (hHandle == NULL) + { + // If hHandle is null, it means Register() failed. Skip flushing to avoid calling + // RtlGrowFunctionTable with a null handle. Avoid copying to the local buffer as + // well since it won't be published anyway. + cPendingCount = 0; + return; + } + // Grab the pending entries under the pending lock, then release it so // other threads can keep accumulating new entries while we flush. T_RUNTIME_FUNCTION localPending[cPendingMaxCount]; @@ -309,13 +318,10 @@ void UnwindInfoTable::FlushPendingEntries() if (localPendingCount == 0) return; - // If a previous Register() call failed, hHandle is NULL. Skip OS updates - // to avoid calling RtlGrowFunctionTable or re-registering with a null handle. - if (hHandle == NULL) - return; - // Sort the pending entries by BeginAddress. // Use a simple insertion sort since cPendingMaxCount is small (32). + static_assert(cPendingMaxCount == 32, + "cPendingMaxCount was updated and might be too large for insertion sort, consider using a better algorithm"); for (ULONG i = 1; i < localPendingCount; i++) { T_RUNTIME_FUNCTION key = localPending[i]; @@ -351,7 +357,7 @@ void UnwindInfoTable::FlushPendingEntries() STRESS_LOG7(LF_JIT, LL_INFO100, "FlushPendingEntries Handle: %p [%p, %p] Merging 0x%x live + 0x%x pending into 0x%x max, from 0x%x\n", hHandle, iRangeStart, iRangeEnd, liveCount, localPendingCount, desiredSpace, cTableMaxCount); - T_RUNTIME_FUNCTION* newPTable = new T_RUNTIME_FUNCTION[desiredSpace]; + NewArrayHolder newPTable(new T_RUNTIME_FUNCTION[desiredSpace]); // Merge-sort the main table and pending buffer into newPTable. ULONG mainIdx = 0; @@ -392,19 +398,17 @@ void UnwindInfoTable::FlushPendingEntries() _ASSERTE(toIdx == newCount); _ASSERTE(toIdx <= desiredSpace); - T_RUNTIME_FUNCTION* oldPTable = pTable; + NewArrayHolder oldPTable(pTable); UnRegister(); - pTable = newPTable; + pTable = newPTable.Extract(); cTableCurCount = toIdx; cTableMaxCount = desiredSpace; cDeletedEntries = 0; // Note that there is a short time when we are not publishing... Register(); - - delete[] oldPTable; } /*****************************************************************************/ From 543d15e969699a6dd6ee561fcdd0f4ef0a3c6f5d Mon Sep 17 00:00:00 2001 From: Eduardo Velarde Date: Wed, 18 Mar 2026 16:46:11 -0700 Subject: [PATCH 11/24] Perf: remove unwind info handle null check --- src/coreclr/vm/codeman.cpp | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index 6bd545fbf5a493..d77cd8866815af 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -252,17 +252,6 @@ void UnwindInfoTable::AddToUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, PT_R _ASSERTE(unwindInfo->iRangeStart == rangeStart); _ASSERTE(unwindInfo->iRangeEnd == rangeEnd); - // Means we had a failure publishing to the OS, in this case we give up - if (unwindInfo->hHandle == NULL) - { - CrstHolder publishLock(s_pUnwindInfoTablePublishLock); - // Re-read the table under the publish lock in case a flush/unregister/register - // sequence was in progress when we first inspected hHandle. - unwindInfo = *unwindInfoPtr; - if (unwindInfo == NULL || unwindInfo->hHandle == NULL) - return; - } - // Add to the pending buffer. If the buffer is full, flush it first and retry. while (true) { From 50c7708920983275f39a28ae8081fd144d5eaf50 Mon Sep 17 00:00:00 2001 From: Eduardo Velarde <32459232+eduardo-vp@users.noreply.github.com> Date: Wed, 18 Mar 2026 17:32:00 -0700 Subject: [PATCH 12/24] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/coreclr/vm/codeman.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index d77cd8866815af..934f5508883b5e 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -289,6 +289,7 @@ void UnwindInfoTable::FlushPendingEntries() // If hHandle is null, it means Register() failed. Skip flushing to avoid calling // RtlGrowFunctionTable with a null handle. Avoid copying to the local buffer as // well since it won't be published anyway. + CrstHolder pendingLock(s_pUnwindInfoTablePendingLock); cPendingCount = 0; return; } From 2f6368544e6c43942253a59a0f015dc3d64bd5ed Mon Sep 17 00:00:00 2001 From: Eduardo Velarde Date: Thu, 19 Mar 2026 14:47:23 -0700 Subject: [PATCH 13/24] Make AddToUnwindInfoTable an instance method --- src/coreclr/vm/codeman.cpp | 86 +++++++++++++++++--------------------- src/coreclr/vm/codeman.h | 6 +-- 2 files changed, 42 insertions(+), 50 deletions(-) diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index 934f5508883b5e..0e7724473f71b6 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -134,12 +134,20 @@ bool InitUnwindFtns() } /****************************************************************************/ -UnwindInfoTable::UnwindInfoTable(ULONG_PTR rangeStart, ULONG_PTR rangeEnd, ULONG size) +UnwindInfoTable::UnwindInfoTable(ULONG_PTR rangeStart, ULONG_PTR rangeEnd) { STANDARD_VM_CONTRACT; _ASSERTE(s_pUnwindInfoTablePublishLock->OwnedByCurrentThread()); _ASSERTE((rangeEnd - rangeStart) <= 0x7FFFFFFF); + // We can choose the average method size estimate dynamically based on past experience + // 128 is the estimated size of an average method, so we can accurately predict + // how many RUNTIME_FUNCTION entries are in each chunk we allocate. + ULONG size = (ULONG) ((rangeEnd - rangeStart) / 128) + 1; + + // To ensure we test the growing logic in debug builds, make the size much smaller. + INDEBUG(size = size / 4 + 1); + cTableCurCount = 0; cTableMaxCount = size; cDeletedEntries = 0; @@ -210,9 +218,7 @@ void UnwindInfoTable::UnRegister() // Add 'data' to the pending buffer for later publication to the OS. // When the buffer is full, entries are flushed under s_pUnwindInfoTablePublishLock. // -/* static */ -void UnwindInfoTable::AddToUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, PT_RUNTIME_FUNCTION data, - TADDR rangeStart, TADDR rangeEnd) +void UnwindInfoTable::AddToUnwindInfoTable(PT_RUNTIME_FUNCTION data) { CONTRACTL { @@ -220,55 +226,29 @@ void UnwindInfoTable::AddToUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, PT_R GC_TRIGGERS; } CONTRACTL_END; - _ASSERTE(data->BeginAddress <= RUNTIME_FUNCTION__EndAddress(data, rangeStart)); - _ASSERTE(RUNTIME_FUNCTION__EndAddress(data, rangeStart) <= (rangeEnd-rangeStart)); - _ASSERTE(unwindInfoPtr != NULL); + _ASSERTE(data->BeginAddress <= RUNTIME_FUNCTION__EndAddress(data, iRangeStart)); + _ASSERTE(RUNTIME_FUNCTION__EndAddress(data, iRangeStart) <= (iRangeEnd - iRangeStart)); if (!s_publishingActive) return; - UnwindInfoTable* unwindInfo = VolatileLoad(unwindInfoPtr); - // was the original list null, If so lazy initialize. - if (unwindInfo == NULL) - { - CrstHolder publishLock(s_pUnwindInfoTablePublishLock); - unwindInfo = *unwindInfoPtr; - if (unwindInfo == NULL) - { - // We can choose the average method size estimate dynamically based on past experience - // 128 is the estimated size of an average method, so we can accurately predict - // how many RUNTIME_FUNCTION entries are in each chunk we allocate. - - ULONG size = (ULONG) ((rangeEnd - rangeStart) / 128) + 1; - - // To ensure we test the growing logic in debug builds, make the size much smaller. - INDEBUG(size = size / 4 + 1); - unwindInfo = (PTR_UnwindInfoTable)new UnwindInfoTable(rangeStart, rangeEnd, size); - unwindInfo->Register(); - VolatileStore(unwindInfoPtr, unwindInfo); - } - } - _ASSERTE(unwindInfo != NULL); // If new had failed, we would have thrown OOM - _ASSERTE(unwindInfo->iRangeStart == rangeStart); - _ASSERTE(unwindInfo->iRangeEnd == rangeEnd); - // Add to the pending buffer. If the buffer is full, flush it first and retry. while (true) { { CrstHolder pendingLock(s_pUnwindInfoTablePendingLock); - if (unwindInfo->cPendingCount < UnwindInfoTable::cPendingMaxCount) + if (cPendingCount < cPendingMaxCount) { - unwindInfo->pPendingTable[unwindInfo->cPendingCount++] = *data; + pPendingTable[cPendingCount++] = *data; STRESS_LOG5(LF_JIT, LL_INFO1000, "AddToUnwindTable Handle: %p [%p, %p] BUFFERED 0x%x, pending 0x%x\n", - unwindInfo->hHandle, unwindInfo->iRangeStart, unwindInfo->iRangeEnd, - data->BeginAddress, unwindInfo->cPendingCount); + hHandle, iRangeStart, iRangeEnd, + data->BeginAddress, cPendingCount); return; } } CrstHolder publishLock(s_pUnwindInfoTablePublishLock); - unwindInfo->FlushPendingEntries(); + FlushPendingEntries(); } } @@ -464,29 +444,41 @@ void UnwindInfoTable::FlushPendingEntries() // Publish the stack unwind data 'data' which is relative 'baseAddress' // to the operating system in a way ETW stack tracing can use. -/* static */ void UnwindInfoTable::PublishUnwindInfoForMethod(TADDR baseAddress, PT_RUNTIME_FUNCTION unwindInfo, int unwindInfoCount) +/* static */ void UnwindInfoTable::PublishUnwindInfoForMethod(TADDR baseAddress, PT_RUNTIME_FUNCTION methodUnwindData, int methodUnwindDataCount) { STANDARD_VM_CONTRACT; if (!s_publishingActive) return; - TADDR entry = baseAddress + unwindInfo->BeginAddress; + TADDR entry = baseAddress + methodUnwindData->BeginAddress; RangeSection * pRS = ExecutionManager::FindCodeRange(entry, ExecutionManager::GetScanFlags()); _ASSERTE(pRS != NULL); if (pRS != NULL) { - for(int i = 0; i < unwindInfoCount; i++) - AddToUnwindInfoTable(&pRS->_pUnwindInfoTable, &unwindInfo[i], pRS->_range.RangeStart(), pRS->_range.RangeEndOpen()); + UnwindInfoTable* unwindInfo = VolatileLoad(&pRS->_pUnwindInfoTable); + if (unwindInfo == NULL) + { + CrstHolder publishLock(s_pUnwindInfoTablePublishLock); + if (pRS->_pUnwindInfoTable == NULL) + { + unwindInfo = new UnwindInfoTable(pRS->_range.RangeStart(), pRS->_range.RangeEndOpen()); + unwindInfo->Register(); + VolatileStore(&pRS->_pUnwindInfoTable, unwindInfo); + } + else + { + unwindInfo = pRS->_pUnwindInfoTable; + } + } + + for (int i = 0; i < methodUnwindDataCount; i++) + unwindInfo->AddToUnwindInfoTable(&methodUnwindData[i]); // Flush any entries that were buffered above so the OS can unwind this // method immediately. Otherwise, we may end up with broken stack traces // for recently JITed methods. CrstHolder publishLock(s_pUnwindInfoTablePublishLock); - UnwindInfoTable* unwindInfoTable = pRS->_pUnwindInfoTable; - if (unwindInfoTable != NULL) - { - unwindInfoTable->FlushPendingEntries(); - } + unwindInfo->FlushPendingEntries(); } } @@ -545,7 +537,7 @@ void UnwindInfoTable::FlushPendingEntries() } #else -/* static */ void UnwindInfoTable::PublishUnwindInfoForMethod(TADDR baseAddress, T_RUNTIME_FUNCTION* unwindInfo, int unwindInfoCount) +/* static */ void UnwindInfoTable::PublishUnwindInfoForMethod(TADDR baseAddress, T_RUNTIME_FUNCTION* methodUnwindData, int methodUnwindDataCount) { LIMITED_METHOD_CONTRACT; } diff --git a/src/coreclr/vm/codeman.h b/src/coreclr/vm/codeman.h index dee41c09bf95b7..d113bbf0c752c4 100644 --- a/src/coreclr/vm/codeman.h +++ b/src/coreclr/vm/codeman.h @@ -597,7 +597,7 @@ class UnwindInfoTable final // All public functions are thread-safe. // These are wrapper functions over the UnwindInfoTable functions that are specific to JIT compile code - static void PublishUnwindInfoForMethod(TADDR baseAddress, T_RUNTIME_FUNCTION* unwindInfo, int unwindInfoCount); + static void PublishUnwindInfoForMethod(TADDR baseAddress, T_RUNTIME_FUNCTION* methodUnwindData, int methodUnwindDataCount); static void UnpublishUnwindInfoForMethod(TADDR entryPoint); static void Initialize(); @@ -606,7 +606,7 @@ class UnwindInfoTable final private: // These are lower level functions that assume you have found the list of UnwindInfoTable entries // These are used by the high-level method functions above - static void AddToUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, T_RUNTIME_FUNCTION* data, TADDR rangeStart, TADDR rangeEnd); + void AddToUnwindInfoTable(T_RUNTIME_FUNCTION* data); static void RemoveFromUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, TADDR baseAddress, TADDR entryPoint); public: @@ -615,7 +615,7 @@ class UnwindInfoTable final private: void UnRegister(); void Register(); - UnwindInfoTable(ULONG_PTR rangeStart, ULONG_PTR rangeEnd, ULONG size); + UnwindInfoTable(ULONG_PTR rangeStart, ULONG_PTR rangeEnd); private: void FlushPendingEntries(); From 28dd859cd09f91ba0b56ef2580f36bc7ea1dc3a0 Mon Sep 17 00:00:00 2001 From: Eduardo Velarde Date: Thu, 19 Mar 2026 15:03:12 -0700 Subject: [PATCH 14/24] Add as many entries as possible after taking the pending lock --- src/coreclr/vm/codeman.cpp | 30 ++++++++++++++++-------------- src/coreclr/vm/codeman.h | 2 +- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index 0e7724473f71b6..083bf21aea456a 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -215,10 +215,10 @@ void UnwindInfoTable::UnRegister() } /*****************************************************************************/ -// Add 'data' to the pending buffer for later publication to the OS. +// Add 'data' entries to the pending buffer for later publication to the OS. // When the buffer is full, entries are flushed under s_pUnwindInfoTablePublishLock. // -void UnwindInfoTable::AddToUnwindInfoTable(PT_RUNTIME_FUNCTION data) +void UnwindInfoTable::AddToUnwindInfoTable(PT_RUNTIME_FUNCTION data, int count) { CONTRACTL { @@ -226,29 +226,32 @@ void UnwindInfoTable::AddToUnwindInfoTable(PT_RUNTIME_FUNCTION data) GC_TRIGGERS; } CONTRACTL_END; - _ASSERTE(data->BeginAddress <= RUNTIME_FUNCTION__EndAddress(data, iRangeStart)); - _ASSERTE(RUNTIME_FUNCTION__EndAddress(data, iRangeStart) <= (iRangeEnd - iRangeStart)); if (!s_publishingActive) return; - // Add to the pending buffer. If the buffer is full, flush it first and retry. - while (true) + for (int i = 0; i < count; ) { { CrstHolder pendingLock(s_pUnwindInfoTablePendingLock); - if (cPendingCount < cPendingMaxCount) + while (i < count && cPendingCount < cPendingMaxCount) { - pPendingTable[cPendingCount++] = *data; + _ASSERTE(data[i].BeginAddress <= RUNTIME_FUNCTION__EndAddress(&data[i], iRangeStart)); + _ASSERTE(RUNTIME_FUNCTION__EndAddress(&data[i], iRangeStart) <= (iRangeEnd - iRangeStart)); + + pPendingTable[cPendingCount++] = data[i]; STRESS_LOG5(LF_JIT, LL_INFO1000, "AddToUnwindTable Handle: %p [%p, %p] BUFFERED 0x%x, pending 0x%x\n", hHandle, iRangeStart, iRangeEnd, - data->BeginAddress, cPendingCount); - return; + data[i].BeginAddress, cPendingCount); + i++; } } - CrstHolder publishLock(s_pUnwindInfoTablePublishLock); - FlushPendingEntries(); + if (i < count) + { + CrstHolder publishLock(s_pUnwindInfoTablePublishLock); + FlushPendingEntries(); + } } } @@ -471,8 +474,7 @@ void UnwindInfoTable::FlushPendingEntries() } } - for (int i = 0; i < methodUnwindDataCount; i++) - unwindInfo->AddToUnwindInfoTable(&methodUnwindData[i]); + unwindInfo->AddToUnwindInfoTable(methodUnwindData, methodUnwindDataCount); // Flush any entries that were buffered above so the OS can unwind this // method immediately. Otherwise, we may end up with broken stack traces diff --git a/src/coreclr/vm/codeman.h b/src/coreclr/vm/codeman.h index d113bbf0c752c4..72b5fb2fe2837c 100644 --- a/src/coreclr/vm/codeman.h +++ b/src/coreclr/vm/codeman.h @@ -606,7 +606,7 @@ class UnwindInfoTable final private: // These are lower level functions that assume you have found the list of UnwindInfoTable entries // These are used by the high-level method functions above - void AddToUnwindInfoTable(T_RUNTIME_FUNCTION* data); + void AddToUnwindInfoTable(T_RUNTIME_FUNCTION* data, int count); static void RemoveFromUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, TADDR baseAddress, TADDR entryPoint); public: From 526beac094736e64eadf126e46e8ff10a92a8046 Mon Sep 17 00:00:00 2001 From: Eduardo Velarde Date: Thu, 19 Mar 2026 15:15:10 -0700 Subject: [PATCH 15/24] Delete old table outside of the lock --- src/coreclr/vm/codeman.cpp | 140 ++++++++++++++++++++----------------- 1 file changed, 74 insertions(+), 66 deletions(-) diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index 083bf21aea456a..096959ad459fa3 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -249,7 +249,6 @@ void UnwindInfoTable::AddToUnwindInfoTable(PT_RUNTIME_FUNCTION data, int count) } if (i < count) { - CrstHolder publishLock(s_pUnwindInfoTablePublishLock); FlushPendingEntries(); } } @@ -265,16 +264,18 @@ void UnwindInfoTable::FlushPendingEntries() } CONTRACTL_END; - _ASSERTE(s_pUnwindInfoTablePublishLock->OwnedByCurrentThread()); - - if (hHandle == NULL) { - // If hHandle is null, it means Register() failed. Skip flushing to avoid calling - // RtlGrowFunctionTable with a null handle. Avoid copying to the local buffer as - // well since it won't be published anyway. - CrstHolder pendingLock(s_pUnwindInfoTablePendingLock); - cPendingCount = 0; - return; + CrstHolder publishLock(s_pUnwindInfoTablePublishLock); + + if (hHandle == NULL) + { + // If hHandle is null, it means Register() failed. Skip flushing to avoid calling + // RtlGrowFunctionTable with a null handle. Avoid copying to the local buffer as + // well since it won't be published anyway. + CrstHolder pendingLock(s_pUnwindInfoTablePendingLock); + cPendingCount = 0; + return; + } } // Grab the pending entries under the pending lock, then release it so @@ -307,81 +308,89 @@ void UnwindInfoTable::FlushPendingEntries() localPending[j] = key; } - // Fast path: if all pending entries can be appended in order with room to spare, - // we can just append and call RtlGrowFunctionTable. - if (cTableCurCount + localPendingCount <= cTableMaxCount - && (cTableCurCount == 0 || pTable[cTableCurCount - 1].BeginAddress < localPending[0].BeginAddress)) + T_RUNTIME_FUNCTION* oldPTable = NULL; { - memcpy(&pTable[cTableCurCount], localPending, localPendingCount * sizeof(T_RUNTIME_FUNCTION)); - cTableCurCount += localPendingCount; - pRtlGrowFunctionTable(hHandle, cTableCurCount); + CrstHolder publishLock(s_pUnwindInfoTablePublishLock); - STRESS_LOG5(LF_JIT, LL_INFO1000, "FlushPendingEntries Handle: %p [%p, %p] APPENDED 0x%x entries, now 0x%x\n", - hHandle, iRangeStart, iRangeEnd, localPendingCount, cTableCurCount); - return; - } + // Fast path: if all pending entries can be appended in order with room to spare, + // we can just append and call RtlGrowFunctionTable. + if (cTableCurCount + localPendingCount <= cTableMaxCount + && (cTableCurCount == 0 || pTable[cTableCurCount - 1].BeginAddress < localPending[0].BeginAddress)) + { + memcpy(&pTable[cTableCurCount], localPending, localPendingCount * sizeof(T_RUNTIME_FUNCTION)); + cTableCurCount += localPendingCount; + pRtlGrowFunctionTable(hHandle, cTableCurCount); - // Merge main table and pending entries. - // Calculate the new table size: live entries from main table + all pending entries - ULONG liveCount = cTableCurCount - cDeletedEntries; - ULONG newCount = liveCount + localPendingCount; - ULONG desiredSpace = newCount * 5 / 4 + 1; // Increase by 20% + STRESS_LOG5(LF_JIT, LL_INFO1000, "FlushPendingEntries Handle: %p [%p, %p] APPENDED 0x%x entries, now 0x%x\n", + hHandle, iRangeStart, iRangeEnd, localPendingCount, cTableCurCount); + return; + } - STRESS_LOG7(LF_JIT, LL_INFO100, "FlushPendingEntries Handle: %p [%p, %p] Merging 0x%x live + 0x%x pending into 0x%x max, from 0x%x\n", - hHandle, iRangeStart, iRangeEnd, liveCount, localPendingCount, desiredSpace, cTableMaxCount); + // Merge main table and pending entries. + // Calculate the new table size: live entries from main table + all pending entries + ULONG liveCount = cTableCurCount - cDeletedEntries; + ULONG newCount = liveCount + localPendingCount; + ULONG desiredSpace = newCount * 5 / 4 + 1; // Increase by 20% - NewArrayHolder newPTable(new T_RUNTIME_FUNCTION[desiredSpace]); + STRESS_LOG7(LF_JIT, LL_INFO100, "FlushPendingEntries Handle: %p [%p, %p] Merging 0x%x live + 0x%x pending into 0x%x max, from 0x%x\n", + hHandle, iRangeStart, iRangeEnd, liveCount, localPendingCount, desiredSpace, cTableMaxCount); - // Merge-sort the main table and pending buffer into newPTable. - ULONG mainIdx = 0; - ULONG pendIdx = 0; - ULONG toIdx = 0; + NewArrayHolder newPTable(new T_RUNTIME_FUNCTION[desiredSpace]); - while (mainIdx < cTableCurCount && pendIdx < localPendingCount) - { - // Skip deleted entries in main table - if (pTable[mainIdx].UnwindData == 0) + // Merge-sort the main table and pending buffer into newPTable. + ULONG mainIdx = 0; + ULONG pendIdx = 0; + ULONG toIdx = 0; + + while (mainIdx < cTableCurCount && pendIdx < localPendingCount) { - mainIdx++; - continue; + // Skip deleted entries in main table + if (pTable[mainIdx].UnwindData == 0) + { + mainIdx++; + continue; + } + + if (localPending[pendIdx].BeginAddress < pTable[mainIdx].BeginAddress) + { + newPTable[toIdx++] = localPending[pendIdx++]; + } + else + { + newPTable[toIdx++] = pTable[mainIdx++]; + } } - if (localPending[pendIdx].BeginAddress < pTable[mainIdx].BeginAddress) + while (mainIdx < cTableCurCount) { - newPTable[toIdx++] = localPending[pendIdx++]; + if (pTable[mainIdx].UnwindData != 0) + newPTable[toIdx++] = pTable[mainIdx]; + mainIdx++; } - else + + while (pendIdx < localPendingCount) { - newPTable[toIdx++] = pTable[mainIdx++]; + newPTable[toIdx++] = localPending[pendIdx++]; } - } - while (mainIdx < cTableCurCount) - { - if (pTable[mainIdx].UnwindData != 0) - newPTable[toIdx++] = pTable[mainIdx]; - mainIdx++; - } + _ASSERTE(toIdx == newCount); + _ASSERTE(toIdx <= desiredSpace); - while (pendIdx < localPendingCount) - { - newPTable[toIdx++] = localPending[pendIdx++]; - } + oldPTable = pTable; - _ASSERTE(toIdx == newCount); - _ASSERTE(toIdx <= desiredSpace); + UnRegister(); - NewArrayHolder oldPTable(pTable); + pTable = newPTable.Extract(); + cTableCurCount = toIdx; + cTableMaxCount = desiredSpace; + cDeletedEntries = 0; - UnRegister(); - - pTable = newPTable.Extract(); - cTableCurCount = toIdx; - cTableMaxCount = desiredSpace; - cDeletedEntries = 0; + // Note that there is a short time when we are not publishing... + Register(); + } - // Note that there is a short time when we are not publishing... - Register(); + // delete[] outside the publish lock — can be expensive for large allocations across threads. + delete[] oldPTable; } /*****************************************************************************/ @@ -479,7 +488,6 @@ void UnwindInfoTable::FlushPendingEntries() // Flush any entries that were buffered above so the OS can unwind this // method immediately. Otherwise, we may end up with broken stack traces // for recently JITed methods. - CrstHolder publishLock(s_pUnwindInfoTablePublishLock); unwindInfo->FlushPendingEntries(); } } From fbe8eb60e44e6d2c22195ef6aadce33456a61866 Mon Sep 17 00:00:00 2001 From: Eduardo Velarde <32459232+eduardo-vp@users.noreply.github.com> Date: Thu, 19 Mar 2026 15:45:23 -0700 Subject: [PATCH 16/24] Flush after adding all the entries Co-authored-by: Jan Kotas --- src/coreclr/vm/codeman.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index 096959ad459fa3..b4cca34d78cc50 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -247,10 +247,9 @@ void UnwindInfoTable::AddToUnwindInfoTable(PT_RUNTIME_FUNCTION data, int count) i++; } } - if (i < count) - { - FlushPendingEntries(); - } + // Flush any pending entries if we run out of space, or when we are at the end + // of the batch so the OS can unwind this method immediately. + FlushPendingEntries(); } } From a65988f48394a46c6e27191603949a15f408e3aa Mon Sep 17 00:00:00 2001 From: Eduardo Velarde <32459232+eduardo-vp@users.noreply.github.com> Date: Thu, 19 Mar 2026 15:46:08 -0700 Subject: [PATCH 17/24] Remove flush at the end of publish method Co-authored-by: Jan Kotas --- src/coreclr/vm/codeman.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index b4cca34d78cc50..295dc7c2b10cbc 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -483,11 +483,6 @@ void UnwindInfoTable::FlushPendingEntries() } unwindInfo->AddToUnwindInfoTable(methodUnwindData, methodUnwindDataCount); - - // Flush any entries that were buffered above so the OS can unwind this - // method immediately. Otherwise, we may end up with broken stack traces - // for recently JITed methods. - unwindInfo->FlushPendingEntries(); } } From f0560fded1fc048f73f7a098fb18baea18959ea4 Mon Sep 17 00:00:00 2001 From: Eduardo Velarde Date: Thu, 19 Mar 2026 16:47:58 -0700 Subject: [PATCH 18/24] Some PR feedback --- src/coreclr/vm/codeman.cpp | 79 ++++++++++++++++---------------------- src/coreclr/vm/codeman.h | 2 +- 2 files changed, 34 insertions(+), 47 deletions(-) diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index 295dc7c2b10cbc..f5a66166ab00fb 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -177,29 +177,19 @@ UnwindInfoTable::~UnwindInfoTable() void UnwindInfoTable::Register() { _ASSERTE(s_pUnwindInfoTablePublishLock->OwnedByCurrentThread()); - EX_TRY + + hHandle = NULL; + NTSTATUS ret = pRtlAddGrowableFunctionTable(&hHandle, pTable, cTableCurCount, cTableMaxCount, iRangeStart, iRangeEnd); + if (ret != STATUS_SUCCESS) { + _ASSERTE(!"Failed to publish UnwindInfo (ignorable)"); hHandle = NULL; - NTSTATUS ret = pRtlAddGrowableFunctionTable(&hHandle, pTable, cTableCurCount, cTableMaxCount, iRangeStart, iRangeEnd); - if (ret != STATUS_SUCCESS) - { - _ASSERTE(!"Failed to publish UnwindInfo (ignorable)"); - hHandle = NULL; - STRESS_LOG3(LF_JIT, LL_ERROR, "UnwindInfoTable::Register ERROR %x creating table [%p, %p]\n", ret, iRangeStart, iRangeEnd); - } - else - { - STRESS_LOG3(LF_JIT, LL_INFO100, "UnwindInfoTable::Register Handle: %p [%p, %p]\n", hHandle, iRangeStart, iRangeEnd); - } + STRESS_LOG3(LF_JIT, LL_ERROR, "UnwindInfoTable::Register ERROR %x creating table [%p, %p]\n", ret, iRangeStart, iRangeEnd); } - EX_CATCH + else { - hHandle = NULL; - STRESS_LOG2(LF_JIT, LL_ERROR, "UnwindInfoTable::Register Exception while creating table [%p, %p]\n", - iRangeStart, iRangeEnd); - _ASSERTE(!"Failed to publish UnwindInfo (ignorable)"); + STRESS_LOG3(LF_JIT, LL_INFO100, "UnwindInfoTable::Register Handle: %p [%p, %p]\n", hHandle, iRangeStart, iRangeEnd); } - EX_END_CATCH } /*****************************************************************************/ @@ -227,8 +217,7 @@ void UnwindInfoTable::AddToUnwindInfoTable(PT_RUNTIME_FUNCTION data, int count) } CONTRACTL_END; - if (!s_publishingActive) - return; + _ASSERTE(s_publishingActive); for (int i = 0; i < count; ) { @@ -239,7 +228,7 @@ void UnwindInfoTable::AddToUnwindInfoTable(PT_RUNTIME_FUNCTION data, int count) _ASSERTE(data[i].BeginAddress <= RUNTIME_FUNCTION__EndAddress(&data[i], iRangeStart)); _ASSERTE(RUNTIME_FUNCTION__EndAddress(&data[i], iRangeStart) <= (iRangeEnd - iRangeStart)); - pPendingTable[cPendingCount++] = data[i]; + pendingTable[cPendingCount++] = data[i]; STRESS_LOG5(LF_JIT, LL_INFO1000, "AddToUnwindTable Handle: %p [%p, %p] BUFFERED 0x%x, pending 0x%x\n", hHandle, iRangeStart, iRangeEnd, @@ -284,8 +273,9 @@ void UnwindInfoTable::FlushPendingEntries() { CrstHolder pendingLock(s_pUnwindInfoTablePendingLock); localPendingCount = cPendingCount; - memcpy(localPending, pPendingTable, cPendingCount * sizeof(T_RUNTIME_FUNCTION)); + memcpy(localPending, pendingTable, cPendingCount * sizeof(T_RUNTIME_FUNCTION)); cPendingCount = 0; + INDEBUG( memset(pendingTable, 0xcc, sizeof(pendingTable)); ) } if (localPendingCount == 0) @@ -307,7 +297,9 @@ void UnwindInfoTable::FlushPendingEntries() localPending[j] = key; } - T_RUNTIME_FUNCTION* oldPTable = NULL; + // delete tables outside the publish lock — can be expensive for large allocations across threads. + NewArrayHolder oldPTable; + NewArrayHolder newPTable; { CrstHolder publishLock(s_pUnwindInfoTablePublishLock); @@ -334,7 +326,7 @@ void UnwindInfoTable::FlushPendingEntries() STRESS_LOG7(LF_JIT, LL_INFO100, "FlushPendingEntries Handle: %p [%p, %p] Merging 0x%x live + 0x%x pending into 0x%x max, from 0x%x\n", hHandle, iRangeStart, iRangeEnd, liveCount, localPendingCount, desiredSpace, cTableMaxCount); - NewArrayHolder newPTable(new T_RUNTIME_FUNCTION[desiredSpace]); + newPTable = new T_RUNTIME_FUNCTION[desiredSpace]; // Merge-sort the main table and pending buffer into newPTable. ULONG mainIdx = 0; @@ -387,9 +379,6 @@ void UnwindInfoTable::FlushPendingEntries() // Note that there is a short time when we are not publishing... Register(); } - - // delete[] outside the publish lock — can be expensive for large allocations across threads. - delete[] oldPTable; } /*****************************************************************************/ @@ -437,10 +426,10 @@ void UnwindInfoTable::FlushPendingEntries() CrstHolder pendingLock(s_pUnwindInfoTablePendingLock); for (ULONG i = 0; i < unwindInfo->cPendingCount; i++) { - if (unwindInfo->pPendingTable[i].BeginAddress <= relativeEntryPoint && - relativeEntryPoint < RUNTIME_FUNCTION__EndAddress(&unwindInfo->pPendingTable[i], unwindInfo->iRangeStart)) + if (unwindInfo->pendingTable[i].BeginAddress <= relativeEntryPoint && + relativeEntryPoint < RUNTIME_FUNCTION__EndAddress(&unwindInfo->pendingTable[i], unwindInfo->iRangeStart)) { - unwindInfo->pPendingTable[i] = unwindInfo->pPendingTable[--unwindInfo->cPendingCount]; + unwindInfo->pendingTable[i] = unwindInfo->pendingTable[--unwindInfo->cPendingCount]; STRESS_LOG1(LF_JIT, LL_INFO100, "RemoveFromUnwindInfoTable Removed pending entry 0x%x\n", i); return; } @@ -464,26 +453,24 @@ void UnwindInfoTable::FlushPendingEntries() TADDR entry = baseAddress + methodUnwindData->BeginAddress; RangeSection * pRS = ExecutionManager::FindCodeRange(entry, ExecutionManager::GetScanFlags()); _ASSERTE(pRS != NULL); - if (pRS != NULL) + + UnwindInfoTable* unwindInfo = VolatileLoad(&pRS->_pUnwindInfoTable); + if (unwindInfo == NULL) { - UnwindInfoTable* unwindInfo = VolatileLoad(&pRS->_pUnwindInfoTable); - if (unwindInfo == NULL) + CrstHolder publishLock(s_pUnwindInfoTablePublishLock); + if (pRS->_pUnwindInfoTable == NULL) { - CrstHolder publishLock(s_pUnwindInfoTablePublishLock); - if (pRS->_pUnwindInfoTable == NULL) - { - unwindInfo = new UnwindInfoTable(pRS->_range.RangeStart(), pRS->_range.RangeEndOpen()); - unwindInfo->Register(); - VolatileStore(&pRS->_pUnwindInfoTable, unwindInfo); - } - else - { - unwindInfo = pRS->_pUnwindInfoTable; - } + unwindInfo = new UnwindInfoTable(pRS->_range.RangeStart(), pRS->_range.RangeEndOpen()); + unwindInfo->Register(); + VolatileStore(&pRS->_pUnwindInfoTable, unwindInfo); + } + else + { + unwindInfo = pRS->_pUnwindInfoTable; } - - unwindInfo->AddToUnwindInfoTable(methodUnwindData, methodUnwindDataCount); } + + unwindInfo->AddToUnwindInfoTable(methodUnwindData, methodUnwindDataCount); } /*****************************************************************************/ diff --git a/src/coreclr/vm/codeman.h b/src/coreclr/vm/codeman.h index 72b5fb2fe2837c..843aab8ebc3fe6 100644 --- a/src/coreclr/vm/codeman.h +++ b/src/coreclr/vm/codeman.h @@ -632,7 +632,7 @@ class UnwindInfoTable final // These entries are accumulated and batch-merged into pTable to amortize the cost of // RtlDeleteGrowableFunctionTable + RtlAddGrowableFunctionTable. static const ULONG cPendingMaxCount = 32; - T_RUNTIME_FUNCTION pPendingTable[cPendingMaxCount]; + T_RUNTIME_FUNCTION pendingTable[cPendingMaxCount]; ULONG cPendingCount; #endif // defined(TARGET_AMD64) && defined(TARGET_WINDOWS) }; From 9db1f738763412d8202df4514ac4524b65904bf4 Mon Sep 17 00:00:00 2001 From: Eduardo Velarde Date: Thu, 19 Mar 2026 16:58:40 -0700 Subject: [PATCH 19/24] Remove unnecessary check of the pending buffer when removing unwind info --- src/coreclr/vm/codeman.cpp | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index f5a66166ab00fb..0888379a91c27b 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -420,22 +420,6 @@ void UnwindInfoTable::FlushPendingEntries() } } - // Check the pending buffer under the pending lock. - // Use swap-remove since the buffer doesn't need to stay sorted. - { - CrstHolder pendingLock(s_pUnwindInfoTablePendingLock); - for (ULONG i = 0; i < unwindInfo->cPendingCount; i++) - { - if (unwindInfo->pendingTable[i].BeginAddress <= relativeEntryPoint && - relativeEntryPoint < RUNTIME_FUNCTION__EndAddress(&unwindInfo->pendingTable[i], unwindInfo->iRangeStart)) - { - unwindInfo->pendingTable[i] = unwindInfo->pendingTable[--unwindInfo->cPendingCount]; - STRESS_LOG1(LF_JIT, LL_INFO100, "RemoveFromUnwindInfoTable Removed pending entry 0x%x\n", i); - return; - } - } - } - STRESS_LOG2(LF_JIT, LL_WARNING, "RemoveFromUnwindInfoTable COULD NOT FIND %p BaseAddress %p\n", entryPoint, baseAddress); } From 8a82cf583d1bc8497f42b781a86daab9aa5f36fc Mon Sep 17 00:00:00 2001 From: Eduardo Velarde Date: Thu, 19 Mar 2026 17:42:05 -0700 Subject: [PATCH 20/24] Take publish lock at the beginning of FlushPendingEntries --- src/coreclr/vm/codeman.cpp | 139 +++++++++++++++++-------------------- 1 file changed, 64 insertions(+), 75 deletions(-) diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index 0888379a91c27b..1bd55b9fe4333a 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -252,22 +252,19 @@ void UnwindInfoTable::FlushPendingEntries() } CONTRACTL_END; - { - CrstHolder publishLock(s_pUnwindInfoTablePublishLock); + CrstHolder publishLock(s_pUnwindInfoTablePublishLock); - if (hHandle == NULL) - { - // If hHandle is null, it means Register() failed. Skip flushing to avoid calling - // RtlGrowFunctionTable with a null handle. Avoid copying to the local buffer as - // well since it won't be published anyway. - CrstHolder pendingLock(s_pUnwindInfoTablePendingLock); - cPendingCount = 0; - return; - } + if (hHandle == NULL) + { + // If hHandle is null, it means Register() failed. Skip flushing to avoid calling + // RtlGrowFunctionTable with a null handle. + CrstHolder pendingLock(s_pUnwindInfoTablePendingLock); + cPendingCount = 0; + return; } // Grab the pending entries under the pending lock, then release it so - // other threads can keep accumulating new entries while we flush. + // other threads can keep accumulating new entries while we publish. T_RUNTIME_FUNCTION localPending[cPendingMaxCount]; ULONG localPendingCount; { @@ -297,88 +294,80 @@ void UnwindInfoTable::FlushPendingEntries() localPending[j] = key; } - // delete tables outside the publish lock — can be expensive for large allocations across threads. - NewArrayHolder oldPTable; - NewArrayHolder newPTable; + // Fast path: if all pending entries can be appended in order with room to spare, + // we can just append and call RtlGrowFunctionTable. + if (cTableCurCount + localPendingCount <= cTableMaxCount + && (cTableCurCount == 0 || pTable[cTableCurCount - 1].BeginAddress < localPending[0].BeginAddress)) { - CrstHolder publishLock(s_pUnwindInfoTablePublishLock); + memcpy(&pTable[cTableCurCount], localPending, localPendingCount * sizeof(T_RUNTIME_FUNCTION)); + cTableCurCount += localPendingCount; + pRtlGrowFunctionTable(hHandle, cTableCurCount); - // Fast path: if all pending entries can be appended in order with room to spare, - // we can just append and call RtlGrowFunctionTable. - if (cTableCurCount + localPendingCount <= cTableMaxCount - && (cTableCurCount == 0 || pTable[cTableCurCount - 1].BeginAddress < localPending[0].BeginAddress)) - { - memcpy(&pTable[cTableCurCount], localPending, localPendingCount * sizeof(T_RUNTIME_FUNCTION)); - cTableCurCount += localPendingCount; - pRtlGrowFunctionTable(hHandle, cTableCurCount); - - STRESS_LOG5(LF_JIT, LL_INFO1000, "FlushPendingEntries Handle: %p [%p, %p] APPENDED 0x%x entries, now 0x%x\n", - hHandle, iRangeStart, iRangeEnd, localPendingCount, cTableCurCount); - return; - } - - // Merge main table and pending entries. - // Calculate the new table size: live entries from main table + all pending entries - ULONG liveCount = cTableCurCount - cDeletedEntries; - ULONG newCount = liveCount + localPendingCount; - ULONG desiredSpace = newCount * 5 / 4 + 1; // Increase by 20% + STRESS_LOG5(LF_JIT, LL_INFO1000, "FlushPendingEntries Handle: %p [%p, %p] APPENDED 0x%x entries, now 0x%x\n", + hHandle, iRangeStart, iRangeEnd, localPendingCount, cTableCurCount); + return; + } - STRESS_LOG7(LF_JIT, LL_INFO100, "FlushPendingEntries Handle: %p [%p, %p] Merging 0x%x live + 0x%x pending into 0x%x max, from 0x%x\n", - hHandle, iRangeStart, iRangeEnd, liveCount, localPendingCount, desiredSpace, cTableMaxCount); + // Merge main table and pending entries. + // Calculate the new table size: live entries from main table + all pending entries + ULONG liveCount = cTableCurCount - cDeletedEntries; + ULONG newCount = liveCount + localPendingCount; + ULONG desiredSpace = newCount * 5 / 4 + 1; // Increase by 20% - newPTable = new T_RUNTIME_FUNCTION[desiredSpace]; + STRESS_LOG7(LF_JIT, LL_INFO100, "FlushPendingEntries Handle: %p [%p, %p] Merging 0x%x live + 0x%x pending into 0x%x max, from 0x%x\n", + hHandle, iRangeStart, iRangeEnd, liveCount, localPendingCount, desiredSpace, cTableMaxCount); - // Merge-sort the main table and pending buffer into newPTable. - ULONG mainIdx = 0; - ULONG pendIdx = 0; - ULONG toIdx = 0; + NewArrayHolder newPTable(new T_RUNTIME_FUNCTION[desiredSpace]); - while (mainIdx < cTableCurCount && pendIdx < localPendingCount) - { - // Skip deleted entries in main table - if (pTable[mainIdx].UnwindData == 0) - { - mainIdx++; - continue; - } + // Merge-sort the main table and pending buffer into newPTable. + ULONG mainIdx = 0; + ULONG pendIdx = 0; + ULONG toIdx = 0; - if (localPending[pendIdx].BeginAddress < pTable[mainIdx].BeginAddress) - { - newPTable[toIdx++] = localPending[pendIdx++]; - } - else - { - newPTable[toIdx++] = pTable[mainIdx++]; - } - } - - while (mainIdx < cTableCurCount) + while (mainIdx < cTableCurCount && pendIdx < localPendingCount) + { + // Skip deleted entries in main table + if (pTable[mainIdx].UnwindData == 0) { - if (pTable[mainIdx].UnwindData != 0) - newPTable[toIdx++] = pTable[mainIdx]; mainIdx++; + continue; } - while (pendIdx < localPendingCount) + if (localPending[pendIdx].BeginAddress < pTable[mainIdx].BeginAddress) { newPTable[toIdx++] = localPending[pendIdx++]; } + else + { + newPTable[toIdx++] = pTable[mainIdx++]; + } + } + + while (mainIdx < cTableCurCount) + { + if (pTable[mainIdx].UnwindData != 0) + newPTable[toIdx++] = pTable[mainIdx]; + mainIdx++; + } + + while (pendIdx < localPendingCount) + { + newPTable[toIdx++] = localPending[pendIdx++]; + } - _ASSERTE(toIdx == newCount); - _ASSERTE(toIdx <= desiredSpace); + _ASSERTE(toIdx == newCount); + _ASSERTE(toIdx <= desiredSpace); - oldPTable = pTable; + NewArrayHolder oldPTable(pTable); - UnRegister(); + UnRegister(); - pTable = newPTable.Extract(); - cTableCurCount = toIdx; - cTableMaxCount = desiredSpace; - cDeletedEntries = 0; + pTable = newPTable.Extract(); + cTableCurCount = toIdx; + cTableMaxCount = desiredSpace; + cDeletedEntries = 0; - // Note that there is a short time when we are not publishing... - Register(); - } + Register(); } /*****************************************************************************/ From d41ee90112765971a4a58c2a03a9af5db990daa6 Mon Sep 17 00:00:00 2001 From: Eduardo Velarde <32459232+eduardo-vp@users.noreply.github.com> Date: Thu, 19 Mar 2026 17:58:01 -0700 Subject: [PATCH 21/24] Free the old table outside the lock Co-authored-by: Jan Kotas --- src/coreclr/vm/codeman.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index 1bd55b9fe4333a..73fc8cbb54c86c 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -252,6 +252,9 @@ void UnwindInfoTable::FlushPendingEntries() } CONTRACTL_END; + // Free the old table outside the lock + NewArrayHolder oldPTable; + CrstHolder publishLock(s_pUnwindInfoTablePublishLock); if (hHandle == NULL) From ee9798097a64e51f3ef9c54623a056661b5327f0 Mon Sep 17 00:00:00 2001 From: Eduardo Velarde <32459232+eduardo-vp@users.noreply.github.com> Date: Thu, 19 Mar 2026 17:58:17 -0700 Subject: [PATCH 22/24] PR feedback Co-authored-by: Jan Kotas --- src/coreclr/vm/codeman.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index 73fc8cbb54c86c..90e26f89ad3d52 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -361,7 +361,7 @@ void UnwindInfoTable::FlushPendingEntries() _ASSERTE(toIdx == newCount); _ASSERTE(toIdx <= desiredSpace); - NewArrayHolder oldPTable(pTable); + oldPTable = pTable; UnRegister(); From fd3cfaaddfad1be172ff13c943296c24656f992c Mon Sep 17 00:00:00 2001 From: Eduardo Velarde Date: Thu, 19 Mar 2026 18:59:34 -0700 Subject: [PATCH 23/24] Update comment --- src/coreclr/vm/codeman.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index 90e26f89ad3d52..395bee4d326abf 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -395,7 +395,9 @@ void UnwindInfoTable::FlushPendingEntries() STRESS_LOG3(LF_JIT, LL_INFO100, "RemoveFromUnwindInfoTable Removing %p BaseAddress %p rel %x\n", entryPoint, baseAddress, relativeEntryPoint); - // First check the main (published) table under the publish lock. + // Check the main (published) table under the publish lock. + // We don't need to check the pending buffer because the method should have already been published + // before it can be removed. { CrstHolder publishLock(s_pUnwindInfoTablePublishLock); for (ULONG i = 0; i < unwindInfo->cTableCurCount; i++) From 84a197a83284a87c83d5a5c75be8032b63137a57 Mon Sep 17 00:00:00 2001 From: Eduardo Velarde Date: Thu, 19 Mar 2026 22:22:46 -0700 Subject: [PATCH 24/24] Nits --- src/coreclr/vm/codeman.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index 395bee4d326abf..fd9ea88f4d75f6 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -178,7 +178,6 @@ void UnwindInfoTable::Register() { _ASSERTE(s_pUnwindInfoTablePublishLock->OwnedByCurrentThread()); - hHandle = NULL; NTSTATUS ret = pRtlAddGrowableFunctionTable(&hHandle, pTable, cTableCurCount, cTableMaxCount, iRangeStart, iRangeEnd); if (ret != STATUS_SUCCESS) { @@ -363,6 +362,14 @@ void UnwindInfoTable::FlushPendingEntries() oldPTable = pTable; + // The OS growable function table API (RtlGrowFunctionTable) only supports + // appending sorted entries, it cannot shrink, reorder, or remove entries. + // We have to tear down the old OS registration and create a new one + // combining the old and pending entries while skipping the deleted ones. + // We should keep the gap between UnRegister and Register as short as possible, + // as OS stack walks will have no unwind info for this range during that + // window. The new table is fully built before UnRegister to minimize this gap. + UnRegister(); pTable = newPTable.Extract();