Page MenuHomePhabricator

Introduce llvm-objdump man page
ClosedPublic

Authored by emaste on Nov 23 2018, 7:28 AM.

Details

Summary

This is an initial take on a man page for llvm-objdump based on usage output and LLVM's doc.

Diff Detail

Repository
rL LLVM

Event Timeline

emaste created this revision.Nov 23 2018, 7:28 AM
emaste added a subscriber: llvm-commits.
Higuoxing added a comment.EditedNov 23 2018, 8:32 PM

lgtm, but seems that some hidden/alias options are not listed here (I could help add them). I am not expert in this area, so I will leave the decision to @kristina

MaskRay added inline comments.
usr.bin/clang/llvm-objdump/llvm-objdump.1
69 ↗(On Diff #175132)

There is no -verify-debug-info

70 ↗(On Diff #175132)

This option is unfortunately pulled in from a static constructor defined in`lib/IR/Dominators.cpp`. It is not an llvm-objdump option.

BTW, usr.bin/clang/llvm-objdump/llvm-objdump.1 here is not FreeBSD phabricator :) (My biggest complaint there is that for someone who doesn't have a @FreeBSD.org email address, the username is weirdly mangled from the email address and it cannot be changed)

MaskRay added inline comments.Nov 23 2018, 11:37 PM
usr.bin/clang/llvm-objdump/llvm-objdump.1
12 ↗(On Diff #175132)

It does not seem very necessary to list all options in SYNOPSIS.

218 ↗(On Diff #175132)

-print-after-all is a legacy PM option defined in lib/IR/LegacyPassManager.cpp. llvm-objdump doesn't need it.

These cl::opt options should at some point be refactored to use tablegen as llvm-objcopy does...

238 ↗(On Diff #175132)

It should be accompanied by -full-sections. My preference is that these options are sorted and aligned by their full names and the single-letter aliases follow (not following GNU convention), as what lld/docs/ld.lld1.1 does:

.It Fl -emit-relocs , Fl q
Generate relocations in the output.
.It Fl -enable-new-dtags
Enable new dynamic tags.
emaste marked 5 inline comments as done.Nov 24 2018, 10:24 AM

lgtm, but seems that some hidden/alias options are not listed here (I could help add them). I am not expert in this area, so I will leave the decision to @kristina

I just based this on the usage output. I will update with added/removed options and we can iterate on it from there.

BTW, usr.bin/clang/llvm-objdump/llvm-objdump.1 here is not FreeBSD phabricator :)

Sorry about that :)

Next upload will be from the proper llvm location - since the content is the same I just uploaded the same diff here.

usr.bin/clang/llvm-objdump/llvm-objdump.1
12 ↗(On Diff #175132)

Same comment was raised in a separate review for FreeBSD in https://reviews.freebsd.org/D18309. Will update.

69 ↗(On Diff #175132)

Will delete.

70 ↗(On Diff #175132)

Will delete.

218 ↗(On Diff #175132)

Will delete.

238 ↗(On Diff #175132)

It was pointed out in some ld.lld.1 review that preference should be for sorting by short option, but I believe that some of the short options are still not listed in ld.lld.1.

kristina added a comment.EditedNov 24 2018, 11:45 PM

Everything that is explicitly marked as non-hidden should be in the man-page.

These cl::opt options should at some point be refactored to use tablegen as llvm-objcopy does...

Yes that's on my todo list still, been trying to deal with an annoying Clang bug as well as reviewing the Hurd driver for Clang and Hurd's addition to supported OS targets. I wrote up the TableGen file, but still need to fix a lot of help lines as well as clean everything up.

I'll try to get on it ASAP.

emaste updated this revision to Diff 175244.Nov 26 2018, 6:49 AM
emaste marked 2 inline comments as done.

Review feedback

emaste added inline comments.Nov 26 2018, 6:50 AM
usr.bin/clang/llvm-objdump/llvm-objdump.1
218 ↗(On Diff #175132)

Presumably --print-before-all also.

238 ↗(On Diff #175132)

Note GNU objdump lists -s|--full-contents

Additional comment that wasn't submitted while reviews.llvm.org was down on Friday: I started working on this as it's needed for us to switch from GNU objdump to llvm-objdump in FreeBSD (FreeBSD PR229046).

As an aside, there are a few outstanding issues in llvm-objdump that we'd like to have addressed (LLVM PRs):
39101 llvm-objdump does not support --demangle
37895 llvm-objdump --help omits many single-letter options
37151 llvm-objdump does not accept -z option
31679 llvm-objdump does not accept combined args as GNU objdump does
30241 llvm-objdump -p omits dynamic headers (in comparison to GNU objdump)
27249 llvm-objdump --dwarf=frames printing wrong values
26892 llvm-objdump -t includes reserved undefined symbol in output, compared with GNU objdump
25124 llvm-objdump prints unknown for relocation info on PPC
21059 llvm-objdump -r reports relocation against symbol `Unknown' on x86
18293 llvm-objdump: warning: invalid instruction encoding
18154 llvm-objdump should have a option to start disassembly at a specified symbol name

It seems that there are option parsing improvements to be made in llvm-objdump, as well as in its usage information. I hope that we can commit this man page and then keep it updates as that work is done.

MaskRay added inline comments.Nov 26 2018, 12:28 PM
docs/llvm-objdump.1
120 ↗(On Diff #175244)

-rng-seed is in lib/Support/RandomNumberGenerator.cpp

145 ↗(On Diff #175244)

-time-passes is from a static constructor EnableTiming defined in lib/IR/PassTimingInfo.cpp. It, -stats and -stat-json should also be removed.

I have an interim copy in FreeBSD and am updating it based on the feedback here; I'll update this review shortly.

The FreeBSD version is here: https://svnweb.freebsd.org/base/head/usr.bin/clang/llvm-objdump/llvm-objdump.1?view=annotate

emaste updated this revision to Diff 176387.Dec 3 2018, 6:23 AM
  • fix warnings from mandoc -Tlint and/or igor
  • remove options that should not be listed

Can I go ahead with this version as a starting point?

MaskRay accepted this revision.Dec 6 2018, 11:12 AM
This revision is now accepted and ready to land.Dec 6 2018, 11:12 AM
kristina accepted this revision.Dec 6 2018, 1:22 PM

LGTM.

Ping for author, this has been approved for a while, do you need this committed?

Closed by commit rL349595: Add llvm-objdump man page (authored by emaste, committed by ). · Explain WhyDec 18 2018, 5:30 PM
This revision was automatically updated to reflect the committed changes.