From 98d2a19fea7016fb2111bab7b2f5096f1b5c342d Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Mon, 5 Jul 2021 10:46:17 -0700 Subject: [PATCH] [llvm-strings] Switch command line parsing from llvm::cl to OptTable Some behavior changes: * `-t=d` is removed. Use `-t d` instead. * one-dash long options like `-all` are supported. Use `--all` instead. * `--all=0` or `--all=false` cannot be used. (Note: `--all` is silently ignored anyway) * `--help-list` is removed. This is a `cl::` specific option. Nobody is likely leveraging any of the above. Advantages: * `-t` diagnostic gets improved. * in the absence of `HideUnrelatedOptions`, `--help` will not list unrelated options if linking against libLLVM-13git.so or linker GC is not used. * Decrease the probability of cl::opt collision if we do decide to support multiplexing Note: because the tool is so simple, used more for forensics instead of a building tool, and its long options are unlikely used in one-dash form, I just drop the one-dash form in this patch. Reviewed By: jhenderson Differential Revision: https://reviews.llvm.org/D104889 --- docs/CommandGuide/llvm-strings.rst | 4 - test/tools/llvm-strings/grouped.test | 4 + test/tools/llvm-strings/help.test | 16 ++-- test/tools/llvm-strings/length.test | 6 +- test/tools/llvm-strings/radix.test | 4 +- tools/llvm-strings/CMakeLists.txt | 7 ++ tools/llvm-strings/Opts.td | 23 ++++++ tools/llvm-strings/llvm-strings.cpp | 117 +++++++++++++++++++++------ 8 files changed, 136 insertions(+), 45 deletions(-) create mode 100644 test/tools/llvm-strings/grouped.test create mode 100644 tools/llvm-strings/Opts.td diff --git a/docs/CommandGuide/llvm-strings.rst b/docs/CommandGuide/llvm-strings.rst index e2fe4cb88c9..f66b22ec8df 100644 --- a/docs/CommandGuide/llvm-strings.rst +++ b/docs/CommandGuide/llvm-strings.rst @@ -52,10 +52,6 @@ OPTIONS Display a summary of command line options. -.. option:: --help-list - - Display an uncategorized summary of command line options. - .. option:: --print-file-name, -f Display the name of the containing file before each string. diff --git a/test/tools/llvm-strings/grouped.test b/test/tools/llvm-strings/grouped.test new file mode 100644 index 00000000000..b5c38b6cc86 --- /dev/null +++ b/test/tools/llvm-strings/grouped.test @@ -0,0 +1,4 @@ +## Show that short options can be grouped. + +RUN: echo abcd | llvm-strings -af | FileCheck %s +CHECK: {standard input}: abcd diff --git a/test/tools/llvm-strings/help.test b/test/tools/llvm-strings/help.test index d5d4ca80a4f..5872dc97532 100644 --- a/test/tools/llvm-strings/help.test +++ b/test/tools/llvm-strings/help.test @@ -1,15 +1,11 @@ ## Show that help text is printed correctly when requested. -RUN: llvm-strings -h | FileCheck %s --check-prefixes=CHECK,CATEG -RUN: llvm-strings --help | FileCheck %s --check-prefixes=CHECK,CATEG -RUN: llvm-strings --help-list \ -RUN: | FileCheck %s --check-prefixes=CHECK,LIST +RUN: llvm-strings -h | FileCheck %s +RUN: llvm-strings --help | FileCheck %s CHECK: OVERVIEW: llvm string dumper -CHECK: USAGE: llvm-strings{{(.exe)?}} [options] {{$}} +CHECK: USAGE: llvm-strings [options] {{$}} CHECK: OPTIONS: -CATEG: General options: -LIST-NOT: General options: -CATEG: Generic Options: -LIST-NOT: Generic Options: -CHECK: @FILE +CHECK: --all +CHECK: -a +CHECK: Pass @FILE as argument to read options from FILE. diff --git a/test/tools/llvm-strings/length.test b/test/tools/llvm-strings/length.test index ac25c523c94..4897931f38f 100644 --- a/test/tools/llvm-strings/length.test +++ b/test/tools/llvm-strings/length.test @@ -21,9 +21,9 @@ RUN: llvm-strings --bytes 2 %t | FileCheck --check-prefix CHECK-2 %s --implicit- ## Show different syntaxes work. RUN: llvm-strings --bytes=2 %t | FileCheck --check-prefix CHECK-2 %s --implicit-check-not={{.}} -RUN: llvm-strings -n=2 %t | FileCheck --check-prefix CHECK-2 %s --implicit-check-not={{.}} +RUN: llvm-strings -n 2 %t | FileCheck --check-prefix CHECK-2 %s --implicit-check-not={{.}} -CHECK-0: invalid minimum string length 0 +CHECK-0: llvm-strings: error: expected a positive integer, but got '0' CHECK-1: a CHECK-1-NEXT: ab @@ -43,4 +43,4 @@ CHECK-5: abcde ## Show that a non-numeric argument is rejected. RUN: not llvm-strings -n foo %t 2>&1 | FileCheck %s --check-prefix=ERR -ERR: llvm-strings{{.*}}: for the --bytes option: 'foo' value invalid for integer argument! +ERR: llvm-strings: error: expected a positive integer, but got 'foo' diff --git a/test/tools/llvm-strings/radix.test b/test/tools/llvm-strings/radix.test index 4dafbd1c84f..e3ef4ce3a0c 100644 --- a/test/tools/llvm-strings/radix.test +++ b/test/tools/llvm-strings/radix.test @@ -26,7 +26,7 @@ RUN: llvm-strings --radix x %t/a.txt | FileCheck %s -check-prefix CHECK-HEX --st ## Show different syntaxes work. RUN: llvm-strings --radix=d %t/a.txt | FileCheck %s -check-prefix CHECK-DEC --strict-whitespace -RUN: llvm-strings -t=d %t/a.txt | FileCheck %s -check-prefix CHECK-DEC --strict-whitespace +RUN: llvm-strings -t d %t/a.txt | FileCheck %s -check-prefix CHECK-DEC --strict-whitespace CHECK-NONE: {{^}}three CHECK-NONE: {{^}}four @@ -58,4 +58,4 @@ CHECK-HEX: {{^}} 28 nine ## Show that an invalid value is rejected. RUN: not llvm-strings --radix z %t/a.txt 2>&1 | FileCheck %s --check-prefix=INVALID -INVALID: llvm-strings{{.*}}: for the --radix option: Cannot find option named 'z'! +INVALID: llvm-strings: error: --radix value should be one of: '' (no offset), 'o' (octal), 'd' (decimal), 'x' (hexadecimal) diff --git a/tools/llvm-strings/CMakeLists.txt b/tools/llvm-strings/CMakeLists.txt index 390f1175139..95dd69c778e 100644 --- a/tools/llvm-strings/CMakeLists.txt +++ b/tools/llvm-strings/CMakeLists.txt @@ -1,11 +1,18 @@ set(LLVM_LINK_COMPONENTS Core Object + Option Support ) +set(LLVM_TARGET_DEFINITIONS Opts.td) +tablegen(LLVM Opts.inc -gen-opt-parser-defs) +add_public_tablegen_target(StringsOptsTableGen) + add_llvm_tool(llvm-strings llvm-strings.cpp + DEPENDS + StringsOptsTableGen ) if(LLVM_INSTALL_BINUTILS_SYMLINKS) diff --git a/tools/llvm-strings/Opts.td b/tools/llvm-strings/Opts.td new file mode 100644 index 00000000000..2ad77fae6c1 --- /dev/null +++ b/tools/llvm-strings/Opts.td @@ -0,0 +1,23 @@ +include "llvm/Option/OptParser.td" + +class F : Flag<["-"], letter>, HelpText; +class FF : Flag<["--"], name>, HelpText; + +multiclass Eq { + def NAME #_EQ : Joined<["--"], name #"=">, + HelpText; + def : Separate<["--"], name>, Alias(NAME #_EQ)>; +} + +def all : FF<"all", "Silently ignored. Present for GNU strings compatibility">; +defm bytes : Eq<"bytes", "Print sequences of the specified length">; +def help : FF<"help", "Display this help">; +def print_file_name : Flag<["--"], "print-file-name">, HelpText<"Print the name of the file before each string">; +defm radix : Eq<"radix", "Print the offset within the file with the specified radix: o (octal), d (decimal), x (hexadecimal)">, MetaVarName<"">; +def version : FF<"version", "Display the version">; + +def : F<"a", "Alias for --all">, Alias; +def : F<"f", "Alias for --print-file-name">, Alias; +def : F<"h", "Alias for --help">, Alias; +def : JoinedOrSeparate<["-"], "n">, Alias, HelpText<"Alias for --bytes">; +def : JoinedOrSeparate<["-"], "t">, Alias, HelpText<"Alias for --radix">, MetaVarName<"">; diff --git a/tools/llvm-strings/llvm-strings.cpp b/tools/llvm-strings/llvm-strings.cpp index 51313d73401..0b068749917 100644 --- a/tools/llvm-strings/llvm-strings.cpp +++ b/tools/llvm-strings/llvm-strings.cpp @@ -11,51 +11,81 @@ // //===----------------------------------------------------------------------===// +#include "Opts.inc" #include "llvm/Object/Binary.h" +#include "llvm/Option/Arg.h" +#include "llvm/Option/ArgList.h" +#include "llvm/Option/Option.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/Error.h" #include "llvm/Support/Format.h" #include "llvm/Support/InitLLVM.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Program.h" +#include "llvm/Support/WithColor.h" #include #include using namespace llvm; using namespace llvm::object; +namespace { +enum ID { + OPT_INVALID = 0, // This is not an option ID. +#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \ + HELPTEXT, METAVAR, VALUES) \ + OPT_##ID, +#include "Opts.inc" +#undef OPTION +}; + +#define PREFIX(NAME, VALUE) const char *const NAME[] = VALUE; +#include "Opts.inc" +#undef PREFIX + +static const opt::OptTable::Info InfoTable[] = { +#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \ + HELPTEXT, METAVAR, VALUES) \ + { \ + PREFIX, NAME, HELPTEXT, \ + METAVAR, OPT_##ID, opt::Option::KIND##Class, \ + PARAM, FLAGS, OPT_##GROUP, \ + OPT_##ALIAS, ALIASARGS, VALUES}, +#include "Opts.inc" +#undef OPTION +}; + +class StringsOptTable : public opt::OptTable { +public: + StringsOptTable() : OptTable(InfoTable) { setGroupedShortOptions(true); } +}; +} // namespace + +const char ToolName[] = "llvm-strings"; + static cl::list InputFileNames(cl::Positional, cl::desc(""), cl::ZeroOrMore); -static cl::opt - PrintFileName("print-file-name", - cl::desc("Print the name of the file before each string")); -static cl::alias PrintFileNameShort("f", cl::desc(""), - cl::aliasopt(PrintFileName)); - -static cl::opt - MinLength("bytes", cl::desc("Print sequences of the specified length"), - cl::init(4)); -static cl::alias MinLengthShort("n", cl::desc(""), cl::aliasopt(MinLength)); - -static cl::opt - AllSections("all", - cl::desc("Check all sections, not just the data section")); -static cl::alias AllSectionsShort("a", cl::desc(""), - cl::aliasopt(AllSections)); +static int MinLength = 4; +static bool PrintFileName; enum radix { none, octal, hexadecimal, decimal }; -static cl::opt - Radix("radix", cl::desc("print the offset within the file"), - cl::values(clEnumValN(octal, "o", "octal"), - clEnumValN(hexadecimal, "x", "hexadecimal"), - clEnumValN(decimal, "d", "decimal")), - cl::init(none)); -static cl::alias RadixShort("t", cl::desc(""), cl::aliasopt(Radix)); +static radix Radix; -static cl::extrahelp - HelpResponse("\nPass @FILE as argument to read options from FILE.\n"); +LLVM_ATTRIBUTE_NORETURN static void reportCmdLineError(const Twine &Message) { + WithColor::error(errs(), ToolName) << Message << "\n"; + exit(1); +} + +template +static void parseIntArg(const opt::InputArgList &Args, int ID, T &Value) { + if (const opt::Arg *A = Args.getLastArg(ID)) { + StringRef V(A->getValue()); + if (!llvm::to_integer(V, Value, 0) || Value <= 0) + reportCmdLineError("expected a positive integer, but got '" + V + "'"); + } +} static void strings(raw_ostream &OS, StringRef FileName, StringRef Contents) { auto print = [&OS, FileName](unsigned Offset, StringRef L) { @@ -96,13 +126,48 @@ static void strings(raw_ostream &OS, StringRef FileName, StringRef Contents) { int main(int argc, char **argv) { InitLLVM X(argc, argv); + BumpPtrAllocator A; + StringSaver Saver(A); + StringsOptTable Tbl; + opt::InputArgList Args = + Tbl.parseArgs(argc, argv, OPT_UNKNOWN, Saver, + [&](StringRef Msg) { reportCmdLineError(Msg); }); + if (Args.hasArg(OPT_help)) { + Tbl.printHelp( + outs(), + (Twine(ToolName) + " [options] ").str().c_str(), + "llvm string dumper"); + // TODO Replace this with OptTable API once it adds extrahelp support. + outs() << "\nPass @FILE as argument to read options from FILE.\n"; + return 0; + } + if (Args.hasArg(OPT_version)) { + outs() << ToolName << '\n'; + cl::PrintVersionMessage(); + return 0; + } + + parseIntArg(Args, OPT_bytes_EQ, MinLength); + PrintFileName = Args.hasArg(OPT_print_file_name); + StringRef R = Args.getLastArgValue(OPT_radix_EQ); + if (R.empty()) + Radix = none; + else if (R == "o") + Radix = octal; + else if (R == "d") + Radix = decimal; + else if (R == "x") + Radix = hexadecimal; + else + reportCmdLineError("--radix value should be one of: '' (no offset), 'o' " + "(octal), 'd' (decimal), 'x' (hexadecimal)"); - cl::ParseCommandLineOptions(argc, argv, "llvm string dumper\n"); if (MinLength == 0) { errs() << "invalid minimum string length 0\n"; return EXIT_FAILURE; } + std::vector InputFileNames = Args.getAllArgValues(OPT_INPUT); if (InputFileNames.empty()) InputFileNames.push_back("-");