[dsymutil] Add -verify option to run DWARF verifier after linking.

[JDevlieghere]

Summary

This patch adds support for running the DWARF verifier on the linked
debug info files. If the -verify options is specified and verification
fails, dsymutil exists with abort with non-zero exit code. This behavior
is *not* enabled by default.

[vsk]

Looks good to me.

test/tools/dsymutil/X86/verify.test

4 ↗

(On Diff #125294)

Why are the copies needed?

[probinson]

tools/dsymutil/dsymutil.cpp

105 ↗

(On Diff #125294)

This looks like an unrelated formatting change?
Also later on a couple of extra blank lines were removed. I'm guessing you ran clang-format on the whole file instead of using clang-format-diff. Generally we prefer whitespace/formatting changes to be done separately from functional changes.

[JDevlieghere]

test/tools/dsymutil/X86/verify.test

4 ↗

(On Diff #125294)

Thanks! I had copied this from another test where the flat mode mattered and output was written to the temp folder. It's not necessary for this test so we don't need the copies.

tools/dsymutil/dsymutil.cpp

105 ↗

(On Diff #125294)

Jup, this was unintentional. In addition to git clang-format before uploading the patch, I usually format new code in my editor by visually selecting it. However if you're in normal mode, it formats the whole file which is what must have happened here.

[JDevlieghere]

• Feedback @vsk and @probinson

[aprantl]

tools/dsymutil/dsymutil.cpp

210 ↗

(On Diff #125535)

should this be gated by --verbose ?

[JDevlieghere]

• Feedback @aprantl

llvm/trunk/test/tools/dsymutil/X86/verify.test

+# Multiple inputs in flat mode
+RUN: not llvm-dsymutil -verify -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-prefix=QUIET
+RUN: not llvm-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-prefix=QUIET --check-prefix=VERBOSE
+
+VERBOSE: Verifying DWARF for architecture: x86_64
+VERBOSE: error: DIE has invalid DW_AT_location encoding:
+VERBOSE: error: DIE has invalid DW_AT_location encoding:
+QUIET: error: verification failed

llvm/trunk/test/tools/dsymutil/cmdline.test

 RUN: llvm-dsymutil -help 2>&1 | FileCheck --check-prefix=HELP %s
 HELP: OVERVIEW: manipulate archived DWARF debug symbol files.
 HELP: USAGE: llvm-dsymutil{{[^ ]*}} [options] <input files>
 HELP-NOT: -reverse-iterate
 HELP: Specific Options:
 HELP: -arch=<arch>
 HELP: -dump-debug-map
 HELP: -flat
 HELP: -no-odr
 HELP: -no-output
 HELP: -no-swiftmodule-timestamp
 HELP: -num-threads=<n>
 HELP: -o=<filename>
 HELP: -oso-prepend-path=<path>
 HELP: -symtab
 HELP: -verbose
+HELP: -verify
 HELP: -y
 HELP-NOT: -reverse-iterate
 
 RUN: llvm-dsymutil --version 2>&1 | FileCheck --check-prefix=VERSION %s
 VERSION: {{ version }}

llvm/trunk/tools/dsymutil/dsymutil.cpp

 #include "dsymutil.h"
 #include "CFBundle.h"
 #include "DebugMap.h"
 #include "MachOUtils.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/DebugInfo/DIContext.h"
+#include "llvm/DebugInfo/DWARF/DWARFContext.h"
+#include "llvm/DebugInfo/DWARF/DWARFVerifier.h"
+#include "llvm/Object/Binary.h"
 #include "llvm/Object/MachO.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/thread.h"
 #include <algorithm>
 #include <cstdint>
 #include <cstdlib>
 #include <string>
 #include <system_error>
 
 using namespace llvm;
 using namespace llvm::cl;
 using namespace llvm::dsymutil;
+using namespace object;
 
 static OptionCategory DsymCategory("Specific Options");
 static opt<bool> Help("h", desc("Alias for -help"), Hidden);
 static opt<bool> Version("v", desc("Alias for -version"), Hidden);
 
 static list<std::string> InputFiles(Positional, OneOrMore,
                                     desc("<input files>"), cat(DsymCategory));
 
static opt<bool> DumpDebugMap(
     desc("Parse and dump the debug map to standard output. Not DWARF link "
          "will take place."),
     init(false), cat(DsymCategory));
 
 static opt<bool> InputIsYAMLDebugMap(
     "y", desc("Treat the input file is a YAML debug map rather than a binary."),
     init(false), cat(DsymCategory));
 
+static opt<bool> Verify("verify", desc("Verify the linked DWARF debug info."),
+                        cat(DsymCategory));
+
 static bool createPlistFile(llvm::StringRef Bin, llvm::StringRef BundleRoot) {
   if (NoOutput)
     return true;
 
   // Create plist file to write to.
   llvm::SmallString<128> InfoPlist(BundleRoot);
   llvm::sys::path::append(InfoPlist, "Contents/Info.plist");
   std::error_code EC;
if (std::error_code EC = create_directories(Bundle.str(), true,
                                               llvm::sys::fs::perms::all_all)) {
     llvm::errs() << "error: cannot create directory " << Bundle << ": "
                  << EC.message() << "\n";
     return false;
   }
   return true;
 }
 
+static bool verify(llvm::StringRef OutputFile, llvm::StringRef Arch) {
+  if (OutputFile == "-") {
+    llvm::errs() << "warning: verification skipped for " << Arch
+                 << "because writing to stdout.\n";
+    return true;
+  }
+
+  Expected<OwningBinary<Binary>> BinOrErr = createBinary(OutputFile);
+  if (!BinOrErr) {
+    errs() << OutputFile << ": " << toString(BinOrErr.takeError());
+    return false;
+  }
+
+  Binary &Binary = *BinOrErr.get().getBinary();
+  if (auto *Obj = dyn_cast<MachOObjectFile>(&Binary)) {
+    raw_ostream &os = Verbose ? errs() : nulls();
+    os << "Verifying DWARF for architecture: " << Arch << "\n";
+    std::unique_ptr<DWARFContext> DICtx = DWARFContext::create(*Obj);
+    DIDumpOptions DumpOpts;
+    bool success = DICtx->verify(os, DumpOpts.noImplicitRecursion());
+    if (!success)
+        errs() << "error: verification failed for " << Arch << '\n';
+    return success;
+  }
+
+  return false;
+}
+
 static std::string getOutputFileName(llvm::StringRef InputFile) {
   if (FlatOut) {
     // If a flat dSYM has been requested, things are pretty simple.
     if (OutputFileOpt.empty()) {
       if (InputFile == "-")
         return "a.out.dwarf";
       return (InputFile + ".dwarf").str();
     }
for (auto &Map : *DebugMapPtrsOrErr) {
           errs() << OutputFile << ": " << EC.message();
           return 1;
         }
       }
 
       std::atomic_char AllOK(1);
       auto LinkLambda = [&]() {
         AllOK.fetch_and(linkDwarf(*OS, *Map, Options));
+        OS->flush();
       };
 
       // FIXME: The DwarfLinker can have some very deep recursion that can max
       // out the (significantly smaller) stack when using threads. We don't
       // want this limitation when we only have a single thread.
       if (NumThreads == 1) {
         LinkLambda();
       } else {
         llvm::ThreadPool Threads(NumThreads);
         Threads.async(LinkLambda);
         Threads.wait();
       }
       if (!AllOK)
         return 1;
 
+      if (Verify && !NoOutput &&
+          !verify(OutputFile, Map->getTriple().getArchName()))
+        return 1;
+
       if (NeedsTempFiles)
         TempFiles.emplace_back(Map->getTriple().getArchName().str(),
                                OutputFile);
     }
 
 
     if (NeedsTempFiles &&
         !MachOUtils::generateUniversalBinary(
             TempFiles, getOutputFileName(InputFile), Options, SDKPath))
       return 1;
   }
 
   return 0;
 }