From 14021c31815f2bd3216996fc36b34e73b63f6636 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 17 Dec 2025 13:28:22 -0800 Subject: [PATCH 1/3] [clr-interp] Tweak GC reporting The interpreter calling convention can result in double reporting of arguments passed to callee's. This works today as all of the arguments will be conservatively reported, but it also adds the problem of excess conservative reporting. This change fixes that most of that problem by only dropping reporting of conservative pointers when they point into a callee's stack space. Note that this does not fix is the scenario where an object reference is held on the IL stack across a call. In those cases the value shall still be reported conservatively. This PR also tweaks the Collect0 test to disable it under the interpreter as the C# compiler generates that particular problematic pattern for this test case. --- src/coreclr/vm/gcinfodecoder.cpp | 33 +++++++++++++++++++++++++++++++- src/tests/GC/API/GC/Collect0.cs | 1 + 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/gcinfodecoder.cpp b/src/coreclr/vm/gcinfodecoder.cpp index 50c09cdd0bb41e..c5663e0ad1adc9 100644 --- a/src/coreclr/vm/gcinfodecoder.cpp +++ b/src/coreclr/vm/gcinfodecoder.cpp @@ -6,6 +6,7 @@ #endif #include "gcinfodecoder.h" +#include "interpexec.h" #ifdef USE_GC_INFO_DECODER @@ -2114,6 +2115,9 @@ template OBJECTREF* TGcInfoDecoder::Ge #endif // Unknown platform #ifdef FEATURE_INTERPRETER + +uintptr_t ZeroValue = 0; + template <> OBJECTREF* TGcInfoDecoder::GetStackSlot( INT32 spOffset, GcStackSlotBase spBase, @@ -2139,7 +2143,25 @@ template <> OBJECTREF* TGcInfoDecoder::GetStackSlot( _ASSERTE(fp); pObjRef = (OBJECTREF*)(fp + spOffset); } + InterpMethodContextFrame* pFrame = (InterpMethodContextFrame*)GetSP(pRD->pCurrentContext); + _ASSERTE(pFrame->pStack == (int8_t *)GetFP(pRD->pCurrentContext)); + InterpMethodContextFrame* pFrameCallee = pFrame->pNext; + // If the stack slot is in a callee's frame, then we do not actually need to report it. This should ONLY happen if the + // stack slot is in the argument area of the caller. As a double check, we validate that we only return this address if + // the + if (pFrameCallee != NULL) + { + if (pFrameCallee->ip != 0) + { + _ASSERTE(pFrameCallee->pStack > pFrame->pStack); // Since only the last funclet is GC reported, we shouldn't have any cases where the stack doesn't grow + if (pFrameCallee->pStack <= (int8_t*)pObjRef) + { + // The stack slot is in the callee's frame, not the caller's frame. + pObjRef = (OBJECTREF*)&ZeroValue; + } + } + } return pObjRef; } #endif @@ -2221,7 +2243,16 @@ template void TGcInfoDecoder::ReportSt OBJECTREF* pObjRef = GetStackSlot(spOffset, spBase, pRD); _ASSERTE(IS_ALIGNED(pObjRef, sizeof(OBJECTREF*))); - +#ifdef FEATURE_INTERPRETER + // This value is returned when the interpreter stack slot is not actually meaningful. + // This should only happen for stack slots which are conservatively reported, and for better perf + // we completely skip reporting them. + if (pObjRef == (OBJECTREF*)&ZeroValue) + { + _ASSERTE((gcFlags & (GC_CALL_PINNED | GC_CALL_INTERIOR)) == (GC_CALL_PINNED | GC_CALL_INTERIOR)); + return; + } +#endif // FEATURE_INTERPRETER #ifdef _DEBUG LOG((LF_GCROOTS, LL_INFO1000, /* Part One */ "Reporting %s" FMT_STK, diff --git a/src/tests/GC/API/GC/Collect0.cs b/src/tests/GC/API/GC/Collect0.cs index 55840378fd5d08..0a8514afca5e12 100644 --- a/src/tests/GC/API/GC/Collect0.cs +++ b/src/tests/GC/API/GC/Collect0.cs @@ -7,6 +7,7 @@ public class Test_Collect0 { [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/118965", typeof(TestLibrary.Utilities), nameof(TestLibrary.Utilities.IsCoreClrInterpreter))] public static int TestEntryPoint() { int[] array = new int[25]; From c96b51e877fba9da9311b6862e533a0e490e7cd8 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Thu, 18 Dec 2025 13:42:47 -0800 Subject: [PATCH 2/3] Address feedback --- src/coreclr/vm/gcinfodecoder.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/coreclr/vm/gcinfodecoder.cpp b/src/coreclr/vm/gcinfodecoder.cpp index c5663e0ad1adc9..c2aff6210e58fd 100644 --- a/src/coreclr/vm/gcinfodecoder.cpp +++ b/src/coreclr/vm/gcinfodecoder.cpp @@ -6,7 +6,9 @@ #endif #include "gcinfodecoder.h" +#ifdef FEATURE_INTERPRETER #include "interpexec.h" +#endif // FEATURE_INTERPRETER #ifdef USE_GC_INFO_DECODER @@ -2116,8 +2118,6 @@ template OBJECTREF* TGcInfoDecoder::Ge #ifdef FEATURE_INTERPRETER -uintptr_t ZeroValue = 0; - template <> OBJECTREF* TGcInfoDecoder::GetStackSlot( INT32 spOffset, GcStackSlotBase spBase, @@ -2148,8 +2148,8 @@ template <> OBJECTREF* TGcInfoDecoder::GetStackSlot( InterpMethodContextFrame* pFrameCallee = pFrame->pNext; // If the stack slot is in a callee's frame, then we do not actually need to report it. This should ONLY happen if the - // stack slot is in the argument area of the caller. As a double check, we validate that we only return this address if - // the + // stack slot is in the argument area of the caller. As a double check, we validate in the caller of this function that + // the stack slot in question is a interior pinned slot (which when FEATURE_INTERPRETER is defined indicates a conservatively reported stack slot). if (pFrameCallee != NULL) { if (pFrameCallee->ip != 0) @@ -2158,7 +2158,8 @@ template <> OBJECTREF* TGcInfoDecoder::GetStackSlot( if (pFrameCallee->pStack <= (int8_t*)pObjRef) { // The stack slot is in the callee's frame, not the caller's frame. - pObjRef = (OBJECTREF*)&ZeroValue; + // Return as a sentinel to indicate nothing is reported here. + pObjRef = NULL; } } } @@ -2247,7 +2248,7 @@ template void TGcInfoDecoder::ReportSt // This value is returned when the interpreter stack slot is not actually meaningful. // This should only happen for stack slots which are conservatively reported, and for better perf // we completely skip reporting them. - if (pObjRef == (OBJECTREF*)&ZeroValue) + if (pObjRef == (OBJECTREF*)NULL) { _ASSERTE((gcFlags & (GC_CALL_PINNED | GC_CALL_INTERIOR)) == (GC_CALL_PINNED | GC_CALL_INTERIOR)); return; From b7cf147437af56d009016918138783bcf26c11b6 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Thu, 18 Dec 2025 15:04:26 -0800 Subject: [PATCH 3/3] Fix test build oops --- src/tests/GC/API/GC/Collect0.csproj | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tests/GC/API/GC/Collect0.csproj b/src/tests/GC/API/GC/Collect0.csproj index 7a51346cd22079..120de12576c7e7 100644 --- a/src/tests/GC/API/GC/Collect0.csproj +++ b/src/tests/GC/API/GC/Collect0.csproj @@ -9,5 +9,6 @@ +