[analyzer] [RetainCountChecker] Track input parameters to the top-level function

Track them for ISL/OS objects by default, and for NS/CF under a flag.

rdar://47536377

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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@352534 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Analysis/RetainSummaryManager.h

@@ -345,6 +345,8 @@ public:
   /// This is only meaningful if the summary applies to CXXMethodDecl*.
   ArgEffect getThisEffect() const { return This; }
 
+  ArgEffect getDefaultEffect() const { return DefaultArgEffect; }
+
   /// Set the effect of the method on "this".
   void setThisEffect(ArgEffect e) { This = e; }
 
@@ -710,6 +712,9 @@ private:
   /// is turned off.
   void updateSummaryForReceiverUnconsumedSelf(const RetainSummary *&S);
 
+  /// Set argument types for arguments which are not doing anything.
+  void updateSummaryForArgumentTypes(const AnyCall &C, const RetainSummary *&RS);
+
   /// Determine whether a declaration {@code D} of correspondent type (return
   /// type for functions/methods) {@code QT} has any of the given attributes,
   /// provided they pass necessary validation checks AND tracking the given

lib/Analysis/RetainSummaryManager.cpp

@@ -145,7 +145,7 @@ static bool isSubclass(const Decl *D,
 }
 
 static bool isOSObjectSubclass(const Decl *D) {
-  return isSubclass(D, "OSMetaClassBase");
+  return D && isSubclass(D, "OSMetaClassBase");
 }
 
 static bool isOSObjectDynamicCast(StringRef S) {
@@ -156,6 +156,15 @@ static bool isOSObjectThisCast(StringRef S) {
   return S == "metaCast";
 }
 
+
+static bool isOSObjectPtr(QualType QT) {
+  return isOSObjectSubclass(QT->getPointeeCXXRecordDecl());
+}
+
+static bool isISLObjectRef(QualType Ty) {
+  return StringRef(Ty.getAsString()).startswith("isl_");
+}
+
 static bool isOSIteratorSubclass(const Decl *D) {
   return isSubclass(D, "OSIterator");
 }
@@ -600,6 +609,38 @@ void RetainSummaryManager::updateSummaryForReceiverUnconsumedSelf(
   Template->setRetEffect(RetEffect::MakeNoRet());
 }
 
+
+void RetainSummaryManager::updateSummaryForArgumentTypes(
+  const AnyCall &C, const RetainSummary *&RS) {
+  RetainSummaryTemplate Template(RS, *this);
+
+  unsigned parm_idx = 0;
+  for (auto pi = C.param_begin(), pe = C.param_end(); pi != pe;
+       ++pi, ++parm_idx) {
+    QualType QT = (*pi)->getType();
+
+    // Skip already created values.
+    if (RS->getArgEffects().contains(parm_idx))
+      continue;
+
+    ObjKind K = ObjKind::AnyObj;
+
+    if (isISLObjectRef(QT)) {
+      K = ObjKind::Generalized;
+    } else if (isOSObjectPtr(QT)) {
+      K = ObjKind::OS;
+    } else if (cocoa::isCocoaObjectRef(QT)) {
+      K = ObjKind::ObjC;
+    } else if (coreFoundation::isCFObjectRef(QT)) {
+      K = ObjKind::CF;
+    }
+
+    if (K != ObjKind::AnyObj)
+      Template->addArg(AF, parm_idx,
+                       ArgEffect(RS->getDefaultArgEffect().getKind(), K));
+  }
+}
+
 const RetainSummary *
 RetainSummaryManager::getSummary(AnyCall C,
                                  bool HasNonZeroCallbackArg,
@@ -636,6 +677,8 @@ RetainSummaryManager::getSummary(AnyCall C,
   if (IsReceiverUnconsumedSelf)
     updateSummaryForReceiverUnconsumedSelf(Summ);
 
+  updateSummaryForArgumentTypes(C, Summ);
+
   assert(Summ && "Unknown call type?");
   return Summ;
 }

lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp

@@ -1308,10 +1308,6 @@ RetainCountChecker::processLeaks(ProgramStateRef state,
   return N;
 }
 
-static bool isISLObjectRef(QualType Ty) {
-  return StringRef(Ty.getAsString()).startswith("isl_");
-}
-
 void RetainCountChecker::checkBeginFunction(CheckerContext &Ctx) const {
   if (!Ctx.inTopFrame())
     return;
@@ -1333,13 +1329,14 @@ void RetainCountChecker::checkBeginFunction(CheckerContext &Ctx) const {
 
     QualType Ty = Param->getType();
     const ArgEffect *AE = CalleeSideArgEffects.lookup(idx);
-    if (AE && AE->getKind() == DecRef && isISLObjectRef(Ty)) {
-      state = setRefBinding(
-          state, Sym, RefVal::makeOwned(ObjKind::Generalized, Ty));
-    } else if (isISLObjectRef(Ty)) {
-      state = setRefBinding(
-          state, Sym,
-          RefVal::makeNotOwned(ObjKind::Generalized, Ty));
+    if (AE) {
+      ObjKind K = AE->getObjKind();
+      if (K == ObjKind::Generalized || K == ObjKind::OS ||
+          (TrackNSCFStartParam && (K == ObjKind::ObjC || K == ObjKind::CF))) {
+        RefVal NewVal = AE->getKind() == DecRef ? RefVal::makeOwned(K, Ty)
+                                                : RefVal::makeNotOwned(K, Ty);
+        state = setRefBinding(state, Sym, NewVal);
+      }
     }
   }
 
@@ -1463,29 +1460,37 @@ bool ento::shouldRegisterRetainCountBase(const LangOptions &LO) {
   return true;
 }
 
+// FIXME: remove this, hack for backwards compatibility:
+// it should be possible to enable the NS/CF retain count checker as
+// osx.cocoa.RetainCount, and it should be possible to disable
+// osx.OSObjectRetainCount using osx.cocoa.RetainCount:CheckOSObject=false.
+static bool getOption(AnalyzerOptions &Options,
+                      StringRef Postfix,
+                      StringRef Value) {
+  auto I = Options.Config.find(
+    (StringRef("osx.cocoa.RetainCount:") + Postfix).str());
+  if (I != Options.Config.end())
+    return I->getValue() == Value;
+  return false;
+}
+
 void ento::registerRetainCountChecker(CheckerManager &Mgr) {
   auto *Chk = Mgr.getChecker<RetainCountChecker>();
   Chk->TrackObjCAndCFObjects = true;
+  Chk->TrackNSCFStartParam = getOption(Mgr.getAnalyzerOptions(),
+                                       "TrackNSCFStartParam",
+                                       "true");
 }
 
 bool ento::shouldRegisterRetainCountChecker(const LangOptions &LO) {
   return true;
 }
 
-// FIXME: remove this, hack for backwards compatibility:
-// it should be possible to enable the NS/CF retain count checker as
-// osx.cocoa.RetainCount, and it should be possible to disable
-// osx.OSObjectRetainCount using osx.cocoa.RetainCount:CheckOSObject=false.
-static bool hasPrevCheckOSObjectOptionDisabled(AnalyzerOptions &Options) {
-  auto I = Options.Config.find("osx.cocoa.RetainCount:CheckOSObject");
-  if (I != Options.Config.end())
-    return I->getValue() == "false";
-  return false;
-}
-
 void ento::registerOSObjectRetainCountChecker(CheckerManager &Mgr) {
   auto *Chk = Mgr.getChecker<RetainCountChecker>();
-  if (!hasPrevCheckOSObjectOptionDisabled(Mgr.getAnalyzerOptions()))
+  if (!getOption(Mgr.getAnalyzerOptions(),
+                 "CheckOSObject",
+                 "false"))
     Chk->TrackOSObjects = true;
 }
 

lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h

@@ -272,6 +272,9 @@ public:
   /// Track sublcasses of OSObject.
   bool TrackOSObjects = false;
 
+  /// Track initial parameters (for the entry point) for NS/CF objects.
+  bool TrackNSCFStartParam = false;
+
   RetainCountChecker() {};
 
   RetainSummaryManager &getSummaryManager(ASTContext &Ctx) const {

lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp

@@ -418,6 +418,38 @@ annotateConsumedSummaryMismatch(const ExplodedNode *N,
   return std::make_shared<PathDiagnosticEventPiece>(L, os.str());
 }
 
+/// Annotate the parameter at the analysis entry point.
+static std::shared_ptr<PathDiagnosticEventPiece>
+annotateStartParameter(const ExplodedNode *N, SymbolRef Sym,
+                       const SourceManager &SM) {
+  auto PP = N->getLocationAs<BlockEdge>();
+  if (!PP)
+    return nullptr;
+
+  const CFGBlock *Src = PP->getSrc();
+  const RefVal *CurrT = getRefBinding(N->getState(), Sym);
+
+  if (&Src->getParent()->getEntry() != Src || !CurrT ||
+      getRefBinding(N->getFirstPred()->getState(), Sym))
+    return nullptr;
+
+  const auto *VR = cast<VarRegion>(cast<SymbolRegionValue>(Sym)->getRegion());
+  const auto *PVD = cast<ParmVarDecl>(VR->getDecl());
+  PathDiagnosticLocation L = PathDiagnosticLocation(PVD, SM);
+
+  std::string s;
+  llvm::raw_string_ostream os(s);
+  os << "Parameter '" << PVD->getNameAsString()
+    << "' starts at +";
+  if (CurrT->getCount() == 1) {
+    os << "1, as it is marked as consuming";
+  } else {
+    assert(CurrT->getCount() == 0);
+    os << "0";
+  }
+  return std::make_shared<PathDiagnosticEventPiece>(L, os.str());
+}
+
 std::shared_ptr<PathDiagnosticPiece>
 RefCountReportVisitor::VisitNode(const ExplodedNode *N,
                               BugReporterContext &BRC, BugReport &BR) {
@@ -435,6 +467,9 @@ RefCountReportVisitor::VisitNode(const ExplodedNode *N,
     if (auto PD = annotateConsumedSummaryMismatch(N, *CE, SM, CEMgr))
       return PD;
 
+  if (auto PD = annotateStartParameter(N, Sym, SM))
+    return PD;
+
   // FIXME: We will eventually need to handle non-statement-based events
   // (__attribute__((cleanup))).
   if (!N->getLocation().getAs<StmtPoint>())
@@ -673,7 +708,7 @@ static AllocationInfo GetAllocationSite(ProgramStateManager &StateMgr,
 
   if (AllocationNodeInCurrentOrParentContext &&
       AllocationNodeInCurrentOrParentContext->getLocationContext() !=
-          LeakContext)
+      LeakContext)
     FirstBinding = nullptr;
 
   return AllocationInfo(AllocationNodeInCurrentOrParentContext,
@@ -757,10 +792,19 @@ RefLeakReportVisitor::getEndPath(BugReporterContext &BRC,
         }
       } else {
         const FunctionDecl *FD = cast<FunctionDecl>(D);
-        os << "whose name ('" << *FD
-           << "') does not contain 'Copy' or 'Create'.  This violates the naming"
-              " convention rules given in the Memory Management Guide for Core"
-              " Foundation";
+        ObjKind K = RV->getObjKind();
+        if (K == ObjKind::ObjC || K == ObjKind::CF) {
+          os << "whose name ('" << *FD
+             << "') does not contain 'Copy' or 'Create'.  This violates the "
+                "naming"
+                " convention rules given in the Memory Management Guide for "
+                "Core"
+                " Foundation";
+        } else if (RV->getObjKind() == ObjKind::OS) {
+          std::string FuncName = FD->getNameAsString();
+          os << "whose name ('" << FuncName
+            << "') starts with '" << StringRef(FuncName).substr(0, 3) << "'";
+        }
       }
     }
   } else {

test/Analysis/osobject-retain-release.cpp

@@ -31,10 +31,12 @@ struct OSArray : public OSObject {
   static OSArray *withCapacity(unsigned int capacity);
   static void consumeArray(OS_CONSUME OSArray * array);
 
-  static OSArray* consumeArrayHasCode(OS_CONSUME OSArray * array) {
-    return nullptr;
+  static OSArray* consumeArrayHasCode(OS_CONSUME OSArray * array) { // expected-note{{Parameter 'array' starts at +1, as it is marked as consuming}}
+    return nullptr; // expected-warning{{Potential leak of an object of type 'OSArray'}}
+// expected-note@-1{{Object leaked: allocated object of type 'OSArray' is not referenced later in this execution path and has a retain count of +1}}
   }
 
+
   static OS_RETURNS_NOT_RETAINED OSArray *MaskedGetter();
   static OS_RETURNS_RETAINED OSArray *getOoopsActuallyCreate();
 
@@ -58,6 +60,13 @@ bool test_meta_cast_no_leak(OSMetaClassBase *arg) {
   return arg && arg->metaCast("blah") != nullptr;
 }
 
+static void consumedMismatch(OS_CONSUME OSObject *a,
+                             OSObject *b) { // expected-note{{Parameter 'b' starts at +0}}
+  a->release();
+  b->retain(); // expected-note{{Reference count incremented. The object now has a +1 retain count}}
+} // expected-warning{{Potential leak of an object of type 'OSObject'}}
+// expected-note@-1{{Object leaked: allocated object of type 'OSObject' is not referenced later in this execution path and has a retain count of +1}}
+
 void escape(void *);
 void escape_with_source(void *p) {}
 bool coin();
@@ -632,6 +641,17 @@ void test_smart_ptr_no_leak() {
   obj->release();
 }
 
+OSObject *getRuleViolation() {
+  return new OSObject; // expected-warning{{Potential leak of an object of type 'OSObject'}}
+// expected-note@-1{{Operator 'new' returns an OSObject of type 'OSObject' with a +1 retain count}}
+// expected-note@-2{{Object leaked: allocated object of type 'OSObject' is returned from a function whose name ('getRuleViolation') starts with 'get'}}
+}
+
+OSObject *createRuleViolation(OSObject *param) { // expected-note{{Parameter 'param' starts at +0}}
+  return param; // expected-warning{{Object with a +0 retain count returned to caller where a +1 (owning) retain count is expected}}
+  // expected-note@-1{{Object with a +0 retain count returned to caller where a +1 (owning) retain count is expected}}
+}
+
 void test_ostypealloc_correct_diagnostic_name() {
   OSArray *arr = OSTypeAlloc(OSArray); // expected-note{{Call to method 'OSMetaClass::alloc' returns an OSObject of type 'OSArray' with a +1 retain count}}
   arr->retain(); // expected-note{{Reference count incremented. The object now has a +2 retain count}}
@@ -659,4 +679,3 @@ void test_tagged_retain_no_uaf() {
   obj->release();
   obj->release();
 }
-

test/Analysis/retain-release.m

@@ -10,6 +10,13 @@
 // RUN:     -analyzer-checker=debug.ExprInspection -fblocks -verify %s\
 // RUN:     -Wno-objc-root-class -analyzer-output=plist -o %t.objcpp.plist\
 // RUN:     -x objective-c++ -std=gnu++98
+// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10\
+// RUN:     -analyzer-checker=core,osx.coreFoundation.CFRetainRelease\
+// RUN:     -analyzer-checker=osx.cocoa.ClassRelease,osx.cocoa.RetainCount\
+// RUN:     -analyzer-checker=debug.ExprInspection -fblocks -verify %s\
+// RUN:     -Wno-objc-root-class -x objective-c++ -std=gnu++98\
+// RUN:     -analyzer-config osx.cocoa.RetainCount:TrackNSCFStartParam=true\
+// RUN:     -DTRACK_START_PARAM
 // RUN: cat %t.objcpp.plist | %diff_plist %S/Inputs/expected-plists/retain-release.m.objcpp.plist -
 // RUN: cat %t.objc.plist | %diff_plist %S/Inputs/expected-plists/retain-release.m.objc.plist -
 
@@ -622,9 +629,15 @@ void f16(int x, CFTypeRef p) {
 
 // Test that an object is non-null after CFRetain/CFRelease/CFMakeCollectable/CFAutorelease.
 void f17(int x, CFTypeRef p) {
+#ifdef TRACK_START_PARAM
+  // expected-warning@-2{{Potential leak of an object of type 'CFTypeRef'}}
+#endif
   switch (x) {
   case 0:
     CFRelease(p);
+#ifdef TRACK_START_PARAM
+  // expected-warning@-2{{Incorrect decrement of the reference count of an object that is not owned at this point by the caller}}
+#endif
     if (!p)
       CFRelease(0); // no-warning
     break;
@@ -647,6 +660,9 @@ void f17(int x, CFTypeRef p) {
     break;
   }
 }
+#ifdef TRACK_START_PARAM
+  // expected-warning@-2{{Object autoreleased too many times}}
+#endif
 
 __attribute__((annotate("rc_ownership_returns_retained"))) isl_basic_map *isl_basic_map_cow(__attribute__((annotate("rc_ownership_consumed"))) isl_basic_map *bmap);