baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: NoQ, Szelethus.
baloghadamsoftware added a project: clang.
Herald added subscribers: Charusso, donat.nagy, mikhail.ramalho, a.sidorin,
rnkovacs, szepet, xazax.hun, whisperity.
Sometimes the return value of a comparison operator call is `UnkownVal`. Since
no assumptions can be made on `UnknownVal`, this leeds to keeping impossible
execution paths in the exploded graph resulting in poor performance and false
positives. To overcome this we replace unknown results of iterator comparisons
by conjured symbols.
Repository:
rC Clang
https://reviews.llvm.org/D70244
Files:
clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
clang/test/Analysis/invalidated-iterator.cpp
clang/test/Analysis/iterator-modelling.cpp
Index: clang/test/Analysis/iterator-modelling.cpp
===================================================================
--- clang/test/Analysis/iterator-modelling.cpp
+++ clang/test/Analysis/iterator-modelling.cpp
@@ -1966,7 +1966,7 @@
clang_analyzer_eval(clang_analyzer_container_end(V) ==
clang_analyzer_iterator_position(first)); //
expected-warning@-1{{FALSE}} expected-warning@-1{{TRUE}}
if (V.end() != first) {
- clang_analyzer_eval(clang_analyzer_container_end(V) ==
- clang_analyzer_iterator_position(first)); //
expected-warning@-1{{FALSE}} expected-warning@-1 0-1{{TRUE}} FIXME: should only
expect FALSE in every case
+ clang_analyzer_eval(clang_analyzer_container_end(V) ==
+ clang_analyzer_iterator_position(first)); //
expected-warning@-1{{FALSE}}
}
}
Index: clang/test/Analysis/invalidated-iterator.cpp
===================================================================
--- clang/test/Analysis/invalidated-iterator.cpp
+++ clang/test/Analysis/invalidated-iterator.cpp
@@ -120,4 +120,3 @@
V.erase(i);
auto j = V.cbegin(); // no-warning
}
-
Index: clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -180,7 +180,7 @@
std::unique_ptr<BugType> InvalidatedBugType;
std::unique_ptr<BugType> DebugMsgBugType;
- void handleComparison(CheckerContext &C, const Expr *CE, const SVal &RetVal,
+ void handleComparison(CheckerContext &C, const Expr *CE, SVal RetVal,
const SVal &LVal, const SVal &RVal,
OverloadedOperatorKind Op) const;
void processComparison(CheckerContext &C, ProgramStateRef State,
@@ -883,7 +883,7 @@
}
void IteratorChecker::handleComparison(CheckerContext &C, const Expr *CE,
- const SVal &RetVal, const SVal &LVal,
+ SVal RetVal, const SVal &LVal,
const SVal &RVal,
OverloadedOperatorKind Op) const {
// Record the operands and the operator of the comparison for the next
@@ -922,6 +922,16 @@
RPos = getIteratorPosition(State, RVal);
}
+ // We cannot make assumpotions on `UnknownVal`. Let us conjure a symbol
+ // instead.
+ if (RetVal.isUnknown()) {
+ auto &SymMgr = C.getSymbolManager();
+ auto *LCtx = C.getLocationContext();
+ RetVal = nonloc::SymbolVal(SymMgr.conjureSymbol(
+ CE, LCtx, C.getASTContext().BoolTy, C.blockCount()));
+ State = State->BindExpr(CE, LCtx, RetVal);
+ }
+
processComparison(C, State, LPos->getOffset(), RPos->getOffset(), RetVal,
Op);
}
@@ -943,7 +953,7 @@
const auto ConditionVal = RetVal.getAs<DefinedSVal>();
if (!ConditionVal)
return;
-
+
if (auto StateTrue = relateSymbols(State, Sym1, Sym2, Op == OO_EqualEqual)) {
StateTrue = StateTrue->assume(*ConditionVal, true);
C.addTransition(StateTrue);
Index: clang/test/Analysis/iterator-modelling.cpp
===================================================================
--- clang/test/Analysis/iterator-modelling.cpp
+++ clang/test/Analysis/iterator-modelling.cpp
@@ -1966,7 +1966,7 @@
clang_analyzer_eval(clang_analyzer_container_end(V) ==
clang_analyzer_iterator_position(first)); // expected-warning@-1{{FALSE}} expected-warning@-1{{TRUE}}
if (V.end() != first) {
- clang_analyzer_eval(clang_analyzer_container_end(V) ==
- clang_analyzer_iterator_position(first)); // expected-warning@-1{{FALSE}} expected-warning@-1 0-1{{TRUE}} FIXME: should only expect FALSE in every case
+ clang_analyzer_eval(clang_analyzer_container_end(V) ==
+ clang_analyzer_iterator_position(first)); // expected-warning@-1{{FALSE}}
}
}
Index: clang/test/Analysis/invalidated-iterator.cpp
===================================================================
--- clang/test/Analysis/invalidated-iterator.cpp
+++ clang/test/Analysis/invalidated-iterator.cpp
@@ -120,4 +120,3 @@
V.erase(i);
auto j = V.cbegin(); // no-warning
}
-
Index: clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -180,7 +180,7 @@
std::unique_ptr<BugType> InvalidatedBugType;
std::unique_ptr<BugType> DebugMsgBugType;
- void handleComparison(CheckerContext &C, const Expr *CE, const SVal &RetVal,
+ void handleComparison(CheckerContext &C, const Expr *CE, SVal RetVal,
const SVal &LVal, const SVal &RVal,
OverloadedOperatorKind Op) const;
void processComparison(CheckerContext &C, ProgramStateRef State,
@@ -883,7 +883,7 @@
}
void IteratorChecker::handleComparison(CheckerContext &C, const Expr *CE,
- const SVal &RetVal, const SVal &LVal,
+ SVal RetVal, const SVal &LVal,
const SVal &RVal,
OverloadedOperatorKind Op) const {
// Record the operands and the operator of the comparison for the next
@@ -922,6 +922,16 @@
RPos = getIteratorPosition(State, RVal);
}
+ // We cannot make assumpotions on `UnknownVal`. Let us conjure a symbol
+ // instead.
+ if (RetVal.isUnknown()) {
+ auto &SymMgr = C.getSymbolManager();
+ auto *LCtx = C.getLocationContext();
+ RetVal = nonloc::SymbolVal(SymMgr.conjureSymbol(
+ CE, LCtx, C.getASTContext().BoolTy, C.blockCount()));
+ State = State->BindExpr(CE, LCtx, RetVal);
+ }
+
processComparison(C, State, LPos->getOffset(), RPos->getOffset(), RetVal, Op);
}
@@ -943,7 +953,7 @@
const auto ConditionVal = RetVal.getAs<DefinedSVal>();
if (!ConditionVal)
return;
-
+
if (auto StateTrue = relateSymbols(State, Sym1, Sym2, Op == OO_EqualEqual)) {
StateTrue = StateTrue->assume(*ConditionVal, true);
C.addTransition(StateTrue);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
