This is an archive of the discontinued LLVM Phabricator instance.

[mac/lld] Add support for the LC_LINKER_OPTION load command in o files
ClosedPublic

Authored by thakis on Dec 3 2020, 5:43 PM.

Details

Summary

clang puts -framework CoreFoundation in this load command for files
that use @available / __builtin_available. Without support for this,
binaries that don't explicitly link to CoreFoundation fail to link.

Diff Detail

Event Timeline

thakis requested review of this revision.Dec 3 2020, 5:43 PM
thakis created this revision.
thakis updated this revision to Diff 309422.Dec 3 2020, 5:46 PM

rename parseLoadCommand -> parseLCLinkerOption

int3 added a subscriber: int3.Dec 3 2020, 9:37 PM
int3 added inline comments.
lld/MachO/Driver.cpp
355

would be good to have a short comment about what LC_LINKER_OPTION contains

370–371

is this necessary given that we are going to error out anyway at line 382 below?

lld/test/MachO/lc-linker-option.ll
21

nit: the other tests generally include the "error: " prefix

thakis updated this revision to Diff 309511.Dec 4 2020, 4:30 AM
thakis marked 2 inline comments as done.

comments

thakis added a comment.Dec 4 2020, 4:30 AM

Thanks!

lld/MachO/Driver.cpp
370–371

I guess it's not strictly necessary. It matches the flow in parseDirectives() in lld/COFF. The reasoning is probably that this first stage gives you a list of generally valid args, and the 2nd pass below checks which of them are valid in directives / load commands.

int3 accepted this revision.Dec 4 2020, 5:37 AM

lgtm

lld/MachO/Driver.cpp
370–371

I see. How about dropping the "ignoring" and just having "unknown argument:" then? "Ignoring" suggests that we are not erroring out... it looks like parseDirectives in the ELF port issues a warning, and {ELF,MachO}OptTable::parse just errors with "unknown argument".

As an aside... it's a bit unfortunate that "unknown argument" refers to the CLI flag, while "missing argument" refers to the argument to a flag... but this overload of "argument" seems to happen in many places in the LLVM codebase already so ¯\_(ツ)_/¯

This revision is now accepted and ready to land.Dec 4 2020, 5:37 AM
thakis marked an inline comment as done.Dec 4 2020, 5:45 AM
thakis added inline comments.
lld/MachO/Driver.cpp
370–371

Sounds good :)

This revision was automatically updated to reflect the committed changes.
thakis marked an inline comment as done.
Herald added a project: Restricted Project. · View Herald TranscriptDec 4 2020, 5:50 AM