r194235 - [analyzer] Track whether an ObjC for-in loop had zero iterations.
NAKAMURA Takumi
geek4civic at gmail.com
Thu Nov 7 20:09:18 PST 2013
Jordan, seems it had not been built on trunk.
Please see my tweak, in r194244.
2013/11/8 Jordan Rose <jordan_rose at apple.com>:
> 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;
> + at end
> +
> + at interface NSDictionary (SomeCategory)
> +- (void)categoryMethodOnNSDictionary;
> @end
>
> @interface NSMutableDictionary : NSDictionary
> +- (void)setObject:(id)obj forKey:(id)key;
> + at end
> +
> + at 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list