[dsymutil] Add a new verification mode

[JDevlieghere]

Summary

This patch a new verification mode called "auto" that runs the DWARF verifier on the input and if the input is valid, also runs the DWARF verifier on the output. The goal is to catch cases where dsymutil turns valid into invalid DWARF. This patch makes this verification mode the default when assertions or expensive checks are enabled.

[avl]

llvm/include/llvm/DWARFLinker/DWARFLinker.h

247

onInputVerificationFailure looks to be a bit more precise name.

351

825

It looks like this variable is never set.

llvm/lib/DWARFLinker/DWARFLinker.cpp

2896–2898

as we have a call to the handler here it is probably better to move warning reporting code into this handler.

2897

if VerificationHandler is not set then we need to have a check for null here, or provide already existing default implementation.

llvm/tools/dsymutil/DwarfLinkerForBinary.cpp

276

default state for the second parameter is DWARFContext::ProcessDebugRelocations::Process. Is it necessary to explicitly specify it here?

llvm/tools/dsymutil/dsymutil.cpp

712–713

probably move this check and warning into the verifyOutput() function? to have a single place where Options.LinkOpts.NoOutput is checked.

[avl]

as DWARFv5 is not fully supported yet - probably not run input and output verifier for 5 version?

[JDevlieghere]

Address Alexey's code review feedback.

[JDevlieghere]

Skip output verification for DWARF 5

[avl]

LGTM.

llvm/tools/dsymutil/dsymutil.cpp

500

AFAIU, common spelling would be DWARFv5

[dyung]

@JDevlieghere, your change is causing 6 test failures on the PS5 Windows bot, can you take a look and revert if you need time to investigate?

https://lab.llvm.org/buildbot/#/builders/216/builds/19140

[haowei]

We are seeing test failures on Fuchsia's Windows bots after you changes are landed:

Command Output (stdout):
--
$ ":" "RUN: at line 1"
$ "rm" "-rf" "C:\src\llvm-project\build\test\tools\dsymutil\X86\Output\update.test.tmp.dir"
$ ":" "RUN: at line 2"
$ "mkdir" "-p" "C:\src\llvm-project\build\test\tools\dsymutil\X86\Output\update.test.tmp.dir"
$ ":" "RUN: at line 3"
$ "cat" "C:\src\llvm-project\llvm\test\tools\dsymutil\X86/../Inputs/basic.macho.x86_64"
$ ":" "RUN: at line 4"
$ "c:\src\llvm-project\build\bin\dsymutil.exe" "-accelerator=Pub" "-oso-prepend-path=C:\src\llvm-project\llvm\test\tools\dsymutil\X86/.." "C:\src\llvm-project\build\test\tools\dsymutil\X86\Output\update.test.tmp.dir/basic"
# command stderr:
warning: C:\src\llvm-project\llvm\test\tools\dsymutil\X86/../Inputs/basic1.macho.x86_64.o: timestamp mismatch between object file (2021-12-06 13:45:00.761849100) and debug map (2014-12-03 17:01:27.000000000)
warning: C:\src\llvm-project\llvm\test\tools\dsymutil\X86/../Inputs/basic2.macho.x86_64.o: timestamp mismatch between object file (2021-12-06 13:45:00.765848900) and debug map (2014-12-03 17:01:27.000000000)
warning: C:\src\llvm-project\llvm\test\tools\dsymutil\X86/../Inputs/basic3.macho.x86_64.o: timestamp mismatch between object file (2021-12-06 13:45:00.768848800) and debug map (2014-12-03 17:01:27.000000000)
warning: C:\src\llvm-project\llvm\test\tools\dsymutil\X86/../Inputs/basic1.macho.x86_64.o: timestamp mismatch between object file (2021-12-06 13:45:00.761849100) and debug map (2014-12-03 17:01:27.000000000)
warning: C:\src\llvm-project\llvm\test\tools\dsymutil\X86/../Inputs/basic2.macho.x86_64.o: timestamp mismatch between object file (2021-12-06 13:45:00.765848900) and debug map (2014-12-03 17:01:27.000000000)
warning: C:\src\llvm-project\llvm\test\tools\dsymutil\X86/../Inputs/basic3.macho.x86_64.o: timestamp mismatch between object file (2021-12-06 13:45:00.768848800) and debug map (2014-12-03 17:01:27.000000000)

