[cfe-commits] r172104 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp test/Analysis/objc_invalidation.m
Anna Zaks
ganna at apple.com
Thu Jan 10 12:59:51 PST 2013
Author: zaks
Date: Thu Jan 10 14:59:51 2013
New Revision: 172104
URL: http://llvm.org/viewvc/llvm-project?rev=172104&view=rev
Log:
[analyzer] Add more checks to the ObjC Ivar Invalidation checker.
Restructured the checker so that it could easily find two new classes of
issues:
- when a class contains an invalidatable ivar, but no declaration of an
invalidation method
- when a class contains an invalidatable ivar, but no definition of an
invalidation method in the @implementation.
The second case might trigger some false positives, for example, when
the method is defined in a category.
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
cfe/trunk/test/Analysis/objc_invalidation.m
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp?rev=172104&r1=172103&r2=172104&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp Thu Jan 10 14:59:51 2013
@@ -30,6 +30,7 @@
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/SmallString.h"
using namespace clang;
@@ -37,9 +38,9 @@
namespace {
class IvarInvalidationChecker :
- public Checker<check::ASTDecl<ObjCMethodDecl> > {
+ public Checker<check::ASTDecl<ObjCImplementationDecl> > {
- typedef llvm::DenseSet<const ObjCMethodDecl*> MethodSet;
+ typedef llvm::SmallSetVector<const ObjCMethodDecl*, 2> MethodSet;
typedef llvm::DenseMap<const ObjCMethodDecl*,
const ObjCIvarDecl*> MethToIvarMapTy;
typedef llvm::DenseMap<const ObjCPropertyDecl*,
@@ -48,14 +49,14 @@
const ObjCPropertyDecl*> IvarToPropMapTy;
- struct IvarInfo {
+ struct InvalidationInfo {
/// Has the ivar been invalidated?
bool IsInvalidated;
/// The methods which can be used to invalidate the ivar.
MethodSet InvalidationMethods;
- IvarInfo() : IsInvalidated(false) {}
+ InvalidationInfo() : IsInvalidated(false) {}
void addInvalidationMethod(const ObjCMethodDecl *MD) {
InvalidationMethods.insert(MD);
}
@@ -86,7 +87,7 @@
}
};
- typedef llvm::DenseMap<const ObjCIvarDecl*, IvarInfo> IvarSet;
+ typedef llvm::DenseMap<const ObjCIvarDecl*, InvalidationInfo> IvarSet;
/// Statement visitor, which walks the method body and flags the ivars
/// referenced in it (either directly or via property).
@@ -170,7 +171,7 @@
/// Check if the any of the methods inside the interface are annotated with
/// the invalidation annotation, update the IvarInfo accordingly.
static void containsInvalidationMethod(const ObjCContainerDecl *D,
- IvarInfo &Out);
+ InvalidationInfo &Out);
/// Check if ivar should be tracked and add to TrackedIvars if positive.
/// Returns true if ivar should be tracked.
@@ -184,11 +185,13 @@
const ObjCInterfaceDecl *InterfaceD,
IvarSet &TrackedIvars);
+ /// Print ivar name or the property if the given ivar backs a property.
+ static void printIvar(llvm::raw_svector_ostream &os,
+ const ObjCIvarDecl *IvarDecl,
+ IvarToPropMapTy &IvarToPopertyMap);
public:
- void checkASTDecl(const ObjCMethodDecl *D, AnalysisManager& Mgr,
+ void checkASTDecl(const ObjCImplementationDecl *D, AnalysisManager& Mgr,
BugReporter &BR) const;
-
- // TODO: We are currently ignoring the ivars coming from class extensions.
};
static bool isInvalidationMethod(const ObjCMethodDecl *M) {
@@ -203,13 +206,14 @@
}
void IvarInvalidationChecker::containsInvalidationMethod(
- const ObjCContainerDecl *D, IvarInfo &OutInfo) {
-
- // TODO: Cache the results.
+ const ObjCContainerDecl *D, InvalidationInfo &OutInfo) {
if (!D)
return;
+ assert(!isa<ObjCImplementationDecl>(D));
+ // TODO: Cache the results.
+
// Check all methods.
for (ObjCContainerDecl::method_iterator
I = D->meth_begin(),
@@ -254,7 +258,7 @@
return false;
const ObjCInterfaceDecl *IvInterf = IvTy->getInterfaceDecl();
- IvarInfo Info;
+ InvalidationInfo Info;
containsInvalidationMethod(IvInterf, Info);
if (Info.needsInvalidation()) {
TrackedIvars[cast<ObjCIvarDecl>(Iv->getCanonicalDecl())] = Info;
@@ -307,16 +311,26 @@
return 0;
}
-void IvarInvalidationChecker::checkASTDecl(const ObjCMethodDecl *D,
+void IvarInvalidationChecker::printIvar(llvm::raw_svector_ostream &os,
+ const ObjCIvarDecl *IvarDecl,
+ IvarToPropMapTy &IvarToPopertyMap) {
+ if (IvarDecl->getSynthesize()) {
+ const ObjCPropertyDecl *PD = IvarToPopertyMap[IvarDecl];
+ assert(PD &&"Do we synthesize ivars for something other than properties?");
+ os << "Property "<< PD->getName() << " ";
+ } else {
+ os << "Instance variable "<< IvarDecl->getName() << " ";
+ }
+}
+
+// Check that the invalidatable interfaces with ivars/properties implement the
+// invalidation methods.
+void IvarInvalidationChecker::checkASTDecl(const ObjCImplementationDecl *ImplD,
AnalysisManager& Mgr,
BugReporter &BR) const {
- // We are only interested in checking the cleanup methods.
- if (!D->hasBody() || !isInvalidationMethod(D))
- return;
-
// Collect all ivars that need cleanup.
IvarSet Ivars;
- const ObjCInterfaceDecl *InterfaceD = D->getClassInterface();
+ const ObjCInterfaceDecl *InterfaceD = ImplD->getClassInterface();
// Collect ivars declared in this class, its extensions and its implementation
ObjCInterfaceDecl *IDecl = const_cast<ObjCInterfaceDecl *>(InterfaceD);
@@ -362,49 +376,96 @@
}
}
-
- // Check which ivars have been invalidated in the method body.
- bool CalledAnotherInvalidationMethod = false;
- MethodCrawler(Ivars,
- CalledAnotherInvalidationMethod,
- PropSetterToIvarMap,
- PropGetterToIvarMap,
- PropertyToIvarMap,
- BR.getContext()).VisitStmt(D->getBody());
-
- if (CalledAnotherInvalidationMethod)
+ // If no ivars need invalidation, there is nothing to check here.
+ if (Ivars.empty())
return;
- // Warn on the ivars that were not accessed by the method.
- for (IvarSet::const_iterator I = Ivars.begin(), E = Ivars.end(); I != E; ++I){
- if (!I->second.isInvalidated()) {
- const ObjCIvarDecl *IvarDecl = I->first;
-
- PathDiagnosticLocation IvarDecLocation =
- PathDiagnosticLocation::createEnd(D->getBody(), BR.getSourceManager(),
- Mgr.getAnalysisDeclContext(D));
-
- SmallString<128> sbuf;
- llvm::raw_svector_ostream os(sbuf);
-
- // Construct the warning message.
- if (IvarDecl->getSynthesize()) {
- const ObjCPropertyDecl *PD = IvarToPopertyMap[IvarDecl];
- assert(PD &&
- "Do we synthesize ivars for something other than properties?");
- os << "Property "<< PD->getName() <<
- " needs to be invalidated or set to nil";
- } else {
- os << "Instance variable "<< IvarDecl->getName()
- << " needs to be invalidated or set to nil";
- }
+ // Find all invalidation methods in this @interface declaration and parents.
+ InvalidationInfo Info;
+ containsInvalidationMethod(InterfaceD, Info);
+
+ // Report an error in case none of the invalidation methods are declared.
+ if (!Info.needsInvalidation()) {
+ SmallString<128> sbuf;
+ llvm::raw_svector_ostream os(sbuf);
+ os << "No invalidation method declared in the @interface for "
+ << InterfaceD->getName() << "; ";
+ const ObjCIvarDecl *IvarDecl = Ivars.begin()->first;
+ printIvar(os, IvarDecl, IvarToPopertyMap);
+ os << "needs to be invalidated";
+
+ PathDiagnosticLocation IvarDecLocation =
+ PathDiagnosticLocation::createBegin(IvarDecl, BR.getSourceManager());
+
+ BR.EmitBasicReport(IvarDecl, "Incomplete invalidation",
+ categories::CoreFoundationObjectiveC, os.str(),
+ IvarDecLocation);
+ return;
+ }
- BR.EmitBasicReport(D,
- "Incomplete invalidation",
- categories::CoreFoundationObjectiveC, os.str(),
- IvarDecLocation);
+ // Check that all ivars are invalidated by the invalidation methods.
+ bool AtImplementationContainsAtLeastOneInvalidationMethod = false;
+ for (MethodSet::iterator I = Info.InvalidationMethods.begin(),
+ E = Info.InvalidationMethods.end(); I != E; ++I) {
+ const ObjCMethodDecl *InterfD = *I;
+
+ // Get the corresponding method in the @implementation.
+ const ObjCMethodDecl *D = ImplD->getMethod(InterfD->getSelector(),
+ InterfD->isInstanceMethod());
+ if (D && D->hasBody()) {
+ AtImplementationContainsAtLeastOneInvalidationMethod = true;
+
+ // Get a copy of ivars needing invalidation.
+ IvarSet IvarsI = Ivars;
+
+ bool CalledAnotherInvalidationMethod = false;
+ MethodCrawler(IvarsI,
+ CalledAnotherInvalidationMethod,
+ PropSetterToIvarMap,
+ PropGetterToIvarMap,
+ PropertyToIvarMap,
+ BR.getContext()).VisitStmt(D->getBody());
+ // If another invalidation method was called, trust that full invalidation
+ // has occurred.
+ if (CalledAnotherInvalidationMethod)
+ continue;
+
+ // Warn on the ivars that were not invalidated by the method.
+ for (IvarSet::const_iterator I = IvarsI.begin(),
+ E = IvarsI.end(); I != E; ++I)
+ if (!I->second.isInvalidated()) {
+ SmallString<128> sbuf;
+ llvm::raw_svector_ostream os(sbuf);
+ printIvar(os, I->first, IvarToPopertyMap);
+ os << "needs to be invalidated or set to nil";
+ PathDiagnosticLocation MethodDecLocation =
+ PathDiagnosticLocation::createEnd(D->getBody(),
+ BR.getSourceManager(),
+ Mgr.getAnalysisDeclContext(D));
+ BR.EmitBasicReport(D, "Incomplete invalidation",
+ categories::CoreFoundationObjectiveC, os.str(),
+ MethodDecLocation);
+ }
}
}
+
+ // Report an error in case none of the invalidation methods are implemented.
+ if (!AtImplementationContainsAtLeastOneInvalidationMethod) {
+ SmallString<128> sbuf;
+ llvm::raw_svector_ostream os(sbuf);
+ os << "No invalidation method defined in the @implementation for "
+ << InterfaceD->getName() << "; ";
+ const ObjCIvarDecl *IvarDecl = Ivars.begin()->first;
+ printIvar(os, IvarDecl, IvarToPopertyMap);
+ os << "needs to be invalidated";
+
+ PathDiagnosticLocation IvarDecLocation =
+ PathDiagnosticLocation::createBegin(IvarDecl,
+ BR.getSourceManager());
+ BR.EmitBasicReport(IvarDecl, "Incomplete invalidation",
+ categories::CoreFoundationObjectiveC, os.str(),
+ IvarDecLocation);
+ }
}
void IvarInvalidationChecker::MethodCrawler::markInvalidated(
Modified: cfe/trunk/test/Analysis/objc_invalidation.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/objc_invalidation.m?rev=172104&r1=172103&r2=172104&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/objc_invalidation.m (original)
+++ cfe/trunk/test/Analysis/objc_invalidation.m Thu Jan 10 14:59:51 2013
@@ -180,3 +180,43 @@
// no-warning
}
@end
+
+ at protocol Invalidation <NSObject>
+- (void)invalidate __attribute__((annotate("objc_instance_variable_invalidator")));
+ at end
+
+ at interface Foo : NSObject <Invalidation>
+ at end
+
+ at class FooBar;
+ at protocol FooBar_Protocol <NSObject>
+ at end
+
+ at interface MissingInvalidationMethod : Foo <FooBar_Protocol>
+ at property (assign) MissingInvalidationMethod *foobar15_warn; // expected-warning {{No invalidation method defined in the @implementation for MissingInvalidationMethod; Property foobar15_warn needs to be invalidated}}
+ at end
+ at implementation MissingInvalidationMethod
+ at end
+
+ at interface MissingInvalidationMethod2 : Foo <FooBar_Protocol> {
+ Foo *Ivar1;// expected-warning {{No invalidation method defined in the @implementation for MissingInvalidationMethod2; Instance variable Ivar1 needs to be invalidated}}
+}
+ at end
+ at implementation MissingInvalidationMethod2
+ at end
+
+ at interface MissingInvalidationMethodDecl : NSObject {
+ Foo *Ivar1;// expected-warning {{No invalidation method declared in the @interface for MissingInvalidationMethodDecl; Instance variable Ivar1 needs to be invalidated}}
+}
+ at end
+ at implementation MissingInvalidationMethodDecl
+ at end
+
+ at interface MissingInvalidationMethodDecl2 : NSObject {
+ at private
+ Foo *_foo1;
+}
+ at property (strong) Foo *bar1; // expected-warning {{No invalidation method declared in the @interface for MissingInvalidationMethodDecl2; Property bar1 needs to be invalidated}}
+ at end
+ at implementation MissingInvalidationMethodDecl2
+ at end
More information about the cfe-commits
mailing list