[analyzer] Add ObjCLoopChecker: objects from NSArray et al are non-nil.

While collections containing nil elements can still be iterated over in an
Objective-C for-in loop, the most common Cocoa collections -- NSArray,
NSDictionary, and NSSet -- cannot contain nil elements. This checker adds
that assumption to the analyzer state.

This was the cause of some minor false positives concerning CFRelease calls
on objects in an NSArray.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@158319 91177308-0d34-0410-b5e6-96231b3b80d8

lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp

@@ -27,6 +27,7 @@
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprObjC.h"
+#include "clang/AST/StmtObjC.h"
 #include "clang/AST/ASTContext.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"
@@ -649,6 +650,75 @@ void VariadicMethodTypeChecker::checkPreObjCMessage(ObjCMessage msg,
   }
 }
 
+//===----------------------------------------------------------------------===//
+// Improves the modeling of loops over Cocoa collections.
+//===----------------------------------------------------------------------===//
+
+namespace {
+class ObjCLoopChecker
+  : public Checker<check::PostStmt<ObjCForCollectionStmt> > {
+  
+public:
+  void checkPostStmt(const ObjCForCollectionStmt *FCS, CheckerContext &C) const;
+};
+}
+
+static bool isKnownNonNilCollectionType(QualType T) {
+  const ObjCObjectPointerType *PT = T->getAs<ObjCObjectPointerType>();
+  if (!PT)
+    return false;
+  
+  const ObjCInterfaceDecl *ID = PT->getInterfaceDecl();
+  if (!ID)
+    return false;
+
+  switch (findKnownClass(ID)) {
+  case FC_NSArray:
+  case FC_NSDictionary:
+  case FC_NSEnumerator:
+  case FC_NSOrderedSet:
+  case FC_NSSet:
+    return true;
+  default:
+    return false;
+  }
+}
+
+void ObjCLoopChecker::checkPostStmt(const ObjCForCollectionStmt *FCS,
+                                    CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  
+  // Check if this is the branch for the end of the loop.
+  SVal CollectionSentinel = State->getSVal(FCS, C.getLocationContext());
+  if (CollectionSentinel.isZeroConstant())
+    return;
+  
+  // See if the collection is one where we /know/ the elements are non-nil.
+  const Expr *Collection = FCS->getCollection();
+  if (!isKnownNonNilCollectionType(Collection->getType()))
+    return;
+  
+  // FIXME: Copied from ExprEngineObjC.
+  const Stmt *Element = FCS->getElement();
+  SVal ElementVar;
+  if (const DeclStmt *DS = dyn_cast<DeclStmt>(Element)) {
+    const VarDecl *ElemDecl = cast<VarDecl>(DS->getSingleDecl());
+    assert(ElemDecl->getInit() == 0);
+    ElementVar = State->getLValue(ElemDecl, C.getLocationContext());
+  } else {
+    ElementVar = State->getSVal(Element, C.getLocationContext());
+  }
+
+  if (!isa<Loc>(ElementVar))
+    return;
+
+  // Go ahead and assume the value is non-nil.
+  SVal Val = State->getSVal(cast<Loc>(ElementVar));
+  State = State->assume(cast<DefinedOrUnknownSVal>(Val), true);
+  C.addTransition(State);
+}
+
+
 //===----------------------------------------------------------------------===//
 // Check registration.
 //===----------------------------------------------------------------------===//