$ ":" "RUN: at line 5"
$ "c:\src\llvm-project\build\bin\llvm-dwarfdump.exe" "-a" "C:\src\llvm-project\build\test\tools\dsymutil\X86\Output\update.test.tmp.dir/basic.dSYM"
$ "c:\src\llvm-project\build\bin\filecheck.exe" "C:\src\llvm-project\llvm\test\tools\dsymutil\X86/basic-linking-x86.test"
$ ":" "RUN: at line 6"
$ "c:\src\llvm-project\build\bin\dsymutil.exe" "-accelerator=Pub" "--update" "C:\src\llvm-project\build\test\tools\dsymutil\X86\Output\update.test.tmp.dir/basic.dSYM"
# command stderr:
error: C:\src\temp\lit-tmp-0ajrr046\dsym.tmp2328a.dwarf: permission denied
error: command failed with exit status: 1

1 example of the build: https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-windows-x64/b8785076599579542465/overview

Even with your latest patch c9f37c450056d6438e0ccc653580600d827ecd80, the test failure persist. Could you revert your changes if you need some time to investigate please?

[JDevlieghere]

I've switched back to no verification as the default (c2d2f724fe708a650b16c9d2f1f1a47156e5b2ea) while I investigate the permission issue.

llvm/include/llvm/DWARFLinker/DWARFLinker.h

public:
 
   /// Warnings for this object file.
   const std::vector<std::string> &Warnings;
 };
 
 typedef std::function<void(const Twine &Warning, StringRef Context,
                            const DWARFDie *DIE)>
     messageHandler;
+typedef std::function<void(const DWARFFile &File)> inputVerificationHandler;
 typedef std::function<ErrorOr<DWARFFile &>(StringRef ContainerName,
                                            StringRef Path)>
     objFileLoader;
 typedef std::map<std::string, std::string> swiftInterfacesMap;
 typedef std::map<std::string, std::string> objectPrefixMap;
 
 typedef function_ref<void(const DWARFUnit &Unit)> CompileUnitHandler;
 
void setWarningHandler(messageHandler Handler) {
     Options.WarningHandler = Handler;
   }
 
   /// Set error handler which would be used to report errors.
   void setErrorHandler(messageHandler Handler) {
     Options.ErrorHandler = Handler;
   }
 
