Skip to content

Conversation

fmayer
Copy link
Contributor

@fmayer fmayer commented Apr 3, 2025

Trap checks fail at most once (when the program crashes).

fmayer added 6 commits April 3, 2025 14:53
Created using spr 1.3.4
Created using spr 1.3.4
Created using spr 1.3.4
Created using spr 1.3.4
Created using spr 1.3.4
Created using spr 1.3.4
@fmayer fmayer requested a review from thurstond April 4, 2025 02:11
@fmayer fmayer marked this pull request as ready for review April 4, 2025 02:11
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. labels Apr 4, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 4, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: Florian Mayer (fmayer)

Changes

Trap checks fail at most once (when the program crashes).


Patch is 22.09 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/134310.diff

6 Files Affected:

  • (modified) clang/lib/CodeGen/CGExpr.cpp (+5-2)
  • (modified) clang/test/CodeGen/allow-ubsan-check.c (+13-11)
  • (modified) clang/test/CodeGen/bounds-checking-debuginfo.c (+4-3)
  • (modified) clang/test/CodeGen/cfi-check-fail.c (+3-3)
  • (modified) clang/test/CodeGen/cfi-check-fail2.c (+5-5)
  • (modified) clang/test/CodeGen/ubsan-trap-merge.c (+22-14)
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 3d3a111f0514a..56e5654bcf7e0 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -3977,16 +3977,19 @@ void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
   NoMerge = NoMerge || !CGM.getCodeGenOpts().OptimizationLevel ||
             (CurCodeDecl && CurCodeDecl->hasAttr<OptimizeNoneAttr>());
 
