Reapply "[ubsan] Add -fsanitize-merge (and -fno-sanitize-merge) (#120…464)" by thurstond · Pull Request #120511 · llvm/llvm-project

thurstond · 2024-12-19T01:40:32Z

This reverts commit 2691b96. This reapply fixes the buildbot breakage of the original patch, by updating clang/test/CodeGen/ubsan-trap-debugloc.c to specify -fsanitize-merge (the default, which is merge, is applied by the driver but not clang_cc1).

This reapply also expands clang/test/CodeGen/ubsan-trap-merge.c.

Original commit message:
'-mllvm -ubsan-unique-traps' (#65972) applies to all UBSan checks. This patch introduces -fsanitize-merge (defaults to on, maintaining the status quo behavior) and -fno-sanitize-merge (equivalent to '-mllvm -ubsan-unique-traps'), with the option to selectively applying non-merged handlers to a subset of UBSan checks (e.g., -fno-sanitize-merge=bool,enum).

N.B. we do not use "trap" in the argument name since #119302 has generalized -ubsan-unique-traps to work for non-trap modes (min-rt and regular rt).

This patch does not remove the -ubsan-unique-traps flag; that will override -f(no-)sanitize-merge.

…#120464)" This reverts commit 2691b96. This reapply fixes the buildbot breakage of the original patch, by updating clang/test/CodeGen/ubsan-trap-debugloc.c to specify -fsanitize-merge (the default, which is merge, is applied by the driver but not clang_cc1). This reapply also expands clang/test/CodeGen/ubsan-trap-merge.c. Original commit message: '-mllvm -ubsan-unique-traps' (llvm#65972) applies to all UBSan checks. This patch introduces -fsanitize-merge (defaults to on, maintaining the status quo behavior) and -fno-sanitize-merge (equivalent to '-mllvm -ubsan-unique-traps'), with the option to selectively applying non-merged handlers to a subset of UBSan checks (e.g., -fno-sanitize-merge=bool,enum). N.B. we do not use "trap" in the argument name since llvm#119302 has generalized -ubsan-unique-traps to work for non-trap modes (min-rt and regular rt). This patch does not remove the -ubsan-unique-traps flag; that will override -f(no-)sanitize-merge.

llvmbot · 2024-12-19T01:41:02Z

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Thurston Dang (thurstond)

Changes

…464)"

This reverts commit 2691b96. This reapply fixes the buildbot breakage of the original patch, by updating clang/test/CodeGen/ubsan-trap-debugloc.c to specify -fsanitize-merge (the default, which is merge, is applied by the driver but not clang_cc1).

This reapply also expands clang/test/CodeGen/ubsan-trap-merge.c.

