Repository navigation
Fix unprepared delegate target signatures under GCStress - #136864
davidwrighton wants to merge 4 commits into
Conversation
…Stress An R2R caller invoking a JIT-compiled delegate target whose signature contains an unloaded value type asserts in the class loader when a GC stress GC walks the prestub frame. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Eagerly prepare delegate target signatures when publishing a fixed entry point, and have crossgen2 emit a precomputed GCRefMap for delegate constructor fixups so the prestub need not load types on the GC thread. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 3 pipeline(s). 12 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @dotnet/crossgen-contrib |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Delegate fixups can cache stale GC maps for struct layouts that change across versions, causing missed GC roots, while regression tests do not cover the new mapped DelegateCtor path.
2 open issues
What changed in this PR
Addresses a delegate-target signature preparation gap exposed by GCStress after caller-side signature loading was removed. The changes span CoreCLR target preparation, crossgen2 import generation, and regression testing.
Changes:
- Prepare JIT-created delegate targets before publication and reuse precomputed GCRefMaps for eligible ReadyToRun delegate constructors.
- Share GCRefMap signature support and add a dedicated delegate-constructor import section.
- Disable early layout verification under GCStress and add an R2R-caller/JIT-target regression test.
| File | Description |
|---|---|
src/tests/Regressions/coreclr/GitHub_136814/test136814.csproj |
Configures isolation, crossgen2, and a JIT-only target assembly. |
src/tests/Regressions/coreclr/GitHub_136814/test136814.cs |
Adds the delegate target and regression entry point. |
src/tests/Regressions/coreclr/GitHub_136814/delegatecaller.csproj |
Defines the helper library project. |
src/tests/Regressions/coreclr/GitHub_136814/delegatecaller.cs |
Provides the non-inlined R2R delegate caller. |
src/tests/Common/CLRTest.CrossGen.targets |
Skips layout verification when GCStress is enabled. |
src/coreclr/vm/prestub.cpp |
Consumes delegate-constructor GCRefMaps during target preparation. |
src/coreclr/vm/jitinterface.cpp |
Prepares delegate targets before exposing entry points. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunSymbolNodeFactory.cs |
Routes eligible delegate constructors to mapped imports. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs |
Creates and roots the delegate-constructor import section. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/SignatureEmbeddedPointerIndirectionNode.cs |
Uses shared GCRefMap support for signature offsets. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/Signature.cs |
Introduces the shared GCRefMap signature base class. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs |
Moves existing map storage into the shared base class. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ImportSectionNode.cs |
Enables map emission through the shared signature abstraction. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/DelegateCtorSignature.cs |
Defines eligibility and emits delegate-target maps. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/R2RTestSuites.cs |
Allows GCRefMaps on additional executable-pointer import sections. |
Review effort: Balanced
| return !method.Unboxing | ||
| && !target.HasInstantiation | ||
| && !target.OwningType.HasInstantiation |
| bool hasMaps = (section.Flags & ReadyToRunImportSectionFlags.GCRefMap) != 0; | ||
| if (section.Type == ReadyToRunImportSectionType.StubDispatch) | ||
| Assert.True(hasMaps); | ||
| else if ((section.Flags & ReadyToRunImportSectionFlags.PCode) == 0) | ||
| Assert.False(hasMaps); |
| <Optimize>true</Optimize> | ||
| <!-- R2R code in the helper assembly calls a JIT-compiled delegate target. --> | ||
| <AlwaysUseCrossGen2>true</AlwaysUseCrossGen2> | ||
| <CLRTestTargetUnsupported Condition="'$(TargetArchitecture)' == 'wasm'">true</CLRTestTargetUnsupported> |
| <RequiresProcessIsolation>true</RequiresProcessIsolation> | ||
| <CLRTestPriority>1</CLRTestPriority> |
There was a problem hiding this comment.
Can we use the [OuterLoop] attribute instead so we can reduce the number of processes?
| <!-- Keep the delegate target JIT-compiled so it has no precompiled GC ref map. --> | ||
| <CLRTestEnvironmentVariable Include="DOTNET_ReadyToRunExcludeList" Value="$(MSBuildProjectName)" /> |
There was a problem hiding this comment.
Replace this with BypassReadyToRunAttribute on the target?


Fixes a GCStress assertion (
clsload.cpp, type load on the GC thread) hit when a GC occurs inMethodDesc::DoPrestubfor a delegate target whose signature contains value types that were never loaded. #135805 removed caller-side signature loading, and delegate targets published viagetFunctionFixedEntryPointwere not prepared, so the prestub had no GCRefMap and fell back to signature walking.Changes:
getFunctionFixedEntryPoint: callPrepareForUseAsACallTargetOfANativeImage()for delegate targets (covers JIT-constructed delegates).DelegateCtorImportssection (only for non-generic, non-virtual, non-unboxing targets, not Wasm). The runtime'sDynamicHelperFixupuses it, with a debug assert that it matches the computed map.--verify-type-and-field-layoutto crossgen2 when GCStress is enabled, since it can hide early type loads.GitHub_136814(R2R caller + JIT target); it relies on the outerloop GCStress runs and does not force GCStress itself.Validation (Checked CoreCLR, x64 Linux): the test asserts under
GCStress=0xC/JitStress=1/ tiering off without the fix and passes with it; passes normally;crossgen2smokeandcoldsignaturedispatchpass underGCStress=0xC. Not run: NativeLibrary tests, the full R2R suite, Windows or Wasm paths.Resolves #136814
Note
This PR description was AI/Copilot-generated.