[nfc][clang-offload-bundler] Don't leak on exit(1) #119178
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Created using spr 1.3.4
|
@llvm/pr-subscribers-clang Author: Vitaly Buka (vitalybuka) Changes
Usually lsan can see pointers of local allocations Fixes leak triggered by f7685af. Full diff: https://github.com/llvm/llvm-project/pull/119178.diff 1 Files Affected:
diff --git a/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp b/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
index 0189fe5d56ab2a..14c584064e311c 100644
--- a/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ b/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -196,13 +196,14 @@ int main(int argc, const char **argv) {
auto reportError = [argv](Error E) {
logAllUnhandledErrors(std::move(E), WithColor::error(errs(), argv[0]));
- exit(1);
+ return 1;
};
auto doWork = [&](std::function<llvm::Error()> Work) {
if (llvm::Error Err = Work()) {
- reportError(std::move(Err));
+ return reportError(std::move(Err));
}
+ return 0;
};
auto warningOS = [argv]() -> raw_ostream & {
@@ -223,14 +224,14 @@ int main(int argc, const char **argv) {
if (!Objcopy)
Objcopy = sys::findProgramByName("llvm-objcopy");
if (!Objcopy)
- reportError(createStringError(Objcopy.getError(),
- "unable to find 'llvm-objcopy' in path"));
+ return reportError(createStringError(
+ Objcopy.getError(), "unable to find 'llvm-objcopy' in path"));
else
BundlerConfig.ObjcopyPath = *Objcopy;
if (InputFileNames.getNumOccurrences() != 0 &&
InputFileNamesDeprecatedOpt.getNumOccurrences() != 0) {
- reportError(createStringError(
+ return reportError(createStringError(
errc::invalid_argument,
"-inputs and -input cannot be used together, use only -input instead"));
}
@@ -246,9 +247,9 @@ int main(int argc, const char **argv) {
if (OutputFileNames.getNumOccurrences() != 0 &&
OutputFileNamesDeprecatedOpt.getNumOccurrences() != 0) {
- reportError(createStringError(errc::invalid_argument,
- "-outputs and -output cannot be used "
- "together, use only -output instead"));
+ return reportError(createStringError(errc::invalid_argument,
+ "-outputs and -output cannot be used "
+ "together, use only -output instead"));
}
if (OutputFileNamesDeprecatedOpt.size()) {
@@ -262,77 +263,77 @@ int main(int argc, const char **argv) {
if (ListBundleIDs) {
if (Unbundle) {
- reportError(
+ return reportError(
createStringError(errc::invalid_argument,
"-unbundle and -list cannot be used together"));
}
if (InputFileNames.size() != 1) {
- reportError(createStringError(errc::invalid_argument,
- "only one input file supported for -list"));
+ return reportError(createStringError(
+ errc::invalid_argument, "only one input file supported for -list"));
}
if (OutputFileNames.size()) {
- reportError(createStringError(errc::invalid_argument,
- "-outputs option is invalid for -list"));
+ return reportError(createStringError(
+ errc::invalid_argument, "-outputs option is invalid for -list"));
}
if (TargetNames.size()) {
- reportError(createStringError(errc::invalid_argument,
- "-targets option is invalid for -list"));
+ return reportError(createStringError(
+ errc::invalid_argument, "-targets option is invalid for -list"));
}
- doWork([&]() { return OffloadBundler::ListBundleIDsInFile(
- InputFileNames.front(),
- BundlerConfig); });
- return 0;
+ return doWork([&]() {
+ return OffloadBundler::ListBundleIDsInFile(InputFileNames.front(),
+ BundlerConfig);
+ });
}
if (BundlerConfig.CheckInputArchive) {
if (!Unbundle) {
- reportError(createStringError(errc::invalid_argument,
- "-check-input-archive cannot be used while "
- "bundling"));
+ return reportError(createStringError(
+ errc::invalid_argument, "-check-input-archive cannot be used while "
+ "bundling"));
}
if (Unbundle && BundlerConfig.FilesType != "a") {
- reportError(createStringError(errc::invalid_argument,
- "-check-input-archive can only be used for "
- "unbundling archives (-type=a)"));
+ return reportError(createStringError(
+ errc::invalid_argument, "-check-input-archive can only be used for "
+ "unbundling archives (-type=a)"));
}
}
if (OutputFileNames.size() == 0) {
- reportError(
+ return reportError(
createStringError(errc::invalid_argument, "no output file specified!"));
}
if (TargetNames.getNumOccurrences() == 0) {
- reportError(createStringError(
+ return reportError(createStringError(
errc::invalid_argument,
"for the --targets option: must be specified at least once!"));
}
if (Unbundle) {
if (InputFileNames.size() != 1) {
- reportError(createStringError(
+ return reportError(createStringError(
errc::invalid_argument,
"only one input file supported in unbundling mode"));
}
if (OutputFileNames.size() != TargetNames.size()) {
- reportError(createStringError(errc::invalid_argument,
- "number of output files and targets should "
- "match in unbundling mode"));
+ return reportError(createStringError(
+ errc::invalid_argument, "number of output files and targets should "
+ "match in unbundling mode"));
}
} else {
if (BundlerConfig.FilesType == "a") {
- reportError(createStringError(errc::invalid_argument,
- "Archive files are only supported "
- "for unbundling"));
+ return reportError(createStringError(errc::invalid_argument,
+ "Archive files are only supported "
+ "for unbundling"));
}
if (OutputFileNames.size() != 1) {
- reportError(createStringError(
- errc::invalid_argument,
- "only one output file supported in bundling mode"));
+ return reportError(
+ createStringError(errc::invalid_argument,
+ "only one output file supported in bundling mode"));
}
if (InputFileNames.size() != TargetNames.size()) {
- reportError(createStringError(
+ return reportError(createStringError(
errc::invalid_argument,
"number of input files and targets should match in bundling mode"));
}
@@ -350,8 +351,8 @@ int main(int argc, const char **argv) {
std::vector<std::string> StandardizedTargetNames;
for (StringRef Target : TargetNames) {
if (!ParsedTargets.insert(Target).second) {
- reportError(createStringError(errc::invalid_argument,
- "Duplicate targets are not allowed"));
+ return reportError(createStringError(
+ errc::invalid_argument, "Duplicate targets are not allowed"));
}
auto OffloadInfo = OffloadTargetInfo(Target, BundlerConfig);
@@ -368,7 +369,7 @@ int main(int argc, const char **argv) {
Msg << ", unknown offloading kind '" << OffloadInfo.OffloadKind << "'";
if (!TripleIsValid)
Msg << ", unknown target triple '" << OffloadInfo.Triple.str() << "'";
- reportError(createStringError(errc::invalid_argument, Msg.str()));
+ return reportError(createStringError(errc::invalid_argument, Msg.str()));
}
TargetIDs[OffloadInfo.OffloadKind.str() + "-" + OffloadInfo.Triple.str()]
@@ -395,7 +396,7 @@ int main(int argc, const char **argv) {
Msg << "Cannot bundle inputs with conflicting targets: '"
<< TargetID.first + "-" + ConflictingTID->first << "' and '"
<< TargetID.first + "-" + ConflictingTID->second << "'";
- reportError(createStringError(errc::invalid_argument, Msg.str()));
+ return reportError(createStringError(errc::invalid_argument, Msg.str()));
}
}
@@ -408,14 +409,14 @@ int main(int argc, const char **argv) {
// treat missing host triple as error if we do unbundling.
if ((Unbundle && HostTargetNum > 1) ||
(!Unbundle && HostTargetNum != 1 && !BundlerConfig.AllowNoHost)) {
- reportError(createStringError(errc::invalid_argument,
- "expecting exactly one host target but got " +
- Twine(HostTargetNum)));
+ return reportError(createStringError(
+ errc::invalid_argument,
+ "expecting exactly one host target but got " + Twine(HostTargetNum)));
}
OffloadBundler Bundler(BundlerConfig);
- doWork([&]() {
+ return doWork([&]() {
if (Unbundle) {
if (BundlerConfig.FilesType == "a")
return Bundler.UnbundleArchive();
@@ -424,5 +425,4 @@ int main(int argc, const char **argv) {
} else
return Bundler.BundleFiles();
});
- return 0;
}
|
vitalybuka
deleted the
users/vitalybuka/spr/nfcclang-offload-bundler-dont-leak-on-exit1
branch
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Mar 28, 2025
`exit(1)` does not calls C++ destructors, however, it calls `at_exit()` handlers, including lsan. Usually lsan can see pointers of local allocations on the stack or in registers, but those can be discarded by `noreturn` `exit` call. Fixes leak triggered by f7685af. (cherry picked from commit d5fe633)
rocm-ci
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
May 20, 2025
`exit(1)` does not calls C++ destructors, however, it calls `at_exit()` handlers, including lsan. Usually lsan can see pointers of local allocations on the stack or in registers, but those can be discarded by `noreturn` `exit` call. Fixes leak triggered by f7685af. (cherry picked from commit d5fe633)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
exit(1)does not calls C++ destructors, however, it callsat_exit()handlers, including lsan.Usually lsan can see pointers of local allocations
on the stack or in registers, but those can be
discarded by
noreturnexitcall.Fixes leak triggered by f7685af.