Index: lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp +++ lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp @@ -18,6 +18,8 @@ #include "clang/AST/DeclObjC.h" #include "clang/AST/Expr.h" #include "clang/AST/ExprObjC.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Basic/LangOptions.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" #include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h" @@ -26,8 +28,85 @@ #include "llvm/Support/raw_ostream.h" using namespace clang; +using namespace clang::ast_matchers; using namespace ento; +class SuperDeallocUseAfterFreeCallback : public MatchFinder::MatchCallback { +public: + SuperDeallocUseAfterFreeCallback(IdentifierInfo *SelfII) + : FoundSuperDealloc(false), m_FoundMatch(false), m_SelfII(SelfII), + m_Stmt(nullptr) {} + + virtual void run(MatchFinder::MatchResult const &Result) { + if (const DeclRefExpr *E = + Result.Nodes.getNodeAs("self")) { + if (E->getDecl()->getIdentifier() == m_SelfII) { + m_FoundMatch = true; + m_Stmt = Result.Nodes.getNodeAs("stmt"); + } + } + } + + bool FoundMatch() { return m_FoundMatch; } + const clang::Stmt *Stmt() { return m_Stmt; } + + bool FoundSuperDealloc; + +private: + bool m_FoundMatch; + IdentifierInfo *m_SelfII; + const clang::Stmt *m_Stmt; +}; + +static bool scan_dealloc_for_self_after_super_dealloc( + Stmt *S, SuperDeallocUseAfterFreeCallback &Callback, ASTContext &Ctx) { + + // Find references to 'self'. + MatchFinder Finder; + StatementMatcher Matcher = + stmt(hasDescendant(declRefExpr(to(varDecl(hasName("self")))) + .bind("self"))).bind("stmt"); + Finder.addMatcher(Matcher, &Callback); + Finder.match(*S, Ctx); + return Callback.FoundMatch(); +} + +static bool +scan_dealloc_for_use_after_free(Stmt *S, Selector Dealloc, + SuperDeallocUseAfterFreeCallback &Callback, + ASTContext &Ctx) { + + // Find [super dealloc] call. + if (ObjCMessageExpr *ME = dyn_cast(S)) + if (ME->getSelector() == Dealloc) { + switch (ME->getReceiverKind()) { + case ObjCMessageExpr::Instance: + break; + case ObjCMessageExpr::SuperInstance: { + Callback.FoundSuperDealloc = true; + break; + } + case ObjCMessageExpr::Class: + break; + case ObjCMessageExpr::SuperClass: + break; + } + } + + // Recurse to children. + for (Stmt::child_iterator I = S->child_begin(), E = S->child_end(); I != E; + ++I) + if (Callback.FoundSuperDealloc) { + if (*I && scan_dealloc_for_self_after_super_dealloc(*I, Callback, Ctx)) + return true; + } else if (*I && + scan_dealloc_for_use_after_free(*I, Dealloc, Callback, Ctx)) { + return true; + } + + return false; +} + static bool scan_ivar_release(Stmt *S, const ObjCIvarDecl *ID, const ObjCPropertyDecl *PD, Selector Release, @@ -103,8 +182,9 @@ static void checkObjCDealloc(const CheckerBase *Checker, const ObjCImplementationDecl *D, - const LangOptions &LOpts, BugReporter &BR) { + AnalysisManager &Mgr, BugReporter &BR) { + const LangOptions &LOpts = Mgr.getLangOpts(); assert(LOpts.getGC() != LangOptions::GCOnly); assert(!LOpts.ObjCAutoRefCount); @@ -128,9 +208,6 @@ } } - if (!containsRetainedSynthesizedWritablePointerProperty) - return; - // Determine if the class subclasses NSObject. IdentifierInfo* NSObjectII = &Ctx.Idents.get("NSObject"); IdentifierInfo* SenTestCaseII = &Ctx.Idents.get("SenTestCase"); @@ -166,30 +243,56 @@ } if (!MD) { // No dealloc found. + if (containsRetainedSynthesizedWritablePointerProperty) { + const char *name = LOpts.getGC() == LangOptions::NonGC + ? "missing -dealloc" + : "missing -dealloc (Hybrid MM, non-GC)"; - const char* name = LOpts.getGC() == LangOptions::NonGC - ? "missing -dealloc" - : "missing -dealloc (Hybrid MM, non-GC)"; + std::string buf; + llvm::raw_string_ostream os(buf); + os << "Objective-C class '" << *D + << "' lacks a 'dealloc' instance method"; + + PathDiagnosticLocation DLoc = + PathDiagnosticLocation::createBegin(D, BR.getSourceManager()); + + BR.EmitBasicReport(D, Checker, name, categories::CoreFoundationObjectiveC, + os.str(), DLoc); + } + return; + } + + // Get the "self" identifier + IdentifierInfo *SelfII = &Ctx.Idents.get("self"); + + // Scan for use of 'self' after [super dealloc]. + SuperDeallocUseAfterFreeCallback Callback(SelfII); + if (scan_dealloc_for_use_after_free(MD->getBody(), S, Callback, Ctx)) { + const char *name = + "use-after-free referencing 'self' after [super dealloc]"; std::string buf; llvm::raw_string_ostream os(buf); - os << "Objective-C class '" << *D << "' lacks a 'dealloc' instance method"; + os << "The 'dealloc' instance method in Objective-C class '" << *D + << "' references 'self' afer a call to [super dealloc], causing" + " a use-after-free memory reference."; + + PathDiagnosticLocation StmtLoc = PathDiagnosticLocation::createBegin( + Callback.Stmt(), BR.getSourceManager(), Mgr.getAnalysisDeclContext(MD)); - PathDiagnosticLocation DLoc = - PathDiagnosticLocation::createBegin(D, BR.getSourceManager()); + BR.EmitBasicReport(MD, Checker, name, categories::CoreFoundationObjectiveC, + os.str(), StmtLoc); - BR.EmitBasicReport(D, Checker, name, categories::CoreFoundationObjectiveC, - os.str(), DLoc); return; } + if (!containsRetainedSynthesizedWritablePointerProperty) + return; + // Get the "release" selector. IdentifierInfo* RII = &Ctx.Idents.get("release"); Selector RS = Ctx.Selectors.getSelector(0, &RII); - // Get the "self" identifier - IdentifierInfo* SelfII = &Ctx.Idents.get("self"); - // Scan for missing and extra releases of ivars used by implementations // of synthesized properties for (const auto *I : D->property_impls()) { @@ -246,13 +349,12 @@ class ObjCDeallocChecker : public Checker< check::ASTDecl > { public: - void checkASTDecl(const ObjCImplementationDecl *D, AnalysisManager& mgr, + void checkASTDecl(const ObjCImplementationDecl *D, AnalysisManager &Mgr, BugReporter &BR) const { - if (mgr.getLangOpts().getGC() == LangOptions::GCOnly || - mgr.getLangOpts().ObjCAutoRefCount) + if (Mgr.getLangOpts().getGC() == LangOptions::GCOnly || + Mgr.getLangOpts().ObjCAutoRefCount) return; - checkObjCDealloc(this, cast(D), mgr.getLangOpts(), - BR); + checkObjCDealloc(this, cast(D), Mgr, BR); } }; } Index: test/Analysis/DeallocUseAfterFreeErrors.m =================================================================== --- /dev/null +++ test/Analysis/DeallocUseAfterFreeErrors.m @@ -0,0 +1,141 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.osx.cocoa.Dealloc %s -verify + +#define nil ((id)0) + +typedef signed char BOOL; +@protocol NSObject +- (instancetype)retain; +- (oneway void)release; +@end + +@interface NSObject { } +- (void)dealloc; +- (instancetype)init; +@end + +typedef struct objc_selector *SEL; + +//===------------------------------------------------------------------------=== +// +// Check that 'self' is not referenced after calling '[super dealloc]'. + +@interface SuperDeallocThenReleaseIvarClass : NSObject { + NSObject *_ivar; +} +@end + +@implementation SuperDeallocThenReleaseIvarClass +- (instancetype)initWithIvar:(NSObject *)ivar { + self = [super init]; + if (!self) + return nil; + _ivar = [ivar retain]; + return self; +} +- (void)dealloc { + [super dealloc]; + [_ivar release]; // expected-warning {{The 'dealloc' instance method in Objective-C class 'SuperDeallocThenReleaseIvarClass' references 'self' afer a call to [super dealloc], causing a use-after-free memory reference}} +} +@end + +@interface SuperDeallocThenAssignNilToIvarClass : NSObject { + NSObject *_delegate; +} +@end + +@implementation SuperDeallocThenAssignNilToIvarClass +- (instancetype)initWithDelegate:(NSObject *)delegate { + self = [super init]; + if (!self) + return nil; + _delegate = delegate; + return self; +} +- (void)dealloc { + [super dealloc]; + _delegate = nil; // expected-warning {{The 'dealloc' instance method in Objective-C class 'SuperDeallocThenAssignNilToIvarClass' references 'self' afer a call to [super dealloc], causing a use-after-free memory reference}} +} +@end + +@interface SuperDeallocThenReleasePropertyClass : NSObject { } +@property (retain) NSObject *ivar; +@end + +@implementation SuperDeallocThenReleasePropertyClass +- (instancetype)initWithProperty:(NSObject *)ivar { + self = [super init]; + if (!self) + return nil; + self.ivar = ivar; + return self; +} +- (void)dealloc { + [super dealloc]; + self.ivar = nil; // expected-warning {{The 'dealloc' instance method in Objective-C class 'SuperDeallocThenReleasePropertyClass' references 'self' afer a call to [super dealloc], causing a use-after-free memory reference}} +} +@end + +@interface SuperDeallocThenAssignNilToPropertyClass : NSObject { } +@property (assign) NSObject *delegate; +@end + +@implementation SuperDeallocThenAssignNilToPropertyClass +- (instancetype)initWithDelegate:(NSObject *)delegate { + self = [super init]; + if (!self) + return nil; + self.delegate = delegate; + return self; +} +- (void)dealloc { + [super dealloc]; + self.delegate = nil; // expected-warning {{The 'dealloc' instance method in Objective-C class 'SuperDeallocThenAssignNilToPropertyClass' references 'self' afer a call to [super dealloc], causing a use-after-free memory reference}} +} +@end + +@interface SuperDeallocThenCallInstanceMethodClass : NSObject { } +- (void)_invalidate; +@end + +@implementation SuperDeallocThenCallInstanceMethodClass +- (void)_invalidate { +} +- (void)dealloc { + [super dealloc]; + [self _invalidate]; // expected-warning {{The 'dealloc' instance method in Objective-C class 'SuperDeallocThenCallInstanceMethodClass' references 'self' afer a call to [super dealloc], causing a use-after-free memory reference}} +} +@end + +@interface SuperDeallocThenCallNonObjectiveCMethodClass : NSObject { } +@end + +static void _invalidate(NSObject *object) { + (void)object; +} + +@implementation SuperDeallocThenCallNonObjectiveCMethodClass +- (void)dealloc { + [super dealloc]; + _invalidate(self); // expected-warning {{The 'dealloc' instance method in Objective-C class 'SuperDeallocThenCallNonObjectiveCMethodClass' references 'self' afer a call to [super dealloc], causing a use-after-free memory reference}} +} +@end + +@interface TwoSuperDeallocCallsClass : NSObject { + NSObject *_ivar; +} +- (void)_invalidate; +@end + +@implementation TwoSuperDeallocCallsClass +- (void)_invalidate { +} +- (void)dealloc { + if (_ivar) { + [_ivar release]; + [super dealloc]; + return; + } + [super dealloc]; + [self _invalidate]; // expected-warning {{The 'dealloc' instance method in Objective-C class 'TwoSuperDeallocCallsClass' references 'self' afer a call to [super dealloc], causing a use-after-free memory reference}} +} +@end Index: tools/clang-check/Makefile =================================================================== --- tools/clang-check/Makefile +++ tools/clang-check/Makefile @@ -20,6 +20,6 @@ clangTooling.a clangParse.a clangSema.a \ clangStaticAnalyzerFrontend.a clangStaticAnalyzerCheckers.a \ clangStaticAnalyzerCore.a clangAnalysis.a clangRewriteFrontend.a \ - clangRewrite.a clangEdit.a clangAST.a clangLex.a clangBasic.a + clangRewrite.a clangEdit.a clangAST.a clangASTMatchers.a clangLex.a clangBasic.a include $(CLANG_LEVEL)/Makefile Index: tools/driver/Makefile =================================================================== --- tools/driver/Makefile +++ tools/driver/Makefile @@ -47,7 +47,7 @@ USEDLIBS += clangARCMigrate.a endif -USEDLIBS += clangAnalysis.a clangEdit.a clangAST.a clangLex.a clangBasic.a +USEDLIBS += clangAnalysis.a clangEdit.a clangAST.a clangASTMatchers.a clangLex.a clangBasic.a include $(CLANG_LEVEL)/Makefile