Filter out irrelevant llvm-nm option

[serge-sans-paille]

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.

[jhenderson]

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?

[serge-sans-paille]

% ./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]

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.

[jhenderson]

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).

[MaskRay]

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

[serge-sans-paille]

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?

[jhenderson]

In D60411#1461905, @serge-sans-paille wrote:

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.

[serge-sans-paille]

Update test

[serge-sans-paille]

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]

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

[serge-sans-paille]

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

[jhenderson]

Great, LGTM.

llvm/trunk/test/tools/llvm-nm/help.test

+RUN: llvm-nm --help 2>&1 | FileCheck --implicit-check-not="General Options:" %s
+CHECK: Generic Options:
+CHECK: llvm-nm Options:

llvm/trunk/tools/llvm-nm/llvm-nm.cpp

 #include "llvm/Support/raw_ostream.h"
 #include <vector>
 
 using namespace llvm;
 using namespace object;
 
 namespace {
 enum OutputFormatTy { bsd, sysv, posix, darwin };
+
+cl::OptionCategory NMCat("llvm-nm Options");
+
 cl::opt<OutputFormatTy> OutputFormat(
     "format", cl::desc("Specify output format"),
     cl::values(clEnumVal(bsd, "BSD format"), clEnumVal(sysv, "System V format"),
                clEnumVal(posix, "POSIX.2 format"),
                clEnumVal(darwin, "Darwin -m format")),
-    cl::init(bsd));
+    cl::init(bsd), cl::cat(NMCat));
 cl::alias OutputFormat2("f", cl::desc("Alias for --format"),
                         cl::aliasopt(OutputFormat));
 
 cl::list<std::string> InputFilenames(cl::Positional, cl::desc("<input files>"),
                                      cl::ZeroOrMore);
 
 cl::opt<bool> UndefinedOnly("undefined-only",
-                            cl::desc("Show only undefined symbols"));
+                            cl::desc("Show only undefined symbols"),
+                            cl::cat(NMCat));
 cl::alias UndefinedOnly2("u", cl::desc("Alias for --undefined-only"),
                          cl::aliasopt(UndefinedOnly), cl::Grouping);
 
 cl::opt<bool> DynamicSyms("dynamic",
                           cl::desc("Display the dynamic symbols instead "
-                                   "of normal symbols."));
+                                   "of normal symbols."),
+                          cl::cat(NMCat));
 cl::alias DynamicSyms2("D", cl::desc("Alias for --dynamic"),
                        cl::aliasopt(DynamicSyms), cl::Grouping);
 
-cl::opt<bool> DefinedOnly("defined-only",
-                          cl::desc("Show only defined symbols"));
+cl::opt<bool> DefinedOnly("defined-only", cl::desc("Show only defined symbols"),
+                          cl::cat(NMCat));
 cl::alias DefinedOnly2("U", cl::desc("Alias for --defined-only"),
                        cl::aliasopt(DefinedOnly), cl::Grouping);
 
 cl::opt<bool> ExternalOnly("extern-only",
                            cl::desc("Show only external symbols"),
-                           cl::ZeroOrMore);
+                           cl::ZeroOrMore, cl::cat(NMCat));
 cl::alias ExternalOnly2("g", cl::desc("Alias for --extern-only"),
                         cl::aliasopt(ExternalOnly), cl::Grouping,
                         cl::ZeroOrMore);
 
-cl::opt<bool> NoWeakSymbols("no-weak",
-                            cl::desc("Show only non-weak symbols"));
+cl::opt<bool> NoWeakSymbols("no-weak", cl::desc("Show only non-weak symbols"),
+                            cl::cat(NMCat));
 cl::alias NoWeakSymbols2("W", cl::desc("Alias for --no-weak"),
                          cl::aliasopt(NoWeakSymbols), cl::Grouping);
 
