[analyzer] Better retain count rules for OSObjects

[NoQ]

clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp

61 ↗

(On Diff #162309)

We have isSameOrDerivedFrom().

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp

 namespace retaincountchecker {
 
 const RefVal *getRefBinding(ProgramStateRef State, SymbolRef Sym) {
   return State->get<RefBindings>(Sym);
 }
 
 ProgramStateRef setRefBinding(ProgramStateRef State, SymbolRef Sym,
                                      RefVal Val) {
+  assert(Sym != nullptr);
   return State->set<RefBindings>(Sym, Val);
 }
 
 ProgramStateRef removeRefBinding(ProgramStateRef State, SymbolRef Sym) {
   return State->remove<RefBindings>(Sym);
 }
 
 } // end namespace retaincountchecker
if (const auto *MsgInvocation = dyn_cast<ObjCMethodCall>(&CallOrMsg)) {
     if (SymbolRef Sym = MsgInvocation->getReceiverSVal().getAsLocSymbol()) {
       if (Summ.getReceiverEffect() == StopTrackingHard) {
         state = removeRefBinding(state, Sym);
       }
     }
   }
 
   // Consult the summary for the return value.
-  SymbolRef Sym = CallOrMsg.getReturnValue().getAsSymbol();
   RetEffect RE = Summ.getRetEffect();
+
+  if (SymbolRef Sym = CallOrMsg.getReturnValue().getAsSymbol()) {
     if (const auto *MCall = dyn_cast<CXXMemberCall>(&CallOrMsg)) {
       if (Optional<RefVal> updatedRefVal =
               refValFromRetEffect(RE, MCall->getResultType())) {
         state = setRefBinding(state, Sym, *updatedRefVal);
       }
     }
 
-  if (RE.getKind() == RetEffect::NoRetHard && Sym)
+    if (RE.getKind() == RetEffect::NoRetHard)
       state = removeRefBinding(state, Sym);
+  }
 
   C.addTransition(state);
 }
 
 static ProgramStateRef updateOutParameter(ProgramStateRef State,
                                           SVal ArgVal,
                                           ArgEffect Effect) {
   auto *ArgRegion = dyn_cast_or_null<TypedValueRegion>(ArgVal.getAsRegion());

cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp

 //===----------------------------------------------------------------------===//
 
 #include "clang/StaticAnalyzer/Core/RetainSummaryManager.h"
 #include "clang/Analysis/DomainSpecific/CocoaConventions.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/ParentMap.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
 
 using namespace clang;
 using namespace ento;
 
 ArgEffects RetainSummaryManager::getArgEffects() {
   ArgEffects AE = ScratchArgs;
   ScratchArgs = AF.getEmptyMap();
   return AE;
if (OldSumm.isSimple()) {
     return &N->getValue();
   }
 
   RetainSummary *Summ = (RetainSummary *) BPAlloc.Allocate<RetainSummary>();
   new (Summ) RetainSummary(OldSumm);
   return Summ;
 }
 
-static bool isOSObjectSubclass(QualType T);
-
-static bool isOSObjectSubclass(const CXXRecordDecl *RD) {
-  if (RD->getDeclName().getAsString() == "OSObject")
-    return true;
-
-  const CXXRecordDecl *RDD = RD->getDefinition();
-  if (!RDD)
-    return false;
-
-  for (const CXXBaseSpecifier Spec : RDD->bases()) {
-    if (isOSObjectSubclass(Spec.getType()))
-      return true;
-  }
-  return false;
+static bool isSubclass(const Decl *D,
+                       StringRef ClassName) {
+  using namespace ast_matchers;
+  DeclarationMatcher SubclassM = cxxRecordDecl(isSameOrDerivedFrom(ClassName));
+  return !(match(SubclassM, *D, D->getASTContext()).empty());
 }
 
-/// \return Whether type represents an OSObject successor.
-static bool isOSObjectSubclass(QualType T) {
-  if (const auto *RD = T->getAsCXXRecordDecl()) {
-    return isOSObjectSubclass(RD);
-  }
-  return false;
+static bool isOSObjectSubclass(const Decl *D) {
+  return isSubclass(D, "OSObject");
+
+static bool isOSIteratorSubclass(const Decl *D) {
+  return isSubclass(D, "OSIterator");
 }
 
 static bool hasRCAnnotation(const Decl *D, StringRef rcAnnotation) {
   for (const auto *Ann : D->specific_attrs<AnnotateAttr>()) {
     if (Ann->getAnnotation() == rcAnnotation)
       return true;
   }
   return false;
if (FName == "pthread_create" || FName == "pthread_setspecific") {
     // Whitelist NSXXInsertXX, for example NSMapInsertIfAbsent, since they can
     // be deallocated by NSMapRemove. (radar://11152419)
     ScratchArgs = AF.add(ScratchArgs, 1, StopTracking);
     ScratchArgs = AF.add(ScratchArgs, 2, StopTracking);
     return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing);
   }
 
   if (RetTy->isPointerType()) {
-    if (TrackOSObjects && isOSObjectSubclass(RetTy->getPointeeType())) {
+
+    const CXXRecordDecl *PD = RetTy->getPointeeType()->getAsCXXRecordDecl();
+    if (TrackOSObjects && PD && isOSObjectSubclass(PD)) {
       if (const IdentifierInfo *II = FD->getIdentifier()) {
-        StringRef FuncName = II->getName();
-        if (FuncName.contains_lower("with")
-            || FuncName.contains_lower("create")
-            || FuncName.contains_lower("copy"))
+
+        // All objects returned with functions starting with "get" are getters.
+        if (II->getName().startswith("get")) {
+
+          // ...except for iterators.
+          if (isOSIteratorSubclass(PD))
             return getOSSummaryCreateRule(FD);
-      }
           return getOSSummaryGetRule(FD);
+        } else {
+          return getOSSummaryCreateRule(FD);
+        }
+      }
     }
 
     // For CoreFoundation ('CF') types.
     if (cocoa::isRefType(RetTy, "CF", FName)) {
       if (isRetain(FD, FName)) {
         // CFRetain isn't supposed to be annotated. However, this may as well
         // be a user-made "safe" CFRetain function that is incorrectly
         // annotated as cf_returns_retained due to lack of better options.
if (RetTy->isPointerType()) {
 
     if (FD->hasAttr<CFAuditedTransferAttr>()) {
       return getCFCreateGetRuleSummary(FD);
     }
   }
 
   if (const auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
     const CXXRecordDecl *Parent = MD->getParent();
-    if (TrackOSObjects && isOSObjectSubclass(Parent)) {
-      if (isRelease(FD, FName))
+    if (TrackOSObjects && Parent && isOSObjectSubclass(Parent)) {
+      if (FName == "release")
         return getOSSummaryReleaseRule(FD);
 
-      if (isRetain(FD, FName))
+      if (FName == "retain")
         return getOSSummaryRetainRule(FD);
     }
   }
 
   // Check for release functions, the only kind of functions that we care
   // about that don't return a pointer type.
   if (FName.size() >= 2 && FName[0] == 'C' &&
       (FName[1] == 'F' || FName[1] == 'G')) {