Index: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -480,20 +480,21 @@ let ParentPackage = OSX in { def NumberObjectConversionChecker : Checker<"NumberObjectConversion">, - InPackage, HelpText<"Check for erroneous conversions of objects representing numbers into numbers">, DescFile<"NumberObjectConversionChecker.cpp">; def MacOSXAPIChecker : Checker<"API">, - InPackage, HelpText<"Check for proper uses of various Apple APIs">, DescFile<"MacOSXAPIChecker.cpp">; def MacOSKeychainAPIChecker : Checker<"SecKeychainAPI">, - InPackage, HelpText<"Check for proper uses of Secure Keychain APIs">, DescFile<"MacOSKeychainAPIChecker.cpp">; +def ObjCPropertyChecker : Checker<"ObjCProperty">, + HelpText<"Check for proper uses of Objective-C properties">, + DescFile<"ObjCPropertyChecker.cpp">; + } // end "osx" let ParentPackage = Cocoa in { Index: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -59,6 +59,7 @@ ObjCContainersASTChecker.cpp ObjCContainersChecker.cpp ObjCMissingSuperCallChecker.cpp + ObjCPropertyChecker.cpp ObjCSelfInitChecker.cpp ObjCSuperDeallocChecker.cpp ObjCUnusedIVarsChecker.cpp Index: cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCPropertyChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCPropertyChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCPropertyChecker.cpp @@ -0,0 +1,82 @@ +//==- ObjCPropertyChecker.cpp - Check ObjC properties ------------*- C++ -*-==// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This checker finds issues with Objective-C properties. +// Currently finds only one kind of issue: +// - Find synthesized properties with copy attribute of mutable NS collection +// types. Calling -copy on such collections produces an immutable copy, +// which contradicts the type of the property. +// +//===----------------------------------------------------------------------===// + +#include "ClangSACheckers.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" +#include "clang/StaticAnalyzer/Core/Checker.h" + +using namespace clang; +using namespace ento; + +namespace { +class ObjCPropertyChecker + : public Checker> { + void checkCopyMutable(const ObjCPropertyDecl *D, BugReporter &BR) const; + +public: + void checkASTDecl(const ObjCPropertyDecl *D, AnalysisManager &Mgr, + BugReporter &BR) const; +}; +} // end anonymous namespace. + +void ObjCPropertyChecker::checkASTDecl(const ObjCPropertyDecl *D, + AnalysisManager &Mgr, + BugReporter &BR) const { + checkCopyMutable(D, BR); +} + +void ObjCPropertyChecker::checkCopyMutable(const ObjCPropertyDecl *D, + BugReporter &BR) const { + if (D->isReadOnly() || D->getSetterKind() != ObjCPropertyDecl::Copy) + return; + + QualType T = D->getType(); + if (!T->isObjCObjectPointerType()) + return; + + const std::string &PropTypeName(T->getPointeeType().getCanonicalType() + .getUnqualifiedType() + .getAsString()); + if (!StringRef(PropTypeName).startswith("NSMutable")) + return; + + const ObjCImplDecl *ImplD = nullptr; + if (const ObjCInterfaceDecl *IntD = + dyn_cast(D->getDeclContext())) { + ImplD = IntD->getImplementation(); + } else { + const ObjCCategoryDecl *CatD = cast(D->getDeclContext()); + ImplD = CatD->getClassInterface()->getImplementation(); + } + + if (!ImplD || ImplD->HasUserDeclaredSetterMethod(D)) + return; + + SmallString<128> Str; + llvm::raw_svector_ostream OS(Str); + OS << "Property of mutable type '" << PropTypeName + << "' has 'copy' attribute; an immutable object will be stored instead"; + + BR.EmitBasicReport( + D, this, "Objective-C property misuse", "Logic error", OS.str(), + PathDiagnosticLocation::createBegin(D, BR.getSourceManager()), + D->getSourceRange()); +} + +void ento::registerObjCPropertyChecker(CheckerManager &Mgr) { + Mgr.registerChecker(); +} Index: cfe/trunk/test/Analysis/ObjCPropertiesSyntaxChecks.m =================================================================== --- cfe/trunk/test/Analysis/ObjCPropertiesSyntaxChecks.m +++ cfe/trunk/test/Analysis/ObjCPropertiesSyntaxChecks.m @@ -0,0 +1,61 @@ +// RUN: %clang_cc1 -w -fblocks -analyze -analyzer-checker=osx.ObjCProperty %s -verify + +#include "Inputs/system-header-simulator-objc.h" + +@interface I : NSObject { + NSMutableString *_mutableExplicitStr; + NSMutableString *_trulyMutableStr; + NSMutableString *_trulyMutableExplicitStr; +} +@property(copy) NSString *str; // no-warning +@property(copy) NSMutableString *mutableStr; // expected-warning{{Property of mutable type 'NSMutableString' has 'copy' attribute; an immutable object will be stored instead}} +@property(copy) NSMutableString *mutableExplicitStr; // expected-warning{{Property of mutable type 'NSMutableString' has 'copy' attribute; an immutable object will be stored instead}} +@property(copy, readonly) NSMutableString *mutableReadonlyStr; // no-warning +@property(copy, readonly) NSMutableString *mutableReadonlyStrOverriddenInChild; // no-warning +@property(copy, readonly) NSMutableString *mutableReadonlyStrOverriddenInCategory; // no-warning +@property(copy) NSMutableString *trulyMutableStr; // no-warning +@property(copy) NSMutableString *trulyMutableExplicitStr; // no-warning +@property(copy) NSMutableString *trulyMutableStrWithSynthesizedStorage; // no-warning +@end + +@interface I () {} +@property(copy) NSMutableString *mutableStrInCategory; // expected-warning{{Property of mutable type 'NSMutableString' has 'copy' attribute; an immutable object will be stored instead}} +@property (copy, readwrite) NSMutableString *mutableReadonlyStrOverriddenInCategory; // expected-warning{{Property of mutable type 'NSMutableString' has 'copy' attribute; an immutable object will be stored instead}} +@end + +@implementation I +@synthesize mutableExplicitStr = _mutableExplicitStr; +- (NSMutableString *)trulyMutableStr { + return _trulyMutableStr; +} +- (void)setTrulyMutableStr: (NSMutableString *) S { + _trulyMutableStr = [S mutableCopy]; +} +@dynamic trulyMutableExplicitStr; +- (NSMutableString *)trulyMutableExplicitStr { + return _trulyMutableExplicitStr; +} +- (void)setTrulyMutableExplicitStr: (NSMutableString *) S { + _trulyMutableExplicitStr = [S mutableCopy]; +} +@synthesize trulyMutableStrWithSynthesizedStorage; +- (NSMutableString *)trulyMutableStrWithSynthesizedStorage { + return trulyMutableStrWithSynthesizedStorage; +} +- (void)setTrulyMutableStrWithSynthesizedStorage: (NSMutableString *) S { + trulyMutableStrWithSynthesizedStorage = [S mutableCopy]; +} +@end + +@interface J : I {} +@property (copy, readwrite) NSMutableString *mutableReadonlyStrOverriddenInChild; // expected-warning{{Property of mutable type 'NSMutableString' has 'copy' attribute; an immutable object will be stored instead}} +@end + +@implementation J +@end + +// If we do not see the implementation then we do not want to warn, +// because we may miss a user-defined setter that works correctly. +@interface IWithoutImpl : NSObject {} +@property(copy) NSMutableString *mutableStr; // no-warning +@end