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