Remove -funique-traps and replace it with a better implementation (rdar://158088410)

clang/include/clang/Basic/CodeGenOptions.def

@@ -336,10 +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(
-    UniqueTrapBlocks, 1,
-    0, Benign) ///< When true, basic blocks that contain traps they will be prevented
-               ///< from being merged by optimization passes and the backends.
+
+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,
@@ -4445,16 +4457,6 @@ defm trap_function_returns : BoolFOption<"trap-function-returns",
           "The trap function specified by -ftrap-function may return normally">,
   NegFlag<SetFalse, [], [ClangOption, CC1Option],
           "The trap function specified by -ftrap-function never returns">>;
-defm unique_traps : BoolFOption<"unique-traps",
-  CodeGenOpts<"UniqueTrapBlocks">, 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 */
 def funroll_loops : Flag<["-"], "funroll-loops">, Group<f_Group>,
   HelpText<"Turn on loop unroller">, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>;

clang/lib/CodeGen/CGExpr.cpp

@@ -4712,7 +4712,6 @@ void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
 
   /*TO_UPSTREAM(BoundsSafety) ON*/
   NoMerge |= CGM.getCodeGenOpts().TrapFuncReturns;
-  NoMerge |= CGM.getCodeGenOpts().UniqueTrapBlocks;
   /*TO_UPSTREAM(BoundsSafety) OFF*/
 
   if (TrapBB && !NoMerge) {
@@ -4741,11 +4740,7 @@ void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
     /* TO_UPSTREAM(BoundsSafety) OFF*/
   } else {
     /*TO_UPSTREAM(BoundsSafety) ON*/
-    if (CGM.getCodeGenOpts().UniqueTrapBlocks &&
-        !CGM.getCodeGenOpts().TrapFuncReturns)
-      TrapBB = createUnmergeableBasicBlock("trap");
-    else
-      TrapBB = createBasicBlock("trap");
+    TrapBB = createBasicBlock("trap");
     auto *BrInst = Builder.CreateCondBr(Checked, Cont, TrapBB,
                          MDHelper.createLikelyBranchWeights());
 
@@ -4802,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/CodeGen/CodeGenFunction.cpp

@@ -41,8 +41,6 @@
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/FPEnv.h"
-// TO_UPSTREAM(BoundsSafety) ON
-#include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
@@ -320,43 +318,6 @@ TypeEvaluationKind CodeGenFunction::getEvaluationKind(QualType type) {
   }
 }
 
-/* TO_UPSTREAM(BoundsSafety) ON*/
-llvm::BasicBlock *CodeGenFunction::createUnmergeableBasicBlock(
-    const Twine &name, llvm::Function *parent, llvm::BasicBlock *before) {
-  auto *BB = createBasicBlock(name, parent, before);
-  // This approach is the same approach used by Swift.
-  // TODO: Find a better way to do this (rdar://137627723).
-
-  // Emit unique side-effecting inline asm calls in order to eliminate
-  // the possibility that an LLVM optimization or code generation pass
-  // will merge these blocks back together again. We emit an empty asm
-  // string with the side-effect flag set, and with a unique integer
-  // argument.
-  llvm::IntegerType *asmArgTy = CGM.Int64Ty;
-  llvm::Type *argTys = {asmArgTy};
-  llvm::FunctionType *asmFnTy =
-      llvm::FunctionType::get(CGM.VoidTy, argTys, /* isVarArg=*/false);
-  // "n" is an input constraint stating that the first argument to the call
-  // will be an integer literal.
-  llvm::InlineAsm *inlineAsm =
-      llvm::InlineAsm::get(asmFnTy, /*AsmString=*/"", /*Constraints=*/"n",
-                           /*hasSideEffects=*/true);
-
-  // Use the builder so that any opt-remarks and attributes are automatically
-  // applied by the builder. The current state of the builder is saved so it
-  // can be used for creating the asm call and then the builder is reset to
-  // its previous state.
-  auto OldInsertPoint = Builder.GetInsertPoint();
-  auto* OldInsertBB = Builder.GetInsertBlock();
-  Builder.SetInsertPoint(BB);
-  Builder.CreateCall(
-      inlineAsm,
-      llvm::ConstantInt::get(asmArgTy, CGM.getAndIncrementUniqueTrapCount()));
-  Builder.SetInsertPoint(OldInsertBB, OldInsertPoint);
-  return BB;
-}
-/* TO_UPSTREAM(BoundsSafety) OFF*/
-
 llvm::DebugLoc CodeGenFunction::EmitReturnBlock() {
   // For cleanliness, we try to avoid emitting the return block for
   // simple cases.

clang/lib/CodeGen/CodeGenFunction.h

@@ -2659,13 +2659,6 @@ class CodeGenFunction : public CodeGenTypeCache {
     return llvm::BasicBlock::Create(getLLVMContext(), name, parent, before);
   }
 
-  /* TO_UPSTREM(BoundsSafety) ON*/
-  llvm::BasicBlock *
-  createUnmergeableBasicBlock(const Twine &name = "",
-                              llvm::Function *parent = nullptr,
-                              llvm::BasicBlock *before = nullptr);
-  /* TO_UPSTREM(BoundsSafety) OFF*/
-
   /// getBasicBlockForLabel - Return the LLVM basicblock that the specified
   /// label maps to.
   JumpDest getJumpDestForLabel(const LabelDecl *S);

clang/lib/CodeGen/CodeGenModule.h

@@ -682,9 +682,6 @@ class CodeGenModule : public CodeGenTypeCache {
   std::optional<PointerAuthQualifier>
   computeVTPointerAuthentication(const CXXRecordDecl *ThisClass);
 
-  // TO_UPSTREAM(BoundsSafety)
-  uint64_t UniqueTrapCount = 0;
-
   AtomicOptions AtomicOpts;
 
   // A set of functions which should be hot-patched; see
@@ -1159,11 +1156,6 @@ class CodeGenModule : public CodeGenTypeCache {
   /// Fetches the global unique block count.
   int getUniqueBlockCount() { return ++Block.GlobalUniqueCount; }
 
-  /* TO_UPSTREAM(BoundsSafety) ON*/
-  /// Fetches and increments unique trap count
-  uint64_t getAndIncrementUniqueTrapCount() { return UniqueTrapCount++; }
-  /* TO_UPSTREAM(BoundsSafety) OFF*/
-
   /// Fetches the type of a generic block descriptor.
   llvm::Type *getBlockDescriptorType();
 

clang/lib/Driver/ToolChains/Clang.cpp

@@ -7042,8 +7042,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction &Job,
           << "-ftrap-function-returns" << "-ftrap-function=";
     }
 
-  Args.addOptInFlag(CmdArgs, options::OPT_funique_traps,
-                    options::OPT_fno_unique_traps);
+  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-trap-blocks-O0-O2-opt-disabled.c → clang/test/BoundsSafety/CodeGen/unique-traps-O0-O2-opt-disabled.c

@@ -1,16 +1,13 @@
-// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --include-generated-funcs --version 5
+// 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:   -funique-traps -emit-llvm %s -o - \
+// 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:   -funique-traps -emit-llvm %s -disable-llvm-passes -o - \
+// RUN:   -fbounds-safety-unique-traps -emit-llvm %s -disable-llvm-passes -o - \
 // RUN: | FileCheck -check-prefix=OPT2 %s
 
 #include <ptrcheck.h>
 
-int consume(int* __bidi_indexable ptr, int idx) {
-  return ptr[idx];
-}
 // OPT0-LABEL: define i32 @consume(
 // OPT0-SAME: ptr dead_on_return noundef [[PTR:%.*]], i32 noundef [[IDX:%.*]]) #[[ATTR0:[0-9]+]] {
 // OPT0-NEXT:  [[ENTRY:.*:]]
@@ -33,28 +30,24 @@ int consume(int* __bidi_indexable ptr, int idx) {
 // 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 asm sideeffect "", "n"(i64 0), !annotation [[META2]]
 // 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 asm sideeffect "", "n"(i64 1), !annotation [[META2]]
 // 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 asm sideeffect "", "n"(i64 2), !annotation [[META4]]
 // 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:.*:]]
@@ -77,32 +70,49 @@ int consume(int* __bidi_indexable ptr, int idx) {
 // 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 asm sideeffect "", "n"(i64 0), !annotation [[META13]]
 // 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 asm sideeffect "", "n"(i64 1), !annotation [[META13]]
 // 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 asm sideeffect "", "n"(i64 2), !annotation [[META15]]
 // 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}