Index: llvm/test/tools/dsymutil/X86/verify.test =================================================================== --- llvm/test/tools/dsymutil/X86/verify.test +++ llvm/test/tools/dsymutil/X86/verify.test @@ -3,14 +3,22 @@ # 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' Index: llvm/tools/dsymutil/BinaryHolder.h =================================================================== --- llvm/tools/dsymutil/BinaryHolder.h +++ llvm/tools/dsymutil/BinaryHolder.h @@ -38,8 +38,9 @@ public: using TimestampTy = sys::TimePoint; - BinaryHolder(IntrusiveRefCntPtr VFS, bool Verbose = false) - : VFS(VFS), Verbose(Verbose) {} + BinaryHolder(IntrusiveRefCntPtr VFS, bool Verbose = false, + bool Verify = false) + : VFS(VFS), Verbose(Verbose), Verify(Verify) {} // Forward declarations for friend declaration. class ObjectEntry; @@ -58,7 +59,7 @@ public: /// Load the given object binary in memory. Error load(IntrusiveRefCntPtr VFS, StringRef Filename, - bool Verbose = false); + bool Verbose = false, bool Verify = false); /// Access all owned ObjectFiles. std::vector getObjects() const; @@ -91,6 +92,8 @@ return cast(*Object); } + void verify(bool Verbose = false) const; + private: std::vector> Objects; friend ArchiveEntry; @@ -110,12 +113,15 @@ /// Load the given object binary in memory. Error load(IntrusiveRefCntPtr VFS, StringRef Filename, - TimestampTy Timestamp, bool Verbose = false); + TimestampTy Timestamp, bool Verbose = false, + bool Verify = false); Expected getObjectEntry(StringRef Filename, TimestampTy Timestamp, bool Verbose = false); + void verify(bool Verbose = false) const; + private: std::vector> Archives; DenseMap MemberCache; @@ -141,6 +147,7 @@ IntrusiveRefCntPtr VFS; bool Verbose; + bool Verify; }; } // namespace dsymutil Index: llvm/tools/dsymutil/BinaryHolder.cpp =================================================================== --- llvm/tools/dsymutil/BinaryHolder.cpp +++ llvm/tools/dsymutil/BinaryHolder.cpp @@ -12,6 +12,8 @@ //===----------------------------------------------------------------------===// #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" @@ -43,7 +45,8 @@ Error BinaryHolder::ArchiveEntry::load(IntrusiveRefCntPtr 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. @@ -83,11 +86,15 @@ Archives.push_back(std::move(*ErrOrArchive)); } + if (Verify) + verify(Verbose); + return Error::success(); } Error BinaryHolder::ObjectEntry::load(IntrusiveRefCntPtr 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() @@ -124,6 +131,9 @@ Objects.push_back(std::move(*ErrOrObjectFile)); } + if (Verify) + verify(Verbose); + return Error::success(); } @@ -229,7 +239,7 @@ 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. @@ -246,7 +256,7 @@ std::lock_guard 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); @@ -256,6 +266,22 @@ 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 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 ArchiveLock(ArchiveCacheMutex); std::lock_guard ObjectLock(ObjectCacheMutex); Index: llvm/tools/dsymutil/Options.td =================================================================== --- llvm/tools/dsymutil/Options.td +++ llvm/tools/dsymutil/Options.td @@ -34,9 +34,15 @@ Group; def verify: F<"verify">, - HelpText<"Run the DWARF verifier on the linked DWARF debug info.">, + HelpText<"Alias for ---verify-dwarf=output">, Group; +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'.">, + Group; +def: Joined<["--", "-"], "verify-dwarf=">, Alias; + def no_output: F<"no-output">, HelpText<"Do the link in memory, but do not emit the result file.">, Group; Index: llvm/tools/dsymutil/dsymutil.cpp =================================================================== --- llvm/tools/dsymutil/dsymutil.cpp +++ llvm/tools/dsymutil/dsymutil.cpp @@ -85,13 +85,28 @@ }; } // namespace +enum class DWARFVerify : unsigned { + None = 0, + Input = 1 << 0, + Output = 1 << 1, +}; + +inline DWARFVerify operator|(DWARFVerify LHS, DWARFVerify RHS) { + return static_cast(static_cast(LHS) | + static_cast(RHS)); +} + +inline DWARFVerify operator&(DWARFVerify LHS, DWARFVerify RHS) { + return static_cast(static_cast(LHS) & + static_cast(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; @@ -99,6 +114,7 @@ std::vector Archs; std::vector InputFiles; unsigned NumThreads; + DWARFVerify Verify = DWARFVerify::None; ReproducerMode ReproMode = ReproducerMode::Off; dsymutil::LinkOptions LinkOpts; }; @@ -211,6 +227,27 @@ return AccelTableKind::Default; } +static Expected 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( + "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 getOptions(opt::InputArgList &Args) { @@ -221,7 +258,12 @@ 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 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); @@ -384,7 +426,7 @@ 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"; @@ -405,7 +447,7 @@ 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; } @@ -575,7 +617,10 @@ } // Shared a single binary holder for all the link steps. - BinaryHolder BinHolder(Options.LinkOpts.VFS); + const bool VerifyInput = + static_cast(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 @@ -595,7 +640,9 @@ 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(Options.Verify & DWARFVerify::Output) && + !Options.LinkOpts.NoOutput; SmallVector TempFiles; std::atomic_char AllOK(1); @@ -655,9 +702,10 @@ 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