[sancov] Switch to OptTable from llvm::cl

[avillega]

Summary

Switch the parse of command line options from llvm::cl to OptTable.

The motivation for this change is to continue adding llvm based tools
to the llvm driver multicall. For more information about the proposal
and motivation, please see https://discourse.llvm.org/t/rfc-llvm-busybox-proposal/58494

[avillega]

Rebase

[leonardchan]

llvm/tools/sancov/Opts.td

50

use_default_ignorelist to match the original flag name

llvm/utils/gn/secondary/llvm/tools/sancov/BUILD.gn

1

I believe the gn sub-build is also peripheral tier and actually updated by a script maintained by @thakis, so BUILD.gn changes shouldn't be included here.

utils/bazel/llvm-project-overlay/llvm/BUILD.bazel

4675

I believe the bazel build is part of the peripheral tier and maintained by someone else who would make the necessary bazel changes. You only need to update the cmake files. Alternatively, if you want to land this also, I think we'd need to find a bazel reviewer to make sure this is correct.

[avillega]

Review comments and rebase.

[avillega]

I've removed the Bazer and gn files.

[avillega]

llvm/tools/sancov/Opts.td

50

Good catch, thanks.

[leonardchan]

LGTM but probably give it a day before landing to give others time to respond

llvm/tools/sancov/sancov.cpp

61

This anonymous namespace might not be necessary since it's already in an anonymous namespace.

llvm/tools/sancov/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   AllTargetsDescs
   AllTargetsDisassemblers
   AllTargetsInfos
   MC
   MCDisassembler
   Object
+  Option
   Support
   Symbolize
   TargetParser
   )
 
+set(LLVM_TARGET_DEFINITIONS Opts.td)
+tablegen(LLVM Opts.inc -gen-opt-parser-defs)
+add_public_tablegen_target(SancovOptsTableGen)
+
 add_llvm_tool(sancov
   sancov.cpp
+
+  DEPENDS
+  SancovOptsTableGen
   )

llvm/tools/sancov/Opts.td

