[clang] [analyzer] [MallocChecker] suspect all release functions as candite for supression (PR #104599)

Pavel Skripkin via cfe-commits cfe-commits at lists.llvm.org
Sat Aug 31 09:44:40 PDT 2024


================
@@ -3648,35 +3648,38 @@ PathDiagnosticPieceRef MallocBugVisitor::VisitNode(const ExplodedNode *N,
           return nullptr;
         }
 
-      // See if we're releasing memory while inlining a destructor
-      // (or one of its callees). This turns on various common
-      // false positive suppressions.
-      bool FoundAnyDestructor = false;
-      for (const LocationContext *LC = CurrentLC; LC; LC = LC->getParent()) {
-        if (const auto *DD = dyn_cast<CXXDestructorDecl>(LC->getDecl())) {
-          if (isReferenceCountingPointerDestructor(DD)) {
-            // This immediately looks like a reference-counting destructor.
-            // We're bad at guessing the original reference count of the object,
-            // so suppress the report for now.
-            BR.markInvalid(getTag(), DD);
-          } else if (!FoundAnyDestructor) {
-            assert(!ReleaseDestructorLC &&
+        // See if we're releasing memory while inlining a destructor or
+        // functions that decrement reference counters (or one of its callees).
+        // This turns on various common false positive suppressions.
+        bool FoundAnyReleaseFunction = false;
+        for (const LocationContext *LC = CurrentLC; LC; LC = LC->getParent()) {
+          if (const auto *DD = dyn_cast<CXXDestructorDecl>(LC->getDecl())) {
+            if (isReferenceCountingPointerDestructor(DD)) {
+              // This immediately looks like a reference-counting destructor.
+              // We're bad at guessing the original reference count of the
+              // object, so suppress the report for now.
+              BR.markInvalid(getTag(), DD);
+              continue;
+            }
+          }
+
+          if (!FoundAnyReleaseFunction) {
----------------
pskrgag wrote:

So, based on my local testings:

Following does not produce warnings before and after this PR

```cpp
template <typename T> class DifferentlyNamedWithInlineRef {
  T *Ptr;
  _Atomic int refs;

public:
  DifferentlyNamedWithInlineRef(T *Ptr) : Ptr(Ptr), refs(1) {}

  DifferentlyNamedWithInlineRef(const DifferentlyNamedWithInlineRef &Other)
      : Ptr(Other.Ptr),
        refs(__c11_atomic_fetch_add(&refs, 1, memory_order_relaxed) + 1) {}

  void releasePtr() { delete Ptr; }

  ~DifferentlyNamedWithInlineRef() {
    if (__c11_atomic_fetch_sub(&refs, 1, memory_order_relaxed) == 1)
      releasePtr();
  }

  T *getPtr() const { return Ptr; } // no-warning
};

```

but following gives regression

```cpp
template <typename T>
class DifferentlyNamedOutlineRelease {
  T *Ptr;

public:
  DifferentlyNamedOutlineRelease(T *Ptr) : Ptr(Ptr) {
    Ptr->incRef();
  }

  DifferentlyNamedOutlineRelease(const DifferentlyNamedOutlineRelease &Other) : Ptr(Other.Ptr) {
    Ptr->incRef();
  }

  void releasePtr(void) {
      delete Ptr;
  }

  ~DifferentlyNamedOutlineRelease() {
    if (Ptr->decRef() == 1)
      releasePtr();
  }

  T *getPtr() const { return Ptr; } // Warning here
};

```

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


More information about the cfe-commits mailing list