From 3a50a9fe74468ea376e59f81131a873fb1b4d7d7 Mon Sep 17 00:00:00 2001 From: George Karpenkov Date: Fri, 11 Jan 2019 18:02:08 +0000 Subject: [PATCH] [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 llvm-svn: 350942 --- clang/include/clang/Basic/Attr.td | 16 +++- clang/include/clang/Basic/AttrDocs.td | 15 ++++ clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 +- clang/lib/Sema/SemaDeclAttr.cpp | 92 +++++++++++++++++----- ...pragma-attribute-supported-attributes-list.test | 6 +- clang/test/Sema/attr-osobject.cpp | 30 ++++++- clang/test/Sema/attr-osobject.mm | 4 +- 7 files changed, 136 insertions(+), 29 deletions(-) diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 311cf94..1fe1dd3 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/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/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index aae2d1d..5773a92 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/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/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 2f77773..57c3abd 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/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; def warn_objc_requires_super_protocol : Warning< "%0 attribute cannot be applied to %select{methods in protocols|dealloc}1">, diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 241eea4..bb4bb7d 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -425,18 +425,17 @@ appendDiagnostics(const Sema::SemaDiagnosticBuilder &Bldr, T &&ExtraArg, 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, +handleSimpleAttributeOrDiagnose(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 @@ handleSimpleAttributeWithCheck(Sema &S, ValueDecl *D, SourceRange SR, handleSimpleAttribute(S, D, SR, SpellingIndex); } +template +static void +handleSimpleAttributeOrDiagnose(Sema &S, Decl *D, const ParsedAttr &AL, + bool PassesCheck, + unsigned DiagID, + DiagnosticArgs&&... ExtraArgs) { + return handleSimpleAttributeOrDiagnose( + S, D, AL.getRange(), AL.getAttributeSpellingListIndex(), PassesCheck, + DiagID, std::forward(ExtraArgs)...); +} + template 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(D); switch (K) { case RetainOwnershipKind::OS: - handleSimpleAttributeWithCheck( - *this, VD, SR, SpellingIndex, &isValidSubjectOfOSAttribute, + handleSimpleAttributeOrDiagnose( + *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, + handleSimpleAttributeOrDiagnose( + *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( + handleSimpleAttributeOrDiagnose( *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(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(); @@ -4857,10 +4892,13 @@ static void handleXReturnsXRetainedAttr(Sema &S, Decl *D, } 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()) { @@ -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(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(S, D, AL); break; + case ParsedAttr::AT_OSReturnsRetainedOnZero: + handleSimpleAttributeOrDiagnose( + 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( + 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/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test index 2f4aaa2..9a6bcca 100644 --- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test +++ b/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) diff --git a/clang/test/Sema/attr-osobject.cpp b/clang/test/Sema/attr-osobject.cpp index 8d87453..1659a82 100644 --- a/clang/test/Sema/attr-osobject.cpp +++ b/clang/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/clang/test/Sema/attr-osobject.mm b/clang/test/Sema/attr-osobject.mm index adabb6d..8b3aadf 100644 --- a/clang/test/Sema/attr-osobject.mm +++ b/clang/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; }; -- 2.7.4