[ubsan] Don't merge non-trap handlers if -ubsan-unique-traps or not optimized#119302
Merged
[ubsan] Don't merge non-trap handlers if -ubsan-unique-traps or not optimized#119302
Conversation
UBSan handlers can sometimes be merged by the backend, which complicates debugging when a backtrace is not available (traps or min-rt handlers). -ubsan-unique-traps prevents merging for trap mode, but not for min-rt's (non-trap) handlers. This patch extends -ubsan-unique-traps to work for min-rt handlers. It does not change the behavior when using the regular ubsan runtime; the backtrace provides enough information for debugging, hence no-merge would simply increase code size with little benefit.
Member
|
@llvm/pr-subscribers-clang-codegen Author: Thurston Dang (thurstond) ChangesUBSan handler calls can sometimes be merged by the backend, which complicates debugging when a backtrace is not available (trap or min-rt). -ubsan-unique-traps prevents merging for trap mode, but not for min-rt's handler calls. This patch extends -ubsan-unique-traps to work for min-rt handlers. It does not change the behavior when using the regular ubsan runtime; the backtrace provides enough information for debugging, hence no-merge would simply increase code size with little benefit. N.B. "-ubsan-unique-traps" becomes somewhat of a misnomer since it will now apply to min-rt handlers as well as traps. We keep the naming for backwards compatibility. Full diff: https://github.com/llvm/llvm-project/pull/119302.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 0d16408aa4de9d..9a3f573fec7725 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -3581,6 +3581,12 @@ static void emitCheckHandlerCall(CodeGenFunction &CGF,
llvm::AttributeList::FunctionIndex, B),
/*Local=*/true);
llvm::CallInst *HandlerCall = CGF.EmitNounwindRuntimeCall(Fn, FnArgs);
+ bool NoMerge = ClSanitizeDebugDeoptimization ||
+ !CGF.CGM.getCodeGenOpts().OptimizationLevel ||
+ (CGF.CurCodeDecl && CGF.CurCodeDecl->hasAttr<OptimizeNoneAttr>());
+ // Regular runtime provides a backtrace, making NoMerge a waste of space
+ if (NoMerge && MinimalRuntime)
+ HandlerCall->addFnAttr(llvm::Attribute::NoMerge);
if (!MayReturn) {
HandlerCall->setDoesNotReturn();
CGF.Builder.CreateUnreachable();
diff --git a/clang/test/CodeGen/ubsan-trap-merge.c b/clang/test/CodeGen/ubsan-trap-merge.c
index e48681ce09377d..5c0760a539ec6a 100644
--- a/clang/test/CodeGen/ubsan-trap-merge.c
+++ b/clang/test/CodeGen/ubsan-trap-merge.c
@@ -266,4 +266,4 @@ int m(int x, int y) {
}
// TRAP: attributes #[[ATTR4]] = { nomerge noreturn nounwind }
// HANDLER: attributes #[[ATTR4]] = { noreturn nounwind }
-// MINRT: attributes #[[ATTR4]] = { noreturn nounwind }
+// MINRT: attributes #[[ATTR4]] = { nomerge noreturn nounwind }
|
Member
|
@llvm/pr-subscribers-clang Author: Thurston Dang (thurstond) ChangesUBSan handler calls can sometimes be merged by the backend, which complicates debugging when a backtrace is not available (trap or min-rt). -ubsan-unique-traps prevents merging for trap mode, but not for min-rt's handler calls. This patch extends -ubsan-unique-traps to work for min-rt handlers. It does not change the behavior when using the regular ubsan runtime; the backtrace provides enough information for debugging, hence no-merge would simply increase code size with little benefit. N.B. "-ubsan-unique-traps" becomes somewhat of a misnomer since it will now apply to min-rt handlers as well as traps. We keep the naming for backwards compatibility. Full diff: https://github.com/llvm/llvm-project/pull/119302.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 0d16408aa4de9d..9a3f573fec7725 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -3581,6 +3581,12 @@ static void emitCheckHandlerCall(CodeGenFunction &CGF,
llvm::AttributeList::FunctionIndex, B),
/*Local=*/true);
llvm::CallInst *HandlerCall = CGF.EmitNounwindRuntimeCall(Fn, FnArgs);
+ bool NoMerge = ClSanitizeDebugDeoptimization ||
+ !CGF.CGM.getCodeGenOpts().OptimizationLevel ||
+ (CGF.CurCodeDecl && CGF.CurCodeDecl->hasAttr<OptimizeNoneAttr>());
+ // Regular runtime provides a backtrace, making NoMerge a waste of space
+ if (NoMerge && MinimalRuntime)
+ HandlerCall->addFnAttr(llvm::Attribute::NoMerge);
if (!MayReturn) {
HandlerCall->setDoesNotReturn();
CGF.Builder.CreateUnreachable();
diff --git a/clang/test/CodeGen/ubsan-trap-merge.c b/clang/test/CodeGen/ubsan-trap-merge.c
index e48681ce09377d..5c0760a539ec6a 100644
--- a/clang/test/CodeGen/ubsan-trap-merge.c
+++ b/clang/test/CodeGen/ubsan-trap-merge.c
@@ -266,4 +266,4 @@ int m(int x, int y) {
}
// TRAP: attributes #[[ATTR4]] = { nomerge noreturn nounwind }
// HANDLER: attributes #[[ATTR4]] = { noreturn nounwind }
-// MINRT: attributes #[[ATTR4]] = { noreturn nounwind }
+// MINRT: attributes #[[ATTR4]] = { nomerge noreturn nounwind }
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
vitalybuka
reviewed
Dec 10, 2024
This reverts commit 1167d26.
vitalybuka
approved these changes
Dec 10, 2024
thurstond
added a commit
to thurstond/llvm-project
that referenced
this pull request
Dec 18, 2024
…d-handlers) '-mllvm -ubsan-unique-traps' (llvm#65972) applies to all UBSan checks. This patch introduces -fsanitize-nonmerged-handlers and -fno-sanitize-nonmerged-handlers, which allows selectively applying non-merged handlers to a subset of UBSan checks. N.B. we use "non-merged handlers" instead of "unique traps", since llvm#119302 has generalized it to work for non-trap mode as well (min-rt and regular rt). This patch does not remove the -ubsan-unique-traps flag; that will override -f(no-)sanitize-non-merged-handlers.
thurstond
added a commit
that referenced
this pull request
Dec 18, 2024
'-mllvm -ubsan-unique-traps' (#65972) applies to all UBSan checks. This patch introduces -fsanitize-merge (defaults to on, maintaining the status quo behavior) and -fno-sanitize-merge (equivalent to '-mllvm -ubsan-unique-traps'), with the option to selectively applying non-merged handlers to a subset of UBSan checks (e.g., -fno-sanitize-merge=bool,enum). N.B. we do not use "trap" in the argument name since #119302 has generalized -ubsan-unique-traps to work for non-trap modes (min-rt and regular rt). This patch does not remove the -ubsan-unique-traps flag; that will override -f(no-)sanitize-merge.
thurstond
added a commit
to thurstond/llvm-project
that referenced
this pull request
Dec 19, 2024
…#120464)" This reverts commit 2691b96. This reapply fixes the buildbot breakage of the original patch, by updating clang/test/CodeGen/ubsan-trap-debugloc.c to specify -fsanitize-merge (the default, which is merge, is applied by the driver but not clang_cc1). This reapply also expands clang/test/CodeGen/ubsan-trap-merge.c. Original commit message: '-mllvm -ubsan-unique-traps' (llvm#65972) applies to all UBSan checks. This patch introduces -fsanitize-merge (defaults to on, maintaining the status quo behavior) and -fno-sanitize-merge (equivalent to '-mllvm -ubsan-unique-traps'), with the option to selectively applying non-merged handlers to a subset of UBSan checks (e.g., -fno-sanitize-merge=bool,enum). N.B. we do not use "trap" in the argument name since llvm#119302 has generalized -ubsan-unique-traps to work for non-trap modes (min-rt and regular rt). This patch does not remove the -ubsan-unique-traps flag; that will override -f(no-)sanitize-merge.
thurstond
added a commit
that referenced
this pull request
Dec 19, 2024
…464)" (#120511) This reverts commit 2691b96. This reapply fixes the buildbot breakage of the original patch, by updating clang/test/CodeGen/ubsan-trap-debugloc.c to specify -fsanitize-merge (the default, which is merge, is applied by the driver but not clang_cc1). This reapply also expands clang/test/CodeGen/ubsan-trap-merge.c. ---- Original commit message: '-mllvm -ubsan-unique-traps' (#65972) applies to all UBSan checks. This patch introduces -fsanitize-merge (defaults to on, maintaining the status quo behavior) and -fno-sanitize-merge (equivalent to '-mllvm -ubsan-unique-traps'), with the option to selectively applying non-merged handlers to a subset of UBSan checks (e.g., -fno-sanitize-merge=bool,enum). N.B. we do not use "trap" in the argument name since #119302 has generalized -ubsan-unique-traps to work for non-trap modes (min-rt and regular rt). This patch does not remove the -ubsan-unique-traps flag; that will override -f(no-)sanitize-merge.
github-actions bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jan 10, 2025
…20464) '-mllvm -ubsan-unique-traps' (llvm/llvm-project#65972) applies to all UBSan checks. This patch introduces -fsanitize-merge (defaults to on, maintaining the status quo behavior) and -fno-sanitize-merge (equivalent to '-mllvm -ubsan-unique-traps'), with the option to selectively applying non-merged handlers to a subset of UBSan checks (e.g., -fno-sanitize-merge=bool,enum). N.B. we do not use "trap" in the argument name since llvm/llvm-project#119302 has generalized -ubsan-unique-traps to work for non-trap modes (min-rt and regular rt). This patch does not remove the -ubsan-unique-traps flag; that will override -f(no-)sanitize-merge.
github-actions bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jan 10, 2025
…erge) (#120…464)" (#120511) This reverts commit 2691b96. This reapply fixes the buildbot breakage of the original patch, by updating clang/test/CodeGen/ubsan-trap-debugloc.c to specify -fsanitize-merge (the default, which is merge, is applied by the driver but not clang_cc1). This reapply also expands clang/test/CodeGen/ubsan-trap-merge.c. ---- Original commit message: '-mllvm -ubsan-unique-traps' (llvm/llvm-project#65972) applies to all UBSan checks. This patch introduces -fsanitize-merge (defaults to on, maintaining the status quo behavior) and -fno-sanitize-merge (equivalent to '-mllvm -ubsan-unique-traps'), with the option to selectively applying non-merged handlers to a subset of UBSan checks (e.g., -fno-sanitize-merge=bool,enum). N.B. we do not use "trap" in the argument name since llvm/llvm-project#119302 has generalized -ubsan-unique-traps to work for non-trap modes (min-rt and regular rt). This patch does not remove the -ubsan-unique-traps flag; that will override -f(no-)sanitize-merge.
This was referenced Jun 2, 2025
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
UBSan handler calls can sometimes be merged by the backend, which complicates debugging. Merging is currently disabled for UBSan traps if -ubsan-unique-traps is specified or if optimization is disabled. This patch applies the same policy to non-trap handler calls.
N.B. "-ubsan-unique-traps" becomes somewhat of a misnomer since it will now apply to non-trap handler calls as well as traps. We keep the naming for backwards compatibility.