-cl::opt<bool> BSDFormat("B", cl::desc("Alias for --format=bsd"),
-                        cl::Grouping);
+cl::opt<bool> BSDFormat("B", cl::desc("Alias for --format=bsd"), cl::Grouping,
+                        cl::cat(NMCat));
 cl::opt<bool> POSIXFormat("P", cl::desc("Alias for --format=posix"),
-                          cl::Grouping);
+                          cl::Grouping, cl::cat(NMCat));
 cl::alias Portability("portability", cl::desc("Alias for --format=posix"),
                       cl::aliasopt(POSIXFormat), cl::NotHidden);
 cl::opt<bool> DarwinFormat("m", cl::desc("Alias for --format=darwin"),
-                           cl::Grouping);
+                           cl::Grouping, cl::cat(NMCat));
 
 static cl::list<std::string>
     ArchFlags("arch", cl::desc("architecture(s) from a Mach-O file to dump"),
-              cl::ZeroOrMore);
+              cl::ZeroOrMore, cl::cat(NMCat));
 bool ArchAll = false;
 
 cl::opt<bool> PrintFileName(
     "print-file-name",
-    cl::desc("Precede each symbol with the object file it came from"));
+    cl::desc("Precede each symbol with the object file it came from"),
+    cl::cat(NMCat));
 
 cl::alias PrintFileNameA("A", cl::desc("Alias for --print-file-name"),
                          cl::aliasopt(PrintFileName), cl::Grouping);
 cl::alias PrintFileNameo("o", cl::desc("Alias for --print-file-name"),
                          cl::aliasopt(PrintFileName), cl::Grouping);
 
 cl::opt<bool> DebugSyms("debug-syms",
-                        cl::desc("Show all symbols, even debugger only"));
+                        cl::desc("Show all symbols, even debugger only"),
+                        cl::cat(NMCat));
 cl::alias DebugSymsa("a", cl::desc("Alias for --debug-syms"),
                      cl::aliasopt(DebugSyms), cl::Grouping);
 
-cl::opt<bool> NumericSort("numeric-sort", cl::desc("Sort symbols by address"));
+cl::opt<bool> NumericSort("numeric-sort", cl::desc("Sort symbols by address"),
+                          cl::cat(NMCat));
 cl::alias NumericSortn("n", cl::desc("Alias for --numeric-sort"),
                        cl::aliasopt(NumericSort), cl::Grouping);
 cl::alias NumericSortv("v", cl::desc("Alias for --numeric-sort"),
                        cl::aliasopt(NumericSort), cl::Grouping);
 
-cl::opt<bool> NoSort("no-sort", cl::desc("Show symbols in order encountered"));
+cl::opt<bool> NoSort("no-sort", cl::desc("Show symbols in order encountered"),
+                     cl::cat(NMCat));
 cl::alias NoSortp("p", cl::desc("Alias for --no-sort"), cl::aliasopt(NoSort),
                   cl::Grouping);
 
 cl::opt<bool> Demangle("demangle", cl::ZeroOrMore,
-                       cl::desc("Demangle C++ symbol names"));
+                       cl::desc("Demangle C++ symbol names"), cl::cat(NMCat));
 cl::alias DemangleC("C", cl::desc("Alias for --demangle"),
                     cl::aliasopt(Demangle), cl::Grouping);
 cl::opt<bool> NoDemangle("no-demangle", cl::init(false), cl::ZeroOrMore,
-                         cl::desc("Don't demangle symbol names"));
+                         cl::desc("Don't demangle symbol names"),
+                         cl::cat(NMCat));
 
-cl::opt<bool> ReverseSort("reverse-sort", cl::desc("Sort in reverse order"));
+cl::opt<bool> ReverseSort("reverse-sort", cl::desc("Sort in reverse order"),
+                          cl::cat(NMCat));
 cl::alias ReverseSortr("r", cl::desc("Alias for --reverse-sort"),
                        cl::aliasopt(ReverseSort), cl::Grouping);
 
 cl::opt<bool> PrintSize("print-size",
-                        cl::desc("Show symbol size instead of address"));
+                        cl::desc("Show symbol size instead of address"),
+                        cl::cat(NMCat));
 cl::alias PrintSizeS("S", cl::desc("Alias for --print-size"),
                      cl::aliasopt(PrintSize), cl::Grouping);
 bool MachOPrintSizeWarning = false;
 
-cl::opt<bool> SizeSort("size-sort", cl::desc("Sort symbols by size"));
+cl::opt<bool> SizeSort("size-sort", cl::desc("Sort symbols by size"),
+                       cl::cat(NMCat));
 
 cl::opt<bool> WithoutAliases("without-aliases", cl::Hidden,
-                             cl::desc("Exclude aliases from output"));
+                             cl::desc("Exclude aliases from output"),
+                             cl::cat(NMCat));
 
