[analyzer] MallocChecker: Recognize std::atomics in smart pointer suppression. (#90918)

Fixes #90498.

Same as 5337efc for atomic builtins, but for `std::atomic` this
time. This is useful because even though the actual builtin atomic is
still there, it may be buried beyond the inlining depth limit.

Also add one popular custom smart pointer class name to the name-based
heuristics, which isn't necessary to fix the bug but arguably a good
idea regardless.

Author: haoNoQ

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

@@ -3451,7 +3451,7 @@ static bool isReferenceCountingPointerDestructor(const CXXDestructorDecl *DD) {
     if (N.contains_insensitive("ptr") || N.contains_insensitive("pointer")) {
       if (N.contains_insensitive("ref") || N.contains_insensitive("cnt") ||
           N.contains_insensitive("intrusive") ||
-          N.contains_insensitive("shared")) {
+          N.contains_insensitive("shared") || N.ends_with_insensitive("rc")) {
         return true;
       }
     }
@@ -3483,13 +3483,24 @@ PathDiagnosticPieceRef MallocBugVisitor::VisitNode(const ExplodedNode *N,
   // original reference count is positive, we should not report use-after-frees
   // on objects deleted in such destructors. This can probably be improved
   // through better shared pointer modeling.
-  if (ReleaseDestructorLC) {
+  if (ReleaseDestructorLC && (ReleaseDestructorLC == CurrentLC ||
+                              ReleaseDestructorLC->isParentOf(CurrentLC))) {
     if (const auto *AE = dyn_cast<AtomicExpr>(S)) {
+      // Check for manual use of atomic builtins.
       AtomicExpr::AtomicOp Op = AE->getOp();
       if (Op == AtomicExpr::AO__c11_atomic_fetch_add ||
           Op == AtomicExpr::AO__c11_atomic_fetch_sub) {
-        if (ReleaseDestructorLC == CurrentLC ||
-            ReleaseDestructorLC->isParentOf(CurrentLC)) {
+        BR.markInvalid(getTag(), S);
+      }
+    } else if (const auto *CE = dyn_cast<CallExpr>(S)) {
+      // Check for `std::atomic` and such. This covers both regular method calls
+      // and operator calls.
+      if (const auto *MD =
+              dyn_cast_or_null<CXXMethodDecl>(CE->getDirectCallee())) {
+        const CXXRecordDecl *RD = MD->getParent();
+        // A bit wobbly with ".contains()" because it may be like
+        // "__atomic_base" or something.
+        if (StringRef(RD->getNameAsString()).contains("atomic")) {
           BR.markInvalid(getTag(), S);
         }
       }

clang/test/Analysis/Inputs/system-header-simulator-cxx.h

@@ -1260,6 +1260,13 @@ template<
     iterator end() const { return iterator(val + 1); }
 };
 
+template <typename T>
+class atomic {
+public:
+  T operator++();
+  T operator--();
+};
+
 namespace execution {
 class sequenced_policy {};
 }

clang/test/Analysis/NewDelete-atomics.cpp

@@ -20,7 +20,7 @@ typedef enum memory_order {
   memory_order_seq_cst = __ATOMIC_SEQ_CST
 } memory_order;
 
-class Obj {
+class RawObj {
   int RefCnt;
 
 public:
@@ -37,11 +37,27 @@ class Obj {
   void foo();
 };
 
+class StdAtomicObj {
+  std::atomic<int> RefCnt;
+
+public:
+  int incRef() {
+    return ++RefCnt;
+  }
+
+  int decRef() {
+    return --RefCnt;
+  }
+
+  void foo();
+};
+
+template <typename T>
 class IntrusivePtr {
-  Obj *Ptr;
+  T *Ptr;
 
 public:
-  IntrusivePtr(Obj *Ptr) : Ptr(Ptr) {
+  IntrusivePtr(T *Ptr) : Ptr(Ptr) {
     Ptr->incRef();
   }
 
@@ -55,22 +71,106 @@ class IntrusivePtr {
       delete Ptr;
   }
 
-  Obj *getPtr() const { return Ptr; } // no-warning
+  T *getPtr() const { return Ptr; } // no-warning
+};
+
+// Also IntrusivePtr but let's dodge name-based heuristics.
+template <typename T>
+class DifferentlyNamed {
+  T *Ptr;
+
+public:
+  DifferentlyNamed(T *Ptr) : Ptr(Ptr) {
+    Ptr->incRef();
+  }
+
+  DifferentlyNamed(const DifferentlyNamed &Other) : Ptr(Other.Ptr) {
+    Ptr->incRef();
+  }
+
+  ~DifferentlyNamed() {
+  // We should not take the path on which the object is deleted.
+    if (Ptr->decRef() == 1)
+      delete Ptr;
+  }
+
+  T *getPtr() const { return Ptr; } // no-warning
 };
 
 void testDestroyLocalRefPtr() {
-  IntrusivePtr p1(new Obj());
+  IntrusivePtr<RawObj> p1(new RawObj());
+  {
+    IntrusivePtr<RawObj> p2(p1);
+  }
+
+  // p1 still maintains ownership. The object is not deleted.
+  p1.getPtr()->foo(); // no-warning
+}
+
+void testDestroySymbolicRefPtr(const IntrusivePtr<RawObj> &p1) {
+  {
+    IntrusivePtr<RawObj> p2(p1);
+  }
+
+  // p1 still maintains ownership. The object is not deleted.
+  p1.getPtr()->foo(); // no-warning
+}
+
+void testDestroyLocalRefPtrWithAtomics() {
+  IntrusivePtr<StdAtomicObj> p1(new StdAtomicObj());
+  {
+    IntrusivePtr<StdAtomicObj> p2(p1);
+  }
+
+  // p1 still maintains ownership. The object is not deleted.
+  p1.getPtr()->foo(); // no-warning
+}
+
+
+void testDestroyLocalRefPtrWithAtomics(const IntrusivePtr<StdAtomicObj> &p1) {
   {
-    IntrusivePtr p2(p1);
+    IntrusivePtr<StdAtomicObj> p2(p1);
   }
 
   // p1 still maintains ownership. The object is not deleted.
   p1.getPtr()->foo(); // no-warning
 }
 
-void testDestroySymbolicRefPtr(const IntrusivePtr &p1) {
+void testDestroyLocalRefPtrDifferentlyNamed() {
+  DifferentlyNamed<RawObj> p1(new RawObj());
+  {
+    DifferentlyNamed<RawObj> p2(p1);
+  }
+
+  // p1 still maintains ownership. The object is not deleted.
+  p1.getPtr()->foo(); // no-warning
+}
+
+void testDestroySymbolicRefPtrDifferentlyNamed(
+    const DifferentlyNamed<RawObj> &p1) {
+  {
+    DifferentlyNamed<RawObj> p2(p1);
+  }
+
+  // p1 still maintains ownership. The object is not deleted.
+  p1.getPtr()->foo(); // no-warning
+}
+
+void testDestroyLocalRefPtrWithAtomicsDifferentlyNamed() {
+  DifferentlyNamed<StdAtomicObj> p1(new StdAtomicObj());
+  {
+    DifferentlyNamed<StdAtomicObj> p2(p1);
+  }
+
+  // p1 still maintains ownership. The object is not deleted.
+  p1.getPtr()->foo(); // no-warning
+}
+
+
+void testDestroyLocalRefPtrWithAtomicsDifferentlyNamed(
+    const DifferentlyNamed<StdAtomicObj> &p1) {
   {
-    IntrusivePtr p2(p1);
+    DifferentlyNamed<StdAtomicObj> p2(p1);
   }
 
   // p1 still maintains ownership. The object is not deleted.