[NativeAOT] Warn if a post-ILC postprocessing step modifies a managed assembly#26137
[NativeAOT] Warn if a post-ILC postprocessing step modifies a managed assembly#26137rolfbjarne wants to merge 4 commits into
Conversation
… assembly When the assembly postprocessing pipeline runs after ILC has compiled the assemblies (the trimmable-static + NativeAOT + PrepareAssemblies reorder), the managed assemblies are frozen: ILC already consumed them, so any IL modification at that point is silently lost. This adds a guard that emits a warning if any step modifies an assembly in that scenario, so such useless modifications are surfaced instead of going unnoticed. To avoid spurious warnings, two "modifications" in the post-ILC pass are fixed: * ManagedRegistrarStep.ProcessType unconditionally reported the assembly as modified. During post-processing it only *collects* information about trampolines already created in the PrepareAssemblies pass (a read-only operation), so it now reports the actual modification state and doesn't trigger a redundant save. * The NSObject.RegisterToggleRef stub (which only exists to let ILC trim the method) was still being applied post-ILC, uselessly modifying the frozen product assembly. It's already applied during the PrepareAssemblies pass before ILC runs, so it's now skipped once the assemblies are frozen. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2b6450f9-2ad9-4e89-823a-3cc6e2f33f53
There was a problem hiding this comment.
Pull request overview
This PR adds a guard for NativeAOT builds to warn when any post-ILC assembly postprocessing step modifies managed assemblies (since those IL changes are no longer consumed by ILC), and adjusts two known sources of spurious “modifications” to avoid false positives.
Changes:
- Emit a warning in the assembly-preparer when any assemblies are modified during NativeAOT post-processing (post-ILC scenario).
- Ensure
ManagedRegistrarStep.ProcessTypereports modification state more accurately for the post-processing pass. - Skip applying the
NSObject.RegisterToggleRefstub during post-processing (it’s only useful pre-ILC).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tools/dotnet-linker/Steps/ManagedRegistrarStep.cs | Avoid post-processing IL mutations and refine “modified” reporting for registrar-generated trampolines. |
| tools/assembly-preparer/AssemblyPreparer.cs | Add a NativeAOT post-processing warning when any managed assemblies are modified after ILC. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
… path The 'modified' flag in ManagedRegistrarStep.ProcessType was only set by CreateUnmanagedCallersMethod, but the factory-method code path also mutates the assembly (adding _Xamarin_ConstructNSObject/_Xamarin_ConstructINativeObject methods and interface implementations) without flipping 'modified'. For non-trimmed (Copy) assemblies this meant the assembly action was never switched from Copy to Save, so those IL changes could be silently dropped. Make ImplementConstructNSObjectFactoryMethod and ImplementConstructINativeObjectFactoryMethod return whether they modified the assembly, and propagate that into 'modified' in ProcessType. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
🔥 [CI Build #f2f51e5] Test results 🔥Test results❌ Tests failed on VSTS: test results 2 tests crashed, 1 tests failed, 186 tests passed. Failures❌ dotnettests tests (tvOS)🔥 Failed catastrophically on VSTS: test results - dotnettests_tvos (no summary found). Html Report (VSDrops) Download ❌ linker tests (iOS)🔥 Failed catastrophically on VSTS: test results - linker_ios (no summary found). Html Report (VSDrops) Download ❌ Tests on macOS Tahoe (26) tests1 tests failed, 4 tests passed.Failed tests
Html Report (VSDrops) Download Successes✅ assembly-processing: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
When the assembly postprocessing pipeline runs after ILC has compiled the
assemblies (the trimmable-static + NativeAOT + PrepareAssemblies reorder), the
managed assemblies are frozen: ILC already consumed them, so any IL modification
at that point is silently lost. This adds a guard that emits a warning if any step
modifies an assembly in that scenario, so such useless modifications are surfaced
instead of going unnoticed.
To avoid spurious warnings, two "modifications" in the post-ILC pass are fixed:
ManagedRegistrarStep.ProcessType unconditionally reported the assembly as
modified. During post-processing it only collects information about trampolines
already created in the PrepareAssemblies pass (a read-only operation), so it now
reports the actual modification state and doesn't trigger a redundant save.
The NSObject.RegisterToggleRef stub (which only exists to let ILC trim the
method) was still being applied post-ILC, uselessly modifying the frozen product
assembly. It's already applied during the PrepareAssemblies pass before ILC runs,
so it's now skipped once the assemblies are frozen.
Copilot-Session: 2b6450f9-2ad9-4e89-823a-3cc6e2f33f53