[BoundsSafety] Support arbitrary check groups for the -fbounds-safety-bringup-missing-checks= flag

delcypher · delcypher · commit 71789b9d1646 · 2025-03-07T10:44:08.000-08:00
Currently we have the `-fbounds-safety-bringup-missing-checks=` flag which allows switching on new bounds checks. This flag has a special `all` value that means switch on all the checks. However `all` isn't sufficient because we may have `-fbounds-safety` adopters using the `-fbounds-safety-bringup-missing-checks` flag (enables `all` checks) already but we want to introduce new bounds checks that don't get enabled automatically. This patch solves this problem by introducing support for arbitrary groups of checks. This is implemented by the `LangOptionsBase::getBoundsSafetyNewChecksMaskForGroup` function which takes a group name and returns a bit mask of the checks that should be enabled for the group. This patch introduces the `batch_0` group. It is currently the same as `all` but as we introduce support of more bounds checks `batch_0` will contain the same set of checks, but `all` will change over time to include any new checks we implement. This means an adopter of `-fbounds-safety` can provide the `-fbounds-safety-bringup-missing-checks=batch_0` flag to get a stable group of checks that will not change over time. This is preferrable to specifying `-fbounds-safety-bringup-missing-checks=all` because the enabled checks will likely change overtime which could cause regressions in an `-fbounds-safety` adopter when the compiler is upgraded. While we're here this patch also fixes how the `BoundsSafetyNewChecksMask` type was being incorrectly used as both an enum and a bitmask. While this technically worked (because enums have an underlying integer type) it isn't conceptually correct because a bit mask and an enum value are not the same thing. A value of enum type should take on only **ONE** of the valid enum values, whereas a bitmask can take on one or more of the valid enum values. This is fixed by introducing `BoundsSafetyNewChecksMaskIntTy` and using it where appropriate and also renaming `BoundsSafetyNewChecksMask` to `BoundsSafetyNewChecks`. rdar://145826593 (cherry picked from commit 704c190)
diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def
@@ -540,7 +540,7 @@ LANGOPT(BoundsAttributesCXXExperimental, 1, 0, "Experimental bounds attributes f
 LANGOPT(BoundsAttributesObjCExperimental, 1, 0, "Experimental bounds attributes for Objective-C")
 LANGOPT(BoundsSafetyRelaxedSystemHeaders, 1, 1,
         "Relax bounds safety assignment rules in system headers")
-ENUM_LANGOPT(BoundsSafetyBringUpMissingChecks, BoundsSafetyNewChecksMask, 7, BS_CHK_Default,
+LANGOPT(BoundsSafetyBringUpMissingChecks, 7, clang::LangOptions::getDefaultBoundsSafetyNewChecksMask(),
         "Bring up new -fbounds-safety run-time checks")
 /* TO_UPSTREAM(BoundsSafety) OFF*/
 
diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
@@ -411,7 +411,7 @@ class LangOptionsBase {
   };
 
   /* TODO(BoundsSafety) Deprecate the flag  */
-  enum BoundsSafetyNewChecksMask {
+  enum BoundsSafetyNewChecks {
     BS_CHK_None = 0,
 
     BS_CHK_AccessSize = 1 << 0,          // rdar://72252593
@@ -421,17 +421,13 @@ class LangOptionsBase {
     BS_CHK_CompoundLiteralInit = 1 << 4, // rdar://110871666
     BS_CHK_LibCAttributes = 1 << 5,      // rdar://84733153
     BS_CHK_ArraySubscriptAgg = 1 << 6,   // rdar://145020583
-
-    BS_CHK_All = BS_CHK_AccessSize | BS_CHK_IndirectCountUpdate |
-                 BS_CHK_ReturnSize | BS_CHK_EndedByLowerBound |
-                 BS_CHK_CompoundLiteralInit | BS_CHK_LibCAttributes |
-                 BS_CHK_ArraySubscriptAgg,
-
-    // This sets the default value assumed by clang if no
-    // `-fbounds-safety-bringup-missing-checks` flags are
-    // passed to clang.
-    BS_CHK_Default = BS_CHK_None,
   };
+  using BoundsSafetyNewChecksMaskIntTy =
+      std::underlying_type_t<BoundsSafetyNewChecks>;
+
+  static BoundsSafetyNewChecksMaskIntTy
+  getBoundsSafetyNewChecksMaskForGroup(StringRef GroupName);
+  static BoundsSafetyNewChecksMaskIntTy getDefaultBoundsSafetyNewChecksMask();
 
   /// Controls the various implementations for complex multiplication and
   // division.
@@ -819,10 +815,10 @@ class LangOptions : public LangOptionsBase {
     return hasBoundsSafetyAttributes() && !hasBoundsSafety();
   }
 
-  // Take `enum BoundsSafetyNewChecksMask` value as input and return true if
-  // the mask is set. New -fbounds-safety run-time checks should use this helper
-  // to decide whether to enable/disable the checks.
-  bool hasNewBoundsSafetyCheck(BoundsSafetyNewChecksMask ChkMask) const {
+  // Take a BoundsSafetyNewChecksMask (or check mask) and return true if
+  // the check (or mask) is set. New -fbounds-safety run-time checks should use
+  // this helper to decide whether to enable/disable the checks.
+  bool hasNewBoundsSafetyCheck(BoundsSafetyNewChecksMaskIntTy ChkMask) const {
     return (BoundsSafetyBringUpMissingChecks & ChkMask) != 0;
   }
 
diff --git a/clang/include/clang/Driver/BoundsSafetyArgs.h b/clang/include/clang/Driver/BoundsSafetyArgs.h
@@ -13,10 +13,8 @@
 
 namespace clang {
 namespace driver {
-using BoundsSafetyNewChecksMaskTy =
-    std::underlying_type<LangOptions::BoundsSafetyNewChecksMask>::type;
 
-BoundsSafetyNewChecksMaskTy
+LangOptions::BoundsSafetyNewChecksMaskIntTy
 ParseBoundsSafetyNewChecksMaskFromArgs(const llvm::opt::ArgList &Args,
                                        DiagnosticsEngine *Diags);
 } // namespace driver
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
@@ -2004,12 +2004,12 @@ defm bounds_safety_adoption_mode : BoolFOption<"bounds-safety-adoption-mode",
 def fbounds_safety_bringup_missing_checks_EQ : CommaJoined<["-"], "fbounds-safety-bringup-missing-checks=">, Group<f_Group>,
   MetaVarName<"<check>">,
   Visibility<[ClangOption, CC1Option]>,
-  HelpText<"Enable a set of new -fbounds-safety run-time checks, (option: access_size, indirect_count_update, return_size, ended_by_lower_bound, compound_literal_init, libc_attributes, array_subscript_agg, all)">;
+  HelpText<"Enable a set of new -fbounds-safety run-time checks, (option: access_size, indirect_count_update, return_size, ended_by_lower_bound, compound_literal_init, libc_attributes, array_subscript_agg, all, batch_0)">;
 
 def fno_bounds_safety_bringup_missing_checks_EQ : CommaJoined<["-"], "fno-bounds-safety-bringup-missing-checks=">, Group<f_Group>,
   MetaVarName<"<check>">,
   Visibility<[ClangOption, CC1Option]>,
-  HelpText<"Disable a set of new -fbounds-safety run-time checks, (option: access_size, indirect_count_update, return_size, ended_by_lower_bound, compound_literal_init, libc_attributes, array_subscript_agg, all)">;
+  HelpText<"Disable a set of new -fbounds-safety run-time checks, (option: access_size, indirect_count_update, return_size, ended_by_lower_bound, compound_literal_init, libc_attributes, array_subscript_agg, all, batch_0)">;
 
 def fbounds_safety_bringup_missing_checks : Flag<["-"], "fbounds-safety-bringup-missing-checks">, Group<f_Group>,
   Visibility<[ClangOption]>,
diff --git a/clang/lib/Basic/LangOptions.cpp b/clang/lib/Basic/LangOptions.cpp
@@ -238,3 +238,25 @@ LLVM_DUMP_METHOD void FPOptionsOverride::dump() {
 #include "clang/Basic/FPOptions.def"
   llvm::errs() << "\n";
 }
+
+/* TO_UPSTREAM(BoundsSafety) ON*/
+LangOptionsBase::BoundsSafetyNewChecksMaskIntTy
+LangOptionsBase::getBoundsSafetyNewChecksMaskForGroup(StringRef GroupName) {
+  const uint64_t Batch0 = BS_CHK_AccessSize | BS_CHK_IndirectCountUpdate |
+                          BS_CHK_ReturnSize | BS_CHK_EndedByLowerBound |
+                          BS_CHK_CompoundLiteralInit | BS_CHK_LibCAttributes |
+                          BS_CHK_ArraySubscriptAgg;
+
+  // Currently "all" and "batch_0" are the same
+  if (GroupName == "all" || GroupName == "batch_0")
+    return Batch0;
+
+  // Invalid GroupName
+  return BS_CHK_None;
+}
+
+LangOptionsBase::BoundsSafetyNewChecksMaskIntTy
+LangOptionsBase::getDefaultBoundsSafetyNewChecksMask() {
+  return BS_CHK_None;
+}
+/* TO_UPSTREAM(BoundsSafety) OFF*/
diff --git a/clang/lib/Driver/BoundsSafetyArgs.cpp b/clang/lib/Driver/BoundsSafetyArgs.cpp
@@ -16,30 +16,30 @@ namespace clang {
 
 namespace driver {
 
-BoundsSafetyNewChecksMaskTy
+LangOptions::BoundsSafetyNewChecksMaskIntTy
 ParseBoundsSafetyNewChecksMaskFromArgs(const llvm::opt::ArgList &Args,
                                        DiagnosticsEngine *Diags) {
   auto FilteredArgs =
       Args.filtered(OPT_fbounds_safety_bringup_missing_checks_EQ,
                     OPT_fno_bounds_safety_bringup_missing_checks_EQ);
   if (FilteredArgs.empty()) {
     // No flags present. Use the default
-    return LangOptions::BS_CHK_Default;
+    return LangOptions::getDefaultBoundsSafetyNewChecksMask();
   }
 
   // If flags are present then start with BS_CHK_None as the initial mask and
   // update the mask based on the flags. This preserves compiler behavior for
   // users that adopted the `-fbounds-safety-bringup-missing-checks` flag when
-  // `BS_CHK_Default == BS_CHK_None`.
-  BoundsSafetyNewChecksMaskTy Result = LangOptions::BS_CHK_None;
+  // `getDefaultBoundsSafetyNewChecksMask() == BS_CHK_None`.
+  LangOptions::BoundsSafetyNewChecksMaskIntTy Result = LangOptions::BS_CHK_None;
   // All the options will be applied as masks in the command line order, to make
   // it easier to enable all but certain checks (or disable all but certain
   // checks).
   for (const Arg *A : FilteredArgs) {
     for (const char *Value : A->getValues()) {
-      std::optional<LangOptions::BoundsSafetyNewChecksMask> Mask =
+      std::optional<LangOptions::BoundsSafetyNewChecksMaskIntTy> Mask =
           llvm::StringSwitch<
-              std::optional<LangOptions::BoundsSafetyNewChecksMask>>(Value)
+              std::optional<LangOptions::BoundsSafetyNewChecksMaskIntTy>>(Value)
               .Case("access_size", LangOptions::BS_CHK_AccessSize)
               .Case("indirect_count_update",
                     LangOptions::BS_CHK_IndirectCountUpdate)
@@ -51,7 +51,11 @@ ParseBoundsSafetyNewChecksMaskFromArgs(const llvm::opt::ArgList &Args,
               .Case("libc_attributes", LangOptions::BS_CHK_LibCAttributes)
               .Case("array_subscript_agg",
                     LangOptions::BS_CHK_ArraySubscriptAgg)
-              .Case("all", LangOptions::BS_CHK_All)
+              .Case("all",
+                    LangOptions::getBoundsSafetyNewChecksMaskForGroup("all"))
+              .Case(
+                  "batch_0",
+                  LangOptions::getBoundsSafetyNewChecksMaskForGroup("batch_0"))
               .Case("none", LangOptions::BS_CHK_None)
               .Default(std::nullopt);
       if (!Mask) {
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3923,10 +3923,15 @@ void CompilerInvocationBase::GenerateLangArgs(const LangOptions &Opts,
 
   /*TO_UPSTREAM(BoundsSafety) ON*/
   if (Opts.BoundsSafety &&
-      Opts.BoundsSafetyBringUpMissingChecks != LangOptions::BS_CHK_Default) {
+      Opts.BoundsSafetyBringUpMissingChecks !=
+          LangOptions::getDefaultBoundsSafetyNewChecksMask()) {
     std::string Checks;
-    if (Opts.BoundsSafetyBringUpMissingChecks == LangOptions::BS_CHK_All) {
+    if (Opts.BoundsSafetyBringUpMissingChecks ==
+        LangOptions::getBoundsSafetyNewChecksMaskForGroup("all")) {
       Checks = "all";
+    } else if (Opts.BoundsSafetyBringUpMissingChecks ==
+               LangOptions::getBoundsSafetyNewChecksMaskForGroup("batch_0")) {
+      Checks = "batch_0";
     } else {
       if (Opts.hasNewBoundsSafetyCheck(LangOptions::BS_CHK_AccessSize))
         Checks += "access_size,";
diff --git a/clang/test/BoundsSafety/Driver/bounds-safety-bringup-missing-checks.c b/clang/test/BoundsSafety/Driver/bounds-safety-bringup-missing-checks.c
@@ -7,6 +7,8 @@
 // EMPTY-NOT: -fno-bounds-safety-bringup-missing-checks
 // EMPTY-NOT: -fbounds-safety-bringup-missing-checks=all
 // EMPTY-NOT: -fno-bounds-safety-bringup-missing-checks=all
+// EMPTY-NOT: -fbounds-safety-bringup-missing-checks=batch_0
+// EMPTY-NOT: -fno-bounds-safety-bringup-missing-checks=batch_0
 
 // RUN: %clang -fbounds-safety -fno-bounds-safety-bringup-missing-checks -c %s -### 2>&1 | FileCheck %s --check-prefix=DISABLED
 // DISABLED: -fno-bounds-safety-bringup-missing-checks=all
@@ -45,6 +47,9 @@
 // RUN: %clang -fbounds-safety -fbounds-safety-bringup-missing-checks=all -c %s -### 2>&1 | FileCheck %s --check-prefix=POS_ALL
 // POS_ALL: -fbounds-safety-bringup-missing-checks=all
 
+// RUN: %clang -fbounds-safety -fbounds-safety-bringup-missing-checks=batch_0 -c %s -### 2>&1 | FileCheck %s --check-prefix=POS_batch_0
+// POS_batch_0: -fbounds-safety-bringup-missing-checks=batch_0
+
 // RUN: %clang -fbounds-safety -fno-bounds-safety-bringup-missing-checks=access_size -c %s -### 2>&1 | FileCheck %s --check-prefix=NEG_ACCESS
 // NEG_ACCESS: -fno-bounds-safety-bringup-missing-checks=access_size
 
@@ -66,6 +71,9 @@
 // RUN: %clang -fbounds-safety -fno-bounds-safety-bringup-missing-checks=all -c %s -### 2>&1 | FileCheck %s --check-prefix=NEG_ALL
 // NEG_ALL: -fno-bounds-safety-bringup-missing-checks=all
 
+// RUN: %clang -fbounds-safety -fno-bounds-safety-bringup-missing-checks=batch_0 -c %s -### 2>&1 | FileCheck %s --check-prefix=NEG_batch_0
+// NEG_batch_0: -fno-bounds-safety-bringup-missing-checks=batch_0
+
 // RUN: %clang -fbounds-safety -fbounds-safety-bringup-missing-checks=access_size,indirect_count_update,return_size,ended_by_lower_bound,compound_literal_init,libc_attributes,all -c %s -### 2>&1 | FileCheck %s --check-prefix=POS_COMMA
 // POS_COMMA: -fbounds-safety-bringup-missing-checks=access_size,indirect_count_update,return_size,ended_by_lower_bound,compound_literal_init,libc_attributes,all
 
@@ -86,3 +94,8 @@
 // RUN: %clang -fbounds-safety-bringup-missing-checks -c %s -### 2>&1 | FileCheck %s --check-prefixes=UNUSED_POS
 // UNUSED_POS: warning: argument unused during compilation: '-fbounds-safety-bringup-missing-checks'
 
+// RUN: %clang -fno-bounds-safety-bringup-missing-checks=batch_0 -c %s -### 2>&1 | FileCheck %s --check-prefixes=UNUSED_NEG_BATCH_0
+// UNUSED_NEG_BATCH_0: warning: argument unused during compilation: '-fno-bounds-safety-bringup-missing-checks=batch_0'
+
+// RUN: %clang -fbounds-safety-bringup-missing-checks=batch_0 -c %s -### 2>&1 | FileCheck %s --check-prefixes=UNUSED_POS_BATCH_0
+// UNUSED_POS_BATCH_0: warning: argument unused during compilation: '-fbounds-safety-bringup-missing-checks=batch_0'
diff --git a/clang/unittests/Tooling/BoundsSafetyBringupMissingChecks.cpp b/clang/unittests/Tooling/BoundsSafetyBringupMissingChecks.cpp
