r341790 - [Analyzer] Iterator Checker - Part 4: Mismatched iterator checker for function parameters

Adam Balogh via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 10 02:03:24 PDT 2018


Author: baloghadamsoftware
Date: Mon Sep 10 02:03:22 2018
New Revision: 341790

URL: http://llvm.org/viewvc/llvm-project?rev=341790&view=rev
Log:
[Analyzer] Iterator Checker - Part 4: Mismatched iterator checker for function parameters

New check added to the checker which checks whether iterator parameters of template functions typed by the same template parameter refer to the same container.

Differential Revision: https://reviews.llvm.org/D32845


Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
    cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
    cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h
    cfe/trunk/test/Analysis/invalidated-iterator.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=341790&r1=341789&r2=341790&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Mon Sep 10 02:03:22 2018
@@ -309,12 +309,16 @@ def DeleteWithNonVirtualDtorChecker : Ch
            "destructor in their base class">,
   DescFile<"DeleteWithNonVirtualDtorChecker.cpp">;
 
+def InvalidatedIteratorChecker : Checker<"InvalidatedIterator">,
+  HelpText<"Check for use of invalidated iterators">,
+  DescFile<"IteratorChecker.cpp">;
+
 def IteratorRangeChecker : Checker<"IteratorRange">,
   HelpText<"Check for iterators used outside their valid ranges">,
   DescFile<"IteratorChecker.cpp">;
 
-def InvalidatedIteratorChecker : Checker<"InvalidatedIterator">,
-  HelpText<"Check for use of invalidated iterators">,
+def MismatchedIteratorChecker : Checker<"MismatchedIterator">,
+  HelpText<"Check for use of iterators of different containers where iterators of the same container are expected">,
   DescFile<"IteratorChecker.cpp">;
 
 def MisusedMovedObjectChecker: Checker<"MisusedMovedObject">,

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp?rev=341790&r1=341789&r2=341790&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp Mon Sep 10 02:03:22 2018
@@ -198,6 +198,7 @@ class IteratorChecker
                      eval::Assume> {
 
   std::unique_ptr<BugType> OutOfRangeBugType;
+  std::unique_ptr<BugType> MismatchedBugType;
   std::unique_ptr<BugType> InvalidatedBugType;
 
   void handleComparison(CheckerContext &C, const SVal &RetVal, const SVal &LVal,
@@ -221,8 +222,14 @@ class IteratorChecker
   void verifyRandomIncrOrDecr(CheckerContext &C, OverloadedOperatorKind Op,
                               const SVal &RetVal, const SVal &LHS,
                               const SVal &RHS) const;
+  void verifyMatch(CheckerContext &C, const SVal &Iter1,
+                   const SVal &Iter2) const;
+
   void reportOutOfRangeBug(const StringRef &Message, const SVal &Val,
                            CheckerContext &C, ExplodedNode *ErrNode) const;
+  void reportMismatchedBug(const StringRef &Message, const SVal &Val1,
+                           const SVal &Val2, CheckerContext &C,
+                           ExplodedNode *ErrNode) const;
   void reportInvalidatedBug(const StringRef &Message, const SVal &Val,
                             CheckerContext &C, ExplodedNode *ErrNode) const;
 
@@ -231,6 +238,7 @@ public:
 
   enum CheckKind {
     CK_IteratorRangeChecker,
+    CK_MismatchedIteratorChecker,
     CK_InvalidatedIteratorChecker,
     CK_NumCheckKinds
   };
@@ -312,6 +320,8 @@ const ContainerData *getContainerData(Pr
 ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
                                  const ContainerData &CData);
 bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont);
+bool isBoundThroughLazyCompoundVal(const Environment &Env,
+                                   const MemRegion *Reg);
 bool isOutOfRange(ProgramStateRef State, const IteratorPosition &Pos);
 bool isZero(ProgramStateRef State, const NonLoc &Val);
 } // namespace
