Repository navigation
fix(relinker): drop Windows from errors the macOS target also reports - #10391
adriadam10 merged 1 commit into
Conversation
The macOS output reuses the Windows load image and relocation builder, so a --macos conversion failed with "Unsupported Windows relocation type 2" for PC32, COPY and 32, and with "Windows output requires a position-independent ELF image" for an ET_EXEC input. These errors now read "Relocation type N is not supported for this target" and "This target requires ...", like the builder's other errors that name no target. Which inputs are accepted does not change.
Tested on Apple M1 Pro, macOS 27.0.1 (x86_64 build under Rosetta 2)Applied at
So the new test cases fail without the source change and pass with it. Each test stops at its first failed case, so I did not see the old relinker's message for the
AI-assisted: yes (Claude Code applied the PR, ran the tests and wrote this report). |
harisibrar1
left a comment
harisibrar1
left a comment
There was a problem hiding this comment.
Checked against main c9b7582 and the relinker sources (MacOsMachOPatcher.cpp, WindowsLoadImage.cpp, WindowsRelocationBuilder.cpp, ValidationPolicy.cpp, ElfReader.cpp):
- I confirmed from main's code that
--macosgoes through the sameWindowsLoadImageandWindowsRelocationBuilder(MacOsMachOPatcher.cpp:64,67, with only the diagnostics check before them), so on main the macOS cases do print the "Windows output requires..." and "Unsupported Windows relocation type N" messages. That covers the macOS half the earlier test run couldn't reach. - Small note: R_X86_64_NONE (type 0) also passes
ValidationPolicyand now gets "Relocation type 0 is not supported for this target", along with 2, 5 and 10. - The ELF64 x86-64 check in
WindowsLoadImagelooks unreachable in practice, becauseElfReader::ReadHeaderandGuestImageReaderreject those inputs first, so the reword is fine without a test. - Optional, out of scope: for a non-JUMP_SLOT entry in DT_JMPREL (e.g. type 6), the message says the type is unsupported for the target, though it's fine in DT_RELA. Naming the table might help in a follow-up. Looks good to me.
Reviewed with AI assistance (Claude Code); the checks above were run locally on my machine.
|
Checked that the three messages are the only uses of the old texts, that |
What
The macOS output reuses the Windows load image and relocation builder (
MacOsMachOPatcher.cpp:64,:67), so a--macosconversion failed with errors that name Windows. Relocation types 2 (PC32), 5 (COPY) and 10 (32) pass the relocation type check, but neither the PE nor the Mach-O output applies them, and the conversion failed withUnsupported Windows relocation type 2. AnET_EXECinput failed withWindows output requires a position-independent ELF image. These errors now readRelocation type 2 is not supported for this targetandThis target requires a position-independent ELF image, like the builder's other errors such asExecutable DTPMOD64 relocation is not supported for this target. The ELF64 x86-64 check in the same constructor gets the same wording. Which inputs are accepted or rejected does not change.executable_tls_relocationsnow checks the message for types 2 and 10 on--windowsand--macos, andelf_headerchecks theET_EXECerror on both targets, a case it used to skip.MacOsMachOPatcher.cpp, which feat(relinker): trace function imports on macOS #9408 edits.Tested
Ubuntu 24.04 x86_64, relinker-only Release build (
-DANYPS5_RELINKER_ONLY=ON -DBUILD_TESTING=ON); synthetic fixtures only, no title or homebrew.ctest --test-dir build-relinker: 240/240 passed.FAIL: Relocation type 2 is not supported for this target (offset 0x1000)on--windowsand--macos, and the same for type 10.python3 tools/check_conventions.py --base upstream/mainwith this title and description passed.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)