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

[JDevlieghere]

Summary

It's already possible to 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.

[avl]

llvm/tools/dsymutil/Options.td

37

there is an extra dash here: ---verify-dwarf

[avl]

llvm/tools/dsymutil/BinaryHolder.cpp

274

I have two suggestions/questions: 1. It would probably be better to do input verification inside DWARFLinker library(DWARFLinker::link()). Then all users of the library will be able to use this functionality. 2. Would it be useful to have failure in verification to be fatal error ?

llvm/tools/dsymutil/dsymutil.cpp

645

It makes sense to display message to the user when Options.Verify is ignored because of Options.LinkOpts.NoOutput. Probably verifyOptions() is a good place for it.

[JDevlieghere]

llvm/tools/dsymutil/BinaryHolder.cpp

274

I think moving it into the DWARFLinker is a good idea, we already have a DWARFContext there.

I don't think we should make input verification failure a fatal error. Depending on the issue dsymutil is capable of generating a valid dSYM from invalid DWARF.

[avl]

llvm/tools/dsymutil/BinaryHolder.cpp

274

I don't think we should make input verification failure a fatal error. Depending on the issue dsymutil is capable of generating a valid dSYM from invalid DWARF.

There is a final set of errors which dsymutil could cure and generate a valid DWARF. The example of such errors is "Overlapping address range". Before dsymutil, there are a lot of address ranges starting from zero(these are address ranges related to deleted code). So input verification would report error "Overlapping address range". dsymutil removes DWARF which uses these address ranges and final DWARF become valid. Other kinds of errors could not be cured by dsymutil. And then it would be useful to fail if input contains such errors.

Probably, we could add a parameter to the DWARFContext.verify() - SkipDWARLinkerKnownErrors. So that if verifier encounter only known errors it would report success, otherwise it would report failure and dsymutil would fail during input validation ?

It looks useful if dsymutil would fail on input verification if input DWARF contains errors which could not be fixed/correctly processed by dsymutil.

[clayborg]

I really like this patch as it will make detecting LTO issues easier as there are often issues in the incoming .o files.

One thing to think about is we want to only verify unique object files one time. In older LTO based code I remember seeing that same "lto.o" file in the debug map multiple time with many different sources file (N_SO) entries with the same path to the same .o file (N_OSO). If this would happen, would this patch end up running the verifier on the same file over and over?

llvm/tools/dsymutil/BinaryHolder.cpp

274

I believe that detection of overlapping address ranges is disabled or .o files since it is known to have unrelocated addresses.

llvm/tools/dsymutil/Options.td

45

If we aren't going to have these errors be fatal, can we possibly add an option that _can_ make verification errors cause it to be?

--verify-errors-fatal

[avl]

llvm/tools/dsymutil/BinaryHolder.cpp

274

probably, yes. My point is that we can ignore errors, which could be cured by dsymutil, and we could stop verifying if the error, which could not be fixed by dsymutil, was met.

[JDevlieghere]

• Rebase
• Move verification into the DWARFLinker

[avl]

llvm/include/llvm/DWARFLinker/DWARFLinker.h

275–276 ↗

