[clang] [LifetimeSafety] Refactor function handlng invalidating calls (PR #195064)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-analysis

Author: NeKon69

<details>
<summary>Changes</summary>

Previously `handleInvalidatingCall` only handled member function calls. This patch extends it to also handle free functions, and adds `std::destroy_at` as one such invalidating call.

Comes as a part of completion of #<!-- -->164963.

Assisted-by: GPT-5.4 for writing some of the tests.

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


5 Files Affected:

- (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h (+2) 
- (modified) clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp (+10-7) 
- (modified) clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp (+9) 
- (modified) clang/test/Sema/Inputs/lifetime-analysis.h (+3) 
- (modified) clang/test/Sema/warn-lifetime-safety-invalidations.cpp (+67) 


``````````diff
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h
index 433239fb385a3..ab85272b75c53 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h
@@ -80,6 +80,8 @@ bool isUniquePtrRelease(const CXXMethodDecl &MD);
 // https://en.cppreference.com/w/cpp/container#Iterator_invalidation
 bool isInvalidationMethod(const CXXMethodDecl &MD);
 
+// Returns true if the given function is a destructor/deleter (destroy_at).
+bool isDestructionFunc(const FunctionDecl &FD);
 /// Returns true for standard library callable wrappers (e.g., std::function)
 /// that can propagate the stored lambda's origins.
 bool isStdCallableWrapperType(const CXXRecordDecl *RD);
diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
index 2c7cc6b38b3e1..d02039ec7bb74 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -768,18 +768,21 @@ void FactsGenerator::handleInvalidatingCall(const Expr *Call,
                                             const FunctionDecl *FD,
                                             ArrayRef<const Expr *> Args) {
   const auto *MD = dyn_cast<CXXMethodDecl>(FD);
-  if (!MD || !MD->isInstance())
+  const bool IsInvalidatingMethod = MD && isInvalidationMethod(*MD);
+  const bool IsDestruction = isDestructionFunc(*FD);
+  if (!IsInvalidatingMethod && !IsDestruction)
     return;
 
-  if (!isInvalidationMethod(*MD))
-    return;
-  // Heuristics to turn-down false positives.
-  auto *DRE = dyn_cast<DeclRefExpr>(Args[0]);
+  // `destroy_at` get an implicit cast for the object argument, methods already
+  // give us the right shape.
+  const Expr *Target = IsDestruction ? Args[0]->IgnoreImpCasts() : Args[0];
+
+  // Heuristics to turn down false positives.
+  const auto *DRE = dyn_cast<DeclRefExpr>(Target);
   if (!DRE || DRE->getDecl()->getType()->isReferenceType())
     return;
 
-  OriginList *ThisList = getOriginsList(*Args[0]);
-  if (ThisList)
+  if (OriginList *ThisList = getOriginsList(*Args[0]))
     CurrentBlockFacts.push_back(FactMgr.createFact<InvalidateOriginFact>(
         ThisList->getOuterOriginID(), Call));
 }
diff --git a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp
index e1cc6a098694c..2519c09cce525 100644
--- a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp
@@ -387,6 +387,15 @@ bool isInvalidationMethod(const CXXMethodDecl &MD) {
   return InvalidatingMethods->contains(MD.getName());
 }
 
+bool isDestructionFunc(const FunctionDecl &FD) {
+  if (isa<CXXDestructorDecl>(FD))
+    return true;
+  if (const auto *II = FD.getIdentifier();
+      isInStlNamespace(&FD) && II && II->getName() == "destroy_at")
+    return true;
+  return false;
+}
+
 bool isStdCallableWrapperType(const CXXRecordDecl *RD) {
   if (!RD || !isInStlNamespace(RD))
     return false;
diff --git a/clang/test/Sema/Inputs/lifetime-analysis.h b/clang/test/Sema/Inputs/lifetime-analysis.h
index c7473d2928adf..eaedba372ee5e 100644
--- a/clang/test/Sema/Inputs/lifetime-analysis.h
+++ b/clang/test/Sema/Inputs/lifetime-analysis.h
@@ -214,6 +214,9 @@ unique_ptr<T> make_unique(Args&&... args) {
   return unique_ptr<T>(new T(args...));
 }
 
+template <class T>
+void destroy_at(T *);
+
 template<typename T>
 struct shared_ptr {
   shared_ptr();
diff --git a/clang/test/Sema/warn-lifetime-safety-invalidations.cpp b/clang/test/Sema/warn-lifetime-safety-invalidations.cpp
index 19e2c9299a650..f21f7309af564 100644
--- a/clang/test/Sema/warn-lifetime-safety-invalidations.cpp
+++ b/clang/test/Sema/warn-lifetime-safety-invalidations.cpp
@@ -540,6 +540,73 @@ void function_captured_ref_invalidated() {
 
 } // namespace callable_wrappers
 
+namespace manual_destruction {
+
+void explicit_destructor_invalidates_pointer() {
+  std::string s = "42";
+  const char *p = s.data(); // expected-warning {{object whose reference is captured is later invalidated}}
+  s.~basic_string();        // expected-note {{invalidated here}}
+  (void)*p;                 // expected-note {{later used here}}
+}
+
+void pointer_destructor_invalidates_pointer() {
+  char storage[sizeof(std::string)];
+  std::string *obj = new (storage) std::string("42"); // expected-warning {{object whose reference is captured is later invalidated}}
+  const char *p = obj->data();
+  obj->~basic_string();                               // expected-note {{invalidated here}}
+  (void)*p;                                           // expected-note {{later used here}}
+}
+
+void destroy_at_invalidates_pointer() {
+  char storage[sizeof(std::string)];
+  std::string *obj = new (storage) std::string("42"); // expected-warning {{object whose reference is captured is later invalidated}}
+  const char *p = obj->data();
+  std::destroy_at(obj);                               // expected-note {{invalidated here}}
+  (void)*p;                                           // expected-note {{later used here}}
+}
+
+void destroy_at_then_placement_new_rescues_pointer() {
+  char storage[sizeof(std::string)];
+  std::string *obj = new (storage) std::string("42");
+  const char *p = obj->data();
+  std::destroy_at(obj);
+  obj = new (storage) std::string("23");
+  p = obj->data();
+  (void)*p;
+}
+
+// FIXME: False-negative
+void destroy_at_invalidates_array_pointer() {
+  std::string arr[1] = {"42"};
+  std::string (&arr_ref)[1] = arr;
+  const char *p = arr[0].data();
+  std::destroy_at(&arr_ref);
+  (void)*p;
+}
+
+// FIXME: False-negative
+void reference_destructor_invalidates_pointer() {
+  std::string s = "42";
+  std::string &ref = s;
+  const char *p = ref.data();
+  std::destroy_at(&ref);
+  (void)*p;
+}
+
+struct StringOwner {
+  std::string s;
+};
+
+// FIXME: False-negative
+void member_destructor_invalidates_pointer() {
+  StringOwner owner = {"42"};
+  const char *p = owner.s.data();
+  owner.s.~basic_string();
+  (void)*p;
+}
+
+} // namespace manual_destruction
+
 namespace unique_ptr_invalidation {
 
 void invalid_after_reset() {

``````````

</details>


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