Skip to content

Commit

Permalink
[analyzer] Model iterator random incrementation symmetrically
Browse files Browse the repository at this point in the history 
Summary:
In case a pointer iterator is incremented in a binary plus expression
(operator+), where the iterator is on the RHS, IteratorModeling should
now detect, and track the resulting value.

Reviewers: Szelethus, baloghadamsoftware

Reviewed By: baloghadamsoftware

Subscribers: rnkovacs, whisperity, xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, Charusso, steakhal, martong, ASDenysPetrov, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D83190
  • Loading branch information
@gamesh411
Endre Fülöp authored and gamesh411 committed Aug 4, 2020
1 parent d9d2210 commit 141cb8a
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 28 deletions.
73 changes: 48 additions & 25 deletions clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ class IteratorModeling
bool Postfix) const;
void handleRandomIncrOrDecr(CheckerContext &C, const Expr *CE,
OverloadedOperatorKind Op, const SVal &RetVal,
const SVal &LHS, const SVal &RHS) const;
const SVal &Iterator, const SVal &Amount) const;
void handlePtrIncrOrDecr(CheckerContext &C, const Expr *Iterator,
OverloadedOperatorKind OK, SVal Offset) const;
void handleAdvance(CheckerContext &C, const Expr *CE, SVal RetVal, SVal Iter,
Expand Down Expand Up @@ -262,20 +262,30 @@ void IteratorModeling::checkPostStmt(const UnaryOperator *UO,

void IteratorModeling::checkPostStmt(const BinaryOperator *BO,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
BinaryOperatorKind OK = BO->getOpcode();
SVal RVal = State->getSVal(BO->getRHS(), C.getLocationContext());
const ProgramStateRef State = C.getState();
const BinaryOperatorKind OK = BO->getOpcode();
const Expr *const LHS = BO->getLHS();
const Expr *const RHS = BO->getRHS();
const SVal LVal = State->getSVal(LHS, C.getLocationContext());
const SVal RVal = State->getSVal(RHS, C.getLocationContext());

if (isSimpleComparisonOperator(BO->getOpcode())) {
SVal LVal = State->getSVal(BO->getLHS(), C.getLocationContext());
SVal Result = State->getSVal(BO, C.getLocationContext());
handleComparison(C, BO, Result, LVal, RVal,
BinaryOperator::getOverloadedOperator(OK));
} else if (isRandomIncrOrDecrOperator(OK)) {
if (!BO->getRHS()->getType()->isIntegralOrEnumerationType())
// In case of operator+ the iterator can be either on the LHS (eg.: it + 1),
// or on the RHS (eg.: 1 + it). Both cases are modeled.
const bool IsIterOnLHS = BO->getLHS()->getType()->isPointerType();
const Expr *const &IterExpr = IsIterOnLHS ? LHS : RHS;
const Expr *const &AmountExpr = IsIterOnLHS ? RHS : LHS;

// The non-iterator side must have an integral or enumeration type.
if (!AmountExpr->getType()->isIntegralOrEnumerationType())
return;
handlePtrIncrOrDecr(C, BO->getLHS(),
BinaryOperator::getOverloadedOperator(OK), RVal);
const SVal &AmountVal = IsIterOnLHS ? RVal : LVal;
handlePtrIncrOrDecr(C, IterExpr, BinaryOperator::getOverloadedOperator(OK),
AmountVal);
}
}

Expand Down Expand Up @@ -368,11 +378,24 @@ IteratorModeling::handleOverloadedOperator(CheckerContext &C,
InstCall->getCXXThisVal(), Call.getArgSVal(0));
return;
}
} else {
if (Call.getNumArgs() >= 2 &&
Call.getArgExpr(1)->getType()->isIntegralOrEnumerationType()) {
} else if (Call.getNumArgs() >= 2) {
const Expr *FirstArg = Call.getArgExpr(0);
const Expr *SecondArg = Call.getArgExpr(1);
const QualType FirstType = FirstArg->getType();
const QualType SecondType = SecondArg->getType();

if (FirstType->isIntegralOrEnumerationType() ||
SecondType->isIntegralOrEnumerationType()) {
// In case of operator+ the iterator can be either on the LHS (eg.:
// it + 1), or on the RHS (eg.: 1 + it). Both cases are modeled.
const bool IsIterFirst = FirstType->isStructureOrClassType();
const SVal FirstArg = Call.getArgSVal(0);
const SVal SecondArg = Call.getArgSVal(1);
const SVal &Iterator = IsIterFirst ? FirstArg : SecondArg;
const SVal &Amount = IsIterFirst ? SecondArg : FirstArg;

handleRandomIncrOrDecr(C, OrigExpr, Op, Call.getReturnValue(),
Call.getArgSVal(0), Call.getArgSVal(1));
Iterator, Amount);
return;
}
}
Expand Down Expand Up @@ -564,35 +587,35 @@ void IteratorModeling::handleDecrement(CheckerContext &C, const SVal &RetVal,
C.addTransition(State);
}

