Skip to content

Bounds Widening: Fix handling of bounds variables for nt_array_ptrs #898

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 18, 2020
Merged

Bounds Widening: Fix handling of bounds variables for nt_array_ptrs #898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a0c8526
Bounds Widening: Fix determination of bounds variables for _Nt_array_…
Aug 11, 2020
84f1399
Change unit test
Aug 12, 2020
6af0588
Fix test
Aug 12, 2020
c9183f9
Moved calculation of bounds vars to FillGenSetForEdge
Aug 14, 2020
5354cd4
Fixed indentation in tests
Aug 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 10 additions & 22 deletions clang/include/clang/Sema/BoundsAnalysis.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,6 @@ namespace clang {
EdgeBoundsTy Gen, Out;
// The Kill set for the block.
StmtDeclSetTy Kill;
// The set of all variables used in bounds expr for each ntptr in the
// block.
BoundsVarTy BoundsVars;

// To compute In[B] we compute the intersection of Out[B*->B], where B*
// are all preds of B. When there is a back edge from block B' to B (for
Expand Down Expand Up @@ -156,9 +153,9 @@ namespace clang {
// to lookup ElevatedCFGBlock from CFGBlock.
BlockMapTy BlockMap;

// A set of all ntptrs in scope. Currently, we simply collect all ntptrs
// defined in the function.
DeclSetTy NtPtrsInScope;
// The mapping of all ntptrs in the function and all variables occurring in
// the bounds expr for each ntptr.
BoundsVarTy NtPtrsInScope;

// To compute In[B] we compute the intersection of Out[B*->B], where B* are
// all preds of B. When there is a back edge from block B' to B (for
Expand Down Expand Up @@ -246,14 +243,13 @@ namespace clang {
// @param[in] Dest block for the edge for which the Gen set is updated.
void FillGenSet(Expr *E, ElevatedCFGBlock *EB, ElevatedCFGBlock *SuccEB);

// Uniformize the expr, fill Gen set and get variables used in bounds expr
// for the ntptr.
// Uniformize the expr, fill Gen set for the edge EB->SuccEB.
// @param[in] E is an ntptr dereference or array subscript expr.
// @param[in] Source block for the edge for which the Gen set is updated.
// @param[in] Dest block for the edge for which the Gen set is updated.
void FillGenSetAndGetBoundsVars(const Expr *E,
ElevatedCFGBlock *EB,
ElevatedCFGBlock *SuccEB);
void FillGenSetForEdge(const Expr *E,
ElevatedCFGBlock *EB,
ElevatedCFGBlock *SuccEB);

// Collect all variables used in bounds expr E.
// @param[in] E represents the bounds expr for an ntptr.
Expand Down Expand Up @@ -297,17 +293,9 @@ namespace clang {
// Get the DeclRefExpr from an expression E.
// @param[in] An expression E which is known to be either an LValueToRValue
// cast or an ArrayToPointerDecay cast.
// @return The DeclRefExpr from the expression E.
// @return The DeclRefExpr from the expression E or nullptr.
DeclRefExpr *GetDeclOperand(const Expr *E);

// A DeclRefExpr can be a reference either to an array subscript (in which
// case it is wrapped around a ArrayToPointerDecay cast) or to a pointer
// dereference (in which case it is wrapped around an LValueToRValue cast).
// @param[in] An expression E.
// @return Whether E is an expression containing a reference to an array
// subscript or a pointer dereference.
bool IsDeclOperand(const Expr *E);

// Make an expression uniform by moving all DeclRefExpr to the LHS and all
// IntegerLiterals to the RHS.
// @param[in] E is the expression which should be made uniform.
Expand Down Expand Up @@ -343,10 +331,10 @@ namespace clang {
// @return The intersection of sets A and B.
template<class T> T Intersect(T &A, T &B) const;

// Compute the union of sets A and B.
// Compute the union of sets A and B and widen the bounds where applicable.
// @param[in] A is a set.
// @param[in] B is a set.
// @return The union of sets A and B.
// @return The union of sets A and B containing the widened bounds.
template<class T> T Union(T &A, T &B) const;

// Compute the set difference of sets A and B.
Expand Down
123 changes: 59 additions & 64 deletions clang/lib/Sema/BoundsAnalysis.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,44 +217,40 @@ void BoundsAnalysis::CollectBoundsVars(const Expr *E, DeclSetTy &BoundsVars) {
CollectBoundsVars(BO->getRHS(), BoundsVars);
}

if (IsDeclOperand(E)) {
const DeclRefExpr *D = GetDeclOperand(E);
if (DeclRefExpr *D = GetDeclOperand(E))
if (const auto *V = dyn_cast<VarDecl>(D->getDecl()))
BoundsVars.insert(V);
}
}

DeclRefExpr *BoundsAnalysis::GetDeclOperand(const Expr *E) {
if (!E || !isa<CastExpr>(E))
return nullptr;
auto *CE = dyn_cast<CastExpr>(E);
assert(CE->getSubExpr() && "invalid CastExpr expression");

return dyn_cast<DeclRefExpr>(IgnoreCasts(CE->getSubExpr()));
}

bool BoundsAnalysis::IsDeclOperand(const Expr *E) {
if (auto *CE = dyn_cast<CastExpr>(E)) {
assert(CE->getSubExpr() && "invalid CastExpr expression");
if (auto *CE = dyn_cast_or_null<CastExpr>(E)) {
const Expr *SubE = CE->getSubExpr();
assert(SubE && "Invalid CastExpr expression");

if (CE->getCastKind() == CastKind::CK_LValueToRValue ||
CE->getCastKind() == CastKind::CK_ArrayToPointerDecay)
return isa<DeclRefExpr>(IgnoreCasts(CE->getSubExpr()));
CE->getCastKind() == CastKind::CK_ArrayToPointerDecay) {
E = Lex.IgnoreValuePreservingOperations(Ctx, const_cast<Expr *>(SubE));
return dyn_cast_or_null<DeclRefExpr>(const_cast<Expr *>(E));
}
}
return false;
return nullptr;
}

void BoundsAnalysis::CollectNtPtrsInScope(FunctionDecl *FD) {
// TODO: Currently, we simply collect all ntptrs defined in the current
// function. Ultimately, we need to do a liveness analysis of what ntptrs are
// in scope for a block.
// TODO: Currently, we simply collect all ntptrs and variables used in their
// declared bounds for the entire function. Ultimately, we need to do a
// liveness analysis of what ntptrs are in scope for a block.

assert(FD && "invalid function");

// Collect ntptrs passed as parameters to the current function.
for (const ParmVarDecl *PD : FD->parameters())
// Note: NtPtrsInScope is a mapping from ntptr to variables used in the
// declared bounds of the ntptr. In this function we store an empty set for
// the bounds variables. This will later be filled in FillGenSetForEdge.
for (const ParmVarDecl *PD : FD->parameters()) {
if (IsNtArrayType(PD))
NtPtrsInScope.insert(PD);
NtPtrsInScope[PD] = DeclSetTy();
}

// Collect all ntptrs defined in the current function. BlockMap contains all
// blocks of the current function. We iterate through all blocks in BlockMap
Expand All @@ -273,15 +269,15 @@ void BoundsAnalysis::CollectNtPtrsInScope(FunctionDecl *FD) {
for (const Decl *D : DS->decls())
if (const auto *V = dyn_cast<VarDecl>(D))
if (IsNtArrayType(V))
NtPtrsInScope.insert(V);
NtPtrsInScope[V] = DeclSetTy();
}
}
}
}

void BoundsAnalysis::FillGenSetAndGetBoundsVars(const Expr *E,
ElevatedCFGBlock *EB,
ElevatedCFGBlock *SuccEB) {
void BoundsAnalysis::FillGenSetForEdge(const Expr *E,
ElevatedCFGBlock *EB,
ElevatedCFGBlock *SuccEB) {

// TODO: Handle accesses of the form:
// "if (*(p + i) && *(p + j) && *(p + k))"
Expand Down Expand Up @@ -327,29 +323,23 @@ void BoundsAnalysis::FillGenSetAndGetBoundsVars(const Expr *E,
// if (*(p + 1)) // no widening
// if (*(p + i)) // widen p and q by 1

// TODO: Currently, we iterate and re-compute info for all ntptrs in scope
// for each ntptr dereference. We can optimize this at the cost of space by
// storing the VarDecls, variables used in bounds exprs and base/offset for
// the declared upper bounds expr for the VarDecl. Then we simply have to
// look this up instead of re-computing.
for (auto item : NtPtrsInScope) {
const VarDecl *V = item.first;

for (const VarDecl *V : NtPtrsInScope) {
// In case the bounds expr for V is not a RangeBoundsExpr, invoke
// ExpandBoundsToRange to expand it to RangeBoundsExpr.
const BoundsExpr *BE = S.ExpandBoundsToRange(V, V->getBoundsExpr());
const auto *RBE = dyn_cast<RangeBoundsExpr>(BE);
BoundsExpr *NormalizedBounds = S.NormalizeBounds(V);
if (!NormalizedBounds)
continue;

const auto *RBE = dyn_cast<RangeBoundsExpr>(NormalizedBounds);
if (!RBE)
continue;

// Collect all variables involved in the upper and lower bounds exprs for
// the ntptr. An assignment to any such variable would kill the widenend
// bounds for the ntptr.
if (!SuccEB->BoundsVars.count(V)) {
DeclSetTy BoundsVars;
CollectBoundsVars(RBE, BoundsVars);
SuccEB->BoundsVars[V] = BoundsVars;
}
DeclSetTy BoundsVars;
CollectBoundsVars(NormalizedBounds, BoundsVars);
NtPtrsInScope[V] = BoundsVars;

// Update the bounds of p on the edge EB->SuccEB only if we haven't already
// updated them.
Expand Down Expand Up @@ -427,8 +417,8 @@ ExprIntPairTy BoundsAnalysis::SplitIntoBaseOffset(const Expr *E) {

E = E->IgnoreParens();

if (IsDeclOperand(E))
return std::make_pair(GetDeclOperand(E), Zero);
if (DeclRefExpr *D = GetDeclOperand(E))
return std::make_pair(D, Zero);

if (!isa<BinaryOperator>(E))
return std::make_pair(nullptr, Zero);
Expand All @@ -450,20 +440,22 @@ ExprIntPairTy BoundsAnalysis::SplitIntoBaseOffset(const Expr *E) {
// p + i ==> return (p, i)
llvm::APSInt IntVal;

if (IsDeclOperand(LHS) && RHS->isIntegerConstantExpr(IntVal, Ctx))
return std::make_pair(GetDeclOperand(LHS), IntVal);
if (DeclRefExpr *D = GetDeclOperand(LHS))
if (RHS->isIntegerConstantExpr(IntVal, Ctx))
return std::make_pair(D, IntVal);

// Case 2: LHS is IntegerLiteral and RHS is DeclRefExpr. We simply need to
// swap LHS and RHS to make expr uniform.
// i + p ==> return (p, i)
if (LHS->isIntegerConstantExpr(IntVal, Ctx) && IsDeclOperand(RHS))
return std::make_pair(GetDeclOperand(RHS), IntVal);
if (DeclRefExpr *D = GetDeclOperand(RHS))
if (LHS->isIntegerConstantExpr(IntVal, Ctx))
return std::make_pair(D, IntVal);

// Case 3: LHS and RHS are both DeclRefExprs. This means there is no
// IntegerLiteral in the expr. In this case, we return the incoming
// BinaryOperator expr with a nullptr for the RHS.
// p + q ==> return (p + q, nullptr)
if (IsDeclOperand(LHS) && IsDeclOperand(RHS))
if (GetDeclOperand(LHS) && GetDeclOperand(RHS))
return std::make_pair(BO, Zero);

// To make parsing simpler, we always try to keep BinaryOperator on the LHS.
Expand Down Expand Up @@ -580,22 +572,23 @@ void BoundsAnalysis::FillGenSet(Expr *E,
AE->getValueKind(), AE->getObjectKind(),
AE->getExprLoc(), FPOptions());

FillGenSetAndGetBoundsVars(BO, EB, SuccEB);
FillGenSetForEdge(BO, EB, SuccEB);

} else if (auto *UO = dyn_cast<UnaryOperator>(E)) {
if (UO->getOpcode() == UO_Deref) {
assert(UO->getSubExpr() && "invalid UnaryOperator expression");

const Expr *UE = IgnoreCasts(UO->getSubExpr());
FillGenSetAndGetBoundsVars(UE, EB, SuccEB);
FillGenSetForEdge(UE, EB, SuccEB);
}
}
}

void BoundsAnalysis::ComputeKillSets(StmtSet NestedStmts) {
// For a block B, a variable v is added to Kill[B][S] if v is assigned to in
// For a block B, a variable V is added to Kill[B][S] if V is assigned to in
// B by Stmt S or some child S1 of S.

// Compute vars killed in the current block.
for (const auto item : BlockMap) {
ElevatedCFGBlock *EB = item.second;

Expand All @@ -604,10 +597,12 @@ void BoundsAnalysis::ComputeKillSets(StmtSet NestedStmts) {
const Stmt *S = Elem.castAs<CFGStmt>().getStmt();
if (!S)
continue;

// Skip top-level statements that are nested in another
// top-level statement.
if (NestedStmts.find(S) != NestedStmts.end())
continue;

FillKillSet(EB, S, S);
}
}
Expand Down Expand Up @@ -641,23 +636,23 @@ void BoundsAnalysis::FillKillSet(ElevatedCFGBlock *EB,
EB->Kill[TopLevelStmt].insert(V);

else {
// Else look for the variable in BoundsVars.
// Else look for the variable in NtPtrsInScope.

// BoundsVars is a mapping from an ntptr to all the variables used in
// its upper and lower bounds exprs. For example:
// NtPtrsInScope is a mapping from an ntptr to all the variables used
// in its upper and lower bounds exprs. For example:

// _Nt_array_ptr<char> p : bounds(p + i, i + p + j + 10);
// _Nt_array_ptr<char> q : bounds(i + q, i + p + q + m);

// EB->BoundsVars: {p: {p, i, j}, q: {i, q, p, m}}
// NtPtrsInScope: {p: {p, i, j}, q: {i, q, p, m}}

for (auto item : EB->BoundsVars) {
for (auto item : NtPtrsInScope) {
const VarDecl *NtPtr = item.first;
DeclSetTy Vars = item.second;
DeclSetTy BoundsVars = item.second;

// If the variable exists in the bounds declaration for the ntptr,
// then add the Stmt:ntptr pair to the Kill set for the block.
if (Vars.count(V))
if (BoundsVars.count(V))
EB->Kill[TopLevelStmt].insert(NtPtr);
}
}
Expand Down Expand Up @@ -695,15 +690,15 @@ void BoundsAnalysis::ComputeOutSets(ElevatedCFGBlock *EB,
WorkListTy &WorkList) {
// Out[B1->B2] = (In[B1] - Kill[B1]) u Gen[B1->B2].

// EB->Kill is a mapping from Stmt to ntptrs. We extract just the ntptrs for
// the block and then use that to compute (In - Kill).
// EB->Kill is a mapping from Stmt to ntptrs. We extract just the ntptrs
// killed for the block and use that to compute (In - Kill).
DeclSetTy KilledVars;
for (auto item : EB->Kill) {
const DeclSetTy Vars = item.second;
KilledVars.insert(Vars.begin(), Vars.end());
}

BoundsMapTy Diff = Difference(EB->In, KilledVars);
BoundsMapTy InMinusKill = Difference(EB->In, KilledVars);

for (const CFGBlock *succ : EB->Block->succs()) {
if (SkipBlock(succ))
Expand All @@ -728,7 +723,7 @@ void BoundsAnalysis::ComputeOutSets(ElevatedCFGBlock *EB,
// B2: if (*(p + 1)) { // In[B2] = {p:1}, Gen[B1->B2] = {p:1} ==> bounds(p) = 2.
// B3: if (*(p + 2)) { // In[B2] = {p:2}, Gen[B1->B2] = {p:2} ==> bounds(p) = 3.

EB->Out[succ] = Union(Diff, EB->Gen[succ]);
EB->Out[succ] = Union(InMinusKill, EB->Gen[succ]);

// The Out set on an edge is marked "empty" if the In set is marked "empty"
// and the Gen set on that edge is empty.
Expand Down Expand Up @@ -776,8 +771,8 @@ Expr *BoundsAnalysis::IgnoreCasts(const Expr *E) {
}

bool BoundsAnalysis::IsNtArrayType(const VarDecl *V) const {
return V->getType()->isCheckedPointerNtArrayType() ||
V->getType()->isNtCheckedArrayType();
return V && (V->getType()->isCheckedPointerNtArrayType() ||
V->getType()->isNtCheckedArrayType());
}

bool BoundsAnalysis::SkipBlock(const CFGBlock *B) const {
Expand Down
26 changes: 25 additions & 1 deletion clang/test/CheckedC/inferred-bounds/widened-bounds.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,6 @@ void f11(int i, int j) {

// CHECK: [B3]
// CHECK: 1: *(p + j)
// CHECK: T: if [B3.1]
// CHECK: [B2]
// CHECK: 1: j = 0
// CHECK: 2: *(p + j + 1)
Expand Down Expand Up @@ -1242,3 +1241,28 @@ void f33() {
// CHECK: [B1]
// CHECK: upper_bound(p) = 1
}

void f34(_Nt_array_ptr<char> p : count(i), int i, int flag) {
if (*(p + i)) {
flag ? i++ : i; // expected-error {{inferred bounds for 'p' are unknown after statement}}

if (*(p + i + 1))
{}
}

// CHECK: In function: f34
// CHECK: [B6]
// CHECK: 1: *(p + i)
// CHECK: [B5]
// CHECK: 1: flag
// CHECK: upper_bound(p) = 1
// CHECK: [B4]
// CHECK: 1: i
// CHECK: upper_bound(p) = 1
// CHECK: [B3]
// CHECK: 1: i++
// CHECK: upper_bound(p) = 1
// CHECK: [B2]
// CHECK: 2: *(p + i + 1)
// CHECK: upper_bound(p) = 1
}