This is an archive of the discontinued LLVM Phabricator instance.

[lld-macho] Accept dylibs with LC_DYLD_EXPORTS_TRIE
ClosedPublic

Authored by BertalanD on Jul 9 2022, 4:00 PM.

Details

Reviewers
thakis
int3
Group Reviewers
Restricted Project
Commits
rG94e0f8e00100: [lld-macho] Accept dylibs with LC_DYLD_EXPORTS_TRIE
Summary

This load command specifies the offset and size of the exports trie.
This information used to be a field in LC_DYLD_INFO, but in newer
libraries, it has a dedicated load command: LC_DYLD_EXPORTS_TRIE.

The format of the trie is the same for both load commands, so the code
for parsing it can be shared.

LLD does not generate this yet; it is mainly useful when chained fixups
are in use, as the other members of LC_DYLD_INFO are unused then, so the
smaller LC_DYLD_EXPORTS_TRIE can be output instead.

LLDB gained support for this in D107673.

Fixes #54550

Diff Detail

Event Timeline

BertalanD created this revision.Jul 9 2022, 4:00 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJul 9 2022, 4:00 PM
BertalanD requested review of this revision.Jul 9 2022, 4:00 PM
BertalanD updated this revision to Diff 443478.Jul 9 2022, 11:47 PM

Removed unused variable

int3 added a subscriber: int3.Jul 10 2022, 7:35 PM

Nice! Could we have a test? We can use yaml2obj to generate the test input.

What should that test look like? Do I just check if lld returns 0, or does the output file need to be checked as well?

It looks like obj2yaml only dumps the exports trie if it sees LC_DYLD_INFO_ONLY. I'm going to submit a patch for that first then...

int3 added a comment.Jul 10 2022, 11:39 PM

Checking that LLD returns zero when looking up a symbol in a dylib using LC_DYLD_EXPORTS_TRIE should be good

BertalanD updated this revision to Diff 444241.EditedJul 13 2022, 6:09 AM

Added a test case (generated using obj2yaml --raw-segment=linkedit as obj2yaml does not automatically dump the trie when it sees LC_DYLD_EXPORTS_TRIE)

thakis accepted this revision.Jul 13 2022, 6:17 AM
thakis added a subscriber: thakis.

Nice!

lld/MachO/InputFiles.cpp
1727–1736

Should we error out if both LC_DYLD_INFO_ONLY and LC_DYLD_EXPORTS_TRIE are present? (Does ld64?)

lld/test/MachO/lc-dyld-exports-trie.yaml
12 ↗(On Diff #444241)

Can you add a comment that explains how you created this file?

This revision is now accepted and ready to land.Jul 13 2022, 6:17 AM
BertalanD added inline comments.Jul 13 2022, 6:32 AM
lld/MachO/InputFiles.cpp
1727–1736

It's not clear to me. The LLDB diff I linked asserts (!) that only one of them can be present; ld64 checks for LC_DYLD_INFO_ONLY first, and only reads the new load command if the former is not specified, while dyld does it the other way around.

Let's add an error then and see if anyone complains.

BertalanD marked an inline comment as done.
int3 accepted this revision.Jul 13 2022, 1:05 PM

lgtm thanks!

lld/test/MachO/lc-dyld-exports-trie.yaml
3 ↗(On Diff #444388)
thakis accepted this revision.Jul 13 2022, 1:27 PM

Lgtm too.

This revision was landed with ongoing or failed builds.Jul 13 2022, 1:34 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 13 2022, 1:34 PM