r240185 - Allow the cf_returns_[not_]retained attributes to appear on out-parameters.
Aaron Ballman
aaron at aaronballman.com
Sat Jun 20 12:55:44 PDT 2015
On Fri, Jun 19, 2015 at 7:17 PM, Douglas Gregor <dgregor at apple.com> wrote:
> Author: dgregor
> Date: Fri Jun 19 18:17:46 2015
> New Revision: 240185
>
> URL: http://llvm.org/viewvc/llvm-project?rev=240185&view=rev
> Log:
> 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
>
> Added:
> cfe/trunk/test/SemaObjC/attr-cf_returns.m
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h
> cfe/trunk/lib/Analysis/CocoaConventions.cpp
> cfe/trunk/lib/Lex/PPMacroExpansion.cpp
> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
> cfe/trunk/test/Analysis/retain-release.m
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=240185&r1=240184&r2=240185&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Jun 19 18:17:46 2015
> @@ -2751,8 +2751,8 @@ def warn_ns_attribute_wrong_return_type
> "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">,
CF-pointer doesn't appear to be a common term in the code base (this
is the only diagnostic with it, and we have one comment on it), or on
the web. Is there a better term we could use? If not, it's fine, I was
mostly curious whether this is a common term of art that users would
situationally understand.
> InGroup<IgnoredAttributes>;
> def warn_objc_requires_super_protocol : Warning<
> "%0 attribute cannot be applied to %select{methods in protocols|dealloc}1">,
>
> Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h?rev=240185&r1=240184&r2=240185&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h (original)
> +++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h Fri Jun 19 18:17:46 2015
> @@ -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.
>
> Modified: cfe/trunk/lib/Analysis/CocoaConventions.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CocoaConventions.cpp?rev=240185&r1=240184&r2=240185&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Analysis/CocoaConventions.cpp (original)
> +++ cfe/trunk/lib/Analysis/CocoaConventions.cpp Fri Jun 19 18:17:46 2015
> @@ -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;
>
> Modified: cfe/trunk/lib/Lex/PPMacroExpansion.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPMacroExpansion.cpp?rev=240185&r1=240184&r2=240185&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Lex/PPMacroExpansion.cpp (original)
> +++ cfe/trunk/lib/Lex/PPMacroExpansion.cpp Fri Jun 19 18:17:46 2015
> @@ -1059,6 +1059,7 @@ static bool HasFeature(const Preprocesso
> .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)
>
> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=240185&r1=240184&r2=240185&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Fri Jun 19 18:17:46 2015
> @@ -3730,10 +3730,31 @@ static void handleNSReturnsRetainedAttr(
> 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(
> }
>
> 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;
> }
>
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp?rev=240185&r1=240184&r2=240185&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp Fri Jun 19 18:17:46 2015
> @@ -906,6 +906,8 @@ static ArgEffect getStopTrackingHardEqui
> case IncRef:
> case IncRefMsg:
> case MakeCollectable:
> + case UnretainedOutParameter:
> + case RetainedOutParameter:
> case MayEscape:
> case StopTracking:
> case StopTrackingHard:
> @@ -1335,7 +1337,18 @@ RetainSummaryManager::updateSummaryFromA
> 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::updateSummaryFromA
> 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(c
>
> 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::processSummaryO
> 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(co
> 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(Program
> 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()) {
>
> Modified: cfe/trunk/test/Analysis/retain-release.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/retain-release.m?rev=240185&r1=240184&r2=240185&view=diff
> ==============================================================================
> --- cfe/trunk/test/Analysis/retain-release.m (original)
> +++ cfe/trunk/test/Analysis/retain-release.m Fri Jun 19 18:17:46 2015
> @@ -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>
>
> Added: cfe/trunk/test/SemaObjC/attr-cf_returns.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/attr-cf_returns.m?rev=240185&view=auto
> ==============================================================================
> --- cfe/trunk/test/SemaObjC/attr-cf_returns.m (added)
> +++ cfe/trunk/test/SemaObjC/attr-cf_returns.m Fri Jun 19 18:17:46 2015
> @@ -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 at -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;
> +
> + at 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;
> +
> + at property int invalidProp1 CF_RETURNS_RETAINED; // expected-warning{{'cf_returns_retained' attribute only applies to properties that return a pointer}}
> + at property void invalidProp2 CF_RETURNS_RETAINED; // expected-warning{{'cf_returns_retained' attribute only applies to properties that return a pointer}}
> +
> + at property CFFooRef valid1 CF_RETURNS_RETAINED;
> + at property id valid2 CF_RETURNS_RETAINED;
> + at 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);
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
~Aaron
More information about the cfe-commits
mailing list