Skip to content

Commit

Permalink
Get variable and constant from upper bounds (#1135)
Browse files Browse the repository at this point in the history 
* Add NormalizeUtils.h and NormalizeUtils.cpp

* Add NormalizeUtil::AddExprs helper method

* Add NormalizeUtil::TransformAdditiveOp method

* Fix typos

* Add ExprCreatorUtil::CreateUnaryOperator method

* Add NormalizeUtil::GetAdditionOperands helper method

* Rename variable in NormalizeUtil::TransformSingleAdditiveOp

* Add ExprUtil::EnsureEqualBitWidths method

* Add NormalizeUtil::GetRHSConstant helper method

* Add NormalizeUtil::TransformAssocLeft method

* Add NormalizeUtil::ConstantFold method

* Remove ConstantFoldUpperOffsets, GetRHSConstant, and EnsureEqualBitWidths methods from BaseRange and call utility methods instead

* Fix typos

* Avoid creating an unnecessary binary operator in TransformAdditiveOp

* Return argument expression from TransformAssocLeft if the argument is already in the output form

* Add NormalizeUpperBound method to CheckBoundsDeclarations

* Add CompareNormalizeBounds method to CheckBoundsDeclarations

* Remove expected warning from bounds widening test in bounds-context.c

* Add tests for comparing normalized bounds to bounds-decl-checking.c

* Move declaration of PointerAndConst

* Add comment explaining why we don't check for B - P

* Add NormalizeUtil::QueryPointerAdditiveConstant helper method

* Add NormalizeUtil::AddConstants helper method

* Add NormalizeUtil::GetVariableAndConstant method

* Call NormalizeUtil::GetVariableAndConstant from CheckBoundsDeclarations::CompareNormalizedBounds

* Remove CheckBoundsDeclarations::NormalizeUpperBound method

* Add and update bounds checking tests

* Don't call TransformAdditiveOp from GetVariableAndConstant

* Remove unused NormalizeUtil::GetAdditionOperands method

* Fix formatting
  • Loading branch information
@kkjeer
kkjeer authored Jul 24, 2021
1 parent 87d1dc7 commit 8d3c184
Show file tree
Hide file tree
Showing 4 changed files with 166 additions and 112 deletions.
50 changes: 47 additions & 3 deletions clang/include/clang/AST/NormalizeUtils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,38 @@ class NormalizeUtil {
// 3. E3 has integer type.
static Expr *TransformAssocLeft(Sema &S, Expr *E);

// GetVariableAndConstant attempts to extract a Variable part and a Constant
// part from E.
//
// If E can be written as C1 +/- C2 +/- ... +/- Cj + E1 +/- Ck +/- ... +/- Cn,
// where:
// 1. E1 is a pointer-typed expression, and:
// 2. Each Ci is an optional integer constant, and:
// 3. C1 +/- ... +/- Cn does not overflow
// then:
// GetVariableAndConstant returns true and sets:
// 1. Variable = E1
// 2. Constant = C1 +/- ... +/- Cn
//
// GetVariableAndConstant applies left-associativity if possible.
// If the pointer-typed subexpression E1 of E is of the form p + (i + j),
// where p is a pointer and i and j are integers, then E1 is rewritten as
// (p + i) + j. If j is an integer constant, this allows
// GetVariableAndConstant to add j to the Constant part, and set the
// Variable part to p + i.
//
// Some examples:
// 1. If E is of the form (p + (i + 1)) + 2:
// Variable = p + i, Constant = 3.
// 2. If E is of the form (p + (1 + i)) + 2:
// Variable = (p + 1) + i, Constant = 2.
// 3. If E is of the form 1 + (2 + (p + (i + 3))):
// Variable = p + i, Constant = 6.
// 4. If E is of the form 1 + (2 + (i + (p + 3))):
// Variable = i + (p + 3), Constant = 3.
static bool GetVariableAndConstant(Sema &S, Expr *E, Expr *&Variable,
llvm::APSInt &Constant);

// ConstantFold performs simple constant folding operations on E.
// It attempts to extract a Variable part and a Constant part, based
// on the form of E.
Expand Down Expand Up @@ -75,16 +107,28 @@ class NormalizeUtil {
// AddExprs returns LHS + RHS.
static Expr *AddExprs(Sema &S, Expr *LHS, Expr *RHS);

// If E is of the form E1 + E2, GetAdditionOperands returns true
// and sets LHS to E1 and RHS to E2.
static bool GetAdditionOperands(Expr *E, Expr *&LHS, Expr *&RHS);
// AddConstants sets C1 to C1 + C2 and returns true if C1 + C2 does not
// overflow.
static bool AddConstants(llvm::APSInt &C1, llvm::APSInt C2);

// If E is of the form E1 +/- C, where C is a constant, GetRHSConstant
// returns true and sets the out parameter Constant.
// If E is of the form E1 + C, Constant will be set to C.
// If E is of the form E1 - C, Constant will be set to -C.
static bool GetRHSConstant(Sema &S, BinaryOperator *E, QualType T,
llvm::APSInt &Constant);

// If E is of the form P +/- C or C + P, where P is a pointer-typed
// expression and C is an integer constant, QueryPointerAdditiveConstant
// returns true and sets:
// 1. PointerExpr = P
// 2. Constant = C if E is of the form P + C or C + P
// 3. Constant = -C if E is of the form P - C
// Note that E cannot be of the form C - P, since a pointer cannot
// appear on the right-hand side of a subtraction operator.
static bool QueryPointerAdditiveConstant(Sema &S, Expr *E,
Expr *&PointerExpr,
llvm::APSInt &Constant);
};

} // end namespace clang
Expand Down
106 changes: 94 additions & 12 deletions clang/lib/AST/NormalizeUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,14 +103,56 @@ Expr *NormalizeUtil::TransformAssocLeft(Sema &S, Expr *E) {
// If E is of the form E1 + (E2 + E3), output expression is (E1 + E2) + E3.
if (RHSBinOp->getOpcode() == BinaryOperatorKind::BO_Add)
return AddExprs(S, AddExprs(S, E1, E2), E3);

// If E is of the form E1 + (E2 - E3), output expression is (E1 + E2) - E3.
else if (RHSBinOp->getOpcode() == BinaryOperatorKind::BO_Sub) {
return ExprCreatorUtil::CreateBinaryOperator(S, AddExprs(S, E1, E2), E3, BinaryOperatorKind::BO_Sub);
}
else if (RHSBinOp->getOpcode() == BinaryOperatorKind::BO_Sub)
return ExprCreatorUtil::CreateBinaryOperator(S, AddExprs(S, E1, E2), E3,
BinaryOperatorKind::BO_Sub);

return nullptr;
}

bool NormalizeUtil::GetVariableAndConstant(Sema &S, Expr *E, Expr *&Variable,
llvm::APSInt &Constant) {
// While possible, split E into a PointerExpr and an integer constant C
// and add C to Constant.
Expr *PointerExpr = E;
llvm::APSInt C;
bool GotPointerAndConst = QueryPointerAdditiveConstant(S, PointerExpr,
PointerExpr, C);
while (GotPointerAndConst) {
if (!AddConstants(Constant, C))
goto exit;
GotPointerAndConst = QueryPointerAdditiveConstant(S, PointerExpr,
PointerExpr, C);
}

if (!PointerExpr)
goto exit;

// If PointerExpr is of the form p + (i +/- j) (where p is a pointer and
// i and j are integers), rewrite PointerExpr as (p + i) +/- j.
if (Expr *AssocLeft = TransformAssocLeft(S, PointerExpr))
PointerExpr = AssocLeft;

// After transforming subtraction operators and applying left-associativity,
// we might be able to get another integer constant from PointerExpr.
// If so, add it to Constant if the addition does not overflow.
if (QueryPointerAdditiveConstant(S, PointerExpr, PointerExpr, C))
if (!AddConstants(Constant, C))
goto exit;

Variable = PointerExpr;
return true;

exit:
// Return (E, 0).
Variable = E;
uint64_t PointerWidth = S.Context.getTargetInfo().getPointerWidth(0);
Constant = llvm::APSInt(PointerWidth, false);
return false;
}

// Input form: (E1 +/- A) +/- B.
// Outputs: Variable: E1, Constant: (+/-)A + (+/-)B.
bool NormalizeUtil::ConstantFold(Sema &S, Expr *E, QualType T, Expr *&Variable,
Expand Down Expand Up @@ -179,15 +221,11 @@ Expr *NormalizeUtil::AddExprs(Sema &S, Expr *LHS, Expr *RHS) {
BinaryOperatorKind::BO_Add);
}

bool NormalizeUtil::GetAdditionOperands(Expr *E, Expr *&LHS, Expr *&RHS) {
BinaryOperator *BO = dyn_cast<BinaryOperator>(E->IgnoreParens());
if (!BO)
return false;
if (BO->getOpcode() != BinaryOperatorKind::BO_Add)
return false;
LHS = BO->getLHS();
RHS = BO->getRHS();
return true;
bool NormalizeUtil::AddConstants(llvm::APSInt &C1, llvm::APSInt C2) {
ExprUtil::EnsureEqualBitWidths(C1, C2);
bool Overflow;
C1 = C1.sadd_ov(C2, Overflow);
return !Overflow;
}

bool NormalizeUtil::GetRHSConstant(Sema &S, BinaryOperator *E, QualType T,
Expand Down Expand Up @@ -217,3 +255,47 @@ bool NormalizeUtil::GetRHSConstant(Sema &S, BinaryOperator *E, QualType T,

return true;
}

bool NormalizeUtil::QueryPointerAdditiveConstant(Sema &S, Expr *E,
Expr *&PointerExpr,
llvm::APSInt &Constant) {
BinaryOperator *BO = dyn_cast<BinaryOperator>(E->IgnoreParens());
if (!BO)
return false;
if (!BO->isAdditiveOp())
return false;

// E must be of the form PointerExpr +/- Constant or Constant + PointerExpr,
// where PointerExpr has pointer type and Constant is an integer constant.
// Note that E cannot be of the form Constant - PointerExpr, since a pointer
// cannot appear on the right-hand side of a subtraction operator.
if (BO->getLHS()->getType()->isPointerType() &&
BO->getRHS()->isIntegerConstantExpr(Constant, S.Context))
PointerExpr = BO->getLHS();
else if (BO->getOpcode() == BinaryOperatorKind::BO_Add &&
BO->getRHS()->getType()->isPointerType() &&
BO->getLHS()->isIntegerConstantExpr(Constant, S.Context))
PointerExpr = BO->getRHS();
else
return false;

bool Overflow;
Constant = ExprUtil::ConvertToSignedPointerWidth(S.Context, Constant, Overflow);
if (Overflow)
return false;
// Normalize the operation by negating the offset if necessary.
if (BO->getOpcode() == BO_Sub) {
uint64_t PointerWidth = S.Context.getTargetInfo().getPointerWidth(0);
Constant = llvm::APSInt(PointerWidth, false).ssub_ov(Constant, Overflow);
if (Overflow)
return false;
}
llvm::APSInt ElemSize;
if (!ExprUtil::getReferentSizeInChars(S.Context, E->getType(), ElemSize))
return false;
Constant = Constant.smul_ov(ElemSize, Overflow);
if (Overflow)
return false;

return true;
}
93 changes: 9 additions & 84 deletions clang/lib/Sema/SemaBounds.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2037,26 +2037,23 @@ namespace {
if (!ExprUtil::EqualValue(S.Context, DeclaredRangeBounds->getLowerExpr(),
SrcRangeBounds->getLowerExpr(), EquivExprs))
return false;

// Attempt to get a variable part and a constant part from the
// declared upper bound by applying certain normalizations.
// declared upper bound and the source upper bound.
Expr *DeclaredVariable = nullptr;
llvm::APSInt DeclaredConstant;
bool NormalizedDeclared =
NormalizeUpperBound(DeclaredRangeBounds->getUpperExpr(),
DeclaredVariable, DeclaredConstant);

// Attempt to get a variable part and a constant part from the
// source upper bound by applying certain normalizations.
bool DeclaredNormalized =
NormalizeUtil::GetVariableAndConstant(S, DeclaredRangeBounds->getUpperExpr(),
DeclaredVariable, DeclaredConstant);
Expr *SrcVariable = nullptr;
llvm::APSInt SrcConstant;
bool NormalizedSrc =
NormalizeUpperBound(SrcRangeBounds->getUpperExpr(),
SrcVariable, SrcConstant);
bool SrcNormalized =
NormalizeUtil::GetVariableAndConstant(S, SrcRangeBounds->getUpperExpr(),
SrcVariable, SrcConstant);

// We must be able to normalize at least one of the upper bounds in
// order to compare them.
if (!NormalizedDeclared && !NormalizedSrc)
if (!DeclaredNormalized && !SrcNormalized)
return false;

// Both upper bounds must have a Variable part.
Expand All @@ -2073,78 +2070,6 @@ namespace {
return DeclaredConstant <= SrcConstant;
}

// NormalizeUpperBound attempts to extract a Variable part and a Constant
// part from the upper bound expression E.
//
// If E can be expressed as:
// 1. (E1 + (E2 op1 A)) op2 B, or:
// 2. B + (E1 + (E2 op1 A))
// where:
// 1. E1 has pointer type, and:
// 2. E2 has integer type, and:
// 3. A and B are integer constants, and:
// 4. op1 and op2 are + or -
// then:
// 1. Variable = E1 + E2
// 2. Constant = A' + B', where A' is -A if op1 is - and B' is -B if
// op2 is -.
//
// If we cannot normalize E to one of these two forms, then Variable = E,
// Constant = 0, and NormalizeUpperBound returns false.
bool NormalizeUpperBound(Expr *E, Expr *&Variable, llvm::APSInt &Constant) {
Expr *PointerExpr;
Expr *ConstExpr;
llvm::APSInt C;
Expr *PointerAndConst;

// E must be of the form X op2 Y, where op2 is + or -.
BinaryOperator *BO = dyn_cast<BinaryOperator>(E->IgnoreParens());
if (!BO)
goto exit;
if (!BinaryOperator::isAdditiveOp(BO->getOpcode()))
goto exit;

// E must be of the form P op2 B or B + P, where P has pointer type
// and B is an integer constant expression.
// Note that E cannot be of the form B - P, since a pointer cannot
// appear on the right-hand side of a subtraction operator.
if (BO->getLHS()->getType()->isPointerType() &&
BO->getRHS()->isIntegerConstantExpr(C, S.Context)) {
PointerExpr = BO->getLHS();
ConstExpr = BO->getRHS();
} else if (BO->getOpcode() == BinaryOperatorKind::BO_Add &&
BO->getRHS()->getType()->isPointerType() &&
BO->getLHS()->isIntegerConstantExpr(C, S.Context)) {
PointerExpr = BO->getRHS();
ConstExpr = BO->getLHS();
} else
goto exit;

// Associate PointerExpr to the left to get (E1 + E2) +/- E3.
// If we cannot, then we cannot continue to normalize.
PointerExpr = NormalizeUtil::TransformAssocLeft(S, PointerExpr);
if (!PointerExpr)
goto exit;

// We have PointerExpr of the form (E1 + E2) + E3. Try to constant fold
// ((E1 + E2) + E3) op2 B to (E1 + E2) + (E3 op2 B).
// If we can perform this constant folding, then Variable = E1 + E2 and
// Constant = E3 op2 B.
// Otherwise, Variable = E and Constant = 0.
PointerAndConst =
ExprCreatorUtil::CreateBinaryOperator(S, PointerExpr, ConstExpr,
BO->getOpcode());
return NormalizeUtil::ConstantFold(S, PointerAndConst, E->getType(),
Variable, Constant);

exit:
// Return (E, 0).
Variable = E;
uint64_t PointerWidth = S.Context.getTargetInfo().getPointerWidth(0);
Constant = llvm::APSInt(PointerWidth, false);
return false;
}

// Try to prove that PtrBase + Offset is within Bounds, where PtrBase has pointer type.
// Offset is optional and may be a nullptr.
ProofResult ProveMemoryAccessInRange(Expr *PtrBase, Expr *Offset, BoundsExpr *Bounds,
Expand Down
29 changes: 16 additions & 13 deletions clang/test/CheckedC/static-checking/bounds-decl-checking.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -762,14 +762,28 @@ void f93(_Nt_array_ptr<char> p : count(len), unsigned int len) {
}
}

void f94(_Array_ptr<int> p : bounds(p, (p + (len + 10)) + 6), int len) {
void f94(_Nt_array_ptr<char> p : count(len), unsigned int len) {
if (*(p + len)) {
if (*(p + len + 1)) {
if (*(p + len + 2)) {
// No warning when proving that inferred bounds (p, ((p + (len - 1)) + 2) + 1)
// imply declared bounds (p, p + len)
++len;
}
}
}
}

void f95(_Array_ptr<int> p : bounds(p, (p + (len + 10)) + 6), int len) {
_Array_ptr<int> q : bounds(q, 2 + (q + (len + 3))) = p;
_Array_ptr<int> r : bounds(r, ((r + len) + 2) - 1) = p;
_Array_ptr<int> s : bounds(s, (s + (len + (1 + 2))) + (3 + 4)) = p;
_Array_ptr<int> t : bounds(t, ((t + len) - 1) + 1) = p;
_Array_ptr<int> u : bounds(u, (u + len) + 1) = p;
_Array_ptr<int> v : bounds(v, (1 + (v + len)) + 1) = p;
}

void f95(_Nt_array_ptr<char> p : bounds(p, (p + (len + 4)) + 4), int len) {
void f96(_Nt_array_ptr<char> p : bounds(p, (p + (len + 4)) + 4), int len) {
_Nt_array_ptr<char> q : bounds(q, ((q + len) + 1) + 1) = p;
_Nt_array_ptr<char> r : bounds(r, ((r + (len + 2))) - 2) = p;

Expand All @@ -778,17 +792,6 @@ void f95(_Nt_array_ptr<char> p : bounds(p, (p + (len + 4)) + 4), int len) {
// expected-note {{(expanded) declared bounds are 'bounds(s, (s + (len * 2)) - 2)'}} \
// expected-note {{(expanded) inferred bounds are 'bounds(p, (p + (len + 4)) + 4)'}}

// (t + len) + 1 is missing the integer constant A, so we cannot express
// it as (P +/- A) +/- B (where P is a pointer and A and B are constants).
_Nt_array_ptr<char> t : bounds(t, (t + len) + 1) = p; // expected-warning {{cannot prove declared bounds for 't' are valid after initialization}} \
// expected-note {{(expanded) declared bounds are 'bounds(t, (t + len) + 1)'}} \
// expected-note {{(expanded) inferred bounds are 'bounds(p, (p + (len + 4)) + 4)'}}

// We currently cannot express 1 + (u + len) as (u + len) + 1.
_Nt_array_ptr<char> u : bounds(u, (1 + (u + len)) + 1) = p; // expected-warning {{cannot prove declared bounds for 'u' are valid after initialization}} \
// expected-note {{(expanded) declared bounds are 'bounds(u, (1 + (u + len)) + 1)'}} \
// expected-note {{(expanded) inferred bounds are 'bounds(p, (p + (len + 4)) + 4)'}}

// We can express (v + (1 + len)) + 2 as ((v + 1) + len) + 2, but we
// currently cannot constant fold this expression since len is not
// an integer constant. Constant folding would expect the expression
Expand Down

0 comments on commit 8d3c184

Please sign in to comment.