[clang] [ubsan] Add -fsanitize-nonmerged-handlers (and -fno-sanitize-nonmerge… (PR #120464)

Thurston Dang via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 18 10:42:44 PST 2024


https://github.com/thurstond created https://github.com/llvm/llvm-project/pull/120464

…d-handlers)

'-mllvm -ubsan-unique-traps' (https://github.com/llvm/llvm-project/pull/65972) applies to all UBSan checks. This patch introduces -fsanitize-nonmerged-handlers and -fno-sanitize-nonmerged-handlers, which allows selectively applying non-merged handlers to a subset of UBSan checks.

N.B. we use "non-merged handlers" instead of "unique traps", since https://github.com/llvm/llvm-project/pull/119302 has generalized it to work for non-trap mode as well (min-rt and regular rt).

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

>From 2c0da9aa6f58900387fa91cdc6bcb41e0235d94c Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Wed, 18 Dec 2024 18:37:11 +0000
Subject: [PATCH] [ubsan] Add -fsanitize-nonmerged-handlers (and
 -fno-sanitize-nonmerged-handlers)

'-mllvm -ubsan-unique-traps' (https://github.com/llvm/llvm-project/pull/65972) applies to all UBSan
checks. This patch introduces -fsanitize-nonmerged-handlers and
-fno-sanitize-nonmerged-handlers, which allows selectively applying
non-merged handlers to a subset of UBSan checks.

N.B. we use "non-merged handlers" instead of "unique traps", since
https://github.com/llvm/llvm-project/pull/119302 has generalized it to
work for non-trap mode as well (min-rt and regular rt).

This patch does not remove the -ubsan-unique-traps flag; that will
override -f(no-)sanitize-non-merged-handlers.
---
 clang/include/clang/Basic/CodeGenOptions.h |  4 ++++
 clang/include/clang/Driver/Options.td      |  4 ++++
 clang/include/clang/Driver/SanitizerArgs.h |  1 +
 clang/lib/CodeGen/CGExpr.cpp               | 26 +++++++++++++---------
 clang/lib/CodeGen/CodeGenFunction.h        |  2 +-
 clang/lib/Driver/SanitizerArgs.cpp         | 24 +++++++++++++++++++-
 clang/lib/Frontend/CompilerInvocation.cpp  |  6 +++++
 7 files changed, 55 insertions(+), 12 deletions(-)

diff --git a/clang/include/clang/Basic/CodeGenOptions.h b/clang/include/clang/Basic/CodeGenOptions.h
index 2dcf98b465661e..9b97adce42cc2a 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 have non-merged handlers (better
+  /// debuggability at the expense of code size).
+  SanitizerSet SanitizeNonMergedHandlers;
+
   /// 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 7b544d2534d469..1a09f08890edad 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2548,6 +2548,10 @@ def fsanitize_trap_EQ : CommaJoined<["-"], "fsanitize-trap=">, Group<f_clang_Gro
 def fno_sanitize_trap_EQ : CommaJoined<["-"], "fno-sanitize-trap=">, Group<f_clang_Group>,
                            Visibility<[ClangOption, CLOption]>,
                            HelpText<"Disable trapping for specified sanitizers">;
+def fsanitize_nonmerged_handlers_EQ : CommaJoined<["-"], "fsanitize-nonmerged-handlers=">, Group<f_clang_Group>,
+                        HelpText<"Enable non-merged handlers for specified sanitizers">;
+def fno_sanitize_nonmerged_handlers_EQ : CommaJoined<["-"], "fno-sanitize-nonmerged-handlers=">, Group<f_clang_Group>,
+                        HelpText<"Disable non-merged handlers for specified sanitizers">;
 def fsanitize_trap : Flag<["-"], "fsanitize-trap">, Group<f_clang_Group>,
                      Alias<fsanitize_trap_EQ>, AliasArgs<["all"]>,
                      HelpText<"Enable trapping for all sanitizers">;
diff --git a/clang/include/clang/Driver/SanitizerArgs.h b/clang/include/clang/Driver/SanitizerArgs.h
index 4f08ea2b260179..28cfe72d6a34bd 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 NonMergedHandlers;
 
   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..9c4dfaa393966b 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,7 +3581,7 @@ static void emitCheckHandlerCall(CodeGenFunction &CGF,
                                llvm::AttributeList::FunctionIndex, B),
       /*Local=*/true);
   llvm::CallInst *HandlerCall = CGF.EmitNounwindRuntimeCall(Fn, FnArgs);
-  bool NoMerge =
+  NoMerge = NoMerge ||
       ClSanitizeDebugDeoptimization ||
       !CGF.CGM.getCodeGenOpts().OptimizationLevel ||
       (CGF.CurCodeDecl && CGF.CurCodeDecl->hasAttr<OptimizeNoneAttr>());
@@ -3608,6 +3608,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 +3619,9 @@ void CodeGenFunction::EmitCheck(
                   ? RecoverableCond
                   : FatalCond;
     Cond = Cond ? Builder.CreateAnd(Cond, Check) : Check;
+
+    if (CGM.getCodeGenOpts().SanitizeNonMergedHandlers.has(Checked[i].second))
+        NoMerge = true;
   }
 
   if (ClSanitizeGuardChecks) {
@@ -3632,7 +3636,7 @@ void CodeGenFunction::EmitCheck(
   }
 
   if (TrapCond)
-    EmitTrapCheck(TrapCond, CheckHandler);
+    EmitTrapCheck(TrapCond, CheckHandler, NoMerge);
   if (!FatalCond && !RecoverableCond)
     return;
 
@@ -3698,7 +3702,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 +3712,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 +3905,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 +3916,10 @@ 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 092d55355a0a17..c8e17b943c35f0 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -5166,7 +5166,7 @@ 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 81f94f23873661..6cd8267aaef1f5 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -68,6 +68,9 @@ static const SanitizerMask TrappingSupported =
     SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
     SanitizerKind::LocalBounds | SanitizerKind::CFI |
     SanitizerKind::FloatDivideByZero | SanitizerKind::ObjCCast;
+static const SanitizerMask NonMergedDefault;
+static const SanitizerMask NonMergedSupported =
+    (SanitizerKind::Undefined & ~SanitizerKind::Vptr);
 static const SanitizerMask TrappingDefault = SanitizerKind::CFI;
 static const SanitizerMask CFIClasses =
     SanitizerKind::CFIVCall | SanitizerKind::CFINVCall |
@@ -696,6 +699,17 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
   TrappingKinds &= Kinds;
   RecoverableKinds &= ~TrappingKinds;
 
+  // Parse -f(no-)?sanitize-nonmerged-handlers flags
+  SanitizerMask AlwaysNonMerged; // Empty
+  SanitizerMask NeverNonMerged = ~(setGroupBits(NonMergedSupported));
+  SanitizerMask NonMergedKinds = parseSanitizeArgs(
+      D, Args, DiagnoseErrors, NonMergedDefault, AlwaysNonMerged,
+      NeverNonMerged, options::OPT_fsanitize_nonmerged_handlers_EQ,
+      options::OPT_fno_sanitize_nonmerged_handlers_EQ);
+  RecoverableKinds |= AlwaysNonMerged;
+  RecoverableKinds &= ~NeverNonMerged;
+  RecoverableKinds &= Kinds;
+
   // Setup ignorelist files.
   // Add default ignorelist from resource directory for activated sanitizers,
   // and validate special case lists format.
@@ -1113,6 +1127,8 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
   TrapSanitizers.Mask |= TrappingKinds;
   assert(!(RecoverableKinds & TrappingKinds) &&
          "Overlap between recoverable and trapping sanitizers");
+
+  NonMergedHandlers.Mask |= NonMergedKinds;
 }
 
 static std::string toString(const clang::SanitizerSet &Sanitizers) {
@@ -1274,6 +1290,10 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
     CmdArgs.push_back(
         Args.MakeArgString("-fsanitize-trap=" + toString(TrapSanitizers)));
 
+  if (!NonMergedHandlers.empty())
+    CmdArgs.push_back(Args.MakeArgString("-fsanitize-nonmerged-handlers=" +
+                                         toString(NonMergedHandlers)));
+
   addSpecialCaseListOpt(Args, CmdArgs,
                         "-fsanitize-ignorelist=", UserIgnorelistFiles);
   addSpecialCaseListOpt(Args, CmdArgs,
@@ -1446,7 +1466,9 @@ SanitizerMask parseArgValues(const Driver &D, const llvm::opt::Arg *A,
           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_fno_sanitize_trap_EQ) ||
+          A->getOption().matches(options::OPT_fsanitize_nonmerged_handlers_EQ) ||
+          A->getOption().matches(options::OPT_fno_sanitize_nonmerged_handlers_EQ)) &&
          "Invalid argument in parseArgValues!");
   SanitizerMask Kinds;
   for (int i = 0, n = A->getNumValues(); i != n; ++i) {
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 298fafc21588a1..fbd736822ca085 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -1792,6 +1792,9 @@ void CompilerInvocationBase::GenerateCodeGenArgs(const CodeGenOptions &Opts,
   for (StringRef Sanitizer : serializeSanitizerKinds(Opts.SanitizeTrap))
     GenerateArg(Consumer, OPT_fsanitize_trap_EQ, Sanitizer);
 
+  for (StringRef Sanitizer : serializeSanitizerKinds(Opts.SanitizeNonMergedHandlers))
+    GenerateArg(Consumer, OPT_fsanitize_nonmerged_handlers_EQ, Sanitizer);
+
   if (!Opts.EmitVersionIdentMetadata)
     GenerateArg(Consumer, OPT_Qn);
 
@@ -2269,6 +2272,9 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args,
   parseSanitizerKinds("-fsanitize-trap=",
                       Args.getAllArgValues(OPT_fsanitize_trap_EQ), Diags,
                       Opts.SanitizeTrap);
+  parseSanitizerKinds("-fsanitize-nonmerged-handlers=",
+                      Args.getAllArgValues(OPT_fsanitize_nonmerged_handlers_EQ), Diags,
+                      Opts.SanitizeNonMergedHandlers);
 
   Opts.EmitVersionIdentMetadata = Args.hasFlag(OPT_Qy, OPT_Qn, true);
 



More information about the cfe-commits mailing list