Skip to content

Commit d10813b

Browse files
delcypherdelcypher
committed
[BoundsSafety] Implement -fbounds-safety-unique-traps
This patch adds a driver/cc1 flag that prevents `-fbounds-safety` traps from being merged. This improves debuggability (because now it is possible to see which check lead to the trap) at the cost of increased code size. It is currently off by default. This implementation adds the `nomerge` attribute on call instructions to `-fbounds-safety` traps at the LLVM IR level and prevents re-using trap blocks during Clang's IRGen when the compiler flag is passed. This was basically already implemented upstream for trapping UBSan which meant the only thing we needed to do was set the `NoMerge` parameter to `true` when calling `EmitTrapCheck`. This is the replacement for the recently removed `-funique-traps` which was less than ideal for several reasons: * The use of `asm` instructions in the trap blocks made analyzing LLVM IR more challenging than it needed to be. * The trap blocks interacted poorly with the hot-cold splitting pass and required fragile workarounds to prevent the trap blocks from being split into their own function. The implementation of `-fbound-safety-unique-traps` has none of these problems. rdar://158088410 (cherry picked from commit 9b1d55c)
1 parent 6369ff1 commit d10813b

File tree

7 files changed

+412
-3
lines changed

7 files changed

+412
-3
lines changed

‎clang/include/clang/Basic/CodeGenOptions.def‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,11 @@ CODEGENOPT(ProfileSampleAccurate, 1, 0, Benign) ///< Sample profile is accurate.
332332
/* TO_UPSTREAM(BoundsSafety) ON*/
333333
CODEGENOPT(TrapFuncReturns , 1, 0, Benign) ///< When true, the function specified with
334334
///< -ftrap-function may return normally
335+
336+
CODEGENOPT(BoundsSafetyUniqueTraps, 1, 0, Benign) ///< When true, merging of
337+
/// -fbounds-safety trap
338+
/// instructions will be
339+
/// prevented.
335340
/* TO_UPSTREAM(BoundsSafety) OFF*/
336341

337342
/// Treat loops as finite: language, always, never.

‎clang/include/clang/Driver/Options.td‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2023,6 +2023,18 @@ def fno_bounds_safety_bringup_missing_checks : Flag<["-"], "fno-bounds-safety-br
20232023
Alias<fno_bounds_safety_bringup_missing_checks_EQ>, AliasArgs<["all"]>,
20242024
HelpText<"Disable new -fbounds-safety run-time checks">;
20252025

2026+
defm bounds_safety_unique_traps : BoolFOption<"bounds-safety-unique-traps",
2027+
CodeGenOpts<"BoundsSafetyUniqueTraps">, DefaultFalse,
2028+
PosFlag<SetTrue, [], [CC1Option],
2029+
"Make trap instructions unique by preventing traps from being merged by the"
2030+
" optimizer. This is useful for preserving the debug information on traps "
2031+
"in optimized code. This makes it easier to debug programs at the cost of "
2032+
"increased code size">,
2033+
NegFlag<SetFalse, [], [CC1Option],
2034+
"Don't try to make trap instructions unique. This allows trap instructions "
2035+
"to be merged by the optimizer.">>;
2036+
// TO_UPSTREAM(BoundsSafety) OFF
2037+
20262038
defm lifetime_safety : BoolFOption<
20272039
"experimental-lifetime-safety",
20282040
LangOpts<"EnableLifetimeSafety">, DefaultFalse,

