Page MenuHomePhabricator

[dsymutil] Add support for linking remarks
ClosedPublic

Authored by thegameg on Oct 17 2019, 2:33 PM.

Details

Summary

This adds support to dsymutil for linking remark files and placing them in the final .dSYM bundle.

The result will be placed in:

  • a.out.dSYM/Contents/Resources/Remarks/a.out

or

  • a.out.dSYM/Contents/Resources/Remarks/a.out-<arch> for universal binaries

When multi-threaded, this runs a third thread which loops over all the object files and parses remarks as it finds __remarks sections.

Testing this involves running dsymutil on pre-built binaries and object files, then running llvm-bcanalyzer on the final result to check for remarks.

Diff Detail

Event Timeline

thegameg created this revision.Oct 17 2019, 2:33 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 17 2019, 2:33 PM
Herald added a subscriber: mgorny. · View Herald Transcript
JDevlieghere added inline comments.Thu, Oct 31, 9:24 AM
llvm/tools/dsymutil/DwarfLinker.cpp
2555

I'm not sure if this comment is more helpful than saying "create the Remarks directory in the Resources directory", and below saying "append the file name, possibly followed by a dash and its architecture in case we're dealing with a fat file", or something like that :-)

2564

Maybe say that this is for fat binaries.

2566

This looks like you're iterating over something. If you go this route I think one is fine.

llvm/tools/dsymutil/LinkUtils.h
72

Doxygen supports "grouping" with @{ and @}. Can we create a group and specify that this is used for the remarks?

llvm/tools/dsymutil/Options.td
148

Please update cmdline.test with these new options.

thegameg updated this revision to Diff 227357.Thu, Oct 31, 2:51 PM
thegameg marked 5 inline comments as done.

Address comments.

JDevlieghere accepted this revision.Fri, Nov 1, 11:17 AM
This revision is now accepted and ready to land.Fri, Nov 1, 11:17 AM
This revision was automatically updated to reflect the committed changes.

Tests are still broken on non-mac even after 86cdf74dc8. Maybe the space between REQUIRES and : in that change confuses lit. If you can't fix soon, please revert; the tree's been broken for several hours by now.