[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

Why are the copies needed?

[probinson]

tools/dsymutil/dsymutil.cpp

105

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

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

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

should this be gated by --verbose ?

[JDevlieghere]

• Feedback @aprantl

test/tools/dsymutil/X86/verify.test

+RUN: cat %p/../Inputs/basic.macho.x86_64 > %T/basic.macho.x86_64
+RUN: cat %p/../Inputs/basic-archive.macho.x86_64 > %T/basic-archive.macho.x86_64
+RUN: cat %p/../Inputs/basic-lto.macho.x86_64 > %T/basic-lto.macho.x86_64
+RUN: cat %p/../Inputs/basic-lto-dw4.macho.x86_64 > %T/basic-lto-dw4.macho.x86_64
+
+# Multiple inputs in flat mode
+RUN: not llvm-dsymutil -verify -f -oso-prepend-path=%p/.. %T/basic.macho.x86_64 %T/basic-archive.macho.x86_64 %T/basic-lto.macho.x86_64 %T/basic-lto-dw4.macho.x86_64 2>&1 | FileCheck %s
+
+CHECK: error: DIE has invalid DW_AT_location encoding:
+CHECK: error: DIE has invalid DW_AT_location encoding:

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 }}

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));
 
NoTimestamp("no-swiftmodule-timestamp",
                 desc("Don't check timestamp for swiftmodule files."),
                 init(false), cat(DsymCategory));
 
 static list<std::string> ArchFlags(
     "arch",
     desc("Link DWARF debug information only for specified CPU architecture\n"
          "types. This option can be specified multiple times, once for each\n"
          "desired architecture. All CPU architectures will be linked by\n"
-         "default."), value_desc("arch"),
-    ZeroOrMore, cat(DsymCategory));
+         "default."),
+    value_desc("arch"), ZeroOrMore, cat(DsymCategory));
 
 static opt<bool>
     NoODR("no-odr",
           desc("Do not use ODR (One Definition Rule) for type uniquing."),
           init(false), cat(DsymCategory));
 
 static opt<bool> DumpDebugMap(
     "dump-debug-map",
     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)) {
+    errs() << "Verifying DWARF for architecture: " << Arch << "\n";
+    std::unique_ptr<DWARFContext> DICtx = DWARFContext::create(*Obj);
+    DIDumpOptions DumpOpts;
+    return DICtx->verify(errs(), DumpOpts.noImplicitRecursion());
+  }
+
+  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 &InputFile : InputFiles) {
     }
 
     if (NumThreads == 0)
       NumThreads = llvm::thread::hardware_concurrency();
     if (DumpDebugMap || Verbose)
       NumThreads = 1;
     NumThreads = std::min<unsigned>(NumThreads, DebugMapPtrsOrErr->size());
 
-
     // If there is more than one link to execute, we need to generate
     // temporary files.
     bool NeedsTempFiles = !DumpDebugMap && (*DebugMapPtrsOrErr).size() != 1;
     llvm::SmallVector<MachOUtils::ArchAndFilename, 4> TempFiles;
     TempFileVector TempFileStore;
     for (auto &Map : *DebugMapPtrsOrErr) {
       if (Verbose || DumpDebugMap)
         Map->print(llvm::outs());
for (auto &Map : *DebugMapPtrsOrErr) {
           return 1;
         }
         OS = llvm::make_unique<raw_fd_ostream>(T->FD, /*shouldClose*/ false);
         OutputFile = T->TmpName;
         TempFileStore.Files.push_back(std::move(*T));
       } else {
         std::error_code EC;
         OS = llvm::make_unique<raw_fd_ostream>(NoOutput ? "-" : OutputFile, EC,
                                                sys::fs::F_None);
         if (EC) {
           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;
 }