-cl::opt<bool> ArchiveMap("print-armap", cl::desc("Print the archive map"));
+cl::opt<bool> ArchiveMap("print-armap", cl::desc("Print the archive map"),
+                         cl::cat(NMCat));
 cl::alias ArchiveMaps("M", cl::desc("Alias for --print-armap"),
                       cl::aliasopt(ArchiveMap), cl::Grouping);
 
 enum Radix { d, o, x };
 cl::opt<Radix>
     AddressRadix("radix", cl::desc("Radix (o/d/x) for printing symbol Values"),
                  cl::values(clEnumVal(d, "decimal"), clEnumVal(o, "octal"),
                             clEnumVal(x, "hexadecimal")),
-                 cl::init(x));
+                 cl::init(x), cl::cat(NMCat));
 cl::alias RadixAlias("t", cl::desc("Alias for --radix"),
                      cl::aliasopt(AddressRadix));
 
 cl::opt<bool> JustSymbolName("just-symbol-name",
-                             cl::desc("Print just the symbol's name"));
+                             cl::desc("Print just the symbol's name"),
+                             cl::cat(NMCat));
 cl::alias JustSymbolNames("j", cl::desc("Alias for --just-symbol-name"),
                           cl::aliasopt(JustSymbolName), cl::Grouping);
 
 // FIXME: This option takes exactly two strings and should be allowed anywhere
 // on the command line.  Such that "llvm-nm -s __TEXT __text foo.o" would work.
 // But that does not as the CommandLine Library does not have a way to make
 // this work.  For now the "-s __TEXT __text" has to be last on the command
 // line.
 cl::list<std::string> SegSect("s", cl::Positional, cl::ZeroOrMore,
+                              cl::value_desc("segment section"), cl::Hidden,
                               cl::desc("Dump only symbols from this segment "
-                                       "and section name, Mach-O only"));
+                                       "and section name, Mach-O only"),
+                              cl::cat(NMCat));
 
-cl::opt<bool> FormatMachOasHex("x", cl::desc("Print symbol entry in hex, "
-                                             "Mach-O only"), cl::Grouping);
+cl::opt<bool> FormatMachOasHex("x",
+                               cl::desc("Print symbol entry in hex, "
+                                        "Mach-O only"),
+                               cl::Grouping, cl::cat(NMCat));
 cl::opt<bool> AddDyldInfo("add-dyldinfo",
                           cl::desc("Add symbols from the dyldinfo not already "
-                                   "in the symbol table, Mach-O only"));
+                                   "in the symbol table, Mach-O only"),
+                          cl::cat(NMCat));
 cl::opt<bool> NoDyldInfo("no-dyldinfo",
                          cl::desc("Don't add any symbols from the dyldinfo, "
-                                  "Mach-O only"));
+                                  "Mach-O only"),
+                         cl::cat(NMCat));
 cl::opt<bool> DyldInfoOnly("dyldinfo-only",
                            cl::desc("Show only symbols from the dyldinfo, "
-                                    "Mach-O only"));
+                                    "Mach-O only"),
+                           cl::cat(NMCat));
 
 cl::opt<bool> NoLLVMBitcode("no-llvm-bc",
-                            cl::desc("Disable LLVM bitcode reader"));
+                            cl::desc("Disable LLVM bitcode reader"),
+                            cl::cat(NMCat));
 
 cl::extrahelp HelpResponse("\nPass @FILE as argument to read options from FILE.\n");
 
 bool PrintAddress = true;
 
 bool MultipleFiles = false;
 
 bool HadError = false;
if (SymbolicFile *O = dyn_cast<SymbolicFile>(&Bin)) {
     if (!checkMachOAndArchFlags(O, Filename))
       return;
     dumpSymbolNamesFromObject(*O, true);
   }
 }
 
 int main(int argc, char **argv) {
   InitLLVM X(argc, argv);
+  cl::HideUnrelatedOptions(NMCat);
   cl::ParseCommandLineOptions(argc, argv, "llvm symbol table dumper\n");
 
   // llvm-nm only reads binary files.
   if (error(sys::ChangeStdinToBinary()))
     return 1;
 
   // These calls are needed so that we can read bitcode correctly.
   llvm::InitializeAllTargetInfos();