[clang] [ubsan] Change ubsan-unique-traps to use nomerge instead of counter (PR #117651)
Thurston Dang via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 25 16:39:26 PST 2024
https://github.com/thurstond created https://github.com/llvm/llvm-project/pull/117651
https://github.com/llvm/llvm-project/pull/65972 (continuation of https://reviews.llvm.org/D148654) had considered adding nomerge to ubsantrap, but did not proceed with that because of https://github.com/llvm/llvm-project/issues/53011. Instead, it added a counter (based on TrapBB->getParent()->size()) to each ubsantrap call. However, this counter is not guaranteed to be unique after inlining, as shown by https://github.com/llvm/llvm-project/pull/83470, which can result in ubsantraps being merged by the backend.
https://github.com/llvm/llvm-project/pull/101549 has since fixed the nomerge limitation ("It sets nomerge flag for the node if the instruction has nomerge arrtibute."). This patch therefore takes advantage of nomerge instead of using the counter, guaranteeing that the ubsantraps are not merged.
This patch is equivalent to https://github.com/llvm/llvm-project/pull/83470 but also adds nomerge and updates the test that was precommitted in https://github.com/llvm/llvm-project/pull/117649.
>From f24a87de48c42f310ee73ecf480ea2dcf631881f Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Tue, 26 Nov 2024 00:33:09 +0000
Subject: [PATCH] [ubsan] Change ubsan-unique-traps to use nomerge instead of
counter
https://github.com/llvm/llvm-project/pull/65972 (continuation of https://reviews.llvm.org/D148654) had considered adding nomerge to ubsantrap, but did not proceed with that because of https://github.com/llvm/llvm-project/issues/53011. Instead, it added a counter (based on TrapBB->getParent()->size()) to each ubsantrap call. However, this counter is not guaranteed to be unique after inlining, as shown by https://github.com/llvm/llvm-project/pull/83470, which can result in ubsantraps being merged by the backend.
https://github.com/llvm/llvm-project/pull/101549 fixed has since fixed the nomerge limitation ("It sets nomerge flag for the node if the instruction has nomerge arrtibute."). This patch therefore takes advantage of nomerge instead of using the counter, guaranteeing that the ubsantraps are not merged.
This patch is equivalent to https://github.com/llvm/llvm-project/pull/83470 but also adds nomerge and updates the test that was precommitted in https://github.com/llvm/llvm-project/pull/117649.
---
clang/lib/CodeGen/CGExpr.cpp | 6 +-
clang/test/CodeGen/bounds-checking.c | 4 +-
clang/test/CodeGen/ubsan-trap-merge.c | 106 ++++++++++++++++++++++++++
3 files changed, 110 insertions(+), 6 deletions(-)
create mode 100644 clang/test/CodeGen/ubsan-trap-merge.c
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index d3f470d401b3d4..f8c1e1cd7a4d68 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -3921,16 +3921,14 @@ void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
llvm::CallInst *TrapCall = Builder.CreateCall(
CGM.getIntrinsic(llvm::Intrinsic::ubsantrap),
- llvm::ConstantInt::get(CGM.Int8Ty,
- ClSanitizeDebugDeoptimization
- ? TrapBB->getParent()->size()
- : static_cast<uint64_t>(CheckHandlerID)));
+ llvm::ConstantInt::get(CGM.Int8Ty, CheckHandlerID));
if (!CGM.getCodeGenOpts().TrapFuncName.empty()) {
auto A = llvm::Attribute::get(getLLVMContext(), "trap-func-name",
CGM.getCodeGenOpts().TrapFuncName);
TrapCall->addFnAttr(A);
}
+ TrapCall->addFnAttr(llvm::Attribute::NoMerge);
TrapCall->setDoesNotReturn();
TrapCall->setDoesNotThrow();
Builder.CreateUnreachable();
diff --git a/clang/test/CodeGen/bounds-checking.c b/clang/test/CodeGen/bounds-checking.c
index 8100e30d0650ad..f6c4880e70a150 100644
--- a/clang/test/CodeGen/bounds-checking.c
+++ b/clang/test/CodeGen/bounds-checking.c
@@ -74,11 +74,11 @@ char B2[10];
// CHECK-LABEL: @f8
void f8(int i, int k) {
// NOOPTLOCAL: call void @llvm.ubsantrap(i8 3)
- // NOOPTARRAY: call void @llvm.ubsantrap(i8 2)
+ // NOOPTARRAY: call void @llvm.ubsantrap(i8 18)
B[i] = '\0';
// NOOPTLOCAL: call void @llvm.ubsantrap(i8 5)
- // NOOPTARRAY: call void @llvm.ubsantrap(i8 4)
+ // NOOPTARRAY: call void @llvm.ubsantrap(i8 18)
B2[k] = '\0';
}
diff --git a/clang/test/CodeGen/ubsan-trap-merge.c b/clang/test/CodeGen/ubsan-trap-merge.c
new file mode 100644
index 00000000000000..e6aa7902262813
--- /dev/null
+++ b/clang/test/CodeGen/ubsan-trap-merge.c
@@ -0,0 +1,106 @@
+// NOTE: Assertions have mostly been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 5
+// The most important assertion is the attributes at the end of the file, which
+// shows that ubsan attaches 'nomerge' to each ubsantrap intrinsic.
+//
+// RUN: %clang -fsanitize=signed-integer-overflow -S -emit-llvm -fsanitize-trap=all -O3 -mllvm -ubsan-unique-traps %s -o - \
+// RUN: | FileCheck %s
+
+#include <stdio.h>
+#include <stdlib.h>
+
+// CHECK-LABEL: define dso_local range(i32 -2147483523, -2147483648) i32 @f(
+// CHECK-SAME: i32 noundef [[X:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT: [[ENTRY:.*:]]
+// CHECK-NEXT: [[TMP0:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[X]], i32 125), !nosanitize [[META5:![0-9]+]]
+// CHECK-NEXT: [[TMP1:%.*]] = extractvalue { i32, i1 } [[TMP0]], 1, !nosanitize [[META5]]
+// CHECK-NEXT: br i1 [[TMP1]], label %[[TRAP:.*]], label %[[CONT:.*]], !nosanitize [[META5]]
+// CHECK: [[TRAP]]:
+// CHECK-NEXT: tail call void @llvm.ubsantrap(i8 0) #[[ATTR4:[0-9]+]], !nosanitize [[META5]]
+// CHECK-NEXT: unreachable, !nosanitize [[META5]]
+// CHECK: [[CONT]]:
+// CHECK-NEXT: [[TMP2:%.*]] = extractvalue { i32, i1 } [[TMP0]], 0, !nosanitize [[META5]]
+// CHECK-NEXT: ret i32 [[TMP2]]
+//
+int f(int x) {
+ return x + 125;
+}
+
+// CHECK-LABEL: define dso_local range(i32 -2147483521, -2147483648) i32 @g(
+// CHECK-SAME: i32 noundef [[X:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT: [[ENTRY:.*:]]
+// CHECK-NEXT: [[TMP0:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[X]], i32 127), !nosanitize [[META5]]
+// CHECK-NEXT: [[TMP1:%.*]] = extractvalue { i32, i1 } [[TMP0]], 1, !nosanitize [[META5]]
+// CHECK-NEXT: br i1 [[TMP1]], label %[[TRAP:.*]], label %[[CONT:.*]], !nosanitize [[META5]]
+// CHECK: [[TRAP]]:
+// CHECK-NEXT: tail call void @llvm.ubsantrap(i8 0) #[[ATTR4]], !nosanitize [[META5]]
+// CHECK-NEXT: unreachable, !nosanitize [[META5]]
+// CHECK: [[CONT]]:
+// CHECK-NEXT: [[TMP2:%.*]] = extractvalue { i32, i1 } [[TMP0]], 0, !nosanitize [[META5]]
+// CHECK-NEXT: ret i32 [[TMP2]]
+//
+int g(int x) {
+ return x + 127;
+}
+
+// CHECK-LABEL: define dso_local range(i32 -2147483521, -2147483648) i32 @h(
+// CHECK-SAME: i32 noundef [[X:%.*]], i32 noundef [[Y:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT: [[ENTRY:.*:]]
+// CHECK-NEXT: [[TMP0:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[X]], i32 127), !nosanitize [[META5]]
+// CHECK-NEXT: [[TMP1:%.*]] = extractvalue { i32, i1 } [[TMP0]], 1, !nosanitize [[META5]]
+// CHECK-NEXT: br i1 [[TMP1]], label %[[TRAP:.*]], label %[[CONT:.*]], !nosanitize [[META5]]
+// CHECK: [[TRAP]]:
+// CHECK-NEXT: tail call void @llvm.ubsantrap(i8 0) #[[ATTR4]], !nosanitize [[META5]]
+// CHECK-NEXT: unreachable, !nosanitize [[META5]]
+// CHECK: [[CONT]]:
+// CHECK-NEXT: [[TMP2:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[Y]], i32 129), !nosanitize [[META5]]
+// CHECK-NEXT: [[TMP3:%.*]] = extractvalue { i32, i1 } [[TMP2]], 1, !nosanitize [[META5]]
+// CHECK-NEXT: br i1 [[TMP3]], label %[[TRAP1:.*]], label %[[CONT2:.*]], !nosanitize [[META5]]
+// CHECK: [[TRAP1]]:
+// CHECK-NEXT: tail call void @llvm.ubsantrap(i8 0) #[[ATTR4]], !nosanitize [[META5]]
+// CHECK-NEXT: unreachable, !nosanitize [[META5]]
+// CHECK: [[CONT2]]:
+// CHECK-NEXT: [[TMP4:%.*]] = extractvalue { i32, i1 } [[TMP2]], 0, !nosanitize [[META5]]
+// CHECK-NEXT: [[TMP5:%.*]] = extractvalue { i32, i1 } [[TMP0]], 0, !nosanitize [[META5]]
+// CHECK-NEXT: [[COND:%.*]] = tail call i32 @llvm.smin.i32(i32 [[TMP5]], i32 [[TMP4]])
+// CHECK-NEXT: ret i32 [[COND]]
+//
+int h(int x, int y) {
+ x += 127;
+ y += 129;
+ return x < y ? x : y;
+}
+
+// CHECK-LABEL: define dso_local noundef i32 @m(
+// CHECK-SAME: i32 noundef [[X:%.*]], i32 noundef [[Y:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT: [[ENTRY:.*:]]
+// CHECK-NEXT: [[TMP0:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[X]], i32 125), !nosanitize [[META5]]
+// CHECK-NEXT: [[TMP1:%.*]] = extractvalue { i32, i1 } [[TMP0]], 1, !nosanitize [[META5]]
+// CHECK-NEXT: br i1 [[TMP1]], label %[[TRAP_I:.*]], label %[[F_EXIT:.*]], !nosanitize [[META5]]
+// CHECK: [[TRAP_I]]:
+// CHECK-NEXT: tail call void @llvm.ubsantrap(i8 0) #[[ATTR4]], !nosanitize [[META5]]
+// CHECK-NEXT: unreachable, !nosanitize [[META5]]
+// CHECK: [[F_EXIT]]:
+// CHECK-NEXT: [[TMP2:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[Y]], i32 127), !nosanitize [[META5]]
+// CHECK-NEXT: [[TMP3:%.*]] = extractvalue { i32, i1 } [[TMP2]], 1, !nosanitize [[META5]]
+// CHECK-NEXT: br i1 [[TMP3]], label %[[TRAP_I2:.*]], label %[[G_EXIT:.*]], !nosanitize [[META5]]
+// CHECK: [[TRAP_I2]]:
+// CHECK-NEXT: tail call void @llvm.ubsantrap(i8 0) #[[ATTR4]], !nosanitize [[META5]]
+// CHECK-NEXT: unreachable, !nosanitize [[META5]]
+// CHECK: [[G_EXIT]]:
+// CHECK-NEXT: [[TMP4:%.*]] = extractvalue { i32, i1 } [[TMP0]], 0, !nosanitize [[META5]]
+// CHECK-NEXT: [[TMP5:%.*]] = extractvalue { i32, i1 } [[TMP2]], 0, !nosanitize [[META5]]
+// CHECK-NEXT: [[TMP6:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[TMP4]], i32 [[TMP5]]), !nosanitize [[META5]]
+// CHECK-NEXT: [[TMP7:%.*]] = extractvalue { i32, i1 } [[TMP6]], 1, !nosanitize [[META5]]
+// CHECK-NEXT: br i1 [[TMP7]], label %[[TRAP:.*]], label %[[CONT:.*]], !nosanitize [[META5]]
+// CHECK: [[TRAP]]:
+// CHECK-NEXT: tail call void @llvm.ubsantrap(i8 0) #[[ATTR4]], !nosanitize [[META5]]
+// CHECK-NEXT: unreachable, !nosanitize [[META5]]
+// CHECK: [[CONT]]:
+// CHECK-NEXT: [[TMP8:%.*]] = extractvalue { i32, i1 } [[TMP6]], 0, !nosanitize [[META5]]
+// CHECK-NEXT: ret i32 [[TMP8]]
+//
+int m(int x, int y) {
+ return f(x) + g(y);
+}
+//.
+// CHECK: attributes #[[ATTR4]] = { nomerge noreturn nounwind }
More information about the cfe-commits
mailing list