Page MenuHomePhabricator

Filter out irrelevant llvm-nm option
ClosedPublic

Authored by serge-sans-paille on Apr 8 2019, 8:49 AM.

Details

Summary

During EuroLLVM binutils BoF, the assistance raised the problem of non-human friendly -help output of binutils. This is a tentative fix that focuses on llvm-nm output, based on Category Filtering.

Diff Detail

Repository
rL LLVM

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptApr 8 2019, 8:49 AM

I don't think a test for every option is wise, but could you paste the output of the help text with your change applied, please, so I can review it easily?

tools/llvm-nm/llvm-nm.cpp
62 ↗(On Diff #194154)

I've not experimented with this, but do you need to put a category for the positional arguments?

101 ↗(On Diff #194154)

Do aliases need a category or do they inherit their base one?

% ./bin/llvm-nm --help
OVERVIEW: llvm symbol table dumper

USAGE: llvm-nm [options] <input files> --s Dump only symbols from this segment and section name, Mach-O only

OPTIONS:

Generic Options:

  -help             - Display available options (-help-hidden for more)
  -help-list        - Display list of available options (-help-list-hidden for more)
  -version          - Display the version of this program

llvm-nm options:

  -B                - Alias for --format=bsd
  -P                - Alias for --format=posix
  -add-dyldinfo     - Add symbols from the dyldinfo not already in the symbol table, Mach-O only
  -arch=<string>    - architecture(s) from a Mach-O file to dump
  -debug-syms       - Show all symbols, even debugger only
  -defined-only     - Show only defined symbols
  -demangle         - Demangle C++ symbol names
  -dyldinfo-only    - Show only symbols from the dyldinfo, Mach-O only
  -dynamic          - Display the dynamic symbols instead of normal symbols.
  -extern-only      - Show only external symbols
  -format=<value>   - Specify output format
    =bsd            -   BSD format
    =sysv           -   System V format
    =posix          -   POSIX.2 format
    =darwin         -   Darwin -m format
  -just-symbol-name - Print just the symbol's name
  -m                - Alias for --format=darwin
  -no-demangle      - Don't demangle symbol names
  -no-dyldinfo      - Don't add any symbols from the dyldinfo, Mach-O only
  -no-llvm-bc       - Disable LLVM bitcode reader
  -no-sort          - Show symbols in order encountered
  -no-weak          - Show only non-weak symbols
  -numeric-sort     - Sort symbols by address
  -portability      - Alias for --format=posix
  -print-armap      - Print the archive map
  -print-file-name  - Precede each symbol with the object file it came from
  -print-size       - Show symbol size instead of address
  -radix=<value>    - Radix (o/d/x) for printing symbol Values
    =d              -   decimal
    =o              -   octal
    =x              -   hexadecimal
  -reverse-sort     - Sort in reverse order
  -s=<string>       - Dump only symbols from this segment and section name, Mach-O only
  -size-sort        - Sort symbols by size
  -undefined-only   - Show only undefined symbols
  -x                - Print symbol entry in hex, Mach-O only

Pass @FILE as argument to read options from FILE.
serge-sans-paille marked 2 inline comments as done.Apr 10 2019, 4:59 AM
serge-sans-paille added inline comments.
tools/llvm-nm/llvm-nm.cpp
62 ↗(On Diff #194154)

Probably not. This may explain the --s glitter, I'll check that.

101 ↗(On Diff #194154)

I'll check too.

Thanks for the help text. I think we could add a simple test actually. It would test that "Generic Options:" and "llvm-nm Options:" are printed, but not "General Options:" (I think that's the name given to the irrelevant LLVM options, but I could be wrong).

Regarding the -help option in llvm-readobj+llvm-objdump, you may want to chime in in https://reviews.llvm.org/D59746

alias don"t need to be marked as part of a category, this makes the diff shorter. Same for positional argument. New output:

OVERVIEW: llvm symbol table dumper

USAGE: llvm-nm [options] <input files> --s Dump only symbols from this segment and section name, Mach-O only

OPTIONS:

Generic Options:

  -help             - Display available options (-help-hidden for more)
  -help-list        - Display list of available options (-help-list-hidden for more)
  -version          - Display the version of this program

llvm-nm Options:

  -B                - Alias for --format=bsd
  -P                - Alias for --format=posix
  -add-dyldinfo     - Add symbols from the dyldinfo not already in the symbol table, Mach-O only
  -arch=<string>    - architecture(s) from a Mach-O file to dump
  -debug-syms       - Show all symbols, even debugger only
  -defined-only     - Show only defined symbols
  -demangle         - Demangle C++ symbol names
  -dyldinfo-only    - Show only symbols from the dyldinfo, Mach-O only
  -dynamic          - Display the dynamic symbols instead of normal symbols.
  -extern-only      - Show only external symbols
  -format=<value>   - Specify output format
    =bsd            -   BSD format
    =sysv           -   System V format
    =posix          -   POSIX.2 format
    =darwin         -   Darwin -m format
  -just-symbol-name - Print just the symbol's name
  -m                - Alias for --format=darwin
  -no-demangle      - Don't demangle symbol names
  -no-dyldinfo      - Don't add any symbols from the dyldinfo, Mach-O only
  -no-llvm-bc       - Disable LLVM bitcode reader
  -no-sort          - Show symbols in order encountered
  -no-weak          - Show only non-weak symbols
  -numeric-sort     - Sort symbols by address
  -print-armap      - Print the archive map
  -print-file-name  - Precede each symbol with the object file it came from
  -print-size       - Show symbol size instead of address
  -radix=<value>    - Radix (o/d/x) for printing symbol Values
    =d              -   decimal
    =o              -   octal
    =x              -   hexadecimal
  -reverse-sort     - Sort in reverse order
  -size-sort        - Sort symbols by size
  -undefined-only   - Show only undefined symbols
  -x                - Print symbol entry in hex, Mach-O only

Pass @FILE as argument to read options from FILE.

Note the USAGE: llvm-nm [options] <input files> --s Dump only symbols from this segment and section name, Mach-O only line, which is not perfect. That's due to the -s flag which is flagged as a positional argument, because it takes two arguments.

I let it as is, I may invest time in the future to fix the real source of the problem.

Question: instead of having -s require two argument, we could require one argument and split on space character, or comma?

Question: instead of having -s require two argument, we could require one argument and split on space character, or comma?

Splitting on a character would make a lot of sense. Most tools use '=' or similar for multi-string arguments. However, if there is a Mach-O version of llvm-nm that inspired this option, we should be compatible with that if at all possible.

test/tools/llvm-nm/help.test
1 ↗(On Diff #194578)

Do an --implicit-check-not="General Options:" here to show that we don't have the old options. I'd also use --help, rather than -help, since that is closer to the norm.

Splitting on a character would make a lot of sense. Most tools use '=' or similar for multi-string arguments.

Leaving that for another commit, that's not directly related to this review.

jhenderson added inline comments.Apr 11 2019, 5:15 AM
test/tools/llvm-nm/help.test
1 ↗(On Diff #194578)

I'd be surprised if this test passed currently...! The --implicit-check-not command should be for FileCheck: https://llvm.org/docs/CommandGuide/FileCheck.html

Fun fact: when --help is active, no error is reported for invalid options :-) Fixed.

jhenderson accepted this revision.Apr 11 2019, 6:15 AM

Great, LGTM.

This revision is now accepted and ready to land.Apr 11 2019, 6:15 AM
Closed by commit rL358185: Make llvm-nm -help great again (authored by serge_sans_paille, committed by ). · Explain WhyApr 11 2019, 8:20 AM
This revision was automatically updated to reflect the committed changes.