[clang] [LifetimeSafety] Generalize invalidating member function detection (PR #194907)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-temporal-safety

@llvm/pr-subscribers-clang-analysis

Author: NeKon69

<details>
<summary>Changes</summary>

This PR adds support for invalidating references after reassigning a `unique_ptr` or calling its `reset` member function.

Previously, invalidation handling was limited to container-like types. This PR generalizes the helper for detecting invalidating member calls and adds `unique_ptr`'s `reset` member function as an invalidating one. Since `unique_ptr` is now handled by this helper, reassignment through `operator=` is also treated as invalidating.

Fixes #<!-- -->184630

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


5 Files Affected:

- (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h (+6-6) 
- (modified) clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp (+1-1) 
- (modified) clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp (+8-4) 
- (modified) clang/test/Sema/Inputs/lifetime-analysis.h (+3) 
- (modified) clang/test/Sema/warn-lifetime-safety-invalidations.cpp (+34) 


``````````diff
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h
index 098c15f4a7fb4..433239fb385a3 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h
@@ -71,14 +71,14 @@ bool isGslOwnerType(QualType QT);
 // when ownership is manually transferred.
 bool isUniquePtrRelease(const CXXMethodDecl &MD);
 
-// Returns true if the given method invalidates references to container
-// elements (e.g. vector::push_back). Methods that only invalidate iterators
-// but not references (e.g. unordered_map::emplace) are not considered
-// invalidating here.
+// 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
+// here.
 //
-// Invalidation rules are based on:
+// Container invalidation rules are based on:
 // https://en.cppreference.com/w/cpp/container#Iterator_invalidation
-bool isContainerInvalidationMethod(const CXXMethodDecl &MD);
+bool isInvalidationMethod(const CXXMethodDecl &MD);
 
 /// Returns true for standard library callable wrappers (e.g., std::function)
 /// that can propagate the stored lambda's origins.
diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
index efdb1a1691ae3..2c7cc6b38b3e1 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -771,7 +771,7 @@ void FactsGenerator::handleInvalidatingCall(const Expr *Call,
   if (!MD || !MD->isInstance())
     return;
 
-  if (!isContainerInvalidationMethod(*MD))
+  if (!isInvalidationMethod(*MD))
     return;
   // Heuristics to turn-down false positives.
   auto *DRE = dyn_cast<DeclRefExpr>(Args[0]);
diff --git a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp
index 27d95821dd0b4..e1cc6a098694c 100644
--- a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp
@@ -277,7 +277,7 @@ bool isUniquePtrRelease(const CXXMethodDecl &MD) {
          MD.getNumParams() == 0 && isStdUniquePtr(*MD.getParent());
 }
 
-bool isContainerInvalidationMethod(const CXXMethodDecl &MD) {
+bool isInvalidationMethod(const CXXMethodDecl &MD) {
   const CXXRecordDecl *RD = MD.getParent();
   if (!isInStlNamespace(RD))
     return false;
@@ -342,11 +342,14 @@ bool isContainerInvalidationMethod(const CXXMethodDecl &MD) {
                                          // Assignment
                                          "replace"};
 
-  const StringRef ContainerName = getName(*RD);
+  static const llvm::StringSet<> UniquePtr = {// Reallocation
+                                              "reset"};
+
+  const StringRef RecordName = getName(*RD);
   // TODO: Consider caching this lookup by CXXMethodDecl pointer if this
   // StringSwitch becomes a performance bottleneck.
   const llvm::StringSet<> *InvalidatingMethods =
-      llvm::StringSwitch<const llvm::StringSet<> *>(ContainerName)
+      llvm::StringSwitch<const llvm::StringSet<> *>(RecordName)
           .Case("vector", &Vector)
           .Case("basic_string", &String)
           .Case("deque", &Deque)
@@ -356,6 +359,7 @@ bool isContainerInvalidationMethod(const CXXMethodDecl &MD) {
                  &NodeBased)
           .Cases({"flat_map", "flat_set", "flat_multimap", "flat_multiset"},
                  &Flat)
+          .Case("unique_ptr", &UniquePtr)
           .Default(nullptr);
 
   if (!InvalidatingMethods)
@@ -371,7 +375,7 @@ bool isContainerInvalidationMethod(const CXXMethodDecl &MD) {
     case OO_Subscript: // operator[] : Invalidation only for
                        // `flat_map` (Insert-or-access).
                        // `map` and `unordered_map` are excluded.
-      return ContainerName == "flat_map";
+      return RecordName == "flat_map";
     default:
       return false;
     }
diff --git a/clang/test/Sema/Inputs/lifetime-analysis.h b/clang/test/Sema/Inputs/lifetime-analysis.h
index 14226c7ff1f13..c7473d2928adf 100644
--- a/clang/test/Sema/Inputs/lifetime-analysis.h
+++ b/clang/test/Sema/Inputs/lifetime-analysis.h
@@ -40,6 +40,7 @@ template<typename T, int N>
 T *begin(T (&array)[N]);
 
 using size_t = decltype(sizeof(0));
+using nullptr_t = decltype(nullptr);
 
 template<typename T>
 struct initializer_list {
@@ -199,6 +200,8 @@ struct unique_ptr {
   explicit unique_ptr(T*);
   unique_ptr(unique_ptr<T>&&);
   unique_ptr& operator=(unique_ptr<T>&&);
+  unique_ptr& operator=(std::nullptr_t);
+  void reset();
   ~unique_ptr();
   T* release();
   T &operator*();
diff --git a/clang/test/Sema/warn-lifetime-safety-invalidations.cpp b/clang/test/Sema/warn-lifetime-safety-invalidations.cpp
index 973e095fb68b4..19e2c9299a650 100644
--- a/clang/test/Sema/warn-lifetime-safety-invalidations.cpp
+++ b/clang/test/Sema/warn-lifetime-safety-invalidations.cpp
@@ -539,3 +539,37 @@ void function_captured_ref_invalidated() {
 }
 
 } // namespace callable_wrappers
+
+namespace unique_ptr_invalidation {
+
+void invalid_after_reset() {
+  std::unique_ptr<int> up(new int);
+  int *p = up.get(); // expected-warning {{object whose reference is captured is later invalidated}}
+  up.reset();        // expected-note {{invalidated here}}
+  (void)*p;          // expected-note {{later used here}}
+}
+
+void invalid_after_move_assign() {
+  std::unique_ptr<int> up(new int);
+  std::unique_ptr<int> other(new int);
+  int *p = up.get();     // expected-warning {{object whose reference is captured is later invalidated}}
+  up = std::move(other); // expected-note {{invalidated here}}
+  (void)*p;              // expected-note {{later used here}}
+}
+
+void invalid_after_null_assign() {
+  std::unique_ptr<int> up(new int);
+  int *p = up.get(); // expected-warning {{object whose reference is captured is later invalidated}}
+  up = nullptr;      // expected-note {{invalidated here}}
+  (void)*p;          // expected-note {{later used here}}
+}
+
+void invalid_after_ternary_reset(bool flag) {
+  std::unique_ptr<int> up(new int);
+  std::unique_ptr<int> other(new int);
+  int *p = flag ? up.get() : other.get(); // expected-warning {{object whose reference is captured is later invalidated}}
+  up.reset();                             // expected-note {{invalidated here}}
+  (void)*p;                               // expected-note {{later used here}}
+}
+
+} // namespace unique_ptr_invalidation

``````````

</details>


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