@@ -672,3 +742,7 @@ void ento::registerClassReleaseChecker(CheckerManager &mgr) {
 void ento::registerVariadicMethodTypeChecker(CheckerManager &mgr) {
   mgr.registerChecker<VariadicMethodTypeChecker>();
 }
+
+void ento::registerObjCLoopChecker(CheckerManager &mgr) {
+  mgr.registerChecker<ObjCLoopChecker>();
+}

lib/StaticAnalyzer/Checkers/Checkers.td

@@ -393,6 +393,10 @@ def ObjCSelfInitChecker : Checker<"SelfInit">,
   HelpText<"Check that 'self' is properly initialized inside an initializer method">,
   DescFile<"ObjCSelfInitChecker.cpp">;
 
+def ObjCLoopChecker : Checker<"Loops">,
+  HelpText<"Improved modeling of loops using Cocoa collection types">,
+  DescFile<"BasicObjCFoundationChecks.cpp">;
+
 def NSErrorChecker : Checker<"NSError">,
   HelpText<"Check usage of NSError** parameters">,
   DescFile<"NSErrorChecker.cpp">;

lib/StaticAnalyzer/Core/ExprEngineObjC.cpp

@@ -74,7 +74,6 @@ void ExprEngine::VisitObjCForCollectionStmt(const ObjCForCollectionStmt *S,
   const Stmt *elem = S->getElement();
   ProgramStateRef state = Pred->getState();
   SVal elementV;
-  StmtNodeBuilder Bldr(Pred, Dst, *currentBuilderContext);
   
   if (const DeclStmt *DS = dyn_cast<DeclStmt>(elem)) {
     const VarDecl *elemD = cast<VarDecl>(DS->getSingleDecl());
@@ -86,10 +85,11 @@ void ExprEngine::VisitObjCForCollectionStmt(const ObjCForCollectionStmt *S,
   }
   
   ExplodedNodeSet dstLocation;
-  Bldr.takeNodes(Pred);
   evalLocation(dstLocation, S, elem, Pred, state, elementV, NULL, false);
-  Bldr.addNodes(dstLocation);
-  
+
+  ExplodedNodeSet Tmp;
+  StmtNodeBuilder Bldr(Pred, Tmp, *currentBuilderContext);
+
   for (ExplodedNodeSet::iterator NI = dstLocation.begin(),
        NE = dstLocation.end(); NI!=NE; ++NI) {
     Pred = *NI;
@@ -126,6 +126,10 @@ void ExprEngine::VisitObjCForCollectionStmt(const ObjCForCollectionStmt *S,
     Bldr.generateNode(S, Pred, hasElems);
     Bldr.generateNode(S, Pred, noElems);
   }
+
+  // Finally, run any custom checkers.
+  // FIXME: Eventually all pre- and post-checks should live in VisitStmt.
+  getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this);
 }
 
 void ExprEngine::VisitObjCMessage(const ObjCMessage &msg,

test/Analysis/objc-for.m

@@ -0,0 +1,58 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=osx.cocoa.Loops,debug.ExprInspection -verify
+
+void clang_analyzer_eval(int);
+
+#define nil ((id)0)
+
+@protocol NSFastEnumeration
+- (int)countByEnumeratingWithState:(void *)state objects:(id *)objects count:(unsigned)count;
+@end
+
+@interface NSObject
++ (instancetype)testObject;
+@end
+
+@interface NSEnumerator <NSFastEnumeration>
+@end
+
+@interface NSArray : NSObject <NSFastEnumeration>
+- (NSEnumerator *)objectEnumerator;
+@end
+
+@interface NSDictionary : NSObject <NSFastEnumeration>
+@end
+
+@interface NSMutableDictionary : NSDictionary
+@end
+
+@interface NSSet : NSObject <NSFastEnumeration>
+@end
+
+@interface NSPointerArray : NSObject <NSFastEnumeration>
+@end
+
+void test() {
+  id x;
+  for (x in [NSArray testObject])
+    clang_analyzer_eval(x != nil); // expected-warning{{TRUE}}
+
+  for (x in [NSMutableDictionary testObject])
+    clang_analyzer_eval(x != nil); // expected-warning{{TRUE}}
+
+  for (x in [NSSet testObject])
+    clang_analyzer_eval(x != nil); // expected-warning{{TRUE}}
+
+  for (x in [[NSArray testObject] objectEnumerator])
+    clang_analyzer_eval(x != nil); // expected-warning{{TRUE}}
+
+  for (x in [NSPointerArray testObject])
+    clang_analyzer_eval(x != nil); // expected-warning{{UNKNOWN}}
+}
+
+void testWithVarInFor() {
+  for (id x in [NSArray testObject])
+    clang_analyzer_eval(x != nil); // expected-warning{{TRUE}}
+  for (id x in [NSPointerArray testObject])
+    clang_analyzer_eval(x != nil); // expected-warning{{UNKNOWN}}
+}
+