[dsymutil] Add the ability to run the DWARF verifier on the input

JDevlieghere · JDevlieghere · commit 1027992dbe0f · 2022-02-14T16:14:41.000-08:00
Currently you can run the DWARF verifier on the linked dsymutil output. This patch extends this functionality and makes it possible to run the DWARF verifier on the input as well. A new option --verify-dwarf allows you to specify input, output, all and none. The existing --verify flag remains unchanged and acts and alias for --verify-dwarf=output. Input verification issues do not result in a non-zero exit code because dsymutil is capable of taking invalid DWARF as input and producing valid DWARF as output. Differential revision: https://reviews.llvm.org/D89216 (cherry picked from commit 409c515)
diff --git a/llvm/include/llvm/DWARFLinker/DWARFLinker.h b/llvm/include/llvm/DWARFLinker/DWARFLinker.h
@@ -272,6 +272,9 @@ class DWARFLinker {
   /// Print statistics to standard output.
   void setStatistics(bool Statistics) { Options.Statistics = Statistics; }
 
+  /// Verify the input DWARF.
+  void setVerifyInputDWARF(bool Verify) { Options.VerifyInputDWARF = Verify; }
+
   /// Do not emit linked dwarf info.
   void setNoOutput(bool NoOut) { Options.NoOutput = NoOut; }
 
@@ -389,6 +392,9 @@ class DWARFLinker {
           Flags(Flags), AncestorIdx(AncestorIdx) {}
   };
 
+  /// Verify the given DWARF file.
+  bool verify(const DWARFFile &File);
+
   /// returns true if we need to translate strings.
   bool needToTranslateStrings() { return StringsTranslator != nullptr; }
 
@@ -778,6 +784,9 @@ class DWARFLinker {
     /// Print statistics.
     bool Statistics = false;
 
+    /// Verify the input DWARF.
+    bool VerifyInputDWARF = false;
+
     /// Skip emitting output
     bool NoOutput = false;
 
diff --git a/llvm/lib/DWARFLinker/DWARFLinker.cpp b/llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -2358,6 +2358,10 @@ bool DWARFLinker::link() {
 
     if (!OptContext.File.Dwarf)
       continue;
+
+    if (Options.VerifyInputDWARF)
+      verify(OptContext.File);
+
     // Look for relocations that correspond to address map entries.
 
     // there was findvalidrelocations previously ... probably we need to gather
@@ -2627,4 +2631,15 @@ bool DWARFLinker::link() {
   return true;
 }
 
+bool DWARFLinker::verify(const DWARFFile &File) {
+  assert(File.Dwarf);
+
+  DIDumpOptions DumpOpts;
+  if (!File.Dwarf->verify(llvm::outs(), DumpOpts.noImplicitRecursion())) {
+    reportWarning("input verification failed", File);
+    return false;
+  }
+  return true;
+}
+
 } // namespace llvm
diff --git a/llvm/test/tools/dsymutil/X86/verify.test b/llvm/test/tools/dsymutil/X86/verify.test
@@ -3,14 +3,27 @@
 # RUN: dsymutil -verify -verbose -oso-prepend-path=%p/.. %p/../Inputs/basic.macho.x86_64 %p/../Inputs/basic-archive.macho.x86_64 %p/../Inputs/basic-lto.macho.x86_64 %p/../Inputs/basic-lto-dw4.macho.x86_64 -o %t 2>&1 | FileCheck %s --check-prefixes=QUIET-SUCCESS,VERBOSE
 
 # VERBOSE: Verifying DWARF for architecture: x86_64
-# QUIET-SUCCESS-NOT: error: verification failed
+# QUIET-SUCCESS-NOT: error: output verification failed
 
-# Negative tests in regular and verbose mode.
+# Negative output tests in regular and verbose mode.
 # (Invalid object generated from ../Inputs/invalid.s by modified the low PC.)
-# RUN: not dsymutil -verify -oso-prepend-path=%p/../Inputs -y %s -o %t 2>&1 | FileCheck %s --check-prefix=QUIET-FAIL
-# RUN: not dsymutil -verify -verbose -oso-prepend-path=%p/../Inputs -y %s -o %t 2>&1 | FileCheck %s --check-prefixes=QUIET-FAIL,VERBOSE
+# RUN: not dsymutil -verify -oso-prepend-path=%p/../Inputs -y %s -o %t 2>&1 | FileCheck %s --check-prefix=QUIET-OUTPUT-FAIL
+# RUN: not dsymutil -verify-dwarf=output -oso-prepend-path=%p/../Inputs -y %s -o %t 2>&1 | FileCheck %s --check-prefix=QUIET-OUTPUT-FAIL
+# RUN: not dsymutil -verify -verbose -oso-prepend-path=%p/../Inputs -y %s -o %t 2>&1 | FileCheck %s --check-prefixes=QUIET-OUTPUT-FAIL,VERBOSE
 
-# QUIET-FAIL: error: verification failed
+# Negative input & output tests in regular and verbose mode. Only output failures result in a non-zero exit code.
+# RUN: dsymutil -verify-dwarf=input -oso-prepend-path=%p/../Inputs -y %s -o %t 2>&1 | FileCheck %s --check-prefix=QUIET-INPUT-FAIL
+# RUN: dsymutil -verify-dwarf=input -verbose -oso-prepend-path=%p/../Inputs -y %s -o %t 2>&1 | FileCheck %s --check-prefix=QUIET-INPUT-FAIL
+# RUN: dsymutil -verify-dwarf=none -verbose -oso-prepend-path=%p/../Inputs -y %s -o %t 2>&1 | FileCheck %s --check-prefixes=QUIET-SUCCESS
+# RUN: not dsymutil -verify-dwarf=bogus -verbose -oso-prepend-path=%p/../Inputs -y %s -o %t 2>&1 | FileCheck %s --check-prefixes=BOGUS
+# RUN: not dsymutil -verify-dwarf=all -oso-prepend-path=%p/../Inputs -y %s -o %t 2>&1 | FileCheck %s --check-prefixes=QUIET-OUTPUT-FAIL,QUIET-INPUT-FAIL,VERBOSE-INPUT-FAIL
+
+# VERBOSE-INPUT-FAIL: error: Abbreviation declaration contains multiple DW_AT_language attributes.
+# QUIET-INPUT-FAIL: warning: input verification failed
+# QUIET-OUTPUT-FAIL: error: output verification failed
+# QUIET-SUCCESS-NOT: input verification failed
+# QUIET-SUCCESS-NOT: output verification failed
+# BOGUS: error: invalid verify type specified: 'bogus'
 
 ---
 triple:          'x86_64-apple-darwin'
diff --git a/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp b/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
@@ -391,6 +391,7 @@ bool DwarfLinkerForBinary::link(const DebugMap &Map) {
 
   GeneralLinker.setVerbosity(Options.Verbose);
   GeneralLinker.setStatistics(Options.Statistics);
+  GeneralLinker.setVerifyInputDWARF(Options.VerifyInputDWARF);
   GeneralLinker.setNoOutput(Options.NoOutput);
   GeneralLinker.setNoODR(Options.NoODR);
   GeneralLinker.setUpdate(Options.Update);
diff --git a/llvm/tools/dsymutil/LinkUtils.h b/llvm/tools/dsymutil/LinkUtils.h
@@ -30,6 +30,9 @@ struct LinkOptions {
   /// Statistics
   bool Statistics = false;
 
+  /// Verify the input DWARF.
+  bool VerifyInputDWARF = false;
+
   /// Skip emitting output
   bool NoOutput = false;
 
diff --git a/llvm/tools/dsymutil/Options.td b/llvm/tools/dsymutil/Options.td
@@ -38,9 +38,15 @@ def statistics: F<"statistics">,
   Group<grp_general>;
 
 def verify: F<"verify">,
-  HelpText<"Run the DWARF verifier on the linked DWARF debug info.">,
+  HelpText<"Alias for --verify-dwarf=output">,
   Group<grp_general>;
 
+def verify_dwarf: Separate<["--", "-"], "verify-dwarf">,
+  MetaVarName<"<verification mode>">,
+  HelpText<"Run the DWARF verifier on the input and/or output. Valid options are 'input', 'output', 'all' or 'none'.">,
+  Group<grp_general>;
+def: Joined<["--", "-"], "verify-dwarf=">, Alias<verify_dwarf>;
+
 def no_output: F<"no-output">,
   HelpText<"Do the link in memory, but do not emit the result file.">,
   Group<grp_general>;
diff --git a/llvm/tools/dsymutil/dsymutil.cpp b/llvm/tools/dsymutil/dsymutil.cpp
@@ -84,13 +84,23 @@ class DsymutilOptTable : public opt::OptTable {
 };
 } // namespace
 
+enum class DWARFVerify : uint8_t {
+  None = 0,
+  Input = 1 << 0,
+  Output = 1 << 1,
+  All = Input | Output,
+};
+
+inline bool flagIsSet(DWARFVerify Flags, DWARFVerify SingleFlag) {
+  return static_cast<uint8_t>(Flags) & static_cast<uint8_t>(SingleFlag);
+}
+
 struct DsymutilOptions {
   bool DumpDebugMap = false;
   bool DumpStab = false;
   bool Flat = false;
   bool InputIsYAMLDebugMap = false;
   bool PaperTrailWarnings = false;
-  bool Verify = false;
   bool ForceKeepFunctionForStatic = false;
   std::string SymbolMap;
   std::string OutputFile;
@@ -99,6 +109,7 @@ struct DsymutilOptions {
   std::vector<std::string> Archs;
   std::vector<std::string> InputFiles;
   unsigned NumThreads;
+  DWARFVerify Verify = DWARFVerify::None;
   ReproducerMode ReproMode = ReproducerMode::Off;
   dsymutil::LinkOptions LinkOpts;
 };
@@ -213,6 +224,27 @@ static Expected<AccelTableKind> getAccelTableKind(opt::InputArgList &Args) {
   return AccelTableKind::Default;
 }
 
+static Expected<DWARFVerify> getVerifyKind(opt::InputArgList &Args) {
+  if (Args.hasArg(OPT_verify))
+    return DWARFVerify::Output;
+  if (opt::Arg *Verify = Args.getLastArg(OPT_verify_dwarf)) {
+    StringRef S = Verify->getValue();
+    if (S == "input")
+      return DWARFVerify::Input;
+    if (S == "output")
+      return DWARFVerify::Output;
+    if (S == "all")
+      return DWARFVerify::All;
+    if (S == "none")
+      return DWARFVerify::None;
+    return make_error<StringError>(
+        "invalid verify type specified: '" + S +
+            "'. Support values are 'input', 'output', 'all' and 'none'.",
+        inconvertibleErrorCode());
+  }
+  return DWARFVerify::None;
+}
+
 /// Parses the command line options into the LinkOptions struct and performs
 /// some sanity checking. Returns an error in case the latter fails.
 static Expected<DsymutilOptions> getOptions(opt::InputArgList &Args) {
@@ -223,9 +255,16 @@ static Expected<DsymutilOptions> getOptions(opt::InputArgList &Args) {
   Options.Flat = Args.hasArg(OPT_flat);
   Options.InputIsYAMLDebugMap = Args.hasArg(OPT_yaml_input);
   Options.PaperTrailWarnings = Args.hasArg(OPT_papertrail);
-  Options.Verify = Args.hasArg(OPT_verify);
+
+  if (Expected<DWARFVerify> Verify = getVerifyKind(Args)) {
+    Options.Verify = *Verify;
+  } else {
+    return Verify.takeError();
+  }
 
   Options.LinkOpts.NoODR = Args.hasArg(OPT_no_odr);
+  Options.LinkOpts.VerifyInputDWARF =
+      flagIsSet(Options.Verify, DWARFVerify::Input);
   Options.LinkOpts.NoOutput = Args.hasArg(OPT_no_output);
   Options.LinkOpts.NoTimestamp = Args.hasArg(OPT_no_swiftmodule_timestamp);
   Options.LinkOpts.Update = Args.hasArg(OPT_update);
@@ -387,7 +426,7 @@ static Error createBundleDir(StringRef BundleBase) {
   return Error::success();
 }
 
-static bool verify(StringRef OutputFile, StringRef Arch, bool Verbose) {
+static bool verifyOutput(StringRef OutputFile, StringRef Arch, bool Verbose) {
   if (OutputFile == "-") {
     WithColor::warning() << "verification skipped for " << Arch
                          << "because writing to stdout.\n";
@@ -408,7 +447,7 @@ static bool verify(StringRef OutputFile, StringRef Arch, bool Verbose) {
     DIDumpOptions DumpOpts;
     bool success = DICtx->verify(os, DumpOpts.noImplicitRecursion());
     if (!success)
-      WithColor::error() << "verification failed for " << Arch << '\n';
+      WithColor::error() << "output verification failed for " << Arch << '\n';
     return success;
   }
 
@@ -612,7 +651,12 @@ int main(int argc, char **argv) {
     const bool NeedsTempFiles =
         !Options.DumpDebugMap && (Options.OutputFile != "-") &&
         (DebugMapPtrsOrErr->size() != 1 || Options.LinkOpts.Update);
-    const bool Verify = Options.Verify && !Options.LinkOpts.NoOutput;
+    bool VerifyOutput = flagIsSet(Options.Verify, DWARFVerify::Output);
+    if (VerifyOutput && Options.LinkOpts.NoOutput) {
+      WithColor::warning()
+          << "skipping output verification because --no-output was passed\n";
+      VerifyOutput = false;
+    }
 
     SmallVector<MachOUtils::ArchAndFile, 4> TempFiles;
     std::atomic_char AllOK(1);
@@ -664,9 +708,10 @@ int main(int argc, char **argv) {
         AllOK.fetch_and(
             linkDwarf(*Stream, BinHolder, *Map, std::move(Options)));
         Stream->flush();
-        if (Verify)
-          AllOK.fetch_and(verify(OutputFile, Map->getTriple().getArchName(),
-                                 Options.Verbose));
+        if (VerifyOutput) {
+          AllOK.fetch_and(verifyOutput(
+              OutputFile, Map->getTriple().getArchName(), Options.Verbose));
+        }
       };
 
       // FIXME: The DwarfLinker can have some very deep recursion that can max
