Skip to content

Commit 871ae81

Browse files
delcypherdelcypher
committed
[BoundsSafety] Add warning diagnostics for uses of legacy bounds checks
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 current implementation supports there being multple batches of checks. However, there is currently only one batch (`batch_0`). Unfortunately I've discovered these diagnostics don't respect `-Werror` or `-Wno-bounds-safety-legacy-checks-enabled`. Fixing that is out-of-scope for this patch and is tracked by rdar://152730261. The intention is to make these warnings be errors eventually. rdar://150805550
1 parent 8011632 commit 871ae81

File tree

5 files changed

+200
-0
lines changed

5 files changed

+200
-0
lines changed

‎clang/include/clang/Basic/DiagnosticFrontendKinds.td

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,25 @@ def warn_bounds_safety_adoption_mode_ignored : Warning<
402402
"-fbounds-safety-adoption-mode without -fbounds-safety is "
403403
"ignored">,
404404
InGroup<BoundsSafetyAdoptionModeIgnored>;
405+
406+
def warn_bounds_safety_new_checks_none : Warning<
407+
"compiling with legacy -fbounds-safety bounds checks is deprecated;"
408+
" compile with -fbounds-safety-bringup-missing-checks=%0 to use the "
409+
"new bound checks">,
410+
InGroup<BoundsSafetyLegacyChecksEnabled>;
411+
def warn_bounds_safety_new_checks_mixed : Warning<
412+
"compiling with \"%select{"
413+
"access_size|" // BS_CHK_AccessSize
414+
"indirect_count_update|" // BS_CHK_IndirectCountUpdate
415+
"return_size|" // BS_CHK_ReturnSize
416+
"ended_by_lower_bound|" // BS_CHK_EndedByLowerBound
417+
"compound_literal_init|" // BS_CHK_CompoundLiteralInit
418+
"libc_attributes|" // BS_CHK_LibCAttributes
419+
"array_subscript_agg" // BS_CHK_ArraySubscriptAgg
420+
"}0\" bounds check disabled is deprecated;"
421+
" compile with -fbounds-safety-bringup-missing-checks=%1 to enable the "
422+
"new bound checks">,
423+
InGroup<BoundsSafetyLegacyChecksEnabled>;
405424
// TO_UPSTREAM(BoundsSafety) OFF
406425

