[Interpreter] Checked runtime asserts Thread::IsObjRefValid(this) on an interpreted virtual call after CreateCustomAttributeInstance unregisters its GC frame #135535

Description

Since #133658 moved custom attribute instantiation to managed code, a Checked windows-x64 runtime running with DOTNET_InterpMode=2 or 3 asserts in OBJECTREF::operator-> on an interpreted virtual call made some time after a custom attribute was instantiated:

Assert failure: Thread::IsObjRefValid(this)
    File: src\coreclr\vm\object.cpp:1031

CORECLR! OBJECTREF::operator-> + 0xCF
CORECLR! InterpExecMethod + 0x3649
CORECLR! ExecuteInterpretedMethodBody + 0xC2
...

The scheduled runtime-interpreter pipeline hits it too. In 20261009.1 (main 4d1b425), the windows x64 checked Pri1 runtime-test work items GC, JIT.performance, Regression_o_1, async, reflection and tracing (DOTNET_InterpMode=3), and the same minus GC (DOTNET_InterpMode=2), fail with this assert, each at its first affected test. (The earlier scheduled runs failed for other reasons, so I can't use them to bracket the change. The managed GCFrameRegistration use that leads here was added by #133658.)

System.Runtime.Tests hits it during xunit discovery, before any test runs, in every run I made: 5 of 5 under mode 3 at main, and 3 of 3 under mode 3 and 1 of 1 under mode 2 with the diagnostic build described below. On linux-x64 and osx-arm64 the same discovery under mode 3 completed in 10 of 10 runs each. As far as I can tell this is a false positive of the debug-only tracking of unprotected OBJECTREFs, not a GC hole.

Reproduction

using System;
using System.Runtime.CompilerServices;

[AttributeUsage(AttributeTargets.Class)]
sealed class FourArgsAttribute : Attribute
{
    public FourArgsAttribute(int a, int b, int c, int d) { }
}

[FourArgs(1, 2, 3, 4)]
sealed class Decorated { }

abstract class Base { public abstract int Get(); }
sealed class Derived : Base { public override int Get() => 1; }

static class Program
{
    static Base s_target = new Derived();

    static int Main()
    {
        // Warm up the paths used below, to reduce later prestub activity (which can flush the debug table).
        typeof(Decorated).GetCustomAttributes(false);
        GC.Collect();
        Walk(1, 1, 1);

        // 1. CreateCustomAttributeInstance registers its argument storage, a local on the interpreter stack, for GC
        //    reporting. Unregistering it puts those addresses in the thread's table of unprotected OBJECTREFs.
        typeof(Decorated).GetCustomAttributes(false);
        // 2. A possible GC point marks the recorded addresses (other transitions can do this too).
        GC.Collect();
        // 3. Virtual calls whose 'this' argument lands on the interpreter stack at many addresses above Main's frame.
        for (int a = 0; a < 200; a++)
            for (int b = 0; b < 4; b++)
                for (int c = 0; c < 4; c++)
                    Walk(a, b, c);

        Console.WriteLine("no assert");
        return 100;
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    static int Walk(int a, int b, int c) => a > 0 ? Walk(a - 1, b, c) : Walk2(b, c);

    [MethodImpl(MethodImplOptions.NoInlining)]
    static int Walk2(int b, int c)
    {
        long x = b * 3L, y = c * 5L;
        return b > 0 ? Walk2(b - 1, c) + (int)(x - y) : Walk3(c) + (int)(y - x);
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    static int Walk3(int c)
    {
        Guid g = new Guid(c, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
        return c > 0 ? Walk3(c - 1) + g.GetHashCode() * 0 : s_target.Get();
    }
}

Checked runtime at main 6ff62b1, run with corerun:

Removing step 2 does not stop the assert on windows-x64, because some other transition marks the table first.

Analysis

1. For constructors with at most four arguments, CreateCustomAttributeInstance keeps byrefs to the argument storage (primitive values or object references) in InlineArray4<IntPtr> byrefStorage (RuntimeCustomAttributeData.CoreCLR.cs#L744), and registers them with GCFrameRegistration.RegisterForGCReporting(..., areByRefs: true) (#L760). When the method is interpreted, that local lives on the interpreter's separately allocated stack.
2. UnregisterForGCReporting (#L794) ends in GCFrame::Remove, which in debug builds calls Thread::ObjectRefNew on each registered slot (frames.cpp#L1294-L1298). For the four-argument repro, the four slot addresses go into the thread's dangerousObjRefs table, even though they are interpreter storage rather than C++ OBJECTREF variables.
3. A later applicable Thread::TriggersGC call (threads.h#L3114) marks the recorded slots as having crossed a possible GC point. DisablePreemptiveGC makes one, for example. No actual collection is needed.
4. After CreateCustomAttributeInstance returns, the interpreter reuses that stack memory for later frames. When an interpreted callvirt has its this argument at one of those addresses, INTOP_CALLVIRT evaluates (*pThisArg)->GetMethodTable() (interpexec.cpp#L3322). Thread::IsObjRefValid (threads.h#L3085) finds the address marked as unprotected across a GC point, and the assert fires. Some interpreter stores to a slot go through the checked OBJECTREF assignment, which would clear the mark. But the pointer-sized moves and loads (INTOP_MOV_P/INTOP_LDIND_I are integer copies) do not, so a reused slot keeps the stale entry.

The outgoing this slot is covered by the interpreter's own GC reporting: call arguments take part in its liveness analysis and are reported through the call. IsObjRefValid, on the other hand, consults only the address-keyed table. So I believe this is a bookkeeping false positive. The provenance below supports that, but it does not by itself prove the pointer is fresh.

I confirmed the sequence on windows-x64 with a temporary diagnostic build. It records a stack for each Thread::ObjectRefNew call whose slot address is outside the native stack, and reports that provenance when IsObjRefValid fails at INTOP_CALLVIRT. In the repro and in each System.Runtime.Tests run, the failing address was one of the slots GCFrame::Remove had entered (GCFrame::Remove ← GCReporting::Unregister ← InvokeManagedMethod ← InterpExecMethod). The virtual calls that tripped on them were Program.Walk3 → Base.Get in the repro, and String.CreateStringFromEncoding → Encoding.GetCharCount or DefaultBinder.BindToMethod → MethodBase.GetParametersAsSpan in the test.

In the linux-x64 run I inspected under lldb, the same four entries were created. They stayed unmarked through the virtual calls, and an ObjectRefFlush from PreStubWorker cleared them afterwards. That explains why that run did not assert. I have not found which transition marks them on Windows but not on Linux.

As a diagnostic experiment, I took the ObjectRefNew loop out of GCFrame::Remove and kept the one in GCFrame::Pop. With that change, the repro prints no assert under mode 3 on windows-x64. System.Runtime.Tests discovery also completes, under mode 3 (3 of 3) and mode 2 (1 of 1). At this commit Remove is called only by the managed registration API (GCReporting::Unregister). Ordinary C++ GCPROTECT teardown still goes through Pop, so it keeps its checking. The change does stop tracking accesses to registered storage after it is unregistered, so this is evidence for the diagnosis, not a proposal of a generally safe fix. Another candidate is for the interpreter to read its own reference storage with the raw-pointer idiom (ObjectToOBJECTREF(*(Object**)p)) and rely on its GC reporting. I haven't tried that.