[analyzer] Move out tracking retain count for OSObjects into a separate checker

[george.karpenkov]

Summary

Allow enabling and disabling tracking of ObjC/CF objects
separately from tracking of OS objects.

ObjC and CF are still grouped together, as we can't rename existing checkers.

[NoQ]

clang/test/Analysis/test-separate-retaincount.cpp

22–49

I think you can merge these two and #ifdef out the warning in each section.

[NoQ]

clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp

32–33

This comment needs to go up a bit.

clang/test/Analysis/test-separate-retaincount.cpp

4–19

Why did we never make a header simulator for those?

[Szelethus]

I'm working on rebasing my dependency branches on top of trunk, and I'm somewhat stuck on this patch, could you lend a hand please?

So, I'm seeing in essence 3 checkers: the class RetainCountChecker, that does the modeling, and two checkers that are exposed to the users, osx.cocoa.RetainCount and osx.OSObjectRetainCount. While generally the solution would be to register checker called RetainCountBase, that is a common base of both of the already registered checkers, it raises a serious problem: all of the plist files that say "this error originates from checker osx.cocoa.RetainCount" will now instead refer to osx.cocoa.RetainCountBase. Unfortunately, I can't just make both checkers have the same name, as how would the user disable one and not the other?

There were other checkers that suffered from this, but those affected very few test files, and this one hits around 9, and I know that very serious work is being put into it from your part. I'd like to propose 3 solutions, and I'm very much open to new ones:

• Deal with the consequences of this, and just correct all plist files to now refer to the new base checker.
• Each time a report is emitted from these checkers, create a ProgramPointTag, and "manually" make sure the emitted checker name doesn't change.
• Rename osx.cocoa.RetainCount to something else. This one is clearly nonsensical, but let's put it here for the sake of completeness.

[george.karpenkov]

Deal with the consequences of this, and just correct all plist files to now refer to the new base checker.

What does it mean?

Each time a report is emitted from these checkers, create a ProgramPointTag, and "manually" make sure the emitted checker name doesn't change.

I'm not sure what do you propose exactly, but sounds pretty bad.

Rename osx.cocoa.RetainCount to something else. This one is clearly nonsensical, but let's put it here for the sake of completeness.

I don't think we can rename the old checker, as older Xcode versions have "osx.cocoa.RetainCount" hardcoded in them.

TBH I'm not really sold on the way we "bundle" multiple checkers (from the tablegen) into a single class.
For one, that means options don't work at all, and essentially the checker name is defined by the class which was registered by the tablegen first (which should not even be visible, and should be an internal implementation detail).
Do you have better proposals on how, conceptually, grouped checkers should be organized?

Essentially, we have a single checker class with two checks, which should be toggle-able orthogonally from each other.
For legacy reasons, we can't quite get rid of osx.cocoa.RetainCount (but even if we could, what then?)

[Szelethus]

Poor wording on my end, sorry about that. Let me clarify.

In D55400#1366684, @george.karpenkov wrote:

Deal with the consequences of this, and just correct all plist files to now refer to the new base checker.

What does it mean?

When a report is emitted in the plist output, it'll also that which checker is "responsible" for that warning. I vaguely remember reading that this isn't used that much in your application, but we use this information extensively here.

Now, what I'd propose, is register a new, osx.RetainCountBase checker, and make both of the already existing checkers depend on that. Long story short, this will make the name of the checker that is listed in that plist entry osx.RetainCountBase, as opposed to osx.cocoa.RetainCount, but this doesn't sound that illogical, considering that both of those checkers emit the same checker name anyways (meaning, that if one report originates from osx.OSObjectRetainCount and one from osx.cocoa.RetainCount, both reports will be listed as they originated from osx.cocoa.RetainCount).

Each time a report is emitted from these checkers, create a ProgramPointTag, and "manually" make sure the emitted checker name doesn't change.

I'm not sure what do you propose exactly, but sounds pretty bad.

It does. I think IvarInvalidation uses something like that, and it's both unsustainable and very ugly.

Rename osx.cocoa.RetainCount to something else. This one is clearly nonsensical, but let's put it here for the sake of completeness.

I don't think we can rename the old checker, as older Xcode versions have "osx.cocoa.RetainCount" hardcoded in them.

Yea, this one makes little sense. Should've just left this one out really.

[..]essentially the checker name is defined by the class which was registered by the tablegen first (which should not even be visible, and should be an internal implementation detail).

Do I understand correctly that you refer that the way checker names are generated is very poor?

TBH I'm not really sold on the way we "bundle" multiple checkers (from the tablegen) into a single class.
Do you have better proposals on how, conceptually, grouped checkers should be organized?

