From 461f239331097998e614da33789d96edd7708479 Mon Sep 17 00:00:00 2001 From: Anna Zaks Date: Thu, 27 Sep 2012 19:45:15 +0000 Subject: [PATCH] [analyzer] Add an experimental ObjC direct ivar assignment checker. llvm-svn: 164790 --- clang/lib/StaticAnalyzer/Checkers/Checkers.td | 4 + .../Checkers/DirectIvarAssignment.cpp | 178 +++++++++++++++++++++ clang/test/Analysis/objc-properties.m | 56 +++++++ 3 files changed, 238 insertions(+) create mode 100644 clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp create mode 100644 clang/test/Analysis/objc-properties.m diff --git a/clang/lib/StaticAnalyzer/Checkers/Checkers.td b/clang/lib/StaticAnalyzer/Checkers/Checkers.td index f5b50fb..6f54012 100644 --- a/clang/lib/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/lib/StaticAnalyzer/Checkers/Checkers.td @@ -417,6 +417,10 @@ def IvarInvalidationChecker : Checker<"InstanceVariableInvalidation">, HelpText<"Check that the invalidatable instance variables are invalidated in the methods annotated with objc_instance_variable_invalidator">, DescFile<"IvarInvalidationChecker.cpp">; +def DirectIvarAssignment : Checker<"DirectIvarAssignment">, + HelpText<"Check that the invalidatable instance variables are invalidated in the methods annotated with objc_instance_variable_invalidator">, + DescFile<"DirectIvarAssignment.cpp">; + } // end "alpha.osx.cocoa" let ParentPackage = CoreFoundation in { diff --git a/clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp b/clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp new file mode 100644 index 0000000..db3a885 --- /dev/null +++ b/clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp @@ -0,0 +1,178 @@ +//=- DirectIvarAssignment.cpp - Check rules on ObjC properties -*- C++ ----*-==// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// Check that Objective C properties follow the following rules: +// - The property should be set with the setter, not though a direct +// assignment. +// +//===----------------------------------------------------------------------===// + +#include "ClangSACheckers.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" +#include "clang/AST/DeclObjC.h" +#include "clang/AST/StmtVisitor.h" +#include "llvm/ADT/DenseMap.h" + +using namespace clang; +using namespace ento; + +namespace { + +class DirectIvarAssignment : + public Checker > { + + typedef llvm::DenseMap IvarToPropertyMapTy; + + /// A helper class, which walks the AST and locates all assignments to ivars + /// in the given function. + class MethodCrawler : public ConstStmtVisitor { + const IvarToPropertyMapTy &IvarToPropMap; + const ObjCMethodDecl *MD; + const ObjCInterfaceDecl *InterfD; + BugReporter &BR; + LocationOrAnalysisDeclContext DCtx; + + public: + MethodCrawler(const IvarToPropertyMapTy &InMap, const ObjCMethodDecl *InMD, + const ObjCInterfaceDecl *InID, + BugReporter &InBR, AnalysisDeclContext *InDCtx) + : IvarToPropMap(InMap), MD(InMD), InterfD(InID), BR(InBR), DCtx(InDCtx) {} + + void VisitStmt(const Stmt *S) { VisitChildren(S); } + + void VisitBinaryOperator(const BinaryOperator *BO); + + void VisitChildren(const Stmt *S) { + for (Stmt::const_child_range I = S->children(); I; ++I) + if (*I) + this->Visit(*I); + } + }; + +public: + void checkASTDecl(const ObjCImplementationDecl *D, AnalysisManager& Mgr, + BugReporter &BR) const; +}; + +static const ObjCIvarDecl *findPropertyBackingIvar(const ObjCPropertyDecl *PD, + ObjCInterfaceDecl *InterD, + ASTContext &Ctx) { + // Check for synthesized ivars. + ObjCIvarDecl *ID = PD->getPropertyIvarDecl(); + if (ID) + return ID; + + // Check for existing "_PropName". + ID = InterD->lookupInstanceVariable(PD->getDefaultSynthIvarName(Ctx)); + if (ID) + return ID; + + // Check for existing "PropName". + IdentifierInfo *PropIdent = PD->getIdentifier(); + ID = InterD->lookupInstanceVariable(PropIdent); + + return ID; +} + +void DirectIvarAssignment::checkASTDecl(const ObjCImplementationDecl *D, + AnalysisManager& Mgr, + BugReporter &BR) const { + const ObjCInterfaceDecl *InterD = D->getClassInterface(); + + + IvarToPropertyMapTy IvarToPropMap; + + // Find all properties for this class. + for (ObjCInterfaceDecl::prop_iterator I = InterD->prop_begin(), + E = InterD->prop_end(); I != E; ++I) { + ObjCPropertyDecl *PD = *I; + + // Find the corresponding IVar. + const ObjCIvarDecl *ID = findPropertyBackingIvar(PD, + const_cast(InterD), + Mgr.getASTContext()); + + if (!ID) + continue; + + // Store the IVar to property mapping. + IvarToPropMap[ID] = PD; + } + + if (IvarToPropMap.empty()) + return; + + for (ObjCImplementationDecl::instmeth_iterator I = D->instmeth_begin(), + E = D->instmeth_end(); I != E; ++I) { + + ObjCMethodDecl *M = *I; + AnalysisDeclContext *DCtx = Mgr.getAnalysisDeclContext(M); + + if (M->getMethodFamily() == OMF_init || + M->getMethodFamily() == OMF_dealloc || + M->getSelector().getAsString().find("init") != StringRef::npos || + M->getSelector().getAsString().find("Init") != StringRef::npos) + continue; + + const Stmt *Body = M->getBody(); + if (!Body) + continue; + + MethodCrawler MC(IvarToPropMap, M->getCanonicalDecl(), InterD, BR, DCtx); + MC.VisitStmt(Body); + } +} + +void DirectIvarAssignment::MethodCrawler::VisitBinaryOperator( + const BinaryOperator *BO) { + if (!BO->isAssignmentOp()) + return; + + const ObjCIvarRefExpr *IvarRef = dyn_cast(BO->getLHS()); + + if (!IvarRef) + return; + + if (const ObjCIvarDecl *D = IvarRef->getDecl()) { + IvarToPropertyMapTy::const_iterator I = IvarToPropMap.find(D); + if (I != IvarToPropMap.end()) { + const ObjCPropertyDecl *PD = I->second; + + ObjCMethodDecl *GetterMethod = + InterfD->getInstanceMethod(PD->getGetterName()); + ObjCMethodDecl *SetterMethod = + InterfD->getInstanceMethod(PD->getSetterName()); + + if (SetterMethod && SetterMethod->getCanonicalDecl() == MD) + return; + + if (GetterMethod && GetterMethod->getCanonicalDecl() == MD) + return; + + + PathDiagnosticLocation IvarRefLocation = + PathDiagnosticLocation::createBegin(IvarRef, + BR.getSourceManager(), DCtx); + + BR.EmitBasicReport(MD, + "Property access", + categories::CoreFoundationObjectiveC, + "Direct assignment to an instance variable backing a property; " + "use the setter instead", IvarRefLocation); + } + } +} +} + +void ento::registerDirectIvarAssignment(CheckerManager &mgr) { + mgr.registerChecker(); +} diff --git a/clang/test/Analysis/objc-properties.m b/clang/test/Analysis/objc-properties.m new file mode 100644 index 0000000..a25791e --- /dev/null +++ b/clang/test/Analysis/objc-properties.m @@ -0,0 +1,56 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.osx.cocoa.DirectIvarAssignment -fobjc-default-synthesize-properties -Wno-objc-root-class -verify -fblocks %s + +@interface MyClass; +@end +@interface TestProperty { + MyClass *_Z; + id _nonSynth; +} + + @property (assign, nonatomic) MyClass* A; // explicitely synthesized, not implemented, non-default ivar name + + @property (assign) MyClass* X; // automatically synthesized, not implemented + + @property (assign, nonatomic) MyClass* Y; // automatically synthesized, implemented + + @property (assign, nonatomic) MyClass* Z; // non synthesized, implemented + @property (readonly) id nonSynth; // non synthesized, explicitly implemented to return ivar with expected name + + - (id) initWithPtr:(MyClass*) value; + - (id) myInitWithPtr:(MyClass*) value; + - (void) someMethod: (MyClass*)In; +@end + +@implementation TestProperty + @synthesize A = __A; + + - (id) initWithPtr: (MyClass*) value { + _Y = value; // no-warning + return self; + } + + - (id) myInitWithPtr: (MyClass*) value { + _Y = value; // no-warning + return self; + } + + - (void) setY:(MyClass*) NewValue { + _Y = NewValue; // no-warning + } + + - (void) setZ:(MyClass*) NewValue { + _Z = NewValue; // no-warning + } + + - (id)nonSynth { + return _nonSynth; + } + + - (void) someMethod: (MyClass*)In { + __A = In; // expected-warning {{Direct assignment to an instance variable backing a property; use the setter instead}} + _X = In; // expected-warning {{Direct assignment to an instance variable backing a property; use the setter instead}} + _Y = In; // expected-warning {{Direct assignment to an instance variable backing a property; use the setter instead}} + _Z = In; // expected-warning {{Direct assignment to an instance variable backing a property; use the setter instead}} + _nonSynth = 0; // expected-warning {{Direct assignment to an instance variable backing a property; use the setter instead}} + } +@end \ No newline at end of file -- 2.7.4