+  /// Set verification handler which would be used to report verification
+  /// errors.
+  void setInputVerificationHandler(inputVerificationHandler Handler) {
+    Options.InputVerificationHandler = Handler;
+  }
+
   /// Set map for Swift interfaces.
   void setSwiftInterfacesMap(swiftInterfacesMap *Map) {
     Options.ParseableSwiftInterfaces = Map;
   }
 
   /// Set prefix map for objects.
   void setObjectPrefixMap(objectPrefixMap *Map) {
     Options.ObjectPrefixMap = Map;
WorklistItem(DWARFDie Die, CompileUnit &CU, WorklistItemType T,
         : Die(Die), Type(T), CU(CU), Flags(0), OtherInfo(OtherInfo) {}
 
     WorklistItem(unsigned AncestorIdx, CompileUnit &CU, unsigned Flags)
         : Type(WorklistItemType::LookForParentDIEsToKeep), CU(CU), Flags(Flags),
           AncestorIdx(AncestorIdx) {}
   };
 
   /// Verify the given DWARF file.
-  bool verify(const DWARFFile &File);
+  void verifyInput(const DWARFFile &File);
 
   /// returns true if we need to translate strings.
   bool needToTranslateStrings() { return StringsTranslator != nullptr; }
 
   void reportWarning(const Twine &Warning, const DWARFFile &File,
                      const DWARFDie *DIE = nullptr) const {
     if (Options.WarningHandler != nullptr)
       Options.WarningHandler(Warning, File.FileName, DIE);
private:
   DwarfLinkerClient DwarfLinkerClientID;
 
   std::function<StringRef(StringRef)> StringsTranslator = nullptr;
 
   /// A unique ID that identifies each compile unit.
   unsigned UniqueUnitID = 0;
 
   /// linking options
   struct DWARFLinkerOptions {
     /// DWARF version for the output.
     uint16_t TargetDWARFVersion = 0;
 
     /// Generate processing log to the standard output.
     bool Verbose = false;
 
     /// Print statistics.
     bool Statistics = false;
struct DWARFLinkerOptions {
     std::string PrependPath;
 
     // warning handler
     messageHandler WarningHandler = nullptr;
 
     // error handler
     messageHandler ErrorHandler = nullptr;
 
+    // input verification handler
+    inputVerificationHandler InputVerificationHandler = nullptr;
+
     /// A list of all .swiftinterface files referenced by the debug
     /// info, mapping Module name to path on disk. The entries need to
     /// be uniqued and sorted and there are only few entries expected
     /// per compile unit, which is why this is a std::map.
     /// this is dsymutil specific fag.
     swiftInterfacesMap *ParseableSwiftInterfaces = nullptr;
 
     /// A list of remappings to apply to file paths.
     objectPrefixMap *ObjectPrefixMap = nullptr;
   } Options;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_DWARFLINKER_DWARFLINKER_H

llvm/lib/DWARFLinker/DWARFLinker.cpp

for (LinkContext &OptContext : ObjectContexts) {
 
     if (emitPaperTrailWarnings(OptContext.File, OffsetsStringPool))
       continue;
 
     if (!OptContext.File.Dwarf)
       continue;
 
     if (Options.VerifyInputDWARF)
-      verify(OptContext.File);
+      verifyInput(OptContext.File);
 
     // Look for relocations that correspond to address map entries.
 
     // there was findvalidrelocations previously ... probably we need to gather
     // info here
     if (LLVM_LIKELY(!Options.Update) &&
         !OptContext.File.Addresses->hasValidRelocs()) {
       if (Options.Verbose)
Error DWARFLinker::cloneModuleUnit(LinkContext &Context, RefModuleUnit &Unit,
   assert(TheDwarfEmitter);
   DIECloner(*this, TheDwarfEmitter, Unit.File, DIEAlloc, CompileUnits,
             Options.Update)
       .cloneAllCompileUnits(*Unit.File.Dwarf, Unit.File, OffsetsStringPool,
                             Unit.File.Dwarf->isLittleEndian());
   return Error::success();
 }
 
-bool DWARFLinker::verify(const DWARFFile &File) {
+void DWARFLinker::verifyInput(const DWARFFile &File) {
   assert(File.Dwarf);
 
   raw_ostream &os = Options.Verbose ? errs() : nulls();
   DIDumpOptions DumpOpts;
   if (!File.Dwarf->verify(os, DumpOpts.noImplicitRecursion())) {
-    reportWarning("input verification failed", File);
-    return false;
+    if (Options.InputVerificationHandler)
+      Options.InputVerificationHandler(File);
   }
-  return true;
 }
 
 } // namespace llvm

llvm/tools/dsymutil/DwarfLinkerForBinary.h

DwarfLinkerForBinary(raw_fd_ostream &OutFile, BinaryHolder &BinHolder,
       : OutFile(OutFile), BinHolder(BinHolder), Options(std::move(Options)) {}
 
   /// Link the contents of the DebugMap.
   bool link(const DebugMap &);
 
   void reportWarning(const Twine &Warning, StringRef Context,
                      const DWARFDie *DIE = nullptr) const;
 
+  /// Returns true if input verification is enabled and verification errors were
+  /// found.
+  bool InputVerificationFailed() const { return HasVerificationErrors; }
+
   /// Flags passed to DwarfLinker::lookForDIEsToKeep
   enum TraversalFlags {
     TF_Keep = 1 << 0,            ///< Mark the traversed DIEs as kept.
     TF_InFunctionScope = 1 << 1, ///< Current scope is a function scope.
     TF_DependencyWalk = 1 << 2,  ///< Walking the dependencies of a kept DIE.
     TF_ParentWalk = 1 << 3,      ///< Walking up the parents of a kept DIE.
     TF_ODR = 1 << 4,             ///< Use the ODR while keeping dependents.
     TF_SkipPC = 1 << 5,          ///< Skip all location attributes.
private:
   /// A list of all .swiftinterface files referenced by the debug
   /// info, mapping Module name to path on disk. The entries need to
   /// be uniqued and sorted and there are only few entries expected
   /// per compile unit, which is why this is a std::map.
   std::map<std::string, std::string> ParseableSwiftInterfaces;
 
   bool ModuleCacheHintDisplayed = false;
   bool ArchiveHintDisplayed = false;
+  bool HasVerificationErrors = false;
 };
 
 } // end namespace dsymutil
 } // end namespace llvm
 
 #endif // LLVM_TOOLS_DSYMUTIL_DWARFLINKER_H

llvm/tools/dsymutil/DwarfLinkerForBinary.cpp

 DwarfLinkerForBinary::loadObject(const DebugMapObject &Obj,
                                  const DebugMap &DebugMap,
                                  remarks::RemarkLinker &RL) {
   auto ErrorOrObj = loadObject(Obj, DebugMap.getTriple());
 
   if (ErrorOrObj) {
     ContextForLinking.push_back(
         std::unique_ptr<DWARFContext>(DWARFContext::create(*ErrorOrObj)));
     AddressMapForLinking.push_back(
         std::make_unique<AddressManager>(*this, *ErrorOrObj, Obj));
 
     ObjectsForLinking.push_back(std::make_unique<DWARFFile>(
         Obj.getObjectFilename(), ContextForLinking.back().get(),
         AddressMapForLinking.back().get(),
         Obj.empty() ? Obj.getWarnings() : EmptyWarnings));
 
     Error E = RL.link(*ErrorOrObj);
bool DwarfLinkerForBinary::link(const DebugMap &Map) {
   GeneralLinker.setWarningHandler(
       [&](const Twine &Warning, StringRef Context, const DWARFDie *DIE) {
         reportWarning(Warning, Context, DIE);
       });
   GeneralLinker.setErrorHandler(
       [&](const Twine &Error, StringRef Context, const DWARFDie *) {
         error(Error, Context);
       });
+  GeneralLinker.setInputVerificationHandler([&](const DWARFFile &File) {
+    reportWarning("input verification failed", File.FileName);
+    HasVerificationErrors = true;
+  });
   objFileLoader Loader = [&DebugMap, &RL,
                           this](StringRef ContainerName,
                                 StringRef Path) -> ErrorOr<DWARFFile &> {
     auto &Obj = DebugMap.addDebugMapObject(
         Path, sys::TimePoint<std::chrono::seconds>(), MachO::N_OSO);
 
     if (auto ErrorOrObj = loadObject(Obj, DebugMap, RL)) {
       return *ErrorOrObj;
if (!Options.NoOutput) {
     ReflectionSectionsPresentInBinary =
         binaryHasStrippableSwiftReflectionSections(Map, Options, BinHolder);
   }
 
   std::vector<MachOUtils::DwarfRelocationApplicationInfo> RelocationsToApply;
   if (!Options.NoOutput && !ReflectionSectionsPresentInBinary) {
     auto SectionToOffsetInDwarf =
         calculateStartOfStrippableReflectionSections(Map);
-    for (const auto &Obj : Map.objects()) 
+    for (const auto &Obj : Map.objects())
       copySwiftReflectionMetadata(Obj.get(), Streamer.get(),
                                   SectionToOffsetInDwarf, RelocationsToApply);
   }
 
   uint16_t MaxDWARFVersion = 0;
   std::function<void(const DWARFUnit &Unit)> OnCUDieLoaded =
       [&MaxDWARFVersion](const DWARFUnit &Unit) {
         MaxDWARFVersion = std::max(Unit.getVersion(), MaxDWARFVersion);
for (const ValidReloc &CurReloc : Relocs) {
     }
     assert(CurReloc.Size <= sizeof(Buf));
     memcpy(&Data[CurReloc.Offset - BaseOffset], Buf, CurReloc.Size);
   }
 
   return Relocs.size() > 0;
 }
 
-bool linkDwarf(raw_fd_ostream &OutFile, BinaryHolder &BinHolder,
-               const DebugMap &DM, LinkOptions Options) {
-  DwarfLinkerForBinary Linker(OutFile, BinHolder, std::move(Options));
-  return Linker.link(DM);
-}
-
 } // namespace dsymutil
 } // namespace llvm

llvm/tools/dsymutil/Options.td

def statistics: F<"statistics">,
   Group<grp_general>;
 
 def verify: F<"verify">,
   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'.">,
+  HelpText<"Run the DWARF verifier on the input and/or output. "
+           "Valid options are 'none, 'input', 'output', 'all' or 'auto' "
+           "which runs the output verifier only if input verification passed.">,
   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">,

llvm/tools/dsymutil/dsymutil.h

parseDebugMap(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
               StringRef PrependPath, bool PaperTrailWarnings, bool Verbose,
               bool InputIsYAML);
 
 /// Dump the symbol table.
 bool dumpStab(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
               StringRef InputFile, ArrayRef<std::string> Archs,
               StringRef PrependPath = "");
 
-/// Link the Dwarf debug info as directed by the passed DebugMap \p DM into a
-/// DwarfFile named \p OutputFilename. \returns false if the link failed.
-bool linkDwarf(raw_fd_ostream &OutFile, BinaryHolder &BinHolder,
-               const DebugMap &DM, LinkOptions Options);
-
 } // end namespace dsymutil
 } // end namespace llvm
 
 #endif // LLVM_TOOLS_DSYMUTIL_DSYMUTIL_H

llvm/tools/dsymutil/dsymutil.cpp

 //===- dsymutil.cpp - Debug info dumping utility for llvm -----------------===//
 //
 // 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 "dsymutil.h"
 #include "BinaryHolder.h"
 #include "CFBundle.h"
 #include "DebugMap.h"
+#include "DwarfLinkerForBinary.h"
 #include "LinkUtils.h"
 #include "MachOUtils.h"
 #include "Reproducer.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
public:
   DsymutilOptTable() : opt::GenericOptTable(InfoTable) {}
 };
 } // namespace
 
 enum class DWARFVerify : uint8_t {
   None = 0,
   Input = 1 << 0,
   Output = 1 << 1,
+  OutputOnValidInput = 1 << 2,
   All = Input | Output,
+  Auto = Input | OutputOnValidInput,
+#if !defined(NDEBUG) || defined(EXPENSIVE_CHECKS)
+  Default = Auto
+#else
+  Default = None
+#endif
 };
 
 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 ForceKeepFunctionForStatic = 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;
+  DWARFVerify Verify = DWARFVerify::Default;
   ReproducerMode ReproMode = ReproducerMode::GenerateOnCrash;
   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.
static Expected<DWARFVerify> getVerifyKind(opt::InputArgList &Args) {
   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 == "auto")
+      return DWARFVerify::Auto;
     if (S == "none")
       return DWARFVerify::None;
-    return make_error<StringError>(
-        "invalid verify type specified: '" + S +
-            "'. Supported values are 'input', 'output', 'all' and 'none'.",
+    return make_error<StringError>("invalid verify type specified: '" + S +
+                                       "'. Supported values are 'none', "
+                                       "'input', 'output', 'all' and 'auto'.",
                                    inconvertibleErrorCode());
   }
-  return DWARFVerify::None;
+  return DWARFVerify::Default;
 }
 
 /// 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);
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 verifyOutput(StringRef OutputFile, StringRef Arch, bool Verbose) {
+static bool verifyOutput(StringRef OutputFile, StringRef Arch,
+                         DsymutilOptions Options) {
+
   if (OutputFile == "-") {
     WithColor::warning() << "verification skipped for " << Arch
-                         << "because writing to stdout.\n";
+                         << " because writing to stdout.\n";
     return true;
   }
 
+  if (Options.LinkOpts.NoOutput) {
+    WithColor::warning() << "verification skipped for " << Arch
+                         << " because --no-output was passed.\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);
+    if (DICtx->getMaxVersion() >= 5) {
+      WithColor::warning() << "verification skipped for " << Arch
+                           << " because DWARFv5 is not fully supported yet.\n";
+      return true;
+    }
+    raw_ostream &os = Options.LinkOpts.Verbose ? errs() : nulls();
+    os << "Verifying DWARF for architecture: " << Arch << "\n";
     DIDumpOptions DumpOpts;
     bool success = DICtx->verify(os, DumpOpts.noImplicitRecursion());
     if (!success)
       WithColor::error() << "output verification failed for " << Arch << '\n';
     return success;
   }
 
   return false;
if (Options.LinkOpts.Threads == 0) {
       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);
-    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;
-    }
 
     // Set up a crash recovery context.
     CrashRecoveryContext::Enable();
     CrashRecoveryContext CRC;
     CRC.DumpStackAndCleanupOnFailure = true;
 
     std::atomic_char AllOK(1);
     SmallVector<MachOUtils::ArchAndFile, 4> TempFiles;
 
const bool Crashed = !CRC.RunSafely([&]() {
           if (EC) {
             WithColor::error() << OutputFile << ": " << EC.message();
             AllOK.fetch_and(false);
             return;
           }
         }
 
         auto LinkLambda = [&,
-                           OutputFile](std::shared_ptr<raw_fd_ostream> Stream,
-                                       LinkOptions Options) {
-          AllOK.fetch_and(
-              linkDwarf(*Stream, BinHolder, *Map, std::move(Options)));
+                           OutputFile](std::shared_ptr<raw_fd_ostream> Stream) {
+          DwarfLinkerForBinary Linker(*Stream, BinHolder, Options.LinkOpts);
+          AllOK.fetch_and(Linker.link(*Map));
           Stream->flush();
-          if (VerifyOutput) {
+          if (flagIsSet(Options.Verify, DWARFVerify::Output) ||
+              (flagIsSet(Options.Verify, DWARFVerify::OutputOnValidInput) &&
+               !Linker.InputVerificationFailed())) {
             AllOK.fetch_and(verifyOutput(
-                OutputFile, Map->getTriple().getArchName(), Options.Verbose));
+                OutputFile, Map->getTriple().getArchName(), Options));
           }
         };
 
         // 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);
+          LinkLambda(OS);
         else
-          Threads.async(LinkLambda, OS, Options.LinkOpts);
+          Threads.async(LinkLambda, OS);
       }
 
       Threads.wait();
     });
 
     if (Crashed)
       (*Repro)->generate();