Cleanup GC mode switching in the interpreter

[janvorli]

The interpreter uses GCX_COOP_NO_DTOR / GCX_PREEMP_NO_DTOR when calling compiled methods with SEH wrapper / unmanaged methods. Due to that, it needed to have forceful restoration of cooperative mode in the catch for ResumeAfterCatchException.
This change switches those usages to GCX_COOP() / GCX_PREEMP() instead. That removes the need to switch the GC mode explicitly in that catch. So I've replaced it by assert.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @BrzVlad, @janvorli
See info in area-owners.md if you want to be subscribed.

[janvorli]

/azp run runtime-interpreter

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

This PR refactors GC mode switching in the CoreCLR interpreter’s SEH/unmanaged-call paths by replacing GCX_COOP_NO_DTOR / GCX_PREEMP_NO_DTOR usage with RAII-based GCX_COOP() / GCX_PREEMP(), and correspondingly simplifying the ResumeAfterCatchException catch path.

Changes:

• Introduces a helper to rethrow the thread’s last thrown managed exception from SEH handlers.
• Routes unmanaged transitions through a wrapper that uses GCX_PREEMP() instead of _NO_DTOR variants.
• Replaces explicit cooperative restoration in the ResumeAfterCatchException catch with an assertion.

Suppressed comments (1)

src/coreclr/vm/interpexec.cpp:483

• InvokeUnmanagedMethod now creates a GCX_PREEMP holder (unwinding object) but is invoked from inside a PAL_TRY (__try on Windows). If this function gets inlined into that PAL_TRY frame under optimization/LTCG, it can reintroduce the VC++ restriction on mixing __try with objects that require unwinding. Mark this helper NOINLINE to keep the GC-mode RAII out of the PAL_TRY function body.

void InvokeUnmanagedMethod(MethodDesc *targetMethod, int8_t *pArgs, int8_t *pRet, PCODE callTarget)
{
    WRAPPER_NO_CONTRACT;

    GCX_PREEMP();
    InvokeManagedMethod(targetMethod, pArgs, pRet, callTarget, NULL);
}

[janvorli]

/azp run runtime-interpreter

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/coreclr/vm/interpexec.cpp:490

• InvokeUnmanagedMethodInPreemptiveMode is invoked from within a PAL_TRY body (which expands to __try in src/coreclr/inc/palclr.h). If this helper gets inlined under optimization, the GCX_PREEMP() RAII object can end up inside the __try region, which is a pattern the SEH-based PAL_TRY macros are designed to avoid. Mark this helper NOINLINE to make the separation robust across configurations/compilers.

void InvokeUnmanagedMethodInPreemptiveMode(MethodDesc *targetMethod, int8_t *pArgs, int8_t *pRet, PCODE callTarget)
{
    WRAPPER_NO_CONTRACT;

    GCX_PREEMP();

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/coreclr/vm/interpexec.cpp:242

• RethrowLastThrownObject() uses GCX_COOP() (RAII) before calling DispatchManagedException(). CallWithSEHWrapper() is invoked under an outer GCX_PREEMP() (e.g., PrepareInterpreterCode), so if this path is entered while preemptive and the exception propagates via C++ unwinding (the PAL_TRY implementation on non-Windows uses try/catch), the GCCoop destructor can restore preemptive mode while the managed exception is being dispatched. This risks reintroducing the GC-mode mismatch the change is trying to eliminate. Use GCX_COOP_NO_DTOR() here since the helper is DECLSPEC_NORETURN and should not restore the incoming mode.

NOINLINE static void DECLSPEC_NORETURN RethrowLastThrownObject()
{
    WRAPPER_NO_CONTRACT;

    GCX_COOP();
    OBJECTREF ohThrowable = GetThread()->LastThrownObject();
    DispatchManagedException(ohThrowable);

[janvorli]

/azp run runtime-interpreter

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[janvorli]

Looks like there is a case when the new assert fires (the GC mode is not restored correctly). I need to investigate it.

[janvorli]

@davidwrighton the issue I am hitting with this change in the JIT/Interpreter test with tiered compilation off in the CI for this change is exactly what you were looking into. In this case, it is a QCALL to IsInstanceOf_NoCacheLookup that ends up throwing. It happens on Windows x64 too.

[davidwrighton]

@janvorli, Great! Now I've got an easier to debug repro case.