Page MenuHomePhabricator

Upstream support for macCatalyst Mach-O binaries.
ClosedPublic

Authored by aprantl on Thu, Aug 22, 5:19 PM.

Details

Summary

On macOS one Mach-O slice can contain multiple load commands: One load command for being loaded into a macOS process and one load command for being loaded into a macCatalyst process. This patch adds support for the new load command and makes sure ObjectFileMachO returns the Architecture that matches the Module.

Diff Detail

Repository
rL LLVM

Event Timeline

aprantl created this revision.Thu, Aug 22, 5:19 PM
friss added inline comments.Fri, Aug 23, 10:46 AM
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
1111 ↗(On Diff #216744)

'required' here sounds weird as we're testing against multiple ones. "Matching" maybe?

1126 ↗(On Diff #216744)

"all only"?

1128 ↗(On Diff #216744)

Is MapFileData guaranteed to succeed? (ie return a valid data_sp)

4976 ↗(On Diff #216744)

can this be a const ref to make it clear it's not an output parameter?

4977 ↗(On Diff #216744)

Can ModuleSpecList be moved efficiently? If so, could we return it with NRVO rather than using an output param?

5060 ↗(On Diff #216744)

Can we merge this loop with the above one? Are there binaries with both LC_VERSION_MIN and LC_BUILD_VERSION? If so merging would give different results.

5072–5073 ↗(On Diff #216744)

Does struct build_version_command have exactly the same size on every platform? Even if that's true, it seems like it would be more principled to extract each field in turn.

friss accepted this revision.Fri, Aug 23, 10:51 AM

This generally looks good. Feel free to address the stylistic bits you find relevant in follow-up patches.

This revision is now accepted and ready to land.Fri, Aug 23, 10:51 AM
friss added inline comments.Fri, Aug 23, 10:53 AM
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
4977 ↗(On Diff #216744)

Sorry, NRVO has nothing to do with move semantics. Move semantics could be another way to return it cheaply though...

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFri, Aug 23, 2:32 PM