Index: clang/lib/StaticAnalyzer/Checkers/InvalidatedIteratorChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/InvalidatedIteratorChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/InvalidatedIteratorChecker.cpp @@ -30,9 +30,7 @@ std::unique_ptr InvalidatedBugType; - void verifyAccess(CheckerContext &C, const SVal &Val) const; - void reportBug(const StringRef &Message, const SVal &Val, - CheckerContext &C, ExplodedNode *ErrNode) const; + void verifyAccess(CheckerContext &C, const SVal &Val, const Expr *Ex) const; public: InvalidatedIteratorChecker(); @@ -40,6 +38,29 @@ }; +class IteratorInvalidationBRVisitor final : public BugReporterVisitor { + SVal Iter; + const Expr *IterExpr; + bool Satisfied = false; + +public: + IteratorInvalidationBRVisitor(SVal It, const Expr *ItEx) : Iter(It), + IterExpr(ItEx) {} + + void Profile(llvm::FoldingSetNodeID &ID) const override { + ID.Add(Iter); + ID.Add(IterExpr); + } + + std::shared_ptr + VisitNode(const ExplodedNode *Succ, BugReporterContext &BRC, + PathSensitiveBugReport &BR) override; + +private: + bool getPredecessor(const ExplodedNode *Node, const Stmt *S, BugReport &BR, + SVal &PredVal, const Expr *&PredExpr) const; +}; + } //namespace InvalidatedIteratorChecker::InvalidatedIteratorChecker() { @@ -58,14 +79,16 @@ isAccessOperator(Func->getOverloadedOperator())) { // Check for any kind of access of invalidated iterator positions if (const auto *InstCall = dyn_cast(&Call)) { - verifyAccess(C, InstCall->getCXXThisVal()); + verifyAccess(C, InstCall->getCXXThisVal(), InstCall->getCXXThisExpr()); } else { - verifyAccess(C, Call.getArgSVal(0)); + verifyAccess(C, Call.getArgSVal(0), Call.getArgExpr(0)); } } } -void InvalidatedIteratorChecker::verifyAccess(CheckerContext &C, const SVal &Val) const { +void InvalidatedIteratorChecker::verifyAccess(CheckerContext &C, + const SVal &Val, + const Expr *Ex) const { auto State = C.getState(); const auto *Pos = getIteratorPosition(State, Val); if (Pos && !Pos->isValid()) { @@ -73,17 +96,151 @@ if (!N) { return; } - reportBug("Invalidated iterator accessed.", Val, C, N); + auto R = + std::make_unique(*InvalidatedBugType, + "Invalidated iterator accessed.", + N); + R->markInteresting(Val); + R->addVisitor(std::make_unique(Val, Ex)); + C.emitReport(std::move(R)); } } -void InvalidatedIteratorChecker::reportBug(const StringRef &Message, - const SVal &Val, CheckerContext &C, - ExplodedNode *ErrNode) const { - auto R = std::make_unique(*InvalidatedBugType, - Message, ErrNode); - R->markInteresting(Val); - C.emitReport(std::move(R)); +std::shared_ptr +IteratorInvalidationBRVisitor::VisitNode(const ExplodedNode *Succ, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) { + if (Satisfied) + return nullptr; + + const ExplodedNode *Pred = Succ->getFirstPred(); + const Stmt *S = nullptr; + const auto SP = Succ->getLocation().getAs(); + if (SP.hasValue()) { + S = SP->getStmt(); + } else { + const auto E = Succ->getLocation().getAs(); + if (E.hasValue()) { + S = E->getSrc()->getTerminator().getStmt(); + } + } + + if (!S) + return nullptr; + + const auto &StateBefore = Pred->getState(); + const auto &StateAfter = Succ->getState(); + const auto *PosBefore = getIteratorPosition(StateBefore, Iter); + const auto *PosAfter = getIteratorPosition(StateAfter, Iter); + + if (!PosAfter) + return nullptr; + + SmallString<256> Buf; + llvm::raw_svector_ostream Out(Buf); + + StringRef Name; + if (const auto *DRE = dyn_cast(IterExpr->IgnoreParenCasts())) { + Name = DRE->getDecl()->getName(); + } + + if ((PosBefore && PosBefore->isValid()) && + (!PosAfter->isValid())) { + + Out << "Iterator " << (!Name.empty() ? ("'" + Name.str() + "' ") : "" ) + << "invalidated."; + Satisfied = true; + } + + SVal PredIter; + const Expr *PredIterExpr; + if (getPredecessor(Succ, S, BR, PredIter, PredIterExpr)) { + StringRef PredName; + if (const auto *DRE = + dyn_cast(PredIterExpr->IgnoreParenCasts())) { + PredName = DRE->getDecl()->getName(); + } + + if (Name != PredName) { + Out << "Iterator " + << (!PredName.empty() ? ("'" + PredName.str() + "' ") : "" ) + << "assigned" + << (!Name.empty() ? (" to '" + Name.str() + "'.") : "." ); + } + + BR.addVisitor(std::make_unique(PredIter, + PredIterExpr)); + Satisfied = true; + } + + if (!Buf.empty()) { + auto L = PathDiagnosticLocation::createBegin(S, BRC.getSourceManager(), + Succ->getLocationContext()); + + if (!L.isValid() || !L.asLocation().isValid()) + return nullptr; + + auto Piece = std::make_shared(L, Out.str()); + Piece->addRange(S->getSourceRange()); + + return std::move(Piece); + } + + return nullptr; +} + +bool +IteratorInvalidationBRVisitor::getPredecessor(const ExplodedNode *Node, + const Stmt *S, + BugReport &BR, + SVal &PredVal, + const Expr *&PredExpr) const { + auto State = Node->getState(); + const auto *LCtx = Node->getLocationContext(); + + const auto IterSym = Iter.getAsSymbol(); + const auto *IterReg = Iter.getAsRegion(); + if (const auto IterLC = Iter.getAs()) { + IterReg = IterLC->getRegion(); + } + const auto *IterPos = getIteratorPosition(State, Iter); + assert(IterPos && + "This function is only for iterators with existing positions."); + + const auto *E = dyn_cast(S); + if (!E) + return false; + + const auto &RetVal = State->getSVal(E, LCtx); + const auto RetSym = RetVal.getAsSymbol(); + const auto *RetReg = RetVal.getAsRegion(); + if (const auto RetLC = RetVal.getAs()) { + RetReg = RetLC->getRegion(); + } + + // If the current iterator is the return value of an operator and it is a an + // assignment operator then its parameter is the predecessor. + if (const auto *OpCall = dyn_cast(E)) { + if (OpCall->getOperator() == OO_Equal) { + if (IterSym == RetSym || IterReg == RetReg) { + PredExpr = OpCall->getArg(0); + PredVal = State->getSVal(PredExpr, LCtx); + return true; + } + } + // If the current iterator is the result of a C++ construct expression + // and the constructor is a copy or move constructor then its parameter is + // the predecessor. + } else if (const auto *ConstrExpr = dyn_cast(E)) { + if (ConstrExpr->getConstructor()->isCopyOrMoveConstructor()) { + if (IterSym == RetSym || IterReg == RetReg) { + PredExpr = ConstrExpr->getArg(0); + PredVal = State->getSVal(PredExpr, LCtx); + return true; + } + } + } + return false; } void ento::registerInvalidatedIteratorChecker(CheckerManager &mgr) { Index: clang/test/Analysis/invalidated-iterator.cpp =================================================================== --- clang/test/Analysis/invalidated-iterator.cpp +++ clang/test/Analysis/invalidated-iterator.cpp @@ -1,5 +1,5 @@ -// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.InvalidatedIterator -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=false %s -verify -// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.InvalidatedIterator -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify +// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.InvalidatedIterator -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=false -analyzer-output=text %s -verify +// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.InvalidatedIterator -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 -analyzer-output=text %s -verify #include "Inputs/system-header-simulator-cxx.h" @@ -12,8 +12,9 @@ void invalidated_dereference(std::vector &V) { auto i = V.cbegin(); - V.erase(i); + V.erase(i); // expected-note{{Iterator 'i' invalidated}} *i; // expected-warning{{Invalidated iterator accessed}} + // expected-note@-1{{Invalidated iterator accessed}} } void normal_prefix_increment(std::vector &V) { @@ -23,8 +24,9 @@ void invalidated_prefix_increment(std::vector &V) { auto i = V.cbegin(); - V.erase(i); + V.erase(i); // expected-note{{Iterator 'i' invalidated}} ++i; // expected-warning{{Invalidated iterator accessed}} + // expected-note@-1{{Invalidated iterator accessed}} } void normal_prefix_decrement(std::vector &V) { @@ -34,8 +36,9 @@ void invalidated_prefix_decrement(std::vector &V) { auto i = ++V.cbegin(); - V.erase(i); + V.erase(i); // expected-note{{Iterator 'i' invalidated}} --i; // expected-warning{{Invalidated iterator accessed}} + // expected-note@-1{{Invalidated iterator accessed}} } void normal_postfix_increment(std::vector &V) { @@ -45,8 +48,9 @@ void invalidated_postfix_increment(std::vector &V) { auto i = V.cbegin(); - V.erase(i); + V.erase(i); // expected-note{{Iterator 'i' invalidated}} i++; // expected-warning{{Invalidated iterator accessed}} + // expected-note@-1{{Invalidated iterator accessed}} } void normal_postfix_decrement(std::vector &V) { @@ -56,8 +60,9 @@ void invalidated_postfix_decrement(std::vector &V) { auto i = ++V.cbegin(); - V.erase(i); + V.erase(i); // expected-note{{Iterator 'i' invalidated}} i--; // expected-warning{{Invalidated iterator accessed}} + // expected-note@-1{{Invalidated iterator accessed}} } void normal_increment_by_2(std::vector &V) { @@ -67,8 +72,9 @@ void invalidated_increment_by_2(std::vector &V) { auto i = V.cbegin(); - V.erase(i); + V.erase(i); // expected-note{{Iterator 'i' invalidated}} i += 2; // expected-warning{{Invalidated iterator accessed}} + // expected-note@-1{{Invalidated iterator accessed}} } void normal_increment_by_2_copy(std::vector &V) { @@ -78,8 +84,9 @@ void invalidated_increment_by_2_copy(std::vector &V) { auto i = V.cbegin(); - V.erase(i); + V.erase(i); // expected-note{{Iterator 'i' invalidated}} auto j = i + 2; // expected-warning{{Invalidated iterator accessed}} + // expected-note@-1{{Invalidated iterator accessed}} } void normal_decrement_by_2(std::vector &V) { @@ -89,8 +96,9 @@ void invalidated_decrement_by_2(std::vector &V) { auto i = V.cbegin(); - V.erase(i); + V.erase(i); // expected-note{{Iterator 'i' invalidated}} i -= 2; // expected-warning{{Invalidated iterator accessed}} + // expected-note@-1{{Invalidated iterator accessed}} } void normal_decrement_by_2_copy(std::vector &V) { @@ -100,8 +108,9 @@ void invalidated_decrement_by_2_copy(std::vector &V) { auto i = V.cbegin(); - V.erase(i); + V.erase(i); // expected-note{{Iterator 'i' invalidated}} auto j = i - 2; // expected-warning{{Invalidated iterator accessed}} + // expected-note@-1{{Invalidated iterator accessed}} } void normal_subscript(std::vector &V) { @@ -111,8 +120,9 @@ void invalidated_subscript(std::vector &V) { auto i = V.cbegin(); - V.erase(i); + V.erase(i); // expected-note{{Iterator 'i' invalidated}} i[1]; // expected-warning{{Invalidated iterator accessed}} + // expected-note@-1{{Invalidated iterator accessed}} } void assignment(std::vector &V) { @@ -120,3 +130,11 @@ V.erase(i); auto j = V.cbegin(); // no-warning } + +void indirect(std::vector &V) { + auto i = V.cbegin(); + V.erase(i); // expected-note{{Iterator 'i' invalidated}} + auto j = i; // expected-note{{Iterator 'i' assigned to 'j'}} + *j; // expected-warning{{Invalidated iterator accessed}} + // expected-note@-1{{Invalidated iterator accessed}} +}