r194235 - [analyzer] Track whether an ObjC for-in loop had zero iterations.
Jordan Rose
jordan_rose at apple.com
Thu Nov 7 17:15:36 PST 2013
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}}
+}
More information about the cfe-commits
mailing list