Page MenuHomePhabricator

llvm-objdump: add --rpaths to macho support
ClosedPublic

Authored by keith on Apr 16 2021, 1:22 PM.

Details

Summary

This prints the rpaths for the given binary

Diff Detail

Event Timeline

keith created this revision.Apr 16 2021, 1:22 PM
keith requested review of this revision.Apr 16 2021, 1:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 16 2021, 1:22 PM
keith updated this revision to Diff 338212.Apr 16 2021, 1:23 PM

Remove blank line

keith updated this revision to Diff 338213.Apr 16 2021, 1:27 PM

Add --rpaths to docs

kastiglione accepted this revision.Apr 16 2021, 4:36 PM
kastiglione added a subscriber: kastiglione.

other than the clang-tidy note about the capitalization of rpath, lgtm

This revision is now accepted and ready to land.Apr 16 2021, 4:36 PM
keith updated this revision to Diff 338265.Apr 16 2021, 6:27 PM

Fix clang-tidy warnings

jhenderson added inline comments.Apr 19 2021, 6:47 AM
llvm/test/tools/llvm-objdump/MachO/rpaths.test
2

A quick summary comment at the start of the test describing the test purpose is usually a good idea, I think.

keith updated this revision to Diff 338566.Apr 19 2021, 11:13 AM

Add comments to tests

keith marked an inline comment as done.Apr 19 2021, 11:13 AM
keith updated this revision to Diff 338576.Apr 19 2021, 11:24 AM

Add no --macho test

jhenderson added inline comments.Apr 20 2021, 1:31 AM
llvm/test/tools/llvm-objdump/MachO/rpaths.test
2

Nit: End comments with a '.'. Also, newer tests in the LLVM binutils tend to use '##' for comments, to help them stand out from lit and FileCheck directives.

Same applies to the other two comments below.

12

You should capture and use FileCheck to check what error message is reported here.

keith updated this revision to Diff 338877.Apr 20 2021, 8:18 AM
keith marked 2 inline comments as done.

Update test formatting

MaskRay accepted this revision.Apr 20 2021, 2:20 PM
MaskRay added inline comments.
llvm/test/tools/llvm-objdump/MachO/rpaths.test
6

If you want to ensure there is no other string, you can use FileCheck %s --match-full-lines --strict-whitespace or add {{^}}...{{$}}

keith added inline comments.Apr 20 2021, 2:22 PM
llvm/test/tools/llvm-objdump/MachO/rpaths.test
6

In this case there's also the header, which I wasn't worried about showing up one way or the other (since I made sure below it still doesn't show up if you pass the flag to disable it)

keith updated this revision to Diff 339060.Apr 20 2021, 5:31 PM
keith marked an inline comment as done.

Rebase, add test for header

jhenderson added inline comments.Apr 21 2021, 12:20 AM
llvm/test/tools/llvm-objdump/MachO/rpaths.test
6

I think @MaskRay's point was that the current and previous patterns don't detect either of the following two cases:

Objc2.64bit.exe.macho-x86_64:
<some other rpath>
@executable_path/../Frameworks
<some other rpath>
Objc2.64bit.exe.macho-x86_64:
some nonsense @executable_path/../Frameworks garbagegarbage

By default, FileCheck looks for substrings anywhere in the output after the previous match, and succeeds if it finds them. It doesn't have to be the whole line, unless you use --match-full-lines. --strict-whitespace also ensures that the line is appropriately indented (or not) as desired, with no leading or trailing whitespace beyond what is checked-for explicitly on each CHECK line. Meanwhile, CHECK-NEXT and CHECK-EMPTY can be used to ensure the output is on the desired lines in relation to each other.

Finally, if you want to ensure there is no output anywhere other than what you check for explicitly, you could use --implicit-check-not={{.}}.

keith updated this revision to Diff 339272.Apr 21 2021, 9:32 AM
keith marked an inline comment as done.

Make test more strict

This revision was landed with ongoing or failed builds.Apr 22 2021, 4:02 PM
This revision was automatically updated to reflect the committed changes.