[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

Author: delcypher

clang/include/clang/Basic/CodeGenOptions.def

@@ -336,6 +336,11 @@ CODEGENOPT(ProfileSampleAccurate, 1, 0, Benign) ///< Sample profile is accurate.
 /* TO_UPSTREAM(BoundsSafety) ON*/
 CODEGENOPT(TrapFuncReturns   , 1, 0, Benign) ///< When true, the function specified with
                                              ///< -ftrap-function may return normally
+
+CODEGENOPT(BoundsSafetyUniqueTraps, 1, 0, Benign) ///< When true, merging of
+                                                  /// -fbounds-safety trap
+                                                  /// instructions will be
+                                                  /// prevented.
 /* TO_UPSTREAM(BoundsSafety) OFF*/
 
 /// Treat loops as finite: language, always, never.

clang/include/clang/Driver/Options.td

@@ -2036,6 +2036,18 @@ def fno_bounds_safety_bringup_missing_checks : Flag<["-"], "fno-bounds-safety-br
   Alias<fno_bounds_safety_bringup_missing_checks_EQ>, AliasArgs<["all"]>,
   HelpText<"Disable new -fbounds-safety run-time checks">;
 
+defm bounds_safety_unique_traps : BoolFOption<"bounds-safety-unique-traps",
+  CodeGenOpts<"BoundsSafetyUniqueTraps">, DefaultFalse,
+  PosFlag<SetTrue, [], [CC1Option],
+    "Make trap instructions unique by preventing traps from being merged by the"
+    " optimizer. This is useful for preserving the debug information on traps "
+    "in optimized code. This makes it easier to debug programs at the cost of "
+    "increased code size">,
+  NegFlag<SetFalse, [], [CC1Option],
+    "Don't try to make trap instructions unique. This allows trap instructions "
+    "to be merged by the optimizer.">>;
+// TO_UPSTREAM(BoundsSafety) OFF
+
 defm lifetime_safety : BoolFOption<
   "experimental-lifetime-safety",
   LangOpts<"EnableLifetimeSafety">, DefaultFalse,

clang/lib/CodeGen/CGExpr.cpp

@@ -4797,9 +4797,9 @@ void CodeGenFunction::EmitBoundsSafetyTrapCheck(llvm::Value *Checked,
   // We still need to pass `OptRemark` because not all emitted instructions
   // can be covered by BoundsSafetyOptRemarkScope. This is because EmitTrapCheck
   // caches basic blocks that contain instructions that need annotating.
-  EmitTrapCheck(Checked, SanitizerHandler::BoundsSafety, /*NoMerge=*/false,
-                /*TR=*/nullptr,
-                GetBoundsSafetyOptRemarkString(OptRemark),
+  EmitTrapCheck(Checked, SanitizerHandler::BoundsSafety,
+                /*NoMerge=*/CGM.getCodeGenOpts().BoundsSafetyUniqueTraps,
+                /*TR=*/nullptr, GetBoundsSafetyOptRemarkString(OptRemark),
                 GetBoundsSafetyTrapMessageSuffix(kind, TrapCtx));
 }
 

clang/lib/Driver/ToolChains/Clang.cpp

@@ -7041,6 +7041,9 @@ void Clang::ConstructJob(Compilation &C, const JobAction &Job,
         D.Diag(diag::err_drv_option_requires_option)
           << "-ftrap-function-returns" << "-ftrap-function=";
     }
+
+  Args.addOptInFlag(CmdArgs, options::OPT_fbounds_safety_unique_traps,
+                    options::OPT_fno_bounds_safety_unique_traps);
   /* TO_UPSTREAM(BoundsSafety) OFF*/
 
   // 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

@@ -0,0 +1,130 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --check-globals all --version 5
+// RUN: %clang_cc1 -O0 -triple arm64e-apple-ios -fbounds-safety \
+// RUN:   -fbounds-safety-unique-traps -emit-llvm %s -o - \
+// RUN: | FileCheck -check-prefix=OPT0 %s
+// RUN: %clang_cc1 -O2 -triple arm64e-apple-ios -fbounds-safety \
+// RUN:   -fbounds-safety-unique-traps -emit-llvm %s -disable-llvm-passes -o - \
+// RUN: | FileCheck -check-prefix=OPT2 %s
+
+#include <ptrcheck.h>
+
+// OPT0-LABEL: define i32 @consume(
+// OPT0-SAME: ptr dead_on_return noundef [[PTR:%.*]], i32 noundef [[IDX:%.*]]) #[[ATTR0:[0-9]+]] {
+// OPT0-NEXT:  [[ENTRY:.*:]]
+// OPT0-NEXT:    [[PTR_INDIRECT_ADDR:%.*]] = alloca ptr, align 8
+// OPT0-NEXT:    [[IDX_ADDR:%.*]] = alloca i32, align 4
+// OPT0-NEXT:    [[AGG_TEMP:%.*]] = alloca %"__bounds_safety::wide_ptr.bidi_indexable", align 8
+// OPT0-NEXT:    store ptr [[PTR]], ptr [[PTR_INDIRECT_ADDR]], align 8
+// OPT0-NEXT:    store i32 [[IDX]], ptr [[IDX_ADDR]], align 4
+// OPT0-NEXT:    call void @llvm.memcpy.p0.p0.i64(ptr align 8 [[AGG_TEMP]], ptr align 8 [[PTR]], i64 24, i1 false)
+// OPT0-NEXT:    [[WIDE_PTR_PTR_ADDR:%.*]] = getelementptr inbounds nuw %"__bounds_safety::wide_ptr.bidi_indexable", ptr [[AGG_TEMP]], i32 0, i32 0
+// OPT0-NEXT:    [[WIDE_PTR_PTR:%.*]] = load ptr, ptr [[WIDE_PTR_PTR_ADDR]], align 8
+// OPT0-NEXT:    [[TMP0:%.*]] = load i32, ptr [[IDX_ADDR]], align 4
+// OPT0-NEXT:    [[IDXPROM:%.*]] = sext i32 [[TMP0]] to i64
+// OPT0-NEXT:    [[ARRAYIDX:%.*]] = getelementptr i32, ptr [[WIDE_PTR_PTR]], i64 [[IDXPROM]]
+// OPT0-NEXT:    [[WIDE_PTR_UB_ADDR:%.*]] = getelementptr inbounds nuw %"__bounds_safety::wide_ptr.bidi_indexable", ptr [[AGG_TEMP]], i32 0, i32 1
+// OPT0-NEXT:    [[WIDE_PTR_UB:%.*]] = load ptr, ptr [[WIDE_PTR_UB_ADDR]], align 8
+// OPT0-NEXT:    [[WIDE_PTR_LB_ADDR:%.*]] = getelementptr inbounds nuw %"__bounds_safety::wide_ptr.bidi_indexable", ptr [[AGG_TEMP]], i32 0, i32 2
+// OPT0-NEXT:    [[WIDE_PTR_LB:%.*]] = load ptr, ptr [[WIDE_PTR_LB_ADDR]], align 8
+// OPT0-NEXT:    [[TMP1:%.*]] = getelementptr i32, ptr [[ARRAYIDX]], i64 1, !annotation [[META2:![0-9]+]]
+// OPT0-NEXT:    [[TMP2:%.*]] = icmp ule ptr [[TMP1]], [[WIDE_PTR_UB]], !annotation [[META2]]
+// OPT0-NEXT:    br i1 [[TMP2]], label %[[CONT:.*]], label %[[TRAP:.*]], !prof [[PROF3:![0-9]+]], !annotation [[META2]]
+// OPT0:       [[TRAP]]:
+// OPT0-NEXT:    call void @llvm.ubsantrap(i8 25) #[[ATTR3:[0-9]+]], !annotation [[META2]]
+// OPT0-NEXT:    unreachable, !annotation [[META2]]
+// OPT0:       [[CONT]]:
+// OPT0-NEXT:    [[TMP3:%.*]] = icmp ule ptr [[ARRAYIDX]], [[TMP1]], !annotation [[META2]]
+// OPT0-NEXT:    br i1 [[TMP3]], label %[[CONT2:.*]], label %[[TRAP1:.*]], !prof [[PROF3]], !annotation [[META2]]
+// OPT0:       [[TRAP1]]:
+// OPT0-NEXT:    call void @llvm.ubsantrap(i8 25) #[[ATTR3]], !annotation [[META2]]
+// OPT0-NEXT:    unreachable, !annotation [[META2]]
+// OPT0:       [[CONT2]]:
+// OPT0-NEXT:    [[TMP4:%.*]] = icmp uge ptr [[ARRAYIDX]], [[WIDE_PTR_LB]], !annotation [[META4:![0-9]+]]
+// OPT0-NEXT:    br i1 [[TMP4]], label %[[CONT4:.*]], label %[[TRAP3:.*]], !prof [[PROF3]], !annotation [[META4]]
+// OPT0:       [[TRAP3]]:
+// OPT0-NEXT:    call void @llvm.ubsantrap(i8 25) #[[ATTR3]], !annotation [[META4]]
+// OPT0-NEXT:    unreachable, !annotation [[META4]]
+// OPT0:       [[CONT4]]:
+// OPT0-NEXT:    [[TMP5:%.*]] = load i32, ptr [[ARRAYIDX]], align 4
+// OPT0-NEXT:    ret i32 [[TMP5]]
+//
+// OPT2-LABEL: define i32 @consume(
+// OPT2-SAME: ptr dead_on_return noundef [[PTR:%.*]], i32 noundef [[IDX:%.*]]) #[[ATTR0:[0-9]+]] {
+// OPT2-NEXT:  [[ENTRY:.*:]]
+// OPT2-NEXT:    [[PTR_INDIRECT_ADDR:%.*]] = alloca ptr, align 8
+// OPT2-NEXT:    [[IDX_ADDR:%.*]] = alloca i32, align 4
+// OPT2-NEXT:    [[AGG_TEMP:%.*]] = alloca %"__bounds_safety::wide_ptr.bidi_indexable", align 8
+// OPT2-NEXT:    store ptr [[PTR]], ptr [[PTR_INDIRECT_ADDR]], align 8, !tbaa [[TBAA2:![0-9]+]]
+// OPT2-NEXT:    store i32 [[IDX]], ptr [[IDX_ADDR]], align 4, !tbaa [[TBAA8:![0-9]+]]
+// 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]+]]
+// OPT2-NEXT:    [[WIDE_PTR_PTR_ADDR:%.*]] = getelementptr inbounds nuw %"__bounds_safety::wide_ptr.bidi_indexable", ptr [[AGG_TEMP]], i32 0, i32 0
+// OPT2-NEXT:    [[WIDE_PTR_PTR:%.*]] = load ptr, ptr [[WIDE_PTR_PTR_ADDR]], align 8
+// OPT2-NEXT:    [[TMP0:%.*]] = load i32, ptr [[IDX_ADDR]], align 4, !tbaa [[TBAA8]]
+// OPT2-NEXT:    [[IDXPROM:%.*]] = sext i32 [[TMP0]] to i64
+// OPT2-NEXT:    [[ARRAYIDX:%.*]] = getelementptr i32, ptr [[WIDE_PTR_PTR]], i64 [[IDXPROM]]
+// OPT2-NEXT:    [[WIDE_PTR_UB_ADDR:%.*]] = getelementptr inbounds nuw %"__bounds_safety::wide_ptr.bidi_indexable", ptr [[AGG_TEMP]], i32 0, i32 1
+// OPT2-NEXT:    [[WIDE_PTR_UB:%.*]] = load ptr, ptr [[WIDE_PTR_UB_ADDR]], align 8
+// OPT2-NEXT:    [[WIDE_PTR_LB_ADDR:%.*]] = getelementptr inbounds nuw %"__bounds_safety::wide_ptr.bidi_indexable", ptr [[AGG_TEMP]], i32 0, i32 2
+// OPT2-NEXT:    [[WIDE_PTR_LB:%.*]] = load ptr, ptr [[WIDE_PTR_LB_ADDR]], align 8
+// OPT2-NEXT:    [[TMP1:%.*]] = getelementptr i32, ptr [[ARRAYIDX]], i64 1, !annotation [[META13:![0-9]+]]
+// OPT2-NEXT:    [[TMP2:%.*]] = icmp ule ptr [[TMP1]], [[WIDE_PTR_UB]], !annotation [[META13]]
+// OPT2-NEXT:    br i1 [[TMP2]], label %[[CONT:.*]], label %[[TRAP:.*]], !prof [[PROF14:![0-9]+]], !annotation [[META13]]
+// OPT2:       [[TRAP]]:
+// OPT2-NEXT:    call void @llvm.ubsantrap(i8 25) #[[ATTR3:[0-9]+]], !annotation [[META13]]
+// OPT2-NEXT:    unreachable, !annotation [[META13]]
+// OPT2:       [[CONT]]:
+// OPT2-NEXT:    [[TMP3:%.*]] = icmp ule ptr [[ARRAYIDX]], [[TMP1]], !annotation [[META13]]
+// OPT2-NEXT:    br i1 [[TMP3]], label %[[CONT2:.*]], label %[[TRAP1:.*]], !prof [[PROF14]], !annotation [[META13]]
+// OPT2:       [[TRAP1]]:
+// OPT2-NEXT:    call void @llvm.ubsantrap(i8 25) #[[ATTR3]], !annotation [[META13]]
+// OPT2-NEXT:    unreachable, !annotation [[META13]]
+// OPT2:       [[CONT2]]:
+// OPT2-NEXT:    [[TMP4:%.*]] = icmp uge ptr [[ARRAYIDX]], [[WIDE_PTR_LB]], !annotation [[META15:![0-9]+]]
+// OPT2-NEXT:    br i1 [[TMP4]], label %[[CONT4:.*]], label %[[TRAP3:.*]], !prof [[PROF14]], !annotation [[META15]]
+// OPT2:       [[TRAP3]]:
+// OPT2-NEXT:    call void @llvm.ubsantrap(i8 25) #[[ATTR3]], !annotation [[META15]]
+// OPT2-NEXT:    unreachable, !annotation [[META15]]
+// OPT2:       [[CONT4]]:
+// OPT2-NEXT:    [[TMP5:%.*]] = load i32, ptr [[ARRAYIDX]], align 4, !tbaa [[TBAA8]]
+// OPT2-NEXT:    ret i32 [[TMP5]]
+//
+int consume(int* __bidi_indexable ptr, int idx) {
+  return ptr[idx];
+}
+//
+//
+//
+//.
+// OPT0: attributes #[[ATTR0]] = { noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
+// OPT0: attributes #[[ATTR1:[0-9]+]] = { nocallback nofree nounwind willreturn memory(argmem: readwrite) }
+// OPT0: attributes #[[ATTR2:[0-9]+]] = { cold noreturn nounwind memory(inaccessiblemem: write) }
+// OPT0: attributes #[[ATTR3]] = { nomerge noreturn nounwind }
+//.
+// OPT2: attributes #[[ATTR0]] = { nounwind "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
+// OPT2: attributes #[[ATTR1:[0-9]+]] = { nocallback nofree nounwind willreturn memory(argmem: readwrite) }
+// OPT2: attributes #[[ATTR2:[0-9]+]] = { cold noreturn nounwind memory(inaccessiblemem: write) }
+// OPT2: attributes #[[ATTR3]] = { nomerge noreturn nounwind }
+//.
+// OPT0: [[META0:![0-9]+]] = !{i32 1, !"wchar_size", i32 4}
+// OPT0: [[META1:![0-9]+]] = !{!"{{.*}}clang version {{.*}}"}
+// OPT0: [[META2]] = !{!"bounds-safety-check-ptr-le-upper-bound"}
+// OPT0: [[PROF3]] = !{!"branch_weights", i32 1048575, i32 1}
+// OPT0: [[META4]] = !{!"bounds-safety-check-ptr-ge-lower-bound"}
+//.
+// OPT2: [[META0:![0-9]+]] = !{i32 1, !"wchar_size", i32 4}
+// OPT2: [[META1:![0-9]+]] = !{!"{{.*}}clang version {{.*}}"}
+// OPT2: [[TBAA2]] = !{[[META3:![0-9]+]], [[META3]], i64 0}
+// OPT2: [[META3]] = !{!"p2 int", [[META4:![0-9]+]], i64 0}
+// OPT2: [[META4]] = !{!"any p2 pointer", [[META5:![0-9]+]], i64 0}
+// OPT2: [[META5]] = !{!"any pointer", [[META6:![0-9]+]], i64 0}
+// OPT2: [[META6]] = !{!"omnipotent char", [[META7:![0-9]+]], i64 0}
+// OPT2: [[META7]] = !{!"Simple C/C++ TBAA"}
+// OPT2: [[TBAA8]] = !{[[META9:![0-9]+]], [[META9]], i64 0}
+// OPT2: [[META9]] = !{!"int", [[META6]], i64 0}
+// OPT2: [[TBAA_STRUCT10]] = !{i64 0, i64 24, [[META11:![0-9]+]]}
+// OPT2: [[META11]] = !{[[META12:![0-9]+]], [[META12]], i64 0}
+// OPT2: [[META12]] = !{!"p1 int", [[META5]], i64 0}
+// OPT2: [[META13]] = !{!"bounds-safety-check-ptr-le-upper-bound"}
+// OPT2: [[PROF14]] = !{!"branch_weights", i32 1048575, i32 1}
+// OPT2: [[META15]] = !{!"bounds-safety-check-ptr-ge-lower-bound"}
+//.