macho: big-ish refactor and report errors to the user using Zig's API #17020
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Write thunks separately from other atoms - this can still be improved by not using atoms at all, but one thing at a time.
Eventually, we will validate DWARF info upfront and report errors to the user but this will require a rewrite of several parts of the linker so leaving as a TODO for the near future.
andrewrk
approved these changes
Aug 30, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In summary, this PR implements some long-overdue updates. It simplifies input file parsing - we now do a pre-check read of the input object header to work out what we are dealing with, and then parse in full. This greatly simplified the parsing decision logic, although there is still lots of room for improvement!
State and logic that should be unified between drivers is now unified. This includes input file parsing and symbol resolution which do not differ almost at all between incremental and
zldmode, but excludes things like section handling where in incremental mode we preallocate them upfront and inzldmode we don't bother with size calculation until after we resolve symbols, dead strip, etc.The
zldmode now also usesTableSectionabstraction to track synthetics such as GOT cells, TLV pointers, stub routines, etc. This has cleaned up committingzldlinker state to file significantly since we can now handle writing of the synthetics separately.Logic responsible for committing
__LINKEDITdata has been unified since it is also shared between drivers in 100% - this includes things likedyldopcodes, export trie, symtabs, etc.In order to make error handling practical, I have removed
Zldstruct, pruned it and merged what was left withMachOstruct. This way, we also refer to an instance ofMachOas context, and we can use across drivers to report user errors with as little hassle as possible.And yes,
MachOnow reports simplistic linker errors to the user via the Zig's error API. Here's a few screenshots of what it now looks like:While working on the error handling, I thought prudent to teach the linker how to parse
LC_BUILD_VERSIONandLC_VERSION_MIN_*load commands so that we can flag any target mismatch to the user. Oh, and I have ported logic for working out which load command to emit based on the min OS version from Apple'sld64. Perhaps won't be of great use to us just yet, but opens the door, however slightly, for older macOS versions support.