Fix NativeAOT RVA field handle scanning - #132439
Conversation
Populate field metadata handles from the field handle nodes that survive final dependency analysis instead of reflection runtime mappings. Keep scanner-only RVA handles from retaining metadata and include owning type metadata for materialized handles. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec90a5ea-59c4-419e-90e5-b6590ef37c11
|
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. |
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib |
There was a problem hiding this comment.
Pull request overview
This PR updates NativeAOT metadata generation so RuntimeFieldHandle-backed fields (including RVA-backed fields) have consistent, dependency-graph-local metadata handle mappings, avoiding KeyNotFoundException during compilation when a field handle is materialized without a reflection runtime mapping.
Changes:
- Track field/type definitions referenced by
RuntimeFieldHandlenodes in the final dependency graph. - Populate the field-handle metadata mapping from the same tracked set that drives metadata emission.
- Extend metadata policies (usage-based and analysis-based) to consider runtime-field-handle-driven metadata requirements.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs | Ensures fields/types referenced via runtime field handles are treated as metadata-generating in the usage-based policy. |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs | Tracks runtime field-handle fields/types during graph marking and uses that set to drive metadata emission and handle lookup. |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/RuntimeFieldHandleNode.cs | Exposes the target field for graph-level tracking. |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs | Exposes the field for generic-dictionary ldtoken field-handle slot tracking. |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/AnalysisBasedMetadataManager.cs | Ensures fields/types referenced via runtime field handles are treated as metadata-generating in the analysis-based policy. |
|
On the phone now, but I think it would be better to do #86373 differently: undo the md manager change from that PR, restore the scanner code, and tighten the rules in the scanner so that it matches intrinsic expansion rules in RyuJIT (the problem with previous code was that it was not tight enough and sometimes declared we're going to intrinsically expand when RyuJIT didn't and that's a crash). Another alternative would be to always emit RuntimeFieldHandle for RVA fields with a zero token (with a comment pointing to the MD manager code that skips dependency analysis). What this PR is doing touches too many components for what is essentially a workaround for the md manager hack added in #86373. The hack stopped being valid after #113413: before that PR in a situation like this we would emit a RuntimeFieldHandle that doesn't resolve; after that PR, we get a crash. |
This reverts commit d069433.
Mirror RyuJIT's InitializeArray and CreateSpan expansion rules in a dedicated IL scanner pattern analyzer. Restore normal reflection metadata dependencies for RVA field handles that are not consumed by expanded intrinsics. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec90a5ea-59c4-419e-90e5-b6590ef37c11
jkoritzinsky
changed the title
|
I've reverted the workaround of the hack and instead implemented the intrinsic expansion to match RyuJIT directly. |
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
NativeAOT's IL scanner historically skipped reflection dependencies for every RVA field token because most are consumed by
RuntimeHelpers.InitializeArrayorCreateSpan<T>. After runtime field handles switched to reflection metadata records, an RVAldtokenthat survives code generation can therefore reachGetMetadataHandleForFieldwithout a corresponding map entry and crash the compiler.Restore normal metadata dependencies for RVA field handles, and suppress them only when a dedicated
RvaIntrinsicPatternAnalyzerproves that RyuJIT will expand the intrinsic. The analyzer follows the same state-machine pattern as the existing type-equality andisinstanalyzers and mirrors RyuJIT's IL shape, type, size, overflow, and RVA-data checks for bothInitializeArrayandCreateSpan<T>.Validation:
.\build.cmd clr.aot -c Debugsrc\tests\build.cmd -Rebuild -Test JIT\Directed\Directed_r.csproj -Test JIT\Directed\Directed_d.csproj -Test JIT\Intrinsics\CreateSpan_il.ilproj x64 Debug -Priority 1 -NativeAOT -SkipNative /p:LibrariesConfiguration=Debugcatch2field handle is retained while validInitializeArrayandCreateSpan<T>RVA blobs are not retained as reflection metadata.Fixes #132266
Note
This pull request description was generated by GitHub Copilot.