Merge pull request #10853 from swiftlang/revert-10800-dliew/rdar-150805550

hnrklssn · web-flow · commit 4acf830ff485 · 2025-06-17T15:23:29.000+02:00
Revert "[BoundsSafety] Add warning diagnostics for uses of legacy bounds checks"
diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -402,36 +402,6 @@ def warn_bounds_safety_adoption_mode_ignored : Warning<
   "-fbounds-safety-adoption-mode without -fbounds-safety is "
   "ignored">,
   InGroup<BoundsSafetyAdoptionModeIgnored>;
-
-def warn_bounds_safety_new_checks_none : Warning<
-  "compiling with legacy -fbounds-safety bounds checks is deprecated;"
-  " compile with -fbounds-safety-bringup-missing-checks=%0 to use the "
-  "new bound checks">,
-  InGroup<BoundsSafetyLegacyChecksEnabled>;
-def warn_bounds_safety_new_checks_mixed : Warning<
-  "compiling with \"%select{"
-    "access_size|" // BS_CHK_AccessSize
-    "indirect_count_update|" // BS_CHK_IndirectCountUpdate
-    "return_size|" // BS_CHK_ReturnSize
-    "ended_by_lower_bound|" // BS_CHK_EndedByLowerBound
-    "compound_literal_init|" // BS_CHK_CompoundLiteralInit
-    "libc_attributes|" // BS_CHK_LibCAttributes
-    "array_subscript_agg" // BS_CHK_ArraySubscriptAgg
-  "}0\" bounds check disabled is deprecated;"
-  " %select{"
-    "compile with -fbounds-safety-bringup-missing-checks=%2|"
-    "remove -fno-bounds-safety-bringup-missing-checks=%select{"
-      "access_size|" // BS_CHK_AccessSize
-      "indirect_count_update|" // BS_CHK_IndirectCountUpdate
-      "return_size|" // BS_CHK_ReturnSize
-      "ended_by_lower_bound|" // BS_CHK_EndedByLowerBound
-      "compound_literal_init|" // BS_CHK_CompoundLiteralInit
-      "libc_attributes|" // BS_CHK_LibCAttributes
-      "array_subscript_agg" // BS_CHK_ArraySubscriptAgg
-      "}0"
-    "|}1"
-  " to enable the new bound checks">,
-  InGroup<BoundsSafetyLegacyChecksEnabled>;
 // TO_UPSTREAM(BoundsSafety) OFF
 
 let CategoryName = "Instrumentation Issue" in {
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1677,8 +1677,6 @@ def BoundsSafetyStrictTerminatedByCast
     : DiagGroup<"bounds-safety-strict-terminated-by-cast">;
 def BoundsSafetyCountAttrArithConstantCount :
   DiagGroup<"bounds-safety-externally-counted-ptr-arith-constant-count">;
-def BoundsSafetyLegacyChecksEnabled :
-  DiagGroup<"bounds-safety-legacy-checks-enabled">;
 // TO_UPSTREAM(BoundsSafety) OFF
 
 // -fbounds-safety and bounds annotation related warnings
diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
@@ -454,8 +454,6 @@ 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_MaxMask = BS_CHK_ArraySubscriptAgg,
   };
   using BoundsSafetyNewChecksMaskIntTy =
       std::underlying_type_t<BoundsSafetyNewChecks>;
diff --git a/clang/include/clang/Driver/BoundsSafetyArgs.h b/clang/include/clang/Driver/BoundsSafetyArgs.h
@@ -16,8 +16,7 @@ namespace driver {
 
 LangOptions::BoundsSafetyNewChecksMaskIntTy
 ParseBoundsSafetyNewChecksMaskFromArgs(const llvm::opt::ArgList &Args,
-                                       DiagnosticsEngine *Diags,
-                                       bool DiagnoseMissingChecks);
+                                       DiagnosticsEngine *Diags);
 } // namespace driver
 } // namespace clang
 