‎clang/lib/CodeGen/CGExpr.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4710,9 +4710,9 @@ void CodeGenFunction::EmitBoundsSafetyTrapCheck(llvm::Value *Checked,
47104710
// We still need to pass `OptRemark` because not all emitted instructions
47114711
// can be covered by BoundsSafetyOptRemarkScope. This is because EmitTrapCheck
47124712
// caches basic blocks that contain instructions that need annotating.
4713-
EmitTrapCheck(Checked, SanitizerHandler::BoundsSafety, /*NoMerge=*/false,
4714-
/*TR=*/nullptr,
4715-
GetBoundsSafetyOptRemarkString(OptRemark),
4713+
EmitTrapCheck(Checked, SanitizerHandler::BoundsSafety,
4714+
/*NoMerge=*/CGM.getCodeGenOpts().BoundsSafetyUniqueTraps,
4715+
/*TR=*/nullptr, GetBoundsSafetyOptRemarkString(OptRemark),
47164716
GetBoundsSafetyTrapMessageSuffix(kind, TrapCtx));
47174717
}
47184718

‎clang/lib/Driver/ToolChains/Clang.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7096,6 +7096,9 @@ void Clang::ConstructJob(Compilation &C, const JobAction &Job,
70967096
D.Diag(diag::err_drv_option_requires_option)
70977097
<< "-ftrap-function-returns" << "-ftrap-function=";
70987098
}
7099+
7100+
Args.addOptInFlag(CmdArgs, options::OPT_fbounds_safety_unique_traps,
7101+
options::OPT_fno_bounds_safety_unique_traps);
70997102
/* TO_UPSTREAM(BoundsSafety) OFF*/
71007103

71017104
// Handle -f[no-]wrapv and -f[no-]strict-overflow, which are used by both

‎clang/test/BoundsSafety/CodeGen/unique-traps-O0-O2-opt-disabled.c‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --check-globals all --version 5
2+
// RUN: %clang_cc1 -O0 -triple arm64e-apple-ios -fbounds-safety \
3+
// RUN: -fbounds-safety-unique-traps -emit-llvm %s -o - \
4+
// RUN: | FileCheck -check-prefix=OPT0 %s
5+
// RUN: %clang_cc1 -O2 -triple arm64e-apple-ios -fbounds-safety \
6+
// RUN: -fbounds-safety-unique-traps -emit-llvm %s -disable-llvm-passes -o - \
7+
// RUN: | FileCheck -check-prefix=OPT2 %s
8+
9+
#include <ptrcheck.h>
10+
11+
// OPT0-LABEL: define i32 @consume(
12+
// OPT0-SAME: ptr noundef [[PTR:%.*]], i32 noundef [[IDX:%.*]]) #[[ATTR0:[0-9]+]] {
13+
// OPT0-NEXT: [[ENTRY:.*:]]
14+
// OPT0-NEXT: [[PTR_INDIRECT_ADDR:%.*]] = alloca ptr, align 8
15+
// OPT0-NEXT: [[IDX_ADDR:%.*]] = alloca i32, align 4
16+
// OPT0-NEXT: [[AGG_TEMP:%.*]] = alloca %"__bounds_safety::wide_ptr.bidi_indexable", align 8
17+
// OPT0-NEXT: store ptr [[PTR]], ptr [[PTR_INDIRECT_ADDR]], align 8
18+
// OPT0-NEXT: store i32 [[IDX]], ptr [[IDX_ADDR]], align 4
19+
// OPT0-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 8 [[AGG_TEMP]], ptr align 8 [[PTR]], i64 24, i1 false)
20+
// OPT0-NEXT: [[WIDE_PTR_PTR_ADDR:%.*]] = getelementptr inbounds nuw %"__bounds_safety::wide_ptr.bidi_indexable", ptr [[AGG_TEMP]], i32 0, i32 0
21+
// OPT0-NEXT: [[WIDE_PTR_PTR:%.*]] = load ptr, ptr [[WIDE_PTR_PTR_ADDR]], align 8
22+
// OPT0-NEXT: [[TMP0:%.*]] = load i32, ptr [[IDX_ADDR]], align 4
23+
// OPT0-NEXT: [[IDXPROM:%.*]] = sext i32 [[TMP0]] to i64
24+
// OPT0-NEXT: [[ARRAYIDX:%.*]] = getelementptr i32, ptr [[WIDE_PTR_PTR]], i64 [[IDXPROM]]
25+
// OPT0-NEXT: [[WIDE_PTR_UB_ADDR:%.*]] = getelementptr inbounds nuw %"__bounds_safety::wide_ptr.bidi_indexable", ptr [[AGG_TEMP]], i32 0, i32 1
26+
// OPT0-NEXT: [[WIDE_PTR_UB:%.*]] = load ptr, ptr [[WIDE_PTR_UB_ADDR]], align 8
27+
// OPT0-NEXT: [[WIDE_PTR_LB_ADDR:%.*]] = getelementptr inbounds nuw %"__bounds_safety::wide_ptr.bidi_indexable", ptr [[AGG_TEMP]], i32 0, i32 2
28+
// OPT0-NEXT: [[WIDE_PTR_LB:%.*]] = load ptr, ptr [[WIDE_PTR_LB_ADDR]], align 8
29+
// OPT0-NEXT: [[TMP1:%.*]] = getelementptr i32, ptr [[ARRAYIDX]], i64 1, !annotation [[META2:![0-9]+]]
30+
// OPT0-NEXT: [[TMP2:%.*]] = icmp ule ptr [[TMP1]], [[WIDE_PTR_UB]], !annotation [[META2]]
31+
// OPT0-NEXT: br i1 [[TMP2]], label %[[CONT:.*]], label %[[TRAP:.*]], !prof [[PROF3:![0-9]+]], !annotation [[META2]]
32+
// OPT0: [[TRAP]]:
33+
// OPT0-NEXT: call void @llvm.ubsantrap(i8 25) #[[ATTR3:[0-9]+]], !annotation [[META2]]
34+
// OPT0-NEXT: unreachable, !annotation [[META2]]
35+
// OPT0: [[CONT]]:
36+
// OPT0-NEXT: [[TMP3:%.*]] = icmp ule ptr [[ARRAYIDX]], [[TMP1]], !annotation [[META2]]
37+
// OPT0-NEXT: br i1 [[TMP3]], label %[[CONT2:.*]], label %[[TRAP1:.*]], !prof [[PROF3]], !annotation [[META2]]
38+
// OPT0: [[TRAP1]]:
39+
// OPT0-NEXT: call void @llvm.ubsantrap(i8 25) #[[ATTR3]], !annotation [[META2]]
40+
// OPT0-NEXT: unreachable, !annotation [[META2]]
41+
// OPT0: [[CONT2]]:
42+
// OPT0-NEXT: [[TMP4:%.*]] = icmp uge ptr [[ARRAYIDX]], [[WIDE_PTR_LB]], !annotation [[META4:![0-9]+]]
43+
// OPT0-NEXT: br i1 [[TMP4]], label %[[CONT4:.*]], label %[[TRAP3:.*]], !prof [[PROF3]], !annotation [[META4]]
44+
// OPT0: [[TRAP3]]:
45+
// OPT0-NEXT: call void @llvm.ubsantrap(i8 25) #[[ATTR3]], !annotation [[META4]]
46+
// OPT0-NEXT: unreachable, !annotation [[META4]]
47+
// OPT0: [[CONT4]]:
48+
// OPT0-NEXT: [[TMP5:%.*]] = load i32, ptr [[ARRAYIDX]], align 4
49+
// OPT0-NEXT: ret i32 [[TMP5]]
50+
//
51+
// OPT2-LABEL: define i32 @consume(
52+
// OPT2-SAME: ptr noundef [[PTR:%.*]], i32 noundef [[IDX:%.*]]) #[[ATTR0:[0-9]+]] {
53+
// OPT2-NEXT: [[ENTRY:.*:]]
54+
// OPT2-NEXT: [[PTR_INDIRECT_ADDR:%.*]] = alloca ptr, align 8
55+
// OPT2-NEXT: [[IDX_ADDR:%.*]] = alloca i32, align 4
56+
// OPT2-NEXT: [[AGG_TEMP:%.*]] = alloca %"__bounds_safety::wide_ptr.bidi_indexable", align 8
57+
// OPT2-NEXT: store ptr [[PTR]], ptr [[PTR_INDIRECT_ADDR]], align 8, !tbaa [[TBAA2:![0-9]+]]
58+
// OPT2-NEXT: store i32 [[IDX]], ptr [[IDX_ADDR]], align 4, !tbaa [[TBAA8:![0-9]+]]
59+
// OPT2-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 8 [[AGG_TEMP]], ptr align 8 [[PTR]], i64 24, i1 false), !tbaa.struct [[TBAA_STRUCT10:![0-9]+]]
60+
// OPT2-NEXT: [[WIDE_PTR_PTR_ADDR:%.*]] = getelementptr inbounds nuw %"__bounds_safety::wide_ptr.bidi_indexable", ptr [[AGG_TEMP]], i32 0, i32 0
61+
// OPT2-NEXT: [[WIDE_PTR_PTR:%.*]] = load ptr, ptr [[WIDE_PTR_PTR_ADDR]], align 8
62+
// OPT2-NEXT: [[TMP0:%.*]] = load i32, ptr [[IDX_ADDR]], align 4, !tbaa [[TBAA8]]
63+
// OPT2-NEXT: [[IDXPROM:%.*]] = sext i32 [[TMP0]] to i64
64+
// OPT2-NEXT: [[ARRAYIDX:%.*]] = getelementptr i32, ptr [[WIDE_PTR_PTR]], i64 [[IDXPROM]]
65+
// OPT2-NEXT: [[WIDE_PTR_UB_ADDR:%.*]] = getelementptr inbounds nuw %"__bounds_safety::wide_ptr.bidi_indexable", ptr [[AGG_TEMP]], i32 0, i32 1
66+
// OPT2-NEXT: [[WIDE_PTR_UB:%.*]] = load ptr, ptr [[WIDE_PTR_UB_ADDR]], align 8
67+
// OPT2-NEXT: [[WIDE_PTR_LB_ADDR:%.*]] = getelementptr inbounds nuw %"__bounds_safety::wide_ptr.bidi_indexable", ptr [[AGG_TEMP]], i32 0, i32 2
68+
// OPT2-NEXT: [[WIDE_PTR_LB:%.*]] = load ptr, ptr [[WIDE_PTR_LB_ADDR]], align 8
69+
// OPT2-NEXT: [[TMP1:%.*]] = getelementptr i32, ptr [[ARRAYIDX]], i64 1, !annotation [[META13:![0-9]+]]
70+
// OPT2-NEXT: [[TMP2:%.*]] = icmp ule ptr [[TMP1]], [[WIDE_PTR_UB]], !annotation [[META13]]
71+
// OPT2-NEXT: br i1 [[TMP2]], label %[[CONT:.*]], label %[[TRAP:.*]], !prof [[PROF14:![0-9]+]], !annotation [[META13]]
72+
// OPT2: [[TRAP]]:
73+
// OPT2-NEXT: call void @llvm.ubsantrap(i8 25) #[[ATTR3:[0-9]+]], !annotation [[META13]]
74+
// OPT2-NEXT: unreachable, !annotation [[META13]]
75+
// OPT2: [[CONT]]:
76+
// OPT2-NEXT: [[TMP3:%.*]] = icmp ule ptr [[ARRAYIDX]], [[TMP1]], !annotation [[META13]]
77+
// OPT2-NEXT: br i1 [[TMP3]], label %[[CONT2:.*]], label %[[TRAP1:.*]], !prof [[PROF14]], !annotation [[META13]]
78+
// OPT2: [[TRAP1]]:
79+
// OPT2-NEXT: call void @llvm.ubsantrap(i8 25) #[[ATTR3]], !annotation [[META13]]
80+
// OPT2-NEXT: unreachable, !annotation [[META13]]
81+
// OPT2: [[CONT2]]:
82+
// OPT2-NEXT: [[TMP4:%.*]] = icmp uge ptr [[ARRAYIDX]], [[WIDE_PTR_LB]], !annotation [[META15:![0-9]+]]
83+
// OPT2-NEXT: br i1 [[TMP4]], label %[[CONT4:.*]], label %[[TRAP3:.*]], !prof [[PROF14]], !annotation [[META15]]
84+
// OPT2: [[TRAP3]]:
85+
// OPT2-NEXT: call void @llvm.ubsantrap(i8 25) #[[ATTR3]], !annotation [[META15]]
86+
// OPT2-NEXT: unreachable, !annotation [[META15]]
87+
// OPT2: [[CONT4]]:
88+
// OPT2-NEXT: [[TMP5:%.*]] = load i32, ptr [[ARRAYIDX]], align 4, !tbaa [[TBAA8]]
89+
// OPT2-NEXT: ret i32 [[TMP5]]
90+
//
91+
int consume(int* __bidi_indexable ptr, int idx) {
92+
return ptr[idx];
93+
}
94+
//
95+
//
96+
//
97+
//.
98+
// OPT0: attributes #[[ATTR0]] = { noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
99+
// OPT0: attributes #[[ATTR1:[0-9]+]] = { nocallback nofree nounwind willreturn memory(argmem: readwrite) }
100+
// OPT0: attributes #[[ATTR2:[0-9]+]] = { cold noreturn nounwind }
101+
// OPT0: attributes #[[ATTR3]] = { nomerge noreturn nounwind }
102+
//.
103+
// OPT2: attributes #[[ATTR0]] = { nounwind "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
104+
// OPT2: attributes #[[ATTR1:[0-9]+]] = { nocallback nofree nounwind willreturn memory(argmem: readwrite) }
105+
// OPT2: attributes #[[ATTR2:[0-9]+]] = { cold noreturn nounwind }
106+
// OPT2: attributes #[[ATTR3]] = { nomerge noreturn nounwind }
107+
//.
108+
// OPT0: [[META0:![0-9]+]] = !{i32 1, !"wchar_size", i32 4}
109+
// OPT0: [[META1:![0-9]+]] = !{!"{{.*}}clang version {{.*}}"}
110+
// OPT0: [[META2]] = !{!"bounds-safety-check-ptr-le-upper-bound"}
111+
// OPT0: [[PROF3]] = !{!"branch_weights", i32 1048575, i32 1}
112+
// OPT0: [[META4]] = !{!"bounds-safety-check-ptr-ge-lower-bound"}
113+
//.
114+
// OPT2: [[META0:![0-9]+]] = !{i32 1, !"wchar_size", i32 4}
115+
// OPT2: [[META1:![0-9]+]] = !{!"{{.*}}clang version {{.*}}"}
116+
// OPT2: [[TBAA2]] = !{[[META3:![0-9]+]], [[META3]], i64 0}
117+
// OPT2: [[META3]] = !{!"p2 int", [[META4:![0-9]+]], i64 0}
118+
// OPT2: [[META4]] = !{!"any p2 pointer", [[META5:![0-9]+]], i64 0}
119+
// OPT2: [[META5]] = !{!"any pointer", [[META6:![0-9]+]], i64 0}
120+
// OPT2: [[META6]] = !{!"omnipotent char", [[META7:![0-9]+]], i64 0}
121+
// OPT2: [[META7]] = !{!"Simple C/C++ TBAA"}
122+
// OPT2: [[TBAA8]] = !{[[META9:![0-9]+]], [[META9]], i64 0}
123+
// OPT2: [[META9]] = !{!"int", [[META6]], i64 0}
124+
// OPT2: [[TBAA_STRUCT10]] = !{i64 0, i64 24, [[META11:![0-9]+]]}
125+
// OPT2: [[META11]] = !{[[META12:![0-9]+]], [[META12]], i64 0}
126+
// OPT2: [[META12]] = !{!"p1 int", [[META5]], i64 0}
127+
// OPT2: [[META13]] = !{!"bounds-safety-check-ptr-le-upper-bound"}
128+
// OPT2: [[PROF14]] = !{!"branch_weights", i32 1048575, i32 1}
129+
// OPT2: [[META15]] = !{!"bounds-safety-check-ptr-ge-lower-bound"}
130+
//.

0 commit comments

Comments
 (0)