Original commit message:
'-mllvm -ubsan-unique-traps' (#65972) applies to all UBSan checks. This patch introduces -fsanitize-merge (defaults to on, maintaining the status quo behavior) and -fno-sanitize-merge (equivalent to '-mllvm -ubsan-unique-traps'), with the option to selectively applying non-merged handlers to a subset of UBSan checks (e.g., -fno-sanitize-merge=bool,enum).

N.B. we do not use "trap" in the argument name since #119302 has generalized -ubsan-unique-traps to work for non-trap modes (min-rt and regular rt).

This patch does not remove the -ubsan-unique-traps flag; that will override -f(no-)sanitize-merge.

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

10 Files Affected:

(modified) clang/include/clang/Basic/CodeGenOptions.h (+4)
(modified) clang/include/clang/Driver/Options.td (+15)
(modified) clang/include/clang/Driver/SanitizerArgs.h (+1)
(modified) clang/lib/CodeGen/CGExpr.cpp (+17-13)
(modified) clang/lib/CodeGen/CodeGenFunction.h (+2-1)
(modified) clang/lib/Driver/SanitizerArgs.cpp (+24-7)
(modified) clang/lib/Frontend/CompilerInvocation.cpp (+7)
(modified) clang/test/CodeGen/ubsan-trap-debugloc.c (+1-1)
(modified) clang/test/CodeGen/ubsan-trap-merge.c (+500-29)
(modified) clang/test/Driver/fsanitize.c (+53)

diff --git a/clang/include/clang/Basic/CodeGenOptions.h b/clang/include/clang/Basic/CodeGenOptions.h
index 2dcf98b465661e..8097c9ef772bc7 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -380,6 +380,10 @@ class CodeGenOptions : public CodeGenOptionsBase {
   /// Set of sanitizer checks that trap rather than diagnose.
   SanitizerSet SanitizeTrap;
 
+  /// Set of sanitizer checks that can merge handlers (smaller code size at
+  /// the expense of debuggability).
+  SanitizerSet SanitizeMergeHandlers;
+
   /// List of backend command-line options for -fembed-bitcode.
   std::vector<uint8_t> CmdArgs;
 
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 14e47f083ecec4..638f8c52053ec5 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2555,6 +2555,21 @@ def fno_sanitize_trap : Flag<["-"], "fno-sanitize-trap">, Group<f_clang_Group>,
                         Alias<fno_sanitize_trap_EQ>, AliasArgs<["all"]>,
                         Visibility<[ClangOption, CLOption]>,
                         HelpText<"Disable trapping for all sanitizers">;
+def fsanitize_merge_handlers_EQ
+    : CommaJoined<["-"], "fsanitize-merge=">,
+      Group<f_clang_Group>,
+      HelpText<"Allow compiler to merge handlers for specified sanitizers">;
+def fno_sanitize_merge_handlers_EQ
+    : CommaJoined<["-"], "fno-sanitize-merge=">,
+      Group<f_clang_Group>,
+      HelpText<"Do not allow compiler to merge handlers for specified sanitizers">;
+def fsanitize_merge_handlers : Flag<["-"], "fsanitize-merge">, Group<f_clang_Group>,
+                     Alias<fsanitize_merge_handlers_EQ>, AliasArgs<["all"]>,
+                     HelpText<"Allow compiler to merge handlers for all sanitizers">;
+def fno_sanitize_merge_handlers : Flag<["-"], "fno-sanitize-merge">, Group<f_clang_Group>,
+                        Alias<fno_sanitize_merge_handlers_EQ>, AliasArgs<["all"]>,
+                        Visibility<[ClangOption, CLOption]>,
+                        HelpText<"Do not allow compiler to merge handlers for any sanitizers">;
 def fsanitize_undefined_trap_on_error
     : Flag<["-"], "fsanitize-undefined-trap-on-error">, Group<f_clang_Group>,
       Alias<fsanitize_trap_EQ>, AliasArgs<["undefined"]>;
diff --git a/clang/include/clang/Driver/SanitizerArgs.h b/clang/include/clang/Driver/SanitizerArgs.h
index 4f08ea2b260179..7410ad4303011c 100644
--- a/clang/include/clang/Driver/SanitizerArgs.h
+++ b/clang/include/clang/Driver/SanitizerArgs.h
@@ -25,6 +25,7 @@ class SanitizerArgs {
   SanitizerSet Sanitizers;
   SanitizerSet RecoverableSanitizers;
   SanitizerSet TrapSanitizers;
+  SanitizerSet MergeHandlers;
 
   std::vector<std::string> UserIgnorelistFiles;
   std::vector<std::string> SystemIgnorelistFiles;
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 79955f55714164..d3fa5be6777ef4 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -3546,7 +3546,7 @@ static void emitCheckHandlerCall(CodeGenFunction &CGF,
                                  ArrayRef<llvm::Value *> FnArgs,
                                  SanitizerHandler CheckHandler,
                                  CheckRecoverableKind RecoverKind, bool IsFatal,
-                                 llvm::BasicBlock *ContBB) {
+                                 llvm::BasicBlock *ContBB, bool NoMerge) {
   assert(IsFatal || RecoverKind != CheckRecoverableKind::Unrecoverable);
   std::optional<ApplyDebugLocation> DL;
   if (!CGF.Builder.getCurrentDebugLocation()) {
@@ -3581,10 +3581,9 @@ static void emitCheckHandlerCall(CodeGenFunction &CGF,
                                llvm::AttributeList::FunctionIndex, B),
       /*Local=*/true);
   llvm::CallInst *HandlerCall = CGF.EmitNounwindRuntimeCall(Fn, FnArgs);
-  bool NoMerge =
-      ClSanitizeDebugDeoptimization ||
-      !CGF.CGM.getCodeGenOpts().OptimizationLevel ||
-      (CGF.CurCodeDecl && CGF.CurCodeDecl->hasAttr<OptimizeNoneAttr>());
+  NoMerge = NoMerge || ClSanitizeDebugDeoptimization ||
+            !CGF.CGM.getCodeGenOpts().OptimizationLevel ||
+            (CGF.CurCodeDecl && CGF.CurCodeDecl->hasAttr<OptimizeNoneAttr>());
   if (NoMerge)
     HandlerCall->addFnAttr(llvm::Attribute::NoMerge);
   if (!MayReturn) {
@@ -3608,6 +3607,7 @@ void CodeGenFunction::EmitCheck(
   llvm::Value *FatalCond = nullptr;
   llvm::Value *RecoverableCond = nullptr;
   llvm::Value *TrapCond = nullptr;
+  bool NoMerge = false;
   for (int i = 0, n = Checked.size(); i < n; ++i) {
     llvm::Value *Check = Checked[i].first;
     // -fsanitize-trap= overrides -fsanitize-recover=.
@@ -3618,6 +3618,9 @@ void CodeGenFunction::EmitCheck(
                   ? RecoverableCond
                   : FatalCond;
     Cond = Cond ? Builder.CreateAnd(Cond, Check) : Check;
+
+    if (!CGM.getCodeGenOpts().SanitizeMergeHandlers.has(Checked[i].second))
+      NoMerge = true;
   }
 
   if (ClSanitizeGuardChecks) {
@@ -3632,7 +3635,7 @@ void CodeGenFunction::EmitCheck(
   }
 
   if (TrapCond)
-    EmitTrapCheck(TrapCond, CheckHandler);
+    EmitTrapCheck(TrapCond, CheckHandler, NoMerge);
   if (!FatalCond && !RecoverableCond)
     return;
 
@@ -3698,7 +3701,7 @@ void CodeGenFunction::EmitCheck(
     // Simple case: we need to generate a single handler call, either
     // fatal, or non-fatal.
     emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind,
-                         (FatalCond != nullptr), Cont);
+                         (FatalCond != nullptr), Cont, NoMerge);
   } else {
     // Emit two handler calls: first one for set of unrecoverable checks,
     // another one for recoverable.
@@ -3708,10 +3711,10 @@ void CodeGenFunction::EmitCheck(
     Builder.CreateCondBr(FatalCond, NonFatalHandlerBB, FatalHandlerBB);
     EmitBlock(FatalHandlerBB);
     emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind, true,
-                         NonFatalHandlerBB);
+                         NonFatalHandlerBB, NoMerge);
     EmitBlock(NonFatalHandlerBB);
     emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind, false,
-                         Cont);
+                         Cont, NoMerge);
   }
 
   EmitBlock(Cont);
@@ -3901,7 +3904,8 @@ void CodeGenFunction::EmitUnreachable(SourceLocation Loc) {
 }
 
 void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
-                                    SanitizerHandler CheckHandlerID) {
+                                    SanitizerHandler CheckHandlerID,
+                                    bool NoMerge) {
   llvm::BasicBlock *Cont = createBasicBlock("cont");
 
   // If we're optimizing, collapse all calls to trap down to just one per
@@ -3911,9 +3915,9 @@ void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
 
   llvm::BasicBlock *&TrapBB = TrapBBs[CheckHandlerID];
 
-  bool NoMerge = ClSanitizeDebugDeoptimization ||
-                 !CGM.getCodeGenOpts().OptimizationLevel ||
-                 (CurCodeDecl && CurCodeDecl->hasAttr<OptimizeNoneAttr>());
+  NoMerge = NoMerge || ClSanitizeDebugDeoptimization ||
+            !CGM.getCodeGenOpts().OptimizationLevel ||
+            (CurCodeDecl && CurCodeDecl->hasAttr<OptimizeNoneAttr>());
 
   if (TrapBB && !NoMerge) {
     auto Call = TrapBB->begin();
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 847999cf1f28a0..4d4139180e100f 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -5171,7 +5171,8 @@ class CodeGenFunction : public CodeGenTypeCache {
 
   /// Create a basic block that will call the trap intrinsic, and emit a
   /// conditional branch to it, for the -ftrapv checks.
-  void EmitTrapCheck(llvm::Value *Checked, SanitizerHandler CheckHandlerID);
+  void EmitTrapCheck(llvm::Value *Checked, SanitizerHandler CheckHandlerID,
+                     bool NoMerge = false);
 
   /// Emit a call to trap or debugtrap and attach function attribute
   /// "trap-func-name" if specified.
diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index 0edfe641416129..727974833e6d1e 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -68,6 +68,7 @@ static const SanitizerMask TrappingSupported =
     SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
     SanitizerKind::LocalBounds | SanitizerKind::CFI |
     SanitizerKind::FloatDivideByZero | SanitizerKind::ObjCCast;
+static const SanitizerMask MergeDefault = SanitizerKind::Undefined;
 static const SanitizerMask TrappingDefault = SanitizerKind::CFI;
 static const SanitizerMask CFIClasses =
     SanitizerKind::CFIVCall | SanitizerKind::CFINVCall |
@@ -696,6 +697,13 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
   TrappingKinds &= Kinds;
   RecoverableKinds &= ~TrappingKinds;
 
+  // Parse -f(no-)?sanitize-nonmerged-handlers flags
+  SanitizerMask MergeKinds =
+      parseSanitizeArgs(D, Args, DiagnoseErrors, MergeDefault, {}, {},
+                        options::OPT_fsanitize_merge_handlers_EQ,
+                        options::OPT_fno_sanitize_merge_handlers_EQ);
+  MergeKinds &= Kinds;
+
   // Setup ignorelist files.
   // Add default ignorelist from resource directory for activated sanitizers,
   // and validate special case lists format.
@@ -1114,6 +1122,8 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
   TrapSanitizers.Mask |= TrappingKinds;
   assert(!(RecoverableKinds & TrappingKinds) &&
          "Overlap between recoverable and trapping sanitizers");
+
+  MergeHandlers.Mask |= MergeKinds;
 }
 
 static std::string toString(const clang::SanitizerSet &Sanitizers) {
@@ -1275,6 +1285,10 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
     CmdArgs.push_back(
         Args.MakeArgString("-fsanitize-trap=" + toString(TrapSanitizers)));
 
+  if (!MergeHandlers.empty())
+    CmdArgs.push_back(
+        Args.MakeArgString("-fsanitize-merge=" + toString(MergeHandlers)));
+
   addSpecialCaseListOpt(Args, CmdArgs,
                         "-fsanitize-ignorelist=", UserIgnorelistFiles);
   addSpecialCaseListOpt(Args, CmdArgs,
@@ -1442,13 +1456,16 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
 
 SanitizerMask parseArgValues(const Driver &D, const llvm::opt::Arg *A,
                              bool DiagnoseErrors) {
-  assert((A->getOption().matches(options::OPT_fsanitize_EQ) ||
-          A->getOption().matches(options::OPT_fno_sanitize_EQ) ||
-          A->getOption().matches(options::OPT_fsanitize_recover_EQ) ||
-          A->getOption().matches(options::OPT_fno_sanitize_recover_EQ) ||
-          A->getOption().matches(options::OPT_fsanitize_trap_EQ) ||
-          A->getOption().matches(options::OPT_fno_sanitize_trap_EQ)) &&
-         "Invalid argument in parseArgValues!");
+  assert(
+      (A->getOption().matches(options::OPT_fsanitize_EQ) ||
+       A->getOption().matches(options::OPT_fno_sanitize_EQ) ||
+       A->getOption().matches(options::OPT_fsanitize_recover_EQ) ||
+       A->getOption().matches(options::OPT_fno_sanitize_recover_EQ) ||
+       A->getOption().matches(options::OPT_fsanitize_trap_EQ) ||
+       A->getOption().matches(options::OPT_fno_sanitize_trap_EQ) ||
+       A->getOption().matches(options::OPT_fsanitize_merge_handlers_EQ) ||
+       A->getOption().matches(options::OPT_fno_sanitize_merge_handlers_EQ)) &&
+      "Invalid argument in parseArgValues!");
   SanitizerMask Kinds;
   for (int i = 0, n = A->getNumValues(); i != n; ++i) {
     const char *Value = A->getValue(i);
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 298fafc21588a1..348c56cc37da3f 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -1792,6 +1792,10 @@ void CompilerInvocationBase::GenerateCodeGenArgs(const CodeGenOptions &Opts,
   for (StringRef Sanitizer : serializeSanitizerKinds(Opts.SanitizeTrap))
     GenerateArg(Consumer, OPT_fsanitize_trap_EQ, Sanitizer);
 
+  for (StringRef Sanitizer :
+       serializeSanitizerKinds(Opts.SanitizeMergeHandlers))
+    GenerateArg(Consumer, OPT_fsanitize_merge_handlers_EQ, Sanitizer);
+
   if (!Opts.EmitVersionIdentMetadata)
     GenerateArg(Consumer, OPT_Qn);
 
@@ -2269,6 +2273,9 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args,
   parseSanitizerKinds("-fsanitize-trap=",
                       Args.getAllArgValues(OPT_fsanitize_trap_EQ), Diags,
                       Opts.SanitizeTrap);
+  parseSanitizerKinds("-fsanitize-merge=",
+                      Args.getAllArgValues(OPT_fsanitize_merge_handlers_EQ),
+                      Diags, Opts.SanitizeMergeHandlers);
 
   Opts.EmitVersionIdentMetadata = Args.hasFlag(OPT_Qy, OPT_Qn, true);
 
diff --git a/clang/test/CodeGen/ubsan-trap-debugloc.c b/clang/test/CodeGen/ubsan-trap-debugloc.c
index 4cad708b0ca50b..87cbfadec7d30c 100644
--- a/clang/test/CodeGen/ubsan-trap-debugloc.c
+++ b/clang/test/CodeGen/ubsan-trap-debugloc.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -O -fsanitize=signed-integer-overflow -fsanitize-trap=signed-integer-overflow %s -o - -debug-info-kind=line-tables-only | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -O -fsanitize=signed-integer-overflow -fsanitize-trap=signed-integer-overflow -fsanitize-merge=signed-integer-overflow %s -o - -debug-info-kind=line-tables-only | FileCheck %s
 
 
 void foo(volatile int a) {
diff --git a/clang/test/CodeGen/ubsan-trap-merge.c b/clang/test/CodeGen/ubsan-trap-merge.c
index 412ec7b09744ef..f211150a09cb67 100644
--- a/clang/test/CodeGen/ubsan-trap-merge.c
+++ b/clang/test/CodeGen/ubsan-trap-merge.c
@@ -1,10 +1,26 @@
 // 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 whether -ubsan-unique-traps attaches 'nomerge' to each ubsan call.
+// The most important assertions are the attributes at the end of the file, which
+// show whether -ubsan-unique-traps and -fno-sanitize-merge attach 'nomerge'
+// to each ubsan call.
 //
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -mllvm -ubsan-unique-traps %s -o - -fsanitize-trap=signed-integer-overflow | FileCheck %s --check-prefix=TRAP
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -mllvm -ubsan-unique-traps %s -o -                                         | FileCheck %s --check-prefix=HANDLER
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -mllvm -ubsan-unique-traps %s -o - -fsanitize-minimal-runtime              | FileCheck %s --check-prefix=MINRT
+// N.B. although the clang driver defaults to -fsanitize-merge=undefined,
+// clang_cc1 defaults to non-merge. (This is similar to -fsanitize-recover, for
+// which the default is also applied at the driver level only.)
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 %s -o - -fsanitize-trap=signed-integer-overflow | FileCheck %s --check-prefixes=TRAP-NOMERGE
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 %s -o -                                         | FileCheck %s --check-prefixes=HANDLER-NOMERGE
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 %s -o - -fsanitize-minimal-runtime              | FileCheck %s --check-prefixes=MINRT-NOMERGE
+//
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -mllvm -ubsan-unique-traps %s -o - -fsanitize-trap=signed-integer-overflow | FileCheck %s --check-prefixes=TRAP-NOMERGE
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -mllvm -ubsan-unique-traps %s -o -                                         | FileCheck %s --check-prefixes=HANDLER-NOMERGE
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -mllvm -ubsan-unique-traps %s -o - -fsanitize-minimal-runtime              | FileCheck %s --check-prefixes=MINRT-NOMERGE
+//
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -fno-sanitize-merge=signed-integer-overflow  %s -o - -fsanitize-trap=signed-integer-overflow | FileCheck %s --check-prefixes=TRAP-NOMERGE
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -fno-sanitize-merge=signed-integer-overflow  %s -o -                                         | FileCheck %s --check-prefixes=HANDLER-NOMERGE
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -fno-sanitize-merge=signed-integer-overflow  %s -o - -fsanitize-minimal-runtime              | FileCheck %s --check-prefixes=MINRT-NOMERGE
+//
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -fsanitize-merge=signed-integer-overflow %s -o - -fsanitize-trap=signed-integer-overflow | FileCheck %s --check-prefixes=TRAP-MERGE
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -fsanitize-merge=signed-integer-overflow %s -o -                                         | FileCheck %s --check-prefixes=HANDLER-MERGE
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -fsanitize-merge=signed-integer-overflow %s -o - -fsanitize-minimal-runtime              | FileCheck %s --check-prefixes=MINRT-MERGE
 //
 // REQUIRES: x86-registered-target
 
@@ -47,6 +63,85 @@
 // MINRT:       [[CONT]]:
 // MINRT-NEXT:    [[TMP2:%.*]] = extractvalue { i32, i1 } [[TMP0]], 0, !nosanitize [[META2]]
 // MINRT-NEXT:    ret i32 [[TMP2]]
+// TRAP-NOMERGE-LABEL: define dso_local range(i32 -2147483523, -2147483648) i32 @f(
+// TRAP-NOMERGE-SAME: i32 noundef [[X:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// 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:       [[TRAP]]:
+// TRAP-NOMERGE-NEXT:    tail call void @llvm.ubsantrap(i8 0) #[[ATTR4:[0-9]+]], !nosanitize [[META2]]
+// TRAP-NOMERGE-NEXT:    unreachable, !nosanitize [[META2]]
+// TRAP-NOMERGE:       [[CONT]]:
+// TRAP-NOMERGE-NEXT:    [[TMP2:%.*]] = extractvalue { i32, i1 } [[TMP0]], 0, !nosanitize [[META2]]
+// TRAP-NOMERGE-NEXT:    ret i32 [[TMP2]]
+//
+// HANDLER-NOMERGE-LABEL: define dso_local range(i32 -2147483523, -2147483648) i32 @f(
+// HANDLER-NOMERGE-SAME: i32 noundef [[X:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// HANDLER-NOMERGE-NEXT:  [[ENTRY:.*:]]
+// HANDLER-NOMERGE-NEXT:    [[TMP0:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[X]], i32 125), !nosanitize [[META2:![0-9]+]]
+// HANDLER-NOMERGE-NEXT:    [[TMP1:%.*]] = extractvalue { i32, i1 } [[TMP0]], 1, !nosanitize [[META2]]
+// HANDLER-NOMERGE-NEXT:    br i1 [[TMP1]], label %[[HANDLER_ADD_OVERFLOW:.*]], label %[[CONT:.*]], !prof [[PROF3:![0-9]+]], !nosanitize [[META2]]
+// HANDLER-NOMERGE:       [[HANDLER_ADD_OVERFLOW]]:
+// HANDLER-NOMERGE-NEXT:    [[TMP2:%.*]] = zext nneg i32 [[X]] to i64, !nosanitize [[META2]]
+// HANDLER-NOMERGE-NEXT:    tail call void @__ubsan_handle_add_overflow_abort(ptr nonnull @[[GLOB1:[0-9]+]], i64 [[TMP2]], i64 125) #[[ATTR4:[0-9]+]], !nosanitize [[META2]]
+// HANDLER-NOMERGE-NEXT:    unreachable, !nosanitize [[META2]]
+// HANDLER-NOMERGE:       [[CONT]]:
+// HANDLER-NOMERGE-NEXT:    [[TMP3:%.*]] = extractvalue { i32, i1 } [[TMP0]], 0, !nosanitize [[META2]]
+// HANDLER-NOMERGE-NEXT:    ret i32 [[TMP3]]
+//
+// MINRT-NOMERGE-LABEL: define dso_local range(i32 -2147483523, -2147483648) i32 @f(
+// MINRT-NOMERGE-SAME: i32 noundef [[X:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// MINRT-NOMERGE-NEXT:  [[ENTRY:.*:]]
+// MINRT-NOMERGE-NEXT:    [[TMP0:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[X]], i32 125), !nosanitize [[META2:![0-9]+]]
+// MINRT-NO...
[truncated]

llvmbot · 2024-12-19T01:41:03Z

@llvm/pr-subscribers-clang-codegen

Author: Thurston Dang (thurstond)

Changes

…464)"

This reverts commit 2691b96. This reapply fixes the buildbot breakage of the original patch, by updating clang/test/CodeGen/ubsan-trap-debugloc.c to specify -fsanitize-merge (the default, which is merge, is applied by the driver but not clang_cc1).

This reapply also expands clang/test/CodeGen/ubsan-trap-merge.c.

Original commit message:
'-mllvm -ubsan-unique-traps' (#65972) applies to all UBSan checks. This patch introduces -fsanitize-merge (defaults to on, maintaining the status quo behavior) and -fno-sanitize-merge (equivalent to '-mllvm -ubsan-unique-traps'), with the option to selectively applying non-merged handlers to a subset of UBSan checks (e.g., -fno-sanitize-merge=bool,enum).

N.B. we do not use "trap" in the argument name since #119302 has generalized -ubsan-unique-traps to work for non-trap modes (min-rt and regular rt).

This patch does not remove the -ubsan-unique-traps flag; that will override -f(no-)sanitize-merge.

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

10 Files Affected:

(modified) clang/include/clang/Basic/CodeGenOptions.h (+4)
(modified) clang/include/clang/Driver/Options.td (+15)
(modified) clang/include/clang/Driver/SanitizerArgs.h (+1)
(modified) clang/lib/CodeGen/CGExpr.cpp (+17-13)
(modified) clang/lib/CodeGen/CodeGenFunction.h (+2-1)
(modified) clang/lib/Driver/SanitizerArgs.cpp (+24-7)
(modified) clang/lib/Frontend/CompilerInvocation.cpp (+7)
(modified) clang/test/CodeGen/ubsan-trap-debugloc.c (+1-1)
(modified) clang/test/CodeGen/ubsan-trap-merge.c (+500-29)
(modified) clang/test/Driver/fsanitize.c (+53)

diff --git a/clang/include/clang/Basic/CodeGenOptions.h b/clang/include/clang/Basic/CodeGenOptions.h
index 2dcf98b465661e..8097c9ef772bc7 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -380,6 +380,10 @@ class CodeGenOptions : public CodeGenOptionsBase {
   /// Set of sanitizer checks that trap rather than diagnose.
   SanitizerSet SanitizeTrap;
 
+  /// Set of sanitizer checks that can merge handlers (smaller code size at
+  /// the expense of debuggability).
+  SanitizerSet SanitizeMergeHandlers;
+
   /// List of backend command-line options for -fembed-bitcode.
   std::vector<uint8_t> CmdArgs;
 
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 14e47f083ecec4..638f8c52053ec5 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2555,6 +2555,21 @@ def fno_sanitize_trap : Flag<["-"], "fno-sanitize-trap">, Group<f_clang_Group>,
                         Alias<fno_sanitize_trap_EQ>, AliasArgs<["all"]>,
                         Visibility<[ClangOption, CLOption]>,
                         HelpText<"Disable trapping for all sanitizers">;
+def fsanitize_merge_handlers_EQ
+    : CommaJoined<["-"], "fsanitize-merge=">,
+      Group<f_clang_Group>,
+      HelpText<"Allow compiler to merge handlers for specified sanitizers">;
+def fno_sanitize_merge_handlers_EQ
+    : CommaJoined<["-"], "fno-sanitize-merge=">,
+      Group<f_clang_Group>,
+      HelpText<"Do not allow compiler to merge handlers for specified sanitizers">;
+def fsanitize_merge_handlers : Flag<["-"], "fsanitize-merge">, Group<f_clang_Group>,
+                     Alias<fsanitize_merge_handlers_EQ>, AliasArgs<["all"]>,
+                     HelpText<"Allow compiler to merge handlers for all sanitizers">;
+def fno_sanitize_merge_handlers : Flag<["-"], "fno-sanitize-merge">, Group<f_clang_Group>,
+                        Alias<fno_sanitize_merge_handlers_EQ>, AliasArgs<["all"]>,
+                        Visibility<[ClangOption, CLOption]>,
+                        HelpText<"Do not allow compiler to merge handlers for any sanitizers">;
 def fsanitize_undefined_trap_on_error
     : Flag<["-"], "fsanitize-undefined-trap-on-error">, Group<f_clang_Group>,
       Alias<fsanitize_trap_EQ>, AliasArgs<["undefined"]>;
diff --git a/clang/include/clang/Driver/SanitizerArgs.h b/clang/include/clang/Driver/SanitizerArgs.h
index 4f08ea2b260179..7410ad4303011c 100644
--- a/clang/include/clang/Driver/SanitizerArgs.h
+++ b/clang/include/clang/Driver/SanitizerArgs.h
@@ -25,6 +25,7 @@ class SanitizerArgs {
   SanitizerSet Sanitizers;
   SanitizerSet RecoverableSanitizers;
   SanitizerSet TrapSanitizers;
+  SanitizerSet MergeHandlers;
 
   std::vector<std::string> UserIgnorelistFiles;
   std::vector<std::string> SystemIgnorelistFiles;
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 79955f55714164..d3fa5be6777ef4 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -3546,7 +3546,7 @@ static void emitCheckHandlerCall(CodeGenFunction &CGF,
                                  ArrayRef<llvm::Value *> FnArgs,
                                  SanitizerHandler CheckHandler,
                                  CheckRecoverableKind RecoverKind, bool IsFatal,
-                                 llvm::BasicBlock *ContBB) {
+                                 llvm::BasicBlock *ContBB, bool NoMerge) {
   assert(IsFatal || RecoverKind != CheckRecoverableKind::Unrecoverable);
   std::optional<ApplyDebugLocation> DL;
   if (!CGF.Builder.getCurrentDebugLocation()) {
@@ -3581,10 +3581,9 @@ static void emitCheckHandlerCall(CodeGenFunction &CGF,
                                llvm::AttributeList::FunctionIndex, B),
       /*Local=*/true);
   llvm::CallInst *HandlerCall = CGF.EmitNounwindRuntimeCall(Fn, FnArgs);
-  bool NoMerge =
-      ClSanitizeDebugDeoptimization ||
-      !CGF.CGM.getCodeGenOpts().OptimizationLevel ||
-      (CGF.CurCodeDecl && CGF.CurCodeDecl->hasAttr<OptimizeNoneAttr>());
+  NoMerge = NoMerge || ClSanitizeDebugDeoptimization ||
+            !CGF.CGM.getCodeGenOpts().OptimizationLevel ||
+            (CGF.CurCodeDecl && CGF.CurCodeDecl->hasAttr<OptimizeNoneAttr>());
   if (NoMerge)
     HandlerCall->addFnAttr(llvm::Attribute::NoMerge);
   if (!MayReturn) {
@@ -3608,6 +3607,7 @@ void CodeGenFunction::EmitCheck(
   llvm::Value *FatalCond = nullptr;
   llvm::Value *RecoverableCond = nullptr;
   llvm::Value *TrapCond = nullptr;
+  bool NoMerge = false;
   for (int i = 0, n = Checked.size(); i < n; ++i) {
     llvm::Value *Check = Checked[i].first;
     // -fsanitize-trap= overrides -fsanitize-recover=.
@@ -3618,6 +3618,9 @@ void CodeGenFunction::EmitCheck(
                   ? RecoverableCond
                   : FatalCond;
     Cond = Cond ? Builder.CreateAnd(Cond, Check) : Check;
+
+    if (!CGM.getCodeGenOpts().SanitizeMergeHandlers.has(Checked[i].second))
+      NoMerge = true;
   }
 
   if (ClSanitizeGuardChecks) {
@@ -3632,7 +3635,7 @@ void CodeGenFunction::EmitCheck(
   }
 
   if (TrapCond)
-    EmitTrapCheck(TrapCond, CheckHandler);
+    EmitTrapCheck(TrapCond, CheckHandler, NoMerge);
   if (!FatalCond && !RecoverableCond)
     return;
 
@@ -3698,7 +3701,7 @@ void CodeGenFunction::EmitCheck(
     // Simple case: we need to generate a single handler call, either
     // fatal, or non-fatal.
     emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind,
-                         (FatalCond != nullptr), Cont);
+                         (FatalCond != nullptr), Cont, NoMerge);
   } else {
     // Emit two handler calls: first one for set of unrecoverable checks,
     // another one for recoverable.
@@ -3708,10 +3711,10 @@ void CodeGenFunction::EmitCheck(
     Builder.CreateCondBr(FatalCond, NonFatalHandlerBB, FatalHandlerBB);
     EmitBlock(FatalHandlerBB);
     emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind, true,
-                         NonFatalHandlerBB);
+                         NonFatalHandlerBB, NoMerge);
     EmitBlock(NonFatalHandlerBB);
     emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind, false,
-                         Cont);
+                         Cont, NoMerge);
   }
 
   EmitBlock(Cont);
@@ -3901,7 +3904,8 @@ void CodeGenFunction::EmitUnreachable(SourceLocation Loc) {
 }
 
 void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
-                                    SanitizerHandler CheckHandlerID) {
+                                    SanitizerHandler CheckHandlerID,
+                                    bool NoMerge) {
   llvm::BasicBlock *Cont = createBasicBlock("cont");
 
   // If we're optimizing, collapse all calls to trap down to just one per
@@ -3911,9 +3915,9 @@ void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
 
   llvm::BasicBlock *&TrapBB = TrapBBs[CheckHandlerID];
 
-  bool NoMerge = ClSanitizeDebugDeoptimization ||
-                 !CGM.getCodeGenOpts().OptimizationLevel ||
-                 (CurCodeDecl && CurCodeDecl->hasAttr<OptimizeNoneAttr>());
+  NoMerge = NoMerge || ClSanitizeDebugDeoptimization ||
+            !CGM.getCodeGenOpts().OptimizationLevel ||
+            (CurCodeDecl && CurCodeDecl->hasAttr<OptimizeNoneAttr>());
 
   if (TrapBB && !NoMerge) {
     auto Call = TrapBB->begin();
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 847999cf1f28a0..4d4139180e100f 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -5171,7 +5171,8 @@ class CodeGenFunction : public CodeGenTypeCache {
 
   /// Create a basic block that will call the trap intrinsic, and emit a
   /// conditional branch to it, for the -ftrapv checks.
-  void EmitTrapCheck(llvm::Value *Checked, SanitizerHandler CheckHandlerID);
+  void EmitTrapCheck(llvm::Value *Checked, SanitizerHandler CheckHandlerID,
+                     bool NoMerge = false);
 
   /// Emit a call to trap or debugtrap and attach function attribute
   /// "trap-func-name" if specified.
diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index 0edfe641416129..727974833e6d1e 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -68,6 +68,7 @@ static const SanitizerMask TrappingSupported =
     SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
     SanitizerKind::LocalBounds | SanitizerKind::CFI |
     SanitizerKind::FloatDivideByZero | SanitizerKind::ObjCCast;
+static const SanitizerMask MergeDefault = SanitizerKind::Undefined;
 static const SanitizerMask TrappingDefault = SanitizerKind::CFI;
 static const SanitizerMask CFIClasses =
     SanitizerKind::CFIVCall | SanitizerKind::CFINVCall |
@@ -696,6 +697,13 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
   TrappingKinds &= Kinds;
   RecoverableKinds &= ~TrappingKinds;
 
+  // Parse -f(no-)?sanitize-nonmerged-handlers flags
+  SanitizerMask MergeKinds =
+      parseSanitizeArgs(D, Args, DiagnoseErrors, MergeDefault, {}, {},
+                        options::OPT_fsanitize_merge_handlers_EQ,
+                        options::OPT_fno_sanitize_merge_handlers_EQ);
+  MergeKinds &= Kinds;
+
   // Setup ignorelist files.
   // Add default ignorelist from resource directory for activated sanitizers,
   // and validate special case lists format.
@@ -1114,6 +1122,8 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
   TrapSanitizers.Mask |= TrappingKinds;
   assert(!(RecoverableKinds & TrappingKinds) &&
          "Overlap between recoverable and trapping sanitizers");
+
+  MergeHandlers.Mask |= MergeKinds;
 }
 
 static std::string toString(const clang::SanitizerSet &Sanitizers) {
@@ -1275,6 +1285,10 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
     CmdArgs.push_back(
         Args.MakeArgString("-fsanitize-trap=" + toString(TrapSanitizers)));
 
+  if (!MergeHandlers.empty())
+    CmdArgs.push_back(
+        Args.MakeArgString("-fsanitize-merge=" + toString(MergeHandlers)));
+
   addSpecialCaseListOpt(Args, CmdArgs,
                         "-fsanitize-ignorelist=", UserIgnorelistFiles);
   addSpecialCaseListOpt(Args, CmdArgs,
@@ -1442,13 +1456,16 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
 
 SanitizerMask parseArgValues(const Driver &D, const llvm::opt::Arg *A,
                              bool DiagnoseErrors) {
-  assert((A->getOption().matches(options::OPT_fsanitize_EQ) ||
-          A->getOption().matches(options::OPT_fno_sanitize_EQ) ||
-          A->getOption().matches(options::OPT_fsanitize_recover_EQ) ||
-          A->getOption().matches(options::OPT_fno_sanitize_recover_EQ) ||
-          A->getOption().matches(options::OPT_fsanitize_trap_EQ) ||
-          A->getOption().matches(options::OPT_fno_sanitize_trap_EQ)) &&
-         "Invalid argument in parseArgValues!");
+  assert(
+      (A->getOption().matches(options::OPT_fsanitize_EQ) ||
+       A->getOption().matches(options::OPT_fno_sanitize_EQ) ||
+       A->getOption().matches(options::OPT_fsanitize_recover_EQ) ||
+       A->getOption().matches(options::OPT_fno_sanitize_recover_EQ) ||
+       A->getOption().matches(options::OPT_fsanitize_trap_EQ) ||
+       A->getOption().matches(options::OPT_fno_sanitize_trap_EQ) ||
+       A->getOption().matches(options::OPT_fsanitize_merge_handlers_EQ) ||
+       A->getOption().matches(options::OPT_fno_sanitize_merge_handlers_EQ)) &&
+      "Invalid argument in parseArgValues!");
   SanitizerMask Kinds;
   for (int i = 0, n = A->getNumValues(); i != n; ++i) {
     const char *Value = A->getValue(i);
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 298fafc21588a1..348c56cc37da3f 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -1792,6 +1792,10 @@ void CompilerInvocationBase::GenerateCodeGenArgs(const CodeGenOptions &Opts,
   for (StringRef Sanitizer : serializeSanitizerKinds(Opts.SanitizeTrap))
     GenerateArg(Consumer, OPT_fsanitize_trap_EQ, Sanitizer);
 
+  for (StringRef Sanitizer :
+       serializeSanitizerKinds(Opts.SanitizeMergeHandlers))
+    GenerateArg(Consumer, OPT_fsanitize_merge_handlers_EQ, Sanitizer);
+
   if (!Opts.EmitVersionIdentMetadata)
     GenerateArg(Consumer, OPT_Qn);
 
@@ -2269,6 +2273,9 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args,
   parseSanitizerKinds("-fsanitize-trap=",
                       Args.getAllArgValues(OPT_fsanitize_trap_EQ), Diags,
                       Opts.SanitizeTrap);
+  parseSanitizerKinds("-fsanitize-merge=",
+                      Args.getAllArgValues(OPT_fsanitize_merge_handlers_EQ),
+                      Diags, Opts.SanitizeMergeHandlers);
 
   Opts.EmitVersionIdentMetadata = Args.hasFlag(OPT_Qy, OPT_Qn, true);
 
diff --git a/clang/test/CodeGen/ubsan-trap-debugloc.c b/clang/test/CodeGen/ubsan-trap-debugloc.c
index 4cad708b0ca50b..87cbfadec7d30c 100644
--- a/clang/test/CodeGen/ubsan-trap-debugloc.c
+++ b/clang/test/CodeGen/ubsan-trap-debugloc.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -O -fsanitize=signed-integer-overflow -fsanitize-trap=signed-integer-overflow %s -o - -debug-info-kind=line-tables-only | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -O -fsanitize=signed-integer-overflow -fsanitize-trap=signed-integer-overflow -fsanitize-merge=signed-integer-overflow %s -o - -debug-info-kind=line-tables-only | FileCheck %s
 
 
 void foo(volatile int a) {
diff --git a/clang/test/CodeGen/ubsan-trap-merge.c b/clang/test/CodeGen/ubsan-trap-merge.c
index 412ec7b09744ef..f211150a09cb67 100644
--- a/clang/test/CodeGen/ubsan-trap-merge.c
+++ b/clang/test/CodeGen/ubsan-trap-merge.c
@@ -1,10 +1,26 @@
 // 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 whether -ubsan-unique-traps attaches 'nomerge' to each ubsan call.
+// The most important assertions are the attributes at the end of the file, which
+// show whether -ubsan-unique-traps and -fno-sanitize-merge attach 'nomerge'
+// to each ubsan call.
 //
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -mllvm -ubsan-unique-traps %s -o - -fsanitize-trap=signed-integer-overflow | FileCheck %s --check-prefix=TRAP
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -mllvm -ubsan-unique-traps %s -o -                                         | FileCheck %s --check-prefix=HANDLER
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -mllvm -ubsan-unique-traps %s -o - -fsanitize-minimal-runtime              | FileCheck %s --check-prefix=MINRT
+// N.B. although the clang driver defaults to -fsanitize-merge=undefined,
+// clang_cc1 defaults to non-merge. (This is similar to -fsanitize-recover, for
+// which the default is also applied at the driver level only.)
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 %s -o - -fsanitize-trap=signed-integer-overflow | FileCheck %s --check-prefixes=TRAP-NOMERGE
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 %s -o -                                         | FileCheck %s --check-prefixes=HANDLER-NOMERGE
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 %s -o - -fsanitize-minimal-runtime              | FileCheck %s --check-prefixes=MINRT-NOMERGE
+//
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -mllvm -ubsan-unique-traps %s -o - -fsanitize-trap=signed-integer-overflow | FileCheck %s --check-prefixes=TRAP-NOMERGE
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -mllvm -ubsan-unique-traps %s -o -                                         | FileCheck %s --check-prefixes=HANDLER-NOMERGE
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -mllvm -ubsan-unique-traps %s -o - -fsanitize-minimal-runtime              | FileCheck %s --check-prefixes=MINRT-NOMERGE
+//
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -fno-sanitize-merge=signed-integer-overflow  %s -o - -fsanitize-trap=signed-integer-overflow | FileCheck %s --check-prefixes=TRAP-NOMERGE
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -fno-sanitize-merge=signed-integer-overflow  %s -o -                                         | FileCheck %s --check-prefixes=HANDLER-NOMERGE
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -fno-sanitize-merge=signed-integer-overflow  %s -o - -fsanitize-minimal-runtime              | FileCheck %s --check-prefixes=MINRT-NOMERGE
+//
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -fsanitize-merge=signed-integer-overflow %s -o - -fsanitize-trap=signed-integer-overflow | FileCheck %s --check-prefixes=TRAP-MERGE
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -fsanitize-merge=signed-integer-overflow %s -o -                                         | FileCheck %s --check-prefixes=HANDLER-MERGE
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -fsanitize-merge=signed-integer-overflow %s -o - -fsanitize-minimal-runtime              | FileCheck %s --check-prefixes=MINRT-MERGE
 //
 // REQUIRES: x86-registered-target
 
@@ -47,6 +63,85 @@
 // MINRT:       [[CONT]]:
 // MINRT-NEXT:    [[TMP2:%.*]] = extractvalue { i32, i1 } [[TMP0]], 0, !nosanitize [[META2]]
 // MINRT-NEXT:    ret i32 [[TMP2]]
+// TRAP-NOMERGE-LABEL: define dso_local range(i32 -2147483523, -2147483648) i32 @f(
+// TRAP-NOMERGE-SAME: i32 noundef [[X:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// 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:       [[TRAP]]:
+// TRAP-NOMERGE-NEXT:    tail call void @llvm.ubsantrap(i8 0) #[[ATTR4:[0-9]+]], !nosanitize [[META2]]
+// TRAP-NOMERGE-NEXT:    unreachable, !nosanitize [[META2]]
+// TRAP-NOMERGE:       [[CONT]]:
+// TRAP-NOMERGE-NEXT:    [[TMP2:%.*]] = extractvalue { i32, i1 } [[TMP0]], 0, !nosanitize [[META2]]
+// TRAP-NOMERGE-NEXT:    ret i32 [[TMP2]]
+//
+// HANDLER-NOMERGE-LABEL: define dso_local range(i32 -2147483523, -2147483648) i32 @f(
+// HANDLER-NOMERGE-SAME: i32 noundef [[X:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// HANDLER-NOMERGE-NEXT:  [[ENTRY:.*:]]
+// HANDLER-NOMERGE-NEXT:    [[TMP0:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[X]], i32 125), !nosanitize [[META2:![0-9]+]]
+// HANDLER-NOMERGE-NEXT:    [[TMP1:%.*]] = extractvalue { i32, i1 } [[TMP0]], 1, !nosanitize [[META2]]
+// HANDLER-NOMERGE-NEXT:    br i1 [[TMP1]], label %[[HANDLER_ADD_OVERFLOW:.*]], label %[[CONT:.*]], !prof [[PROF3:![0-9]+]], !nosanitize [[META2]]
+// HANDLER-NOMERGE:       [[HANDLER_ADD_OVERFLOW]]:
+// HANDLER-NOMERGE-NEXT:    [[TMP2:%.*]] = zext nneg i32 [[X]] to i64, !nosanitize [[META2]]
+// HANDLER-NOMERGE-NEXT:    tail call void @__ubsan_handle_add_overflow_abort(ptr nonnull @[[GLOB1:[0-9]+]], i64 [[TMP2]], i64 125) #[[ATTR4:[0-9]+]], !nosanitize [[META2]]
+// HANDLER-NOMERGE-NEXT:    unreachable, !nosanitize [[META2]]
+// HANDLER-NOMERGE:       [[CONT]]:
+// HANDLER-NOMERGE-NEXT:    [[TMP3:%.*]] = extractvalue { i32, i1 } [[TMP0]], 0, !nosanitize [[META2]]
+// HANDLER-NOMERGE-NEXT:    ret i32 [[TMP3]]
+//
+// MINRT-NOMERGE-LABEL: define dso_local range(i32 -2147483523, -2147483648) i32 @f(
+// MINRT-NOMERGE-SAME: i32 noundef [[X:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// MINRT-NOMERGE-NEXT:  [[ENTRY:.*:]]
+// MINRT-NOMERGE-NEXT:    [[TMP0:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[X]], i32 125), !nosanitize [[META2:![0-9]+]]
+// MINRT-NO...
[truncated]

vitalybuka · 2024-12-19T02:08:47Z

clang/test/CodeGen/ubsan-trap-merge.c

-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -mllvm -ubsan-unique-traps %s -o -                                         | FileCheck %s --check-prefix=HANDLER
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -mllvm -ubsan-unique-traps %s -o - -fsanitize-minimal-runtime              | FileCheck %s --check-prefix=MINRT
+// N.B. although the clang driver defaults to -fsanitize-merge=undefined,
+// clang_cc1 defaults to non-merge. (This is similar to -fsanitize-recover, for


how about removing -ubsan-unique-traps from tests?
Later we can remove it completely.

maybe removing from tests later is also OK

I am planning to write a patch that will remove -ubsan-unique-traps and update the tests accordingly.

I am planning to write a patch that will remove -ubsan-unique-traps and update the tests accordingly.

#120613

-fno-sanitize-merge (introduced in llvm#120511) duplicates the functionality of -ubsan-unique-traps but also allows individual checks to be specified e.g., * "-fno-sanitize-merge" without arguments is equivalent to -ubsan-unique-traps * "-fno-sanitize-merge=bool,enum" will apply it only to those two checks This patch also adds negative test examples to bounds-checking.c, and strengthens the NOOPTARRAY assertion to prevent spurious matches. Note: this change necessarily breaks compatibility with '-mllvm -ubsan-unique-traps'. We hope that this is acceptable since '-mllvm -ubsan-unique-traps' was an experimental flag. "-bounds-checking-unique-traps" is unaffected by this patch.

…#120613) -fno-sanitize-merge (introduced in #120511) duplicates the functionality of -ubsan-unique-traps but also allows individual checks to be specified e.g., * "-fno-sanitize-merge" without arguments is equivalent to -ubsan-unique-traps * "-fno-sanitize-merge=bool,enum" will apply it only to those two checks Additionally, the naming is more consistent with the rest of the -fsanitize- family. This patch therefore removes -ubsan-unique-traps. This breaks backwards compatibility; we hope that this is acceptable since '-mllvm -ubsan-unique-traps' was an experimental flag. This patch also adds negative test examples to bounds-checking.c, and strengthens the NOOPTARRAY assertion to prevent spurious matches. "-bounds-checking-unique-traps" is unaffected by this patch.

…e=local-bounds) -fno-sanitize-merge (introduced in llvm#120511) combines the functionality of -ubsan-unique-traps and -bounds-checking-unique-traps, while allowing fine-grained control of which UBSan checks to prevent merging. llvm#120613 removed -ubsan-unique-traps. This patch removes -bound-checking-unique-traps, which can be controlled via -fno-sanitize-merge=local-bounds. Note: this patch subtly changes -fsanitize-merge (the default) to also include -fsanitize-merge=local-bounds. This is different from the previous behavior, where -fsanitize-merge (or the old -ubsan-unique-traps) did not affect local-bounds (requiring the separate -bounds-checking-unique-traps). However, we argue that the new behavior is more intuitive. Removing -bounds-checking-unique-traps and merging its functionality into -fsanitize-merge breaks backwards compatibility; we hope that this is acceptable since '-mllvm -bounds-checking-unique-traps' was an experimental flag.

…e=local-bounds) (#120682) #120613 removed -ubsan-unique-traps and replaced it with -fno-sanitize-merge (introduced in #120511), which allows fine-grained control of which UBSan checks to prevent merging. This analogous patch removes -bound-checking-unique-traps, and allows it to be controlled via -fno-sanitize-merge=local-bounds. Most of this patch is simply plumbing through the compiler flags into the bounds checking pass. Note: this patch subtly changes -fsanitize-merge (the default) to also include -fsanitize-merge=local-bounds. This is different from the previous behavior, where -fsanitize-merge (or the old -ubsan-unique-traps) did not affect local-bounds (requiring the separate -bounds-checking-unique-traps). However, we argue that the new behavior is more intuitive. Removing -bounds-checking-unique-traps and merging its functionality into -fsanitize-merge breaks backwards compatibility; we hope that this is acceptable since '-mllvm -bounds-checking-unique-traps' was an experimental flag.

…tize-merge) (#120613) -fno-sanitize-merge (introduced in llvm/llvm-project#120511) duplicates the functionality of -ubsan-unique-traps but also allows individual checks to be specified e.g., * "-fno-sanitize-merge" without arguments is equivalent to -ubsan-unique-traps * "-fno-sanitize-merge=bool,enum" will apply it only to those two checks Additionally, the naming is more consistent with the rest of the -fsanitize- family. This patch therefore removes -ubsan-unique-traps. This breaks backwards compatibility; we hope that this is acceptable since '-mllvm -ubsan-unique-traps' was an experimental flag. This patch also adds negative test examples to bounds-checking.c, and strengthens the NOOPTARRAY assertion to prevent spurious matches. "-bounds-checking-unique-traps" is unaffected by this patch.

thurstond requested review from kstoimenov and vitalybuka December 19, 2024 01:40

Remove unnecessary header line

5f29873

llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen IR generation bugs: mangling, exceptions, etc. labels Dec 19, 2024

thurstond changed the title ~~Reapply "[ubsan] Add -fsanitize-merge (and -fno-sanitize-merge) (#120…~~ Reapply "[ubsan] Add -fsanitize-merge (and -fno-sanitize-merge) (#120…464)" Dec 19, 2024

vitalybuka reviewed Dec 19, 2024

View reviewed changes

vitalybuka approved these changes Dec 19, 2024

View reviewed changes

thurstond merged commit ffff7bb into llvm:main Dec 19, 2024

thurstond mentioned this pull request Dec 19, 2024

[ubsan] Remove -ubsan-unique-traps (replace with -fno-sanitize-merge) #120613

Merged

thurstond mentioned this pull request Dec 20, 2024

Remove -bounds-checking-unique-traps (replace with -fno-sanitize-merge=local-bounds) #120682

Merged

This was referenced Jun 2, 2025

[MTE] [NFC] use vector to collect globals to tag (#120283) #142329

Closed

[MTE] [NFC] use vector to collect globals to tag (#120283) #142330

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reapply "[ubsan] Add -fsanitize-merge (and -fno-sanitize-merge) (#120…464)"#120511

Reapply "[ubsan] Add -fsanitize-merge (and -fno-sanitize-merge) (#120…464)"#120511
thurstond merged 2 commits intollvm:mainfrom
thurstond:reland_sanitize_merge

thurstond commented Dec 19, 2024 •

edited

Loading

Uh oh!

llvmbot commented Dec 19, 2024 •

edited

Loading

Uh oh!

llvmbot commented Dec 19, 2024

Uh oh!

vitalybuka Dec 19, 2024

Uh oh!

vitalybuka Dec 19, 2024

Uh oh!

thurstond Dec 19, 2024

Uh oh!

thurstond Dec 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

thurstond commented Dec 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Dec 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Dec 19, 2024

Uh oh!

vitalybuka Dec 19, 2024

Choose a reason for hiding this comment

Uh oh!

vitalybuka Dec 19, 2024

Choose a reason for hiding this comment

Uh oh!

thurstond Dec 19, 2024

Choose a reason for hiding this comment

Uh oh!

thurstond Dec 19, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thurstond commented Dec 19, 2024 •

edited

Loading

llvmbot commented Dec 19, 2024 •

edited

Loading