Skip to content

Commit

Permalink
[Analyzer] Fix errors in iterator modeling
Browse files Browse the repository at this point in the history
There is major a bug found in iterator modeling: upon adding a value
to or subtracting a value from an iterator the position of the original
iterator is also changed beside the result. This patch fixes this bug.

To catch such bugs in the future we also changed the tests to look for
regular expressions including an end-of-line symbol (`$`) so we can
prevent false matches where only the tested prefix matches.

Another minor bug is that when printing the state, all the iterator
positions are printed in a single line. This patch also fixes this.

Differential Revision: https://reviews.llvm.org/D82385
  • Loading branch information
Adam Balogh committed Jul 1, 2020
1 parent 4da65c2 commit ea563da
Show file tree
Hide file tree
Showing 2 changed files with 182 additions and 161 deletions.
22 changes: 16 additions & 6 deletions clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ void IteratorModeling::processComparison(CheckerContext &C,
StateTrue = StateTrue->assume(*ConditionVal, true);
C.addTransition(StateTrue);
}

if (auto StateFalse = relateSymbols(State, Sym1, Sym2, Op != OO_EqualEqual)) {
StateFalse = StateFalse->assume(*ConditionVal, false);
C.addTransition(StateFalse);
Expand Down Expand Up @@ -540,14 +540,16 @@ void IteratorModeling::handleRandomIncrOrDecr(CheckerContext &C,

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

auto NewState =
advancePosition(State, LHS, Op, *value);
if (NewState) {
const auto *NewPos = getIteratorPosition(NewState, LHS);
// `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);
if (AdvancedState) {
const auto *NewPos = getIteratorPosition(AdvancedState, LHS);
assert(NewPos &&
"Iterator should have position after successful advancement");

State = setIteratorPosition(NewState, TgtVal, *NewPos);
State = setIteratorPosition(State, TgtVal, *NewPos);
C.addTransition(State);
} else {
assignToContainer(C, CE, TgtVal, Pos->getContainer());
Expand Down Expand Up @@ -611,10 +613,15 @@ void IteratorModeling::printState(raw_ostream &Out, ProgramStateRef State,
const char *NL, const char *Sep) const {
auto SymbolMap = State->get<IteratorSymbolMap>();
auto RegionMap = State->get<IteratorRegionMap>();
// Use a counter to add newlines before every line except the first one.
unsigned Count = 0;

if (!SymbolMap.isEmpty() || !RegionMap.isEmpty()) {
Out << Sep << "Iterator Positions :" << NL;
for (const auto &Sym : SymbolMap) {
if (Count++)
Out << NL;

Sym.first->dumpToStream(Out);
Out << " : ";
const auto Pos = Sym.second;
Expand All @@ -625,6 +632,9 @@ void IteratorModeling::printState(raw_ostream &Out, ProgramStateRef State,
}

for (const auto &Reg : RegionMap) {
if (Count++)
Out << NL;

Reg.first->dumpToStream(Out);
Out << " : ";
const auto Pos = Reg.second;
Expand Down
Loading

0 comments on commit ea563da

Please sign in to comment.