This is an archive of the discontinued LLVM Phabricator instance.

[lld-macho] De-templatize mach_header operations
ClosedPublic

Authored by int3 on May 2 2021, 11:33 AM.

Details

Reviewers
thakis
Group Reviewers
Restricted Project
Commits
rG001ba65375f7: [lld-macho] De-templatize mach_header operations
Summary

@thakis pointed out that mach_header and mach_header_64
actually have the same set of (used) fields, with the 64-bit version
having extra padding. So we can access the fields we need using the
single mach_header type instead of using templates to switch between
the two.

I also spotted a potential issue where hasObjCSection tries to parse a
file w/o checking if it does indeed match the target arch... As such,
I've added a quick magic number check to ensure we don't access invalid
memory during findCommand().

Addresses PR50180.

Diff Detail

Event Timeline

int3 created this revision.May 2 2021, 11:33 AM
Herald added a project: Restricted Project. Β· View Herald TranscriptMay 2 2021, 11:33 AM
int3 requested review of this revision.May 2 2021, 11:33 AM
Herald added a project: Restricted Project. Β· View Herald TranscriptMay 2 2021, 11:33 AM
Herald added a subscriber: llvm-commits. Β· View Herald Transcript
int3 updated this revision to Diff 342254.May 2 2021, 11:48 AM

remove a cast

int3 updated this revision to Diff 342256.May 2 2021, 12:22 PM
int3 retitled this revision from [lld-macho][nfc] De-templatize mach_header operations to [lld-macho] De-templatize mach_header operations.
int3 edited the summary of this revision. (Show Details)

add magic number check

thakis accepted this revision.May 3 2021, 5:52 AM

Nice, thanks!

lld/MachO/SyntheticSections.cpp
130

We should probably memset(buf + sizeof(mac_header), target->headerSize - sizeof(mac_header), 0) here to make sure that the reserved field is set to some deterministic data here. (Or is the output pre-zeroed? Probably not?)

This revision is now accepted and ready to land.May 3 2021, 5:52 AM
int3 added inline comments.May 3 2021, 10:01 AM
lld/MachO/SyntheticSections.cpp
130

There are several other places in the code that assume the output is pre-zeroed... and in practice that seems to work... though I'm not sure why πŸ€”

This revision was landed with ongoing or failed builds.May 3 2021, 3:31 PM
This revision was automatically updated to reflect the committed changes.