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

[NoQ]

Summary

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. Let's see if we'll ever get to actually supporting these semantics.

[dcoughlin]

Looks good to me!

[NoQ]

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

[NoQ]

Fix whitespace.

lib/Analysis/RetainSummaryManager.cpp

if (const auto *ND = dyn_cast<NamespaceDecl>(RD->getDeclContext()))
       if (ND->getNameAsString() == "os")
         return true;
   return false;
 }
 
 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))
 
-        // 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);
-        }
-      }
-    }
-  }
+      // 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")) ||
 
   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);
 
       if (FName == "retain" || FName == "taggedRetain")
         return getOSSummaryRetainRule(FD);
 
       if (FName == "free")
         return getOSSummaryFreeRule(FD);

test/Analysis/osobject-retain-release.cpp

 }
 
 void test_tagged_retain_no_uaf() {
   OSObject *obj = new OSObject;
   obj->taggedRetain();
   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
+}