This is an archive of the discontinued LLVM Phabricator instance.

Add a CommandGuide for llvm-objdump
ClosedPublic

Authored by mtrent on Jul 30 2018, 6:23 PM.

Details

Summary

Add a CommandGuide for llvm-objdump summarizing its usage along with some
general context.

Diff Detail

Event Timeline

mtrent created this revision.Jul 30 2018, 6:23 PM
Eugene.Zelenko added inline comments.
docs/CommandGuide/llvm-objdump.rst
281

Probably doc should be used instead of manpage.

mtrent edited reviewers, added: beanz; removed: enderby.Jul 30 2018, 11:09 PM
mtrent added inline comments.Jul 30 2018, 11:12 PM
docs/CommandGuide/llvm-objdump.rst
281

Can you explain? This file is written in the same style as all the other documents in llvm/docs, in rst which can be converted into html, man, epub or whatever you like.

mtrent marked an inline comment as done.Jul 30 2018, 11:21 PM
mtrent added inline comments.
docs/CommandGuide/llvm-objdump.rst
281

Oh I see what you mean. I'll have a look.

mtrent updated this revision to Diff 158179.Jul 30 2018, 11:54 PM
mtrent marked an inline comment as done.

Specify a manpages_url so that CommandGuide references can be used both
as traditional UNIX man-page references and also as nicely hyperlinked
documentation.

Given how mach-o is radically different from ELF, it might make sense to sort or group ELF and mach-o specific options. Right now we only call out options that only work on Mach-o, but I assume some of the options only work on ELF and/or COFF. Might make sense to document that behavior correctly too.

Below I've marked out a few places where mach-o options are missing the annotation.

docs/CommandGuide/llvm-objdump.rst
29

Does this require -macho?

57

I assume this also requires -macho?

78

requires -macho?

107

requires -macho?

174

Maybe change "Mach-O-only" to "requires -macho" for consistency.

186

I assume this also requires -macho?

202

Side note for a separate conversation. Do we really need this flag? Is there a reason we can't just use the magic bits at the beginning of the file stream to set the mode?

mtrent marked an inline comment as done.Jul 31 2018, 1:11 PM

The short summary is "-macho" doesn't mean the target file is Mach-O. "-macho" means use the Mach-O specific file parser for walking the file contents, instead of using llvm-Object. When run in this way llvm-objdump will display information in a different way. Like it or hate it, it's what llvm-objdump does currently.

docs/CommandGuide/llvm-objdump.rst
29

No. The input file must be Mach-O (I assume) but you don't need to use the "-macho" specific binary parser.

57

No. The input file must be Mach-O (I assume) but you don't need to use the "-macho" specific binary parser.

78

No. The input file must be Mach-O (I assume) but you don't need to use the "-macho" specific binary parser.

107

No. The input file must be Mach-O (I assume) but you don't need to use the "-macho" specific binary parser.

174

Actually, looks like the -cfg option has been removed from newer objdumps, so I'll pull it from the CommandGuide.

202

This flag is basically magic sauce meaning "run llvm-objdump in a otool compatible way", affecting both how the data is consumed and how it is printed. You can verify this yourself by comparing the output of "llvm-objdump -r" and "llvm-objdump -r -macho". It's entirely possible that general purpose developer tools would like to have a common llvm-y way of describing object file details, such as for some kind of CI system or packaging automation. So the same way you should use the high-level llvm-Object API to return data for general files, you can use the high-level llvm-objdump CLI to return this same data.

As Mach-O specific features that were implemented using the llbm-Ojbect API, one could argue that those features should have been made in the Mach-O specific parser instead of llvm-Object. I am going to argue that changing the implementation of llvm-objdump is beyond the scope of my task, which is to make a CommandGuide describing llvm-objdump's current behavior. :)

mtrent updated this revision to Diff 158369.Jul 31 2018, 1:12 PM

Remove the obsolete "-cfg" option, as called out during review feedback.

beanz added a comment.Aug 2 2018, 3:21 PM

One last small fix

docs/CommandGuide/llvm-objdump.rst
277

I think what you want here is:

:program:`llvm-nm`

That should produce the correct link in both the online and manpage documentation.

mtrent marked 2 inline comments as done.Aug 4 2018, 7:27 PM

One last small fix

It turns out :program: does not make a hyperlink in the comman guide. Email with details sent.

The :manpage: Sphinx Role does one job beyond a basic style change, and that is let you carve up the man ref syntax <page>(<section>) and arrange it into a URL. I have modified the docs/conf.py file to do this for llvm.org in the first round of review feedback. So I believe with great confidence that :manpage: is the correct Sphinx Role to use for the “See Also” section of this (and all) CommandGuide pages. It may be the only job it is good for, but for this task, I haven’t seen anything better so far.

beanz accepted this revision.Aug 6 2018, 3:40 PM

@mtrent and I talked offline, and he explained how the sphinx documentation covers the manpage and program directives, and I agree with his assertion that using manpage is correct.

This revision is now accepted and ready to land.Aug 6 2018, 3:40 PM
This revision was automatically updated to reflect the committed changes.