[llvm] be91b4e - [dsymutil] Add a new automatic verification mode
Jonas Devlieghere via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 31 09:59:53 PDT 2023
Author: Jonas Devlieghere
Date: 2023-03-31T09:59:46-07:00
New Revision: be91b4e3f4f9b13b16f8482d9bfd9734898e52e1
URL: https://github.com/llvm/llvm-project/commit/be91b4e3f4f9b13b16f8482d9bfd9734898e52e1
DIFF: https://github.com/llvm/llvm-project/commit/be91b4e3f4f9b13b16f8482d9bfd9734898e52e1.diff
LOG: [dsymutil] Add a new automatic verification mode
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 DWARF into invalid DWARF. This patch makes this verification mode
the default when assertions or expensive checks are enabled.
Differential revision: https://reviews.llvm.org/D147203
Added:
Modified:
llvm/include/llvm/DWARFLinker/DWARFLinker.h
llvm/lib/DWARFLinker/DWARFLinker.cpp
llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
llvm/tools/dsymutil/DwarfLinkerForBinary.h
llvm/tools/dsymutil/Options.td
llvm/tools/dsymutil/dsymutil.cpp
llvm/tools/dsymutil/dsymutil.h
Removed:
################################################################################
diff --git a/llvm/include/llvm/DWARFLinker/DWARFLinker.h b/llvm/include/llvm/DWARFLinker/DWARFLinker.h
index f58bb3f6ec32b..06a0696c03c93 100644
--- a/llvm/include/llvm/DWARFLinker/DWARFLinker.h
+++ b/llvm/include/llvm/DWARFLinker/DWARFLinker.h
@@ -244,6 +244,7 @@ class DWARFFile {
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;
@@ -345,6 +346,12 @@ class DWARFLinker {
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 @@ class DWARFLinker {
};
/// 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 @@ class DWARFLinker {
// 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
index 1a61506b04a03..94a711a10fefa 100644
--- a/llvm/lib/DWARFLinker/DWARFLinker.cpp
+++ b/llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -2577,7 +2577,7 @@ Error DWARFLinker::link() {
continue;
if (Options.VerifyInputDWARF)
- verify(OptContext.File);
+ verifyInput(OptContext.File);
// Look for relocations that correspond to address map entries.
@@ -2888,16 +2888,15 @@ Error DWARFLinker::cloneModuleUnit(LinkContext &Context, RefModuleUnit &Unit,
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.cpp b/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
index c5e1bf1eb91ac..cf3c87bc630af 100644
--- a/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
+++ b/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
@@ -589,6 +589,10 @@ bool DwarfLinkerForBinary::link(const DebugMap &Map) {
[&](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 &> {
@@ -654,7 +658,7 @@ bool DwarfLinkerForBinary::link(const DebugMap &Map) {
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 @@ bool DwarfLinkerForBinary::AddressManager::applyValidRelocs(
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/DwarfLinkerForBinary.h b/llvm/tools/dsymutil/DwarfLinkerForBinary.h
index 1f7422f43401e..7fd15a1eb71f3 100644
--- a/llvm/tools/dsymutil/DwarfLinkerForBinary.h
+++ b/llvm/tools/dsymutil/DwarfLinkerForBinary.h
@@ -44,6 +44,10 @@ class DwarfLinkerForBinary {
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 @@ class DwarfLinkerForBinary {
bool ModuleCacheHintDisplayed = false;
bool ArchiveHintDisplayed = false;
+ bool HasVerificationErrors = false;
};
} // end namespace dsymutil
diff --git a/llvm/tools/dsymutil/Options.td b/llvm/tools/dsymutil/Options.td
index 8af6dbef0c3d5..abcdc91977f7f 100644
--- a/llvm/tools/dsymutil/Options.td
+++ b/llvm/tools/dsymutil/Options.td
@@ -43,7 +43,9 @@ def verify: F<"verify">,
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>;
diff --git a/llvm/tools/dsymutil/dsymutil.cpp b/llvm/tools/dsymutil/dsymutil.cpp
index d868459f4327c..e6e5d7bdcfc0d 100644
--- 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 @@ 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) {
@@ -115,7 +123,7 @@ struct DsymutilOptions {
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;
};
@@ -266,14 +274,16 @@ static Expected<DWARFVerify> getVerifyKind(opt::InputArgList &Args) {
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'.",
- inconvertibleErrorCode());
+ 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
@@ -461,10 +471,18 @@ static Error createBundleDir(StringRef BundleBase) {
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 @@ static bool verifyOutput(StringRef OutputFile, StringRef Arch, bool Verbose) {
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)
@@ -687,12 +710,6 @@ int main(int argc, char **argv) {
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 @@ int main(int argc, char **argv) {
}
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));
}
};
@@ -766,9 +784,9 @@ int main(int argc, char **argv) {
// 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();
diff --git a/llvm/tools/dsymutil/dsymutil.h b/llvm/tools/dsymutil/dsymutil.h
index f88f57bb20a55..e6799286610df 100644
--- a/llvm/tools/dsymutil/dsymutil.h
+++ b/llvm/tools/dsymutil/dsymutil.h
@@ -45,11 +45,6 @@ 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
More information about the llvm-commits
mailing list