Allow the cf_returns_[not_]retained attributes to appear on out-parameters.

Includes a simple static analyzer check and not much else, but we'll also
be able to take advantage of this in Swift.

This feature can be tested for using __has_feature(cf_returns_on_parameters).

This commit also contains two fixes:
- Look through non-typedef sugar when deciding whether something is a CF type.
- When (cf|ns)_returns(_not)?_retained is applied to invalid properties,
  refer to "property" instead of "method" in the error message.

rdar://problem/18742441

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

8 files changed, 200 insertions, 13 deletions

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 18fe09a98b..803203cc50 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2751,8 +2751,8 @@ def warn_ns_attribute_wrong_return_type : Warning<
   "return %select{an Objective-C object|a pointer|a non-retainable pointer}2">,
   InGroup<IgnoredAttributes>;
 def warn_ns_attribute_wrong_parameter_type : Warning<
-  "%0 attribute only applies to %select{Objective-C object|pointer}1 "
-  "parameters">,
+  "%0 attribute only applies to "
+  "%select{Objective-C object|pointer|pointer-to-CF-pointer}1 parameters">,
   InGroup<IgnoredAttributes>;
 def warn_objc_requires_super_protocol : Warning<
   "%0 attribute cannot be applied to %select{methods in protocols|dealloc}1">,
diff --git a/include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h b/include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h
index ab92a2465c..5850656916 100644
--- a/include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h
+++ b/include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h
@@ -69,6 +69,14 @@ enum ArgEffect {
   /// transfers the object to the Garbage Collector under GC.
   MakeCollectable,
 
+  /// The argument is a pointer to a retain-counted object; on exit, the new
+  /// value of the pointer is a +0 value or NULL.
+  UnretainedOutParameter,
+
+  /// The argument is a pointer to a retain-counted object; on exit, the new
+  /// value of the pointer is a +1 value or NULL.
+  RetainedOutParameter,
+
   /// The argument is treated as potentially escaping, meaning that
   /// even when its reference count hits 0 it should be treated as still
   /// possibly being alive as someone else *may* be holding onto the object.
diff --git a/lib/Analysis/CocoaConventions.cpp b/lib/Analysis/CocoaConventions.cpp
index 0db3cac58b..be1262dc99 100644
--- a/lib/Analysis/CocoaConventions.cpp
+++ b/lib/Analysis/CocoaConventions.cpp
@@ -25,7 +25,7 @@ using namespace ento;
 bool cocoa::isRefType(QualType RetTy, StringRef Prefix,
                       StringRef Name) {
   // Recursively walk the typedef stack, allowing typedefs of reference types.
-  while (const TypedefType *TD = dyn_cast<TypedefType>(RetTy.getTypePtr())) {
+  while (const TypedefType *TD = RetTy->getAs<TypedefType>()) {
     StringRef TDName = TD->getDecl()->getIdentifier()->getName();
     if (TDName.startswith(Prefix) && TDName.endswith("Ref"))
       return true;
diff --git a/lib/Lex/PPMacroExpansion.cpp b/lib/Lex/PPMacroExpansion.cpp
index ad115bace9..5e0b283bd7 100644
--- a/lib/Lex/PPMacroExpansion.cpp
+++ b/lib/Lex/PPMacroExpansion.cpp
@@ -1059,6 +1059,7 @@ static bool HasFeature(const Preprocessor &PP, const IdentifierInfo *II) {
       .Case("attribute_availability_app_extension", true)
       .Case("attribute_cf_returns_not_retained", true)
       .Case("attribute_cf_returns_retained", true)
+      .Case("attribute_cf_returns_on_parameters", true)
       .Case("attribute_deprecated_with_message", true)
       .Case("attribute_ext_vector_type", true)
       .Case("attribute_ns_returns_not_retained", true)
diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp
index 0c2f9cf279..43790c2f37 100644
--- a/lib/Sema/SemaDeclAttr.cpp
+++ b/lib/Sema/SemaDeclAttr.cpp
@@ -3730,10 +3730,31 @@ static void handleNSReturnsRetainedAttr(Sema &S, Decl *D,
     returnType = PD->getType();
   else if (FunctionDecl *FD = dyn_cast<FunctionDecl>(D))
     returnType = FD->getReturnType();
-  else {
+  else if (auto *Param = dyn_cast<ParmVarDecl>(D)) {
+    returnType = Param->getType()->getPointeeType();
+    if (returnType.isNull()) {
+      S.Diag(D->getLocStart(), diag::warn_ns_attribute_wrong_parameter_type)
+          << Attr.getName() << /*pointer-to-CF*/2
+          << Attr.getRange();
+      return;
+    }
+  } else {
+    AttributeDeclKind ExpectedDeclKind;
+    switch (Attr.getKind()) {
+    default: llvm_unreachable("invalid ownership attribute");
+    case AttributeList::AT_NSReturnsRetained:
+    case AttributeList::AT_NSReturnsAutoreleased:
+    case AttributeList::AT_NSReturnsNotRetained:
+      ExpectedDeclKind = ExpectedFunctionOrMethod;
+      break;
+
+    case AttributeList::AT_CFReturnsRetained:
+    case AttributeList::AT_CFReturnsNotRetained:
+      ExpectedDeclKind = ExpectedFunctionMethodOrParameter;
+      break;
+    }
     S.Diag(D->getLocStart(), diag::warn_attribute_wrong_decl_type)
-        << Attr.getRange() << Attr.getName()
-        << ExpectedFunctionOrMethod;
+        << Attr.getRange() << Attr.getName() << ExpectedDeclKind;
     return;
   }
 
@@ -3760,8 +3781,25 @@ static void handleNSReturnsRetainedAttr(Sema &S, Decl *D,
   }
 
   if (!typeOK) {
-    S.Diag(D->getLocStart(), diag::warn_ns_attribute_wrong_return_type)
-      << Attr.getRange() << Attr.getName() << isa<ObjCMethodDecl>(D) << cf;
+    if (isa<ParmVarDecl>(D)) {
+      S.Diag(D->getLocStart(), diag::warn_ns_attribute_wrong_parameter_type)
+          << Attr.getName() << /*pointer-to-CF*/2
+          << Attr.getRange();
+    } else {
+      // Needs to be kept in sync with warn_ns_attribute_wrong_return_type.
+      enum : unsigned {
+        Function,
+        Method,
+        Property
+      } SubjectKind = Function;
+      if (isa<ObjCMethodDecl>(D))
+        SubjectKind = Method;
+      else if (isa<ObjCPropertyDecl>(D))
+        SubjectKind = Property;
+      S.Diag(D->getLocStart(), diag::warn_ns_attribute_wrong_return_type)
+          << Attr.getName() << SubjectKind << cf
+          << Attr.getRange();
+    }
     return;
   }
 
diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
index 58c27d49ac..49eef236e9 100644
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -906,6 +906,8 @@ static ArgEffect getStopTrackingHardEquivalent(ArgEffect E) {
   case IncRef:
   case IncRefMsg:
   case MakeCollectable:
+  case UnretainedOutParameter:
+  case RetainedOutParameter:
   case MayEscape:
   case StopTracking:
   case StopTrackingHard:
@@ -1335,7 +1337,18 @@ RetainSummaryManager::updateSummaryFromAnnotations(const RetainSummary *&Summ,
     if (pd->hasAttr<NSConsumedAttr>())
       Template->addArg(AF, parm_idx, DecRefMsg);
     else if (pd->hasAttr<CFConsumedAttr>())
-      Template->addArg(AF, parm_idx, DecRef);      
+      Template->addArg(AF, parm_idx, DecRef);
+    else if (pd->hasAttr<CFReturnsRetainedAttr>()) {
+      QualType PointeeTy = pd->getType()->getPointeeType();
+      if (!PointeeTy.isNull())
+        if (coreFoundation::isCFObjectRef(PointeeTy))
+          Template->addArg(AF, parm_idx, RetainedOutParameter);
+    } else if (pd->hasAttr<CFReturnsNotRetainedAttr>()) {
+      QualType PointeeTy = pd->getType()->getPointeeType();
+      if (!PointeeTy.isNull())
+        if (coreFoundation::isCFObjectRef(PointeeTy))
+          Template->addArg(AF, parm_idx, UnretainedOutParameter);
+    }
   }
 
   QualType RetTy = FD->getReturnType();
@@ -1366,7 +1379,17 @@ RetainSummaryManager::updateSummaryFromAnnotations(const RetainSummary *&Summ,
       Template->addArg(AF, parm_idx, DecRefMsg);      
     else if (pd->hasAttr<CFConsumedAttr>()) {
       Template->addArg(AF, parm_idx, DecRef);      
-    }   
+    } else if (pd->hasAttr<CFReturnsRetainedAttr>()) {
+      QualType PointeeTy = pd->getType()->getPointeeType();
+      if (!PointeeTy.isNull())
+        if (coreFoundation::isCFObjectRef(PointeeTy))
+          Template->addArg(AF, parm_idx, RetainedOutParameter);
+    } else if (pd->hasAttr<CFReturnsNotRetainedAttr>()) {
+      QualType PointeeTy = pd->getType()->getPointeeType();
+      if (!PointeeTy.isNull())
+        if (coreFoundation::isCFObjectRef(PointeeTy))
+          Template->addArg(AF, parm_idx, UnretainedOutParameter);
+    }
   }
 
   QualType RetTy = MD->getReturnType();
@@ -2746,7 +2769,6 @@ void RetainCountChecker::checkPostStmt(const CastExpr *CE,
   
   if (hasErr) {
     // FIXME: If we get an error during a bridge cast, should we report it?
-    // Should we assert that there is no error?
     return;
   }
 
@@ -2951,6 +2973,40 @@ void RetainCountChecker::processSummaryOfInlined(const RetainSummary &Summ,
   C.addTransition(state);
 }
 
+static ProgramStateRef updateOutParameter(ProgramStateRef State,
+                                          SVal ArgVal,
+                                          ArgEffect Effect) {
+  auto *ArgRegion = dyn_cast_or_null<TypedValueRegion>(ArgVal.getAsRegion());
+  if (!ArgRegion)
+    return State;
+
+  QualType PointeeTy = ArgRegion->getValueType();
+  if (!coreFoundation::isCFObjectRef(PointeeTy))
+    return State;
+
+  SVal PointeeVal = State->getSVal(ArgRegion);
+  SymbolRef Pointee = PointeeVal.getAsLocSymbol();
+  if (!Pointee)
+    return State;
+
+  switch (Effect) {
+  case UnretainedOutParameter:
+    State = setRefBinding(State, Pointee,
+                          RefVal::makeNotOwned(RetEffect::CF, PointeeTy));
+    break;
+  case RetainedOutParameter:
+    // Do nothing. Retained out parameters will either point to a +1 reference
+    // or NULL, but the way you check for failure differs depending on the API.
+    // Consequently, we don't have a good way to track them yet.
+    break;
+
+  default:
+    llvm_unreachable("only for out parameters");
+  }
+
+  return State;
+}
+
 void RetainCountChecker::checkSummary(const RetainSummary &Summ,
                                       const CallEvent &CallOrMsg,
                                       CheckerContext &C) const {
@@ -2964,9 +3020,12 @@ void RetainCountChecker::checkSummary(const RetainSummary &Summ,
   for (unsigned idx = 0, e = CallOrMsg.getNumArgs(); idx != e; ++idx) {
     SVal V = CallOrMsg.getArgSVal(idx);
 
-    if (SymbolRef Sym = V.getAsLocSymbol()) {
+    ArgEffect Effect = Summ.getArg(idx);
+    if (Effect == RetainedOutParameter || Effect == UnretainedOutParameter) {
+      state = updateOutParameter(state, V, Effect);
+    } else if (SymbolRef Sym = V.getAsLocSymbol()) {
       if (const RefVal *T = getRefBinding(state, Sym)) {
-        state = updateSymbol(state, Sym, *T, Summ.getArg(idx), hasErr, C);
+        state = updateSymbol(state, Sym, *T, Effect, hasErr, C);
         if (hasErr) {
           ErrorRange = CallOrMsg.getArgSourceRange(idx);
           ErrorSym = Sym;
@@ -3115,6 +3174,11 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym,
     case DecRefMsgAndStopTrackingHard:
       llvm_unreachable("DecRefMsg/IncRefMsg/MakeCollectable already converted");
 
+    case UnretainedOutParameter:
+    case RetainedOutParameter:
+      llvm_unreachable("Applies to pointer-to-pointer parameters, which should "
+                       "not have ref state.");
+
     case Dealloc:
       // Any use of -dealloc in GC is *bad*.
       if (C.isObjCGCEnabled()) {
diff --git a/test/Analysis/retain-release.m b/test/Analysis/retain-release.m
index 6973f9bcd6..e3ad409037 100644
--- a/test/Analysis/retain-release.m
+++ b/test/Analysis/retain-release.m
@@ -2153,6 +2153,33 @@ id returnNSNull() {
   return [NSNull null]; // no-warning
 }
 
+//===----------------------------------------------------------------------===//
+// cf_returns_[not_]retained on parameters
+//===----------------------------------------------------------------------===//
+
+void testCFReturnsNotRetained() {
+  extern void getViaParam(CFTypeRef * CF_RETURNS_NOT_RETAINED outObj);
+  CFTypeRef obj;
+  getViaParam(&obj);
+  CFRelease(obj); // // expected-warning {{Incorrect decrement of the reference count of an object that is not owned at this point by the caller}}
+}
+
+void testCFReturnsRetained() {
+  extern int copyViaParam(CFTypeRef * CF_RETURNS_RETAINED outObj);
+  CFTypeRef obj;
+  copyViaParam(&obj);
+  CFRelease(obj);
+  CFRelease(obj); // // FIXME-warning {{Incorrect decrement of the reference count of an object that is not owned at this point by the caller}}
+}
+
+void testCFReturnsRetainedError() {
+  extern int copyViaParam(CFTypeRef * CF_RETURNS_RETAINED outObj);
+  CFTypeRef obj;
+  if (copyViaParam(&obj) == -42)
+    return; // no-warning
+  CFRelease(obj);
+}
+
 // CHECK:  <key>diagnostics</key>
 // CHECK-NEXT:  <array>
 // CHECK-NEXT:   <dict>
diff --git a/test/SemaObjC/attr-cf_returns.m b/test/SemaObjC/attr-cf_returns.m
new file mode 100644
index 0000000000..560d40dc28
--- /dev/null
+++ b/test/SemaObjC/attr-cf_returns.m
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fobjc-arc -fblocks %s
+
+#if __has_feature(attribute_cf_returns_on_parameters)
+# error "okay!"
+// expected-error@-1 {{okay!}}
+#else
+# error "uh-oh"
+#endif
+
+#define CF_RETURNS_RETAINED __attribute__((cf_returns_retained))
+#define CF_RETURNS_NOT_RETAINED __attribute__((cf_returns_not_retained))
+
+int x CF_RETURNS_RETAINED; // expected-warning{{'cf_returns_retained' attribute only applies to functions, methods, and parameters}}
+int y CF_RETURNS_NOT_RETAINED; // expected-warning{{'cf_returns_not_retained' attribute only applies to functions, methods, and parameters}}
+
+typedef struct __CFFoo *CFFooRef;
+
+int invalid1() CF_RETURNS_RETAINED; // expected-warning{{'cf_returns_retained' attribute only applies to functions that return a pointer}}
+void invalid2() CF_RETURNS_RETAINED; // expected-warning{{'cf_returns_retained' attribute only applies to functions that return a pointer}}
+
+CFFooRef valid1() CF_RETURNS_RETAINED;
+id valid2() CF_RETURNS_RETAINED;
+
+@interface Test
+- (int)invalid1 CF_RETURNS_RETAINED; // expected-warning{{'cf_returns_retained' attribute only applies to methods that return a pointer}}
+- (void)invalid2 CF_RETURNS_RETAINED; // expected-warning{{'cf_returns_retained' attribute only applies to methods that return a pointer}}
+
+- (CFFooRef)valid1 CF_RETURNS_RETAINED;
+- (id)valid2 CF_RETURNS_RETAINED;
+
+@property int invalidProp1 CF_RETURNS_RETAINED; // expected-warning{{'cf_returns_retained' attribute only applies to properties that return a pointer}}
+@property void invalidProp2 CF_RETURNS_RETAINED; // expected-warning{{'cf_returns_retained' attribute only applies to properties that return a pointer}}
+
+@property CFFooRef valid1 CF_RETURNS_RETAINED;
+@property id valid2 CF_RETURNS_RETAINED;
+@end
+
+void invalidParam(int a CF_RETURNS_RETAINED, // expected-warning{{'cf_returns_retained' attribute only applies to pointer-to-CF-pointer parameters}}
+                  int *b CF_RETURNS_RETAINED, // expected-warning{{'cf_returns_retained' attribute only applies to pointer-to-CF-pointer parameters}}
+                  id c CF_RETURNS_RETAINED, // expected-warning{{'cf_returns_retained' attribute only applies to pointer-to-CF-pointer parameters}}
+                  void *d CF_RETURNS_RETAINED, // expected-warning{{'cf_returns_retained' attribute only applies to pointer-to-CF-pointer parameters}}
+                  CFFooRef e CF_RETURNS_RETAINED); // expected-warning{{'cf_returns_retained' attribute only applies to pointer-to-CF-pointer parameters}}
+
+void validParam(id *a CF_RETURNS_RETAINED,
+                CFFooRef *b CF_RETURNS_RETAINED,
+                void **c CF_RETURNS_RETAINED);
+void validParam2(id *a CF_RETURNS_NOT_RETAINED,
+                 CFFooRef *b CF_RETURNS_NOT_RETAINED,
+                 void **c CF_RETURNS_NOT_RETAINED);