This is an archive of the discontinued LLVM Phabricator instance.

[lld-macho] Generate ObjC symbols from .tbd files
ClosedPublic

Authored by int3 on Aug 4 2020, 3:54 PM.

Details

Reviewers
smeenai
Group Reviewers
Restricted Project
Commits
rGa499898e86ec: [lld-macho] Generate ObjC symbols from .tbd files
Summary

I followed similar logic in TapiFile.cpp.

Diff Detail

Event Timeline

int3 created this revision.Aug 4 2020, 3:54 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 4 2020, 3:54 PM
int3 requested review of this revision.Aug 4 2020, 3:54 PM
compnerd added inline comments.
lld/MachO/InputFiles.cpp
406

Does this really need the full context?

433

Same as last time - would be nice to actually compose with the user label prefix.

Remember that x86_64-apple-coff is a real target - and the user label prefix is empty there. This breaks because you are assuming that the user label prefix is _.

lld/test/MachO/Inputs/MacOSX.sdk/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation.tbd
13

Id really rather see a second library added (Foundation.tbd) rather than put this into CoreFoundation. This makes it confusing for others IMO.

lld/test/MachO/stub-link.s
6

I think that we should go through and actually improve all the tests to use -syslibroot. This really looks far better (and matches the real world usage).

int3 added a subscriber: ruiu.Aug 5 2020, 9:29 AM
int3 added inline comments.
lld/MachO/InputFiles.cpp
406

I don't think explicit captures add much to readability, especially for such a small lambda body...

(I don't think avoiding auto does either, that was @ruiu's preference)

433

Remember that x86_64-apple-coff is a real target

oh what... is this actually used?

lld/test/MachO/Inputs/MacOSX.sdk/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation.tbd
13

these members are from CoreFoundation in the actual SDK, though...

lld/test/MachO/stub-link.s
6

agreed, will do it in a separate commit

smeenai added a subscriber: smeenai.Aug 5 2020, 6:37 PM
smeenai added inline comments.
lld/MachO/InputFiles.cpp
422

Huh, interesting. I thought -ObjC only influenced the behavior for static libraries.

433

I believe it's used for UEFI. Presumably that's something for LLD for COFF to worry about and not LLD for Mach-O though, right? Also, I'd be surprised if Objective-C is being used for UEFI :)

compnerd added inline comments.Aug 6 2020, 10:09 AM
lld/MachO/InputFiles.cpp
433

Sorry, that was a partial thought. What I was getting at is that the user label prefix is important and in that case, you have MachO conversions that occur. We should be more careful about the user label prefix rather than assume that it is guaranteed to be _. That is a convention that is target dependent and not MachO dependent.

smeenai added inline comments.Aug 6 2020, 7:12 PM
lld/MachO/InputFiles.cpp
433

I agree with you in theory, but in practice, are there any targets we'll be linking with LLD for Mach-O where you wouldn't expect the underscore? I think we're pretty much assuming that Mach-O == an Apple Mach-O target; we're certainly not testing with anything else.

Out of curiosity, what does ld64 do?

int3 added inline comments.Aug 10 2020, 2:34 PM
lld/MachO/InputFiles.cpp
433

ld64 includes the underscore prefixes in their string literals

smeenai accepted this revision.Aug 12 2020, 5:43 PM

LGTM

lld/MachO/InputFiles.cpp
433

I think it's pretty justifiable to use the underscore directly as well here then.

lld/test/MachO/Inputs/MacOSX.sdk/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation.tbd
13

Huh, TIL.

This revision is now accepted and ready to land.Aug 12 2020, 5:43 PM
This revision was landed with ongoing or failed builds.Aug 12 2020, 7:51 PM
This revision was automatically updated to reflect the committed changes.