[lld] [ELF] Add context-aware diagnostic functions (PR #112319)
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 21 23:01:34 PDT 2024
https://github.com/MaskRay updated https://github.com/llvm/llvm-project/pull/112319
>From dccc00a07ba591e0d5b937b0231f30f760003a7c Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Sun, 13 Oct 2024 23:06:41 -0700
Subject: [PATCH] [ELF] Add context-aware diagnostic functions
The current diagnostic functions log/warn/error/fatal lack a context
argument and call the global `lld::errorHandler()`, which prevents
multiple lld instances in one process.
This patch introduces context-aware replacements:
* log => Log(ctx)
* warn => Warn(ctx)
* errorOrWarn => Err(ctx)
* error => ErrAlways(ctx)
* fatal => Fatal(ctx)
Example: `errorOrWarn(toString(f) + "xxx")` => `Err(ctx) << f << "xxx"`.
(`toString(f)` is shortened to `f` as a bonus and may access `ctx`
without accessing the global variable (see `Target.cpp`)).
`ctx.e = &context->e;` can be replaced with a non-global Errorhandler
when `ctx` becomes a local variable.
(For the ELF port, the long term goal is to eliminate `error`. Most can
be straightforwardly converted to `Err(ctx)`.)
Pull Request: https://github.com/llvm/llvm-project/pull/112319
---
lld/Common/ErrorHandler.cpp | 18 +++++++++++++++
lld/ELF/Config.h | 33 +++++++++++++++++++++++++++
lld/ELF/Driver.cpp | 20 ++++++++++++----
lld/ELF/InputFiles.cpp | 6 +++++
lld/ELF/InputFiles.h | 2 ++
lld/ELF/Target.cpp | 9 ++++++++
lld/ELF/Target.h | 2 ++
lld/include/lld/Common/ErrorHandler.h | 17 +++++++++++++-
8 files changed, 102 insertions(+), 5 deletions(-)
diff --git a/lld/Common/ErrorHandler.cpp b/lld/Common/ErrorHandler.cpp
index 4e3a1bc31ade50..21755481665961 100644
--- a/lld/Common/ErrorHandler.cpp
+++ b/lld/Common/ErrorHandler.cpp
@@ -335,3 +335,21 @@ void ErrorHandler::fatal(const Twine &msg) {
error(msg);
exitLld(1);
}
+
+SyncStream::~SyncStream() {
+ os.flush();
+ switch (level) {
+ case DiagLevel::Log:
+ e.log(buf);
+ break;
+ case DiagLevel::Warn:
+ e.warn(buf);
+ break;
+ case DiagLevel::Err:
+ e.error(buf);
+ break;
+ case DiagLevel::Fatal:
+ e.fatal(buf);
+ break;
+ }
+}
diff --git a/lld/ELF/Config.h b/lld/ELF/Config.h
index f5d65d26ef4479..2811b55e098ba1 100644
--- a/lld/ELF/Config.h
+++ b/lld/ELF/Config.h
@@ -547,6 +547,8 @@ struct Ctx {
LinkerScript *script;
std::unique_ptr<TargetInfo> target;
+ ErrorHandler *errHandler;
+
// These variables are initialized by Writer and should not be used before
// Writer is initialized.
uint8_t *bufferStart;
@@ -679,6 +681,37 @@ static inline void internalLinkerError(StringRef loc, const Twine &msg) {
llvm::getBugReportMsg());
}
+struct ELFSyncStream : SyncStream {
+ Ctx &ctx;
+ ELFSyncStream(Ctx &ctx, DiagLevel level)
+ : SyncStream(*ctx.errHandler, level), ctx(ctx) {}
+};
+
+template <typename T>
+std::enable_if_t<!std::is_pointer_v<std::remove_reference_t<T>>,
+ const ELFSyncStream &>
+operator<<(const ELFSyncStream &s, T &&v) {
+ s.os << std::forward<T>(v);
+ return s;
+}
+
+// Report a log if --verbose is specified.
+ELFSyncStream Log(Ctx &ctx);
+
+// Report a warning. Upgraded to an error if --fatal-warnings is specified.
+ELFSyncStream Warn(Ctx &ctx);
+
+// Report an error that will suppress the output file generation. Downgraded to
+// a warning if --noinhibit-exec is specified.
+ELFSyncStream Err(Ctx &ctx);
+
+// Report an error regardless of --noinhibit-exec.
+ELFSyncStream ErrAlways(Ctx &ctx);
+
+// Report a fatal error that exits immediately. This should generally be avoided
+// in favor of Err.
+ELFSyncStream Fatal(Ctx &ctx);
+
} // namespace lld::elf
#endif
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 0d7712f904dab8..af9ebdc27c1c36 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -91,6 +91,14 @@ void elf::errorOrWarn(const Twine &msg) {
error(msg);
}
+ELFSyncStream elf::Log(Ctx &ctx) { return {ctx, DiagLevel::Log}; }
+ELFSyncStream elf::Warn(Ctx &ctx) { return {ctx, DiagLevel::Warn}; }
+ELFSyncStream elf::Err(Ctx &ctx) {
+ return {ctx, ctx.arg.noinhibitExec ? DiagLevel::Warn : DiagLevel::Err};
+}
+ELFSyncStream elf::ErrAlways(Ctx &ctx) { return {ctx, DiagLevel::Err}; }
+ELFSyncStream elf::Fatal(Ctx &ctx) { return {ctx, DiagLevel::Fatal}; }
+
Ctx::Ctx() : driver(*this) {}
void Ctx::reset() {
@@ -101,6 +109,8 @@ void Ctx::reset() {
script = nullptr;
target.reset();
+ errHandler = nullptr;
+
bufferStart = nullptr;
mainPart = nullptr;
tlsPhdr = nullptr;
@@ -169,6 +179,7 @@ bool link(ArrayRef<const char *> args, llvm::raw_ostream &stdoutOS,
Ctx &ctx = elf::ctx;
LinkerScript script(ctx);
ctx.script = &script;
+ ctx.errHandler = &context->e;
ctx.symAux.emplace_back();
ctx.symtab = std::make_unique<SymbolTable>(ctx);
@@ -366,7 +377,7 @@ void LinkerDriver::addFile(StringRef path, bool withLOption) {
files.push_back(createObjFile(ctx, mbref, "", inLib));
break;
default:
- error(path + ": unknown file type");
+ ErrAlways(ctx) << path << ": unknown file type";
}
}
@@ -2780,8 +2791,9 @@ static void readSecurityNotes(Ctx &ctx) {
if (ctx.arg.zForceBti && !(features & GNU_PROPERTY_AARCH64_FEATURE_1_BTI)) {
features |= GNU_PROPERTY_AARCH64_FEATURE_1_BTI;
if (ctx.arg.zBtiReport == "none")
- warn(toString(f) + ": -z force-bti: file does not have "
- "GNU_PROPERTY_AARCH64_FEATURE_1_BTI property");
+ Warn(ctx) << f
+ << ": -z force-bti: file does not have "
+ "GNU_PROPERTY_AARCH64_FEATURE_1_BTI property";
} else if (ctx.arg.zForceIbt &&
!(features & GNU_PROPERTY_X86_FEATURE_1_IBT)) {
if (ctx.arg.zCetReport == "none")
@@ -3048,7 +3060,7 @@ template <class ELFT> void LinkerDriver::link(opt::InputArgList &args) {
oldFilenames.insert(f->getName());
for (InputFile *newFile : newInputFiles)
if (!oldFilenames.contains(newFile->getName()))
- errorOrWarn("input file '" + newFile->getName() + "' added after LTO");
+ Err(ctx) << "input file '" << newFile->getName() << "' added after LTO";
}
// Handle --exclude-libs again because lto.tmp may reference additional
diff --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp
index 0d3db373138874..e9a75b37026fd2 100644
--- a/lld/ELF/InputFiles.cpp
+++ b/lld/ELF/InputFiles.cpp
@@ -66,6 +66,12 @@ std::string lld::toString(const InputFile *f) {
return std::string(f->toStringCache);
}
+const ELFSyncStream &elf::operator<<(const ELFSyncStream &s,
+ const InputFile *f) {
+ s << toString(f);
+ return s;
+}
+
static ELFKind getELFKind(MemoryBufferRef mb, StringRef archiveName) {
unsigned char size;
unsigned char endian;
diff --git a/lld/ELF/InputFiles.h b/lld/ELF/InputFiles.h
index f80413b215047d..e00e2c83d017c2 100644
--- a/lld/ELF/InputFiles.h
+++ b/lld/ELF/InputFiles.h
@@ -386,6 +386,8 @@ ELFFileBase *createObjFile(Ctx &, MemoryBufferRef mb,
std::string replaceThinLTOSuffix(Ctx &, StringRef path);
+const ELFSyncStream &operator<<(const ELFSyncStream &, const InputFile *);
+
} // namespace elf
} // namespace lld
diff --git a/lld/ELF/Target.cpp b/lld/ELF/Target.cpp
index 96d132a2a8f369..b38446f287bf71 100644
--- a/lld/ELF/Target.cpp
+++ b/lld/ELF/Target.cpp
@@ -45,6 +45,15 @@ std::string lld::toString(RelType type) {
return std::string(s);
}
+const ELFSyncStream &elf::operator<<(const ELFSyncStream &s, RelType type) {
+ StringRef buf = getELFRelocationTypeName(s.ctx.arg.emachine, type);
+ if (buf == "Unknown")
+ s << ("Unknown (" + Twine(type) + ")").str();
+ else
+ s << buf;
+ return s;
+}
+
void elf::setTarget(Ctx &ctx) {
switch (ctx.arg.emachine) {
case EM_386:
diff --git a/lld/ELF/Target.h b/lld/ELF/Target.h
index 27986b0a127e8e..250a29d6184313 100644
--- a/lld/ELF/Target.h
+++ b/lld/ELF/Target.h
@@ -327,6 +327,8 @@ inline uint64_t overwriteULEB128(uint8_t *bufLoc, uint64_t val) {
*bufLoc = val;
return val;
}
+
+const ELFSyncStream &operator<<(const ELFSyncStream &, RelType);
} // namespace elf
} // namespace lld
diff --git a/lld/include/lld/Common/ErrorHandler.h b/lld/include/lld/Common/ErrorHandler.h
index 0b69bb6202b32c..b9858b0e26ece9 100644
--- a/lld/include/lld/Common/ErrorHandler.h
+++ b/lld/include/lld/Common/ErrorHandler.h
@@ -73,11 +73,11 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/FileOutputBuffer.h"
+#include "llvm/Support/raw_ostream.h"
#include <mutex>
namespace llvm {
class DiagnosticInfo;
-class raw_ostream;
}
namespace lld {
@@ -152,6 +152,21 @@ void message(const Twine &msg, llvm::raw_ostream &s = outs());
void warn(const Twine &msg);
uint64_t errorCount();
+enum class DiagLevel { Log, Warn, Err, Fatal };
+
+// A class that synchronizes thread writing to the same stream similar
+// std::osyncstream.
+class SyncStream {
+ ErrorHandler &e;
+ DiagLevel level;
+ std::string buf;
+
+public:
+ mutable llvm::raw_string_ostream os{buf};
+ SyncStream(ErrorHandler &e, DiagLevel level) : e(e), level(level) {}
+ ~SyncStream();
+};
+
[[noreturn]] void exitLld(int val);
void diagnosticHandler(const llvm::DiagnosticInfo &di);
More information about the llvm-commits
mailing list