+  llvm::MDBuilder MDHelper(getLLVMContext());
   if (TrapBB && !NoMerge) {
     auto Call = TrapBB->begin();
     assert(isa<llvm::CallInst>(Call) && "Expected call in trap BB");
 
     Call->applyMergedLocation(Call->getDebugLoc(),
                               Builder.getCurrentDebugLocation());
-    Builder.CreateCondBr(Checked, Cont, TrapBB);
+    Builder.CreateCondBr(Checked, Cont, TrapBB,
+                         MDHelper.createLikelyBranchWeights());
   } else {
     TrapBB = createBasicBlock("trap");
-    Builder.CreateCondBr(Checked, Cont, TrapBB);
+    Builder.CreateCondBr(Checked, Cont, TrapBB,
+                         MDHelper.createLikelyBranchWeights());
     EmitBlock(TrapBB);
 
     llvm::CallInst *TrapCall =
diff --git a/clang/test/CodeGen/allow-ubsan-check.c b/clang/test/CodeGen/allow-ubsan-check.c
index c116604288546..e225fb63f08eb 100644
--- a/clang/test/CodeGen/allow-ubsan-check.c
+++ b/clang/test/CodeGen/allow-ubsan-check.c
@@ -49,7 +49,7 @@
 // TR-NEXT:    [[TMP7:%.*]] = xor i1 [[TMP6]], true, !nosanitize [[META2]]
 // TR-NEXT:    [[TMP8:%.*]] = or i1 [[OR]], [[TMP7]], !nosanitize [[META2]]
 // TR-NEXT:    [[TMP9:%.*]] = and i1 [[TMP5]], [[TMP8]], !nosanitize [[META2]]
-// TR-NEXT:    br i1 [[TMP9]], label %[[CONT:.*]], label %[[TRAP:.*]], !nosanitize [[META2]]
+// TR-NEXT:    br i1 [[TMP9]], label %[[CONT:.*]], label %[[TRAP:.*]], !prof [[PROF3:![0-9]+]], !nosanitize [[META2]]
 // TR:       [[TRAP]]:
 // TR-NEXT:    tail call void @llvm.ubsantrap(i8 3) #[[ATTR5:[0-9]+]], !nosanitize [[META2]]
 // TR-NEXT:    unreachable, !nosanitize [[META2]]
@@ -107,12 +107,12 @@ int div(int x, int y) {
 // TR-NEXT:    [[TMP0:%.*]] = icmp eq ptr [[X]], null, !nosanitize [[META2]]
 // TR-NEXT:    [[TMP1:%.*]] = tail call i1 @llvm.allow.ubsan.check(i8 29), !nosanitize [[META2]]
 // TR-NEXT:    [[DOTNOT1:%.*]] = and i1 [[TMP0]], [[TMP1]]
-// TR-NEXT:    br i1 [[DOTNOT1]], label %[[TRAP:.*]], label %[[CONT:.*]], !nosanitize [[META2]]
+// TR-NEXT:    br i1 [[DOTNOT1]], label %[[TRAP:.*]], label %[[CONT:.*]], !prof [[PROF4:![0-9]+]], !nosanitize [[META2]]
 // TR:       [[TRAP]]:
 // TR-NEXT:    tail call void @llvm.ubsantrap(i8 22) #[[ATTR5]], !nosanitize [[META2]]
 // TR-NEXT:    unreachable, !nosanitize [[META2]]
 // TR:       [[CONT]]:
-// TR-NEXT:    [[TMP2:%.*]] = load i32, ptr [[X]], align 4, !tbaa [[TBAA3:![0-9]+]]
+// TR-NEXT:    [[TMP2:%.*]] = load i32, ptr [[X]], align 4, !tbaa [[TBAA5:![0-9]+]]
 // TR-NEXT:    ret i32 [[TMP2]]
 //
 // REC-LABEL: define dso_local i32 @null(
@@ -159,7 +159,7 @@ int null(int* x) {
 // TR-NEXT:    [[TMP1:%.*]] = extractvalue { i32, i1 } [[TMP0]], 1, !nosanitize [[META2]]
 // TR-NEXT:    [[TMP2:%.*]] = tail call i1 @llvm.allow.ubsan.check(i8 41), !nosanitize [[META2]]
 // TR-NEXT:    [[DOTDEMORGAN:%.*]] = and i1 [[TMP1]], [[TMP2]]
-// TR-NEXT:    br i1 [[DOTDEMORGAN]], label %[[TRAP:.*]], label %[[CONT:.*]], !nosanitize [[META2]]
+// TR-NEXT:    br i1 [[DOTDEMORGAN]], label %[[TRAP:.*]], label %[[CONT:.*]], !prof [[PROF4]], !nosanitize [[META2]]
 // TR:       [[TRAP]]:
 // TR-NEXT:    tail call void @llvm.ubsantrap(i8 0) #[[ATTR5]], !nosanitize [[META2]]
 // TR-NEXT:    unreachable, !nosanitize [[META2]]
@@ -224,7 +224,7 @@ void use(double*);
 // TR-NEXT:    br i1 [[TMP3]], label %[[TRAP:.*]], label %[[BB4:.*]]
 // TR:       [[BB4]]:
 // TR-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds double, ptr [[VLA]], i64 [[IDXPROM]]
-// TR-NEXT:    [[TMP5:%.*]] = load double, ptr [[ARRAYIDX]], align 8, !tbaa [[TBAA7:![0-9]+]]
+// TR-NEXT:    [[TMP5:%.*]] = load double, ptr [[ARRAYIDX]], align 8, !tbaa [[TBAA9:![0-9]+]]
 // TR-NEXT:    ret double [[TMP5]]
 // TR:       [[TRAP]]:
 // TR-NEXT:    call void @llvm.ubsantrap(i8 71) #[[ATTR5]], !nosanitize [[META2]]
@@ -267,12 +267,14 @@ double lbounds(int b, int i) {
 // CHECK: [[META10]] = !{!"double", [[META7]], i64 0}
 //.
 // TR: [[META2]] = !{}
-// TR: [[TBAA3]] = !{[[META4:![0-9]+]], [[META4]], i64 0}
-// TR: [[META4]] = !{!"int", [[META5:![0-9]+]], i64 0}
-// TR: [[META5]] = !{!"omnipotent char", [[META6:![0-9]+]], i64 0}
-// TR: [[META6]] = !{!"Simple C/C++ TBAA"}
-// TR: [[TBAA7]] = !{[[META8:![0-9]+]], [[META8]], i64 0}
-// TR: [[META8]] = !{!"double", [[META5]], i64 0}
+// TR: [[PROF3]] = !{!"branch_weights", i32 1048575, i32 1}
+// TR: [[PROF4]] = !{!"branch_weights", i32 1, i32 1048575}
+// TR: [[TBAA5]] = !{[[META6:![0-9]+]], [[META6]], i64 0}
+// TR: [[META6]] = !{!"int", [[META7:![0-9]+]], i64 0}
+// TR: [[META7]] = !{!"omnipotent char", [[META8:![0-9]+]], i64 0}
+// TR: [[META8]] = !{!"Simple C/C++ TBAA"}
+// TR: [[TBAA9]] = !{[[META10:![0-9]+]], [[META10]], i64 0}
+// TR: [[META10]] = !{!"double", [[META7]], i64 0}
 //.
 // REC: [[META2]] = !{}
 // REC: [[PROF3]] = !{!"branch_weights", i32 1048575, i32 1}
diff --git a/clang/test/CodeGen/bounds-checking-debuginfo.c b/clang/test/CodeGen/bounds-checking-debuginfo.c
index 61c7af6e7c5b8..4f5ba2b76eeeb 100644
--- a/clang/test/CodeGen/bounds-checking-debuginfo.c
+++ b/clang/test/CodeGen/bounds-checking-debuginfo.c
@@ -23,7 +23,7 @@ void d(double*);
 // CHECK-TRAP-NEXT:    [[CALL:%.*]] = call i32 (...) @f(), !dbg [[DBG22:![0-9]+]]
 // CHECK-TRAP-NEXT:    [[TMP0:%.*]] = sext i32 [[CALL]] to i64, !dbg [[DBG23:![0-9]+]], !nosanitize [[META10:![0-9]+]]
 // CHECK-TRAP-NEXT:    [[TMP1:%.*]] = icmp ult i64 [[TMP0]], 10, !dbg [[DBG23]], !nosanitize [[META10]]
-// CHECK-TRAP-NEXT:    br i1 [[TMP1]], label %[[CONT:.*]], label %[[TRAP:.*]], !dbg [[DBG23]], !nosanitize [[META10]]
+// CHECK-TRAP-NEXT:    br i1 [[TMP1]], label %[[CONT:.*]], label %[[TRAP:.*]], !dbg [[DBG23]], !prof [[PROF27:![0-9]+]], !nosanitize [[META10]]
 // CHECK-TRAP:       [[TRAP]]:
 // CHECK-TRAP-NEXT:    call void @llvm.ubsantrap(i8 18) #[[ATTR3:[0-9]+]], !dbg [[DBG23]], !nosanitize [[META10]]
 // CHECK-TRAP-NEXT:    unreachable, !dbg [[DBG23]], !nosanitize [[META10]]
@@ -31,7 +31,7 @@ void d(double*);
 // CHECK-TRAP-NEXT:    [[IDXPROM:%.*]] = sext i32 [[CALL]] to i64, !dbg [[DBG26:![0-9]+]]
 // CHECK-TRAP-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds [10 x double], ptr [[A]], i64 0, i64 [[IDXPROM]], !dbg [[DBG26]]
 // CHECK-TRAP-NEXT:    [[TMP2:%.*]] = load double, ptr [[ARRAYIDX]], align 8, !dbg [[DBG26]]
-// CHECK-TRAP-NEXT:    ret double [[TMP2]], !dbg [[DBG27:![0-9]+]]
+// CHECK-TRAP-NEXT:    ret double [[TMP2]], !dbg [[DBG28:![0-9]+]]
 //
 // CHECK-NOTRAP-LABEL: define dso_local double @f1(
 // CHECK-NOTRAP-SAME: i32 noundef [[B:%.*]], i32 noundef [[I:%.*]]) #[[ATTR0:[0-9]+]] !dbg [[DBG4:![0-9]+]] {
@@ -92,7 +92,8 @@ double f1(int b, int i) {
 // CHECK-TRAP: [[META24]] = distinct !DISubprogram(name: "__ubsan_check_array_bounds", scope: [[META5]], file: [[META5]], type: [[META25:![0-9]+]], flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: [[META0]])
 // CHECK-TRAP: [[META25]] = !DISubroutineType(types: null)
 // CHECK-TRAP: [[DBG26]] = !DILocation(line: 66, column: 10, scope: [[DBG4]])
-// CHECK-TRAP: [[DBG27]] = !DILocation(line: 66, column: 3, scope: [[DBG4]])
+// CHECK-TRAP: [[PROF27]] = !{!"branch_weights", i32 1048575, i32 1}
+// CHECK-TRAP: [[DBG28]] = !DILocation(line: 66, column: 3, scope: [[DBG4]])
 //.
 // CHECK-NOTRAP: [[META0:![0-9]+]] = distinct !DICompileUnit(language: DW_LANG_C11, file: [[META1:![0-9]+]], isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
 // CHECK-NOTRAP: [[META1]] = !DIFile(filename: "<stdin>", directory: {{.*}})
diff --git a/clang/test/CodeGen/cfi-check-fail.c b/clang/test/CodeGen/cfi-check-fail.c
index 15f6c77abf2b2..d4262b20adc3f 100644
--- a/clang/test/CodeGen/cfi-check-fail.c
+++ b/clang/test/CodeGen/cfi-check-fail.c
@@ -13,7 +13,7 @@ void caller(void (*f)(void)) {
 // CHECK: %[[DATA:.*]] = load ptr, ptr %[[ALLOCA0]], align 8
 // CHECK: %[[ADDR:.*]] = load ptr, ptr %[[ALLOCA1]], align 8
 // CHECK: %[[ICMP_NOT_NULL:.*]] = icmp ne ptr %[[DATA]], null
-// CHECK: br i1 %[[ICMP_NOT_NULL]], label %[[CONT0:.*]], label %[[TRAP:.*]],
+// CHECK: br i1 %[[ICMP_NOT_NULL]], label %[[CONT0:.*]], label %[[TRAP:.*]], !prof
 
 // CHECK: [[TRAP]]:
 // CHECK-NEXT:   call void @llvm.ubsantrap(i8 2)
@@ -35,7 +35,7 @@ void caller(void (*f)(void)) {
 
 // CHECK: [[CONT1]]:
 // CHECK:   %[[NOT_1:.*]] = icmp ne i8 %[[KIND]], 1
-// CHECK:   br i1 %[[NOT_1]], label %[[CONT2:.*]], label %[[HANDLE1:.*]], !nosanitize
+// CHECK:   br i1 %[[NOT_1]], label %[[CONT2:.*]], label %[[HANDLE1:.*]], !prof !{{[0-9]+}}, !nosanitize
 
 // CHECK: [[HANDLE1]]:
 // CHECK-NEXT:   call void @llvm.ubsantrap(i8 2)
@@ -63,7 +63,7 @@ void caller(void (*f)(void)) {
 
 // CHECK: [[CONT4]]:
 // CHECK:   %[[NOT_4:.*]] = icmp ne i8 %[[KIND]], 4
-// CHECK:   br i1 %[[NOT_4]], label %[[CONT5:.*]], label %[[HANDLE4:.*]], !nosanitize
+// CHECK:   br i1 %[[NOT_4]], label %[[CONT5:.*]], label %[[HANDLE4:.*]], !prof !{{[0-9]+}}, !nosanitize
 
 // CHECK: [[HANDLE4]]:
 // CHECK-NEXT:   call void @llvm.ubsantrap(i8 2)
diff --git a/clang/test/CodeGen/cfi-check-fail2.c b/clang/test/CodeGen/cfi-check-fail2.c
index d904ee41f607b..ae0ba78085894 100644
--- a/clang/test/CodeGen/cfi-check-fail2.c
+++ b/clang/test/CodeGen/cfi-check-fail2.c
@@ -19,7 +19,7 @@ void caller(void (*f)(void)) {
 // CHECK: %[[DATA:.*]] = load ptr, ptr %[[ALLOCA0]], align 8
 // CHECK: %[[ADDR:.*]] = load ptr, ptr %[[ALLOCA1]], align 8
 // CHECK: %[[ICMP_NOT_NULL:.*]] = icmp ne ptr %[[DATA]], null
-// CHECK: br i1 %[[ICMP_NOT_NULL]], label %[[CONT0:.*]], label %[[TRAP:.*]],
+// CHECK: br i1 %[[ICMP_NOT_NULL]], label %[[CONT0:.*]], label %[[TRAP:.*]], !prof
 
 // CHECK: [[TRAP]]:
 // CHECK-NEXT:   call void @llvm.ubsantrap(i8 2)
@@ -41,7 +41,7 @@ void caller(void (*f)(void)) {
 
 // CHECK: [[CONT1]]:
 // CHECK:   %[[NOT_1:.*]] = icmp ne i8 %[[KIND]], 1
-// CHECK:   br i1 %[[NOT_1]], label %[[CONT2:.*]], label %[[HANDLE1:.*]], !nosanitize
+// CHECK:   br i1 %[[NOT_1]], label %[[CONT2:.*]], label %[[HANDLE1:.*]], !prof
 
 // CHECK: [[HANDLE1]]:
 // CHECK-NEXT:   call void @llvm.ubsantrap(i8 2)
@@ -49,7 +49,7 @@ void caller(void (*f)(void)) {
 
 // CHECK: [[CONT2]]:
 // CHECK:   %[[NOT_2:.*]] = icmp ne i8 %[[KIND]], 2
-// CHECK:   br i1 %[[NOT_2]], label %[[CONT3:.*]], label %[[HANDLE2:.*]], !nosanitize
+// CHECK:   br i1 %[[NOT_2]], label %[[CONT3:.*]], label %[[HANDLE2:.*]], !prof
 
 // CHECK: [[HANDLE2]]:
 // CHECK-NEXT:   call void @llvm.ubsantrap(i8 2)
@@ -57,7 +57,7 @@ void caller(void (*f)(void)) {
 
 // CHECK: [[CONT3]]:
 // CHECK:   %[[NOT_3:.*]] = icmp ne i8 %[[KIND]], 3
-// CHECK:   br i1 %[[NOT_3]], label %[[CONT4:.*]], label %[[HANDLE3:.*]], !nosanitize
+// CHECK:   br i1 %[[NOT_3]], label %[[CONT4:.*]], label %[[HANDLE3:.*]], !prof
 
 // CHECK: [[HANDLE3]]:
 // CHECK-NEXT:   call void @llvm.ubsantrap(i8 2)
@@ -65,7 +65,7 @@ void caller(void (*f)(void)) {
 
 // CHECK: [[CONT4]]:
 // CHECK:   %[[NOT_4:.*]] = icmp ne i8 %[[KIND]], 4
-// CHECK:   br i1 %[[NOT_4]], label %[[CONT5:.*]], label %[[HANDLE4:.*]], !nosanitize
+// CHECK:   br i1 %[[NOT_4]], label %[[CONT5:.*]], label %[[HANDLE4:.*]], !prof
 
 // CHECK: [[HANDLE4]]:
 // CHECK-NEXT:   call void @llvm.ubsantrap(i8 2)
diff --git a/clang/test/CodeGen/ubsan-trap-merge.c b/clang/test/CodeGen/ubsan-trap-merge.c
index 486aa55f5b811..b06420950d941 100644
--- a/clang/test/CodeGen/ubsan-trap-merge.c
+++ b/clang/test/CodeGen/ubsan-trap-merge.c
@@ -64,7 +64,7 @@
 // TRAP-NOMERGE-NEXT:  [[ENTRY:.*:]]
 // TRAP-NOMERGE-NEXT:    [[TMP0:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[X]], i32 125), !nosanitize [[META2:![0-9]+]]
 // TRAP-NOMERGE-NEXT:    [[TMP1:%.*]] = extractvalue { i32, i1 } [[TMP0]], 1, !nosanitize [[META2]]
-// TRAP-NOMERGE-NEXT:    br i1 [[TMP1]], label %[[TRAP:.*]], label %[[CONT:.*]], !nosanitize [[META2]]
+// TRAP-NOMERGE-NEXT:    br i1 [[TMP1]], label %[[TRAP:.*]], label %[[CONT:.*]], !prof [[PROF3:![0-9]+]], !nosanitize [[META2]]
 // TRAP-NOMERGE:       [[TRAP]]:
 // TRAP-NOMERGE-NEXT:    tail call void @llvm.ubsantrap(i8 0) #[[ATTR4:[0-9]+]], !nosanitize [[META2]]
 // TRAP-NOMERGE-NEXT:    unreachable, !nosanitize [[META2]]
@@ -104,7 +104,7 @@
 // TRAP-MERGE-NEXT:  [[ENTRY:.*:]]
 // TRAP-MERGE-NEXT:    [[TMP0:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[X]], i32 125), !nosanitize [[META2:![0-9]+]]
 // TRAP-MERGE-NEXT:    [[TMP1:%.*]] = extractvalue { i32, i1 } [[TMP0]], 1, !nosanitize [[META2]]
-// TRAP-MERGE-NEXT:    br i1 [[TMP1]], label %[[TRAP:.*]], label %[[CONT:.*]], !nosanitize [[META2]]
+// TRAP-MERGE-NEXT:    br i1 [[TMP1]], label %[[TRAP:.*]], label %[[CONT:.*]], !prof [[PROF3:![0-9]+]], !nosanitize [[META2]]
 // TRAP-MERGE:       [[TRAP]]:
 // TRAP-MERGE-NEXT:    tail call void @llvm.ubsantrap(i8 0) #[[ATTR4:[0-9]+]], !nosanitize [[META2]]
 // TRAP-MERGE-NEXT:    unreachable, !nosanitize [[META2]]
@@ -187,7 +187,7 @@ int f(int x) {
 // TRAP-NOMERGE-NEXT:  [[ENTRY:.*:]]
 // TRAP-NOMERGE-NEXT:    [[TMP0:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[X]], i32 127), !nosanitize [[META2]]
 // TRAP-NOMERGE-NEXT:    [[TMP1:%.*]] = extractvalue { i32, i1 } [[TMP0]], 1, !nosanitize [[META2]]
-// TRAP-NOMERGE-NEXT:    br i1 [[TMP1]], label %[[TRAP:.*]], label %[[CONT:.*]], !nosanitize [[META2]]
+// TRAP-NOMERGE-NEXT:    br i1 [[TMP1]], label %[[TRAP:.*]], label %[[CONT:.*]], !prof [[PROF3]], !nosanitize [[META2]]
 // TRAP-NOMERGE:       [[TRAP]]:
 // TRAP-NOMERGE-NEXT:    tail call void @llvm.ubsantrap(i8 0) #[[ATTR4]], !nosanitize [[META2]]
 // TRAP-NOMERGE-NEXT:    unreachable, !nosanitize [[META2]]
@@ -227,7 +227,7 @@ int f(int x) {
 // TRAP-MERGE-NEXT:  [[ENTRY:.*:]]
 // TRAP-MERGE-NEXT:    [[TMP0:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[X]], i32 127), !nosanitize [[META2]]
 // TRAP-MERGE-NEXT:    [[TMP1:%.*]] = extractvalue { i32, i1 } [[TMP0]], 1, !nosanitize [[META2]]
-// TRAP-MERGE-NEXT:    br i1 [[TMP1]], label %[[TRAP:.*]], label %[[CONT:.*]], !nosanitize [[META2]]
+// TRAP-MERGE-NEXT:    br i1 [[TMP1]], label %[[TRAP:.*]], label %[[CONT:.*]], !prof [[PROF3]], !nosanitize [[META2]]
 // TRAP-MERGE:       [[TRAP]]:
 // TRAP-MERGE-NEXT:    tail call void @llvm.ubsantrap(i8 0) #[[ATTR4]], !nosanitize [[META2]]
 // TRAP-MERGE-NEXT:    unreachable, !nosanitize [[META2]]
@@ -317,14 +317,14 @@ int g(int x) {
 // TRAP-NOMERGE-NEXT:  [[ENTRY:.*:]]
 // TRAP-NOMERGE-NEXT:    [[TMP0:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[X]], i32 127), !nosanitize [[META2]]
 // TRAP-NOMERGE-NEXT:    [[TMP1:%.*]] = extractvalue { i32, i1 } [[TMP0]], 1, !nosanitize [[META2]]
-// TRAP-NOMERGE-NEXT:    br i1 [[TMP1]], label %[[TRAP:.*]], label %[[CONT:.*]], !nosanitize [[META2]]
+// TRAP-NOMERGE-NEXT:    br i1 [[TMP1]], label %[[TRAP:.*]], label %[[CONT:.*]], !prof [[PROF3]], !nosanitize [[META2]]
 // TRAP-NOMERGE:       [[TRAP]]:
 // TRAP-NOMERGE-NEXT:    tail call void @llvm.ubsantrap(i8 0) #[[ATTR4]], !nosanitize [[META2]]
 // TRAP-NOMERGE-NEXT:    unreachable, !nosanitize [[META2]]
 // TRAP-NOMERGE:       [[CONT]]:
 // TRAP-NOMERGE-NEXT:    [[TMP2:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[Y]], i32 129), !nosanitize [[META2]]
 // TRAP-NOMERGE-NEXT:    [[TMP3:%.*]] = extractvalue { i32, i1 } [[TMP2]], 1, !nosanitize [[META2]]
-// TRAP-NOMERGE-NEXT:    br i1 [[TMP3]], label %[[TRAP1:.*]], label %[[CONT2:.*]], !nosanitize [[META2]]
+// TRAP-NOMERGE-NEXT:    br i1 [[TMP3]], label %[[TRAP1:.*]], label %[[CONT2:.*]], !prof [[PROF3]], !nosanitize [[META2]]
 // TRAP-NOMERGE:       [[TRAP1]]:
 // TRAP-NOMERGE-NEXT:    tail call void @llvm.ubsantrap(i8 0) #[[ATTR4]], !nosanitize [[META2]]
 // TRAP-NOMERGE-NEXT:    unreachable, !nosanitize [[META2]]
@@ -385,14 +385,14 @@ int g(int x) {
 // TRAP-MERGE-NEXT:  [[ENTRY:.*:]]
 // TRAP-MERGE-NEXT:    [[TMP0:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[X]], i32 127), !nosanitize [[META2]]
 // TRAP-MERGE-NEXT:    [[TMP1:%.*]] = extractvalue { i32, i1 } [[TMP0]], 1, !nosanitize [[META2]]
-// TRAP-MERGE-NEXT:    br i1 [[TMP1]], label %[[TRAP:.*]], label %[[CONT:.*]], !nosanitize [[META2]]
+// TRAP-MERGE-NEXT:    br i1 [[TMP1]], label %[[TRAP:.*]], label %[[CONT:.*]], !prof [[PROF3]], !nosanitize [[META2]]
 // TRAP-MERGE:       [[TRAP]]:
 // TRAP-MERGE-NEXT:    tail call void @llvm.ubsantrap(i8 0) #[[ATTR4]], !nosanitize [[META2]]
 // TRAP-MERGE-NEXT:    unreachable, !nosanitize [[META2]]
 // TRAP-MERGE:       [[CONT]]:
 // TRAP-MERGE-NEXT:    [[TMP2:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[Y]], i32 129), !nosanitize [[META2]]
 // TRAP-MERGE-NEXT:    [[TMP3:%.*]] = extractvalue { i32, i1 } [[TMP2]], 1, !nosanitize [[META2]]
-// TRAP-MERGE-NEXT:    br i1 [[TMP3]], label %[[TRAP]], label %[[CONT1:.*]], !nosanitize [[META2]]
+// TRAP-MERGE-NEXT:    br i1 [[TMP3]], label %[[TRAP]], label %[[CONT1:.*]], !prof [[PROF3]], !nosanitize [[META2]]
 // TRAP-MERGE:       [[CONT1]]:
 // TRAP-MERGE-NEXT:    [[TMP4:%.*]] = extractvalue { i32, i1 } [[TMP2]], 0, !nosanitize [[META2]]
 // TRAP-MERGE-NEXT:    [[TMP5:%.*]] = extractvalue { i32, i1 } [[TMP0]], 0, !nosanitize [[META2]]
@@ -546,14 +546,14 @@ int h(int x, int y) {
 // TRAP-NOMERGE-NEXT:  [[ENTRY:.*:]]
 // TRAP-NOMERGE-NEXT:    [[TMP0:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[X]], i32 125), !nosanitize [[META2]]
 // TRAP-NOMERGE-NEXT:    [[TMP1:%.*]] = extractvalue { i32, i1 } [[TMP0]], 1, !nosanitize [[META2]]
-// TRAP-NOMERGE-NEXT:    br i1 [[TMP1]], label %[[TRAP_I:.*]], label %[[F_EXIT:.*]], !nosanitize [[META2]]
+// TRAP-NOMERGE-NEXT:    br i1 [[TMP1]], label %[[TRAP_I:.*]], label %[[F_EXIT:.*]], !prof [[PROF3]], !nosanitize [[META2]]
 // TRAP-NOMERGE:       [[TRAP_I]]:
 // TRAP-NOMERGE-NEXT:    tail call void @llvm.ubsantrap(i8 0) #[[ATTR4]], !nosanitize [[META2]]
 // TRAP-NOMERGE-NEXT:    unreachable, !nosanitize [[META2]]
 // TRAP-NOMERGE:       [[F_EXIT]]:
 // TRAP-NOMERGE-NEXT:    [[TMP2:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[Y]], i32 127), !nosanitize [[META2]]
 // TRAP-NOMERGE-NEXT:    [[TMP3:%.*]] = extractvalue { i32, i1 } [[TMP2]], 1, !nosanitize [[META2]]
-// TRAP-NOMERGE-NEXT:    br i1 [[TMP3]], label %[[TRAP_I2:.*]], label %[[G_EXIT:.*]], !nosanitize [[META2]]
+// TRAP-NOMERGE-NEXT:    br i1 [[TMP3]], label %[[TRAP_I2:.*]], label %[[G_EXIT:.*]], !prof [[PROF3]], !nosanitize [[META2]]
 // TRAP-NOMERGE:       [[TRAP_I2]]:
 // TRAP-NOMERGE-NEXT:    tail call void @llvm.ubsantrap(i8 0) #[[ATTR4]], !nosanitize [[META2]]
 // TRAP-NOMERGE-NEXT:    unreachable, !nosanitize [[META2]]
@@ -562,7 +562,7 @@ int h(int x, int y) {
 // TRAP-NOMERGE-NEXT:    [[TMP5:%.*]] = extractvalue { i32, i1 } [[TMP2]], 0, !nosanitize [[META2]]
 // TRAP-NOMERGE-NEXT:    [[TMP6:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[TMP4]], i32 [[TMP5]]), !nosanitize [[META2]]
 // TRAP-NOMERGE-NEXT:    [[TMP7:%.*]] = extractvalue { i32, i1 } [[TMP6]], 1, !nosanitize [[META2]]
-// TRAP-NOMERGE-NEXT:    br i1 [[TMP7]], label %[[TRAP:.*]], label %[[CONT:.*]], !nosanitize [[META2]]
+// TRAP-NOMERGE-NEXT:    br i1 [[TMP7]], label %[[TRAP:.*]], label %[[CONT:.*]], !prof [[PROF3]], !nosanitize [[META2]]
 // TRAP-NOMERGE:       [[TRAP]]:
 // TRAP-NOMERGE-NEXT:    tail call void @llvm.ubsantrap(i8 0) #[[ATTR4]], !nosanitize [[META2]]
 // TRAP-NOMERGE-NEXT:    unreachable, !nosanitize [[META2]]
@@ -637,14 +637,14 @@ int h(int x, int y) {
 // TRAP-MERGE-NEXT:  [[ENTRY:.*:]]
 // TRAP-MERGE-NEXT:    [[TMP0:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[X]], i32 125), !nosanitize [[META2]]
 // TRAP-MERGE-NEXT:    [[TMP1:%.*]] = extractvalue { i32, i1 } [[TMP0]], 1, !nosanitize [[META2]]
-// TRAP-MERGE-NEXT:    br i1 [[TMP1]], label %[[TRAP_I:.*]], label %[[F_EXIT:.*]], !nosanitize [[META2]]
+// TRAP-MERGE-NEXT:    br i1 [[TMP1]], label %[[TRAP_I:.*]], label %[[F_EXIT:.*]], !prof [[PROF3]], !nosanitize [[META2]]
 // TRAP-MERGE:       [[TRAP_I]]:
 // TRAP-MERGE-NEXT:    tail call void @llvm.ubsantrap(i8 0) #[[ATTR4]], !nosanitize [[META2]]
 // TRAP-MERGE-NEXT:    unreachable, !nosanitize [[META2]]
 // TRAP-MERGE:       [[F_E...
[truncated]

@thurstond
Copy link
Contributor

thurstond commented Apr 4, 2025

Trap checks fail at most once (when the program crashes).

It's technically possible to have a signal handler that consumes SIGILL and then jumps over the offending instruction. The UBSan docs allude to this: "-fsanitize-trap=...: execute a trap instruction (doesn’t require UBSan run-time support). If the signal is not caught, the program will typically terminate due to a SIGILL or SIGTRAP signal." (https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html)

That said, I agree that it is extremely likely a trap happens at most once.

@fmayer fmayer merged commit 30f2e92 into main Apr 4, 2025
15 checks passed
@fmayer fmayer deleted the users/fmayer/spr/clang-sanitizer-predict-trap-checks-succeed branch April 4, 2025 17:58
@vitalybuka
Copy link
Collaborator

Last time I checks, if the branch endup with unreachable, compiler can figure it out as UNLIKELY. So PR like this is effectively NOP.


// CHECK: [[CONT3]]:
// CHECK: %[[NOT_3:.*]] = icmp ne i8 %[[KIND]], 3
// CHECK: br i1 %[[NOT_3]], label %[[CONT4:.*]], label %[[HANDLE3:.*]], !nosanitize
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why those nosanitize are gone?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they aren't gone. AFAICT they were just there in the pattern to not match too much in the .*

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's auto generated pattern?

delcypher added a commit to swiftlang/llvm-project that referenced this pull request Apr 11, 2025
…trap branches

```
commit 30f2e92
Author: Florian Mayer <[email protected]>
Date:   Fri Apr 4 10:58:08 2025 -0700

    [clang] [sanitizer] predict trap checks succeed (llvm#134310)

    Trap checks fail at most once (when the program crashes).
```

added profile metadata to trap branches that says its likely the trap
succeeds. This shows up in the IR which broke many of our codegen tests.

Many of the tests were automatically updated using
`clang/utils/bounds_safety_fixup_codegen_tests.sh` but several had to be
updated by hand.

rdar://148767987
delcypher added a commit to swiftlang/llvm-project that referenced this pull request Apr 11, 2025
…trap branches

```
commit 30f2e92
Author: Florian Mayer <[email protected]>
Date:   Fri Apr 4 10:58:08 2025 -0700

    [clang] [sanitizer] predict trap checks succeed (llvm#134310)

    Trap checks fail at most once (when the program crashes).
```

added profile metadata to trap branches that says its likely the trap
succeeds. This shows up in the IR which broke many of our codegen tests.

Many of the tests were automatically updated using
`clang/utils/bounds_safety_fixup_codegen_tests.sh` but several had to be
updated by hand.

rdar://148767987
@fmayer
Copy link
Contributor Author

fmayer commented Apr 23, 2025

Last time I checks, if the branch endup with unreachable, compiler can figure it out as UNLIKELY. So PR like this is effectively NOP.

I built tcmalloc with and without this change and get different codegen

delcypher added a commit to swiftlang/llvm-project that referenced this pull request Oct 1, 2025
The implementation was a bit hacky and interacted badly with the
hot-cold splitting pass. So it is going to be removed in favor of a
different implementation that's simpler.

I also investated why
`clang/test/BoundsSafety/CodeGen/unique-trap-blocks-O2.c` broke (rdar://150559639).
It turns outthat the "trap blocks" started being unintentionally split
by the hot-cold splitting pass because in upstream we started adding
branch profiling weights
(llvm#134310) which the hot-cold
splitting pass used as a signal to split the "trap blocks" into their
own function (by-passing the logic I originally added to prevent the
splitting). While we could fix this it would be very fragile and given
that we are removing the implementation anyway it doesn't matter
anymore.

rdar://158088410
delcypher added a commit to delcypher/apple-llvm-project that referenced this pull request Oct 1, 2025
The implementation was a bit hacky and interacted badly with the
hot-cold splitting pass. So it is going to be removed in favor of a
different implementation that's simpler.

I also investated why
`clang/test/BoundsSafety/CodeGen/unique-trap-blocks-O2.c` broke (rdar://150559639).
It turns outthat the "trap blocks" started being unintentionally split
by the hot-cold splitting pass because in upstream we started adding
branch profiling weights
(llvm#134310) which the hot-cold
splitting pass used as a signal to split the "trap blocks" into their
own function (by-passing the logic I originally added to prevent the
splitting). While we could fix this it would be very fragile and given
that we are removing the implementation anyway it doesn't matter
anymore.

rdar://158088410
(cherry picked from commit 8c4e017)

Conflicts:
	clang/test/BoundsSafety-legacy-checks/CodeGen/unique-trap-blocks-O0-O2-opt-disabled.c
  clang/test/BoundsSafety/CodeGen/unique-trap-blocks-O0-O2-opt-disabled.c
delcypher added a commit to swiftlang/llvm-project that referenced this pull request Oct 1, 2025
The implementation was a bit hacky and interacted badly with the
hot-cold splitting pass. So it is going to be removed in favor of a
different implementation that's simpler.

I also investated why
`clang/test/BoundsSafety/CodeGen/unique-trap-blocks-O2.c` broke (rdar://150559639).
It turns outthat the "trap blocks" started being unintentionally split
by the hot-cold splitting pass because in upstream we started adding
branch profiling weights
(llvm#134310) which the hot-cold
splitting pass used as a signal to split the "trap blocks" into their
own function (by-passing the logic I originally added to prevent the
splitting). While we could fix this it would be very fragile and given
that we are removing the implementation anyway it doesn't matter
anymore.

rdar://158088410
(cherry picked from commit 8c4e017)

Conflicts:
	clang/test/BoundsSafety-legacy-checks/CodeGen/unique-trap-blocks-O0-O2-opt-disabled.c
  clang/test/BoundsSafety/CodeGen/unique-trap-blocks-O0-O2-opt-disabled.c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants