Jordan, seems it had not been built on trunk. Please see my tweak, in r194244.
2013/11/8 Jordan Rose <[email protected]>: > Author: jrose > Date: Thu Nov 7 19:15:35 2013 > New Revision: 194235 > > URL: http://llvm.org/viewvc/llvm-project?rev=194235&view=rev > Log: > [analyzer] Track whether an ObjC for-in loop had zero iterations. > > An Objective-C for-in loop will have zero iterations if the collection is > empty. Previously, we could only detect this case if the program asked for > the collection's -count /before/ the for-in loop. Now, the analyzer > distinguishes for-in loops that had zero iterations from those with at > least one, and can use this information to constrain the result of calling > -count after the loop. > > In order to make this actually useful, teach the checker that methods on > NSArray, NSDictionary, and the other immutable collection classes don't > change the count. > > <rdar://problem/14992886> > > Modified: > cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp > cfe/trunk/test/Analysis/objc-for.m > > Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp?rev=194235&r1=194234&r2=194235&view=diff > ============================================================================== > --- cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp > (original) > +++ cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp Thu > Nov 7 19:15:35 2013 > @@ -64,7 +64,8 @@ enum FoundationClass { > FC_NSString > }; > > -static FoundationClass findKnownClass(const ObjCInterfaceDecl *ID) { > +static FoundationClass findKnownClass(const ObjCInterfaceDecl *ID, > + bool IncludeSuperclasses = true) { > static llvm::StringMap<FoundationClass> Classes; > if (Classes.empty()) { > Classes["NSArray"] = FC_NSArray; > @@ -78,7 +79,7 @@ static FoundationClass findKnownClass(co > > // FIXME: Should we cache this at all? > FoundationClass result = Classes.lookup(ID->getIdentifier()->getName()); > - if (result == FC_None) > + if (result == FC_None && IncludeSuperclasses) > if (const ObjCInterfaceDecl *Super = ID->getSuperClass()) > return findKnownClass(Super); > > @@ -789,6 +790,7 @@ void VariadicMethodTypeChecker::checkPre > // The map from container symbol to the container count symbol. > // We currently will remember the last countainer count symbol encountered. > REGISTER_MAP_WITH_PROGRAMSTATE(ContainerCountMap, SymbolRef, SymbolRef) > +REGISTER_MAP_WITH_PROGRAMSTATE(ContainerNonEmptyMap, SymbolRef, bool) > > namespace { > class ObjCLoopChecker > @@ -896,19 +898,19 @@ static ProgramStateRef checkElementNonNi > > /// Returns NULL state if the collection is known to contain elements > /// (or is known not to contain elements if the Assumption parameter is > false.) > -static ProgramStateRef assumeCollectionNonEmpty(CheckerContext &C, > - ProgramStateRef State, > - const ObjCForCollectionStmt *FCS, > - bool Assumption = false) { > - if (!State) > - return NULL; > - > - SymbolRef CollectionS = C.getSVal(FCS->getCollection()).getAsSymbol(); > - if (!CollectionS) > +static ProgramStateRef > +assumeCollectionNonEmpty(CheckerContext &C, ProgramStateRef State, > + SymbolRef CollectionS, bool Assumption) { > + if (!State || !CollectionS) > return State; > + > const SymbolRef *CountS = State->get<ContainerCountMap>(CollectionS); > - if (!CountS) > - return State; > + if (!CountS) { > + const bool *KnownNonEmpty = > State->get<ContainerNonEmptyMap>(CollectionS); > + if (!KnownNonEmpty) > + return State->set<ContainerNonEmptyMap>(CollectionS, Assumption); > + return (Assumption == *KnownNonEmpty) ? State : NULL; > + } > > SValBuilder &SvalBuilder = C.getSValBuilder(); > SVal CountGreaterThanZeroVal = > @@ -927,6 +929,19 @@ static ProgramStateRef assumeCollectionN > return State->assume(*CountGreaterThanZero, Assumption); > } > > +static ProgramStateRef > +assumeCollectionNonEmpty(CheckerContext &C, ProgramStateRef State, > + const ObjCForCollectionStmt *FCS, > + bool Assumption) { > + if (!State) > + return NULL; > + > + SymbolRef CollectionS = > + State->getSVal(FCS->getCollection(), > C.getLocationContext()).getAsSymbol(); > + return assumeCollectionNonEmpty(C, State, CollectionS, Assumption); > +} > + > + > /// If the fist block edge is a back edge, we are reentering the loop. > static bool alreadyExecutedAtLeastOneLoopIteration(const ExplodedNode *N, > const ObjCForCollectionStmt > *FCS) { > @@ -1017,20 +1032,64 @@ void ObjCLoopChecker::checkPostObjCMessa > SymbolRef CountS = C.getSVal(MsgExpr).getAsSymbol(); > if (CountS) { > ProgramStateRef State = C.getState(); > + > C.getSymbolManager().addSymbolDependency(ContainerS, CountS); > State = State->set<ContainerCountMap>(ContainerS, CountS); > + > + if (const bool *NonEmpty = State->get<ContainerNonEmptyMap>(ContainerS)) > { > + State = State->remove<ContainerNonEmptyMap>(ContainerS); > + State = assumeCollectionNonEmpty(C, State, ContainerS, *NonEmpty); > + } > + > C.addTransition(State); > } > return; > } > > +static SymbolRef getMethodReceiverIfKnownImmutable(const CallEvent *Call) { > + const ObjCMethodCall *Message = dyn_cast_or_null<ObjCMethodCall>(Call); > + if (!Message) > + return 0; > + > + const ObjCMethodDecl *MD = Message->getDecl(); > + if (!MD) > + return 0; > + > + const ObjCInterfaceDecl *StaticClass; > + if (isa<ObjCProtocolDecl>(MD->getDeclContext())) { > + // We can't find out where the method was declared without doing more > work. > + // Instead, see if the receiver is statically typed as a known immutable > + // collection. > + StaticClass = Message->getOriginExpr()->getReceiverInterface(); > + } else { > + StaticClass = MD->getClassInterface(); > + } > + > + if (!StaticClass) > + return 0; > + > + switch (findKnownClass(StaticClass, /*IncludeSuper=*/false)) { > + case FC_None: > + return 0; > + case FC_NSArray: > + case FC_NSDictionary: > + case FC_NSEnumerator: > + case FC_NSNull: > + case FC_NSOrderedSet: > + case FC_NSSet: > + case FC_NSString: > + break; > + } > + > + return Message->getReceiverSVal().getAsSymbol(); > +} > + > ProgramStateRef > ObjCLoopChecker::checkPointerEscape(ProgramStateRef State, > const InvalidatedSymbols &Escaped, > const CallEvent *Call, > PointerEscapeKind Kind) const { > - // TODO: If we know that the call cannot change the collection count, there > - // is nothing to do, just return. > + SymbolRef ImmutableReceiver = getMethodReceiverIfKnownImmutable(Call); > > // Remove the invalidated symbols form the collection count map. > for (InvalidatedSymbols::const_iterator I = Escaped.begin(), > @@ -1038,9 +1097,17 @@ ObjCLoopChecker::checkPointerEscape(Prog > I != E; ++I) { > SymbolRef Sym = *I; > > + // Don't invalidate this symbol's count if we know the method being > called > + // is declared on an immutable class. This isn't completely correct if > the > + // receiver is also passed as an argument, but in most uses of NSArray, > + // NSDictionary, etc. this isn't likely to happen in a dangerous way. > + if (Sym == ImmutableReceiver) > + continue; > + > // The symbol escaped. Pessimistically, assume that the count could have > // changed. > State = State->remove<ContainerCountMap>(Sym); > + State = State->remove<ContainerNonEmptyMap>(Sym); > } > return State; > } > @@ -1054,8 +1121,10 @@ void ObjCLoopChecker::checkDeadSymbols(S > for (ContainerCountMapTy::iterator I = Tracked.begin(), > E = Tracked.end(); I != E; ++I) { > SymbolRef Sym = I->first; > - if (SymReaper.isDead(Sym)) > + if (SymReaper.isDead(Sym)) { > State = State->remove<ContainerCountMap>(Sym); > + State = State->remove<ContainerNonEmptyMap>(Sym); > + } > } > > C.addTransition(State); > > Modified: cfe/trunk/test/Analysis/objc-for.m > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/objc-for.m?rev=194235&r1=194234&r2=194235&view=diff > ============================================================================== > --- cfe/trunk/test/Analysis/objc-for.m (original) > +++ cfe/trunk/test/Analysis/objc-for.m Thu Nov 7 19:15:35 2013 > @@ -7,6 +7,7 @@ void clang_analyzer_eval(int); > typedef unsigned long NSUInteger; > @protocol NSFastEnumeration > - (int)countByEnumeratingWithState:(void *)state objects:(id *)objects > count:(unsigned)count; > +- (void)protocolMethod; > @end > > @interface NSObject > @@ -23,9 +24,19 @@ typedef unsigned long NSUInteger; > > @interface NSDictionary : NSObject <NSFastEnumeration> > - (NSUInteger)count; > +- (id)objectForKey:(id)key; > +@end > + > +@interface NSDictionary (SomeCategory) > +- (void)categoryMethodOnNSDictionary; > @end > > @interface NSMutableDictionary : NSDictionary > +- (void)setObject:(id)obj forKey:(id)key; > +@end > + > +@interface NSMutableArray : NSArray > +- (void)addObject:(id)obj; > @end > > @interface NSSet : NSObject <NSFastEnumeration> > @@ -169,10 +180,13 @@ void onlySuppressLoopExitAfterZeroIterat > } > } > > -int consistencyBetweenLoopsWhenCountIsUnconstrained(NSMutableDictionary *D) { > - // Note, The current limitation is that we need to have a count. > - // TODO: This should work even when we do not call count. > - int count = [D count]; > +int consistencyBetweenLoopsWhenCountIsUnconstrained(NSMutableDictionary *D, > + int shouldUseCount) { > + // Test with or without an initial count. > + int count; > + if (shouldUseCount) > + count = [D count]; > + > int i; > int j = 0; > for (NSString *key in D) { > @@ -185,8 +199,12 @@ int consistencyBetweenLoopsWhenCountIsUn > return 0; > } > > -int consistencyBetweenLoopsWhenCountIsUnconstrained_dual(NSMutableDictionary > *D) { > - int count = [D count]; > +int consistencyBetweenLoopsWhenCountIsUnconstrained_dual(NSMutableDictionary > *D, > + int shouldUseCount) > { > + int count; > + if (shouldUseCount) > + count = [D count]; > + > int i = 8; > int j = 1; > for (NSString *key in D) { > @@ -199,3 +217,110 @@ int consistencyBetweenLoopsWhenCountIsUn > } > return 5/i; > } > + > +int consistencyCountThenLoop(NSArray *array) { > + if ([array count] == 0) > + return 0; > + > + int x; > + for (id y in array) > + x = 0; > + return x; // no-warning > +} > + > +int consistencyLoopThenCount(NSArray *array) { > + int x; > + for (id y in array) > + x = 0; > + > + if ([array count] == 0) > + return 0; > + > + return x; // no-warning > +} > + > +void nonMutatingMethodsDoNotInvalidateCountDictionary(NSMutableDictionary > *dict, > + NSMutableArray *other) > { > + if ([dict count]) > + return; > + > + for (id key in dict) > + clang_analyzer_eval(0); // no-warning > + > + (void)[dict objectForKey:@""]; > + > + for (id key in dict) > + clang_analyzer_eval(0); // no-warning > + > + [dict categoryMethodOnNSDictionary]; > + > + for (id key in dict) > + clang_analyzer_eval(0); // no-warning > + > + [dict setObject:@"" forKey:@""]; > + > + for (id key in dict) > + clang_analyzer_eval(0); // expected-warning{{FALSE}} > + > + // Reset. > + if ([dict count]) > + return; > + > + for (id key in dict) > + clang_analyzer_eval(0); // no-warning > + > + [other addObject:dict]; > + > + for (id key in dict) > + clang_analyzer_eval(0); // expected-warning{{FALSE}} > +} > + > +void nonMutatingMethodsDoNotInvalidateCountArray(NSMutableArray *array, > + NSMutableArray *other) { > + if ([array count]) > + return; > + > + for (id key in array) > + clang_analyzer_eval(0); // no-warning > + > + (void)[array objectEnumerator]; > + > + for (id key in array) > + clang_analyzer_eval(0); // no-warning > + > + [array addObject:@""]; > + > + for (id key in array) > + clang_analyzer_eval(0); // expected-warning{{FALSE}} > + > + // Reset. > + if ([array count]) > + return; > + > + for (id key in array) > + clang_analyzer_eval(0); // no-warning > + > + [other addObject:array]; > + > + for (id key in array) > + clang_analyzer_eval(0); // expected-warning{{FALSE}} > +} > + > +void protocolMethods(NSMutableArray *array) { > + if ([array count]) > + return; > + > + for (id key in array) > + clang_analyzer_eval(0); // no-warning > + > + NSArray *immutableArray = array; > + [immutableArray protocolMethod]; > + > + for (id key in array) > + clang_analyzer_eval(0); // no-warning > + > + [array protocolMethod]; > + > + for (id key in array) > + clang_analyzer_eval(0); // expected-warning{{FALSE}} > +} > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
