llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Thurston Dang (thurstond) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/117651.diff 3 Files Affected: - (modified) clang/lib/CodeGen/CGExpr.cpp (+2-4) - (modified) clang/test/CodeGen/bounds-checking.c (+2-2) - (added) clang/test/CodeGen/ubsan-trap-merge.c (+106) ``````````diff 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 } `````````` </details> https://github.com/llvm/llvm-project/pull/117651 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
