Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Phabricator shutdown timeline

[ObjectYAML/obj2yaml/yaml2obj][MachO] Support indirect symbol table

Authored by thevinster on Nov 22 2021, 7:58 PM.



Tools such as llvm-objdump or llvm-readobj support indirect symbol
tables. Here, support it for obj2yaml and yaml2obj.

Diff Detail

Event Timeline

thevinster created this revision.Nov 22 2021, 7:58 PM


thevinster retitled this revision from [obj2yaml/yaml2obj/ObjectYAML][MachO] Support indirect symbol table to [ObjectYAML/obj2yaml/yaml2obj][MachO] Support indirect symbol table.Nov 22 2021, 8:03 PM
thevinster edited the summary of this revision. (Show Details)
thevinster published this revision for review.Nov 22 2021, 8:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 22 2021, 8:06 PM
drodriguez added inline comments.Nov 23 2021, 10:26 AM

If I am not wrong, llvm::yaml::Hex32 should have the same size as an uint32_t. In any case, it might be better to use a sizeof(llvm::yaml::Hex32::BaseType) or similar instead of a naked 4 in here.

thevinster marked an inline comment as done.Nov 23 2021, 8:22 PM
thevinster added inline comments.

I like sizeof(llvm::yaml::Hex32::BaseType) since it's clearer.

thevinster marked an inline comment as done.
thevinster edited the summary of this revision. (Show Details)

Update to use sizeof() instead of hard-coded number

drodriguez accepted this revision.Nov 29 2021, 10:26 AM
This revision is now accepted and ready to land.Nov 29 2021, 10:26 AM
jhenderson accepted this revision.Nov 30 2021, 12:12 AM

A few nits, but mostly looks good.


Shouldn't need the explicit llvm prefix, since you're already in the llvm namespace.


UpperCamelCase for variable names.

Also, I'd not use auto here: the type isn't obvious from the RHS (although if you're keen on following existing style in this immediate area, I can live with it).


Is this the minimum this document can be reduced to? There's a lot of stuff that has no value to the test, although I know that Mach-O yaml2obj's side doesn't allow much to be removed to minimise noise.

thevinster marked 4 inline comments as done.Nov 30 2021, 3:03 PM
thevinster added inline comments.

Updated. Most of the LinkEditData could be removed (except the one under test), but the FileHeader and LoadCommands are required in order to not break the continuity of the binary.

thevinster marked an inline comment as done.

Addressing nits

This revision was landed with ongoing or failed builds.Nov 30 2021, 4:15 PM
This revision was automatically updated to reflect the committed changes.