diff --git a/clang/lib/Driver/BoundsSafetyArgs.cpp b/clang/lib/Driver/BoundsSafetyArgs.cpp
@@ -7,11 +7,7 @@
 //===----------------------------------------------------------------------===//
 #include "clang/Driver/BoundsSafetyArgs.h"
 #include "clang/Basic/DiagnosticDriver.h"
-#include "clang/Basic/DiagnosticFrontend.h"
 #include "clang/Driver/Options.h"
-#include "llvm/ADT/StringSet.h"
-#include "llvm/ADT/bit.h"
-#include <array>
 
 using namespace llvm::opt;
 using namespace clang::driver::options;
@@ -20,103 +16,15 @@ namespace clang {
 
 namespace driver {
 
-static void DiagnoseDisabledBoundsSafetyChecks(
-    LangOptions::BoundsSafetyNewChecksMaskIntTy EnabledChecks,
-    DiagnosticsEngine *Diags,
-    LangOptions::BoundsSafetyNewChecksMaskIntTy
-        IndividualChecksExplicitlyDisabled) {
-  struct BoundsCheckBatch {
-    const char *Name;
-    LangOptionsBase::BoundsSafetyNewChecksMaskIntTy Checks;
-  };
-
-  // Batches of checks should be ordered with newest first
-  std::array<BoundsCheckBatch, 2> Batches = {
-      {// We deliberately don't include `all` here because that batch
-       // isn't stable over time (unlike batches like `batch_0`) so we
-       // don't want to suggest users start using it.
-       {"batch_0",
-        LangOptions::getBoundsSafetyNewChecksMaskForGroup("batch_0")},
-       {"none", LangOptions::BS_CHK_None}}};
-
-  LangOptionsBase::BoundsSafetyNewChecksMaskIntTy DiagnosedDisabledChecks =
-      LangOptions::BS_CHK_None;
-
-  // Loop over all batches except "none"
-  for (size_t BatchIdx = 0; BatchIdx < Batches.size() - 1; ++BatchIdx) {
-    auto ChecksInCurrentBatch = Batches[BatchIdx].Checks;
-    auto ChecksInOlderBatch = Batches[BatchIdx + 1].Checks;
-    auto ChecksInCurrentBatchOnly = ChecksInCurrentBatch & ~ChecksInOlderBatch;
-    const auto *CurrentBatchName = Batches[BatchIdx].Name;
-
-    if ((EnabledChecks & ChecksInCurrentBatchOnly) == ChecksInCurrentBatchOnly)
-      continue; // All checks in batch are enabled. Nothing to diagnose.
-
-    // Diagnose disabled bounds checks
-
-    if ((EnabledChecks & ChecksInCurrentBatchOnly) == 0) {
-      // None of the checks in the current batch are enabled. Diagnose this
-      // once for all the checks in the batch.
-      if ((DiagnosedDisabledChecks & ChecksInCurrentBatchOnly) !=
-          ChecksInCurrentBatchOnly) {
-        Diags->Report(diag::warn_bounds_safety_new_checks_none)
-            << CurrentBatchName;
-        DiagnosedDisabledChecks |= ChecksInCurrentBatchOnly;
-      }
-      continue;
-    }
-
-    // Some (but not all) checks are disabled in the current batch. Iterate over
-    // each check in the batch and emit a diagnostic for each disabled check
-    // in the batch.
-    assert(ChecksInCurrentBatch > 0);
-    auto FirstCheckInBatch = 1 << llvm::countr_zero(ChecksInCurrentBatch);
-    for (size_t CheckBit = FirstCheckInBatch;
-         CheckBit <= LangOptions::BS_CHK_MaxMask; CheckBit <<= 1) {
-      assert(CheckBit != 0);
-      if ((CheckBit & ChecksInCurrentBatch) == 0)
-        continue; // Check not in batch
-
-      if (EnabledChecks & CheckBit)
-        continue; // Check is active
-
-      // Diagnose disabled check
-      if (!(DiagnosedDisabledChecks & CheckBit)) {
-        size_t CheckNumber = llvm::countr_zero(CheckBit);
-        // If we always suggested enabling the current batch that
-        // could be confusing if the user passed something like
-        // `-fbounds-safety-bringup-missing-checks=batch_0
-        // -fno-bounds-safety-bringup-missing-checks=access_size`. To avoid
-        // this we detect when the check corresponding to `CheckBit` has been
-        // explicitly disabled on the command line and in that case we suggeset
-        // removing the flag.
-        bool SuggestRemovingFlag =
-            CheckBit & IndividualChecksExplicitlyDisabled;
-        Diags->Report(diag::warn_bounds_safety_new_checks_mixed)
-            << CheckNumber << SuggestRemovingFlag << CurrentBatchName;
-        DiagnosedDisabledChecks |= CheckBit;
-      }
-    }
-  }
-}
-
 LangOptions::BoundsSafetyNewChecksMaskIntTy
 ParseBoundsSafetyNewChecksMaskFromArgs(const llvm::opt::ArgList &Args,
-                                       DiagnosticsEngine *Diags,
-                                       bool DiagnoseMissingChecks) {
-  assert((!DiagnoseMissingChecks || Diags) &&
-         "Cannot diagnose missing checks when Diags is a nullptr");
-  LangOptions::BoundsSafetyNewChecksMaskIntTy
-      IndividualChecksExplicitlyDisabled = LangOptions::BS_CHK_None;
+                                       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
-    auto Result = LangOptions::getDefaultBoundsSafetyNewChecksMask();
-    DiagnoseDisabledBoundsSafetyChecks(Result, Diags,
-                                       IndividualChecksExplicitlyDisabled);
-    return Result;
+    return LangOptions::getDefaultBoundsSafetyNewChecksMask();
   }
 
   // If flags are present then start with BS_CHK_None as the initial mask and
@@ -127,11 +35,6 @@ ParseBoundsSafetyNewChecksMaskFromArgs(const llvm::opt::ArgList &Args,
   // 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).
-  const auto Batch0Checks =
-      LangOptions::getBoundsSafetyNewChecksMaskForGroup("batch_0");
-  const auto AllChecks =
-      LangOptions::getBoundsSafetyNewChecksMaskForGroup("all");
-  bool Errors = false;
   for (const Arg *A : FilteredArgs) {
     for (const char *Value : A->getValues()) {
       std::optional<LangOptions::BoundsSafetyNewChecksMaskIntTy> Mask =
@@ -148,16 +51,17 @@ ParseBoundsSafetyNewChecksMaskFromArgs(const llvm::opt::ArgList &Args,
               .Case("libc_attributes", LangOptions::BS_CHK_LibCAttributes)
               .Case("array_subscript_agg",
                     LangOptions::BS_CHK_ArraySubscriptAgg)
-              .Case("all", AllChecks)
-              .Case("batch_0", Batch0Checks)
+              .Case("all",
+                    LangOptions::getBoundsSafetyNewChecksMaskForGroup("all"))
+              .Case(
+                  "batch_0",
+                  LangOptions::getBoundsSafetyNewChecksMaskForGroup("batch_0"))
               .Case("none", LangOptions::BS_CHK_None)
               .Default(std::nullopt);
-
       if (!Mask) {
         if (Diags)
           Diags->Report(diag::err_drv_invalid_value)
               << A->getSpelling() << Value;
-        Errors = true;
         break;
       }
 
@@ -177,7 +81,6 @@ ParseBoundsSafetyNewChecksMaskFromArgs(const llvm::opt::ArgList &Args,
               << A->getSpelling() << Value
               << (IsPosFlag ? "-fno-bounds-safety-bringup-missing-checks"
                             : "-fbounds-safety-bringup-missing-checks");
-        Errors = true;
         break;
       }
 
@@ -188,25 +91,8 @@ ParseBoundsSafetyNewChecksMaskFromArgs(const llvm::opt::ArgList &Args,
             OPT_fno_bounds_safety_bringup_missing_checks_EQ));
         Result &= ~(*Mask);
       }
-
-      // Update which checks have been explicitly disabled. E.g.
-      // `-fno-bounds-safety-bringup-missing-checks=access_size`.
-      if (llvm::has_single_bit(*Mask)) {
-        // A single check was enabled/disabled
-        if (IsPosFlag)
-          IndividualChecksExplicitlyDisabled &= ~(*Mask);
-        else
-          IndividualChecksExplicitlyDisabled |= *Mask;
-      } else {
-        // A batch of checks were enabled/disabled. Any checks in that batch
-        // are no longer explicitly set.
-        IndividualChecksExplicitlyDisabled &= ~(*Mask);
-      }
     }
   }
-  if (DiagnoseMissingChecks && Diags && !Errors)
-    DiagnoseDisabledBoundsSafetyChecks(Result, Diags,
-                                       IndividualChecksExplicitlyDisabled);
   return Result;
 }
 
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -25,7 +25,6 @@
 #include "clang/Basic/Version.h"
 #include "clang/Config/config.h"
 #include "clang/Driver/Action.h"
-#include "clang/Driver/BoundsSafetyArgs.h" // TO_UPSTREAM(BoundsSafety)
 #include "clang/Driver/CommonArgs.h"
 #include "clang/Driver/Distro.h"
 #include "clang/Driver/InputInfo.h"
@@ -7612,15 +7611,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction &Job,
       Args.addAllArgs(
           CmdArgs, {options::OPT_fbounds_safety_bringup_missing_checks_EQ,
                     options::OPT_fno_bounds_safety_bringup_missing_checks_EQ});
-
-      // Validate the `-fbounds-safety-bringup-missing-checks` flags and emit
-      // warnings for disabled checks. We do this here because if we emit
-      // warnings in CC1 (where `ParseBoundsSafetyNewChecksMaskFromArgs` is also
-      // called) they cannot be suppressed and ignore `-Werror`
-      // (rdar://152730261).
-      (void)ParseBoundsSafetyNewChecksMaskFromArgs(
-          Args, &D.getDiags(),
-          /*DiagnoseMissingChecks=*/true);
     }
   }
 
diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1336,8 +1336,7 @@ void DarwinClang::addClangTargetOptions(
                                 options::OPT_fno_bounds_safety, false)) {
     LangOptions::BoundsSafetyNewChecksMaskIntTy NewChecks =
         ParseBoundsSafetyNewChecksMaskFromArgs(DriverArgs,
-                                               /*DiagnosticsEngine=*/nullptr,
-                                               /*DiagnoseMissingChecks=*/false);
+                                               /*DiagnosticsEngine=*/nullptr);
     if (NewChecks & LangOptions::BS_CHK_LibCAttributes) {
       bool TargetWithoutUserspaceLibc = false;
       if (getTriple().isOSDarwin() && !TargetWithoutUserspaceLibc) {
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4575,13 +4575,9 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
       Args.hasFlag(OPT_ftrigraphs, OPT_fno_trigraphs, Opts.Trigraphs);
 
   /*TO_UPSTREAM(BoundsSafety) ON*/
-  // Parse the enabled checks and emit any necessary diagnostics.
-  // We do not diagnose missing checks here because warnings emitted in this
-  // context cannot be surpressed and don't respect `-Werror`
-  // (rdar://152730261). Instead we warn about missing checks in the driver.
+  // Parse the enabled checks and emit any necessary diagnostics
   Opts.BoundsSafetyBringUpMissingChecks =
-      ParseBoundsSafetyNewChecksMaskFromArgs(Args, &Diags,
-                                             /*DiagnoseMissingChecks=*/false);
+      ParseBoundsSafetyNewChecksMaskFromArgs(Args, &Diags);
 
   // -fbounds-safety should be automatically marshalled into `Opts` in
   // GenerateFrontendArgs() via `LangOpts<"BoundsSafety">` on
diff --git a/clang/test/BoundsSafety/Driver/bounds-safety-bringup-missing-checks-disabled.c b/clang/test/BoundsSafety/Driver/bounds-safety-bringup-missing-checks-disabled.c