407426
let CategoryName = "Instrumentation Issue" in {

‎clang/include/clang/Basic/DiagnosticGroups.td

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1670,6 +1670,8 @@ def BoundsSafetyStrictTerminatedByCast
16701670
: DiagGroup<"bounds-safety-strict-terminated-by-cast">;
16711671
def BoundsSafetyCountAttrArithConstantCount :
16721672
DiagGroup<"bounds-safety-externally-counted-ptr-arith-constant-count">;
1673+
def BoundsSafetyLegacyChecksEnabled :
1674+
DiagGroup<"bounds-safety-legacy-checks-enabled">;
16731675
// TO_UPSTREAM(BoundsSafety) OFF
16741676

16751677
// -fbounds-safety and bounds annotation related warnings

‎clang/include/clang/Basic/LangOptions.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,8 @@ class LangOptionsBase {
454454
BS_CHK_CompoundLiteralInit = 1 << 4, // rdar://110871666
455455
BS_CHK_LibCAttributes = 1 << 5, // rdar://84733153
456456
BS_CHK_ArraySubscriptAgg = 1 << 6, // rdar://145020583
457+
458+
BS_CHK_MaxMask = BS_CHK_ArraySubscriptAgg,
457459
};
458460
using BoundsSafetyNewChecksMaskIntTy =
459461
std::underlying_type_t<BoundsSafetyNewChecks>;

‎clang/lib/Frontend/CompilerInvocation.cpp

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3961,6 +3961,78 @@ static StringRef GetInputKindName(InputKind IK) {
39613961
llvm_unreachable("unknown input language");
39623962
}
39633963

3964+
/* TO_UPSTREAM(BoundsSafety) ON*/
3965+
static void DiagnoseDisabledBoundsSafetyChecks(const LangOptions &Opts,
3966+
DiagnosticsEngine &Diags) {
3967+
struct BoundsCheckBatch {
3968+
const char *Name;
3969+
LangOptionsBase::BoundsSafetyNewChecksMaskIntTy Checks;
3970+
};
3971+
3972+
// Batches of checks should be ordered with newest first
3973+
BoundsCheckBatch Batches[] = {
3974+
// We deliberately don't include `all` here because that batch
3975+
// isn't stable over time (unlike batches like `batch_0`) so we
3976+
// don't want to suggest users start using it.
3977+
{"batch_0", LangOptions::getBoundsSafetyNewChecksMaskForGroup("batch_0")},
3978+
{"none", LangOptions::BS_CHK_None}};
3979+
3980+
ArrayRef<BoundsCheckBatch> BatchesAR(Batches);
3981+
LangOptionsBase::BoundsSafetyNewChecksMaskIntTy DiagnosedDisabledChecks =
3982+
LangOptions::BS_CHK_None;
3983+
3984+
// Loop over all batches except "none"
3985+
for (size_t BatchIdx = 0; BatchIdx < BatchesAR.size() - 1; ++BatchIdx) {
3986+
auto ChecksInCurrentBatch = BatchesAR[BatchIdx].Checks;
3987+
auto ChecksInOlderBatch = BatchesAR[BatchIdx + 1].Checks;
3988+
auto ChecksInCurrentBatchOnly = ChecksInCurrentBatch & ~ChecksInOlderBatch;
3989+
3990+
if ((Opts.BoundsSafetyBringUpMissingChecks & ChecksInCurrentBatchOnly) ==
3991+
ChecksInCurrentBatchOnly)
3992+
continue; // All checks in batch are enabled. Nothing to diagnose.
3993+
3994+
// Diagnose disabled bounds checks
3995+
3996+
if ((Opts.BoundsSafetyBringUpMissingChecks & ChecksInCurrentBatchOnly) ==
3997+
0) {
3998+
// None of the checks in the current batch are enabled. Diagnose this
3999+
// once for all the checks in the batch.
4000+
if ((DiagnosedDisabledChecks & ChecksInCurrentBatchOnly) !=
4001+
ChecksInCurrentBatchOnly) {
4002+
Diags.Report(diag::warn_bounds_safety_new_checks_none)
4003+
<< BatchesAR[BatchIdx].Name;
4004+
DiagnosedDisabledChecks |= ChecksInCurrentBatchOnly;
4005+
continue;
4006+
}
4007+
continue;
4008+
}
4009+
4010+
// Some (but not all) checks are disabled in the current batch. Iterate over
4011+
// each check in the batch and emit a diagnostic for each disabled check
4012+
// in the batch.
4013+
assert(ChecksInCurrentBatch > 0);
4014+
auto FirstCheckInBatch = 1 << __builtin_ctz(ChecksInCurrentBatch);
4015+
for (size_t CheckBit = FirstCheckInBatch;
4016+
CheckBit <= LangOptions::BS_CHK_MaxMask; CheckBit <<= 1) {
4017+
assert(CheckBit != 0);
4018+
if ((CheckBit & ChecksInCurrentBatch) == 0)
4019+
continue; // Check not in batch
4020+
4021+
if (Opts.BoundsSafetyBringUpMissingChecks & CheckBit)
4022+
continue; // Check is active
4023+
4024+
// Diagnose disabled check
4025+
if (!(DiagnosedDisabledChecks & CheckBit)) {
4026+
size_t CheckNumber = __builtin_ctz(CheckBit);
4027+
Diags.Report(diag::warn_bounds_safety_new_checks_mixed)
4028+
<< CheckNumber << BatchesAR[BatchIdx].Name;
4029+
DiagnosedDisabledChecks |= CheckBit;
4030+
}
4031+
}
4032+
}
4033+
}
4034+
/* TO_UPSTREAM(BoundsSafety) OFF*/
4035+
39644036
void CompilerInvocationBase::GenerateLangArgs(const LangOptions &Opts,
39654037
ArgumentConsumer Consumer,
39664038
const llvm::Triple &T,
@@ -4609,6 +4681,8 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
46094681
}
46104682
if (!SupportsBoundsSafety)
46114683
Diags.Report(diag::error_bounds_safety_lang_not_supported);
4684+
4685+
DiagnoseDisabledBoundsSafetyChecks(Opts, Diags);
46124686
}
46134687

46144688
bool IsBoundsSafetyAttributesExplicitlyDisabled =

