Implement a workaround for ld-prime hitting an assert on large addends#124721
Implement a workaround for ld-prime hitting an assert on large addends#124721filipnavara wants to merge 5 commits into
Conversation
|
Needs more work. I'll investigate the unit test failures first. |
|
I don't get the System.Linq.Expressions test failure on Xcode 26.2 anymore. According the log the compiler/linker version on CI is |
|
Going back to the drawing board now. |
|
@akoeplinger Is there some easy way to switch some CI lane to use newer Xcode? They should be available in the runner images, just not as default. |
|
I assume adding xcode-select to the path mentioned in https://github.com/actions/runner-images/blob/main/images/macos/macos-15-Readme.md#xcode somewhere early in the build should suffice? |
Thanks, seems to pass the smoke test with Xcode 26.2. I'll probably open a separate PR to check what is the behavior just with the Xcode bump alone. |
|
Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it. |
There was a problem hiding this comment.
Pull request overview
This PR adds a workaround in the Mach-O object writer to avoid Apple ld-prime assertions when emitting IMAGE_REL_BASED_RELPTR32 as *_RELOC_SUBTRACTOR + *_RELOC_UNSIGNED with large addends, by introducing reusable per-section temporary labels to keep addends within the linker’s expected signed 20-bit range.
Changes:
- Track and emit per-section temporary labels to bound relocation addends for
IMAGE_REL_BASED_RELPTR32on ARM64 and x64.eh_frame. - Adjust Mach relocation emission to optionally reference a temporary label symbol (instead of the section base symbol) for SUBTRACTOR relocs.
- Modify
build.shto attempt switching Xcode versions on macOS.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/coreclr/tools/Common/Compiler/ObjectWriter/MachObjectWriter.cs | Adds temporary-label generation/reuse and uses label symbol indices to avoid ld-prime large-addend assertions. |
| build.sh | Adds macOS-only logic to switch Xcode via sudo xcode-select. |
|
The version from @agocke resulted in corrupted |
|
So, the approach with labels at fixed offset (as suggested by @agocke) fails with Both of the anchors are marked with |
|
Couple more observations:
|
|
I have tried to attack the "relocation anchor grid" approach pretty much from all possible angles, but I think it's impossible to execute due to a combination of ld-classic bugs.
Given these constraints any artificially added label inside code section is a non-starter. That said, the current state of the PR is not fully correct either. It generates temporary labels too. While it does so in different fashion it works only as a coincidence of the fact that we don't use IMAGE_REL_BASED_RELPTR32 inside code sections. I guess the only reasonable course of action is to locate the nearest label below the address of the relocation and write the relocation relative to it. We can emit temporary labels in non-code sections if necessary. |
We translate the IMAGE_REL_BASED_RELPTR32 relocation into ARM64_RELOC_SUBTRACTOR and ARM64_RELOC_UNSIGNED pair on ARM64. To emulate the behavior of PC relative relocation we bake the section-relative PC offset into the addend with negative sign. The ARM64_RELOC_SUBTRACTOR relocation then subtract the base address of the section and finally the ARM64_RELOC_UNSIGNED relocation adds the target symbol address. This works fine for addends that fit into signed 20-bit integer, but ld-prime hits an assert when the addend is larger. To workaround it we create anchor labels at fixed 2^19 byte offsets in the section and adjust the addend to be relative to the nearest anchor label. This way we can guarantee the addend for ARM64_RELOC_SUBTRACTOR is always within signed 20-bit range. Same logic applies to X86_64_RELOC_SUBTRACTOR + X86_64_RELOC_UNSIGNED pair for x64 targets. Co-authored-by: Andy Gocke <andy@commentout.net> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ec0dcbe to
fb2b935
Compare
| // We cannot emit anchor symbols in executable sections as they may split | ||
| // existing functions into multiple atoms, and ld-classic corrupts the | ||
| // unwinding information for such functions. In theory we could support this | ||
| // by using nearest preceeding symbol as an anchor but that requires more |
| // unwinding information for such functions. In theory we could support this | ||
| // by using nearest preceeding symbol as an anchor but that requires more | ||
| // complex handling and we don't have any such relocs in our current scenarios. | ||
| throw new NotSupportedException("Executable sections cannot contain RELPTR32 relocations on Mach-O"); |
There was a problem hiding this comment.
This is intentionally broad. It could even be an "assert" if we decide that's enough.
There was a problem hiding this comment.
Assert would be fine, we don't do textual exceptions in the compiler ourside the object writer. They introduce localization burden.
|
@MichalStrehovsky @agocke I iterated on it a bit. This is now a reduced version based on Andy's idea of generating anchor labels at fixed distances. Unlike the previous iteration we only do that for sections where the problematic relocations are actually used. In addition I added a check that would throw an exception if someone tried to use the relocation in a code section. It's easier to trace back the exception to a comment, run "git blame" and find this PR than to stare at randomly corrupted linker output. Retested with ld-classic (Xcode 16.4), ld-prime (Xcode 26.2) and ld-prime (Xcode 26.4). |
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
The last Azure Pipelines check for osx-arm64 died (timeout?) without reporting it back to GitHub. Rest of failures seems to be the same across all platforms. |
Yeah, #129123 reverted the break. I'll just retrigger. |
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
The CI for outerloop is still pinned on Xcode 16.4 and ld-classic. The failure in System.Text.Json.SourceGeneration.Roslyn4.4.Tests is thus unrelated to the fix in this PR. I downloaded the Helix payload and double checked that ld64 955.13 was used for the linking. We will need to do separate analysis to see why the unwind information seems to be corrupted there. |
This reverts commit ae75bcf.
| long labelOffset = offset & ~(RelocAnchorGranularity - 1); | ||
| long stored = addend - (offset - labelOffset); | ||
| Debug.Assert(stored >= -(1L << RelocAnchorLog2Granularity) && stored < (1L << RelocAnchorLog2Granularity)); | ||
| BinaryPrimitives.WriteInt32LittleEndian( | ||
| data, | ||
| BinaryPrimitives.ReadInt32LittleEndian(data) + | ||
| (int)(addend - offset)); | ||
| (int)stored); |
We translate the IMAGE_REL_BASED_RELPTR32 relocation into ARM64_RELOC_SUBTRACTOR and ARM64_RELOC_UNSIGNED pair on ARM64. To emulate the behavior of PC relative relocation we bake the section-relative PC offset into the addend with negative sign. The ARM64_RELOC_SUBTRACTOR relocation then subtract the base address of the section and finally the ARM64_RELOC_UNSIGNED relocation adds the target symbol address.
This works fine for addends that fit into signed 20-bit integer, but ld-prime hits an assert when the addend is larger. To workaround it we create anchor labels at fixed 2^19 byte offsets in the section and adjust the addend to be relative to the nearest anchor label. This way we can guarantee the addend for ARM64_RELOC_SUBTRACTOR is
always within signed 20-bit range.
Same logic applies to X86_64_RELOC_SUBTRACTOR + X86_64_RELOC_UNSIGNED pair for x64 targets.
Fixes #119380