diff --git a/lib/Analysis/AnalysisDeclContext.cpp b/lib/Analysis/AnalysisDeclContext.cpp index 6392e52e444a9e7598e461f68bae94beba6ad351..79918a3dbcc4278e49de13d5833d9351ba12d9ad 100644 --- a/lib/Analysis/AnalysisDeclContext.cpp +++ b/lib/Analysis/AnalysisDeclContext.cpp @@ -94,14 +94,21 @@ Stmt *AnalysisDeclContext::getBody(bool &IsAutosynthesized) const { if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) { Stmt *Body = FD->getBody(); if (!Body && Manager && Manager->synthesizeBodies()) { - IsAutosynthesized = true; - return getBodyFarm(getASTContext()).getBody(FD); + Body = getBodyFarm(getASTContext()).getBody(FD); + if (Body) + IsAutosynthesized = true; } return Body; } - else if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D)) - return MD->getBody(); - else if (const BlockDecl *BD = dyn_cast<BlockDecl>(D)) + else if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D)) { + Stmt *Body = MD->getBody(); + if (!Body && Manager && Manager->synthesizeBodies()) { + Body = getBodyFarm(getASTContext()).getBody(MD); + if (Body) + IsAutosynthesized = true; + } + return Body; + } else if (const BlockDecl *BD = dyn_cast<BlockDecl>(D)) return BD->getBody(); else if (const FunctionTemplateDecl *FunTmpl = dyn_cast_or_null<FunctionTemplateDecl>(D)) diff --git a/lib/Analysis/BodyFarm.cpp b/lib/Analysis/BodyFarm.cpp index 4d5c2ee236f9905d63d469005cc308ec3b199c66..a6567ade52f39356f34808791427fe69fc7018dd 100644 --- a/lib/Analysis/BodyFarm.cpp +++ b/lib/Analysis/BodyFarm.cpp @@ -74,6 +74,9 @@ public: /// Create an Objective-C bool literal. ObjCBoolLiteralExpr *makeObjCBool(bool Val); + + /// Create an Objective-C ivar reference. + ObjCIvarRefExpr *makeObjCIvarRef(const Expr *Base, const ObjCIvarDecl *IVar); /// Create a Return statement. ReturnStmt *makeReturn(const Expr *RetVal); @@ -147,6 +150,15 @@ ObjCBoolLiteralExpr *ASTMaker::makeObjCBool(bool Val) { return new (C) ObjCBoolLiteralExpr(Val, Ty, SourceLocation()); } +ObjCIvarRefExpr *ASTMaker::makeObjCIvarRef(const Expr *Base, + const ObjCIvarDecl *IVar) { + return new (C) ObjCIvarRefExpr(const_cast<ObjCIvarDecl*>(IVar), + IVar->getType(), SourceLocation(), + SourceLocation(), const_cast<Expr*>(Base), + /*arrow=*/true, /*free=*/false); +} + + ReturnStmt *ASTMaker::makeReturn(const Expr *RetVal) { return new (C) ReturnStmt(SourceLocation(), const_cast<Expr*>(RetVal), 0); } @@ -374,3 +386,61 @@ Stmt *BodyFarm::getBody(const FunctionDecl *D) { return Val.getValue(); } +static Stmt *createObjCPropertyGetter(ASTContext &Ctx, + const ObjCPropertyDecl *Prop) { + const ObjCIvarDecl *IVar = Prop->getPropertyIvarDecl(); + if (!IVar) + return 0; + if (Prop->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_weak) + return 0; + if (IVar->getType().getCanonicalType() != + Prop->getType().getNonReferenceType().getCanonicalType()) + return 0; + + // C++ records require copy constructors, so we can't just synthesize an AST. + // FIXME: Use ObjCPropertyImplDecl's already-synthesized AST. Currently it's + // not in a form the analyzer can use. + if (Prop->getType()->getAsCXXRecordDecl()) + return 0; + + ASTMaker M(Ctx); + + const VarDecl *selfVar = Prop->getGetterMethodDecl()->getSelfDecl(); + + Expr *loadedIVar = + M.makeObjCIvarRef( + M.makeLvalueToRvalue( + M.makeDeclRefExpr(selfVar), + selfVar->getType()), + IVar); + + if (!Prop->getType()->isReferenceType()) + loadedIVar = M.makeLvalueToRvalue(loadedIVar, IVar->getType()); + + return M.makeReturn(loadedIVar); +} + +Stmt *BodyFarm::getBody(const ObjCMethodDecl *D, const ObjCPropertyDecl *Prop) { + if (!D->isPropertyAccessor()) + return 0; + + D = D->getCanonicalDecl(); + + Optional<Stmt *> &Val = Bodies[D]; + if (Val.hasValue()) + return Val.getValue(); + Val = 0; + + if (!Prop) + Prop = D->findPropertyDecl(); + if (!Prop) + return 0; + + if (D->param_size() != 0) + return 0; + + Val = createObjCPropertyGetter(C, Prop); + + return Val.getValue(); +} + diff --git a/lib/Analysis/BodyFarm.h b/lib/Analysis/BodyFarm.h index 96f61df40d7fc8d2fd084e003cf6c425c2003e33..c4f3d1599e738932a75872739ac69435c5876353 100644 --- a/lib/Analysis/BodyFarm.h +++ b/lib/Analysis/BodyFarm.h @@ -24,6 +24,8 @@ namespace clang { class ASTContext; class Decl; class FunctionDecl; +class ObjCMethodDecl; +class ObjCPropertyDecl; class Stmt; class BodyFarm { @@ -32,7 +34,10 @@ public: /// Factory method for creating bodies for ordinary functions. Stmt *getBody(const FunctionDecl *D); - + + /// Factory method for creating bodies for Objective-C properties. + Stmt *getBody(const ObjCMethodDecl *D, const ObjCPropertyDecl *Prop = 0); + private: typedef llvm::DenseMap<const Decl *, Optional<Stmt *> > BodyMap; diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp index 1108b090c1b6100d8bf10bba098fb5fe1ebe4b73..32f762e1453389db4140df142eb1391b07d40333 100644 --- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp @@ -2735,6 +2735,16 @@ static QualType GetReturnType(const Expr *RetE, ASTContext &Ctx) { return RetTy; } +static bool wasSynthesizedProperty(const ObjCMethodCall *Call, + ExplodedNode *N) { + if (!Call || !Call->getDecl()->isPropertyAccessor()) + return false; + + CallExitEnd PP = N->getLocation().castAs<CallExitEnd>(); + const StackFrameContext *Frame = PP.getCalleeContext(); + return Frame->getAnalysisDeclContext()->isBodyAutosynthesized(); +} + // We don't always get the exact modeling of the function with regards to the // retain count checker even when the function is inlined. For example, we need // to stop tracking the symbols which were marked with StopTrackingHard. @@ -2769,6 +2779,15 @@ void RetainCountChecker::processSummaryOfInlined(const RetainSummary &Summ, SymbolRef Sym = CallOrMsg.getReturnValue().getAsSymbol(); if (Sym) state = removeRefBinding(state, Sym); + } else if (RE.getKind() == RetEffect::NotOwnedSymbol) { + if (wasSynthesizedProperty(MsgInvocation, C.getPredecessor())) { + // Believe the summary if we synthesized the body and the return value is + // untracked. This handles property getters. + SymbolRef Sym = CallOrMsg.getReturnValue().getAsSymbol(); + if (Sym && !getRefBinding(state, Sym)) + state = setRefBinding(state, Sym, RefVal::makeNotOwned(RE.getObjKind(), + Sym->getType())); + } } C.addTransition(state); diff --git a/lib/StaticAnalyzer/Core/CallEvent.cpp b/lib/StaticAnalyzer/Core/CallEvent.cpp index 838d273cc0af188785ed53cc87c601e2aecde985..71895051e2cf65fbc6488120c04979b6ec67134b 100644 --- a/lib/StaticAnalyzer/Core/CallEvent.cpp +++ b/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -863,9 +863,17 @@ RuntimeDefinition ObjCMethodCall::getRuntimeDefinition() const { Optional<const ObjCMethodDecl *> &Val = PMC[std::make_pair(IDecl, Sel)]; // Query lookupPrivateMethod() if the cache does not hit. - if (!Val.hasValue()) + if (!Val.hasValue()) { Val = IDecl->lookupPrivateMethod(Sel); + // If the method is a property accessor, we should try to "inline" it + // even if we don't actually have an implementation. + if (!*Val) + if (const ObjCMethodDecl *CompileTimeMD = E->getMethodDecl()) + if (CompileTimeMD->isPropertyAccessor()) + Val = IDecl->lookupInstanceMethod(Sel); + } + const ObjCMethodDecl *MD = Val.getValue(); if (CanBeSubClassed) return RuntimeDefinition(MD, Receiver); diff --git a/test/Analysis/properties.m b/test/Analysis/properties.m index ddd0068d36986e3d16b3b378840c5aea6ff2d799..b3e654f377a98583b9c87f669363a98f25b0a493 100644 --- a/test/Analysis/properties.m +++ b/test/Analysis/properties.m @@ -1,4 +1,6 @@ -// RUN: %clang_cc1 -analyze -analyzer-checker=core,osx.cocoa.RetainCount -analyzer-store=region -verify -Wno-objc-root-class %s +// RUN: %clang_cc1 -analyze -analyzer-checker=core,osx.cocoa.RetainCount,debug.ExprInspection -analyzer-store=region -verify -Wno-objc-root-class %s + +void clang_analyzer_eval(int); typedef signed char BOOL; typedef unsigned int NSUInteger; @@ -14,6 +16,7 @@ typedef struct _NSZone NSZone; -(id)autorelease; -(id)copy; -(id)retain; +-(oneway void)release; @end @interface NSString : NSObject <NSCopying, NSMutableCopying, NSCoding> - (NSUInteger)length; @@ -166,3 +169,79 @@ void rdar6611873() { @end +//------ +// Property accessor synthesis +//------ + +void testConsistency(Person *p) { + clang_analyzer_eval(p.name == p.name); // expected-warning{{TRUE}} + + extern void doSomethingWithPerson(Person *p); + id origName = p.name; + clang_analyzer_eval(p.name == origName); // expected-warning{{TRUE}} + doSomethingWithPerson(p); + clang_analyzer_eval(p.name == origName); // expected-warning{{UNKNOWN}} +} + +void testOverrelease(Person *p) { + [p.name release]; // expected-warning{{not owned}} +} + +@interface IntWrapper +@property int value; +@end + +@implementation IntWrapper +@synthesize value; +@end + +void testConsistencyInt(IntWrapper *w) { + clang_analyzer_eval(w.value == w.value); // expected-warning{{TRUE}} + + int origValue = w.value; + if (origValue != 42) + return; + + clang_analyzer_eval(w.value == 42); // expected-warning{{TRUE}} +} + +void testConsistencyInt2(IntWrapper *w) { + if (w.value != 42) + return; + + clang_analyzer_eval(w.value == 42); // expected-warning{{TRUE}} +} + +typedef struct { + int value; +} IntWrapperStruct; + +@interface StructWrapper +@property IntWrapperStruct inner; +@end + +@implementation StructWrapper +@synthesize inner; +@end + +void testConsistencyStruct(StructWrapper *w) { + clang_analyzer_eval(w.inner.value == w.inner.value); // expected-warning{{TRUE}} + + int origValue = w.inner.value; + if (origValue != 42) + return; + + clang_analyzer_eval(w.inner.value == 42); // expected-warning{{TRUE}} +} + + +@interface OpaqueIntWrapper +@property int value; +@end + +// For now, don't assume a property is implemented using an ivar unless we can +// actually see that it is. +void testOpaqueConsistency(OpaqueIntWrapper *w) { + clang_analyzer_eval(w.value == w.value); // expected-warning{{UNKNOWN}} +} + diff --git a/test/Analysis/properties.mm b/test/Analysis/properties.mm new file mode 100644 index 0000000000000000000000000000000000000000..dd44219856f81f87b91e6e85265d7b188e85a9e2 --- /dev/null +++ b/test/Analysis/properties.mm @@ -0,0 +1,80 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=core,osx.cocoa.RetainCount,debug.ExprInspection -analyzer-store=region -verify -Wno-objc-root-class %s + +void clang_analyzer_eval(bool); +void clang_analyzer_checkInlined(bool); + +@interface IntWrapper +@property (readonly) int &value; +@end + +@implementation IntWrapper +@synthesize value; +@end + +void testReferenceConsistency(IntWrapper *w) { + clang_analyzer_eval(w.value == w.value); // expected-warning{{TRUE}} + clang_analyzer_eval(&w.value == &w.value); // expected-warning{{TRUE}} + + if (w.value != 42) + return; + + clang_analyzer_eval(w.value == 42); // expected-warning{{TRUE}} +} + +void testReferenceAssignment(IntWrapper *w) { + w.value = 42; + clang_analyzer_eval(w.value == 42); // expected-warning{{TRUE}} +} + + +// FIXME: Handle C++ structs, which need to go through the copy constructor. + +struct IntWrapperStruct { + int value; +}; + +@interface StructWrapper +@property IntWrapperStruct inner; +@end + +@implementation StructWrapper +@synthesize inner; +@end + +void testConsistencyStruct(StructWrapper *w) { + clang_analyzer_eval(w.inner.value == w.inner.value); // expected-warning{{UNKNOWN}} + + int origValue = w.inner.value; + if (origValue != 42) + return; + + clang_analyzer_eval(w.inner.value == 42); // expected-warning{{UNKNOWN}} +} + + +class CustomCopy { +public: + CustomCopy() : value(0) {} + CustomCopy(const CustomCopy &other) { + clang_analyzer_checkInlined(false); + } + int value; +}; + +@interface CustomCopyWrapper +@property CustomCopy inner; +@end + +@implementation CustomCopyWrapper +@synthesize inner; +@end + +void testConsistencyCustomCopy(CustomCopyWrapper *w) { + clang_analyzer_eval(w.inner.value == w.inner.value); // expected-warning{{UNKNOWN}} + + int origValue = w.inner.value; + if (origValue != 42) + return; + + clang_analyzer_eval(w.inner.value == 42); // expected-warning{{UNKNOWN}} +}