[PATCH] D61161: [analyzer] RetainCount: Add a suppression for functions that follow "the Matching rule".

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 196763.
NoQ added a comment.

Make tests more C++-y. They still test the C function case.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61161/new/

https://reviews.llvm.org/D61161

Files:
  clang/lib/Analysis/RetainSummaryManager.cpp
  clang/test/Analysis/osobject-retain-release.cpp


Index: clang/test/Analysis/osobject-retain-release.cpp
===================================================================
--- clang/test/Analysis/osobject-retain-release.cpp
+++ clang/test/Analysis/osobject-retain-release.cpp
@@ -679,3 +679,26 @@
   obj->release();
   obj->release();
 }
+
+class IOService {
+public:
+  OSObject *somethingMatching(OSObject *table = 0);
+};
+
+OSObject *testSuppressionForMethodsEndingWithMatching(IOService *svc,
+                                                      OSObject *table = 0) {
+  // This probably just passes table through. We should probably not make
+  // ptr1 definitely equal to table, but we should not warn about leaks.
+  OSObject *ptr1 = svc->somethingMatching(table); // no-warning
+
+  // FIXME: This, however, should follow the Create Rule regardless.
+  // We should warn about the leak here.
+  OSObject *ptr2 = svc->somethingMatching(); // no-warning
+
+  if (!table)
+    table = OSTypeAlloc(OSArray);
+
+  // This function itself ends with "Matching"! Do not warn when we're
+  // returning from it at +0.
+  return table; // no-warning
+}
Index: clang/lib/Analysis/RetainSummaryManager.cpp
===================================================================
--- clang/lib/Analysis/RetainSummaryManager.cpp
+++ clang/lib/Analysis/RetainSummaryManager.cpp
@@ -228,29 +228,35 @@
 const RetainSummary *
 RetainSummaryManager::getSummaryForOSObject(const FunctionDecl *FD,
                                             StringRef FName, QualType RetTy) {
+  assert(TrackOSObjects &&
+         "Requesting a summary for an OSObject but OSObjects are not tracked");
+
   if (RetTy->isPointerType()) {
     const CXXRecordDecl *PD = RetTy->getPointeeType()->getAsCXXRecordDecl();
     if (PD && isOSObjectSubclass(PD)) {
-      if (const IdentifierInfo *II = FD->getIdentifier()) {
-        StringRef FuncName = II->getName();
-        if (isOSObjectDynamicCast(FuncName) || isOSObjectThisCast(FuncName))
-          return getDefaultSummary();
-
-        // All objects returned with functions *not* starting with
-        // get, or iterators, are returned at +1.
-        if ((!FuncName.startswith("get") && !FuncName.startswith("Get")) ||
-            isOSIteratorSubclass(PD)) {
-          return getOSSummaryCreateRule(FD);
-        } else {
-          return getOSSummaryGetRule(FD);
-        }
+      if (isOSObjectDynamicCast(FName) || isOSObjectThisCast(FName))
+        return getDefaultSummary();
+      // TODO: Add support for the slightly common *Matching(table) idiom.
+      // Cf. IOService::nameMatching() etc. - these function have an unusual
+      // contract of returning at +0 or +1 depending on their last argument.
+      if (FName.endswith("Matching")) {
+        return getPersistentStopSummary();
+      }
+
+      // All objects returned with functions *not* starting with 'get',
+      // or iterators, are returned at +1.
+      if ((!FName.startswith("get") && !FName.startswith("Get")) ||
+          isOSIteratorSubclass(PD)) {
+        return getOSSummaryCreateRule(FD);
+      } else {
+        return getOSSummaryGetRule(FD);
       }
     }
   }
 
   if (const auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
     const CXXRecordDecl *Parent = MD->getParent();
-    if (TrackOSObjects && Parent && isOSObjectSubclass(Parent)) {
+    if (Parent && isOSObjectSubclass(Parent)) {
       if (FName == "release" || FName == "taggedRelease")
         return getOSSummaryReleaseRule(FD);
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D61161.196763.patch
Type: text/x-patch
Size: 3471 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190426/09b37c7e/attachment.bin>