@@ -320,6 +330,9 @@ IteratorChecker::IteratorChecker() {
   OutOfRangeBugType.reset(
       new BugType(this, "Iterator out of range", "Misuse of STL APIs"));
   OutOfRangeBugType->setSuppressOnSink(true);
+  MismatchedBugType.reset(
+      new BugType(this, "Iterator(s) mismatched", "Misuse of STL APIs"));
+  MismatchedBugType->setSuppressOnSink(true);
   InvalidatedBugType.reset(
       new BugType(this, "Iterator invalidated", "Misuse of STL APIs"));
   InvalidatedBugType->setSuppressOnSink(true);
@@ -368,6 +381,65 @@ void IteratorChecker::checkPreCall(const
         verifyDereference(C, Call.getArgSVal(0));
       }
     }
+  } else if (!isa<CXXConstructorCall>(&Call)) {
+    // The main purpose of iterators is to abstract away from different
+    // containers and provide a (maybe limited) uniform access to them.
+    // This implies that any correctly written template function that
+    // works on multiple containers using iterators takes different
+    // template parameters for different containers. So we can safely
+    // assume that passing iterators of different containers as arguments
+    // whose type replaces the same template parameter is a bug.
+    //
+    // Example:
+    // template<typename I1, typename I2>
+    // void f(I1 first1, I1 last1, I2 first2, I2 last2);
+    // 
+    // In this case the first two arguments to f() must be iterators must belong
+    // to the same container and the last to also to the same container but
+    // not neccessarily to the same as the first two.
+
+    if (!ChecksEnabled[CK_MismatchedIteratorChecker])
+      return;
+
+    const auto *Templ = Func->getPrimaryTemplate();
+    if (!Templ)
+      return;
+
+    const auto *TParams = Templ->getTemplateParameters();
+    const auto *TArgs = Func->getTemplateSpecializationArgs();
+
+    // Iterate over all the template parameters
+    for (size_t I = 0; I < TParams->size(); ++I) {
+      const auto *TPDecl = dyn_cast<TemplateTypeParmDecl>(TParams->getParam(I));
+      if (!TPDecl)
+        continue;
+
+      if (TPDecl->isParameterPack())
+        continue;
+
+      const auto TAType = TArgs->get(I).getAsType();
+      if (!isIteratorType(TAType))
+        continue;
+
+      SVal LHS = UndefinedVal();
+
+      // For every template parameter which is an iterator type in the
+      // instantiation look for all functions' parameters' type by it and
+      // check whether they belong to the same container
+      for (auto J = 0U; J < Func->getNumParams(); ++J) {
+        const auto *Param = Func->getParamDecl(J);
+        const auto *ParamType =
+            Param->getType()->getAs<SubstTemplateTypeParmType>();
+        if (!ParamType ||
+            ParamType->getReplacedParameter()->getDecl() != TPDecl)
+          continue;
+        if (LHS.isUndef()) {
+          LHS = Call.getArgSVal(J);
+        } else {
+          verifyMatch(C, LHS, Call.getArgSVal(J));
+        }
+      }
+    }
   }
 }
 
@@ -529,7 +601,12 @@ void IteratorChecker::checkDeadSymbols(S
   auto RegionMap = State->get<IteratorRegionMap>();
   for (const auto Reg : RegionMap) {
     if (!SR.isLiveRegion(Reg.first)) {
-      State = State->remove<IteratorRegionMap>(Reg.first);
+      // The region behind the `LazyCompoundVal` is often cleaned up before
+      // the `LazyCompoundVal` itself. If there are iterator positions keyed
+      // by these regions their cleanup must be deferred.
+      if (!isBoundThroughLazyCompoundVal(State->getEnvironment(), Reg.first)) {
+        State = State->remove<IteratorRegionMap>(Reg.first);
+      }
     }
   }
 
@@ -820,6 +897,21 @@ void IteratorChecker::verifyRandomIncrOr
   }
 }
 
