[clang] [LifetimeSafety] Add support for iterator arithmetic (PR #195442)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-temporal-safety

Author: NeKon69

<details>
<summary>Changes</summary>

This PR adds origin propagation through iterator arithmetic.

It also adds a few tests to check the new behavior.

Closes #<!-- -->190140

---
Full diff: https://github.com/llvm/llvm-project/pull/195442.diff


5 Files Affected:

- (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h (+3) 
- (modified) clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp (+17) 
- (modified) clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp (+19) 
- (modified) clang/test/Sema/Inputs/lifetime-analysis.h (+65-1) 
- (modified) clang/test/Sema/warn-lifetime-safety-invalidations.cpp (+30) 


``````````diff
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h
index 54d52fee6bea7..2f814216a4285 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h
@@ -71,6 +71,9 @@ bool isGslOwnerType(QualType QT);
 // when ownership is manually transferred.
 bool isUniquePtrRelease(const CXXMethodDecl &MD);
 
+bool isIteratorType(const CXXRecordDecl *RD);
+
+bool isPropagatingIteratorOp(OverloadedOperatorKind OP);
 // Returns true if the given method invalidates references tracked by lifetime
 // analysis (e.g. vector::push_back). Methods that only invalidate iterators but
 // not references (e.g. unordered_map::emplace) are not considered invalidating
diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
index dc80d2783fc03..f32e6b57c10e5 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -542,6 +542,23 @@ void FactsGenerator::VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE) {
     }
   }
 
+  if (OCE->getNumArgs() < 3 && isPropagatingIteratorOp(OCE->getOperator())) {
+    const Expr *IteratorArg = nullptr;
+    for (const Expr *Arg : OCE->arguments()) {
+      if (isIteratorType(Arg->getType()->getAsCXXRecordDecl())) {
+        IteratorArg = Arg;
+        break;
+      }
+    }
+
+    if (IteratorArg) {
+      flow(getOriginsList(*OCE),
+           getRValueOrigins(IteratorArg, getOriginsList(*IteratorArg)),
+           /*Kill=*/true);
+      return;
+    }
+  }
+
   ArrayRef Args = {OCE->getArgs(), OCE->getNumArgs()};
   // For `static operator()`, the first argument is the object argument,
   // remove it from the argument list to avoid off-by-one errors.
diff --git a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp
index b33fb8edc100b..15ee38ebd2b09 100644
--- a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp
@@ -283,6 +283,25 @@ bool isUniquePtrRelease(const CXXMethodDecl &MD) {
          MD.getNumParams() == 0 && isStdUniquePtr(*MD.getParent());
 }
 