(On Diff #407767)

It looks useful to explicitly show that library verifies input DWARF.

787–788 ↗

(On Diff #407767)

It looks useful to explicitly show that library verifies input DWARF.

llvm/lib/DWARFLinker/DWARFLinker.cpp

2643 ↗

(On Diff #407767)

It is probably better to use DWARFLinker::reportWarning method instead of direct call to WithColor::warning().

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

18

it would be good to test -verify-dwarf=none and -verify-dwarf=bad_value.

llvm/tools/dsymutil/LinkUtils.h

33–34 ↗

(On Diff #407767)

llvm/tools/dsymutil/dsymutil.cpp

88

it is not expected to have a lot of values here. Probably, uint8_t instead of unsigned would be better.

91

All = Input | Output,

may be useful later in getVerifyKind().

271

static_cast<bool>(Options.Verify & DWARFVerify::Input);

looks overweight. Probably implement a helper ?:

inline bool FlagIsSet (DWARFVerify Flags, DWARFVerify SingleFlag) {
  return static_cast<uint8_t>(Flags) & static_cast<uint8_t>(SingleFlag);
}

Options.LinkOpts.Verify = FlagIsSet(Options.Verify, DWARFVerify::Input);

[JDevlieghere]

Thanks for the prompt review, Alexey! I think I've addressed all your comments.

[avl]

Thank you Jonas! There are still several inline nits...

llvm/include/llvm/DWARFLinker/DWARFLinker.h

787 ↗

(On Diff #407972)

llvm/tools/dsymutil/LinkUtils.h

33 ↗

(On Diff #407972)

llvm/tools/dsymutil/dsymutil.cpp

88

95

now it is not used and probably might be deleted.

100

now it is not used and probably might be deleted.

105

should start from the small letter.

240

[JDevlieghere]

Address nits

[avl]

LGTM. please wait some time if other reviewers have any comment. Thanks!

[clayborg]

Most of the patch looks fine, just wanted to make sure we aren't changing it so that "--verify" can't be specified without a now required argument as it would break people's scripts for anyone that is using this flag. We are using on all iOS DWARF output right now at Facebook/Meta. I would rather us add a new "--verify-input" that can be specified, and add leave "--verify" to mean verify the output, or make a "--verify-output" as well and alias "--verify" to that flag.

llvm/tools/dsymutil/dsymutil.cpp

262

Is this now changing it so "--verify" no longer works and now requires an additional argument? It would hate to break anyone's scripts that might be currently using the "--verify" option by changing its semantics.

[clayborg]

In D89216#3320823, @clayborg wrote:

Most of the patch looks fine, just wanted to make sure we aren't changing it so that "--verify" can't be specified without a now required argument as it would break people's scripts for anyone that is using this flag. We are using on all iOS DWARF output right now at Facebook/Meta. I would rather us add a new "--verify-input" that can be specified, and add leave "--verify" to mean verify the output, or make a "--verify-output" as well and alias "--verify" to that flag.

This will teach me to read all of the posts and the original description before commenting... If everything is as it says in the description this looks good to me.

[JDevlieghere]

Yup.

@clayborg Do you still want a flag to make verification issues fatal? I'm happy to put that in a follow up patch.

[clayborg]

In D89216#3321019, @JDevlieghere wrote:

Yup.

@clayborg Do you still want a flag to make verification issues fatal? I'm happy to put that in a follow up patch.

Actually no, so much DWARF can have issues that might not fully pass verification that I am not sure it would be usable, so lets hold off for now.

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

 # Positive tests in regular and verbose mode.
 # RUN: 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 --allow-empty --check-prefix=QUIET-SUCCESS
 # 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: 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 for {{.*}}invalid.o
+# QUIET-OUTPUT-FAIL: error: output verification failed
 
 ---
 triple:          'x86_64-apple-darwin'
 objects:
   - filename:        invalid.o
     timestamp:       1518197670
     symbols:
       - { sym: _main, objAddr: 0x0000000000000010, binAddr: 0x0000000100000FB0, size: 0x00000008 }
       - { sym: _g, objAddr: 0x0000000000000000, binAddr: 0x0000000100000FA0, size: 0x00000010 }
 ...

llvm/tools/dsymutil/BinaryHolder.h

 /// The BinaryHolder class is responsible for creating and owning
 /// ObjectFiles and their underlying MemoryBuffers. It differs from a simple
 /// OwningBinary in that it handles accessing and caching of archives and its
 /// members.
 class BinaryHolder {
 public:
   using TimestampTy = sys::TimePoint<std::chrono::seconds>;
 
-  BinaryHolder(IntrusiveRefCntPtr<vfs::FileSystem> VFS, bool Verbose = false)
-      : VFS(VFS), Verbose(Verbose) {}
+  BinaryHolder(IntrusiveRefCntPtr<vfs::FileSystem> VFS, bool Verbose = false,
+               bool Verify = false)
+      : VFS(VFS), Verbose(Verbose), Verify(Verify) {}
 
   // Forward declarations for friend declaration.
   class ObjectEntry;
   class ArchiveEntry;
 
   /// Base class shared by cached entries, representing objects and archives.
   class EntryBase {
   protected:
     std::unique_ptr<MemoryBuffer> MemBuffer;
     std::unique_ptr<object::MachOUniversalBinary> FatBinary;
     std::string FatBinaryName;
   };
 
   /// Cached entry holding one or more (in case of a fat binary) object files.
   class ObjectEntry : public EntryBase {
   public:
     /// Load the given object binary in memory.
     Error load(IntrusiveRefCntPtr<vfs::FileSystem> VFS, StringRef Filename,
-               bool Verbose = false);
+               bool Verbose = false, bool Verify = false);
 
     /// Access all owned ObjectFiles.
     std::vector<const object::ObjectFile *> getObjects() const;
 
     /// Access to a derived version of all the currently owned ObjectFiles. The
     /// conversion might be invalid, in which case an Error is returned.
     template <typename ObjectFileType>
     Expected<std::vector<const ObjectFileType *>> getObjectsAs() const {
public:
     template <typename ObjectFileType>
     Expected<const ObjectFileType &> getObjectAs(const Triple &T) const {
       auto Object = getObject(T);
       if (!Object)
         return Object.takeError();
       return cast<ObjectFileType>(*Object);
     }
 
+    void verify(bool Verbose = false) const;
+
   private:
     std::vector<std::unique_ptr<object::ObjectFile>> Objects;
     friend ArchiveEntry;
   };
 
   /// Cached entry holding one or more (in the of a fat binary) archive files.
   class ArchiveEntry : public EntryBase {
   public:
     struct KeyTy {
       std::string Filename;
       TimestampTy Timestamp;
 
       KeyTy() : Filename(), Timestamp() {}
       KeyTy(StringRef Filename, TimestampTy Timestamp)
           : Filename(Filename.str()), Timestamp(Timestamp) {}
     };
 
     /// Load the given object binary in memory.
     Error load(IntrusiveRefCntPtr<vfs::FileSystem> VFS, StringRef Filename,
-               TimestampTy Timestamp, bool Verbose = false);
+               TimestampTy Timestamp, bool Verbose = false,
+               bool Verify = false);
 
     Expected<const ObjectEntry &> getObjectEntry(StringRef Filename,
                                                  TimestampTy Timestamp,
                                                  bool Verbose = false);
 
+    void verify(bool Verbose = false) const;
+
   private:
     std::vector<std::unique_ptr<object::Archive>> Archives;
     DenseMap<KeyTy, ObjectEntry> MemberCache;
     std::mutex MemberCacheMutex;
   };
 
   Expected<const ObjectEntry &>
   getObjectEntry(StringRef Filename, TimestampTy Timestamp = TimestampTy());
private:
   /// Object entries for objects that are not in a static archive.
   StringMap<ObjectEntry> ObjectCache;
   std::mutex ObjectCacheMutex;
 
   /// Virtual File System instance.
   IntrusiveRefCntPtr<vfs::FileSystem> VFS;
 
   bool Verbose;
+  bool Verify;
 };
 
 } // namespace dsymutil
 
 template <> struct DenseMapInfo<dsymutil::BinaryHolder::ArchiveEntry::KeyTy> {
 
   static inline dsymutil::BinaryHolder::ArchiveEntry::KeyTy getEmptyKey() {
     return dsymutil::BinaryHolder::ArchiveEntry::KeyTy();

llvm/tools/dsymutil/BinaryHolder.cpp

 //===-- BinaryHolder.cpp --------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This program is a utility that aims to be a dropin replacement for
 // Darwin's dsymutil.
 //
 //===----------------------------------------------------------------------===//
 
 #include "BinaryHolder.h"
+#include "llvm/DebugInfo/DWARF/DWARFContext.h"
+#include "llvm/DebugInfo/DWARF/DWARFVerifier.h"
 #include "llvm/Object/MachO.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace llvm {
 namespace dsymutil {
 
 static std::pair<StringRef, StringRef>
for (auto It = Fat.begin_objects(), End = Fat.end_objects(); It != End;
     StringRef ObjData = FatData.substr(It->getOffset(), It->getSize());
     Buffers.emplace_back(ObjData, Filename);
   }
   return Buffers;
 }
 
 Error BinaryHolder::ArchiveEntry::load(IntrusiveRefCntPtr<vfs::FileSystem> VFS,
                                        StringRef Filename,
-                                       TimestampTy Timestamp, bool Verbose) {
+                                       TimestampTy Timestamp, bool Verbose,
+                                       bool Verify) {
   StringRef ArchiveFilename = getArchiveAndObjectName(Filename).first;
 
   // Try to load archive and force it to be memory mapped.
   auto ErrOrBuff = (ArchiveFilename == "-")
                        ? MemoryBuffer::getSTDIN()
                        : VFS->getBufferForFile(ArchiveFilename, -1, false);
   if (auto Err = ErrOrBuff.getError())
     return errorCodeToError(Err);
Error BinaryHolder::ArchiveEntry::load(IntrusiveRefCntPtr<vfs::FileSystem> VFS,
   Archives.reserve(ArchiveBuffers.size());
   for (auto MemRef : ArchiveBuffers) {
     auto ErrOrArchive = object::Archive::create(MemRef);
     if (!ErrOrArchive)
       return ErrOrArchive.takeError();
     Archives.push_back(std::move(*ErrOrArchive));
   }
 
+  if (Verify)
+    verify(Verbose);
+
   return Error::success();
 }
 
 Error BinaryHolder::ObjectEntry::load(IntrusiveRefCntPtr<vfs::FileSystem> VFS,
-                                      StringRef Filename, bool Verbose) {
+                                      StringRef Filename, bool Verbose,
+                                      bool Verify) {
   // Try to load regular binary and force it to be memory mapped.
   auto ErrOrBuff = (Filename == "-")
                        ? MemoryBuffer::getSTDIN()
                        : VFS->getBufferForFile(Filename, -1, false);
   if (auto Err = ErrOrBuff.getError())
     return errorCodeToError(Err);
 
   MemBuffer = std::move(*ErrOrBuff);
Error BinaryHolder::ObjectEntry::load(IntrusiveRefCntPtr<vfs::FileSystem> VFS,
   Objects.reserve(ObjectBuffers.size());
   for (auto MemRef : ObjectBuffers) {
     auto ErrOrObjectFile = object::ObjectFile::createObjectFile(MemRef);
     if (!ErrOrObjectFile)
       return ErrOrObjectFile.takeError();
     Objects.push_back(std::move(*ErrOrObjectFile));
   }
 
+  if (Verify)
+    verify(Verbose);
+
   return Error::success();
 }
 
 std::vector<const object::ObjectFile *>
 BinaryHolder::ObjectEntry::getObjects() const {
   std::vector<const object::ObjectFile *> Result;
   Result.reserve(Objects.size());
   for (auto &Object : Objects) {
BinaryHolder::getObjectEntry(StringRef Filename, TimestampTy Timestamp) {
   if (isArchive(Filename)) {
     StringRef ArchiveFilename = getArchiveAndObjectName(Filename).first;
     std::lock_guard<std::mutex> Lock(ArchiveCacheMutex);
     if (ArchiveCache.count(ArchiveFilename)) {
       return ArchiveCache[ArchiveFilename].getObjectEntry(Filename, Timestamp,
                                                           Verbose);
     } else {
       ArchiveEntry &AE = ArchiveCache[ArchiveFilename];
-      auto Err = AE.load(VFS, Filename, Timestamp, Verbose);
+      auto Err = AE.load(VFS, Filename, Timestamp, Verbose, Verify);
       if (Err) {
         ArchiveCache.erase(ArchiveFilename);
         // Don't return the error here: maybe the file wasn't an archive.
         llvm::consumeError(std::move(Err));
       } else {
         return ArchiveCache[ArchiveFilename].getObjectEntry(Filename, Timestamp,
                                                             Verbose);
       }
     }
   }
 
   // If this is an object, we might have it cached. If not we'll have to load
   // it from the file system and cache it now.
   std::lock_guard<std::mutex> Lock(ObjectCacheMutex);
   if (!ObjectCache.count(Filename)) {
     ObjectEntry &OE = ObjectCache[Filename];
-    auto Err = OE.load(VFS, Filename, Verbose);
+    auto Err = OE.load(VFS, Filename, Verbose, Verify);
     if (Err) {
       ObjectCache.erase(Filename);
       return std::move(Err);
     }
   }
 
   return ObjectCache[Filename];
 }
 
+void BinaryHolder::ObjectEntry::verify(bool Verbose) const {
+  raw_ostream &OS = Verbose ? errs() : nulls();
+  DIDumpOptions DumpOpts;
+  for (auto &Object : Objects) {
+    std::unique_ptr<DWARFContext> DICtx = DWARFContext::create(*Object);
+    if (!DICtx->verify(OS, DumpOpts.noImplicitRecursion()))
+      WithColor::warning() << "input verification failed for "
+                           << Object->getFileName() << '\n';
+  }
+}
+
+void BinaryHolder::ArchiveEntry::verify(bool Verbose) const {
+  for (auto &Member : MemberCache)
+    Member.second.verify(Verbose);
+}
+
 void BinaryHolder::clear() {
   std::lock_guard<std::mutex> ArchiveLock(ArchiveCacheMutex);
   std::lock_guard<std::mutex> ObjectLock(ObjectCacheMutex);
   ArchiveCache.clear();
   ObjectCache.clear();
 }
 
 } // namespace dsymutil
 } // namespace llvm

llvm/tools/dsymutil/Options.td

HelpText<"Print statistics about the contribution of each object file to "
            "the linked debug info. This prints a table after linking with the "
            "object file name, the size of the debug info in the object file "
            "(in bytes) and the size contributed (in bytes) to the linked dSYM. "
            "The table is sorted by the output size listing the object files "
            "with the largest contribution first.">,
   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>;
 
 def no_swiftmodule_timestamp: F<"no-swiftmodule-timestamp">,
   HelpText<"Don't check timestamp for swiftmodule files.">,
   Group<grp_general>;
 

llvm/tools/dsymutil/dsymutil.cpp

 };
 
 class DsymutilOptTable : public opt::OptTable {
 public:
   DsymutilOptTable() : OptTable(InfoTable) {}
 };
 } // namespace
 
+enum class DWARFVerify : unsigned {
+  None = 0,
+  Input = 1 << 0,
+  Output = 1 << 1,
+};
+
+inline DWARFVerify operator|(DWARFVerify LHS, DWARFVerify RHS) {
+  return static_cast<DWARFVerify>(static_cast<unsigned>(LHS) |
+                                  static_cast<unsigned>(RHS));
+}
+
+inline DWARFVerify operator&(DWARFVerify LHS, DWARFVerify RHS) {
+  return static_cast<DWARFVerify>(static_cast<unsigned>(LHS) &
+                                  static_cast<unsigned>(RHS));
+}
+
 struct DsymutilOptions {
   bool DumpDebugMap = false;
   bool DumpStab = false;
   bool Flat = false;
   bool InputIsYAMLDebugMap = false;
   bool PaperTrailWarnings = false;
-  bool Verify = false;
   std::string SymbolMap;
   std::string OutputFile;
   std::string Toolchain;
   std::string ReproducerPath;
   std::vector<std::string> Archs;
   std::vector<std::string> InputFiles;
   unsigned NumThreads;
+  DWARFVerify Verify = DWARFVerify::None;
   ReproducerMode ReproMode = ReproducerMode::Off;
   dsymutil::LinkOptions LinkOpts;
 };
 
 /// Return a list of input files. This function has logic for dealing with the
 /// special case where we might have dSYM bundles as input. The function
 /// returns an error when the directory structure doesn't match that of a dSYM
 /// bundle.
if (opt::Arg *Accelerator = Args.getLastArg(OPT_accelerator)) {
     return make_error<StringError>(
         "invalid accelerator type specified: '" + S +
             "'. Support values are 'Apple', 'Dwarf' and 'Default'.",
         inconvertibleErrorCode());
   }
   return AccelTableKind::Default;
 }
 
+static Expected<DWARFVerify> getVerify(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::Input | DWARFVerify::Output;
+    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) {
   DsymutilOptions Options;
 
   Options.DumpDebugMap = Args.hasArg(OPT_dump_debug_map);
   Options.DumpStab = Args.hasArg(OPT_symtab);
   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 = getVerify(Args)) {
+    Options.Verify = *Verify;
+  } else {
+    return Verify.takeError();
+  }
 
   Options.LinkOpts.Minimize = Args.hasArg(OPT_minimize);
   Options.LinkOpts.NoODR = Args.hasArg(OPT_no_odr);
   Options.LinkOpts.NoOutput = Args.hasArg(OPT_no_output);
   Options.LinkOpts.NoTimestamp = Args.hasArg(OPT_no_swiftmodule_timestamp);
   Options.LinkOpts.Update = Args.hasArg(OPT_update);
   Options.LinkOpts.Verbose = Args.hasArg(OPT_verbose);
   Options.LinkOpts.Statistics = Args.hasArg(OPT_statistics);
 
   if (opt::Arg *ReproducerPath = Args.getLastArg(OPT_use_reproducer)) {
     Options.ReproMode = ReproducerMode::Use;
     Options.ReproducerPath = ReproducerPath->getValue();
   }
static Error createBundleDir(StringRef BundleBase) {
   if (std::error_code EC =
           create_directories(Bundle.str(), true, sys::fs::perms::all_all))
     return make_error<StringError>(
         "cannot create bundle: " + toString(errorCodeToError(EC)), EC);
 
   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";
     return true;
   }
 
   Expected<OwningBinary<Binary>> BinOrErr = createBinary(OutputFile);
   if (!BinOrErr) {
     WithColor::error() << 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)
-      WithColor::error() << "verification failed for " << Arch << '\n';
+      WithColor::error() << "output verification failed for " << Arch << '\n';
     return success;
   }
 
   return false;
 }
 
 namespace {
 struct OutputLocation {
for (auto &InputFile : Options.InputFiles) {
 
     // Ensure that the debug map is not empty (anymore).
     if (DebugMapPtrsOrErr->empty()) {
       WithColor::error() << "no architecture to link\n";
       return 1;
     }
 
     // Shared a single binary holder for all the link steps.
-    BinaryHolder BinHolder(Options.LinkOpts.VFS);
+    const bool VerifyInput =
+        static_cast<bool>(Options.Verify & DWARFVerify::Input);
+    BinaryHolder BinHolder(Options.LinkOpts.VFS, Options.LinkOpts.Verbose,
+                           VerifyInput);
 
     // Statistics only require different architectures to be processed
     // sequentially, the link itself can still happen in parallel. Change the
     // thread pool strategy here instead of modifying LinkOpts.Threads.
     ThreadPoolStrategy S = hardware_concurrency(
         Options.LinkOpts.Statistics ? 1 : Options.LinkOpts.Threads);
     if (Options.LinkOpts.Threads == 0) {
       // If NumThreads is not specified, create one thread for each input, up to
       // the number of hardware threads.
       S.ThreadsRequested = DebugMapPtrsOrErr->size();
       S.Limit = true;
     }
     ThreadPool Threads(S);
 
     // If there is more than one link to execute, we need to generate
     // temporary files.
     const bool NeedsTempFiles =
         !Options.DumpDebugMap && (Options.OutputFile != "-") &&
         (DebugMapPtrsOrErr->size() != 1 || Options.LinkOpts.Update);
-    const bool Verify = Options.Verify && !Options.LinkOpts.NoOutput;
+    const bool VerifyOutput =
+        static_cast<bool>(Options.Verify & DWARFVerify::Output) &&
+        !Options.LinkOpts.NoOutput;
 
     SmallVector<MachOUtils::ArchAndFile, 4> TempFiles;
     std::atomic_char AllOK(1);
     for (auto &Map : *DebugMapPtrsOrErr) {
       if (Options.LinkOpts.Verbose || Options.DumpDebugMap)
         Map->print(outs());
 
       if (Options.DumpDebugMap)
for (auto &Map : *DebugMapPtrsOrErr) {
         }
       }
 
       auto LinkLambda = [&, OutputFile](std::shared_ptr<raw_fd_ostream> Stream,
                                         LinkOptions Options) {
         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
       // out the (significantly smaller) stack when using threads. We don't
       // want this limitation when we only have a single thread.
       if (S.ThreadsRequested == 1)
         LinkLambda(OS, Options.LinkOpts);
       else