[dotnet-linker] Emit the macro/short form of IL instructions in generated method bodies. - #26471
[dotnet-linker] Emit the macro/short form of IL instructions in generated method bodies.#26471rolfbjarne wants to merge 3 commits into
Conversation
…ated method bodies.
The trimmable static registrar (and the Dlfcn inlining step) generate method
bodies using the long form of the ldloc/stloc/ldarg/ldloca instructions.
This trips a bug in the CoreCLR interpreter, where the store/load peephole
optimization reads the operand of the long form of ldloc/stloc at the wrong
offset (ip + 1 instead of ip + 2). The resulting bogus local index is used to
index an array, which leads to an out-of-bounds read and a SIGBUS at runtime:
make clean build run-bare -C tests/monotouch-test/dotnet/MacCatalyst TEST_VARIATION=trimmable-static-registrar
crashed in MonoTouchFixtures.ObjCRuntime.RegistrarTest.TestConstrainedGenericType.
The interpreter bug has been fixed in dotnet/runtime#131547,
but we still need to work around it until we get a runtime with that fix.
Emitting the macro/short form of the instructions avoids the problematic
instruction sequence entirely, and it also makes the generated IL smaller,
which is a good thing in itself.
For monotouch-test on macOS, this removes all 3709 adjacent long-form
'stloc; ldloc' pairs from the generated _monotouchtest.TypeMap.dll.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a002000e-438e-48ff-9c22-c716d1a2e7aa
There was a problem hiding this comment.
Pull request overview
This PR updates the dotnet-linker’s IL generation to emit macro/short-form IL instructions (where possible) and introduces a reusable “finalize generated body” helper. This serves as a workaround for a CoreCLR interpreter bug involving long-form ldloc/stloc operands, and also reduces generated IL size across registrar- and Dlfcn-related rewriting.
Changes:
- Introduces
OptimizeGeneratedBody/FinalizeGeneratedBodyhelpers (optimize to macro/short forms + assign unique instruction offsets for linker/trimmer usage). - Switches generated registrar/lookup-table bodies from
GenerateILOffsets()toFinalizeGeneratedBody(). - Applies macro/short-form optimization to IL generated/replaced in
InlineDlfcnMethodsStep.
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 |
|---|---|
| tools/dotnet-linker/Steps/ManagedRegistrarStep.cs | Finalizes generated registrar method bodies using the new helper (optimization + linker-friendly offsets). |
| tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs | Finalizes generated lookup-table method bodies using the new helper. |
| tools/dotnet-linker/Steps/InlineDlfcnMethodsStep.cs | Adds optimization for generated/replaced IL in Dlfcn inlining paths. |
| tools/dotnet-linker/CecilExtensions.cs | Adds macro/short-form optimization helper and a finalization helper that also assigns unique instruction offsets. |
| tools/dotnet-linker/AppBundleRewriter.cs | Updates generated factory methods to use the new finalization helper. |
Suppressed comments (1)
tools/dotnet-linker/Steps/InlineDlfcnMethodsStep.cs:923
- ❌ error: After clearing/rebuilding
method.Body.Instructions, the newly created instructions will have default/duplicate offsets, andOptimizeGeneratedBody()won’t fix that. Because this step is configured to run beforeMarkStep, ensure unique instruction offsets by callingFinalizeGeneratedBody()(which also applies the macro/short-form optimization).
// See the comment on CecilExtensions.OptimizeGeneratedBody for why this is needed.
method.Body.OptimizeGeneratedBody ();
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| // See the comment on CecilExtensions.OptimizeGeneratedBody for why this is needed. | ||
| body.OptimizeGeneratedBody (); | ||
|
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Address review feedback: InlineDlfcnMethodsStep runs before MarkStep (see the _TrimmerCustomSteps item in Xamarin.Shared.Sdk.targets), so the generated method bodies must also get the unique instruction offsets the trimmer relies on, not just the macro/short form optimization. Since all the call sites now use FinalizeGeneratedBody, the OptimizeGeneratedBody helper is no longer needed, so remove it again and move the comment back into FinalizeGeneratedBody. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a002000e-438e-48ff-9c22-c716d1a2e7aa
✅ 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 |
🔥 [CI Build #27d260d] Test results 🔥Test results❌ Tests failed on VSTS: test results 1 tests crashed, 0 tests failed, 200 tests passed. Failures❌ Tests on macOS Ventura (13) tests🔥 Failed catastrophically on VSTS: test results - mac_ventura (no summary found). 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 |
The trimmable static registrar (and the Dlfcn inlining step) generate method
bodies using the long form of the ldloc/stloc/ldarg/ldloca instructions.
This trips a bug in the CoreCLR interpreter, where the store/load peephole
optimization reads the operand of the long form of ldloc/stloc at the wrong
offset (ip + 1 instead of ip + 2). The resulting bogus local index is used to
index an array, which leads to an out-of-bounds read and a SIGBUS at runtime:
crashed in MonoTouchFixtures.ObjCRuntime.RegistrarTest.TestConstrainedGenericType.
The interpreter bug has been fixed in dotnet/runtime#131547,
but we still need to work around it until we get a runtime with that fix.
Emitting the macro/short form of the instructions avoids the problematic
instruction sequence entirely, and it also makes the generated IL smaller,
which is a good thing in itself.
For monotouch-test on macOS, this removes all 3709 adjacent long-form
'stloc; ldloc' pairs from the generated _monotouchtest.TypeMap.dll.
Copilot-Session: a002000e-438e-48ff-9c22-c716d1a2e7aa