[analyzer] Detect ObjC properties that are both (copy) and Mutable.

When an Objective-C property has a (copy) attribute, the default setter
for this property performs a -copy on the object assigned.

Calling -copy on a mutable NS object such as NSMutableString etc.
produces an immutable object, NSString in our example.
Hence the getter becomes type-incorrect.

rdar://problem/21022397

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

llvm-svn: 289554

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

@@ -480,20 +480,21 @@ def CStringNotNullTerm : Checker<"NotNullTerminated">,
 let ParentPackage = OSX in {
 
 def NumberObjectConversionChecker : Checker<"NumberObjectConversion">,
-  InPackage<OSX>,
   HelpText<"Check for erroneous conversions of objects representing numbers into numbers">,
   DescFile<"NumberObjectConversionChecker.cpp">;
 
 def MacOSXAPIChecker : Checker<"API">,
-  InPackage<OSX>,
   HelpText<"Check for proper uses of various Apple APIs">,
   DescFile<"MacOSXAPIChecker.cpp">;
 
 def MacOSKeychainAPIChecker : Checker<"SecKeychainAPI">,
-  InPackage<OSX>,
   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 {

clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt

@@ -59,6 +59,7 @@ add_clang_library(clangStaticAnalyzerCheckers
   ObjCContainersASTChecker.cpp
   ObjCContainersChecker.cpp
   ObjCMissingSuperCallChecker.cpp
+  ObjCPropertyChecker.cpp
   ObjCSelfInitChecker.cpp
   ObjCSuperDeallocChecker.cpp
   ObjCUnusedIVarsChecker.cpp

clang/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<check::ASTDecl<ObjCPropertyDecl>> {
+  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<ObjCInterfaceDecl>(D->getDeclContext())) {
+    ImplD = IntD->getImplementation();
+  } else {
+    const ObjCCategoryDecl *CatD = cast<ObjCCategoryDecl>(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<ObjCPropertyChecker>();
+}

clang/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