[llvm] [llvm-profdata] Fix memory leak caused by exit() (PR #228139)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 1 10:04:00 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-pgo
Author: Vitaly Buka (vitalybuka)
<details>
<summary>Changes</summary>
- **[NFC][llvm-profdata] Introduce ProfdataError, makeError, and reportError**
- **[NFCI][llvm-profdata] Propagate Error in order subcommand**
- **[NFCI][llvm-profdata] Propagate Error in overlap subcommand**
- **[NFCI][llvm-profdata] Propagate Error in show subcommand**
- **[NFCI][llvm-profdata] Propagate Error in merge subcommand**
- **[NFCI][llvm-profdata] Propagate Error in loadInput and mergeWriterContexts**
- **[llvm-profdata] Remove exitWithError and LSan leak workaround**
---
Patch is 51.52 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/228139.diff
1 Files Affected:
- (modified) llvm/tools/llvm-profdata/llvm-profdata.cpp (+352-251)
``````````diff
diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp
index a8bf8b8760c4419..02bd24c394ef03a 100644
--- a/llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -50,15 +50,6 @@
#include <cmath>
#include <optional>
-#if LLVM_ADDRESS_SANITIZER_BUILD || LLVM_HWADDRESS_SANITIZER_BUILD
-#include <sanitizer/lsan_interface.h>
-static int SkipLeakCheck;
-LLVM_ATTRIBUTE_USED int __lsan_is_turned_off() { return SkipLeakCheck; }
-static void skipLeakCheck() { SkipLeakCheck = 1; }
-#else
-static void skipLeakCheck() {}
-#endif
-
using namespace llvm;
using ProfCorrelatorKind = InstrProfCorrelator::ProfCorrelatorKind;
@@ -544,48 +535,85 @@ static void warn(Error E, StringRef Whence = "") {
}
}
-static void exitWithError(Twine Message, StringRef Whence = "",
- StringRef Hint = "") {
- WithColor::error();
- if (!Whence.empty())
- errs() << Whence << ": ";
- errs() << Message << "\n";
- if (!Hint.empty())
- WithColor::note() << Hint << "\n";
- // exit() terminates without unwinding the stack or running destructors, and
- // there is no guaranty that pointers to allocations will be preserved, so
- // LSan reports in-flight heap allocations as leaks at atexit.
- skipLeakCheck();
- ::exit(1);
+namespace {
+class ProfdataError : public ErrorInfo<ProfdataError> {
+public:
+ static char ID;
+
+ ProfdataError(Twine Message, Twine Whence = "", Twine Hint = "")
+ : Message(Message.str()), Whence(Whence.str()), Hint(Hint.str()) {}
+
+ void log(raw_ostream &OS) const override {
+ if (!Whence.empty())
+ OS << Whence << ": ";
+ OS << Message;
+ }
+
+ void print() const {
+ WithColor::error();
+ log(errs());
+ errs() << "\n";
+ if (!Hint.empty())
+ WithColor::note() << Hint << "\n";
+ }
+
+ std::error_code convertToErrorCode() const override {
+ return inconvertibleErrorCode();
+ }
+
+private:
+ std::string Message;
+ std::string Whence;
+ std::string Hint;
+};
+
+char ProfdataError::ID = 0;
+} // namespace
+
+static Error makeError(Twine Message, StringRef Whence = "",
+ StringRef Hint = "") {
+ return make_error<ProfdataError>(Message, Whence, Hint);
}
-static void exitWithError(Error E, StringRef Whence = "") {
+static Error makeError(Error E, StringRef Whence = "") {
if (E.isA<InstrProfError>()) {
+ std::string Msg;
+ std::string Hint;
handleAllErrors(std::move(E), [&](const InstrProfError &IPE) {
instrprof_error instrError = IPE.get();
- StringRef Hint = "";
if (instrError == instrprof_error::unrecognized_format) {
// Hint in case user missed specifying the profile type.
Hint = "Perhaps you forgot to use the --sample or --memory option?";
}
- exitWithError(IPE.message(), Whence, Hint);
+ Msg = IPE.message();
});
- return;
+ return makeError(Msg, Whence, Hint);
}
- exitWithError(toString(std::move(E)), Whence);
+ return makeError(toString(std::move(E)), Whence);
+}
+
+static Error makeError(std::error_code EC, StringRef Whence = "") {
+ return makeError(EC.message(), Whence);
}
-static void exitWithErrorCode(std::error_code EC, StringRef Whence = "") {
- exitWithError(EC.message(), Whence);
+static int reportError(Error E) {
+ if (!E)
+ return 0;
+ handleAllErrors(
+ std::move(E), [](const ProfdataError &PE) { PE.print(); },
+ [](const ErrorInfoBase &EIB) {
+ WithColor::error() << EIB.message() << "\n";
+ });
+ return 1;
}
-static void warnOrExitGivenError(FailureMode FailMode, std::error_code EC,
- StringRef Whence = "") {
+static Error warnOrErrorGivenError(FailureMode FailMode, std::error_code EC,
+ StringRef Whence = "") {
if (FailMode == failIfAnyAreInvalid)
- exitWithErrorCode(EC, Whence);
- else
- warn(EC.message(), Whence);
+ return makeError(EC, Whence);
+ warn(EC.message(), Whence);
+ return Error::success();
}
static void handleMergeWriterError(Error E, StringRef WhenceFile = "",
@@ -633,10 +661,10 @@ class SymbolRemapper {
public:
/// Build a SymbolRemapper from a file containing a list of old/new symbols.
- static std::unique_ptr<SymbolRemapper> create(StringRef InputFile) {
+ static Expected<std::unique_ptr<SymbolRemapper>> create(StringRef InputFile) {
auto BufOrError = MemoryBuffer::getFileOrSTDIN(InputFile);
if (!BufOrError)
- exitWithErrorCode(BufOrError.getError(), InputFile);
+ return makeError(BufOrError.getError(), InputFile);
auto Remapper = std::make_unique<SymbolRemapper>();
Remapper->File = std::move(BufOrError.get());
@@ -646,13 +674,13 @@ class SymbolRemapper {
std::pair<StringRef, StringRef> Parts = LineIt->split(' ');
if (Parts.first.empty() || Parts.second.empty() ||
Parts.second.count(' ')) {
- exitWithError("unexpected line in remapping file",
- (InputFile + ":" + Twine(LineIt.line_number())).str(),
- "expected 'old_symbol new_symbol'");
+ return makeError("unexpected line in remapping file",
+ (InputFile + ":" + Twine(LineIt.line_number())).str(),
+ "expected 'old_symbol new_symbol'");
}
Remapper->RemappingTable.insert(Parts);
}
- return Remapper;
+ return std::move(Remapper);
}
/// Attempt to map the given old symbol into a new symbol.
@@ -694,6 +722,11 @@ struct WriterContext {
MemProfVersionRequested, MemProfFullSchema,
MemprofGenerateRandomHotness, RandomSeed),
ErrLock(ErrLock), WriterErrorCodes(WriterErrorCodes) {}
+
+ ~WriterContext() {
+ for (auto &ErrorPair : Errors)
+ consumeError(std::move(ErrorPair.first));
+ }
};
/// Computer the overlap b/w profile BaseFilename and TestFileName,
@@ -725,7 +758,7 @@ static void overlapInput(const std::string &BaseFilename,
}
/// Load an input into a writer context.
-static void
+static Error
loadInput(const WeightedFile &Input, SymbolRemapper *Remapper,
const InstrProfCorrelator *Correlator, const StringRef ProfiledBinary,
WriterContext *WC, const object::BuildIDFetcher *BIDFetcher = nullptr,
@@ -740,9 +773,8 @@ loadInput(const WeightedFile &Input, SymbolRemapper *Remapper,
using ::llvm::memprof::RawMemProfReader;
if (RawMemProfReader::hasFormat(Input.Filename)) {
auto ReaderOrErr = RawMemProfReader::create(Input.Filename, ProfiledBinary);
- if (!ReaderOrErr) {
- exitWithError(ReaderOrErr.takeError(), Input.Filename);
- }
+ if (!ReaderOrErr)
+ return makeError(ReaderOrErr.takeError(), Input.Filename);
std::unique_ptr<RawMemProfReader> Reader = std::move(ReaderOrErr.get());
// Check if the profile types can be merged, e.g. clang frontend profiles
// should not be merged with memprof profiles.
@@ -753,7 +785,7 @@ loadInput(const WeightedFile &Input, SymbolRemapper *Remapper,
"Cannot merge MemProf profile with Clang generated profile.",
std::error_code()),
Filename);
- return;
+ return Error::success();
}
auto MemProfError = [&](Error E) {
@@ -763,14 +795,14 @@ loadInput(const WeightedFile &Input, SymbolRemapper *Remapper,
};
WC->Writer.addMemProfData(Reader->takeMemProfData(), MemProfError);
- return;
+ return Error::success();
}
using ::llvm::memprof::YAMLMemProfReader;
if (YAMLMemProfReader::hasFormat(Input.Filename)) {
auto ReaderOrErr = YAMLMemProfReader::create(Input.Filename);
if (!ReaderOrErr)
- exitWithError(ReaderOrErr.takeError(), Input.Filename);
+ return makeError(ReaderOrErr.takeError(), Input.Filename);
std::unique_ptr<YAMLMemProfReader> Reader = std::move(ReaderOrErr.get());
// Check if the profile types can be merged, e.g. clang frontend profiles
// should not be merged with memprof profiles.
@@ -781,7 +813,7 @@ loadInput(const WeightedFile &Input, SymbolRemapper *Remapper,
"Cannot merge MemProf profile with incompatible profile.",
std::error_code()),
Filename);
- return;
+ return Error::success();
}
auto MemProfError = [&](Error E) {
@@ -804,7 +836,7 @@ loadInput(const WeightedFile &Input, SymbolRemapper *Remapper,
WC->Writer.addMemProfData(std::move(MemProfData), MemProfError);
WC->Writer.addDataAccessProfData(std::move(DataAccessProfData));
- return;
+ return Error::success();
}
auto FS = vfs::getRealFileSystem();
@@ -840,13 +872,13 @@ loadInput(const WeightedFile &Input, SymbolRemapper *Remapper,
if (ErrCode != instrprof_error::empty_raw_profile)
WC->Errors.emplace_back(make_error<InstrProfError>(ErrCode, Msg),
Filename);
- return;
+ return Error::success();
}
auto Reader = std::move(ReaderOrErr.get());
if (Error E = WC->Writer.mergeProfileKind(Reader->getProfileKind())) {
WC->Errors.emplace_back(std::move(E), Filename);
- return;
+ return Error::success();
}
for (auto &I : *Reader) {
@@ -887,14 +919,14 @@ loadInput(const WeightedFile &Input, SymbolRemapper *Remapper,
if (Reader->hasError()) {
if (Error E = Reader->getError()) {
WC->Errors.emplace_back(std::move(E), Filename);
- return;
+ return Error::success();
}
}
std::vector<llvm::object::BuildID> BinaryIds;
if (Error E = Reader->readBinaryIds(BinaryIds)) {
WC->Errors.emplace_back(std::move(E), Filename);
- return;
+ return Error::success();
}
WC->Writer.addBinaryIds(BinaryIds);
@@ -902,16 +934,17 @@ loadInput(const WeightedFile &Input, SymbolRemapper *Remapper,
WC->Errors.emplace_back(std::move(ReaderWarning->first),
ReaderWarning->second);
}
+ return Error::success();
}
/// Merge the \p Src writer context into \p Dst.
-static void mergeWriterContexts(WriterContext *Dst, WriterContext *Src) {
+static Error mergeWriterContexts(WriterContext *Dst, WriterContext *Src) {
for (auto &ErrorPair : Src->Errors)
Dst->Errors.push_back(std::move(ErrorPair));
Src->Errors.clear();
if (Error E = Dst->Writer.mergeProfileKind(Src->Writer.getProfileKind()))
- exitWithError(std::move(E));
+ return makeError(std::move(E));
Dst->Writer.mergeRecordsFromWriter(std::move(Src->Writer), [&](Error E) {
auto [ErrorCode, Msg] = InstrProfError::take(std::move(E));
@@ -920,6 +953,7 @@ static void mergeWriterContexts(WriterContext *Dst, WriterContext *Src) {
if (firstTime)
warn(toString(make_error<InstrProfError>(ErrorCode, Msg)));
});
+ return Error::success();
}
static StringRef
@@ -932,12 +966,11 @@ getFuncName(const SampleProfileMap::value_type &Val) {
return Val.second.getContext().toString();
}
-template <typename T>
-static void filterFunctions(T &ProfileMap) {
+template <typename T> static Error filterFunctions(T &ProfileMap) {
bool hasFilter = !FuncNameFilter.empty();
bool hasNegativeFilter = !FuncNameNegativeFilter.empty();
if (!hasFilter && !hasNegativeFilter)
- return;
+ return Error::success();
// If filter starts with '?' it is MSVC mangled name, not a regex.
llvm::Regex ProbablyMSVCMangledName("[?@$_0-9A-Za-z]+");
@@ -951,11 +984,11 @@ static void filterFunctions(T &ProfileMap) {
size_t Count = ProfileMap.size();
llvm::Regex Pattern(FuncNameFilter);
llvm::Regex NegativePattern(FuncNameNegativeFilter);
- std::string Error;
- if (hasFilter && !Pattern.isValid(Error))
- exitWithError(Error);
- if (hasNegativeFilter && !NegativePattern.isValid(Error))
- exitWithError(Error);
+ std::string RegexError;
+ if (hasFilter && !Pattern.isValid(RegexError))
+ return makeError(RegexError);
+ if (hasNegativeFilter && !NegativePattern.isValid(RegexError))
+ return makeError(RegexError);
// Handle MD5 profile, so it is still able to match using the original name.
std::string MD5Name = std::to_string(llvm::MD5Hash(FuncNameFilter));
@@ -974,51 +1007,55 @@ static void filterFunctions(T &ProfileMap) {
llvm::dbgs() << Count - ProfileMap.size() << " of " << Count << " functions "
<< "in the original profile are filtered.\n";
+ return Error::success();
}
-static void writeInstrProfile(StringRef OutputFilename,
- ProfileFormat OutputFormat,
- InstrProfWriter &Writer) {
+static Error writeInstrProfile(StringRef OutputFilename,
+ ProfileFormat OutputFormat,
+ InstrProfWriter &Writer) {
std::error_code EC;
raw_fd_ostream Output(OutputFilename.data(), EC,
OutputFormat == PF_Text ? sys::fs::OF_TextWithCRLF
: sys::fs::OF_None);
if (EC)
- exitWithErrorCode(EC, OutputFilename);
+ return makeError(EC, OutputFilename);
if (OutputFormat == PF_Text) {
if (Error E = Writer.writeText(Output))
warn(std::move(E));
} else {
if (Output.is_displayed())
- exitWithError("cannot write a non-text format profile to the terminal");
+ return makeError(
+ "cannot write a non-text format profile to the terminal");
if (Error E = Writer.write(Output))
warn(std::move(E));
}
+ return Error::success();
}
-static void mergeInstrProfile(const WeightedFileVector &Inputs,
- SymbolRemapper *Remapper,
- int MaxDbgCorrelationWarnings,
- const StringRef ProfiledBinary) {
+static Error mergeInstrProfile(const WeightedFileVector &Inputs,
+ SymbolRemapper *Remapper,
+ int MaxDbgCorrelationWarnings,
+ const StringRef ProfiledBinary) {
const uint64_t TraceReservoirSize = TemporalProfTraceReservoirSize.getValue();
const uint64_t MaxTraceLength = TemporalProfMaxTraceLength.getValue();
if (OutputFormat == PF_Compact_Binary)
- exitWithError("Compact Binary is deprecated");
+ return makeError("Compact Binary is deprecated");
if (OutputFormat != PF_Binary && OutputFormat != PF_Ext_Binary &&
OutputFormat != PF_Text)
- exitWithError("unknown format is specified");
+ return makeError("unknown format is specified");
// TODO: Maybe we should support correlation with mixture of different
// correlation modes(w/wo debug-info/object correlation).
if (DebugInfoFilename.empty()) {
if (!BinaryFilename.empty() && (DebugInfod || !DebugFileDirectory.empty()))
- exitWithError("Expected only one of -binary-file, -debuginfod or "
- "-debug-file-directory");
+ return makeError("Expected only one of -binary-file, -debuginfod or "
+ "-debug-file-directory");
} else if (!BinaryFilename.empty() || DebugInfod ||
!DebugFileDirectory.empty()) {
- exitWithError("Expected only one of -debug-info, -binary-file, -debuginfod "
- "or -debug-file-directory");
+ return makeError(
+ "Expected only one of -debug-info, -binary-file, -debuginfod "
+ "or -debug-file-directory");
}
std::string CorrelateFilename;
ProfCorrelatorKind CorrelateKind = ProfCorrelatorKind::NONE;
@@ -1034,9 +1071,9 @@ static void mergeInstrProfile(const WeightedFileVector &Inputs,
if (CorrelateKind != InstrProfCorrelator::NONE) {
if (auto Err = InstrProfCorrelator::get(CorrelateFilename, CorrelateKind)
.moveInto(Correlator))
- exitWithError(std::move(Err), CorrelateFilename);
+ return makeError(std::move(Err), CorrelateFilename);
if (auto Err = Correlator->correlateProfileData(MaxDbgCorrelationWarnings))
- exitWithError(std::move(Err), CorrelateFilename);
+ return makeError(std::move(Err), CorrelateFilename);
}
ProfCorrelatorKind BIDFetcherCorrelateKind = ProfCorrelatorKind::NONE;
@@ -1045,17 +1082,17 @@ static void mergeInstrProfile(const WeightedFileVector &Inputs,
llvm::HTTPClient::initialize();
BIDFetcher = std::make_unique<DebuginfodFetcher>(DebugFileDirectory);
if (!BIDFetcherProfileCorrelate)
- exitWithError("Expected --correlate when --debuginfod is provided");
+ return makeError("Expected --correlate when --debuginfod is provided");
BIDFetcherCorrelateKind = BIDFetcherProfileCorrelate;
} else if (!DebugFileDirectory.empty()) {
BIDFetcher = std::make_unique<object::BuildIDFetcher>(DebugFileDirectory);
if (!BIDFetcherProfileCorrelate)
- exitWithError("Expected --correlate when --debug-file-directory "
- "is provided");
+ return makeError("Expected --correlate when --debug-file-directory "
+ "is provided");
BIDFetcherCorrelateKind = BIDFetcherProfileCorrelate;
} else if (BIDFetcherProfileCorrelate) {
- exitWithError("Expected --debuginfod or --debug-file-directory when "
- "--correlate is provided");
+ return makeError("Expected --debuginfod or --debug-file-directory when "
+ "--correlate is provided");
}
std::mutex ErrorLock;
@@ -1075,20 +1112,46 @@ static void mergeInstrProfile(const WeightedFileVector &Inputs,
if (NumThreads == 1) {
for (const auto &Input : Inputs)
- loadInput(Input, Remapper, Correlator.get(), ProfiledBinary,
- Contexts[0].get(), BIDFetcher.get(), &BIDFetcherCorrelateKind);
+ if (Error E = loadInput(Input, Remapper, Correlator.get(), ProfiledBinary,
+ Contexts[0].get(), BIDFetcher.get(),
+ &BIDFetcherCorrelateKind))
+ return E;
} else {
+ Error FatalError = Error::success();
+ auto hasFatalError = [&] {
+ std::unique_lock<std::mutex> ErrGuard{ErrorLock};
+ return static_cast<bool>(FatalError);
+ };
+
DefaultThreadPool Pool(hardware_concurrency(NumThreads));
+ auto Async = [&](auto F, auto &&...Args) {
+ Pool.async(
+ [&, F](auto &&...InnerArgs) {
+ if (hasFatalError())
+ return;
+ if (Error E = F(std::forward<decltype(InnerArgs)>(InnerArgs)...)) {
+ std::unique_lock<std::mutex> ErrGuard{ErrorLock};
+ if (FatalError)
+ consumeError(std::move(E));
+ else
+ FatalError = std::move(E);
+ }
+ },
+ std::forward<decltype(Args)>(Args)...);
+ };
// Load the inputs in parallel (N/NumThreads serial steps).
unsigned Ctx = 0;
for (const auto &Input : Inputs) {
- Pool.async(loadInput, Input, Remapper, Correlator.get(), ProfiledBinary,
- Contexts[Ctx].get(), BIDFetcher.get(),
- &BIDFetcherCorrelateKind);
+ if (hasFatalError())
+ break;
+ Async(loadInput, Input, Remapper, Correlator.get(), ProfiledBinary,
+ Contexts[Ctx].get(), BIDFetcher.get(), &BIDFetcherCorrelateKind);
Ctx = (Ctx + 1) % NumThreads;
}
Pool.wait();
+ if (FatalError)
+ return FatalError;
// Merge the writer contexts together (~ lg(NumThreads) serial steps).
unsigned Mid = Contexts.size() / 2;
@@ -1096,13 +1159,15 @@ static void mergeInstrProfile(const WeightedFileVector &Inputs,
assert(Mid > 0 && "Expected more than one context");
do {
for (unsigned I = 0; I < Mid; ++I)
- Pool.async(mergeWriterContexts, Contexts[I].get(),
- Contexts[I + Mid].get());
+ Async(mergeWriterContexts, Contexts[I].get(), Contexts[I + Mid].get());
Pool.wait();
+ if (FatalError)
+ return FatalError;
if (End & 1) {
- Pool.async(mergeWriterContexts, Contexts[0].get(),
- Contexts[End - 1].get());
+ Async(mergeWriterContexts, Contexts[0].get(), Contexts[End - 1].get());
Pool.wait();
+ if (FatalError)
+ return FatalError;
}
End = Mid;
Mid /= 2;
@@ -1120,11 +1185,12 @@ static void mergeInstrProfile(const WeightedFileVector &Inputs,
}
if ((NumErrors == Inputs.size() && FailMode == failIfAllAreInvalid) ||
(NumErrors > 0 && FailMode == failIfAny...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/228139
More information about the llvm-commits
mailing list