This is an archive of the discontinued LLVM Phabricator instance.

[MachO] Support exports trie in both LC_DYLD_INFO and LC_DYLD_EXPORTS_TRIE
ClosedPublic

Authored by drodriguez on Sep 23 2022, 3:56 PM.

Details

Summary

The exports trie used to be pointed by the information in LC_DYLD_INFO,
but when chained fixups are present, the exports trie is pointed by
LC_DYLD_EXPORTS_TRIE instead.

Modify the Object library to give access to the information pointed by
each of the load commands, and to fallback from one into the other when
the exports are requested.

Modify ObjectYAML to support dumping the export trie when pointed by
LC_DYLD_EXPORTS_TRIE and to parse the existence of a export trie also
when the load command is present.

This is a split of D134250 with improvements on top.

Diff Detail

Event Timeline

drodriguez created this revision.Sep 23 2022, 3:56 PM
Herald added a reviewer: MaskRay. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
drodriguez requested review of this revision.Sep 23 2022, 3:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 23 2022, 3:56 PM

Can someone have a look at this one? Thanks!

beanz added inline comments.Oct 13 2022, 7:28 PM
llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp
255 ↗(On Diff #462600)

Hypothetically, without this assert does the code otherwise function to generate the export trie twice?

The reason I'm asking, is that one of the original goals for the mach-o yaml tooling was to be able to construct invalid object files for testing purposes.

drodriguez added inline comments.Oct 14 2022, 10:02 AM
llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp
255 ↗(On Diff #462600)

I think without this assert the export trie information will end up in two places in the binary.

If later a tool like MachOReader from ObjCopy, MachOObjectFile of Object, or macho2yaml tries to read that information, they all prefer the one in the DYLD_INFO vs. the one in EXPORTS_TRIE.

If I understood things correctly what determines whether one or the other exist should be the presence of chained fixups. With chained fixups, the DYLD_INFO does not need to be present, but then you need a place to put the exports trie, so the extra LC is used. Since the chained fixup API is marked as non-final and not to be relied on, I did not want to depend on it too heavily, so I used the presence of the EXPORTS_TRIE as a proxy.

I can remove the assert, but I think it is a nice help that something is being fishy.

I am going to try to rewrite the diffs on top of this one to avoid the dependency, but if someone has some opinion about how to do this properly or what can I improve to make it better, I will appreciate it.

@drodriguez - if it's not too hard i'd probably split this diff into two. 1. changes to ObjectYAML (it would be good to have a test that invokes obj2yaml and yaml2obj) 2. changes to ObjCopy

I will try to split them. Not breaking anything with two separate changes might be complicated.

Isolate ObjectYAML and libObject changes from ObjCopy changes. Add test for ObjectYAML changes.

drodriguez edited the summary of this revision. (Show Details)Nov 11 2022, 5:13 PM

LG (with minor nit: for consistency with other tests it'd be good to rename export_trie-lc_dyld_info_only.yaml -> export_trie_lc_dyld_info_only.yaml, export_trie-lc_dyld_exports_trie.yaml -> export_trie_lc_dyld_exports_trie.yaml)

This revision is now accepted and ready to land.Nov 21 2022, 12:17 AM

Rename test cases

llvm/test/ObjectYAML/MachO/export_trie-lc_dyld_exports_trie.yaml