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

[Donát Nagy via cfe-commits]

================
@@ -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) {
+            assert(!ReleaseFunctionLC &&
                    "There can be only one release point!");
-            // Suspect that it's a reference counting pointer destructor.
-            // On one of the next nodes might find out that it has atomic
-            // reference counting operations within it (see the code above),
-            // and if so, we'd conclude that it likely is a reference counting
-            // pointer destructor.
-            ReleaseDestructorLC = LC->getStackFrame();
+            // Suspect that it's a reference counting pointer
+            // destructor/function. On one of the next nodes might find out that
+            // it has atomic reference counting operations within it (see the
+            // code above), and if so, we'd conclude that it likely is a
+            // reference counting pointer destructor.
+            ReleaseFunctionLC = LC->getStackFrame();
             // It is unlikely that releasing memory is delegated to a destructor
             // inside a destructor of a shared pointer, because it's fairly hard
             // to pass the information that the pointer indeed needs to be
             // released into it. So we're only interested in the innermost
----------------
NagyDonat wrote:

This reasoning seems to be obsolete now that the code is no longer limited to destructors. You should probably rewrite this comment block (according to the the heuristics that you implement).

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