From 0aab1a8b0fcb12b539eec24cf4ee3d7afa64e1f9 Mon Sep 17 00:00:00 2001 From: Dan Liew Date: Wed, 14 May 2025 10:35:07 -0700 Subject: [PATCH] [BoundsSafety][Attempt 2] Add warning diagnostics for uses of legacy bounds checks ** This is the second attempt landing this patch. This first (#10800 4c8f6e7a8b51be79ce33fc4f1bae967dbbb55f48) failed because there was a bug. ** The bug was in `ParseBoundsSafetyNewChecksMaskFromArgs` where the call to `DiagnoseDisabledBoundsSafetyChecks` was not guarded with `DiagnoseMissingChecks`. This missing check caused diagnostics to appear when they shouldn't and caused crashes in cases where the `Diags` pointer was nullptr. To fix this the missing guard has been added and an assert that `Diags` is not null has been added. Unfortunately it isn't possible to write a regression test for this because the behavior depends on having at least some bounds checks disabled by default, which is not desirable behavior. While techinically we could add a hidden flag to change the default for the purposes of testing the added complexity of doing this doesn't really justify the added test coverage. This version of the patch also guards assigning to `Opts.BoundsSafetyBringUpMissingChecks` in `CompilerInvocation::ParseLangArgs` so that it is only assigned to when `-fbounds-safety` is enabled. This is done for several reasons * It avoids unnecessarily parsing the `-fbounds-safety-bringup-missing-checks=` flags when `-fbounds-safety` is disabled. * Isolates code built without `-fbounds-safety` from bugs in `ParseBoundsSafetyNewChecksMaskFromArgs`. * It is more consistent with `CompilerInvocationBase::GenerateLangArgs` which only emits `-fbounds-safety-bring-checks=` when `-fbounds-safety` is enabled. --- This adds warning diagnostics when any of the new bounds checks that can be enabled with `-fbounds-safety-bringup-missing-checks=batch_0` are disabled. If all bounds checks in the batch are disabled a single diagnostic is emitted. If only some of the bounds checks in the batch are disabled then a diagnostic is emitted for each disabled bounds check. The implementation will either suggest enabling a batch of checks (e.g. `-fbounds-safety-bringup-missing-checks=batch_0`) or will suggest removing a flag that is explicitly disabling a check (e.g. `-fno-bounds-safety-bringup-missing-checks=access_size`). The current implementation supports there being multple batches of checks. However, there is currently only one batch (`batch_0`). I originally tried to emit these warnings in the frontend. Unfortunately it turns out warning suppression (i.e. `-Wno-bounds-safety-legacy-checks-enabled`) and `-Werror` don't work correctly if warnings are emitted from the frontend (rdar://152730261). To workaround this the `-fbounds-safety-bringup-missing-checks=` flags are now also parsed in the Driver and at this point (and only this point) diagnostics for missing checks are emitted. The intention is to make these warnings be errors eventually. rdar://150805550 --- .../clang/Basic/DiagnosticFrontendKinds.td | 30 ++ clang/include/clang/Basic/DiagnosticGroups.td | 2 + clang/include/clang/Basic/LangOptions.h | 2 + clang/include/clang/Driver/BoundsSafetyArgs.h | 3 +- clang/lib/Driver/BoundsSafetyArgs.cpp | 130 ++++++++- clang/lib/Driver/ToolChains/Clang.cpp | 10 + clang/lib/Driver/ToolChains/Darwin.cpp | 3 +- clang/lib/Frontend/CompilerInvocation.cpp | 12 +- ...s-safety-bringup-missing-checks-disabled.c | 275 ++++++++++++++++++ .../BoundsSafetyBringupMissingChecks.cpp | 21 +- 10 files changed, 471 insertions(+), 17 deletions(-) create mode 100644 clang/test/BoundsSafety/Driver/bounds-safety-bringup-missing-checks-disabled.c diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td b/clang/include/clang/Basic/DiagnosticFrontendKinds.td index 44644ed3b7104..a45500887588a 100644 --- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td +++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td @@ -402,6 +402,36 @@ def warn_bounds_safety_adoption_mode_ignored : Warning< "-fbounds-safety-adoption-mode without -fbounds-safety is " "ignored">, InGroup; + +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; +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; // 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 index 1c1090289315f..cca6b868bc84b 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -1679,6 +1679,8 @@ 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 index 47c93ee219e7a..49c4800d37f91 100644 --- a/clang/include/clang/Basic/LangOptions.h +++ b/clang/include/clang/Basic/LangOptions.h @@ -471,6 +471,8 @@ 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; diff --git a/clang/include/clang/Driver/BoundsSafetyArgs.h b/clang/include/clang/Driver/BoundsSafetyArgs.h index 310bdf8a7faf6..15cb0e5f96ad6 100644 --- a/clang/include/clang/Driver/BoundsSafetyArgs.h +++ b/clang/include/clang/Driver/BoundsSafetyArgs.h @@ -16,7 +16,8 @@ namespace driver { LangOptions::BoundsSafetyNewChecksMaskIntTy ParseBoundsSafetyNewChecksMaskFromArgs(const llvm::opt::ArgList &Args, - DiagnosticsEngine *Diags); + DiagnosticsEngine *Diags, + bool DiagnoseMissingChecks); } // namespace driver } // namespace clang diff --git a/clang/lib/Driver/BoundsSafetyArgs.cpp b/clang/lib/Driver/BoundsSafetyArgs.cpp index 6aa72ed118f63..292e979102e6d 100644 --- a/clang/lib/Driver/BoundsSafetyArgs.cpp +++ b/clang/lib/Driver/BoundsSafetyArgs.cpp @@ -7,7 +7,11 @@ //===----------------------------------------------------------------------===// #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 using namespace llvm::opt; using namespace clang::driver::options; @@ -16,15 +20,105 @@ namespace clang { namespace driver { +static void DiagnoseDisabledBoundsSafetyChecks( + LangOptions::BoundsSafetyNewChecksMaskIntTy EnabledChecks, + DiagnosticsEngine *Diags, + LangOptions::BoundsSafetyNewChecksMaskIntTy + IndividualChecksExplicitlyDisabled) { + assert(Diags); + struct BoundsCheckBatch { + const char *Name; + LangOptionsBase::BoundsSafetyNewChecksMaskIntTy Checks; + }; + + // Batches of checks should be ordered with newest first + std::array 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) { + DiagnosticsEngine *Diags, + bool DiagnoseMissingChecks) { + assert((!DiagnoseMissingChecks || Diags) && + "Cannot diagnose missing checks when Diags is a nullptr"); + LangOptions::BoundsSafetyNewChecksMaskIntTy + IndividualChecksExplicitlyDisabled = LangOptions::BS_CHK_None; 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::getDefaultBoundsSafetyNewChecksMask(); + auto Result = LangOptions::getDefaultBoundsSafetyNewChecksMask(); + if (DiagnoseMissingChecks && Diags) + DiagnoseDisabledBoundsSafetyChecks(Result, Diags, + IndividualChecksExplicitlyDisabled); + return Result; } // If flags are present then start with BS_CHK_None as the initial mask and @@ -35,6 +129,11 @@ 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 Mask = @@ -51,17 +150,16 @@ ParseBoundsSafetyNewChecksMaskFromArgs(const llvm::opt::ArgList &Args, .Case("libc_attributes", LangOptions::BS_CHK_LibCAttributes) .Case("array_subscript_agg", LangOptions::BS_CHK_ArraySubscriptAgg) - .Case("all", - LangOptions::getBoundsSafetyNewChecksMaskForGroup("all")) - .Case( - "batch_0", - LangOptions::getBoundsSafetyNewChecksMaskForGroup("batch_0")) + .Case("all", AllChecks) + .Case("batch_0", Batch0Checks) .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; } @@ -81,6 +179,7 @@ ParseBoundsSafetyNewChecksMaskFromArgs(const llvm::opt::ArgList &Args, << A->getSpelling() << Value << (IsPosFlag ? "-fno-bounds-safety-bringup-missing-checks" : "-fbounds-safety-bringup-missing-checks"); + Errors = true; break; } @@ -91,8 +190,25 @@ 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 index 78273dad49345..8ed7997f71b04 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -25,6 +25,7 @@ #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" @@ -7641,6 +7642,15 @@ 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 index ca24959ed3a1d..9fa2d3e764c1c 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -1336,7 +1336,8 @@ void DarwinClang::addClangTargetOptions( options::OPT_fno_bounds_safety, false)) { LangOptions::BoundsSafetyNewChecksMaskIntTy NewChecks = ParseBoundsSafetyNewChecksMaskFromArgs(DriverArgs, - /*DiagnosticsEngine=*/nullptr); + /*DiagnosticsEngine=*/nullptr, + /*DiagnoseMissingChecks=*/false); 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 index c2497b08d6fd9..0a40815d84933 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -4635,9 +4635,15 @@ 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 - Opts.BoundsSafetyBringUpMissingChecks = - ParseBoundsSafetyNewChecksMaskFromArgs(Args, &Diags); + if (Opts.BoundsSafety) { + // 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. + Opts.BoundsSafetyBringUpMissingChecks = + ParseBoundsSafetyNewChecksMaskFromArgs(Args, &Diags, + /*DiagnoseMissingChecks=*/false); + } // -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 new file mode 100644 index 0000000000000..748a158e72f06 --- /dev/null +++ b/clang/test/BoundsSafety/Driver/bounds-safety-bringup-missing-checks-disabled.c @@ -0,0 +1,275 @@ +// ============================================================================= +// All checks on +// ============================================================================= + +// RUN: %clang -fbounds-safety -fbounds-safety-bringup-missing-checks=all \ +// RUN: -fsyntax-only %s 2>&1 | \ +// RUN: FileCheck --allow-empty --check-prefix=EMPTY %s + +// ============================================================================= +// batch_0 on +// ============================================================================= + +// RUN: %clang -fbounds-safety -fsyntax-only %s 2>&1 | \ +// RUN: FileCheck --check-prefix=EMPTY --allow-empty %s + +// RUN: %clang -fbounds-safety \ +// RUN: -fbounds-safety-bringup-missing-checks=batch_0 \ +// RUN: -fsyntax-only %s 2>&1 | FileCheck --check-prefix=EMPTY --allow-empty %s + +// EMPTY-NOT: warning: + +// ============================================================================= +// All checks off +// ============================================================================= + +// RUN: %clang -fbounds-safety \ +// RUN: -fno-bounds-safety-bringup-missing-checks=all \ +// RUN: -fsyntax-only %s 2>&1 | FileCheck --check-prefix=NONE %s + +// RUN: %clang -fbounds-safety \ +// RUN: -fbounds-safety-bringup-missing-checks=batch_0 \ +// RUN: -fno-bounds-safety-bringup-missing-checks=batch_0 \ +// RUN: -fsyntax-only %s 2>&1 | FileCheck --check-prefix=NONE %s + +// RUN: %clang -fbounds-safety \ +// RUN: -fbounds-safety-bringup-missing-checks=batch_0 \ +// RUN: -fno-bounds-safety-bringup-missing-checks=batch_0 \ +// RUN: -fno-bounds-safety-bringup-missing-checks=access_size \ +// RUN: -fsyntax-only %s 2>&1 | FileCheck --check-prefix=NONE %s + +// NONE: warning: compiling with legacy -fbounds-safety bounds checks is deprecated; compile with -fbounds-safety-bringup-missing-checks=batch_0 to use the new bound checks + +// ============================================================================= +// all on then one check off +// ============================================================================= + +// RUN: %clang -fbounds-safety \ +// RUN: -fbounds-safety-bringup-missing-checks=all \ +// RUN: -fno-bounds-safety-bringup-missing-checks=access_size \ +// RUN: -fsyntax-only %s 2>&1 | FileCheck --check-prefix=AS-OFF-REM-FLAG %s +// AS-OFF-REM-FLAG: warning: compiling with "access_size" bounds check disabled is deprecated; remove -fno-bounds-safety-bringup-missing-checks=access_size to enable the new bound checks + +// RUN: %clang -fbounds-safety \ +// RUN: -fbounds-safety-bringup-missing-checks=all \ +// RUN: -fno-bounds-safety-bringup-missing-checks=indirect_count_update \ +// RUN: -fsyntax-only %s 2>&1 | FileCheck --check-prefix=ICU-OFF-REM-FLAG %s +// ICU-OFF-REM-FLAG: warning: compiling with "indirect_count_update" bounds check disabled is deprecated; remove -fno-bounds-safety-bringup-missing-checks=indirect_count_update to enable the new bound checks + +// RUN: %clang -fbounds-safety \ +// RUN: -fbounds-safety-bringup-missing-checks=all \ +// RUN: -fno-bounds-safety-bringup-missing-checks=return_size \ +// RUN: -fsyntax-only %s 2>&1 | FileCheck --check-prefix=RS-OFF-REM-FLAG %s +// RS-OFF-REM-FLAG: warning: compiling with "return_size" bounds check disabled is deprecated; remove -fno-bounds-safety-bringup-missing-checks=return_size to enable the new bound checks + +// RUN: %clang -fbounds-safety \ +// RUN: -fbounds-safety-bringup-missing-checks=all \ +// RUN: -fno-bounds-safety-bringup-missing-checks=ended_by_lower_bound \ +// RUN: -fsyntax-only %s 2>&1 | FileCheck --check-prefix=EBLB-OFF-REM-FLAG %s +// EBLB-OFF-REM-FLAG: warning: compiling with "ended_by_lower_bound" bounds check disabled is deprecated; remove -fno-bounds-safety-bringup-missing-checks=ended_by_lower_bound to enable the new bound checks + +// RUN: %clang -fbounds-safety \ +// RUN: -fbounds-safety-bringup-missing-checks=all \ +// RUN: -fno-bounds-safety-bringup-missing-checks=compound_literal_init \ +// RUN: -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CLI-OFF-REM-FLAG %s +// CLI-OFF-REM-FLAG: warning: compiling with "compound_literal_init" bounds check disabled is deprecated; remove -fno-bounds-safety-bringup-missing-checks=compound_literal_init to enable the new bound checks + +// RUN: %clang -fbounds-safety \ +// RUN: -fbounds-safety-bringup-missing-checks=all \ +// RUN: -fno-bounds-safety-bringup-missing-checks=libc_attributes \ +// RUN: -fsyntax-only %s 2>&1 | FileCheck --check-prefix=LIBCA-OFF-REM-FLAG %s +// LIBCA-OFF-REM-FLAG: compiling with "libc_attributes" bounds check disabled is deprecated; remove -fno-bounds-safety-bringup-missing-checks=libc_attributes to enable the new bound checks + +// RUN: %clang -fbounds-safety \ +// RUN: -fbounds-safety-bringup-missing-checks=all \ +// RUN: -fno-bounds-safety-bringup-missing-checks=array_subscript_agg \ +// RUN: -fsyntax-only %s 2>&1 | FileCheck --check-prefix=ASA-OFF-REM-FLAG %s +// ASA-OFF-REM-FLAG: warning: compiling with "array_subscript_agg" bounds check disabled is deprecated; remove -fno-bounds-safety-bringup-missing-checks=array_subscript_agg to enable the new bound checks + + +// ============================================================================= +// Individual checks enabled +// ============================================================================= + +// RUN: %clang -fbounds-safety \ +// RUN: -fno-bounds-safety-bringup-missing-checks=all \ +// RUN: -fbounds-safety-bringup-missing-checks=access_size \ +// RUN: -fbounds-safety-bringup-missing-checks=indirect_count_update \ +// RUN: -fbounds-safety-bringup-missing-checks=return_size \ +// RUN: -fsyntax-only %s 2>&1 | \ +// RUN: FileCheck \ +// RUN: --check-prefixes=EBLB-OFF-ADD-FLAG,CLI-OFF-ADD-FLAG,LIBCA-OFF-ADD-FLAG,ASA-OFF-ADD-FLAG %s + +// RUN: %clang -fbounds-safety \ +// RUN: -fno-bounds-safety-bringup-missing-checks=all \ +// RUN: -fbounds-safety-bringup-missing-checks=ended_by_lower_bound\ +// RUN: -fbounds-safety-bringup-missing-checks=compound_literal_init \ +// RUN: -fbounds-safety-bringup-missing-checks=libc_attributes \ +// RUN: -fbounds-safety-bringup-missing-checks=array_subscript_agg \ +// RUN: -fsyntax-only %s 2>&1 | \ +// RUN: FileCheck \ +// RUN: --check-prefixes=AS-OFF-ADD-FLAG,ICU-OFF-ADD-FLAG,RS-OFF-ADD-FLAG %s + +// AS-OFF-ADD-FLAG: warning: compiling with "access_size" bounds check disabled is deprecated; compile with -fbounds-safety-bringup-missing-checks=batch_0 to enable the new bound checks +// ICU-OFF-ADD-FLAG: warning: compiling with "indirect_count_update" bounds check disabled is deprecated; compile with -fbounds-safety-bringup-missing-checks=batch_0 to enable the new bound checks +// RS-OFF-ADD-FLAG: warning: compiling with "return_size" bounds check disabled is deprecated; compile with -fbounds-safety-bringup-missing-checks=batch_0 to enable the new bound checks +// EBLB-OFF-ADD-FLAG: warning: compiling with "ended_by_lower_bound" bounds check disabled is deprecated; compile with -fbounds-safety-bringup-missing-checks=batch_0 to enable the new bound checks +// CLI-OFF-ADD-FLAG: warning: compiling with "compound_literal_init" bounds check disabled is deprecated; compile with -fbounds-safety-bringup-missing-checks=batch_0 to enable the new bound checks +// LIBCA-OFF-ADD-FLAG: warning: compiling with "libc_attributes" bounds check disabled is deprecated; compile with -fbounds-safety-bringup-missing-checks=batch_0 to enable the new bound checks +// ASA-OFF-ADD-FLAG: warning: compiling with "array_subscript_agg" bounds check disabled is deprecated; compile with -fbounds-safety-bringup-missing-checks=batch_0 to enable the new bound checks + + +// ============================================================================= +// all on then two checks off +// ============================================================================= + +// Don't be exhaustive to keep this test case smaller + +// RUN: %clang -fbounds-safety -fbounds-safety-bringup-missing-checks=all \ +// RUN: -fno-bounds-safety-bringup-missing-checks=access_size \ +// RUN: -fno-bounds-safety-bringup-missing-checks=indirect_count_update \ +// RUN: -fsyntax-only %s 2>&1 | \ +// RUN: FileCheck --check-prefixes=AS-OFF-REM-FLAG,ICU-OFF-REM-FLAG %s + +// RUN: %clang -fbounds-safety -fbounds-safety-bringup-missing-checks=all \ +// RUN: -fno-bounds-safety-bringup-missing-checks=access_size \ +// RUN: -fno-bounds-safety-bringup-missing-checks=return_size \ +// RUN: -fsyntax-only %s 2>&1 | \ +// RUN: FileCheck --check-prefixes=AS-OFF-REM-FLAG,RS-OFF-REM-FLAG %s + +// RUN: %clang -fbounds-safety -fbounds-safety-bringup-missing-checks=all \ +// RUN: -fno-bounds-safety-bringup-missing-checks=access_size \ +// RUN: -fno-bounds-safety-bringup-missing-checks=ended_by_lower_bound \ +// RUN: -fsyntax-only %s 2>&1 | \ +// RUN: FileCheck --check-prefixes=AS-OFF-REM-FLAG,EBLB-OFF-REM-FLAG %s + +// RUN: %clang -fbounds-safety -fbounds-safety-bringup-missing-checks=all \ +// RUN: -fno-bounds-safety-bringup-missing-checks=access_size \ +// RUN: -fno-bounds-safety-bringup-missing-checks=compound_literal_init \ +// RUN: -fsyntax-only %s 2>&1 | \ +// RUN: FileCheck --check-prefixes=AS-OFF-REM-FLAG,CLI-OFF-REM-FLAG %s + +// RUN: %clang -fbounds-safety -fbounds-safety-bringup-missing-checks=all \ +// RUN: -fno-bounds-safety-bringup-missing-checks=access_size \ +// RUN: -fno-bounds-safety-bringup-missing-checks=libc_attributes \ +// RUN: -fsyntax-only %s 2>&1 | \ +// RUN: FileCheck --check-prefixes=AS-OFF-REM-FLAG,LIBCA-OFF-REM-FLAG %s + +// RUN: %clang -fbounds-safety -fbounds-safety-bringup-missing-checks=all \ +// RUN: -fno-bounds-safety-bringup-missing-checks=access_size \ +// RUN: -fno-bounds-safety-bringup-missing-checks=array_subscript_agg \ +// RUN: -fsyntax-only %s 2>&1 | \ +// RUN: FileCheck --check-prefixes=AS-OFF-REM-FLAG,ASA-OFF-REM-FLAG %s + +// ============================================================================= +// batch_0 on then one check off +// ============================================================================= + +// RUN: %clang -fbounds-safety \ +// RUN: -fbounds-safety-bringup-missing-checks=batch_0 \ +// RUN: -fno-bounds-safety-bringup-missing-checks=access_size \ +// RUN: -fsyntax-only %s 2>&1 | \ +// RUN: FileCheck --check-prefixes=AS-OFF-REM-FLAG %s + +// RUN: %clang -fbounds-safety \ +// RUN: -fbounds-safety-bringup-missing-checks=batch_0 \ +// RUN: -fno-bounds-safety-bringup-missing-checks=indirect_count_update \ +// RUN: -fsyntax-only %s 2>&1 | \ +// RUN: FileCheck --check-prefixes=ICU-OFF-REM-FLAG %s + +// RUN: %clang -fbounds-safety \ +// RUN: -fbounds-safety-bringup-missing-checks=batch_0 \ +// RUN: -fno-bounds-safety-bringup-missing-checks=return_size \ +// RUN: -fsyntax-only %s 2>&1 | \ +// RUN: FileCheck --check-prefixes=RS-OFF-REM-FLAG %s + +// RUN: %clang -fbounds-safety \ +// RUN: -fbounds-safety-bringup-missing-checks=batch_0 \ +// RUN: -fno-bounds-safety-bringup-missing-checks=ended_by_lower_bound \ +// RUN: -fsyntax-only %s 2>&1 | \ +// RUN: FileCheck --check-prefixes=EBLB-OFF-REM-FLAG %s + +// RUN: %clang -fbounds-safety \ +// RUN: -fbounds-safety-bringup-missing-checks=batch_0 \ +// RUN: -fno-bounds-safety-bringup-missing-checks=compound_literal_init \ +// RUN: -fsyntax-only %s 2>&1 | \ +// RUN: FileCheck --check-prefixes=CLI-OFF-REM-FLAG %s + +// RUN: %clang -fbounds-safety \ +// RUN: -fbounds-safety-bringup-missing-checks=batch_0 \ +// RUN: -fno-bounds-safety-bringup-missing-checks=libc_attributes \ +// RUN: -fsyntax-only %s 2>&1 | \ +// RUN: FileCheck --check-prefixes=LIBCA-OFF-REM-FLAG %s + +// RUN: %clang -fbounds-safety \ +// RUN: -fbounds-safety-bringup-missing-checks=batch_0 \ +// RUN: -fno-bounds-safety-bringup-missing-checks=array_subscript_agg \ +// RUN: -fsyntax-only %s 2>&1 | \ +// RUN: FileCheck --check-prefixes=ASA-OFF-REM-FLAG %s + +// ============================================================================= +// batch_0 on then all off, then enable some +// +// In this case we shouldn't suggest removing `-fno-bounds-safety-bringup-missing-checks=` +// because it was never provided by the user. +// ============================================================================= + +// RUN: %clang -fbounds-safety \ +// RUN: -fbounds-safety-bringup-missing-checks=batch_0 \ +// RUN: -fno-bounds-safety-bringup-missing-checks=all \ +// RUN: -fbounds-safety-bringup-missing-checks=access_size \ +// RUN: -fbounds-safety-bringup-missing-checks=indirect_count_update \ +// RUN: -fbounds-safety-bringup-missing-checks=return_size \ +// RUN: -fbounds-safety-bringup-missing-checks=ended_by_lower_bound \ +// RUN: -fbounds-safety-bringup-missing-checks=compound_literal_init \ +// RUN: -fsyntax-only %s 2>&1 | \ +// RUN: FileCheck --check-prefixes=LIBCA-OFF-ADD-FLAG,ASA-OFF-ADD-FLAG %s + +// ============================================================================= +// Check the diagnostics can be made into errors +// ============================================================================= + +// RUN: not %clang -fbounds-safety \ +// RUN: -fno-bounds-safety-bringup-missing-checks=all \ +// RUN: -Werror=bounds-safety-legacy-checks-enabled %s 2>&1 | \ +// RUN: FileCheck --check-prefix=NONE-ERROR %s + +// RUN: not %clang -fbounds-safety \ +// RUN: -fno-bounds-safety-bringup-missing-checks=all \ +// RUN: -Werror %s 2>&1 | \ +// RUN: FileCheck --check-prefix=NONE-ERROR %s + +// RUN: not %clang -fbounds-safety \ +// RUN: -fno-bounds-safety-bringup-missing-checks=batch_0 \ +// RUN: -Werror %s 2>&1 | \ +// RUN: FileCheck --check-prefix=NONE-ERROR %s + +// NONE-ERROR: error: compiling with legacy -fbounds-safety bounds checks is deprecated; compile with -fbounds-safety-bringup-missing-checks=batch_0 to use the new bound checks + +// RUN: not %clang -fbounds-safety \ +// RUN: -fbounds-safety-bringup-missing-checks=batch_0 \ +// RUN: -fno-bounds-safety-bringup-missing-checks=access_size \ +// RUN: -Werror=bounds-safety-legacy-checks-enabled %s 2>&1 | \ +// RUN: FileCheck --check-prefix=AS-OFF-REM-FLAG-ERROR %s + +// AS-OFF-REM-FLAG-ERROR: error: compiling with "access_size" bounds check disabled is deprecated; remove -fno-bounds-safety-bringup-missing-checks=access_size to enable the new bound checks + +// ============================================================================= +// Check the diagnostics can be suppressed +// ============================================================================= + +// RUN: %clang -fbounds-safety \ +// RUN: -fno-bounds-safety-bringup-missing-checks=all \ +// RUN: -fsyntax-only -Werror \ +// RUN: -Wno-bounds-safety-legacy-checks-enabled %s + +// RUN: %clang -fbounds-safety \ +// RUN: -fno-bounds-safety-bringup-missing-checks=batch_0 \ +// RUN: -fsyntax-only -Werror \ +// RUN: -Wno-bounds-safety-legacy-checks-enabled %s + +// RUN: %clang -fbounds-safety \ +// RUN: -fbounds-safety-bringup-missing-checks=batch_0 \ +// RUN: -fno-bounds-safety-bringup-missing-checks=access_size \ +// RUN: -fsyntax-only -Werror \ +// RUN: -Wno-bounds-safety-legacy-checks-enabled %s diff --git a/clang/unittests/Tooling/BoundsSafetyBringupMissingChecks.cpp b/clang/unittests/Tooling/BoundsSafetyBringupMissingChecks.cpp index 9c3cc7548b13e..7613f990b878d 100644 --- a/clang/unittests/Tooling/BoundsSafetyBringupMissingChecks.cpp +++ b/clang/unittests/Tooling/BoundsSafetyBringupMissingChecks.cpp @@ -198,15 +198,20 @@ TEST(BoundsSafetyBringUpMissingChecks, ChkPairValidMask) { // Default behavior // ============================================================================= +// Note: Use of ` "-Wno-bounds-safety-legacy-checks-enabled"` in tests is to +// make the output of running the unit test less noisy. + TEST(BoundsSafetyBringUpMissingChecks, DefaultWithBoundsSafety) { bool Result = runOnToolAndCheckLangOptions( - {NEED_CC1_ARG "-fbounds-safety"}, [](LangOptions &LO) { + {NEED_CC1_ARG "-fbounds-safety", + "-Wno-bounds-safety-legacy-checks-enabled"}, + [](LangOptions &LO) { EXPECT_EQ(LO.BoundsSafetyBringUpMissingChecks, LangOptions::getDefaultBoundsSafetyNewChecksMask()); - // "batch_0" is the default. - EXPECT_EQ(LO.BoundsSafetyBringUpMissingChecks, - LangOptions::getBoundsSafetyNewChecksMaskForGroup("batch_0")); + LangOptionsBase::BoundsSafetyNewChecksMaskIntTy DefaultCheckMask = + LangOptions::getBoundsSafetyNewChecksMaskForGroup("batch_0"); + EXPECT_EQ(LO.BoundsSafetyBringUpMissingChecks, DefaultCheckMask); }); ASSERT_TRUE(Result); } @@ -298,6 +303,7 @@ TEST(BoundsSafetyBringUpMissingChecks, only_one_check) { for (size_t ChkIdx = 0; ChkIdx < NumChkDescs; ++ChkIdx) { ChkDesc Chk = CheckKinds[ChkIdx]; std::vector Args = {NEED_CC1_ARG "-fbounds-safety", + "-Wno-bounds-safety-legacy-checks-enabled", NEED_CC1_ARG "-fbounds-safety-bringup-missing-checks="}; Args[Args.size() - 1].append(Chk.arg); @@ -333,6 +339,7 @@ TEST(BoundsSafetyBringUpMissingChecks, all_pairs) { auto Second = CheckKinds[secondIdx]; std::vector Args = { NEED_CC1_ARG "-fbounds-safety", + "-Wno-bounds-safety-legacy-checks-enabled", NEED_CC1_ARG "-fbounds-safety-bringup-missing-checks="}; Args[Args.size() - 1].append(First.arg); Args[Args.size() - 1].append(","); @@ -368,6 +375,7 @@ TEST(BoundsSafetyBringUpMissingChecks, all_with_one_removed) { ChkDesc Chk = CheckKinds[ChkIdx]; std::vector Args = { NEED_CC1_ARG "-fbounds-safety", + "-Wno-bounds-safety-legacy-checks-enabled", NEED_CC1_ARG "-fbounds-safety-bringup-missing-checks=all", NEED_CC1_ARG "-fno-bounds-safety-bringup-missing-checks="}; Args[Args.size() - 1].append(Chk.arg); @@ -400,6 +408,7 @@ TEST(BoundsSafetyBringUpMissingChecks, batch_0_with_one_removed) { std::vector Args = { NEED_CC1_ARG "-fbounds-safety", + "-Wno-bounds-safety-legacy-checks-enabled", NEED_CC1_ARG "-fbounds-safety-bringup-missing-checks=batch_0", NEED_CC1_ARG "-fno-bounds-safety-bringup-missing-checks="}; Args[Args.size() - 1].append(Chk.arg); @@ -433,6 +442,7 @@ TEST(BoundsSafetyBringUpMissingChecks, batch_0_with_one_removed) { TEST(BoundsSafetyBringUpMissingChecks, all_disabled) { bool Result = runOnToolAndCheckLangOptions( {NEED_CC1_ARG "-fbounds-safety", + "-Wno-bounds-safety-legacy-checks-enabled", NEED_CC1_ARG "-fno-bounds-safety-bringup-missing-checks"}, [](LangOptions &LO) { EXPECT_EQ(LO.BoundsSafetyBringUpMissingChecks, @@ -444,6 +454,7 @@ TEST(BoundsSafetyBringUpMissingChecks, all_disabled) { TEST(BoundsSafetyBringUpMissingChecks, all_enable_then_disable) { bool Result = runOnToolAndCheckLangOptions( {NEED_CC1_ARG "-fbounds-safety", + "-Wno-bounds-safety-legacy-checks-enabled", NEED_CC1_ARG "-fbounds-safety-bringup-missing-checks", NEED_CC1_ARG "-fno-bounds-safety-bringup-missing-checks"}, [](LangOptions &LO) { @@ -462,6 +473,7 @@ TEST(BoundsSafetyBringUpMissingChecks, all_disabled_then_enable_one) { ChkDesc Chk = CheckKinds[ChkIdx]; std::vector Args = { NEED_CC1_ARG "-fbounds-safety", + "-Wno-bounds-safety-legacy-checks-enabled", NEED_CC1_ARG "-fno-bounds-safety-bringup-missing-checks", NEED_CC1_ARG "-fbounds-safety-bringup-missing-checks="}; Args[Args.size() - 1].append(Chk.arg); @@ -482,4 +494,3 @@ TEST(BoundsSafetyBringUpMissingChecks, all_disabled_then_enable_one) { ASSERT_TRUE(Result); } } -