diff --git a/llvm/include/llvm/DWARFLinker/DWARFLinker.h b/llvm/include/llvm/DWARFLinker/DWARFLinker.h --- a/llvm/include/llvm/DWARFLinker/DWARFLinker.h +++ b/llvm/include/llvm/DWARFLinker/DWARFLinker.h @@ -244,6 +244,7 @@ typedef std::function messageHandler; +typedef std::function inputVerificationHandler; typedef std::function(StringRef ContainerName, StringRef Path)> objFileLoader; @@ -345,6 +346,12 @@ 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; @@ -424,7 +431,7 @@ }; /// 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; } @@ -856,6 +863,9 @@ // 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 diff --git a/llvm/lib/DWARFLinker/DWARFLinker.cpp b/llvm/lib/DWARFLinker/DWARFLinker.cpp --- a/llvm/lib/DWARFLinker/DWARFLinker.cpp +++ b/llvm/lib/DWARFLinker/DWARFLinker.cpp @@ -2577,7 +2577,7 @@ continue; if (Options.VerifyInputDWARF) - verify(OptContext.File); + verifyInput(OptContext.File); // Look for relocations that correspond to address map entries. @@ -2888,16 +2888,15 @@ 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 diff --git a/llvm/tools/dsymutil/DwarfLinkerForBinary.h b/llvm/tools/dsymutil/DwarfLinkerForBinary.h --- a/llvm/tools/dsymutil/DwarfLinkerForBinary.h +++ b/llvm/tools/dsymutil/DwarfLinkerForBinary.h @@ -44,6 +44,10 @@ 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. @@ -230,6 +234,7 @@ bool ModuleCacheHintDisplayed = false; bool ArchiveHintDisplayed = false; + bool HasVerificationErrors = false; }; } // end namespace dsymutil diff --git a/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp b/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp --- a/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp +++ b/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp @@ -589,6 +589,10 @@ [&](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 { @@ -654,7 +658,7 @@ 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); } @@ -1070,11 +1074,5 @@ 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 diff --git a/llvm/tools/dsymutil/Options.td b/llvm/tools/dsymutil/Options.td --- a/llvm/tools/dsymutil/Options.td +++ b/llvm/tools/dsymutil/Options.td @@ -43,7 +43,9 @@ def verify_dwarf: Separate<["--", "-"], "verify-dwarf">, MetaVarName<"">, - 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; def: Joined<["--", "-"], "verify-dwarf=">, Alias; diff --git a/llvm/tools/dsymutil/dsymutil.h b/llvm/tools/dsymutil/dsymutil.h --- a/llvm/tools/dsymutil/dsymutil.h +++ b/llvm/tools/dsymutil/dsymutil.h @@ -45,11 +45,6 @@ StringRef InputFile, ArrayRef 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 diff --git a/llvm/tools/dsymutil/dsymutil.cpp b/llvm/tools/dsymutil/dsymutil.cpp --- a/llvm/tools/dsymutil/dsymutil.cpp +++ b/llvm/tools/dsymutil/dsymutil.cpp @@ -14,6 +14,7 @@ #include "BinaryHolder.h" #include "CFBundle.h" #include "DebugMap.h" +#include "DwarfLinkerForBinary.h" #include "LinkUtils.h" #include "MachOUtils.h" #include "Reproducer.h" @@ -94,7 +95,14 @@ 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) { @@ -115,7 +123,7 @@ std::vector Archs; std::vector InputFiles; unsigned NumThreads; - DWARFVerify Verify = DWARFVerify::None; + DWARFVerify Verify = DWARFVerify::Default; ReproducerMode ReproMode = ReproducerMode::GenerateOnCrash; dsymutil::LinkOptions LinkOpts; }; @@ -266,14 +274,16 @@ return DWARFVerify::Output; if (S == "all") return DWARFVerify::All; + if (S == "auto") + return DWARFVerify::Auto; if (S == "none") return DWARFVerify::None; - return make_error( - "invalid verify type specified: '" + S + - "'. Supported values are 'input', 'output', 'all' and 'none'.", - inconvertibleErrorCode()); + return make_error("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 @@ -461,10 +471,18 @@ 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; } @@ -476,9 +494,14 @@ Binary &Binary = *BinOrErr.get().getBinary(); if (auto *Obj = dyn_cast(&Binary)) { - raw_ostream &os = Verbose ? errs() : nulls(); - os << "Verifying DWARF for architecture: " << Arch << "\n"; std::unique_ptr 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) @@ -687,12 +710,6 @@ 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(); @@ -751,14 +768,15 @@ } auto LinkLambda = [&, - OutputFile](std::shared_ptr Stream, - LinkOptions Options) { - AllOK.fetch_and( - linkDwarf(*Stream, BinHolder, *Map, std::move(Options))); + OutputFile](std::shared_ptr 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)); } }; @@ -766,9 +784,9 @@ // 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();