[clang] 141cb8a - [analyzer] Model iterator random incrementation symmetrically
Endre Fulop via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 4 02:04:02 PDT 2020
Author: Endre Fülöp
Date: 2020-08-04T11:04:12+02:00
New Revision: 141cb8a1eecc0c843cdd4e788a28d2b6715e4dc5
URL: https://github.com/llvm/llvm-project/commit/141cb8a1eecc0c843cdd4e788a28d2b6715e4dc5
DIFF: https://github.com/llvm/llvm-project/commit/141cb8a1eecc0c843cdd4e788a28d2b6715e4dc5.diff
LOG: [analyzer] Model iterator random incrementation symmetrically
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
Added:
Modified:
clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
clang/test/Analysis/iterator-modeling.cpp
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
index 632de9e5dc83..ab5e6a1c9991 100644
--- a/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
@@ -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,
@@ -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);
}
}
@@ -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;
}
}
@@ -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");
diff --git a/clang/test/Analysis/iterator-modeling.cpp b/clang/test/Analysis/iterator-modeling.cpp
index 0b76b0bfa723..f1538839d06c 100644
--- a/clang/test/Analysis/iterator-modeling.cpp
+++ b/clang/test/Analysis/iterator-modeling.cpp
@@ -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()");
@@ -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()");
@@ -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();
@@ -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()");
@@ -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();
More information about the cfe-commits
mailing list