I think the current system would be fine as long as there would be a way to hide these implementation detail checkers, which could also be defined in tblgen. Having a dependency tree would be neat, and also somewhat unavoidable in order to move forward with some very important changes I'm planning, like listing all checker options.

For one, that means options don't work at all, and [...]

Hmmm, does this mess with options that bad? Could you please clarify?

[george.karpenkov]

Hmmm, does this mess with options that bad? Could you please clarify?

registerChecker gets-or-creates a checker object. A checker name (used for getting the options) is set the first time it's created.
The checker which was created first "wins" and gets to name the resulting checker.
In practice it basically means that options and checkers reusing the same class do not work.
Do you have better ideas on how this could be arranged?

I think the current situation is a mess - ideally I would prefer to be able to access the options in the constructor, but we can't even do that,
since registerChecker sets the checker name and is called after the object has been constructed.
It seems that it would only make sense if the checker name is known at the time the checker is constructed: probably the function registerXChecker should get it as an argument.

[Szelethus]

In D55400#1367046, @george.karpenkov wrote:

Hmmm, does this mess with options that bad? Could you please clarify?

registerChecker gets-or-creates a checker object. A checker name (used for getting the options) is set the first time it's created.
The checker which was created first "wins" and gets to name the resulting checker.
In practice it basically means that options and checkers reusing the same class do not work.
Do you have better ideas on how this could be arranged?

Sure, in fact its already implemented and got accepted by @NoQ, I just didnt have the time to commit and fine tune :)

The solution is essentially to create CheckerManager::getChecker alongside registerChecker, and making them in order assert if the checker is unregistered, and assert when the checked is already registered. In an earlier patch, I implement handling of dependencies on a higher level, essentially making CheckerRegistry take care of this, instead of making checker registry functions register multiple checkers, or, like in this case, register a common base. Before the registry function for either RetainCound or OSObjectRetainCount is called (which will now invoke getChecker), the registry function for RetainCountBase will be called, that will register the actual checker object.

You can take a look at how this would look like in D54438.

Would love to hear your thoughts on this! :)

I think the current situation is a mess - ideally I would prefer to be able to access the options in the constructor, but we can't even do that,
since registerChecker sets the checker name and is called after the object has been constructed.
It seems that it would only make sense if the checker name is known at the time the checker is constructed: probably the function registerXChecker should get it as an argument.

The proposed solution solves this issue as well.

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

def MacOSXAPIChecker : Checker<"API">,
   HelpText<"Check for proper uses of various Apple APIs">;
 
 def MacOSKeychainAPIChecker : Checker<"SecKeychainAPI">,
   HelpText<"Check for proper uses of Secure Keychain APIs">;
 
 def ObjCPropertyChecker : Checker<"ObjCProperty">,
   HelpText<"Check for proper uses of Objective-C properties">;
 
+def OSObjectRetainCountChecker : Checker<"OSObjectRetainCount">,
+  HelpText<"Check for leaks and improper reference count management for OSObject">;
+
 } // end "osx"
 
 let ParentPackage = Cocoa in {
 
 def RunLoopAutoreleaseLeakChecker : Checker<"RunLoopAutoreleaseLeak">,
   HelpText<"Check for leaked memory in autorelease pools that will never be "
            "drained">;
 

clang/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h

const RetainSummary *& operator[](ObjCSummaryKey K) {
     return M[K];
   }
 
   const RetainSummary *& operator[](Selector S) {
     return M[ ObjCSummaryKey(S) ];
   }
 };
 
