port(MachO): Align __LINKEDIT sections#2186
Conversation
davidlattimore
left a comment
There was a problem hiding this comment.
Thanks! Do you think it's practical to add a test that would catch this bug?
|
One possible way in which this might be tested is to look at making linker-diff detect that the alignment of these sections is different from what lld emits. |
|
Thanks for looking this over and for the suggestion! :) I was also wondering what the best way to add a regression test here would be - I briefly looked into assertions but I’ll try supporting this directly with linker-diff then! |
|
Thank you @joanna-jo for the very first contribution! Really appreciate your research part where you investigated what's the expected alignment of the various |
|
This is now the linker-diff output before vs. after our fix on a file with misaligned Before: After: |
Fixes #2063.
According to dyld (alignments, alignment check) and lld (alignments, code signature alignment), the current
__LINKEDITblobs/“sections” we have are expected to have a minimum alignment of:STRTABandCODE_SIGNATUREare already correctly aligned, so the alignments were only changed forCHAINED_FIXUPSandSYMTAB.CHAINED_FIXUPS’ size was also aligned upwards to USIZE sinceOffsetVerifierneeds this “section” to have an aligned size (wild: error: Unexpected memory offsets: Part #36 (section DYLD_CHAINED_FIXUPS_TABLE alignment: 8) has non aligned size: 0x4e). This follows what lld does for paddingCHAINED_FIXUPS.I manually checked with dyld_info and otool using misaligned files, and while this fixes the errors for misaligned
__LINKEDITcontent, there’s another dyld_info error (chained fixups, segment_offset does not match vmaddr from LC_SEGMENT in segment #2) that seems like an off-by-one in how we calculate the index for__DATA_CONSTwhen writing the chained fixup table. I’ve opened a separate issue for that here: #2185.