+void IteratorChecker::verifyMatch(CheckerContext &C, const SVal &Iter1,
+                                  const SVal &Iter2) const {
+  // Verify match between the containers of two iterators
+  auto State = C.getState();
+  const auto *Pos1 = getIteratorPosition(State, Iter1);
+  const auto *Pos2 = getIteratorPosition(State, Iter2);
+  if (Pos1 && Pos2 && Pos1->getContainer() != Pos2->getContainer()) {
+    auto *N = C.generateNonFatalErrorNode(State);
+    if (!N)
+      return;
+    reportMismatchedBug("Iterators of different containers used where the "
+                        "same container is expected.", Iter1, Iter2, C, N);
+  }
+}
+
 void IteratorChecker::handleBegin(CheckerContext &C, const Expr *CE,
                                   const SVal &RetVal, const SVal &Cont) const {
   const auto *ContReg = Cont.getAsRegion();
@@ -917,6 +1009,16 @@ void IteratorChecker::reportOutOfRangeBu
   C.emitReport(std::move(R));
 }
 
+void IteratorChecker::reportMismatchedBug(const StringRef &Message,
+                                          const SVal &Val1, const SVal &Val2,
+                                          CheckerContext &C,
+                                          ExplodedNode *ErrNode) const {
+  auto R = llvm::make_unique<BugReport>(*MismatchedBugType, Message, ErrNode);
+  R->markInteresting(Val1);
+  R->markInteresting(Val2);
+  C.emitReport(std::move(R));
+}
+
 void IteratorChecker::reportInvalidatedBug(const StringRef &Message,
                                            const SVal &Val, CheckerContext &C,
                                            ExplodedNode *ErrNode) const {
@@ -1261,6 +1363,18 @@ bool hasLiveIterators(ProgramStateRef St
   return false;
 }
 
+bool isBoundThroughLazyCompoundVal(const Environment &Env,
+                                   const MemRegion *Reg) {
+  for (const auto Binding: Env) {
+    if (const auto LCVal = Binding.second.getAs<nonloc::LazyCompoundVal>()) {
+      if (LCVal->getRegion() == Reg)
+        return true;
+    }
+  }
+
+  return false;
+}
+
 template <typename Condition, typename Process>
 ProgramStateRef processIteratorPositions(ProgramStateRef State, Condition Cond,
                                          Process Proc) {
@@ -1374,4 +1488,5 @@ bool compare(ProgramStateRef State, NonL
   }
 
 REGISTER_CHECKER(IteratorRangeChecker)
+REGISTER_CHECKER(MismatchedIteratorChecker)
 REGISTER_CHECKER(InvalidatedIteratorChecker)

Modified: cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h?rev=341790&r1=341789&r2=341790&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h (original)
+++ cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h Mon Sep 10 02:03:22 2018
@@ -642,6 +642,12 @@ namespace std {
   template <class InputIterator, class T>
   InputIterator find(InputIterator first, InputIterator last, const T &val);
 
+  template <class ForwardIterator1, class ForwardIterator2>
+  ForwardIterator1 find_first_of(ForwardIterator1 first1,
+                                 ForwardIterator1 last1,
+                                 ForwardIterator2 first2,
+                                 ForwardIterator2 last2);
+
   template <class InputIterator, class OutputIterator>
   OutputIterator copy(InputIterator first, InputIterator last,
                       OutputIterator result);

Modified: cfe/trunk/test/Analysis/invalidated-iterator.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/invalidated-iterator.cpp?rev=341790&r1=341789&r2=341790&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/invalidated-iterator.cpp (original)
+++ cfe/trunk/test/Analysis/invalidated-iterator.cpp Mon Sep 10 02:03:22 2018
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.InvalidatedIterator -analyzer-config aggressive-relational-comparison-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-relational-comparison-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 %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
 
 #include "Inputs/system-header-simulator-cxx.h"
 




More information about the cfe-commits mailing list