+class RetainSummaryTemplate;
+
 class RetainSummaryManager {
   typedef llvm::DenseMap<const FunctionDecl*, const RetainSummary *>
           FuncSummariesTy;
 
   typedef ObjCSummaryCache ObjCMethodSummariesTy;
 
   typedef llvm::FoldingSetNodeWrapper<RetainSummary> CachedSummaryNode;
 
   /// Ctx - The ASTContext object for the analyzed ASTs.
   ASTContext &Ctx;
 
   /// Records whether or not the analyzed code runs in ARC mode.
   const bool ARCEnabled;
 
-  /// Track sublcasses of OSObject
+  /// Track Objective-C and CoreFoundation objects.
+  const bool TrackObjCAndCFObjects;
+
+  /// Track sublcasses of OSObject.
   const bool TrackOSObjects;
 
   /// FuncSummaries - A map from FunctionDecls to summaries.
   FuncSummariesTy FuncSummaries;
 
   /// ObjCClassMethodSummaries - A map from selectors (for instance methods)
   ///  to summaries.
   ObjCMethodSummariesTy ObjCClassMethodSummaries;
class RetainSummaryManager {
   void addClsMethSummary(IdentifierInfo *II, const RetainSummary *Summ,
                          Keywords *... Kws) {
     addMethodSummary(II, ObjCClassMethodSummaries, Summ, Kws...);
   }
 
   const RetainSummary * generateSummary(const FunctionDecl *FD,
                                         bool &AllowAnnotations);
 
+  /// Return a summary for OSObject, or nullptr if not found.
+  const RetainSummary *getSummaryForOSObject(const FunctionDecl *FD,
+                                             StringRef FName, QualType RetTy);
+
+  /// Return a summary for Objective-C or CF object, or nullptr if not found.
+  const RetainSummary *getSummaryForObjCOrCFObject(
+    const FunctionDecl *FD,
+    StringRef FName,
+    QualType RetTy,
+    const FunctionType *FT,
+    bool &AllowAnnotations);
+
+  /// Apply the annotation of {@code pd} in function {@code FD}
+  /// to the resulting summary stored in out-parameter {@code Template}.
+  /// \return whether an annotation was applied.
+  bool applyFunctionParamAnnotationEffect(const ParmVarDecl *pd,
+                                        unsigned parm_idx,
+                                        const FunctionDecl *FD,
+                                        ArgEffects::Factory &AF,
+                                        RetainSummaryTemplate &Template);
+
 public:
   RetainSummaryManager(ASTContext &ctx,
                        bool usesARC,
-                       bool trackOSObject)
+                       bool trackObjCAndCFObjects,
+                       bool trackOSObjects)
    : Ctx(ctx),
      ARCEnabled(usesARC),
-     TrackOSObjects(trackOSObject),
+     TrackObjCAndCFObjects(trackObjCAndCFObjects),
+     TrackOSObjects(trackOSObjects),
      AF(BPAlloc), ScratchArgs(AF.getEmptyMap()),
      ObjCAllocRetE(usesARC ? RetEffect::MakeNotOwned(RetEffect::ObjC)
                                : RetEffect::MakeOwned(RetEffect::ObjC)),
      ObjCInitRetE(usesARC ? RetEffect::MakeNotOwned(RetEffect::ObjC)
                                : RetEffect::MakeOwnedWhenTrackedReceiver()) {
     InitializeClassMethodSummaries();
     InitializeMethodSummaries();
   }
Optional<RetEffect> getRetEffectFromAnnotations(QualType RetTy,
                                                   const Decl *D);
 
   void updateSummaryFromAnnotations(const RetainSummary *&Summ,
                                     const ObjCMethodDecl *MD);
 
   void updateSummaryFromAnnotations(const RetainSummary *&Summ,
                                     const FunctionDecl *FD);
 
+
   void updateSummaryForCall(const RetainSummary *&Summ,
                             const CallEvent &Call);
 
   bool isARCEnabled() const { return ARCEnabled; }
 
   RetEffect getObjAllocRetEffect() const { return ObjCAllocRetE; }
 
+  /// \return True if the declaration has an attribute {@code T},
+  /// AND we are tracking that attribute. False otherwise.
+  template <class T>
+  bool hasEnabledAttr(const Decl *D) {
+    return isAttrEnabled<T>() && D->hasAttr<T>();
+  }
+
+  /// Check whether we are tracking properties specified by the attributes.
+  template <class T>
+  bool isAttrEnabled();
+
   friend class RetainSummaryTemplate;
 };
 
