diff --git a/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp --- a/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp @@ -272,6 +272,8 @@ handleComparison(C, BO, Result, LVal, RVal, BinaryOperator::getOverloadedOperator(OK)); } else if (isRandomIncrOrDecrOperator(OK)) { + if (!BO->getRHS()->getType()->isIntegralOrEnumerationType()) + return; handlePtrIncrOrDecr(C, BO->getLHS(), BinaryOperator::getOverloadedOperator(OK), RVal); } @@ -461,6 +463,12 @@ RPos = getIteratorPosition(State, RVal); } + // If the value for which we just tried to set a new iterator position is + // an `SVal`for which no iterator position can be set then the setting was + // unsuccessful. We cannot handle the comparison in this case. + if (!LPos || !RPos) + return; + // We cannot make assumptions on `UnknownVal`. Let us conjure a symbol // instead. if (RetVal.isUnknown()) { @@ -599,6 +607,9 @@ const Expr *Iterator, OverloadedOperatorKind OK, SVal Offset) const { + if (!Offset.getAs()) + return; + QualType PtrType = Iterator->getType(); if (!PtrType->isPointerType()) return; @@ -612,13 +623,11 @@ return; SVal NewVal; - if (OK == OO_Plus || OK == OO_PlusEqual) + if (OK == OO_Plus || OK == OO_PlusEqual) { NewVal = State->getLValue(ElementType, Offset, OldVal); - else { - const llvm::APSInt &OffsetInt = - Offset.castAs().getValue(); - auto &BVF = C.getSymbolManager().getBasicVals(); - SVal NegatedOffset = nonloc::ConcreteInt(BVF.getValue(-OffsetInt)); + } else { + auto &SVB = C.getSValBuilder(); + SVal NegatedOffset = SVB.evalMinus(Offset.castAs()); NewVal = State->getLValue(ElementType, NegatedOffset, OldVal); } @@ -684,9 +693,14 @@ const auto StateBefore = N->getState(); const auto *PosBefore = getIteratorPosition(StateBefore, Iter); - - assert(PosBefore && "`std::advance() should not create new iterator " - "position but change existing ones"); + // FIXME: `std::advance()` should not create a new iterator position but + // change existing ones. However, in case of iterators implemented as + // pointers the handling of parameters in `std::advance()`-like + // functions is still incomplete which may result in cases where + // the new position is assigned to the wrong pointer. This causes + // crash if we use an assertion here. + if (!PosBefore) + return false; return PosBefore->getOffset() == PosAfter->getOffset(); } diff --git a/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp @@ -169,6 +169,8 @@ verifyDereference(C, LVal); } else if (isRandomIncrOrDecrOperator(OK)) { SVal RVal = State->getSVal(BO->getRHS(), C.getLocationContext()); + if (!BO->getRHS()->getType()->isIntegralOrEnumerationType()) + return; verifyRandomIncrOrDecr(C, BinaryOperator::getOverloadedOperator(OK), LVal, RVal); } diff --git a/clang/test/Analysis/iterator-modeling.cpp b/clang/test/Analysis/iterator-modeling.cpp --- a/clang/test/Analysis/iterator-modeling.cpp +++ b/clang/test/Analysis/iterator-modeling.cpp @@ -1948,6 +1948,13 @@ clang_analyzer_express(clang_analyzer_iterator_position(i)); // expected-warning{{$c.end() - 2}} } +void minus_equal_ptr_iterator_variable(const cont_with_ptr_iterator &c, + int n) { + auto i = c.end(); + + i -= n; // no-crash +} + void plus_ptr_iterator(const cont_with_ptr_iterator &c) { auto i1 = c.begin(); @@ -1972,6 +1979,17 @@ clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning{{$c.end() - 2}} } +void ptr_iter_diff(cont_with_ptr_iterator &c) { + auto i0 = c.begin(), i1 = c.end(); + ptrdiff_t len = i1 - i0; // no-crash +} + +void ptr_iter_cmp_nullptr(cont_with_ptr_iterator &c) { + auto i0 = c.begin(); + if (i0 != nullptr) // no-crash + ++i0; +} + void clang_analyzer_printState(); void print_state(std::vector &V) { diff --git a/clang/test/Analysis/iterator-range.cpp b/clang/test/Analysis/iterator-range.cpp --- a/clang/test/Analysis/iterator-range.cpp +++ b/clang/test/Analysis/iterator-range.cpp @@ -935,3 +935,7 @@ // expected-note@-1{{Iterator decremented ahead of its valid range}} } +void ptr_iter_diff(cont_with_ptr_iterator &c) { + auto i0 = c.begin(), i1 = c.end(); + ptrdiff_t len = i1 - i0; // no-crash +}