Skip to content

Commit 00f49f1

Browse files
authored
Warning for when bounds declarations are not provably true. (#343)
This adds a warning message for when bounds declarations are not provably true. The warning is off by default because we cannot prove much yet about bounds declarations. This addresses work item #338. We test the error message by adding checking of bounds declarations after assignments. We handle a few basic cases where the declared bounds for the target variable are implied by the inferred bounds of the source expression, that is a check for subsumption of bounds. Given e1 = e2, we allow the cases where: - the declared bounds of e1 and inferred bounds of e2 are syntactically equal. - the declared bounds of e1 is bounds(none), in which case any inferred bounds works for e2. - the inferred bounds of e2 is bounds(any). Testing: - Added two new test cases. In one case, the bounds has syntactically identical. In another case, they are not syntactically identical because of the way that 'count' expands, so we produce a warning. When we extend the bounds subsumption check to understand facts about variables being equal, the second test case should no longer produce a warning.
1 parent 42aa4e4 commit 00f49f1

File tree

3 files changed

+63
-3
lines changed

3 files changed

+63
-3
lines changed

include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9351,6 +9351,17 @@ def err_bounds_type_annotation_lost_checking : Error<
93519351
def err_bounds_cast_error_with_array_syntax
93529352
: Error<"invalid bounds cast: expected _Array_ptr type">;
93539353

9354+
def warn_bounds_declaration_not_true : Warning<
9355+
"bounds declaration for '%0' may not be true after assignment">,
9356+
DefaultIgnore,
9357+
InGroup<DiagGroup<"check-bounds-decls">>;
9358+
9359+
def note_declared_bounds_for_expr : Note<
9360+
"declared bounds for '%0' are '%1'">;
9361+
9362+
def note_inferred_bounds_for_expr : Note<
9363+
"inferred bounds for right-hand side are '%0'">;
9364+
93549365
} // end of Checked C Category
93559366

93569367
} // end of sema component.

lib/Sema/SemaBounds.cpp

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1113,6 +1113,33 @@ namespace {
11131113
return false;
11141114
}
11151115

1116+
// Given an assignment target = e, where target has declared bounds
1117+
// DeclaredBounds and and e has inferred bounds SrcBounds, make sure
1118+
// that SrcBounds implies that DeclaredBounds is provably true.
1119+
void CheckBoundsDeclIsProvable(SourceLocation ExprLoc, Expr *Target,
1120+
BoundsExpr *DeclaredBounds, Expr *Src,
1121+
BoundsExpr *SrcBounds) {
1122+
if (S.Diags.isIgnored(diag::warn_bounds_declaration_not_true, ExprLoc))
1123+
return;
1124+
1125+
// source bounds(any) implies that any other bounds is valid.
1126+
if (SrcBounds->isAny())
1127+
return;
1128+
1129+
// target bounds(none) implied by any other bounds.
1130+
if (DeclaredBounds->isNone())
1131+
return;
1132+
1133+
if (!S.Context.EquivalentBounds(DeclaredBounds, SrcBounds)) {
1134+
S.Diag(ExprLoc, diag::warn_bounds_declaration_not_true) << Target
1135+
<< Target->getSourceRange() << Src->getSourceRange();
1136+
S.Diag(Target->getExprLoc(), diag::note_declared_bounds_for_expr) <<
1137+
Target << DeclaredBounds << Target->getSourceRange();
1138+
S.Diag(Src->getExprLoc(), diag::note_inferred_bounds_for_expr) <<
1139+
SrcBounds << Src->getSourceRange();
1140+
}
1141+
}
1142+
11161143
public:
11171144
CheckBoundsDeclarations(Sema &S) : S(S),
11181145
DumpBounds(S.getLangOpts().DumpInferredBounds) {}
@@ -1145,9 +1172,13 @@ namespace {
11451172
else
11461173
RHSBounds = S.InferRValueBounds(RHS);
11471174
if (RHSBounds->isNone()) {
1148-
S.Diag(RHS->getLocStart(), diag::err_expected_bounds_for_assignment) << RHS->getSourceRange();
1175+
S.Diag(RHS->getLocStart(),
1176+
diag::err_expected_bounds_for_assignment)
1177+
<< RHS->getSourceRange();
11491178
RHSBounds = S.CreateInvalidBoundsExpr();
1150-
}
1179+
} else
1180+
CheckBoundsDeclIsProvable(E->getExprLoc(), LHS, LHSTargetBounds,
1181+
RHS, RHSBounds);
11511182
}
11521183
}
11531184

@@ -1200,7 +1231,9 @@ namespace {
12001231
BoundsExpr *SrcBounds =
12011232
S.InferRValueBounds(E->getSubExpr());
12021233
if (SrcBounds->isNone()) {
1203-
S.Diag(E->getSubExpr()->getLocStart(), diag::err_expected_bounds_for_ptr_cast) << E->getSubExpr()->getSourceRange();
1234+
S.Diag(E->getSubExpr()->getLocStart(),
1235+
diag::err_expected_bounds_for_ptr_cast)
1236+
<< E->getSubExpr()->getSourceRange();
12041237
SrcBounds = S.CreateInvalidBoundsExpr();
12051238
}
12061239
assert(SrcBounds);

test/CheckedC/bounds-decl-checking.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Tests for checking that bounds declarations hold after assignments to
2+
// variables and initialization of variables. Because the static checker is
3+
// mostly unimplemented, we only issue warnings when bounds declarations
4+
// cannot be provided to hold.
5+
//
6+
// RUN: %clang -cc1 -fcheckedc-extension -Wcheck-bounds-decls -verify -verify-ignore-unexpected=note %s
7+
8+
void f1(_Array_ptr<int> p : bounds(p, p + x), int x) {
9+
_Array_ptr<int> r : bounds(p, p + x) = 0;
10+
r = p;
11+
}
12+
13+
void f2(_Array_ptr<int> p : count(x), int x) {
14+
_Array_ptr<int> r : count(x) = 0;
15+
r = p; // expected-warning {{may not be true}}
16+
}

0 commit comments

Comments
 (0)