1 files changed, 64 insertions, 54 deletions

diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index f412c049c7..28a2999f04 100644
--- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -168,12 +168,13 @@ public:
 private:
   void initIdentifierInfo(ASTContext &C) const;
 
+  ///@{
   /// Check if this is one of the functions which can allocate/reallocate memory 
   /// pointed to by one of its arguments.
   bool isMemFunction(const FunctionDecl *FD, ASTContext &C) const;
   bool isFreeFunction(const FunctionDecl *FD, ASTContext &C) const;
   bool isAllocationFunction(const FunctionDecl *FD, ASTContext &C) const;
-
+  ///@}
   static ProgramStateRef MallocMemReturnsAttr(CheckerContext &C,
                                               const CallExpr *CE,
                                               const OwnershipAttr* Att);
@@ -218,10 +219,13 @@ private:
   bool checkUseAfterFree(SymbolRef Sym, CheckerContext &C,
                          const Stmt *S = 0) const;
 
-  /// Check if the function is not known to us. So, for example, we could
-  /// conservatively assume it can free/reallocate it's pointer arguments.
-  bool doesNotFreeMemory(const CallEvent *Call,
-                         ProgramStateRef State) const;
+  /// Check if the function is known not to free memory, or if it is
+  /// "interesting" and should be modeled explicitly.
+  ///
+  /// We assume that pointers do not escape through calls to system functions
+  /// not handled by this checker.
+  bool doesNotFreeMemOrInteresting(const CallEvent *Call,
+                                   ProgramStateRef State) const;
 
   static bool SummarizeValue(raw_ostream &os, SVal V);
   static bool SummarizeRegion(raw_ostream &os, const MemRegion *MR);
@@ -495,14 +499,30 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
   C.addTransition(State);
 }
 
