r359264 - [analyzer] RetainCount: Add a suppression for "the Matching rule".

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Thu Apr 25 19:05:18 2019
New Revision: 359264

URL: http://llvm.org/viewvc/llvm-project?rev=359264&view=rev
Log:
[analyzer] RetainCount: Add a suppression for "the Matching rule".

In the OSObject universe there appears to be another slightly popular contract,
apart from "create" and "get", which is "matching". It optionally consumes
a "table" parameter and if a table is passed, it fills in the table and
returns it at +0; otherwise, it creates a new table, fills it in and
returns it at +1.

For now suppress false positives by doing a conservative escape on all functions
that end with "Matching", which is the naming convention that seems to be
followed by all such methods.

Differential Revision: https://reviews.llvm.org/D61161

Modified:
    cfe/trunk/lib/Analysis/RetainSummaryManager.cpp
    cfe/trunk/test/Analysis/osobject-retain-release.cpp

Modified: cfe/trunk/lib/Analysis/RetainSummaryManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/RetainSummaryManager.cpp?rev=359264&r1=359263&r2=359264&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/RetainSummaryManager.cpp (original)
+++ cfe/trunk/lib/Analysis/RetainSummaryManager.cpp Thu Apr 25 19:05:18 2019
@@ -228,29 +228,36 @@ RetainSummaryManager::isKnownSmartPointe
 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();
+      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 ((!FuncName.startswith("get") && !FuncName.startswith("Get")) ||
-            isOSIteratorSubclass(PD)) {
-          return getOSSummaryCreateRule(FD);
-        } else {
-          return getOSSummaryGetRule(FD);
-        }
+      // 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);
 

Modified: cfe/trunk/test/Analysis/osobject-retain-release.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/osobject-retain-release.cpp?rev=359264&r1=359263&r2=359264&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/osobject-retain-release.cpp (original)
+++ cfe/trunk/test/Analysis/osobject-retain-release.cpp Thu Apr 25 19:05:18 2019
@@ -679,3 +679,26 @@ void test_tagged_retain_no_uaf() {
   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
+}