[Static Analyzer] Remove ObjCContainersChecker size information when a CFMutableArrayRef escapes

[dcoughlin]

Summary

Update ObjCContainersChecker to be notified when pointers escape so it can remove size information for escaping CFMutableArrayRefs. When such pointers escape, un-analyzed code could mutate the array and cause the size information to be incorrect.

rdar://problem/19406485

[zaks.anna]

LGTM! Please, commit.

[dcoughlin]

This was committed in r239709.

lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp

 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 
 using namespace clang;
 using namespace ento;
 
 namespace {
 class ObjCContainersChecker : public Checker< check::PreStmt<CallExpr>,
-                                             check::PostStmt<CallExpr> > {
+                                             check::PostStmt<CallExpr>,
+                                             check::PointerEscape> {
   mutable std::unique_ptr<BugType> BT;
   inline void initBugType() const {
     if (!BT)
       BT.reset(new BugType(this, "CFArray API",
                            categories::CoreFoundationObjectiveC));
   }
 
   inline SymbolRef getArraySym(const Expr *E, CheckerContext &C) const {
     SVal ArrayRef = C.getState()->getSVal(E, C.getLocationContext());
     SymbolRef ArraySym = ArrayRef.getAsSymbol();
     return ArraySym;
   }
 
   void addSizeInfo(const Expr *Array, const Expr *Size,
                    CheckerContext &C) const;
 
 public:
   /// A tag to id this checker.
   static void *getTag() { static int Tag; return &Tag; }
 
   void checkPostStmt(const CallExpr *CE, CheckerContext &C) const;
   void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
+  ProgramStateRef checkPointerEscape(ProgramStateRef State,
+                                     const InvalidatedSymbols &Escaped,
+                                     const CallEvent *Call,
+                                     PointerEscapeKind Kind) const;
 };
 } // end anonymous namespace
 
 // ProgramState trait - a map from array symbol to its state.
 REGISTER_MAP_WITH_PROGRAMSTATE(ArraySizeMap, SymbolRef, DefinedSVal)
 
 void ObjCContainersChecker::addSizeInfo(const Expr *Array, const Expr *Size,
                                         CheckerContext &C) const {
if (StOutBound && !StInBound) {
       BugReport *R = new BugReport(*BT, "Index is out of bounds", N);
       R->addRange(IdxExpr->getSourceRange());
       C.emitReport(R);
       return;
     }
   }
 }
 
+ProgramStateRef
+ObjCContainersChecker::checkPointerEscape(ProgramStateRef State,
+                                          const InvalidatedSymbols &Escaped,
+                                          const CallEvent *Call,
+                                          PointerEscapeKind Kind) const {
+  for (InvalidatedSymbols::const_iterator I = Escaped.begin(),
+                                          E = Escaped.end();
+                                          I != E; ++I) {
+    SymbolRef Sym = *I;
+    // When a symbol for a mutable array escapes, we can't reason precisely
+    // about its size any more -- so remove it from the map.
+    // Note that we aren't notified here when a CFMutableArrayRef escapes as a
+    // CFArrayRef. This is because CFArrayRef is typedef'd as a pointer to a
+    // const-qualified type.
+    State = State->remove<ArraySizeMap>(Sym);
+  }
+  return State;
+}
 /// Register checker.
 void ento::registerObjCContainersChecker(CheckerManager &mgr) {
   mgr.registerChecker<ObjCContainersChecker>();
 }

test/Analysis/CFContainers.mm

typedef struct {
     CFIndex version;
     CFArrayRetainCallBack retain;
     CFArrayReleaseCallBack release;
     CFArrayCopyDescriptionCallBack copyDescription;
     CFArrayEqualCallBack equal;
 } CFArrayCallBacks;
 typedef const struct __CFArray * CFArrayRef;
 CFArrayRef CFArrayCreate(CFAllocatorRef allocator, const void **values, CFIndex numValues, const CFArrayCallBacks *callBacks);
+typedef struct __CFArray * CFMutableArrayRef;
 typedef const struct __CFString * CFStringRef;
 enum {
     kCFNumberSInt8Type = 1,
     kCFNumberSInt16Type = 2,
     kCFNumberSInt32Type = 3,
     kCFNumberSInt64Type = 4,
     kCFNumberFloat32Type = 5,
     kCFNumberFloat64Type = 6,
void TestConst(CFArrayRef A, CFIndex sIndex, void* x[]) {
   CFArrayRef B = CFArrayCreate(0, (const void **) &x, 4, 0); 
   const void *s1 = CFArrayGetValueAtIndex(B, 2);
 
 }
 
 void TestNullArray() {
   CFArrayGetValueAtIndex(0, 0);
 }
+
+void ArrayRefMutableEscape(CFMutableArrayRef a);
+void ArrayRefEscape(CFArrayRef a);
+
+void TestCFMutableArrayRefEscapeViaMutableArgument(CFMutableArrayRef a) {
+  CFIndex aLen = CFArrayGetCount(a);
+  ArrayRefMutableEscape(a);
+
+  // ArrayRefMutableEscape could mutate a to make it have
+  // at least aLen + 1 elements, so do not report an error here.
+  CFArrayGetValueAtIndex(a, aLen);
+}
+
+void TestCFMutableArrayRefEscapeViaImmutableArgument(CFMutableArrayRef a) {
+  CFIndex aLen = CFArrayGetCount(a);
+  ArrayRefEscape(a);
+
+  // ArrayRefEscape is declared to take a CFArrayRef (i.e, an immutable array)
+  // so we assume it does not change the length of a.
+  CFArrayGetValueAtIndex(a, aLen); // expected-warning {{Index is out of bounds}}
+}