This is an archive of the discontinued LLVM Phabricator instance.

[lld-macho] If export_size is zero, export_off must be zero
ClosedPublic

Authored by int3 on Oct 26 2021, 6:34 PM.

Details

Summary

Otherwise tools like codesign_allocate will choke. We were already
handling this correctly for the other DYLD_INFO sections.

Doing this correctly is a bit subtle: we don't know if export_size will
be zero until we have run ExportSection::finalizeContents(). However,
we must still add the ExportSection to the __LINKEDIT segment in order
that it gets sorted during sortSectionsAndSegments().

Diff Detail

Event Timeline

int3 created this revision.Oct 26 2021, 6:34 PM
Herald added a project: Restricted Project. · View Herald Transcript
int3 requested review of this revision.Oct 26 2021, 6:34 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 26 2021, 6:34 PM
int3 added inline comments.Oct 26 2021, 6:39 PM
lld/test/MachO/no-unneeded-dyld-info.s
16–17

I think we missed testing this the first time around because back then we couldn't emit anything other than executables, which would always have one exported symbol (i.e. main).

oontvoo accepted this revision.Oct 26 2021, 6:40 PM
oontvoo added a subscriber: oontvoo.

LGTM

optional : need tests for the case export_off is non-zero too?

This revision is now accepted and ready to land.Oct 26 2021, 6:40 PM
int3 planned changes to this revision.Oct 26 2021, 7:15 PM
int3 updated this revision to Diff 382744.Oct 27 2021, 11:56 AM
int3 edited the summary of this revision. (Show Details)

actual fix

This revision is now accepted and ready to land.Oct 27 2021, 11:56 AM
int3 added a comment.Oct 27 2021, 11:57 AM

optional : need tests for the case export_off is non-zero too?

I think that's sufficiently covered via our other tests. Actually I'd forgotten to run them when uploading the diff the first time around :) when I did I saw that my initial fix actually broke most of them... but that's addressed now.

This revision was landed with ongoing or failed builds.Oct 27 2021, 11:59 AM
This revision was automatically updated to reflect the committed changes.