+
 // Used to avoid allocating long-term (BPAlloc'd) memory for default retain
 // summaries. If a function or method looks like it has a default summary, but
 // it has annotations, the annotations are added to the stack-based template
 // and then copied into managed memory.
 class RetainSummaryTemplate {
   RetainSummaryManager &Manager;
   const RetainSummary *&RealSummary;
   RetainSummary ScratchSummary;

clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h

private:
   /// The (static) type of the object at the time we started tracking it.
   QualType T;
 
   /// The current state of the object.
   ///
   /// See the RefVal::Kind enum for possible values.
   unsigned RawKind : 5;
 
-  /// The kind of object being tracked (CF or ObjC), if known.
+  /// The kind of object being tracked (CF or ObjC or OSObject), if known.
   ///
   /// See the RetEffect::ObjKind enum for possible values.
   unsigned RawObjectKind : 3;
 
   /// True if the current state and/or retain count may turn out to not be the
   /// best possible approximation of the reference counting state.
   ///
   /// If true, the checker may decide to throw away ("override") this state
class RetainCountChecker
   mutable SymbolTagMap DeadSymbolTags;
 
   mutable std::unique_ptr<RetainSummaryManager> Summaries;
   mutable SummaryLogTy SummaryLog;
 
   mutable bool ShouldResetSummaryLog;
 
 public:
-  /// Optional setting to indicate if leak reports should include
-  /// the allocation line.
-  bool IncludeAllocationLine;
-  bool ShouldCheckOSObjectRetainCount;
+
+  /// Track Objective-C and CoreFoundation objects.
+  bool TrackObjCAndCFObjects = false;
+
+  /// Track sublcasses of OSObject.
+  bool TrackOSObjects = false;
 
   RetainCountChecker() : ShouldResetSummaryLog(false) {}
 
   ~RetainCountChecker() override { DeleteContainerSeconds(DeadSymbolTags); }
 
   void checkEndAnalysis(ExplodedGraph &G, BugReporter &BR,
                         ExprEngine &Eng) const;
 
   CFRefBug *getLeakWithinFunctionBug(const LangOptions &LOpts) const;
 
   CFRefBug *getLeakAtReturnBug(const LangOptions &LOpts) const;
 
   RetainSummaryManager &getSummaryManager(ASTContext &Ctx) const {
     // FIXME: We don't support ARC being turned on and off during one analysis.
     // (nor, for that matter, do we support changing ASTContexts)
     bool ARCEnabled = (bool)Ctx.getLangOpts().ObjCAutoRefCount;
     if (!Summaries) {
       Summaries.reset(new RetainSummaryManager(
-          Ctx, ARCEnabled, ShouldCheckOSObjectRetainCount));
+          Ctx, ARCEnabled, TrackObjCAndCFObjects, TrackOSObjects));
     } else {
       assert(Summaries->isARCEnabled() == ARCEnabled);
     }
     return *Summaries;
   }
 
   RetainSummaryManager &getSummaryManager(CheckerContext &C) const {
     return getSummaryManager(C.getASTContext());

clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp

 }
 
 //===----------------------------------------------------------------------===//
 // Checker registration.
 //===----------------------------------------------------------------------===//
 
 void ento::registerRetainCountChecker(CheckerManager &Mgr) {
   auto *Chk = Mgr.registerChecker<RetainCountChecker>();
+  Chk->TrackObjCAndCFObjects = true;
+}
 
-  AnalyzerOptions &Options = Mgr.getAnalyzerOptions();
-
-  Chk->ShouldCheckOSObjectRetainCount = Options.getCheckerBooleanOption(
-                                                    "CheckOSObject", true, Chk);
+void ento::registerOSObjectRetainCountChecker(CheckerManager &Mgr) {
+  auto *Chk = Mgr.registerChecker<RetainCountChecker>();
+  Chk->TrackOSObjects = true;
 }

clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp

 #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;
 
+template <class T>
+constexpr static bool isOneOf() {
+  return false;
+}
+
+/// Helper function to check whether the class is one of the
+/// rest of varargs.
+template <class T, class P, class... ToCompare>
+constexpr static bool isOneOf() {
+  return std::is_same<T, P>::value || isOneOf<T, ToCompare...>();
+}
+
+
+template <class T> bool RetainSummaryManager::isAttrEnabled() {
+  if (isOneOf<T, CFConsumedAttr, CFReturnsRetainedAttr,
+              CFReturnsNotRetainedAttr, NSConsumedAttr, NSConsumesSelfAttr,
+              NSReturnsAutoreleasedAttr, NSReturnsRetainedAttr,
+              NSReturnsNotRetainedAttr>()) {
+    return TrackObjCAndCFObjects;
+  } else if (isOneOf<T, OSConsumedAttr, OSConsumesThisAttr,
+                     OSReturnsNotRetainedAttr, OSReturnsRetainedAttr>()) {
+    return TrackOSObjects;
+  }
+  llvm_unreachable("Unexpected attribute passed");
+}
+
 ArgEffects RetainSummaryManager::getArgEffects() {
   ArgEffects AE = ScratchArgs;
   ScratchArgs = AF.getEmptyMap();
   return AE;
 }
 
 const RetainSummary *
 RetainSummaryManager::getPersistentSummary(const RetainSummary &OldSumm) {
if (!PT.isNull())
         if (isOSObjectSubclass(RD))
           return true;
   }
 
   return false;
 }
 
 const RetainSummary *
