Merge pull request #3943 from apple/🍒/austria/409c515f3f9e2228ee10648011943d307ccd8860

JDevlieghere · web-flow · commit 1df72aec83b0 · 2022-02-15T08:24:06.000-08:00
[dsymutil] Add the ability to run the DWARF verifier on the input
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