+include "llvm/Option/OptParser.td"
+
+class F<string name, string help> : Flag<["-", "--"], name>, HelpText<help>;
+
+multiclass B<string name, string help1, string help2> {
+  def NAME: Flag<["-", "--"], name>, HelpText<help1>;
+  def no_ # NAME: Flag<["-", "--"], "no-" # name>, HelpText<help2>;
+}
+
+multiclass Eq<string name, string help> {
+  def NAME #_EQ : Joined<["-", "--"], name #"=">,
+                  HelpText<help>;
+  def : Separate<["-", "--"], name>, Alias<!cast<Joined>(NAME #_EQ)>;
+}
+
+def generic_grp : OptionGroup<"Genric Options">, HelpText<"Generic Options">;
+def help : F<"help", "Display this help">, Group<generic_grp>;
+def : Flag<["-"], "h">, Alias<help>, HelpText<"Alias for --help">, Group<generic_grp>;
+def version : F<"version", "Display the version">, Group<generic_grp>;
+def : Flag<["-"], "v">, Alias<version>, HelpText<"Alias for --version">, Group<generic_grp>;
+
+def action_grp : OptionGroup<"Action">, HelpText<"Action (required)">;
+def print : F<"print", "Print coverage addresses">,
+  Group<action_grp>;
+def printCoveragePcs : F<"print-coverage-pcs", "Print coverage instrumentation points addresses.">,
+  Group<action_grp>;
+def coveredFunctions : F<"covered-functions", "Print all covered funcions.">,
+  Group<action_grp>;
+def notCoveredFunctions : F<"not-covered-functions", "Print all not covered funcions.">,
+  Group<action_grp>;
+def printCoverageStats : F<"print-coverage-stats", "Print coverage statistics.">,
+  Group<action_grp>;
+def htmlReport : F<"html-report", "REMOVED. Use -symbolize & coverage-report-server.py.">,
+  Group<action_grp>;
+def symbolize : F<"symbolize", "Produces a symbolized JSON report from binary report.">,
+  Group<action_grp>;
+def merge : F<"merge", "Merges reports.">,
+  Group<action_grp>;
+
+defm demangle : B<"demangle", "Demangle function names", "Do not demangle function names">;
+defm skipDeadFiles : B<"skip-dead-files", "Do not list dead source files in reports",
+                                          "List dead source files in reports">;
+defm useDefaultIgnoreList :
+  B<"use_default_ignore_list", "Use the default ignore list", "Don't use the default ignore list">,
+  Flags<[HelpHidden]>;
+
+// Compatibility aliases
+def : Flag<["-"], "demangle=0">, Alias<no_demangle>, HelpText<"Alias for --no-demangle">;
+def : Flag<["-"], "skip-dead-files=0">, Alias<no_skipDeadFiles>, HelpText<"Alias for --no-skip-dead-files">;
+def : Flag<["-"], "use_default_ignore_list=0">, Alias<no_useDefaultIgnoreList>, HelpText<"Alias for --no-use_default_ignore_list">;
+
+defm stripPathPrefix
+    : Eq<"strip_path_prefix", "Strip this prefix from files paths in reports">,
+      MetaVarName<"<string>">;
+
+defm ignorelist
+    : Eq<"ignorelist", "Ignorelist file (sanitizer ignorelist format)">,
+      MetaVarName<"<string>">;

llvm/tools/sancov/sancov.cpp

 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/MC/MCTargetOptions.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Object/MachO.h"
 #include "llvm/Object/ObjectFile.h"
+#include "llvm/Option/ArgList.h"
+#include "llvm/Option/Option.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/MD5.h"

 
 #include <set>
 #include <vector>
 
 using namespace llvm;
 
 namespace {
 
-// --------- COMMAND LINE FLAGS ---------
+// Command-line option boilerplate.
+namespace {
+using namespace llvm::opt;
+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)                                                    \
+  static constexpr StringLiteral NAME##_init[] = VALUE;                        \
+  static constexpr ArrayRef<StringLiteral> NAME(NAME##_init,                   \
+                                                std::size(NAME##_init) - 1);
+#include "Opts.inc"
+#undef PREFIX
+
+static constexpr 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 SancovOptTable : public opt::GenericOptTable {
+public:
+  SancovOptTable() : GenericOptTable(InfoTable) {}
+};
+} // namespace
 
-cl::OptionCategory Cat("sancov Options");
+// --------- COMMAND LINE FLAGS ---------
 
 enum ActionType {
   CoveredFunctionsAction,
   HtmlReportAction,
   MergeAction,
   NotCoveredFunctionsAction,
   PrintAction,
   PrintCovPointsAction,
   StatsAction,
   SymbolizeAction
 };
 
-cl::opt<ActionType> Action(
-    cl::desc("Action (required)"), cl::Required,
-    cl::values(
-        clEnumValN(PrintAction, "print", "Print coverage addresses"),
-        clEnumValN(PrintCovPointsAction, "print-coverage-pcs",
-                   "Print coverage instrumentation points addresses."),
-        clEnumValN(CoveredFunctionsAction, "covered-functions",
-                   "Print all covered funcions."),
-        clEnumValN(NotCoveredFunctionsAction, "not-covered-functions",
-                   "Print all not covered funcions."),
-        clEnumValN(StatsAction, "print-coverage-stats",
-                   "Print coverage statistics."),
-        clEnumValN(HtmlReportAction, "html-report",
-                   "REMOVED. Use -symbolize & coverage-report-server.py."),
-        clEnumValN(SymbolizeAction, "symbolize",
-                   "Produces a symbolized JSON report from binary report."),
-        clEnumValN(MergeAction, "merge", "Merges reports.")),
-    cl::cat(Cat));
-
-static cl::list<std::string>
-    ClInputFiles(cl::Positional, cl::OneOrMore,
-                 cl::desc("<action> <binary files...> <.sancov files...> "
-                          "<.symcov files...>"),
-                 cl::cat(Cat));
-
-static cl::opt<bool> ClDemangle("demangle", cl::init(true),
-                                cl::desc("Print demangled function name"),
-                                cl::cat(Cat));
-
-static cl::opt<bool>
-    ClSkipDeadFiles("skip-dead-files", cl::init(true),
-                    cl::desc("Do not list dead source files in reports"),
-                    cl::cat(Cat));
-
-static cl::opt<std::string>
-    ClStripPathPrefix("strip_path_prefix", cl::init(""),
-                      cl::desc("Strip this prefix from file paths in reports"),
-                      cl::cat(Cat));
-
-static cl::opt<std::string>
-    ClIgnorelist("ignorelist", cl::init(""),
-                 cl::desc("Ignorelist file (sanitizer ignorelist format)"),
-                 cl::cat(Cat));
-
-static cl::opt<bool> ClUseDefaultIgnorelist(
-    "use_default_ignorelist", cl::init(true), cl::Hidden,
-    cl::desc("Controls if default ignorelist should be used"), cl::cat(Cat));
+static ActionType Action;
+static std::vector<std::string> ClInputFiles;
+static bool ClDemangle;
+static bool ClSkipDeadFiles;
+static bool ClUseDefaultIgnorelist;
+static std::string ClStripPathPrefix;
+static std::string ClIgnorelist;
 
 static const char *const DefaultIgnorelistStr = "fun:__sanitizer_.*\n"
                                                 "src:/usr/include/.*\n"
                                                 "src:.*/libc\\+\\+/.*\n";
 
 // --------- FORMAT SPECIFICATION ---------
 
 struct FileHeader {
findSanitizerCovFunctions(const object::ObjectFile &O) {
   }
 
   return Result;
 }
 
 // Ported from
 // compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.h:GetPreviousInstructionPc
 // GetPreviousInstructionPc.
-static uint64_t getPreviousInstructionPc(uint64_t PC,
-                                         Triple TheTriple) {
+static uint64_t getPreviousInstructionPc(uint64_t PC, Triple TheTriple) {
   if (TheTriple.isARM())
     return (PC - 3) & (~1);
   if (TheTriple.isMIPS() || TheTriple.isSPARC())
     return PC - 8;
   if (TheTriple.isRISCV())
     return PC - 2;
   if (TheTriple.isX86() || TheTriple.isSystemZ())
     return PC - 1;
std::vector<std::unique_ptr<SymbolizedCoverage>> Coverages;
     }
   }
 
   return merge(Coverages);
 }
 
 } // namespace
 
-int main(int Argc, char **Argv) {
-  llvm::InitLLVM X(Argc, Argv);
-  cl::HideUnrelatedOptions(Cat);
-
-  llvm::InitializeAllTargetInfos();
-  llvm::InitializeAllTargetMCs();
-  llvm::InitializeAllDisassemblers();
+static void parseArgs(int Argc, char **Argv) {
+  SancovOptTable Tbl;
+  llvm::BumpPtrAllocator A;
+  llvm::StringSaver Saver{A};
+  opt::InputArgList Args =
+      Tbl.parseArgs(Argc, Argv, OPT_UNKNOWN, Saver, [&](StringRef Msg) {
+        llvm::errs() << Msg << '\n';
+        std::exit(1);
+      });
 
-  cl::ParseCommandLineOptions(Argc, Argv,
+  if (Args.hasArg(OPT_help)) {
+    Tbl.printHelp(
+        llvm::outs(),
+        "sancov [options] <action> <binary files...> <.sancov files...> "
+        "<.symcov files...>",
         "Sanitizer Coverage Processing Tool (sancov)\n\n"
         "  This tool can extract various coverage-related information from: \n"
         "  coverage-instrumented binary files, raw .sancov files and their "
         "symbolized .symcov version.\n"
         "  Depending on chosen action the tool expects different input files:\n"
         "    -print-coverage-pcs     - coverage-instrumented binary files\n"
         "    -print-coverage         - .sancov files\n"
         "    <other actions>         - .sancov files & corresponding binary "
-      "files, .symcov files\n"
-      );
+        "files, .symcov files\n");
+    std::exit(0);
+  }
+
+  if (Args.hasArg(OPT_version)) {
+    cl::PrintVersionMessage();
+    std::exit(0);
+  }
+
+  if (Args.hasMultipleArgs(OPT_action_grp)) {
+    fail("Only one action option is allowed");
+  }
+
+  for (const opt::Arg *A : Args.filtered(OPT_INPUT)) {
+    ClInputFiles.emplace_back(A->getValue());
+  }
+
+  if (const llvm::opt::Arg *A = Args.getLastArg(OPT_action_grp)) {
+    switch (A->getOption().getID()) {
+    case OPT_print:
+      Action = ActionType::PrintAction;
+      break;
+    case OPT_printCoveragePcs:
+      Action = ActionType::PrintCovPointsAction;
+      break;
+    case OPT_coveredFunctions:
+      Action = ActionType::CoveredFunctionsAction;
+      break;
+    case OPT_notCoveredFunctions:
+      Action = ActionType::NotCoveredFunctionsAction;
+      break;
+    case OPT_printCoverageStats:
+      Action = ActionType::StatsAction;
+      break;
+    case OPT_htmlReport:
+      Action = ActionType::HtmlReportAction;
+      break;
+    case OPT_symbolize:
+      Action = ActionType::SymbolizeAction;
+      break;
+    case OPT_merge:
+      Action = ActionType::MergeAction;
+      break;
+    default:
+      fail("Invalid Action");
+    }
+  }
+
+  ClDemangle = Args.hasFlag(OPT_demangle, OPT_no_demangle, true);
+  ClSkipDeadFiles = Args.hasFlag(OPT_skipDeadFiles, OPT_no_skipDeadFiles, true);
+  ClUseDefaultIgnorelist =
+      Args.hasFlag(OPT_useDefaultIgnoreList, OPT_no_useDefaultIgnoreList, true);
+
+  ClStripPathPrefix = Args.getLastArgValue(OPT_stripPathPrefix_EQ);
+  ClIgnorelist = Args.getLastArgValue(OPT_ignorelist_EQ);
+}
+
+int main(int Argc, char **Argv) {
+  llvm::InitLLVM X(Argc, Argv);
+
+  llvm::InitializeAllTargetInfos();
+  llvm::InitializeAllTargetMCs();
+  llvm::InitializeAllDisassemblers();
+
+  parseArgs(Argc, Argv);
 
   // -print doesn't need object files.
   if (Action == PrintAction) {
     readAndPrintRawCoverage(ClInputFiles, outs());
     return 0;
-  } else if (Action == PrintCovPointsAction) {
+  }
+  if (Action == PrintCovPointsAction) {
     // -print-coverage-points doesn't need coverage files.
     for (const std::string &ObjFile : ClInputFiles) {
       printCovPoints(ObjFile, outs());
     }
     return 0;
   }
 
   auto Coverage = readSymbolizeAndMergeCmdArguments(ClInputFiles);

llvm/utils/gn/secondary/llvm/tools/sancov/BUILD.gn

+import("//llvm/utils/TableGen/tablegen.gni")
+
+tablegen("Opts") {
+  visibility = [ ":sancov" ]
+  args = [ "-gen-opt-parser-defs" ]
+}
+
 executable("sancov") {
   deps = [
+    ":Opts",
     "//llvm/lib/DebugInfo/DWARF",
     "//llvm/lib/DebugInfo/PDB",
     "//llvm/lib/DebugInfo/Symbolize",
     "//llvm/lib/MC",
     "//llvm/lib/MC/MCDisassembler",
     "//llvm/lib/Object",
+    "//llvm/lib/Option",
     "//llvm/lib/Support",
     "//llvm/lib/Target:AllTargetsDescs",
     "//llvm/lib/Target:AllTargetsDisassemblers",
     "//llvm/lib/Target:AllTargetsInfos",
     "//llvm/lib/TargetParser",
   ]
   sources = [ "sancov.cpp" ]
 }

utils/bazel/llvm-project-overlay/llvm/BUILD.bazel

deps = [
         ":Target",
         ":TargetParser",
         ":TransformUtils",
         ":common_transforms",
         ":config",
     ],
 )
 
+gentbl(
+    name = "SancovOptsTableGen",
+    strip_include_prefix = "tools/sancov",
+    tbl_outs = [(
+        "-gen-opt-parser-defs",
+        "tools/sancov/Opts.inc",
+    )],
+    tblgen = ":llvm-tblgen",
+    td_file = "tools/sancov/Opts.td",
+    td_srcs = ["include/llvm/Option/OptParser.td"],
+)
+
 cc_binary(
     name = "sancov",
     srcs = glob([
         "tools/sancov/*.cpp",
         "tools/sancov/*.h",
     ]),
     copts = llvm_copts,
     stamp = 0,
     deps = [
         ":AllTargetsCodeGens",
         ":AllTargetsDisassemblers",
         ":DebugInfoDWARF",
         ":DebugInfoPDB",
         ":MC",
         ":MCDisassembler",
         ":Object",
+        ":Option",
+        ":SancovOptsTableGen",
         ":Support",
         ":Symbolize",
     ],
 )
 
 cc_binary(
     name = "sanstats",
     srcs = glob([