+bool isIteratorType(const CXXRecordDecl *RD) {
+  // FIXME: Extend this to cover more iterator wrapper types used by standard
+  // library implementations.
+  static const llvm::StringSet<> Iterators = {
+      // Standard reverse iterator wrapper.
+      "reverse_iterator",
+      // libstdc++ contiguous iterator wrapper.
+      "__normal_iterator",
+      // libc++ contiguous iterator wrapper.
+      "__wrap_iter"};
+  return RD && isInStlNamespace(RD) && Iterators.contains(getName(*RD));
+}
+
+bool isPropagatingIteratorOp(OverloadedOperatorKind OP) {
+  llvm::SmallDenseSet<OverloadedOperatorKind> PropagatingOperators = {
+      OO_Plus, OO_Minus, OO_PlusPlus, OO_MinusMinus};
+  return PropagatingOperators.contains(OP);
+}
+
 bool isInvalidationMethod(const CXXMethodDecl &MD) {
   const CXXRecordDecl *RD = MD.getParent();
   if (!isInStlNamespace(RD))
diff --git a/clang/test/Sema/Inputs/lifetime-analysis.h b/clang/test/Sema/Inputs/lifetime-analysis.h
index eaedba372ee5e..4c6f833de9e3b 100644
--- a/clang/test/Sema/Inputs/lifetime-analysis.h
+++ b/clang/test/Sema/Inputs/lifetime-analysis.h
@@ -11,6 +11,27 @@ template<typename T>
 bool operator==(basic_iterator<T>, basic_iterator<T>);
 template<typename T>
 bool operator!=(basic_iterator<T>, basic_iterator<T>);
+
+// These iterator spellings match libstdc++ names documented at:
+// https://gcc.gnu.org/onlinedocs/libstdc++/libstdc++-html-USERS-4.2/namespace____gnu__cxx.html
+template <typename T>
+struct __normal_iterator {
+  __normal_iterator operator++();
+  __normal_iterator operator--();
+  __normal_iterator operator+(int) const;
+  __normal_iterator operator-(int) const;
+  T& operator*() const;
+  T* operator->() const;
+};
+
+template<typename T>
+bool operator==(__normal_iterator<T>, __normal_iterator<T>);
+template<typename T>
+bool operator!=(__normal_iterator<T>, __normal_iterator<T>);
+template<typename T>
+__normal_iterator<T> operator+(int, __normal_iterator<T>);
+template<typename T>
+__normal_iterator<T> operator-(int, __normal_iterator<T>);
 }
 
 namespace std {
@@ -47,11 +68,54 @@ struct initializer_list {
   const T* ptr; size_t sz;
 };
 template<typename T> class allocator {};
+
+template <typename T>
+struct __wrap_iter {
+  __wrap_iter operator++();
+  __wrap_iter operator--();
+  __wrap_iter operator+(int) const;
+  __wrap_iter operator-(int) const;
+  T& operator*() const;
+  T* operator->() const;
+};
+
+template<typename T>
+bool operator==(__wrap_iter<T>, __wrap_iter<T>);
+template<typename T>
+bool operator!=(__wrap_iter<T>, __wrap_iter<T>);
+template<typename T>
+__wrap_iter<T> operator+(int, __wrap_iter<T>);
+template<typename T>
+__wrap_iter<T> operator-(int, __wrap_iter<T>);
+
+template <typename Iterator>
+struct reverse_iterator {
+  reverse_iterator operator++();
+  reverse_iterator operator--();
+  reverse_iterator operator+(int) const;
+  reverse_iterator operator-(int) const;
+  decltype(*Iterator()) operator*() const;
+};
+
+template <typename Iterator>
+reverse_iterator<Iterator> operator+(int, reverse_iterator<Iterator>);
+template <typename Iterator>
+reverse_iterator<Iterator> operator-(int, reverse_iterator<Iterator>);
+
 template <typename T, typename Alloc = allocator<T>>
 struct vector {
-  typedef __gnu_cxx::basic_iterator<T> iterator;
+  using iterator = __wrap_iter<T>;
+  using const_iterator = __wrap_iter<const T>;
+  using reverse_iterator = std::reverse_iterator<iterator>;
+  using const_reverse_iterator = std::reverse_iterator<const_iterator>;
   iterator begin();
   iterator end();
+  const_iterator cbegin() const;
+  const_iterator cend() const;
+  reverse_iterator rbegin();
+  reverse_iterator rend();
+  const_reverse_iterator crbegin() const;
+  const_reverse_iterator crend() const;
   const T *data() const;
   vector();
   ~vector();
diff --git a/clang/test/Sema/warn-lifetime-safety-invalidations.cpp b/clang/test/Sema/warn-lifetime-safety-invalidations.cpp
index f1044e2ad1cdd..5c812ff318cee 100644
--- a/clang/test/Sema/warn-lifetime-safety-invalidations.cpp
+++ b/clang/test/Sema/warn-lifetime-safety-invalidations.cpp
@@ -252,6 +252,36 @@ void IteratorUsedAfterPushBack(std::vector<int> v) {
   }
   ++it;             // expected-note {{later used here}}
 }
+
+void IteratorUsedAfterPreIncrement() {
+  std::vector<int> v;
+  auto it = v.begin();      // expected-warning {{object whose reference is captured is later invalidated}}
+  auto next = ++it;
+  v.push_back(4);           // expected-note {{invalidated here}}
+  (void)*next;              // expected-note {{later used here}}
+}
+
+void IteratorUsedAfterPreDecrement(std::vector<int> v) {
+  auto it = v.rbegin();     // expected-warning {{object whose reference is captured is later invalidated}}
+  auto prev = --it;
+  v.resize(8);              // expected-note {{invalidated here}}
+  (void)*prev;              // expected-note {{later used here}}
+}
+
+void IteratorUsedAfterAddition() {
+  std::vector<int> v;
+  auto it = v.cbegin();     // expected-warning {{object whose reference is captured is later invalidated}}
+  auto next = it + 5;
+  v.insert(v.begin(), 0);   // expected-note {{invalidated here}}
+  (void)*next;              // expected-note {{later used here}}
+}
+
+void IteratorUsedAfterReverseSubtraction(std::vector<int> v) {
+  auto it = v.crbegin();    // expected-warning {{object whose reference is captured is later invalidated}}
+  auto prev = 5 - it;
+  v.clear();                // expected-note {{invalidated here}}
+  (void)*prev;              // expected-note {{later used here}}
+}
 }  // namespace SimpleInvalidIterators
 
 namespace ElementReferences {

``````````

</details>


https://github.com/llvm/llvm-project/pull/195442