[attributes] Extend os_returns_(not_?)_retained attributes to parameters

When applied to out-parameters, the attributes specify the expected lifetime of the written-into object.

Additionally, introduce OSReturnsRetainedOn(Non)Zero attributes, which
specify that an ownership transfer happens depending on a return code.

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

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

diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td
index 311cf943dfabc5f9ec92201e0b832cbba28eb0a0..1fe1dd39948a1b68b47bc99fd56bb5ded34864e4 100644 (file)
--- a/include/clang/Basic/Attr.td
+++ b/include/clang/Basic/Attr.td
@@ -841,13 +841,25 @@ def OSConsumed : InheritableParamAttr {
 
 def OSReturnsRetained : InheritableAttr {
   let Spellings = [Clang<"os_returns_retained">];
-  let Subjects = SubjectList<[Function, ObjCMethod, ObjCProperty]>;
+  let Subjects = SubjectList<[Function, ObjCMethod, ObjCProperty, ParmVar]>;
   let Documentation = [RetainBehaviorDocs];
 }
 
 def OSReturnsNotRetained : InheritableAttr {
   let Spellings = [Clang<"os_returns_not_retained">];
-  let Subjects = SubjectList<[Function, ObjCMethod, ObjCProperty]>;
+  let Subjects = SubjectList<[Function, ObjCMethod, ObjCProperty, ParmVar]>;
+  let Documentation = [RetainBehaviorDocs];
+}
+
+def OSReturnsRetainedOnZero : InheritableAttr {
+  let Spellings = [Clang<"os_returns_retained_on_zero">];
+  let Subjects = SubjectList<[ParmVar]>;
+  let Documentation = [RetainBehaviorDocs];
+}
+
+def OSReturnsRetainedOnNonZero : InheritableAttr {
+  let Spellings = [Clang<"os_returns_retained_on_non_zero">];
+  let Subjects = SubjectList<[ParmVar]>;
   let Documentation = [RetainBehaviorDocs];
 }
 

diff --git a/include/clang/Basic/AttrDocs.td b/include/clang/Basic/AttrDocs.td
index aae2d1ddfc9e3061d3e8df365ee3bfe4614837c8..5773a92c9c15dd8084cdd76495eca751e3b57b87 100644 (file)
--- a/include/clang/Basic/AttrDocs.td
+++ b/include/clang/Basic/AttrDocs.td
@@ -888,6 +888,21 @@ Similar to ``__attribute__((ns_consumes_self))``,
 ``__attribute__((os_consumes_this))`` specifies that the method call consumes
 the reference to "this" (e.g., when attaching it to a different object supplied
 as a parameter).
+Out parameters (parameters the function is meant to write into,
+either via pointers-to-pointers or references-to-pointers)
+may be annotated with ``__attribute__((os_returns_retained))``
+or ``__attribute__((os_returns_not_retained))`` which specifies that the object
+written into the out parameter should (or respectively should not) be released
+after use.
+Since often out parameters may or may not be written depending on the exit
+code of the function,
+annotations ``__attribute__((os_returns_retained_on_zero))``
+and ``__attribute__((os_returns_retained_on_non_zero))`` specify that
+an out parameter at ``+1`` is written if and only if the function returns a zero
+(respectively non-zero) error code.
+Observe that return-code-dependent out parameter annotations are only
+available for retained out parameters, as non-retained object do not have to be
+released by the callee.
 These attributes are only used by the Clang Static Analyzer.
 
 The family of attributes ``X_returns_X_retained`` can be added to functions,

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 2f77773ac8026fca0173b575aeea99b770a25738..57c3abd048997c71851f6e41ce9c7b31cce8ea5a 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3425,7 +3425,7 @@ def err_ns_attribute_wrong_parameter_type : Error<
   "%select{Objective-C object|pointer|pointer-to-CF-pointer}1 parameters">;
 def warn_ns_attribute_wrong_parameter_type : Warning<
   "%0 attribute only applies to "
-  "%select{Objective-C object|pointer|pointer-to-CF-pointer}1 parameters">,
+  "%select{Objective-C object|pointer|pointer-to-CF-pointer|pointer/reference-to-OSObject-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/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp
index 241eea4927ce748e00e16c7508ad0e2712bb7528..bb4bb7da2fb187dc172f0c2d188956ced223563d 100644 (file)
--- a/lib/Sema/SemaDeclAttr.cpp
+++ b/lib/Sema/SemaDeclAttr.cpp
@@ -425,18 +425,17 @@ appendDiagnostics(const Sema::SemaDiagnosticBuilder &Bldr, T &&ExtraArg,
                            std::forward<DiagnosticArgs>(ExtraArgs)...);
 }
 
-/// Add an attribute {@code AttrType} to declaration {@code D},
-/// provided the given {@code Check} function returns {@code true}
-/// on type of {@code D}.
-/// If check does not pass, emit diagnostic {@code DiagID},
-/// passing in all parameters specified in {@code ExtraArgs}.
+/// Add an attribute {@code AttrType} to declaration {@code D}, provided that
+/// {@code PassesCheck} is true.
+/// Otherwise, emit diagnostic {@code DiagID}, passing in all parameters
+/// specified in {@code ExtraArgs}.
 template <typename AttrType, typename... DiagnosticArgs>
 static void
-handleSimpleAttributeWithCheck(Sema &S, ValueDecl *D, SourceRange SR,
+handleSimpleAttributeOrDiagnose(Sema &S, Decl *D, SourceRange SR,
                                unsigned SpellingIndex,
-                               llvm::function_ref<bool(QualType)> Check,
-                               unsigned DiagID, DiagnosticArgs... ExtraArgs) {
-  if (!Check(D->getType())) {
+                               bool PassesCheck,
+                               unsigned DiagID, DiagnosticArgs&&... ExtraArgs) {
+  if (!PassesCheck) {
     Sema::SemaDiagnosticBuilder DB = S.Diag(D->getBeginLoc(), DiagID);
     appendDiagnostics(DB, std::forward<DiagnosticArgs>(ExtraArgs)...);
     return;
@@ -444,6 +443,17 @@ handleSimpleAttributeWithCheck(Sema &S, ValueDecl *D, SourceRange SR,
   handleSimpleAttribute<AttrType>(S, D, SR, SpellingIndex);
 }
 
+template <typename AttrType, typename... DiagnosticArgs>
+static void
+handleSimpleAttributeOrDiagnose(Sema &S, Decl *D, const ParsedAttr &AL,
+                               bool PassesCheck,
+                               unsigned DiagID,
+                               DiagnosticArgs&&... ExtraArgs) {
+  return handleSimpleAttributeOrDiagnose<AttrType>(
+      S, D, AL.getRange(), AL.getAttributeSpellingListIndex(), PassesCheck,
+      DiagID, std::forward<DiagnosticArgs>(ExtraArgs)...);
+}
+
 template <typename AttrType>
 static void handleSimpleAttributeWithExclusions(Sema &S, Decl *D,
                                                 const ParsedAttr &AL) {
@@ -4781,7 +4791,10 @@ static bool isValidSubjectOfCFAttribute(QualType QT) {
 }
 
 static bool isValidSubjectOfOSAttribute(QualType QT) {
-  return QT->isDependentType() || QT->isPointerType();
+  if (QT->isDependentType())
+    return true;
+  QualType PT = QT->getPointeeType();
+  return !PT.isNull() && PT->getAsCXXRecordDecl() != nullptr;
 }
 
 void Sema::AddXConsumedAttr(Decl *D, SourceRange SR, unsigned SpellingIndex,
@@ -4790,14 +4803,14 @@ void Sema::AddXConsumedAttr(Decl *D, SourceRange SR, unsigned SpellingIndex,
   ValueDecl *VD = cast<ValueDecl>(D);
   switch (K) {
   case RetainOwnershipKind::OS:
-    handleSimpleAttributeWithCheck<OSConsumedAttr>(
-        *this, VD, SR, SpellingIndex, &isValidSubjectOfOSAttribute,
+    handleSimpleAttributeOrDiagnose<OSConsumedAttr>(
+        *this, VD, SR, SpellingIndex, isValidSubjectOfOSAttribute(VD->getType()),
         diag::warn_ns_attribute_wrong_parameter_type,
         /*ExtraArgs=*/SR, "os_consumed", /*pointers*/ 1);
     return;
   case RetainOwnershipKind::NS:
-    handleSimpleAttributeWithCheck<NSConsumedAttr>(
-        *this, VD, SR, SpellingIndex, &isValidSubjectOfNSAttribute,
+    handleSimpleAttributeOrDiagnose<NSConsumedAttr>(
+        *this, VD, SR, SpellingIndex, isValidSubjectOfNSAttribute(VD->getType()),
 
         // These attributes are normally just advisory, but in ARC, ns_consumed
         // is significant.  Allow non-dependent code to contain inappropriate
@@ -4809,9 +4822,9 @@ void Sema::AddXConsumedAttr(Decl *D, SourceRange SR, unsigned SpellingIndex,
         /*ExtraArgs=*/SR, "ns_consumed", /*objc pointers*/ 0);
     return;
   case RetainOwnershipKind::CF:
-    handleSimpleAttributeWithCheck<CFConsumedAttr>(
+    handleSimpleAttributeOrDiagnose<CFConsumedAttr>(
         *this, VD, SR, SpellingIndex,
-        &isValidSubjectOfCFAttribute,
+        isValidSubjectOfCFAttribute(VD->getType()),
         diag::warn_ns_attribute_wrong_parameter_type,
         /*ExtraArgs=*/SR, "cf_consumed", /*pointers*/1);
     return;
@@ -4822,10 +4835,21 @@ static Sema::RetainOwnershipKind
 parsedAttrToRetainOwnershipKind(const ParsedAttr &AL) {
   switch (AL.getKind()) {
   case ParsedAttr::AT_CFConsumed:
+  case ParsedAttr::AT_CFReturnsRetained:
+  case ParsedAttr::AT_CFReturnsNotRetained:
     return Sema::RetainOwnershipKind::CF;
+  case ParsedAttr::AT_OSConsumesThis:
   case ParsedAttr::AT_OSConsumed:
+  case ParsedAttr::AT_OSReturnsRetained:
+  case ParsedAttr::AT_OSReturnsNotRetained:
+  case ParsedAttr::AT_OSReturnsRetainedOnZero:
+  case ParsedAttr::AT_OSReturnsRetainedOnNonZero:
     return Sema::RetainOwnershipKind::OS;
+  case ParsedAttr::AT_NSConsumesSelf:
   case ParsedAttr::AT_NSConsumed:
+  case ParsedAttr::AT_NSReturnsRetained:
+  case ParsedAttr::AT_NSReturnsNotRetained:
+  case ParsedAttr::AT_NSReturnsAutoreleased:
     return Sema::RetainOwnershipKind::NS;
   default:
     llvm_unreachable("Wrong argument supplied");
@@ -4841,9 +4865,20 @@ bool Sema::checkNSReturnsRetainedReturnType(SourceLocation Loc, QualType QT) {
   return true;
 }
 
+/// \return whether the parameter is a pointer to OSObject pointer.
+static bool isValidOSObjectOutParameter(const Decl *D) {
+  const auto *PVD = dyn_cast<ParmVarDecl>(D);
+  if (!PVD)
+    return false;
+  QualType QT = PVD->getType();
+  QualType PT = QT->getPointeeType();
+  return !PT.isNull() && isValidSubjectOfOSAttribute(PT);
+}
+
 static void handleXReturnsXRetainedAttr(Sema &S, Decl *D,
                                         const ParsedAttr &AL) {
   QualType ReturnType;
+  Sema::RetainOwnershipKind K = parsedAttrToRetainOwnershipKind(AL);
 
   if (const auto *MD = dyn_cast<ObjCMethodDecl>(D)) {
     ReturnType = MD->getReturnType();
@@ -4857,10 +4892,13 @@ static void handleXReturnsXRetainedAttr(Sema &S, Decl *D,
   } else if (const auto *Param = dyn_cast<ParmVarDecl>(D)) {
     // Attributes on parameters are used for out-parameters,
     // passed as pointers-to-pointers.
+    unsigned DiagID = K == Sema::RetainOwnershipKind::CF
+            ? /*pointer-to-CF-pointer*/2
+            : /*pointer-to-OSObject-pointer*/3;
     ReturnType = Param->getType()->getPointeeType();
     if (ReturnType.isNull()) {
       S.Diag(D->getBeginLoc(), diag::warn_ns_attribute_wrong_parameter_type)
-          << AL << /*pointer-to-CF-pointer*/ 2 << AL.getRange();
+          << AL << DiagID << AL.getRange();
       return;
     }
   } else if (AL.isUsedAsTypeAttr()) {
@@ -4872,11 +4910,11 @@ static void handleXReturnsXRetainedAttr(Sema &S, Decl *D,
     case ParsedAttr::AT_NSReturnsRetained:
     case ParsedAttr::AT_NSReturnsAutoreleased:
     case ParsedAttr::AT_NSReturnsNotRetained:
-    case ParsedAttr::AT_OSReturnsRetained:
-    case ParsedAttr::AT_OSReturnsNotRetained:
       ExpectedDeclKind = ExpectedFunctionOrMethod;
       break;
 
+    case ParsedAttr::AT_OSReturnsRetained:
+    case ParsedAttr::AT_OSReturnsNotRetained:
     case ParsedAttr::AT_CFReturnsRetained:
     case ParsedAttr::AT_CFReturnsNotRetained:
       ExpectedDeclKind = ExpectedFunctionMethodOrParameter;
@@ -4889,6 +4927,7 @@ static void handleXReturnsXRetainedAttr(Sema &S, Decl *D,
 
   bool TypeOK;
   bool Cf;
+  unsigned ParmDiagID = 2; // Pointer-to-CF-pointer
   switch (AL.getKind()) {
   default: llvm_unreachable("invalid ownership attribute");
   case ParsedAttr::AT_NSReturnsRetained:
@@ -4912,6 +4951,7 @@ static void handleXReturnsXRetainedAttr(Sema &S, Decl *D,
   case ParsedAttr::AT_OSReturnsNotRetained:
     TypeOK = isValidSubjectOfOSAttribute(ReturnType);
     Cf = true;
+    ParmDiagID = 3; // Pointer-to-OSObject-pointer
     break;
   }
 
@@ -4921,7 +4961,7 @@ static void handleXReturnsXRetainedAttr(Sema &S, Decl *D,
 
     if (isa<ParmVarDecl>(D)) {
       S.Diag(D->getBeginLoc(), diag::warn_ns_attribute_wrong_parameter_type)
-          << AL << /*pointer-to-CF*/ 2 << AL.getRange();
+          << AL << ParmDiagID << AL.getRange();
     } else {
       // Needs to be kept in sync with warn_ns_attribute_wrong_return_type.
       enum : unsigned {
@@ -6513,6 +6553,18 @@ static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D,
   case ParsedAttr::AT_OSConsumesThis:
     handleSimpleAttribute<OSConsumesThisAttr>(S, D, AL);
     break;
+  case ParsedAttr::AT_OSReturnsRetainedOnZero:
+    handleSimpleAttributeOrDiagnose<OSReturnsRetainedOnZeroAttr>(
+        S, D, AL, isValidOSObjectOutParameter(D),
+        diag::warn_ns_attribute_wrong_parameter_type,
+        /*Extra Args=*/AL, /*pointer-to-OSObject-pointer*/ 3, AL.getRange());
+    break;
+  case ParsedAttr::AT_OSReturnsRetainedOnNonZero:
+    handleSimpleAttributeOrDiagnose<OSReturnsRetainedOnNonZeroAttr>(
+        S, D, AL, isValidOSObjectOutParameter(D),
+        diag::warn_ns_attribute_wrong_parameter_type,
+        /*Extra Args=*/AL, /*pointer-to-OSObject-poointer*/ 3, AL.getRange());
+    break;
   case ParsedAttr::AT_NSReturnsAutoreleased:
   case ParsedAttr::AT_NSReturnsNotRetained:
   case ParsedAttr::AT_NSReturnsRetained:

diff --git a/test/Misc/pragma-attribute-supported-attributes-list.test b/test/Misc/pragma-attribute-supported-attributes-list.test
index 2f4aaa200c55af51cbd42057974b5aeca8441029..9a6bcca1bd36bf6903c27d805ef5cc287ffd65eb 100644 (file)
--- a/test/Misc/pragma-attribute-supported-attributes-list.test
+++ b/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -86,8 +86,10 @@
 // CHECK-NEXT: NoThrow (SubjectMatchRule_function)
 // CHECK-NEXT: NotTailCalled (SubjectMatchRule_function)
 // CHECK-NEXT: OSConsumed (SubjectMatchRule_variable_is_parameter)
-// CHECK-NEXT: OSReturnsNotRetained (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
-// CHECK-NEXT: OSReturnsRetained (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
+// CHECK-NEXT: OSReturnsNotRetained (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_variable_is_parameter)
+// CHECK-NEXT: OSReturnsRetained (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_variable_is_parameter)
+// CHECK-NEXT: OSReturnsRetainedOnNonZero (SubjectMatchRule_variable_is_parameter)
+// CHECK-NEXT: OSReturnsRetainedOnZero (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: ObjCBoxable (SubjectMatchRule_record)
 // CHECK-NEXT: ObjCBridge (SubjectMatchRule_record, SubjectMatchRule_type_alias)
 // CHECK-NEXT: ObjCBridgeMutable (SubjectMatchRule_record)

diff --git a/test/Sema/attr-osobject.cpp b/test/Sema/attr-osobject.cpp
index 8d87453d00a2a7395e621948b76f2ddf4336e24e..1659a82533be792d8b982c49267accd4443016d7 100644 (file)
--- a/test/Sema/attr-osobject.cpp
+++ b/test/Sema/attr-osobject.cpp
@@ -37,12 +37,38 @@ __attribute__((os_returns_retained(10))) S* returns_retained_no_extra_arg() { //
   return nullptr;
 }
 
-struct __attribute__((os_returns_retained)) NoRetainAttrOnStruct {}; // expected-warning{{'os_returns_retained' attribute only applies to functions, Objective-C methods, and Objective-C properties}}
+struct __attribute__((os_returns_retained)) NoRetainAttrOnStruct {}; // expected-warning{{'os_returns_retained' attribute only applies to functions, Objective-C methods, Objective-C properties, and parameters}}
 
 __attribute__((os_returns_not_retained(10))) S* os_returns_no_retained_no_extra_args( S *arg) { // expected-error{{'os_returns_not_retained' attribute takes no arguments}}
   return nullptr;
 }
 
-struct __attribute__((os_returns_not_retained)) NoNotRetainedAttrOnStruct {}; // expected-warning{{'os_returns_not_retained' attribute only applies to functions, Objective-C methods, and Objective-C properties}}
+struct __attribute__((os_returns_not_retained)) NoNotRetainedAttrOnStruct {}; // expected-warning{{'os_returns_not_retained' attribute only applies to functions, Objective-C methods, Objective-C properties, and parameters}}
 
 __attribute__((os_consumes_this)) void no_consumes_this_on_function() {} // expected-warning{{'os_consumes_this' attribute only applies to non-static member functions}}
+
+void write_into_out_parameter(__attribute__((os_returns_retained)) S** out) {}
+
+bool write_into_out_parameter_on_nonzero(__attribute__((os_returns_retained_on_non_zero)) S** out) {}
+
+bool write_into_out_parameter_on_nonzero_invalid(__attribute__((os_returns_retained_on_non_zero)) S* out) {} // expected-warning{{'os_returns_retained_on_non_zero' attribute only applies to pointer/reference-to-OSObject-pointer parameters}}
+
+bool write_into_out_parameter_on_zero(__attribute__((os_returns_retained_on_zero)) S** out) {}
+
+bool write_into_out_parameter_on_zero_invalid(__attribute__((os_returns_retained_on_zero)) S* out) {} // expected-warning{{'os_returns_retained_on_zero' attribute only applies to pointer/reference-to-OSObject-pointer parameters}}
+
+void write_into_out_parameter_ref(__attribute__((os_returns_retained)) S*& out) {}
+
+typedef S* SPtr;
+
+void write_into_out_parameter_typedef(__attribute__((os_returns_retained)) SPtr* out) {}
+
+void write_into_out_parameter_invalid(__attribute__((os_returns_retained)) S* out) {} // expected-warning{{'os_returns_retained' attribute only applies to pointer/reference-to-OSObject-pointer parameters}}
+
+void write_into_out_parameter_not_retained(__attribute__((os_returns_not_retained)) S **out) {}
+
+void write_into_out_parameter_not_retained(__attribute__((os_returns_not_retained)) S volatile * const * const out) {}
+
+void write_into_not_retainedout_parameter_invalid(__attribute__((os_returns_not_retained)) S* out) {} // expected-warning{{'os_returns_not_retained' attribute only applies to pointer/reference-to-OSObject-pointer parameters}}
+
+void write_into_not_retainedout_parameter_too_many_pointers(__attribute__((os_returns_not_retained)) S*** out) {} // expected-warning{{'os_returns_not_retained' attribute only applies to pointer/reference-to-OSObject-pointer parameters}}

diff --git a/test/Sema/attr-osobject.mm b/test/Sema/attr-osobject.mm
index adabb6df0fcca5cf09ea9af5a5515b69b70644b0..8b3aadff801c9122399e45f38bcd5dc5a289dec4 100644 (file)
--- a/test/Sema/attr-osobject.mm
+++ b/test/Sema/attr-osobject.mm
@@ -8,8 +8,8 @@ struct S {};
   - (void) takeS:(S*) __attribute__((os_consumed)) s;
 @end
 
-typedef __attribute__((os_returns_retained)) id (^blockType)(); // expected-warning{{'os_returns_retained' attribute only applies to functions, Objective-C methods, and Objective-C properties}}
+typedef __attribute__((os_returns_retained)) id (^blockType)(); // expected-warning{{'os_returns_retained' attribute only applies to functions, Objective-C methods, Objective-C properties, and parameters}}
 
-__auto_type b = ^ id (id filter)  __attribute__((os_returns_retained))  { // expected-warning{{'os_returns_retained' attribute only applies to functions, Objective-C methods, and Objective-C properties}}
+__auto_type b = ^ id (id filter)  __attribute__((os_returns_retained))  { // expected-warning{{'os_returns_retained' attribute only applies to functions, Objective-C methods, Objective-C properties, and parameters}}
   return filter;
 };