Skip to content

Conversation

@thurstond
Copy link
Contributor

@thurstond thurstond commented Jan 4, 2025

This adds a function to parse weighted sanitizer flags (e.g., -fsanitize-blah=undefined=0.5,null=0.3) and adds the plumbing to apply that to a new flag, -fsanitize-skip-hot-cutoff.

-fsanitize-skip-hot-cutoff currently has no effect; future work will use it to generalize ubsan-guard-checks (originally introduced in 5f9ed2f).

This adds a function to parse weighted sanitizer flags (e.g.,
-fsanitize-blah=undefined=0.5,null=0.3) and adds the plumbing to
apply that to -fno-sanitize-top-hot from the frontend to backend.

-fno-sanitize-top-hot currently has no effect; future work will
use it to generalize ubsan-guard-checks (originaly introduced in 5f9ed2f).
@github-actions
Copy link

github-actions bot commented Jan 4, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@thurstond thurstond marked this pull request as ready for review January 6, 2025 23:59
@thurstond thurstond requested a review from vitalybuka January 7, 2025 00:00
@llvmbot 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" labels Jan 7, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 7, 2025

@llvm/pr-subscribers-clang-driver

Author: Thurston Dang (thurstond)

Changes

This adds a function to parse weighted sanitizer flags (e.g., -fsanitize-blah=undefined=0.5,null=0.3) and adds the plumbing to apply that to a new flag, -fno-sanitize-top-hot, from the frontend to backend.

-fno-sanitize-top-hot currently has no effect; future work will use it to generalize ubsan-guard-checks (originaly introduced in 5f9ed2f).


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

9 Files Affected:

  • (modified) clang/include/clang/Basic/CodeGenOptions.h (+8)
  • (modified) clang/include/clang/Basic/Sanitizers.h (+16)
  • (modified) clang/include/clang/Driver/Options.td (+7)
  • (modified) clang/include/clang/Driver/SanitizerArgs.h (+2)
  • (modified) clang/lib/Basic/Sanitizers.cpp (+40)
  • (modified) clang/lib/Driver/SanitizerArgs.cpp (+72-23)
  • (modified) clang/lib/Frontend/CompilerInvocation.cpp (+24)
  • (modified) clang/test/CodeGen/allow-ubsan-check.c (-1)
  • (modified) clang/test/Driver/fsanitize.c (+19)