-RetainSummaryManager::generateSummary(const FunctionDecl *FD,
-                                      bool &AllowAnnotations) {
-  // We generate "stop" summaries for implicitly defined functions.
-  if (FD->isImplicit()) {
-    return getPersistentStopSummary();
+RetainSummaryManager::getSummaryForOSObject(const FunctionDecl *FD,
+                                            StringRef FName, QualType RetTy) {
+  if (RetTy->isPointerType()) {
+    const CXXRecordDecl *PD = RetTy->getPointeeType()->getAsCXXRecordDecl();
+    if (PD && isOSObjectSubclass(PD)) {
+      if (const IdentifierInfo *II = FD->getIdentifier()) {
+        if (isOSObjectDynamicCast(II->getName()))
+          return getDefaultSummary();
+
+        // All objects returned with functions *not* starting with
+        // get, or iterators, are returned at +1.
+        if ((!II->getName().startswith("get") &&
+             !II->getName().startswith("Get")) ||
+            isOSIteratorSubclass(PD)) {
+          return getOSSummaryCreateRule(FD);
+        } else {
+          return getOSSummaryGetRule(FD);
+        }
+      }
+    }
   }
 
-  const IdentifierInfo *II = FD->getIdentifier();
+  if (const auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
+    const CXXRecordDecl *Parent = MD->getParent();
+    if (TrackOSObjects && Parent && isOSObjectSubclass(Parent)) {
+      if (FName == "release")
+        return getOSSummaryReleaseRule(FD);
 
-  StringRef FName = II ? II->getName() : "";
+      if (FName == "retain")
+        return getOSSummaryRetainRule(FD);
 
-  // Strip away preceding '_'.  Doing this here will effect all the checks
-  // down below.
-  FName = FName.substr(FName.find_first_not_of('_'));
+      if (FName == "free")
+        return getOSSummaryFreeRule(FD);
+
+      if (MD->getOverloadedOperator() == OO_New)
+        return getOSSummaryCreateRule(MD);
+    }
+  }
+
+  return nullptr;
+}
+
+const RetainSummary *RetainSummaryManager::getSummaryForObjCOrCFObject(
+    const FunctionDecl *FD,
+    StringRef FName,
+    QualType RetTy,
+    const FunctionType *FT,
+    bool &AllowAnnotations) {
 
-  // Inspect the result type. Strip away any typedefs.
-  const auto *FT = FD->getType()->getAs<FunctionType>();
-  QualType RetTy = FT->getReturnType();
   std::string RetTyName = RetTy.getAsString();
 
   // FIXME: This should all be refactored into a chain of "summary lookup"
   //  filters.
   assert(ScratchArgs.isEmpty());
-
   if (FName == "pthread_create" || FName == "pthread_setspecific") {
     // Part of: <rdar://problem/7299394> and <rdar://problem/11282706>.
     // This will be addressed better with IPA.
     return getPersistentStopSummary();
   } else if(FName == "NSMakeCollectable") {
     // Handle: id NSMakeCollectable(CFTypeRef)
     AllowAnnotations = false;
     return RetTy->isObjCIdType() ? getUnarySummary(FT, cfmakecollectable)
if (FName == "pthread_create" || FName == "pthread_setspecific") {
     // 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()) {
 
-    const CXXRecordDecl *PD = RetTy->getPointeeType()->getAsCXXRecordDecl();
-    if (TrackOSObjects && PD && isOSObjectSubclass(PD)) {
-      if (const IdentifierInfo *II = FD->getIdentifier()) {
-
-        if (isOSObjectDynamicCast(II->getName()))
-          return getDefaultSummary();
-
-        // All objects returned with functions *not* starting with
-        // get, or iterators, are returned at +1.
-        if ((!II->getName().startswith("get") &&
-             !II->getName().startswith("Get")) ||
-            isOSIteratorSubclass(PD)) {
-          return getOSSummaryCreateRule(FD);
-        } else {
-          return getOSSummaryGetRule(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
+        // 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.
         // We want to ignore such annotation.
         AllowAnnotations = false;
 
         return getUnarySummary(FT, cfretain);
       } else if (isAutorelease(FD, FName)) {
         // The headers use cf_consumed, but we can fully model CFAutorelease
         // ourselves.
if (coreFoundation::isCFObjectRef(RetTy)) {
       return getCFCreateGetRuleSummary(FD);
     }
 
     if (FD->hasAttr<CFAuditedTransferAttr>()) {
       return getCFCreateGetRuleSummary(FD);
     }
   }
 
-  if (const auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
-    const CXXRecordDecl *Parent = MD->getParent();
-    if (TrackOSObjects && Parent && isOSObjectSubclass(Parent)) {
-      if (FName == "release")
-        return getOSSummaryReleaseRule(FD);
-
-      if (FName == "retain")
-        return getOSSummaryRetainRule(FD);
-
-      if (FName == "free")
-        return getOSSummaryFreeRule(FD);
-
-      if (MD->getOverloadedOperator() == OO_New)
-        return getOSSummaryCreateRule(MD);
-    }
-  }
-
   // 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')) {
+  if (FName.startswith("CG") || FName.startswith("CF")) {
     // Test for 'CGCF'.
     FName = FName.substr(FName.startswith("CGCF") ? 4 : 2);
 
     if (isRelease(FD, FName))
       return getUnarySummary(FT, cfrelease);
     else {
       assert(ScratchArgs.isEmpty());
       // Remaining CoreFoundation and CoreGraphics functions.
else {
                      StrInStrNoCase(FName, "SetAttribute") != StringRef::npos)
                         ? MayEscape
                         : DoNothing;
 
       return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, E);
     }
   }
 
-  if (const auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
+  return nullptr;
+}
+
+const RetainSummary *
+RetainSummaryManager::generateSummary(const FunctionDecl *FD,
+                                      bool &AllowAnnotations) {
+  // We generate "stop" summaries for implicitly defined functions.
+  if (FD->isImplicit())
+    return getPersistentStopSummary();
+
+  const IdentifierInfo *II = FD->getIdentifier();
+
+  StringRef FName = II ? II->getName() : "";
+
+  // Strip away preceding '_'.  Doing this here will effect all the checks
+  // down below.
+  FName = FName.substr(FName.find_first_not_of('_'));
+
+  // Inspect the result type. Strip away any typedefs.
+  const auto *FT = FD->getType()->getAs<FunctionType>();
+  QualType RetTy = FT->getReturnType();
+
+  if (TrackOSObjects)
+    if (const RetainSummary *S = getSummaryForOSObject(FD, FName, RetTy))
+      return S;
+
+  if (TrackObjCAndCFObjects)
+    if (const RetainSummary *S =
+            getSummaryForObjCOrCFObject(FD, FName, RetTy, FT, AllowAnnotations))
+      return S;
+
+  if (const auto *MD = dyn_cast<CXXMethodDecl>(FD))
     if (!(TrackOSObjects && isOSObjectRelated(MD)))
       return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, StopTracking,
                                   DoNothing);
-  }
 
   return getDefaultSummary();
 }
 
 const RetainSummary *
 RetainSummaryManager::getFunctionSummary(const FunctionDecl *FD) {
   // If we don't know what function we're calling, use our default summary.
   if (!FD)

 
 //===----------------------------------------------------------------------===//
 // Summary creation for Selectors.
 //===----------------------------------------------------------------------===//
 
 Optional<RetEffect>
 RetainSummaryManager::getRetEffectFromAnnotations(QualType RetTy,
                                                   const Decl *D) {
-  if (cocoa::isCocoaObjectRef(RetTy)) {
+  if (TrackObjCAndCFObjects && cocoa::isCocoaObjectRef(RetTy)) {
     if (D->hasAttr<NSReturnsRetainedAttr>())
       return ObjCAllocRetE;
 
     if (D->hasAttr<NSReturnsNotRetainedAttr>() ||
         D->hasAttr<NSReturnsAutoreleasedAttr>())
       return RetEffect::MakeNotOwned(RetEffect::ObjC);
 
   } else if (!RetTy->isPointerType()) {
     return None;
   }
 
-  if (D->hasAttr<CFReturnsRetainedAttr>()) {
+  if (hasEnabledAttr<CFReturnsRetainedAttr>(D)) {
     return RetEffect::MakeOwned(RetEffect::CF);
-  } else if (D->hasAttr<OSReturnsRetainedAttr>()) {
+  } else if (hasEnabledAttr<OSReturnsRetainedAttr>(D)) {
     return RetEffect::MakeOwned(RetEffect::OS);
   } else if (hasRCAnnotation(D, "rc_ownership_returns_retained")) {
     return RetEffect::MakeOwned(RetEffect::Generalized);
   }
 
-  if (D->hasAttr<CFReturnsNotRetainedAttr>()) {
+  if (hasEnabledAttr<CFReturnsNotRetainedAttr>(D)) {
     return RetEffect::MakeNotOwned(RetEffect::CF);
-  } else if (D->hasAttr<OSReturnsNotRetainedAttr>()) {
+  } else if (hasEnabledAttr<OSReturnsNotRetainedAttr>(D)) {
     return RetEffect::MakeNotOwned(RetEffect::OS);
   } else if (hasRCAnnotation(D, "rc_ownership_returns_not_retained")) {
     return RetEffect::MakeNotOwned(RetEffect::Generalized);
   }
 
   if (const auto *MD = dyn_cast<CXXMethodDecl>(D))
     for (const auto *PD : MD->overridden_methods())
       if (auto RE = getRetEffectFromAnnotations(RetTy, PD))
         return RE;
 
   return None;
 }
 
-/// Apply the annotation of {@code pd} in function {@code FD}
-/// to the resulting summary stored in out-parameter {@code Template}.
-/// Return whether an annotation was applied.
-bool applyFunctionParamAnnotationEffect(const ParmVarDecl *pd,
+bool RetainSummaryManager::applyFunctionParamAnnotationEffect(const ParmVarDecl *pd,
                                         unsigned parm_idx,
                                         const FunctionDecl *FD,
                                         ArgEffects::Factory &AF,
                                         RetainSummaryTemplate &Template) {
-  if (pd->hasAttr<NSConsumedAttr>()) {
+  if (hasEnabledAttr<NSConsumedAttr>(pd)) {
     Template->addArg(AF, parm_idx, DecRefMsg);
     return true;
-  } else if (pd->hasAttr<CFConsumedAttr>() || pd->hasAttr<OSConsumedAttr>() ||
+  } else if (hasEnabledAttr<CFConsumedAttr>(pd) ||
+             hasEnabledAttr<OSConsumedAttr>(pd) ||
              hasRCAnnotation(pd, "rc_ownership_consumed")) {
     Template->addArg(AF, parm_idx, DecRef);
     return true;
-  } else if (pd->hasAttr<CFReturnsRetainedAttr>() ||
+  } else if (hasEnabledAttr<CFReturnsRetainedAttr>(pd) ||
              hasRCAnnotation(pd, "rc_ownership_returns_retained")) {
     QualType PointeeTy = pd->getType()->getPointeeType();
     if (!PointeeTy.isNull()) {
       if (coreFoundation::isCFObjectRef(PointeeTy)) {
         Template->addArg(AF, parm_idx, RetainedOutParameter);
         return true;
       }
     }
-  } else if (pd->hasAttr<CFReturnsNotRetainedAttr>()) {
+  } else if (hasEnabledAttr<CFReturnsNotRetainedAttr>(pd)) {
     QualType PointeeTy = pd->getType()->getPointeeType();
     if (!PointeeTy.isNull()) {
       if (coreFoundation::isCFObjectRef(PointeeTy)) {
         Template->addArg(AF, parm_idx, UnretainedOutParameter);
         return true;
       }
     }
   } else {
for (auto pi = FD->param_begin(),
     const ParmVarDecl *pd = *pi;
     applyFunctionParamAnnotationEffect(pd, parm_idx, FD, AF, Template);
   }
 
   QualType RetTy = FD->getReturnType();
   if (Optional<RetEffect> RetE = getRetEffectFromAnnotations(RetTy, FD))
     Template->setRetEffect(*RetE);
 
-  if (FD->hasAttr<OSConsumesThisAttr>())
+  if (hasEnabledAttr<OSConsumesThisAttr>(FD))
     Template->setThisEffect(DecRef);
 }
 
 void
 RetainSummaryManager::updateSummaryFromAnnotations(const RetainSummary *&Summ,
                                                    const ObjCMethodDecl *MD) {
   if (!MD)
     return;
 
   assert(Summ && "Must have a valid summary to add annotations to");
   RetainSummaryTemplate Template(Summ, *this);
 
   // Effects on the receiver.
   if (MD->hasAttr<NSConsumesSelfAttr>())
     Template->setReceiverEffect(DecRefMsg);
 
   // Effects on the parameters.
   unsigned parm_idx = 0;
-  for (ObjCMethodDecl::param_const_iterator
-         pi=MD->param_begin(), pe=MD->param_end();
+  for (auto pi=MD->param_begin(), pe=MD->param_end();
        pi != pe; ++pi, ++parm_idx) {
     const ParmVarDecl *pd = *pi;
     if (pd->hasAttr<NSConsumedAttr>()) {
       Template->addArg(AF, parm_idx, DecRefMsg);
     } else if (pd->hasAttr<CFConsumedAttr>() || pd->hasAttr<OSConsumedAttr>()) {
       Template->addArg(AF, parm_idx, DecRef);
     } else if (pd->hasAttr<CFReturnsRetainedAttr>()) {
       QualType PointeeTy = pd->getType()->getPointeeType();
return getMethodSummary(S, ReceiverClass, Method, Msg.getResultType(),
                           ObjCMethodSummaries);
 }
 
 const RetainSummary *
 RetainSummaryManager::getMethodSummary(Selector S, const ObjCInterfaceDecl *ID,
                                        const ObjCMethodDecl *MD, QualType RetTy,
                                        ObjCMethodSummariesTy &CachedSummaries) {
 
+  // Objective-C method summaries are only applicable to ObjC and CF objects.
+  if (!TrackObjCAndCFObjects)
+    return getDefaultSummary();
+
   // Look up a summary in our summary cache.
   const RetainSummary *Summ = CachedSummaries.find(ID, S);
 
   if (!Summ) {
     Summ = getStandardMethodSummary(MD, S, RetTy);
 
     // Annotations override defaults.
     updateSummaryFromAnnotations(Summ, MD);
void RetainSummaryManager::InitializeMethodSummaries() {
   addInstMethSummary("CIContext", CFAllocSumm, "createCGImage", "fromRect",
                      "format", "colorSpace");
   addInstMethSummary("CIContext", CFAllocSumm, "createCGLayerWithSize", "info");
 }
 
 CallEffects CallEffects::getEffect(const ObjCMethodDecl *MD) {
   ASTContext &Ctx = MD->getASTContext();
   LangOptions L = Ctx.getLangOpts();
-  RetainSummaryManager M(Ctx, L.ObjCAutoRefCount, /*TrackOSObjects=*/false);
+  RetainSummaryManager M(Ctx, L.ObjCAutoRefCount,
+                         /*TrackNSAndCFObjects=*/true,
+                         /*TrackOSObjects=*/false);
   const RetainSummary *S = M.getMethodSummary(MD);
   CallEffects CE(S->getRetEffect());
   CE.Receiver = S->getReceiverEffect();
   unsigned N = MD->param_size();
   for (unsigned i = 0; i < N; ++i) {
     CE.Args.push_back(S->getArg(i));
   }
   return CE;
 }
 
 CallEffects CallEffects::getEffect(const FunctionDecl *FD) {
   ASTContext &Ctx = FD->getASTContext();
   LangOptions L = Ctx.getLangOpts();
-  RetainSummaryManager M(Ctx, L.ObjCAutoRefCount, /*TrackOSObjects=*/false);
+  RetainSummaryManager M(Ctx, L.ObjCAutoRefCount,
+                         /*TrackNSAndCFObjects=*/true,
+                         /*TrackOSObjects=*/false);
   const RetainSummary *S = M.getFunctionSummary(FD);
   CallEffects CE(S->getRetEffect());
   unsigned N = FD->param_size();
   for (unsigned i = 0; i < N; ++i) {
     CE.Args.push_back(S->getArg(i));
   }
   return CE;
 }

clang/test/Analysis/osobject-retain-release.cpp

-// RUN: %clang_analyze_cc1 -analyze -analyzer-checker=core,osx.cocoa.RetainCount -analyzer-output=text -verify %s
+// RUN: %clang_analyze_cc1 -analyze -analyzer-checker=core,osx -analyzer-output=text -verify %s
 
 struct OSMetaClass;
 
 #define OS_CONSUME __attribute__((os_consumed))
 #define OS_RETURNS_RETAINED __attribute__((os_returns_retained))
 #define OS_RETURNS_NOT_RETAINED __attribute__((os_returns_not_retained))
 #define OS_CONSUMES_THIS __attribute__((os_consumes_this))
 

clang/test/Analysis/test-separate-retaincount.cpp

+// RUN: %clang_analyze_cc1 -analyzer-checker=core,osx -analyzer-disable-checker osx.cocoa.RetainCount -DNO_CF_OBJECT -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,osx -analyzer-disable-checker osx.OSObjectRetainCount -DNO_OS_OBJECT -verify %s
+
+typedef const void * CFTypeRef;
+extern CFTypeRef CFRetain(CFTypeRef cf);
+extern void CFRelease(CFTypeRef cf);
+
+#define CF_RETURNS_RETAINED __attribute__((cf_returns_retained))
+extern CFTypeRef CFCreate() CF_RETURNS_RETAINED;
+
+using size_t = decltype(sizeof(int));
+
+struct OSObject {
+  virtual void retain();
+  virtual void release();
+
+  static void * operator new(size_t size);
+  virtual ~OSObject(){}
+};
+
+void cf_overrelease() {
+  CFTypeRef cf = CFCreate();
+  CFRelease(cf);
+  CFRelease(cf);
+#ifndef NO_CF_OBJECT
+  // expected-warning@-2{{Reference-counted object is used after it is released}}
+#endif
+}
+
+void osobject_overrelease() {
+  OSObject *o = new OSObject;
+  o->release();
+  o->release();
+#ifndef NO_OS_OBJECT
+  // expected-warning@-2{{Reference-counted object is used after it is released}}
+#endif
+}