[cfe-commits] r158319 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp lib/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Core/ExprEngineObjC.cpp test/Analysis/objc-for.m

Jordan Rose jordan_rose at apple.com
Mon Jun 11 09:40:41 PDT 2012


Author: jrose
Date: Mon Jun 11 11:40:41 2012
New Revision: 158319

URL: http://llvm.org/viewvc/llvm-project?rev=158319&view=rev
Log:
[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.

Added:
    cfe/trunk/test/Analysis/objc-for.m
Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
    cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp?rev=158319&r1=158318&r2=158319&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp Mon Jun 11 11:40:41 2012
@@ -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"
@@ -650,6 +651,75 @@
 }
 
 //===----------------------------------------------------------------------===//
+// 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::registerVariadicMethodTypeChecker(CheckerManager &mgr) {
   mgr.registerChecker<VariadicMethodTypeChecker>();
 }
+
+void ento::registerObjCLoopChecker(CheckerManager &mgr) {
+  mgr.registerChecker<ObjCLoopChecker>();
+}

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td?rev=158319&r1=158318&r2=158319&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td Mon Jun 11 11:40:41 2012
@@ -393,6 +393,10 @@
   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">;

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp?rev=158319&r1=158318&r2=158319&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp Mon Jun 11 11:40:41 2012
@@ -74,7 +74,6 @@
   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 @@
   }
   
   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 @@
     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,

Added: cfe/trunk/test/Analysis/objc-for.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/objc-for.m?rev=158319&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/objc-for.m (added)
+++ cfe/trunk/test/Analysis/objc-for.m Mon Jun 11 11:40:41 2012
@@ -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)
+
+ at protocol NSFastEnumeration
+- (int)countByEnumeratingWithState:(void *)state objects:(id *)objects count:(unsigned)count;
+ at end
+
+ at interface NSObject
++ (instancetype)testObject;
+ at end
+
+ at interface NSEnumerator <NSFastEnumeration>
+ at end
+
+ at interface NSArray : NSObject <NSFastEnumeration>
+- (NSEnumerator *)objectEnumerator;
+ at end
+
+ at interface NSDictionary : NSObject <NSFastEnumeration>
+ at end
+
+ at interface NSMutableDictionary : NSDictionary
+ at end
+
+ at interface NSSet : NSObject <NSFastEnumeration>
+ at end
+
+ at interface NSPointerArray : NSObject <NSFastEnumeration>
+ at 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}}
+}
+





More information about the cfe-commits mailing list