-static bool isFreeWhenDoneSetToZero(const ObjCMethodCall &Call) {
+static bool isKnownDeallocObjCMethodName(const ObjCMethodCall &Call) {
+  // If the first selector piece is one of the names below, assume that the
+  // object takes ownership of the memory, promising to eventually deallocate it
+  // with free().
+  // Ex:  [NSData dataWithBytesNoCopy:bytes length:10];
+  // (...unless a 'freeWhenDone' parameter is false, but that's checked later.)
+  StringRef FirstSlot = Call.getSelector().getNameForSlot(0);
+  if (FirstSlot == "dataWithBytesNoCopy" ||
+      FirstSlot == "initWithBytesNoCopy" ||
+      FirstSlot == "initWithCharactersNoCopy")
+    return true;
+
+  return false;
+}
+
+static Optional<bool> getFreeWhenDoneArg(const ObjCMethodCall &Call) {
   Selector S = Call.getSelector();
+
+  // FIXME: We should not rely on fully-constrained symbols being folded.
   for (unsigned i = 1; i < S.getNumArgs(); ++i)
     if (S.getNameForSlot(i).equals("freeWhenDone"))
-      if (Call.getArgSVal(i).isConstant(0))
-        return true;
+      return !Call.getArgSVal(i).isZeroConstant();
 
-  return false;
+  return None;
 }
 
 void MallocChecker::checkPostObjCMessage(const ObjCMethodCall &Call,
@@ -510,25 +530,20 @@ void MallocChecker::checkPostObjCMessage(const ObjCMethodCall &Call,
   if (C.wasInlined)
     return;
 
-  // If the first selector is dataWithBytesNoCopy, assume that the memory will
-  // be released with 'free' by the new object.
-  // Ex:  [NSData dataWithBytesNoCopy:bytes length:10];
-  // Unless 'freeWhenDone' param set to 0.
-  // TODO: Check that the memory was allocated with malloc.
-  bool ReleasedAllocatedMemory = false;
-  Selector S = Call.getSelector();
-  if ((S.getNameForSlot(0) == "dataWithBytesNoCopy" ||
-       S.getNameForSlot(0) == "initWithBytesNoCopy" ||
-       S.getNameForSlot(0) == "initWithCharactersNoCopy") &&
-      !isFreeWhenDoneSetToZero(Call)){
-    unsigned int argIdx  = 0;
-    ProgramStateRef State = FreeMemAux(C, Call.getArgExpr(argIdx),
-                                       Call.getOriginExpr(), C.getState(), true,
-                                       ReleasedAllocatedMemory,
-                                       /* RetNullOnFailure*/ true);
-
-    C.addTransition(State);
-  }
+  if (!isKnownDeallocObjCMethodName(Call))
+    return;
+
+  if (Optional<bool> FreeWhenDone = getFreeWhenDoneArg(Call))
+    if (!*FreeWhenDone)
+      return;
+
+  bool ReleasedAllocatedMemory;
+  ProgramStateRef State = FreeMemAux(C, Call.getArgExpr(0),
+                                     Call.getOriginExpr(), C.getState(),
+                                     /*Hold=*/true, ReleasedAllocatedMemory,
+                                     /*RetNullOnFailure=*/true);
+
+  C.addTransition(State);
 }
 
 ProgramStateRef MallocChecker::MallocMemReturnsAttr(CheckerContext &C,
@@ -1356,12 +1371,8 @@ ProgramStateRef MallocChecker::evalAssume(ProgramStateRef state,
   return state;
 }
 
-// Check if the function is known to us. So, for example, we could
-// conservatively assume it can free/reallocate its pointer arguments.
-// (We assume that the pointers cannot escape through calls to system
-// functions not handled by this checker.)
-bool MallocChecker::doesNotFreeMemory(const CallEvent *Call,
-                                      ProgramStateRef State) const {
+bool MallocChecker::doesNotFreeMemOrInteresting(const CallEvent *Call,
+                                                ProgramStateRef State) const {
   assert(Call);
 
   // For now, assume that any C++ call can free memory.
@@ -1378,24 +1389,23 @@ bool MallocChecker::doesNotFreeMemory(const CallEvent *Call,
     if (!Call->isInSystemHeader() || Call->hasNonZeroCallbackArg())
       return false;
 
-    Selector S = Msg->getSelector();
-
-    // Whitelist the ObjC methods which do free memory.
-    // - Anything containing 'freeWhenDone' param set to 1.
-    //   Ex: dataWithBytesNoCopy:length:freeWhenDone.
-    for (unsigned i = 1; i < S.getNumArgs(); ++i) {
-      if (S.getNameForSlot(i).equals("freeWhenDone")) {
-        if (Call->getArgSVal(i).isConstant(1))
-          return false;
-        else
-          return true;
-      }
-    }
+    // If it's a method we know about, handle it explicitly post-call.
+    // This should happen before the "freeWhenDone" check below.
+    if (isKnownDeallocObjCMethodName(*Msg))
+      return true;
 
-    // If the first selector ends with NoCopy, assume that the ownership is
-    // transferred as well.
-    // Ex:  [NSData dataWithBytesNoCopy:bytes length:10];
-    StringRef FirstSlot = S.getNameForSlot(0);
+    // If there's a "freeWhenDone" parameter, but the method isn't one we know
+    // about, we can't be sure that the object will use free() to deallocate the
+    // memory, so we can't model it explicitly. The best we can do is use it to
+    // decide whether the pointer escapes.
+    if (Optional<bool> FreeWhenDone = getFreeWhenDoneArg(*Msg))
+      return !*FreeWhenDone;
+
+    // If the first selector piece ends with "NoCopy", and there is no
+    // "freeWhenDone" parameter set to zero, we know ownership is being
+    // transferred. Again, though, we can't be sure that the object will use
+    // free() to deallocate the memory, so we can't model it explicitly.
+    StringRef FirstSlot = Msg->getSelector().getNameForSlot(0);
     if (FirstSlot.endswith("NoCopy"))
       return false;
 
@@ -1504,11 +1514,11 @@ ProgramStateRef MallocChecker::checkPointerEscape(ProgramStateRef State,
                                              const InvalidatedSymbols &Escaped,
                                              const CallEvent *Call,
                                              PointerEscapeKind Kind) const {
-  // If we know that the call does not free memory, keep tracking the top
-  // level arguments.       
+  // If we know that the call does not free memory, or we want to process the
+  // call later, keep tracking the top level arguments.
   if ((Kind == PSK_DirectEscapeOnCall ||
        Kind == PSK_IndirectEscapeOnCall) &&
-      doesNotFreeMemory(Call, State)) {
+      doesNotFreeMemOrInteresting(Call, State)) {
     return State;
   }