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

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.

Author: thurstond

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;
 

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"]>;

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;

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();

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.

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.
@@ -1113,6 +1121,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) {
@@ -1274,6 +1284,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,
@@ -1441,13 +1455,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);

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);
 

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) {