From d3d0ecb4f9544cdbffd8cf66bc420df7327468e2 Mon Sep 17 00:00:00 2001 From: Argiris Kirtzidis Date: Tue, 25 Jan 2011 00:04:03 +0000 Subject: [PATCH] [analyzer] Handle the dot syntax for properties in the ExprEngine. We translate property accesses to obj-c messages by simulating "loads" or "stores" to properties using a pseudo-location SVal kind (ObjCPropRef). Checkers can now reason about obj-c messages for both explicit message expressions and implicit messages due to property accesses. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@124161 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../StaticAnalyzer/PathSensitive/ExprEngine.h | 3 + include/clang/StaticAnalyzer/PathSensitive/SVals.h | 24 +++- lib/StaticAnalyzer/BasicStore.cpp | 1 + lib/StaticAnalyzer/CFRefCount.cpp | 5 +- .../Checkers/CallAndMessageChecker.cpp | 7 +- lib/StaticAnalyzer/Checkers/ExprEngine.cpp | 58 ++++++++- lib/StaticAnalyzer/RegionStore.cpp | 3 + lib/StaticAnalyzer/SVals.cpp | 17 +++ test/Analysis/properties.m | 135 +++++++++++++++++++++ 9 files changed, 241 insertions(+), 12 deletions(-) create mode 100644 test/Analysis/properties.m diff --git a/include/clang/StaticAnalyzer/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/PathSensitive/ExprEngine.h index 8a9a9f2cd..7b8aab0d6 100644 --- a/include/clang/StaticAnalyzer/PathSensitive/ExprEngine.h +++ b/include/clang/StaticAnalyzer/PathSensitive/ExprEngine.h @@ -375,6 +375,9 @@ public: void VisitObjCAtSynchronizedStmt(const ObjCAtSynchronizedStmt *S, ExplodedNode *Pred, ExplodedNodeSet &Dst); + void VisitObjCPropertyRefExpr(const ObjCPropertyRefExpr *E, + ExplodedNode *Pred, ExplodedNodeSet &Dst); + /// Transfer function logic for computing the lvalue of an Objective-C ivar. void VisitLvalObjCIvarRefExpr(const ObjCIvarRefExpr* DR, ExplodedNode* Pred, ExplodedNodeSet& Dst); diff --git a/include/clang/StaticAnalyzer/PathSensitive/SVals.h b/include/clang/StaticAnalyzer/PathSensitive/SVals.h index d4d846954..721c3598d 100644 --- a/include/clang/StaticAnalyzer/PathSensitive/SVals.h +++ b/include/clang/StaticAnalyzer/PathSensitive/SVals.h @@ -427,7 +427,7 @@ public: namespace loc { -enum Kind { GotoLabelKind, MemRegionKind, ConcreteIntKind }; +enum Kind { GotoLabelKind, MemRegionKind, ConcreteIntKind, ObjCPropRefKind }; class GotoLabel : public Loc { public: @@ -505,6 +505,28 @@ public: } }; +/// \brief Pseudo-location SVal used by the ExprEngine to simulate a "load" or +/// "store" of an ObjC property for the dot syntax. +class ObjCPropRef : public Loc { +public: + explicit ObjCPropRef(const ObjCPropertyRefExpr *E) + : Loc(ObjCPropRefKind, E) {} + + const ObjCPropertyRefExpr *getPropRefExpr() const { + return static_cast(Data); + } + + // Implement isa support. + static inline bool classof(const SVal* V) { + return V->getBaseKind() == LocKind && + V->getSubKind() == ObjCPropRefKind; + } + + static inline bool classof(const Loc* V) { + return V->getSubKind() == ObjCPropRefKind; + } +}; + } // end ento::loc namespace } // end GR namespace diff --git a/lib/StaticAnalyzer/BasicStore.cpp b/lib/StaticAnalyzer/BasicStore.cpp index 42fd3391f..0254ba777 100644 --- a/lib/StaticAnalyzer/BasicStore.cpp +++ b/lib/StaticAnalyzer/BasicStore.cpp @@ -196,6 +196,7 @@ SVal BasicStoreManager::Retrieve(Store store, Loc loc, QualType T) { return V.isUnknownOrUndef() ? V : CastRetrievedVal(V, TR, T); } + case loc::ObjCPropRefKind: case loc::ConcreteIntKind: // Support direct accesses to memory. It's up to individual checkers // to flag an error. diff --git a/lib/StaticAnalyzer/CFRefCount.cpp b/lib/StaticAnalyzer/CFRefCount.cpp index 5cc4a3c3e..473b7313b 100644 --- a/lib/StaticAnalyzer/CFRefCount.cpp +++ b/lib/StaticAnalyzer/CFRefCount.cpp @@ -2033,9 +2033,10 @@ PathDiagnosticPiece* CFRefReport::VisitNode(const ExplodedNode* N, else os << "function call"; } - else { - assert (isa(S)); + else if (isa(S)) { os << "Method"; + } else { + os << "Property"; } if (CurrV.getObjKind() == RetEffect::CF) { diff --git a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp index e6a40bb59..1f5f233b0 100644 --- a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp @@ -246,10 +246,11 @@ void CallAndMessageChecker::preVisitObjCMessage(CheckerContext &C, return; } + const char *bugDesc = msg.isPropertySetter() ? + "Argument for property setter is an uninitialized value" + : "Argument in message expression is an uninitialized value"; // Check for any arguments that are uninitialized/undefined. - PreVisitProcessArgs(C, CallOrObjCMessage(msg, state), - "Argument in message expression " - "is an uninitialized value", BT_msg_arg); + PreVisitProcessArgs(C, CallOrObjCMessage(msg, state), bugDesc, BT_msg_arg); } bool CallAndMessageChecker::evalNilReceiver(CheckerContext &C, diff --git a/lib/StaticAnalyzer/Checkers/ExprEngine.cpp b/lib/StaticAnalyzer/Checkers/ExprEngine.cpp index 9a74bd99a..53931dc60 100644 --- a/lib/StaticAnalyzer/Checkers/ExprEngine.cpp +++ b/lib/StaticAnalyzer/Checkers/ExprEngine.cpp @@ -865,6 +865,10 @@ void ExprEngine::Visit(const Stmt* S, ExplodedNode* Pred, VisitObjCAtSynchronizedStmt(cast(S), Pred, Dst); break; + case Stmt::ObjCPropertyRefExprClass: + VisitObjCPropertyRefExpr(cast(S), Pred, Dst); + break; + // Cases not handled yet; but will handle some day. case Stmt::DesignatedInitExprClass: case Stmt::ExtVectorElementExprClass: @@ -875,7 +879,6 @@ void ExprEngine::Visit(const Stmt* S, ExplodedNode* Pred, case Stmt::ObjCAtTryStmtClass: case Stmt::ObjCEncodeExprClass: case Stmt::ObjCIsaExprClass: - case Stmt::ObjCPropertyRefExprClass: case Stmt::ObjCProtocolExprClass: case Stmt::ObjCSelectorExprClass: case Stmt::ObjCStringLiteralClass: @@ -1799,6 +1802,17 @@ void ExprEngine::evalStore(ExplodedNodeSet& Dst, const Expr *AssignE, assert(Builder && "StmtNodeBuilder must be defined."); + // Proceed with the store. We use AssignE as the anchor for the PostStore + // ProgramPoint if it is non-NULL, and LocationE otherwise. + const Expr *StoreE = AssignE ? AssignE : LocationE; + + if (isa(location)) { + loc::ObjCPropRef prop = cast(location); + ExplodedNodeSet src = Pred; + return VisitObjCMessage(ObjCPropertySetter(prop.getPropRefExpr(), + StoreE, Val), src, Dst); + } + // Evaluate the location (checks for bad dereferences). ExplodedNodeSet Tmp; evalLocation(Tmp, LocationE, Pred, state, location, tag, false); @@ -1811,10 +1825,6 @@ void ExprEngine::evalStore(ExplodedNodeSet& Dst, const Expr *AssignE, SaveAndRestore OldSPointKind(Builder->PointKind, ProgramPoint::PostStoreKind); - // Proceed with the store. We use AssignE as the anchor for the PostStore - // ProgramPoint if it is non-NULL, and LocationE otherwise. - const Expr *StoreE = AssignE ? AssignE : LocationE; - for (ExplodedNodeSet::iterator NI=Tmp.begin(), NE=Tmp.end(); NI!=NE; ++NI) evalBind(Dst, StoreE, *NI, GetState(*NI), location, Val); } @@ -1825,6 +1835,13 @@ void ExprEngine::evalLoad(ExplodedNodeSet& Dst, const Expr *Ex, const void *tag, QualType LoadTy) { assert(!isa(location) && "location cannot be a NonLoc."); + if (isa(location)) { + loc::ObjCPropRef prop = cast(location); + ExplodedNodeSet src = Pred; + return VisitObjCMessage(ObjCPropertyGetter(prop.getPropRefExpr(), Ex), + src, Dst); + } + // Are we loading from a region? This actually results in two loads; one // to fetch the address of the referenced value and one to fetch the // referenced value. @@ -2048,6 +2065,34 @@ void ExprEngine::VisitCall(const CallExpr* CE, ExplodedNode* Pred, } //===----------------------------------------------------------------------===// +// Transfer function: Objective-C dot-syntax to access a property. +//===----------------------------------------------------------------------===// + +void ExprEngine::VisitObjCPropertyRefExpr(const ObjCPropertyRefExpr *Ex, + ExplodedNode *Pred, + ExplodedNodeSet &Dst) { + + // Visit the base expression, which is needed for computing the lvalue + // of the ivar. + ExplodedNodeSet dstBase; + const Expr *baseExpr = Ex->getBase(); + Visit(baseExpr, Pred, dstBase); + + ExplodedNodeSet dstPropRef; + + // Using the base, compute the lvalue of the instance variable. + for (ExplodedNodeSet::iterator I = dstBase.begin(), E = dstBase.end(); + I!=E; ++I) { + ExplodedNode *nodeBase = *I; + const GRState *state = GetState(nodeBase); + SVal baseVal = state->getSVal(baseExpr); + MakeNode(dstPropRef, Ex, *I, state->BindExpr(Ex, loc::ObjCPropRef(Ex))); + } + + Dst.insert(dstPropRef); +} + +//===----------------------------------------------------------------------===// // Transfer function: Objective-C ivar references. //===----------------------------------------------------------------------===// @@ -2426,7 +2471,8 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, ExplodedNodeSet S2; CheckerVisit(CastE, S2, S1, PreVisitStmtCallback); - if (CastE->getCastKind() == CK_LValueToRValue) { + if (CastE->getCastKind() == CK_LValueToRValue || + CastE->getCastKind() == CK_GetObjCProperty) { for (ExplodedNodeSet::iterator I = S2.begin(), E = S2.end(); I!=E; ++I) { ExplodedNode *subExprNode = *I; const GRState *state = GetState(subExprNode); diff --git a/lib/StaticAnalyzer/RegionStore.cpp b/lib/StaticAnalyzer/RegionStore.cpp index 986278e62..d5af1c274 100644 --- a/lib/StaticAnalyzer/RegionStore.cpp +++ b/lib/StaticAnalyzer/RegionStore.cpp @@ -985,6 +985,9 @@ SVal RegionStoreManager::Retrieve(Store store, Loc L, QualType T) { if (isa(L)) { return UnknownVal(); } + if (!isa(L)) { + return UnknownVal(); + } const MemRegion *MR = cast(L).getRegion(); diff --git a/lib/StaticAnalyzer/SVals.cpp b/lib/StaticAnalyzer/SVals.cpp index dd8508a50..1b2f1de49 100644 --- a/lib/StaticAnalyzer/SVals.cpp +++ b/lib/StaticAnalyzer/SVals.cpp @@ -13,6 +13,7 @@ //===----------------------------------------------------------------------===// #include "clang/StaticAnalyzer/PathSensitive/GRState.h" +#include "clang/AST/ExprObjC.h" #include "clang/Basic/IdentifierTable.h" using namespace clang; @@ -354,6 +355,22 @@ void Loc::dumpToStream(llvm::raw_ostream& os) const { case loc::MemRegionKind: os << '&' << cast(this)->getRegion()->getString(); break; + case loc::ObjCPropRefKind: { + const ObjCPropertyRefExpr *E = cast(this)->getPropRefExpr(); + os << "objc-prop{"; + if (E->isSuperReceiver()) + os << "super."; + else if (E->getBase()) + os << "."; + + if (E->isImplicitProperty()) + os << E->getImplicitPropertyGetter()->getSelector().getAsString(); + else + os << E->getExplicitProperty()->getName(); + + os << "}"; + break; + } default: assert(false && "Pretty-printing not implemented for this Loc."); break; diff --git a/test/Analysis/properties.m b/test/Analysis/properties.m new file mode 100644 index 000000000..b0325a1ce --- /dev/null +++ b/test/Analysis/properties.m @@ -0,0 +1,135 @@ +// RUN: %clang_cc1 -analyze -analyzer-check-objc-mem -analyzer-store=region -verify %s + +typedef signed char BOOL; +typedef unsigned int NSUInteger; +typedef struct _NSZone NSZone; +@class NSInvocation, NSMethodSignature, NSCoder, NSString, NSEnumerator; +@protocol NSObject - (BOOL)isEqual:(id)object; @end +@protocol NSCopying - (id)copyWithZone:(NSZone *)zone; @end +@protocol NSCoding - (void)encodeWithCoder:(NSCoder *)aCoder; @end +@protocol NSMutableCopying - (id)mutableCopyWithZone:(NSZone *)zone; @end +@interface NSObject {} ++(id)alloc; +-(id)init; +-(id)autorelease; +-(id)copy; +-(id)retain; +@end +@interface NSString : NSObject +- (NSUInteger)length; +-(id)initWithFormat:(NSString *)f,...; +-(BOOL)isEqualToString:(NSString *)s; ++ (id)string; +@end +@interface NSNumber : NSObject {} ++(id)alloc; +-(id)initWithInteger:(int)i; +@end + +// rdar://6946338 + +@interface Test1 : NSObject { + NSString *text; +} +-(id)myMethod; +@property (nonatomic, assign) NSString *text; +@end + + +@implementation Test1 + +@synthesize text; + +-(id)myMethod { + Test1 *cell = [[[Test1 alloc] init] autorelease]; + + NSString *string1 = [[NSString alloc] initWithFormat:@"test %f", 0.0]; // expected-warning {{Potential leak}} + cell.text = string1; + + return cell; +} + +@end + + +// rdar://8824416 + +@interface MyNumber : NSObject +{ + NSNumber* _myNumber; +} + +- (id)initWithNumber:(NSNumber *)number; + +@property (nonatomic, readonly) NSNumber* myNumber; +@property (nonatomic, readonly) NSNumber* newMyNumber; + +@end + +@implementation MyNumber +@synthesize myNumber=_myNumber; + +- (id)initWithNumber:(NSNumber *)number +{ + self = [super init]; + + if ( self ) + { + _myNumber = [number copy]; + } + + return self; +} + +- (NSNumber*)newMyNumber +{ + if ( _myNumber ) + return [_myNumber retain]; + + return [[NSNumber alloc] initWithInteger:1]; +} + +- (id)valueForUndefinedKey:(NSString*)key +{ + id value = 0; + + if ([key isEqualToString:@"MyIvarNumberAsPropertyOverReleased"]) + value = self.myNumber; // _myNumber will be over released, since the value returned from self.myNumber is not retained. + else if ([key isEqualToString:@"MyIvarNumberAsPropertyOk"]) + value = [self.myNumber retain]; // this line fixes the over release + else if ([key isEqualToString:@"MyIvarNumberAsNewMyNumber"]) + value = self.newMyNumber; // this one is ok, since value is returned retained + else + value = [[NSNumber alloc] initWithInteger:0]; + + return [value autorelease]; // expected-warning {{Object sent -autorelease too many times}} +} + +@end + +NSNumber* numberFromMyNumberProperty(MyNumber* aMyNumber) +{ + NSNumber* result = aMyNumber.myNumber; + + return [result autorelease]; // expected-warning {{Object sent -autorelease too many times}} +} + + +// rdar://6611873 + +@interface Person : NSObject { + NSString *_name; +} +@property (retain) NSString * name; +@end + +@implementation Person +@synthesize name = _name; +@end + +void rdar6611873() { + Person *p = [[[Person alloc] init] autorelease]; + + p.name = [[NSString string] retain]; // expected-warning {{leak}} + p.name = [[NSString alloc] init]; // expected-warning {{leak}} +} -- 2.11.4.GIT