void IteratorModeling::handleRandomIncrOrDecr(CheckerContext &C,
const Expr *CE,
void IteratorModeling::handleRandomIncrOrDecr(CheckerContext &C, const Expr *CE,
OverloadedOperatorKind Op,
const SVal &RetVal,
const SVal &LHS,
const SVal &RHS) const {
const SVal &Iterator,
const SVal &Amount) const {
// Increment or decrement the symbolic expressions which represents the
// position of the iterator
auto State = C.getState();

const auto *Pos = getIteratorPosition(State, LHS);
const auto *Pos = getIteratorPosition(State, Iterator);
if (!Pos)
return;

const auto *value = &RHS;
SVal val;
if (auto loc = RHS.getAs<Loc>()) {
val = State->getRawSVal(*loc);
value = &val;
const auto *Value = &Amount;
SVal Val;
if (auto LocAmount = Amount.getAs<Loc>()) {
Val = State->getRawSVal(*LocAmount);
Value = &Val;
}

auto &TgtVal = (Op == OO_PlusEqual || Op == OO_MinusEqual) ? LHS : RetVal;
const auto &TgtVal =
(Op == OO_PlusEqual || Op == OO_MinusEqual) ? Iterator : RetVal;

// `AdvancedState` is a state where the position of `LHS` is advanced. We
// only need this state to retrieve the new position, but we do not want
// to change the position of `LHS` (in every case).
auto AdvancedState = advancePosition(State, LHS, Op, *value);
auto AdvancedState = advancePosition(State, Iterator, Op, *Value);
if (AdvancedState) {
const auto *NewPos = getIteratorPosition(AdvancedState, LHS);
const auto *NewPos = getIteratorPosition(AdvancedState, Iterator);
assert(NewPos &&
"Iterator should have position after successful advancement");

Expand Down
42 changes: 39 additions & 3 deletions clang/test/Analysis/iterator-modeling.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ void copy(const std::vector<int> &v) {
clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning-re {{$v.end(){{$}}}}
}

void plus(const std::vector<int> &v) {
void plus_lhs(const std::vector<int> &v) {
auto i1 = v.begin();

clang_analyzer_denote(clang_analyzer_container_begin(v), "$v.begin()");
Expand All @@ -161,7 +161,19 @@ void plus(const std::vector<int> &v) {
clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning-re{{$v.begin() + 2{{$}}}}
}

void plus_negative(const std::vector<int> &v) {
void plus_rhs(const std::vector<int> &v) {
auto i1 = v.begin();

clang_analyzer_denote(clang_analyzer_container_begin(v), "$v.begin()");

auto i2 = 2 + i1;

clang_analyzer_eval(clang_analyzer_iterator_container(i2) == &v); // expected-warning{{TRUE}}
clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning-re{{$v.begin(){{$}}}}
clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning-re{{$v.begin() + 2{{$}}}}
}

void plus_lhs_negative(const std::vector<int> &v) {
auto i1 = v.end();

clang_analyzer_denote(clang_analyzer_container_end(v), "$v.end()");
Expand All @@ -173,6 +185,18 @@ void plus_negative(const std::vector<int> &v) {
clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning-re {{$v.end() - 2{{$}}}}
}

void plus_rhs_negative(const std::vector<int> &v) {
auto i1 = v.end();

clang_analyzer_denote(clang_analyzer_container_end(v), "$v.end()");

auto i2 = (-2) + i1;

clang_analyzer_eval(clang_analyzer_iterator_container(i2) == &v); // expected-warning{{TRUE}}
clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning-re {{$v.end(){{$}}}}
clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning-re {{$v.end() - 2{{$}}}}
}

void minus(const std::vector<int> &v) {
auto i1 = v.end();

Expand Down Expand Up @@ -1955,7 +1979,7 @@ void minus_equal_ptr_iterator_variable(const cont_with_ptr_iterator<int> &c,
i -= n; // no-crash
}

void plus_ptr_iterator(const cont_with_ptr_iterator<int> &c) {
void plus_lhs_ptr_iterator(const cont_with_ptr_iterator<int> &c) {
auto i1 = c.begin();

clang_analyzer_denote(clang_analyzer_container_begin(c), "$c.begin()");
Expand All @@ -1967,6 +1991,18 @@ void plus_ptr_iterator(const cont_with_ptr_iterator<int> &c) {
clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning{{$c.begin() + 2}}
}

void plus_rhs_ptr_iterator(const cont_with_ptr_iterator<int> &c) {
auto i1 = c.begin();

clang_analyzer_denote(clang_analyzer_container_begin(c), "$c.begin()");

auto i2 = 2 + i1;

clang_analyzer_eval(clang_analyzer_iterator_container(i2) == &c); // expected-warning{{TRUE}}
clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning{{$c.begin()}}
clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning{{$c.begin() + 2}}
}

void minus_ptr_iterator(const cont_with_ptr_iterator<int> &c) {
auto i1 = c.end();

Expand Down

0 comments on commit 141cb8a

Please sign in to comment.