diff --git a/clang/include/clang/Basic/CodeGenOptions.h b/clang/include/clang/Basic/CodeGenOptions.h
index 8097c9ef772bc7..227bddf831002e 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -384,6 +384,14 @@ class CodeGenOptions : public CodeGenOptionsBase {
   /// the expense of debuggability).
   SanitizerSet SanitizeMergeHandlers;
 
+  /// Set of thresholds: the top hottest code responsible for the given
+  /// fraction of PGO counters will be excluded from sanitization
+  /// (0.0 [default] = skip none, 1.0 = skip all).
+  SanitizerSet NoSanitizeTopHot;
+  /// N.B. The cutoffs contain strictly more information than the SanitizerSet,
+  /// but the SanitizerSet is more efficient for some calculations.
+  SanitizerMaskCutoffs NoSanitizeTopHotCutoffs = {0};
+
   /// List of backend command-line options for -fembed-bitcode.
   std::vector<uint8_t> CmdArgs;
 
diff --git a/clang/include/clang/Basic/Sanitizers.h b/clang/include/clang/Basic/Sanitizers.h
index c890242269b334..12c2c93a7f89f6 100644
--- a/clang/include/clang/Basic/Sanitizers.h
+++ b/clang/include/clang/Basic/Sanitizers.h
@@ -154,6 +154,8 @@ struct SanitizerKind {
 #include "clang/Basic/Sanitizers.def"
 }; // SanitizerKind
 
+using SanitizerMaskCutoffs = std::array<float, SanitizerKind::SO_Count>;
+
 struct SanitizerSet {
   /// Check if a certain (single) sanitizer is enabled.
   bool has(SanitizerMask K) const {
@@ -186,10 +188,24 @@ struct SanitizerSet {
 /// Returns a non-zero SanitizerMask, or \c 0 if \p Value is not known.
 SanitizerMask parseSanitizerValue(StringRef Value, bool AllowGroups);
 
+/// Parse a single weighted value (e.g., 'undefined=0.05') from a -fsanitize= or
+/// -fno-sanitize= value list.
+/// Returns a non-zero SanitizerMask, or \c 0 if \p Value is not known.
+/// The relevant weight(s) are updated in the passed array.
+/// Individual Cutoffs are never reset to zero unless explicitly set
+/// (e.g., 'null=0.0').
+SanitizerMask parseSanitizerWeightedValue(StringRef Value, bool AllowGroups,
+                                          SanitizerMaskCutoffs *Cutoffs);
+
 /// Serialize a SanitizerSet into values for -fsanitize= or -fno-sanitize=.
 void serializeSanitizerSet(SanitizerSet Set,
                            SmallVectorImpl<StringRef> &Values);
 
+/// Serialize a SanitizerMaskCutoffs into values for -fsanitize= or
+/// -fno-sanitize=.
+void serializeSanitizerMaskCutoffs(const SanitizerMaskCutoffs Cutoffs,
+                                   SmallVectorImpl<std::string> &Values);
+
 /// For each sanitizer group bit set in \p Kinds, set the bits for sanitizers
 /// this group enables.
 SanitizerMask expandSanitizerGroups(SanitizerMask Kinds);
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index d922709db17786..027093157d4c73 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2649,6 +2649,13 @@ def fsanitize_undefined_strip_path_components_EQ : Joined<["-"], "fsanitize-unde
   HelpText<"Strip (or keep only, if negative) a given number of path components "
            "when emitting check metadata.">,
   MarshallingInfoInt<CodeGenOpts<"EmitCheckPathComponentsToStrip">, "0", "int">;
+def fno_sanitize_top_hot_EQ
+    : CommaJoined<["-"], "fno-sanitize-top-hot=">,
+      Group<f_clang_Group>,
+      HelpText<"Exclude sanitization for the top hottest code responsible for "
+               "the given fraction of PGO counters "
+               "(0.0 [default] = skip none; 1.0 = skip all). "
+               "Argument format: <sanitizer1>=,<sanitizer2>=,...">;
 
 } // end -f[no-]sanitize* flags
 
diff --git a/clang/include/clang/Driver/SanitizerArgs.h b/clang/include/clang/Driver/SanitizerArgs.h
index 3b275092bbbe86..2462228f533746 100644
--- a/clang/include/clang/Driver/SanitizerArgs.h
+++ b/clang/include/clang/Driver/SanitizerArgs.h
@@ -26,6 +26,8 @@ class SanitizerArgs {
   SanitizerSet RecoverableSanitizers;
   SanitizerSet TrapSanitizers;
   SanitizerSet MergeHandlers;
+  SanitizerSet TopHot;
+  SanitizerMaskCutoffs TopHotCutoffs = {0};
 
   std::vector<std::string> UserIgnorelistFiles;
   std::vector<std::string> SystemIgnorelistFiles;
diff --git a/clang/lib/Basic/Sanitizers.cpp b/clang/lib/Basic/Sanitizers.cpp
index 62ccdf8e9bbf28..6711b05c4539dd 100644
--- a/clang/lib/Basic/Sanitizers.cpp
+++ b/clang/lib/Basic/Sanitizers.cpp
@@ -36,6 +36,37 @@ SanitizerMask clang::parseSanitizerValue(StringRef Value, bool AllowGroups) {
   return ParsedKind;
 }
 
+SanitizerMask
+clang::parseSanitizerWeightedValue(StringRef Value, bool AllowGroups,
+                                   SanitizerMaskCutoffs *Cutoffs) {
+  SanitizerMask ParsedKind = llvm::StringSwitch<SanitizerMask>(Value)
+#define SANITIZER(NAME, ID) .StartsWith(NAME "=", SanitizerKind::ID)
+#define SANITIZER_GROUP(NAME, ID, ALIAS)                                       \
+  .StartsWith(NAME "=",                                                        \
+              AllowGroups ? SanitizerKind::ID##Group : SanitizerMask())
+#include "clang/Basic/Sanitizers.def"
+                                 .Default(SanitizerMask());
+
+  if (ParsedKind && Cutoffs) {
+    size_t equalsIndex = Value.find_first_of('=');
+    if (equalsIndex != llvm::StringLiteral::npos) {
+      double arg;
+      if ((Value.size() > (equalsIndex + 1)) &&
+          !Value.substr(equalsIndex + 1).getAsDouble(arg)) {
+        // AllowGroups is already taken into account for ParsedKind,
+        // hence we unconditionally expandSanitizerGroups.
+        SanitizerMask ExpandedKind = expandSanitizerGroups(ParsedKind);
+
+        for (unsigned int i = 0; i < SanitizerKind::SO_Count; i++)
+          if (ExpandedKind & SanitizerMask::bitPosToMask(i))
+            (*Cutoffs)[i] = arg;
+      }
+    }
+  }
+
+  return ParsedKind;
+}
+
 void clang::serializeSanitizerSet(SanitizerSet Set,
                                   SmallVectorImpl<StringRef> &Values) {
 #define SANITIZER(NAME, ID)                                                    \
@@ -44,6 +75,15 @@ void clang::serializeSanitizerSet(SanitizerSet Set,
 #include "clang/Basic/Sanitizers.def"
 }
 
+void clang::serializeSanitizerMaskCutoffs(
+    const SanitizerMaskCutoffs Cutoffs, SmallVectorImpl<std::string> &Values) {
+#define SANITIZER(NAME, ID)                                                    \
+  if (Cutoffs[SanitizerKind::SO_##ID])                                         \
+    Values.push_back(std::string(NAME "=") +                                   \
+                     std::to_string(Cutoffs[SanitizerKind::SO_##ID]));
+#include "clang/Basic/Sanitizers.def"
+}
+
 SanitizerMask clang::expandSanitizerGroups(SanitizerMask Kinds) {
 #define SANITIZER(NAME, ID)
 #define SANITIZER_GROUP(NAME, ID, ALIAS)                                       \
diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index 98116e2c8336b8..f7db3e5032ce1a 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -111,7 +111,8 @@ enum BinaryMetadataFeature {
 /// Parse a -fsanitize= or -fno-sanitize= argument's values, diagnosing any
 /// invalid components. Returns a SanitizerMask.
 static SanitizerMask parseArgValues(const Driver &D, const llvm::opt::Arg *A,
-                                    bool DiagnoseErrors);
+                                    bool DiagnoseErrors,
+                                    SanitizerMaskCutoffs *Cutoffs);
 
 /// Parse -f(no-)?sanitize-coverage= flag values, diagnosing any invalid
 /// components. Returns OR of members of \c CoverageFeature enumeration.
@@ -260,7 +261,7 @@ static SanitizerMask
 parseSanitizeArgs(const Driver &D, const llvm::opt::ArgList &Args,
                   bool DiagnoseErrors, SanitizerMask Default,
                   SanitizerMask AlwaysIn, SanitizerMask AlwaysOut, int OptInID,
-                  int OptOutID) {
+                  int OptOutID, SanitizerMaskCutoffs *Cutoffs) {
   assert(!(AlwaysIn & AlwaysOut) &&
          "parseSanitizeArgs called with contradictory in/out requirements");
 
@@ -271,7 +272,7 @@ parseSanitizeArgs(const Driver &D, const llvm::opt::ArgList &Args,
   SanitizerMask DiagnosedAlwaysOutViolations;
   for (const auto *Arg : Args) {
     if (Arg->getOption().matches(OptInID)) {
-      SanitizerMask Add = parseArgValues(D, Arg, DiagnoseErrors);
+      SanitizerMask Add = parseArgValues(D, Arg, DiagnoseErrors, Cutoffs);
       // Report error if user explicitly tries to opt-in to an always-out
       // sanitizer.
       if (SanitizerMask KindsToDiagnose =
@@ -287,7 +288,7 @@ parseSanitizeArgs(const Driver &D, const llvm::opt::ArgList &Args,
       Output |= expandSanitizerGroups(Add);
       Arg->claim();
     } else if (Arg->getOption().matches(OptOutID)) {
-      SanitizerMask Remove = parseArgValues(D, Arg, DiagnoseErrors);
+      SanitizerMask Remove = parseArgValues(D, Arg, DiagnoseErrors, Cutoffs);
       // Report error if user explicitly tries to opt-out of an always-in
       // sanitizer.
       if (SanitizerMask KindsToDiagnose =
@@ -320,7 +321,15 @@ static SanitizerMask parseSanitizeTrapArgs(const Driver &D,
   // (not even in recover mode) in order to avoid the need for a ubsan runtime.
   return parseSanitizeArgs(D, Args, DiagnoseErrors, TrappingDefault, AlwaysTrap,
                            NeverTrap, options::OPT_fsanitize_trap_EQ,
-                           options::OPT_fno_sanitize_trap_EQ);
+                           options::OPT_fno_sanitize_trap_EQ, nullptr);
+}
+
+static SanitizerMask parseNoSanitizeHotArgs(const Driver &D,
+                                            const llvm::opt::ArgList &Args,
+                                            bool DiagnoseErrors,
+                                            SanitizerMaskCutoffs *Cutoffs) {
+  return parseSanitizeArgs(D, Args, DiagnoseErrors, {}, {}, {},
+                           options::OPT_fno_sanitize_top_hot_EQ, -1, Cutoffs);
 }
 
 bool SanitizerArgs::needsFuzzerInterceptors() const {
@@ -403,7 +412,7 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
   for (const llvm::opt::Arg *Arg : llvm::reverse(Args)) {
     if (Arg->getOption().matches(options::OPT_fsanitize_EQ)) {
       Arg->claim();
-      SanitizerMask Add = parseArgValues(D, Arg, DiagnoseErrors);
+      SanitizerMask Add = parseArgValues(D, Arg, DiagnoseErrors, nullptr);
 
       if (RemoveObjectSizeAtO0) {
         AllRemove |= SanitizerKind::ObjectSize;
@@ -573,7 +582,7 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
       Kinds |= Add;
     } else if (Arg->getOption().matches(options::OPT_fno_sanitize_EQ)) {
       Arg->claim();
-      SanitizerMask Remove = parseArgValues(D, Arg, DiagnoseErrors);
+      SanitizerMask Remove = parseArgValues(D, Arg, DiagnoseErrors, nullptr);
       AllRemove |= expandSanitizerGroups(Remove);
     }
   }
@@ -698,7 +707,7 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
   SanitizerMask RecoverableKinds = parseSanitizeArgs(
       D, Args, DiagnoseErrors, RecoverableByDefault, AlwaysRecoverable,
       Unrecoverable, options::OPT_fsanitize_recover_EQ,
-      options::OPT_fno_sanitize_recover_EQ);
+      options::OPT_fno_sanitize_recover_EQ, nullptr);
   RecoverableKinds |= AlwaysRecoverable;
   RecoverableKinds &= ~Unrecoverable;
   RecoverableKinds &= Kinds;
@@ -710,9 +719,14 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
   SanitizerMask MergeKinds =
       parseSanitizeArgs(D, Args, DiagnoseErrors, MergeDefault, {}, {},
                         options::OPT_fsanitize_merge_handlers_EQ,
-                        options::OPT_fno_sanitize_merge_handlers_EQ);
+                        options::OPT_fno_sanitize_merge_handlers_EQ, nullptr);
   MergeKinds &= Kinds;
 
+  // Parse -fno-sanitize-top-hot flags
+  SanitizerMask TopHotMask =
+      parseNoSanitizeHotArgs(D, Args, DiagnoseErrors, &TopHotCutoffs);
+  (void)TopHotMask;
+
   // Setup ignorelist files.
   // Add default ignorelist from resource directory for activated sanitizers,
   // and validate special case lists format.
@@ -1132,6 +1146,15 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
          "Overlap between recoverable and trapping sanitizers");
 
   MergeHandlers.Mask |= MergeKinds;
+
+  TopHotMask &= Sanitizers.Mask;
+  TopHot.Mask = TopHotMask;
+
+  // Zero out TopHot for unused sanitizers
+  for (unsigned int i = 0; i < SanitizerKind::SO_Count; i++) {
+    if (!(Sanitizers.Mask & SanitizerMask::bitPosToMask(i)))
+      TopHotCutoffs[i] = 0;
+  }
 }
 
 static std::string toString(const clang::SanitizerSet &Sanitizers) {
@@ -1146,6 +1169,19 @@ static std::string toString(const clang::SanitizerSet &Sanitizers) {
   return Res;
 }
 
+static std::string toString(const clang::SanitizerMaskCutoffs &Cutoffs) {
+  std::string Res;
+#define SANITIZER(NAME, ID)                                                    \
+  if (Cutoffs[SanitizerKind::SO_##ID]) {                                       \
+    if (!Res.empty())                                                          \
+      Res += ",";                                                              \
+    Res += std::string(NAME) + "=" +                                           \
+           std::to_string(Cutoffs[SanitizerKind::SO_##ID]);                    \
+  }
+#include "clang/Basic/Sanitizers.def"
+  return Res;
+}
+
 static void addSpecialCaseListOpt(const llvm::opt::ArgList &Args,
                                   llvm::opt::ArgStringList &CmdArgs,
                                   const char *SCLOptFlag,
@@ -1297,6 +1333,11 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
     CmdArgs.push_back(
         Args.MakeArgString("-fsanitize-merge=" + toString(MergeHandlers)));
 
+  std::string TopHotCutoffsStr = toString(TopHotCutoffs);
+  if (TopHotCutoffsStr != "")
+    CmdArgs.push_back(
+        Args.MakeArgString("-fno-sanitize-top-hot=" + TopHotCutoffsStr));
+
   addSpecialCaseListOpt(Args, CmdArgs,
                         "-fsanitize-ignorelist=", UserIgnorelistFiles);
   addSpecialCaseListOpt(Args, CmdArgs,
@@ -1463,17 +1504,18 @@ 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) ||
-       A->getOption().matches(options::OPT_fsanitize_merge_handlers_EQ) ||
-       A->getOption().matches(options::OPT_fno_sanitize_merge_handlers_EQ)) &&
-      "Invalid argument in parseArgValues!");
+                             bool DiagnoseErrors,
+                             SanitizerMaskCutoffs *Cutoffs) {
+  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) ||
+          A->getOption().matches(options::OPT_fno_sanitize_top_hot_EQ)) &&
+         "Invalid argument in parseArgValues!");
   SanitizerMask Kinds;
   for (int i = 0, n = A->getNumValues(); i != n; ++i) {
     const char *Value = A->getValue(i);
@@ -1482,8 +1524,15 @@ SanitizerMask parseArgValues(const Driver &D, const llvm::opt::Arg *A,
     if (A->getOption().matches(options::OPT_fsanitize_EQ) &&
         0 == strcmp("all", Value))
       Kind = SanitizerMask();
-    else
+    else if (A->getOption().matches(options::OPT_fno_sanitize_top_hot_EQ)) {
+      assert(
+          Cutoffs &&
+          "Null Cutoffs parameter provided for parsing fno_sanitize_top_hot!");
+      Kind = parseSanitizerWeightedValue(Value, /*AllowGroups=*/true, Cutoffs);
+    } else {
+      assert((!Cutoffs) && "Non-null Cutoffs parameter erroneously provided!");
       Kind = parseSanitizerValue(Value, /*AllowGroups=*/true);
+    }
 
     if (Kind)
       Kinds |= Kind;
@@ -1586,12 +1635,12 @@ std::string lastArgumentForMask(const Driver &D, const llvm::opt::ArgList &Args,
     const auto *Arg = *I;
     if (Arg->getOption().matches(options::OPT_fsanitize_EQ)) {
       SanitizerMask AddKinds =
-          expandSanitizerGroups(parseArgValues(D, Arg, false));
+          expandSanitizerGroups(parseArgValues(D, Arg, false, nullptr));
       if (AddKinds & Mask)
         return describeSanitizeArg(Arg, Mask);
     } else if (Arg->getOption().matches(options::OPT_fno_sanitize_EQ)) {
       SanitizerMask RemoveKinds =
-          expandSanitizerGroups(parseArgValues(D, Arg, false));
+          expandSanitizerGroups(parseArgValues(D, Arg, false, nullptr));
       Mask &= ~RemoveKinds;
     }
   }
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 348c56cc37da3f..78dd5099259f18 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -1436,6 +1436,19 @@ static SmallVector<StringRef, 4> serializeSanitizerKinds(SanitizerSet S) {
   return Values;
 }
 
+static void parseSanitizerWeightedKinds(
+    StringRef FlagName, const std::vector<std::string> &Sanitizers,
+    DiagnosticsEngine &Diags, SanitizerSet &S, SanitizerMaskCutoffs *Cutoffs) {
+  for (const auto &Sanitizer : Sanitizers) {
+    SanitizerMask K =
+        parseSanitizerWeightedValue(Sanitizer, /*AllowGroups=*/false, Cutoffs);
+    if (K == SanitizerMask())
+      Diags.Report(diag::err_drv_invalid_value) << FlagName << Sanitizer;
+    else
+      S.set(K, true);
+  }
+}
+
 static void parseXRayInstrumentationBundle(StringRef FlagName, StringRef Bundle,
                                            ArgList &Args, DiagnosticsEngine &D,
                                            XRayInstrSet &S) {
@@ -1796,6 +1809,11 @@ void CompilerInvocationBase::GenerateCodeGenArgs(const CodeGenOptions &Opts,
        serializeSanitizerKinds(Opts.SanitizeMergeHandlers))
     GenerateArg(Consumer, OPT_fsanitize_merge_handlers_EQ, Sanitizer);
 
+  SmallVector<std::string, 4> Values;
+  serializeSanitizerMaskCutoffs(Opts.NoSanitizeTopHotCutoffs, Values);
+  for (std::string Sanitizer : Values)
+    GenerateArg(Consumer, OPT_fno_sanitize_top_hot_EQ, Sanitizer);
+
   if (!Opts.EmitVersionIdentMetadata)
     GenerateArg(Consumer, OPT_Qn);
 
@@ -2277,6 +2295,12 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args,
                       Args.getAllArgValues(OPT_fsanitize_merge_handlers_EQ),
                       Diags, Opts.SanitizeMergeHandlers);
 
+  // Parse -fno-sanitize-top-hot= arguments.
+  parseSanitizerWeightedKinds("-fno-sanitize-top-hot=",
+                              Args.getAllArgValues(OPT_fno_sanitize_top_hot_EQ),
+                              Diags, Opts.NoSanitizeTopHot,
+                              &Opts.NoSanitizeTopHotCutoffs);
+
   Opts.EmitVersionIdentMetadata = Args.hasFlag(OPT_Qy, OPT_Qn, true);
 
   if (!LangOpts->CUDAIsDevice)
diff --git a/clang/test/CodeGen/allow-ubsan-check.c b/clang/test/CodeGen/allow-ubsan-check.c
index 5232d240854666..1c76049b57bda8 100644
--- a/clang/test/CodeGen/allow-ubsan-check.c
+++ b/clang/test/CodeGen/allow-ubsan-check.c
@@ -3,7 +3,6 @@
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -o - %s -fsanitize=signed-integer-overflow,integer-divide-by-zero,null -mllvm -ubsan-guard-checks -fsanitize-trap=signed-integer-overflow,integer-divide-by-zero,null | FileCheck %s --check-prefixes=TRAP
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -o - %s -fsanitize=signed-integer-overflow,integer-divide-by-zero,null -mllvm -ubsan-guard-checks -fsanitize-recover=signed-integer-overflow,integer-divide-by-zero,null | FileCheck %s --check-prefixes=RECOVER
 
-
 // CHECK-LABEL: define dso_local i32 @d...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Jan 7, 2025

@llvm/pr-subscribers-clang

Author: Thurston Dang (thurstond)

Changes

This adds a function to parse weighted sanitizer flags (e.g., -fsanitize-blah=undefined=0.5,null=0.3) and adds the plumbing to apply that to a new flag, -fno-sanitize-top-hot, from the frontend to backend.

-fno-sanitize-top-hot currently has no effect; future work will use it to generalize ubsan-guard-checks (originaly introduced in 5f9ed2f).


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

9 Files Affected:

  • (modified) clang/include/clang/Basic/CodeGenOptions.h (+8)
  • (modified) clang/include/clang/Basic/Sanitizers.h (+16)
  • (modified) clang/include/clang/Driver/Options.td (+7)
  • (modified) clang/include/clang/Driver/SanitizerArgs.h (+2)
  • (modified) clang/lib/Basic/Sanitizers.cpp (+40)
  • (modified) clang/lib/Driver/SanitizerArgs.cpp (+72-23)
  • (modified) clang/lib/Frontend/CompilerInvocation.cpp (+24)
  • (modified) clang/test/CodeGen/allow-ubsan-check.c (-1)
  • (modified) clang/test/Driver/fsanitize.c (+19)
diff --git a/clang/include/clang/Basic/CodeGenOptions.h b/clang/include/clang/Basic/CodeGenOptions.h
index 8097c9ef772bc7..227bddf831002e 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -384,6 +384,14 @@ class CodeGenOptions : public CodeGenOptionsBase {
   /// the expense of debuggability).
   SanitizerSet SanitizeMergeHandlers;
 
+  /// Set of thresholds: the top hottest code responsible for the given
+  /// fraction of PGO counters will be excluded from sanitization
+  /// (0.0 [default] = skip none, 1.0 = skip all).
+  SanitizerSet NoSanitizeTopHot;
+  /// N.B. The cutoffs contain strictly more information than the SanitizerSet,
+  /// but the SanitizerSet is more efficient for some calculations.
+  SanitizerMaskCutoffs NoSanitizeTopHotCutoffs = {0};
+
   /// List of backend command-line options for -fembed-bitcode.
   std::vector<uint8_t> CmdArgs;
 
diff --git a/clang/include/clang/Basic/Sanitizers.h b/clang/include/clang/Basic/Sanitizers.h
index c890242269b334..12c2c93a7f89f6 100644
--- a/clang/include/clang/Basic/Sanitizers.h
+++ b/clang/include/clang/Basic/Sanitizers.h
@@ -154,6 +154,8 @@ struct SanitizerKind {
 #include "clang/Basic/Sanitizers.def"
 }; // SanitizerKind
 
+using SanitizerMaskCutoffs = std::array<float, SanitizerKind::SO_Count>;
+
 struct SanitizerSet {
   /// Check if a certain (single) sanitizer is enabled.
   bool has(SanitizerMask K) const {
@@ -186,10 +188,24 @@ struct SanitizerSet {
 /// Returns a non-zero SanitizerMask, or \c 0 if \p Value is not known.
 SanitizerMask parseSanitizerValue(StringRef Value, bool AllowGroups);
 
+/// Parse a single weighted value (e.g., 'undefined=0.05') from a -fsanitize= or
+/// -fno-sanitize= value list.
+/// Returns a non-zero SanitizerMask, or \c 0 if \p Value is not known.
+/// The relevant weight(s) are updated in the passed array.
+/// Individual Cutoffs are never reset to zero unless explicitly set
+/// (e.g., 'null=0.0').
+SanitizerMask parseSanitizerWeightedValue(StringRef Value, bool AllowGroups,
+                                          SanitizerMaskCutoffs *Cutoffs);
+
 /// Serialize a SanitizerSet into values for -fsanitize= or -fno-sanitize=.
 void serializeSanitizerSet(SanitizerSet Set,
                            SmallVectorImpl<StringRef> &Values);
 
+/// Serialize a SanitizerMaskCutoffs into values for -fsanitize= or
+/// -fno-sanitize=.
+void serializeSanitizerMaskCutoffs(const SanitizerMaskCutoffs Cutoffs,
+                                   SmallVectorImpl<std::string> &Values);
+
 /// For each sanitizer group bit set in \p Kinds, set the bits for sanitizers
 /// this group enables.
 SanitizerMask expandSanitizerGroups(SanitizerMask Kinds);
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index d922709db17786..027093157d4c73 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2649,6 +2649,13 @@ def fsanitize_undefined_strip_path_components_EQ : Joined<["-"], "fsanitize-unde
   HelpText<"Strip (or keep only, if negative) a given number of path components "
            "when emitting check metadata.">,
   MarshallingInfoInt<CodeGenOpts<"EmitCheckPathComponentsToStrip">, "0", "int">;
+def fno_sanitize_top_hot_EQ
+    : CommaJoined<["-"], "fno-sanitize-top-hot=">,
+      Group<f_clang_Group>,
+      HelpText<"Exclude sanitization for the top hottest code responsible for "
+               "the given fraction of PGO counters "
+               "(0.0 [default] = skip none; 1.0 = skip all). "
+               "Argument format: <sanitizer1>=,<sanitizer2>=,...">;
 
 } // end -f[no-]sanitize* flags
 
diff --git a/clang/include/clang/Driver/SanitizerArgs.h b/clang/include/clang/Driver/SanitizerArgs.h
index 3b275092bbbe86..2462228f533746 100644
--- a/clang/include/clang/Driver/SanitizerArgs.h
+++ b/clang/include/clang/Driver/SanitizerArgs.h
@@ -26,6 +26,8 @@ class SanitizerArgs {
   SanitizerSet RecoverableSanitizers;
   SanitizerSet TrapSanitizers;
   SanitizerSet MergeHandlers;
+  SanitizerSet TopHot;
+  SanitizerMaskCutoffs TopHotCutoffs = {0};
 
   std::vector<std::string> UserIgnorelistFiles;
   std::vector<std::string> SystemIgnorelistFiles;
diff --git a/clang/lib/Basic/Sanitizers.cpp b/clang/lib/Basic/Sanitizers.cpp
index 62ccdf8e9bbf28..6711b05c4539dd 100644
--- a/clang/lib/Basic/Sanitizers.cpp
+++ b/clang/lib/Basic/Sanitizers.cpp
@@ -36,6 +36,37 @@ SanitizerMask clang::parseSanitizerValue(StringRef Value, bool AllowGroups) {
   return ParsedKind;
 }
 
+SanitizerMask
+clang::parseSanitizerWeightedValue(StringRef Value, bool AllowGroups,
+                                   SanitizerMaskCutoffs *Cutoffs) {
+  SanitizerMask ParsedKind = llvm::StringSwitch<SanitizerMask>(Value)
+#define SANITIZER(NAME, ID) .StartsWith(NAME "=", SanitizerKind::ID)
+#define SANITIZER_GROUP(NAME, ID, ALIAS)                                       \
+  .StartsWith(NAME "=",                                                        \
+              AllowGroups ? SanitizerKind::ID##Group : SanitizerMask())
+#include "clang/Basic/Sanitizers.def"
+                                 .Default(SanitizerMask());
+
+  if (ParsedKind && Cutoffs) {
+    size_t equalsIndex = Value.find_first_of('=');
+    if (equalsIndex != llvm::StringLiteral::npos) {
+      double arg;
+      if ((Value.size() > (equalsIndex + 1)) &&
+          !Value.substr(equalsIndex + 1).getAsDouble(arg)) {
+        // AllowGroups is already taken into account for ParsedKind,
+        // hence we unconditionally expandSanitizerGroups.
+        SanitizerMask ExpandedKind = expandSanitizerGroups(ParsedKind);
+
+        for (unsigned int i = 0; i < SanitizerKind::SO_Count; i++)
+          if (ExpandedKind & SanitizerMask::bitPosToMask(i))
+            (*Cutoffs)[i] = arg;
+      }
+    }
+  }
+
+  return ParsedKind;
+}
+
 void clang::serializeSanitizerSet(SanitizerSet Set,
                                   SmallVectorImpl<StringRef> &Values) {
 #define SANITIZER(NAME, ID)                                                    \
@@ -44,6 +75,15 @@ void clang::serializeSanitizerSet(SanitizerSet Set,
 #include "clang/Basic/Sanitizers.def"
 }
 
+void clang::serializeSanitizerMaskCutoffs(
+    const SanitizerMaskCutoffs Cutoffs, SmallVectorImpl<std::string> &Values) {
+#define SANITIZER(NAME, ID)                                                    \
+  if (Cutoffs[SanitizerKind::SO_##ID])                                         \
+    Values.push_back(std::string(NAME "=") +                                   \
+                     std::to_string(Cutoffs[SanitizerKind::SO_##ID]));
+#include "clang/Basic/Sanitizers.def"
+}
+
 SanitizerMask clang::expandSanitizerGroups(SanitizerMask Kinds) {
 #define SANITIZER(NAME, ID)
 #define SANITIZER_GROUP(NAME, ID, ALIAS)                                       \
diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index 98116e2c8336b8..f7db3e5032ce1a 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -111,7 +111,8 @@ enum BinaryMetadataFeature {
 /// Parse a -fsanitize= or -fno-sanitize= argument's values, diagnosing any
 /// invalid components. Returns a SanitizerMask.
 static SanitizerMask parseArgValues(const Driver &D, const llvm::opt::Arg *A,
-                                    bool DiagnoseErrors);
+                                    bool DiagnoseErrors,
+                                    SanitizerMaskCutoffs *Cutoffs);
 
 /// Parse -f(no-)?sanitize-coverage= flag values, diagnosing any invalid
 /// components. Returns OR of members of \c CoverageFeature enumeration.
@@ -260,7 +261,7 @@ static SanitizerMask
 parseSanitizeArgs(const Driver &D, const llvm::opt::ArgList &Args,
                   bool DiagnoseErrors, SanitizerMask Default,
                   SanitizerMask AlwaysIn, SanitizerMask AlwaysOut, int OptInID,
-                  int OptOutID) {
+                  int OptOutID, SanitizerMaskCutoffs *Cutoffs) {
   assert(!(AlwaysIn & AlwaysOut) &&
          "parseSanitizeArgs called with contradictory in/out requirements");
 
@@ -271,7 +272,7 @@ parseSanitizeArgs(const Driver &D, const llvm::opt::ArgList &Args,
   SanitizerMask DiagnosedAlwaysOutViolations;
   for (const auto *Arg : Args) {
     if (Arg->getOption().matches(OptInID)) {
-      SanitizerMask Add = parseArgValues(D, Arg, DiagnoseErrors);
+      SanitizerMask Add = parseArgValues(D, Arg, DiagnoseErrors, Cutoffs);
       // Report error if user explicitly tries to opt-in to an always-out
       // sanitizer.
       if (SanitizerMask KindsToDiagnose =
@@ -287,7 +288,7 @@ parseSanitizeArgs(const Driver &D, const llvm::opt::ArgList &Args,
       Output |= expandSanitizerGroups(Add);
       Arg->claim();
     } else if (Arg->getOption().matches(OptOutID)) {
-      SanitizerMask Remove = parseArgValues(D, Arg, DiagnoseErrors);
+      SanitizerMask Remove = parseArgValues(D, Arg, DiagnoseErrors, Cutoffs);
       // Report error if user explicitly tries to opt-out of an always-in
       // sanitizer.
       if (SanitizerMask KindsToDiagnose =
@@ -320,7 +321,15 @@ static SanitizerMask parseSanitizeTrapArgs(const Driver &D,
   // (not even in recover mode) in order to avoid the need for a ubsan runtime.
   return parseSanitizeArgs(D, Args, DiagnoseErrors, TrappingDefault, AlwaysTrap,
                            NeverTrap, options::OPT_fsanitize_trap_EQ,
-                           options::OPT_fno_sanitize_trap_EQ);
+                           options::OPT_fno_sanitize_trap_EQ, nullptr);
+}
+
+static SanitizerMask parseNoSanitizeHotArgs(const Driver &D,
+                                            const llvm::opt::ArgList &Args,
+                                            bool DiagnoseErrors,
+                                            SanitizerMaskCutoffs *Cutoffs) {
+  return parseSanitizeArgs(D, Args, DiagnoseErrors, {}, {}, {},
+                           options::OPT_fno_sanitize_top_hot_EQ, -1, Cutoffs);
 }
 
 bool SanitizerArgs::needsFuzzerInterceptors() const {
@@ -403,7 +412,7 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
   for (const llvm::opt::Arg *Arg : llvm::reverse(Args)) {
     if (Arg->getOption().matches(options::OPT_fsanitize_EQ)) {
       Arg->claim();
-      SanitizerMask Add = parseArgValues(D, Arg, DiagnoseErrors);
+      SanitizerMask Add = parseArgValues(D, Arg, DiagnoseErrors, nullptr);
 
       if (RemoveObjectSizeAtO0) {
         AllRemove |= SanitizerKind::ObjectSize;
@@ -573,7 +582,7 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
       Kinds |= Add;
     } else if (Arg->getOption().matches(options::OPT_fno_sanitize_EQ)) {
       Arg->claim();
-      SanitizerMask Remove = parseArgValues(D, Arg, DiagnoseErrors);
+      SanitizerMask Remove = parseArgValues(D, Arg, DiagnoseErrors, nullptr);
       AllRemove |= expandSanitizerGroups(Remove);
     }
   }
@@ -698,7 +707,7 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
   SanitizerMask RecoverableKinds = parseSanitizeArgs(
       D, Args, DiagnoseErrors, RecoverableByDefault, AlwaysRecoverable,
       Unrecoverable, options::OPT_fsanitize_recover_EQ,
-      options::OPT_fno_sanitize_recover_EQ);
+      options::OPT_fno_sanitize_recover_EQ, nullptr);
   RecoverableKinds |= AlwaysRecoverable;
   RecoverableKinds &= ~Unrecoverable;
   RecoverableKinds &= Kinds;
@@ -710,9 +719,14 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
   SanitizerMask MergeKinds =
       parseSanitizeArgs(D, Args, DiagnoseErrors, MergeDefault, {}, {},
                         options::OPT_fsanitize_merge_handlers_EQ,
-                        options::OPT_fno_sanitize_merge_handlers_EQ);
+                        options::OPT_fno_sanitize_merge_handlers_EQ, nullptr);
   MergeKinds &= Kinds;
 
+  // Parse -fno-sanitize-top-hot flags
+  SanitizerMask TopHotMask =
+      parseNoSanitizeHotArgs(D, Args, DiagnoseErrors, &TopHotCutoffs);
+  (void)TopHotMask;
+
   // Setup ignorelist files.
   // Add default ignorelist from resource directory for activated sanitizers,
   // and validate special case lists format.
@@ -1132,6 +1146,15 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
          "Overlap between recoverable and trapping sanitizers");
 
   MergeHandlers.Mask |= MergeKinds;
+
+  TopHotMask &= Sanitizers.Mask;
+  TopHot.Mask = TopHotMask;
+
+  // Zero out TopHot for unused sanitizers
+  for (unsigned int i = 0; i < SanitizerKind::SO_Count; i++) {
+    if (!(Sanitizers.Mask & SanitizerMask::bitPosToMask(i)))
+      TopHotCutoffs[i] = 0;
+  }
 }
 
 static std::string toString(const clang::SanitizerSet &Sanitizers) {
@@ -1146,6 +1169,19 @@ static std::string toString(const clang::SanitizerSet &Sanitizers) {
   return Res;
 }
 
+static std::string toString(const clang::SanitizerMaskCutoffs &Cutoffs) {
+  std::string Res;
+#define SANITIZER(NAME, ID)                                                    \
+  if (Cutoffs[SanitizerKind::SO_##ID]) {                                       \
+    if (!Res.empty())                                                          \
+      Res += ",";                                                              \
+    Res += std::string(NAME) + "=" +                                           \
+           std::to_string(Cutoffs[SanitizerKind::SO_##ID]);                    \
+  }
+#include "clang/Basic/Sanitizers.def"
+  return Res;
+}
+
 static void addSpecialCaseListOpt(const llvm::opt::ArgList &Args,
                                   llvm::opt::ArgStringList &CmdArgs,
                                   const char *SCLOptFlag,
@@ -1297,6 +1333,11 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
     CmdArgs.push_back(
         Args.MakeArgString("-fsanitize-merge=" + toString(MergeHandlers)));
 
+  std::string TopHotCutoffsStr = toString(TopHotCutoffs);
+  if (TopHotCutoffsStr != "")
+    CmdArgs.push_back(
+        Args.MakeArgString("-fno-sanitize-top-hot=" + TopHotCutoffsStr));
+
   addSpecialCaseListOpt(Args, CmdArgs,
                         "-fsanitize-ignorelist=", UserIgnorelistFiles);
   addSpecialCaseListOpt(Args, CmdArgs,
@@ -1463,17 +1504,18 @@ 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) ||
-       A->getOption().matches(options::OPT_fsanitize_merge_handlers_EQ) ||
-       A->getOption().matches(options::OPT_fno_sanitize_merge_handlers_EQ)) &&
-      "Invalid argument in parseArgValues!");
+                             bool DiagnoseErrors,
+                             SanitizerMaskCutoffs *Cutoffs) {
+  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) ||
+          A->getOption().matches(options::OPT_fno_sanitize_top_hot_EQ)) &&
+         "Invalid argument in parseArgValues!");
   SanitizerMask Kinds;
   for (int i = 0, n = A->getNumValues(); i != n; ++i) {
     const char *Value = A->getValue(i);
@@ -1482,8 +1524,15 @@ SanitizerMask parseArgValues(const Driver &D, const llvm::opt::Arg *A,
     if (A->getOption().matches(options::OPT_fsanitize_EQ) &&
         0 == strcmp("all", Value))
       Kind = SanitizerMask();
-    else
+    else if (A->getOption().matches(options::OPT_fno_sanitize_top_hot_EQ)) {
+      assert(
+          Cutoffs &&
+          "Null Cutoffs parameter provided for parsing fno_sanitize_top_hot!");
+      Kind = parseSanitizerWeightedValue(Value, /*AllowGroups=*/true, Cutoffs);
+    } else {
+      assert((!Cutoffs) && "Non-null Cutoffs parameter erroneously provided!");
       Kind = parseSanitizerValue(Value, /*AllowGroups=*/true);
+    }
 
     if (Kind)
       Kinds |= Kind;
@@ -1586,12 +1635,12 @@ std::string lastArgumentForMask(const Driver &D, const llvm::opt::ArgList &Args,
     const auto *Arg = *I;
     if (Arg->getOption().matches(options::OPT_fsanitize_EQ)) {
       SanitizerMask AddKinds =
-          expandSanitizerGroups(parseArgValues(D, Arg, false));
+          expandSanitizerGroups(parseArgValues(D, Arg, false, nullptr));
       if (AddKinds & Mask)
         return describeSanitizeArg(Arg, Mask);
     } else if (Arg->getOption().matches(options::OPT_fno_sanitize_EQ)) {
       SanitizerMask RemoveKinds =
-          expandSanitizerGroups(parseArgValues(D, Arg, false));
+          expandSanitizerGroups(parseArgValues(D, Arg, false, nullptr));
       Mask &= ~RemoveKinds;
     }
   }
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 348c56cc37da3f..78dd5099259f18 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -1436,6 +1436,19 @@ static SmallVector<StringRef, 4> serializeSanitizerKinds(SanitizerSet S) {
   return Values;
 }
 
+static void parseSanitizerWeightedKinds(
+    StringRef FlagName, const std::vector<std::string> &Sanitizers,
+    DiagnosticsEngine &Diags, SanitizerSet &S, SanitizerMaskCutoffs *Cutoffs) {
+  for (const auto &Sanitizer : Sanitizers) {
+    SanitizerMask K =
+        parseSanitizerWeightedValue(Sanitizer, /*AllowGroups=*/false, Cutoffs);
+    if (K == SanitizerMask())
+      Diags.Report(diag::err_drv_invalid_value) << FlagName << Sanitizer;
+    else
+      S.set(K, true);
+  }
+}
+
 static void parseXRayInstrumentationBundle(StringRef FlagName, StringRef Bundle,
                                            ArgList &Args, DiagnosticsEngine &D,
                                            XRayInstrSet &S) {
@@ -1796,6 +1809,11 @@ void CompilerInvocationBase::GenerateCodeGenArgs(const CodeGenOptions &Opts,
        serializeSanitizerKinds(Opts.SanitizeMergeHandlers))
     GenerateArg(Consumer, OPT_fsanitize_merge_handlers_EQ, Sanitizer);
 
+  SmallVector<std::string, 4> Values;
+  serializeSanitizerMaskCutoffs(Opts.NoSanitizeTopHotCutoffs, Values);
+  for (std::string Sanitizer : Values)
+    GenerateArg(Consumer, OPT_fno_sanitize_top_hot_EQ, Sanitizer);
+
   if (!Opts.EmitVersionIdentMetadata)
     GenerateArg(Consumer, OPT_Qn);
 
@@ -2277,6 +2295,12 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args,
                       Args.getAllArgValues(OPT_fsanitize_merge_handlers_EQ),
                       Diags, Opts.SanitizeMergeHandlers);
 
+  // Parse -fno-sanitize-top-hot= arguments.
+  parseSanitizerWeightedKinds("-fno-sanitize-top-hot=",
+                              Args.getAllArgValues(OPT_fno_sanitize_top_hot_EQ),
+                              Diags, Opts.NoSanitizeTopHot,
+                              &Opts.NoSanitizeTopHotCutoffs);
+
   Opts.EmitVersionIdentMetadata = Args.hasFlag(OPT_Qy, OPT_Qn, true);
 
   if (!LangOpts->CUDAIsDevice)
diff --git a/clang/test/CodeGen/allow-ubsan-check.c b/clang/test/CodeGen/allow-ubsan-check.c
index 5232d240854666..1c76049b57bda8 100644
--- a/clang/test/CodeGen/allow-ubsan-check.c
+++ b/clang/test/CodeGen/allow-ubsan-check.c
@@ -3,7 +3,6 @@
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -o - %s -fsanitize=signed-integer-overflow,integer-divide-by-zero,null -mllvm -ubsan-guard-checks -fsanitize-trap=signed-integer-overflow,integer-divide-by-zero,null | FileCheck %s --check-prefixes=TRAP
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -o - %s -fsanitize=signed-integer-overflow,integer-divide-by-zero,null -mllvm -ubsan-guard-checks -fsanitize-recover=signed-integer-overflow,integer-divide-by-zero,null | FileCheck %s --check-prefixes=RECOVER
 
-
 // CHECK-LABEL: define dso_local i32 @d...
[truncated]

Rename a few internal variables as well
@thurstond thurstond requested a review from vitalybuka January 9, 2025 20:51
@thurstond thurstond changed the title [sanitizer] Parse weighted sanitizer args and -fno-sanitize-top-hot [sanitizer] Parse weighted sanitizer args and -fsanitize-skip-hot-cutoff Jan 9, 2025
void SanitizerMaskCutoffs::set(SanitizerMask K, double V) {
if (V < SanitizerMaskCutoffsEps && Cutoffs.empty())
return;
for (unsigned int i = 0; i < SanitizerKind::SO_Count; i++)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

llvm prefer ++i

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 10e0a55. Thanks for the review!

@thurstond thurstond merged commit 76fac9c into llvm:main Jan 10, 2025
5 of 7 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jan 10, 2025

LLVM Buildbot has detected a new failure on builder openmp-offload-amdgpu-runtime running on omp-vega20-0 while building clang at step 6 "test-openmp".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/30/builds/13517

Here is the relevant piece of the build log for the reference
Step 6 (test-openmp) failure: test (failure)
******************** TEST 'libomp :: tasking/issue-94260-2.c' FAILED ********************
Exit Code: -11

Command Output (stdout):
--
# RUN: at line 1
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/clang -fopenmp   -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/runtime/test -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src  -fno-omit-frame-pointer -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/runtime/test/ompt /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/runtime/test/tasking/issue-94260-2.c -o /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp -lm -latomic && /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/clang -fopenmp -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/runtime/test -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -fno-omit-frame-pointer -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/runtime/test/ompt /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/runtime/test/tasking/issue-94260-2.c -o /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp -lm -latomic
# note: command had no output on stdout or stderr
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp
# note: command had no output on stdout or stderr
# error: command failed with exit status: -11

--

********************


BaiXilin pushed a commit to BaiXilin/llvm-project that referenced this pull request Jan 12, 2025
…off (llvm#121619)

This adds a function to parse weighted sanitizer flags (e.g.,
`-fsanitize-blah=undefined=0.5,null=0.3`) and adds the plumbing to apply
that to a new flag, `-fsanitize-skip-hot-cutoff`.

`-fsanitize-skip-hot-cutoff` currently has no effect; future work will
use it to generalize ubsan-guard-checks (originally introduced in
5f9ed2f).

---------

Co-authored-by: Vitaly Buka <vitalybuka@google.com>
thurstond added a commit to thurstond/llvm-project that referenced this pull request Jan 13, 2025
This is glue code to convert LowerAllowCheckPass from a FUNCTION_PASS to FUNCTION_PASS_WITH_PARAMS. The parameters are currently unused.

Future work will plumb `-fsanitize-skip-hot-cutoff`
(introduced in llvm#121619) to
LowerAllowCheckOptions.
thurstond added a commit that referenced this pull request Jan 14, 2025
This is glue code to convert LowerAllowCheckPass from a FUNCTION_PASS to
FUNCTION_PASS_WITH_PARAMS. The parameters are currently unused.

Future work will plumb `-fsanitize-skip-hot-cutoff` (introduced in
#121619) to
LowerAllowCheckOptions.
github-actions bot pushed a commit to arm/arm-toolchain that referenced this pull request Jan 14, 2025
…Pass (#122765)

This is glue code to convert LowerAllowCheckPass from a FUNCTION_PASS to
FUNCTION_PASS_WITH_PARAMS. The parameters are currently unused.

Future work will plumb `-fsanitize-skip-hot-cutoff` (introduced in
llvm/llvm-project#121619) to
LowerAllowCheckOptions.
thurstond added a commit to thurstond/llvm-project that referenced this pull request Jan 23, 2025
This passes through the values of -fsanitize-skip-hot-cutoff (introduced
patch in llvm#121619) to the
LowerAllowCheckPass, via the Options parameter (introduced in
llvm#122994), and adjusts the
instrumentation accordingly.

The net effect is that -fsanitize-skip-hot-cutoff now combines the functionality of -ubsan-guard-checks and -lower-allow-check-percentile-cutoff (though this patch does not remove those yet), and generalizes the latter to allow per-sanitizer cutoffs.

Note: this patch replaces Intrinsic::allow_ubsan_check's SanitizerHandler parameter
with SanitizerOrdinal; this is necessary because the hot cutoffs are
specified in terms of SanitizerOrdinal (e.g., null, alignment), not
SanitizerHandler (e.g., TypeMismatch).
thurstond added a commit that referenced this pull request Jan 28, 2025
…LowerAllowCheckPass (#124211)

This adds and utilizes a cutoffs parameter for LowerAllowCheckPass, via the Options parameter (introduced in #122994).

Future work will connect -fsanitize-skip-hot-cutoff (introduced patch in #121619) in the clang frontend to the cutoffs parameter used here.
github-actions bot pushed a commit to arm/arm-toolchain that referenced this pull request Jan 28, 2025
…=90000> in LowerAllowCheckPass (#124211)

This adds and utilizes a cutoffs parameter for LowerAllowCheckPass, via the Options parameter (introduced in llvm/llvm-project#122994).

Future work will connect -fsanitize-skip-hot-cutoff (introduced patch in llvm/llvm-project#121619) in the clang frontend to the cutoffs parameter used here.
thurstond added a commit to thurstond/llvm-project that referenced this pull request Jan 28, 2025
…offs>

This adds the plumbing between -fsanitize-skip-hot-cutoff (introduced
in llvm#121619) and LowerAllowCheckPass<cutoffs> (introduced in llvm#124211).

The net effect is that -fsanitize-skip-hot-cutoff now combines the functionality of -ubsan-guard-checks and -lower-allow-check-percentile-cutoff (though this patch does not remove those yet), and generalizes the latter to allow per-sanitizer cutoffs.

Note: this patch replaces Intrinsic::allow_ubsan_check's SanitizerHandler parameter
with SanitizerOrdinal; this is necessary because the hot cutoffs are
specified in terms of SanitizerOrdinal (e.g., null, alignment), not
SanitizerHandler (e.g., TypeMismatch).
thurstond added a commit that referenced this pull request Jan 30, 2025
…offs> (#124857)

This adds the plumbing between -fsanitize-skip-hot-cutoff (introduced in
#121619) and
LowerAllowCheckPass<cutoffs> (introduced in
#124211).
    
The net effect is that -fsanitize-skip-hot-cutoff now combines the
functionality of -ubsan-guard-checks and
-lower-allow-check-percentile-cutoff (though this patch does not remove
those yet), and generalizes the latter to allow per-sanitizer cutoffs.
    
Note: this patch replaces Intrinsic::allow_ubsan_check's
SanitizerHandler parameter with SanitizerOrdinal; this is necessary
because the hot cutoffs are specified in terms of SanitizerOrdinal
(e.g., null, alignment), not SanitizerHandler (e.g., TypeMismatch).
Likewise, CodeGenFunction::EmitCheck is changed to emit
allow_ubsan_check() for each individual check.

---------

Co-authored-by: Vitaly Buka <vitalybuka@gmail.com>
Co-authored-by: Vitaly Buka <vitalybuka@google.com>
github-actions bot pushed a commit to arm/arm-toolchain that referenced this pull request Jan 30, 2025
…eckPass<cutoffs> (#124857)

This adds the plumbing between -fsanitize-skip-hot-cutoff (introduced in
llvm/llvm-project#121619) and
LowerAllowCheckPass<cutoffs> (introduced in
llvm/llvm-project#124211).

The net effect is that -fsanitize-skip-hot-cutoff now combines the
functionality of -ubsan-guard-checks and
-lower-allow-check-percentile-cutoff (though this patch does not remove
those yet), and generalizes the latter to allow per-sanitizer cutoffs.

Note: this patch replaces Intrinsic::allow_ubsan_check's
SanitizerHandler parameter with SanitizerOrdinal; this is necessary
because the hot cutoffs are specified in terms of SanitizerOrdinal
(e.g., null, alignment), not SanitizerHandler (e.g., TypeMismatch).
Likewise, CodeGenFunction::EmitCheck is changed to emit
allow_ubsan_check() for each individual check.

---------

Co-authored-by: Vitaly Buka <vitalybuka@gmail.com>
Co-authored-by: Vitaly Buka <vitalybuka@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants