Index: clang/include/clang/Basic/Attr.td =================================================================== --- clang/include/clang/Basic/Attr.td +++ clang/include/clang/Basic/Attr.td @@ -841,13 +841,25 @@ 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]; } Index: clang/include/clang/Basic/AttrDocs.td =================================================================== --- clang/include/clang/Basic/AttrDocs.td +++ clang/include/clang/Basic/AttrDocs.td @@ -888,6 +888,21 @@ ``__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, Index: clang/include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -3425,7 +3425,7 @@ "%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; def warn_objc_requires_super_protocol : Warning< "%0 attribute cannot be applied to %select{methods in protocols|dealloc}1">, Index: clang/lib/Sema/SemaDeclAttr.cpp =================================================================== --- clang/lib/Sema/SemaDeclAttr.cpp +++ clang/lib/Sema/SemaDeclAttr.cpp @@ -425,18 +425,17 @@ std::forward(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 static void -handleSimpleAttributeWithCheck(Sema &S, ValueDecl *D, SourceRange SR, +handleSimpleAttributeWithCheck(Sema &S, Decl *D, SourceRange SR, unsigned SpellingIndex, - llvm::function_ref 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(ExtraArgs)...); return; @@ -444,6 +443,17 @@ handleSimpleAttribute(S, D, SR, SpellingIndex); } +template +static void +handleSimpleAttributeWithCheck(Sema &S, Decl *D, const ParsedAttr &AL, + bool PassesCheck, + unsigned DiagID, + DiagnosticArgs&&... ExtraArgs) { + return handleSimpleAttributeWithCheck( + S, D, AL.getRange(), AL.getAttributeSpellingListIndex(), PassesCheck, + DiagID, std::forward(ExtraArgs)...); +} + template static void handleSimpleAttributeWithExclusions(Sema &S, Decl *D, const ParsedAttr &AL) { @@ -4773,7 +4783,10 @@ } 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, @@ -4783,13 +4796,13 @@ switch (K) { case RetainOwnershipKind::OS: handleSimpleAttributeWithCheck( - *this, VD, SR, SpellingIndex, &isValidSubjectOfOSAttribute, + *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( - *this, VD, SR, SpellingIndex, &isValidSubjectOfNSAttribute, + *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 @@ -4803,7 +4816,7 @@ case RetainOwnershipKind::CF: handleSimpleAttributeWithCheck( *this, VD, SR, SpellingIndex, - &isValidSubjectOfCFAttribute, + isValidSubjectOfCFAttribute(VD->getType()), diag::warn_ns_attribute_wrong_parameter_type, /*ExtraArgs=*/SR, "cf_consumed", /*pointers*/1); return; @@ -4814,10 +4827,21 @@ 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"); @@ -4833,9 +4857,21 @@ return true; } +/// Return that the parameter is a pointer to pointer, +/// and the parent function has a non-void return type. +static bool isValidOSObjectOutParameter(const Decl *D) { + const ParmVarDecl *PVD = dyn_cast(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(D)) { ReturnType = MD->getReturnType(); @@ -4849,10 +4885,13 @@ } else if (const auto *Param = dyn_cast(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()) { @@ -4864,11 +4903,11 @@ 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; @@ -4881,6 +4920,7 @@ bool TypeOK; bool Cf; + unsigned ParmDiagID = 2; // Pointer-to-CF-pointer switch (AL.getKind()) { default: llvm_unreachable("invalid ownership attribute"); case ParsedAttr::AT_NSReturnsRetained: @@ -4904,6 +4944,7 @@ case ParsedAttr::AT_OSReturnsNotRetained: TypeOK = isValidSubjectOfOSAttribute(ReturnType); Cf = true; + ParmDiagID = 3; // Pointer-to-OSObject-pointer break; } @@ -4913,7 +4954,7 @@ if (isa(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 { @@ -6505,6 +6546,18 @@ case ParsedAttr::AT_OSConsumesThis: handleSimpleAttribute(S, D, AL); break; + case ParsedAttr::AT_OSReturnsRetainedOnZero: + handleSimpleAttributeWithCheck( + 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: + handleSimpleAttributeWithCheck( + 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: Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test =================================================================== --- clang/test/Misc/pragma-attribute-supported-attributes-list.test +++ clang/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) Index: clang/test/Sema/attr-osobject.cpp =================================================================== --- clang/test/Sema/attr-osobject.cpp +++ clang/test/Sema/attr-osobject.cpp @@ -37,12 +37,38 @@ 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}} Index: clang/test/Sema/attr-osobject.mm =================================================================== --- clang/test/Sema/attr-osobject.mm +++ clang/test/Sema/attr-osobject.mm @@ -8,8 +8,8 @@ - (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; };