‎clang/test/BoundsSafety/Frontend/bounds-safety-bringup-missing-chekcs-disabled.c

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
// =============================================================================
2+
// All checks on
3+
// =============================================================================
4+
5+
// RUN: %clang_cc1 -fbounds-safety -fbounds-safety-bringup-missing-checks=all -fsyntax-only %s -verify=all
6+
7+
// all-no-diagnostics
8+
9+
// =============================================================================
10+
// batch_0 on
11+
// =============================================================================
12+
13+
// RUN: %clang_cc1 -fbounds-safety -fsyntax-only %s -verify=batch_0
14+
// RUN: %clang_cc1 -fbounds-safety -fbounds-safety-bringup-missing-checks=batch_0 -fsyntax-only %s -verify=batch_0
15+
16+
// batch_0-no-diagnostics
17+
18+
// =============================================================================
19+
// All checks off
20+
// =============================================================================
21+
22+
// RUN: %clang_cc1 -fbounds-safety -fno-bounds-safety-bringup-missing-checks=all -fsyntax-only %s -verify=none
23+
24+
// 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}}
25+
26+
27+
// =============================================================================
28+
// one check off
29+
// =============================================================================
30+
31+
// RUN: %clang_cc1 -fbounds-safety -fbounds-safety-bringup-missing-checks=all -fno-bounds-safety-bringup-missing-checks=access_size -fsyntax-only %s -verify=as-off
32+
// as-off-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}}
33+
34+
// RUN: %clang_cc1 -fbounds-safety -fbounds-safety-bringup-missing-checks=all -fno-bounds-safety-bringup-missing-checks=indirect_count_update -fsyntax-only %s -verify=icu-off
35+
// icu-off-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}}
36+
37+
// RUN: %clang_cc1 -fbounds-safety -fbounds-safety-bringup-missing-checks=all -fno-bounds-safety-bringup-missing-checks=return_size -fsyntax-only %s -verify=rs-off
38+
// rs-off-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}}
39+
40+
// RUN: %clang_cc1 -fbounds-safety -fbounds-safety-bringup-missing-checks=all -fno-bounds-safety-bringup-missing-checks=ended_by_lower_bound -fsyntax-only %s -verify=eblb-off
41+
// eblb-off-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}}
42+
43+
// RUN: %clang_cc1 -fbounds-safety -fbounds-safety-bringup-missing-checks=all -fno-bounds-safety-bringup-missing-checks=compound_literal_init -fsyntax-only %s -verify=cli-off
44+
// cli-off-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}}
45+
46+
// RUN: %clang_cc1 -fbounds-safety -fbounds-safety-bringup-missing-checks=all -fno-bounds-safety-bringup-missing-checks=libc_attributes -fsyntax-only %s -verify=libca-off
47+
// libca-off-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}}
48+
49+
// RUN: %clang_cc1 -fbounds-safety -fbounds-safety-bringup-missing-checks=all -fno-bounds-safety-bringup-missing-checks=array_subscript_agg -fsyntax-only %s -verify=asa-off
50+
// asa-off-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}}
51+
52+
// =============================================================================
53+
// two checks off
54+
// =============================================================================
55+
56+
// Don't be exhaustive to keep this test case smaller
57+
58+
// RUN: %clang_cc1 -fbounds-safety -fbounds-safety-bringup-missing-checks=all \
59+
// RUN: -fno-bounds-safety-bringup-missing-checks=access_size \
60+
// RUN: -fno-bounds-safety-bringup-missing-checks=indirect_count_update \
61+
// RUN: -fsyntax-only %s -verify=as-off,icu-off
62+
63+
// RUN: %clang_cc1 -fbounds-safety -fbounds-safety-bringup-missing-checks=all \
64+
// RUN: -fno-bounds-safety-bringup-missing-checks=access_size \
65+
// RUN: -fno-bounds-safety-bringup-missing-checks=return_size \
66+
// RUN: -fsyntax-only %s -verify=as-off,rs-off
67+
68+
// RUN: %clang_cc1 -fbounds-safety -fbounds-safety-bringup-missing-checks=all \
69+
// RUN: -fno-bounds-safety-bringup-missing-checks=access_size \
70+
// RUN: -fno-bounds-safety-bringup-missing-checks=ended_by_lower_bound \
71+
// RUN: -fsyntax-only %s -verify=as-off,eblb-off
72+
73+
// RUN: %clang_cc1 -fbounds-safety -fbounds-safety-bringup-missing-checks=all \
74+
// RUN: -fno-bounds-safety-bringup-missing-checks=access_size \
75+
// RUN: -fno-bounds-safety-bringup-missing-checks=compound_literal_init \
76+
// RUN: -fsyntax-only %s -verify=as-off,cli-off
77+
78+
// RUN: %clang_cc1 -fbounds-safety -fbounds-safety-bringup-missing-checks=all \
79+
// RUN: -fno-bounds-safety-bringup-missing-checks=access_size \
80+
// RUN: -fno-bounds-safety-bringup-missing-checks=libc_attributes \
81+
// RUN: -fsyntax-only %s -verify=as-off,libca-off
82+
83+
// RUN: %clang_cc1 -fbounds-safety -fbounds-safety-bringup-missing-checks=all \
84+
// RUN: -fno-bounds-safety-bringup-missing-checks=access_size \
85+
// RUN: -fno-bounds-safety-bringup-missing-checks=array_subscript_agg \
86+
// RUN: -fsyntax-only %s -verify=as-off,asa-off
87+
88+
89+
// =============================================================================
90+
// Check the diagnostics can be made into errors
91+
// =============================================================================
92+
93+
// FIXME: This doesn't work right now because
94+
// `TextDiagnosticBuffer::FlushDiagnostics` doesn't preserve diagnostic IDs when
95+
// flushing diagnostics (rdar://152730261)
96+
97+
// =============================================================================
98+
// Check the diagnostics can be suppressed
99+
// =============================================================================
100+
101+
// FIXME: This doesn't work right now because
102+
// `TextDiagnosticBuffer::FlushDiagnostics` doesn't preserve diagnostic IDs when
103+
// flushing diagnostics (rdar://152730261)

0 commit comments

Comments
 (0)