Catch free()s on non-regions and regions known to be not from malloc(), by checking...

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

diff --git a/lib/Checker/MallocChecker.cpp b/lib/Checker/MallocChecker.cpp
index 22b8b29f7850a947d82910230997997fb73187f4..30c4f172d0e9ff885efcbf8ce196717fcd78de38 100644 (file)
--- a/lib/Checker/MallocChecker.cpp
+++ b/lib/Checker/MallocChecker.cpp
@@ -59,11 +59,12 @@ class MallocChecker : public CheckerVisitor<MallocChecker> {
   BuiltinBug *BT_DoubleFree;
   BuiltinBug *BT_Leak;
   BuiltinBug *BT_UseFree;
+  BuiltinBug *BT_BadFree;
   IdentifierInfo *II_malloc, *II_free, *II_realloc, *II_calloc;
 
 public:
   MallocChecker() 
-    : BT_DoubleFree(0), BT_Leak(0), BT_UseFree(0), 
+    : BT_DoubleFree(0), BT_Leak(0), BT_UseFree(0), BT_BadFree(0),
       II_malloc(0), II_free(0), II_realloc(0), II_calloc(0) {}
   static void *getTag();
   bool EvalCallExpr(CheckerContext &C, const CallExpr *CE);
@@ -90,6 +91,10 @@ private:
 
   void ReallocMem(CheckerContext &C, const CallExpr *CE);
   void CallocMem(CheckerContext &C, const CallExpr *CE);
+  
+  bool SummarizeValue(llvm::raw_ostream& os, SVal V);
+  bool SummarizeRegion(llvm::raw_ostream& os, const MemRegion *MR);
+  void ReportBadFree(CheckerContext &C, SVal ArgVal, SourceRange range);
 };
 } // end anonymous namespace
 
@@ -191,18 +196,59 @@ void MallocChecker::FreeMem(CheckerContext &C, const CallExpr *CE) {
 
 const GRState *MallocChecker::FreeMemAux(CheckerContext &C, const CallExpr *CE,
                                          const GRState *state) {
-  SVal ArgVal = state->getSVal(CE->getArg(0));
+  const Expr *ArgExpr = CE->getArg(0);
+  SVal ArgVal = state->getSVal(ArgExpr);
 
   // If ptr is NULL, no operation is preformed.
   if (ArgVal.isZeroConstant())
     return state;
+  
+  // Unknown values could easily be okay
+  // Undefined values are handled elsewhere
+  if (ArgVal.isUnknownOrUndef())
+    return state;
 
-  SymbolRef Sym = ArgVal.getAsLocSymbol();
-
+  const MemRegion *R = ArgVal.getAsRegion();
+  
+  // Nonlocs can't be freed, of course.
+  // Non-region locations (labels and fixed addresses) also shouldn't be freed.
+  if (!R) {
+    ReportBadFree(C, ArgVal, ArgExpr->getSourceRange());
+    return NULL;
+  }
+  
+  R = R->StripCasts();
+  
+  // Blocks might show up as heap data, but should not be free()d
+  if (isa<BlockDataRegion>(R)) {
+    ReportBadFree(C, ArgVal, ArgExpr->getSourceRange());
+    return NULL;
+  }
+  
+  const MemSpaceRegion *MS = R->getMemorySpace();
+  
+  // Parameters, locals, statics, and globals shouldn't be freed.
+  if (!(isa<UnknownSpaceRegion>(MS) || isa<HeapSpaceRegion>(MS))) {
+    // FIXME: at the time this code was written, malloc() regions were
+    // represented by conjured symbols, which are all in UnknownSpaceRegion.
+    // This means that there isn't actually anything from HeapSpaceRegion
+    // that should be freed, even though we allow it here.
+    // Of course, free() can work on memory allocated outside the current
+    // function, so UnknownSpaceRegion is always a possibility.
+    // False negatives are better than false positives.
+    
+    ReportBadFree(C, ArgVal, ArgExpr->getSourceRange());
+    return NULL;
+  }
+  
+  const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(R);
   // Various cases could lead to non-symbol values here.
-  if (!Sym)
+  // For now, ignore them.
+  if (!SR)
     return state;
 
+  SymbolRef Sym = SR->getSymbol();
+  
   const RefState *RS = state->get<RegionState>(Sym);
 
   // If the symbol has not been tracked, return. This is possible when free() is
@@ -230,6 +276,135 @@ const GRState *MallocChecker::FreeMemAux(CheckerContext &C, const CallExpr *CE,
   return state->set<RegionState>(Sym, RefState::getReleased(CE));
 }
 
+bool MallocChecker::SummarizeValue(llvm::raw_ostream& os, SVal V) {
+  if (nonloc::ConcreteInt *IntVal = dyn_cast<nonloc::ConcreteInt>(&V))
+    os << "an integer (" << IntVal->getValue() << ")";
+  else if (loc::ConcreteInt *ConstAddr = dyn_cast<loc::ConcreteInt>(&V))
+    os << "a constant address (" << ConstAddr->getValue() << ")";
+  else if (loc::GotoLabel *Label = dyn_cast<loc::GotoLabel>(&V))
+    os << "the address of the label '"
+       << Label->getLabel()->getID()->getName()
+       << "'";
+  else
+    return false;
+  
+  return true;
+}
+
+bool MallocChecker::SummarizeRegion(llvm::raw_ostream& os,
+                                    const MemRegion *MR) {
+  switch (MR->getKind()) {
+  case MemRegion::FunctionTextRegionKind: {
+    const FunctionDecl *FD = cast<FunctionTextRegion>(MR)->getDecl();
+    if (FD)
+      os << "the address of the function '" << FD << "'";
+    else
+      os << "the address of a function";
+    return true;
+  }
+  case MemRegion::BlockTextRegionKind:
+    os << "block text";
+    return true;
+  case MemRegion::BlockDataRegionKind:
+    // FIXME: where the block came from?
+    os << "a block";
+    return true;
+  default: {
+    const MemSpaceRegion *MS = MR->getMemorySpace();
+    
+    switch (MS->getKind()) {
+    case MemRegion::StackLocalsSpaceRegionKind: {
+      const VarRegion *VR = dyn_cast<VarRegion>(MR);
+      const VarDecl *VD;
+      if (VR)
+        VD = VR->getDecl();
+      else
+        VD = NULL;
+      
+      if (VD)
+        os << "the address of the local variable '" << VD->getName() << "'";
+      else
+        os << "the address of a local stack variable";
+      return true;
+    }
+    case MemRegion::StackArgumentsSpaceRegionKind: {
+      const VarRegion *VR = dyn_cast<VarRegion>(MR);
+      const VarDecl *VD;
+      if (VR)
+        VD = VR->getDecl();
+      else
+        VD = NULL;
+      
+      if (VD)
+        os << "the address of the parameter '" << VD->getName() << "'";
+      else
+        os << "the address of a parameter";
+      return true;
+    }
+    case MemRegion::GlobalsSpaceRegionKind: {
+      const VarRegion *VR = dyn_cast<VarRegion>(MR);
+      const VarDecl *VD;
+      if (VR)
+        VD = VR->getDecl();
+      else
+        VD = NULL;
+      
+      if (VD) {
+        if (VD->isStaticLocal())
+          os << "the address of the static variable '" << VD->getName() << "'";
+        else
+          os << "the address of the global variable '" << VD->getName() << "'";
+      } else
+        os << "the address of a global variable";
+      return true;
+    }
+    default:
+      return false;
+    }
+  }
+  }
+}
+
+void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal,
+                                  SourceRange range) {
+  ExplodedNode *N = C.GenerateSink();
+  if (N) {
+    if (!BT_BadFree)
+      BT_BadFree = new BuiltinBug("Bad free");
+    
+    llvm::SmallString<100> buf;
+    llvm::raw_svector_ostream os(buf);
+    
+    const MemRegion *MR = ArgVal.getAsRegion();
+    if (MR) {
+      while (const ElementRegion *ER = dyn_cast<ElementRegion>(MR))
+        MR = ER->getSuperRegion();
+      
+      // Special case for alloca()
+      if (isa<AllocaRegion>(MR))
+        os << "Argument to free() was allocated by alloca(), not malloc()";
+      else {
+        os << "Argument to free() is ";
+        if (SummarizeRegion(os, MR))
+          os << ", which is not memory allocated by malloc()";
+        else
+          os << "not memory allocated by malloc()";
+      }
+    } else {
+      os << "Argument to free() is ";
+      if (SummarizeValue(os, ArgVal))
+        os << ", which is not memory allocated by malloc()";
+      else
+        os << "not memory allocated by malloc()";
+    }
+    
+    os.flush();
+    EnhancedBugReport *R = new EnhancedBugReport(*BT_BadFree, buf.str(), N);
+    R->addRange(range);
+    C.EmitReport(R);
+  }
+}
+
 void MallocChecker::ReallocMem(CheckerContext &C, const CallExpr *CE) {
   const GRState *state = C.getState();
   const Expr *Arg0 = CE->getArg(0);

diff --git a/test/Analysis/free.c b/test/Analysis/free.c
new file mode 100644 (file)
index 0000000..60bb3f2
--- /dev/null
+++ b/test/Analysis/free.c
@@ -0,0 +1,71 @@
+// RUN: %clang_cc1 -analyze -analyzer-experimental-internal-checks -analyzer-check-objc-mem -analyzer-store=region -analyzer-experimental-checks -fblocks -verify %s
+void free(void *);
+
+void t1 () {
+  int a[] = { 1 };
+  free(a); // expected-warning {{Argument to free() is the address of the local variable 'a', which is not memory allocated by malloc()}}
+}
+
+void t2 () {
+  int a = 1;
+  free(&a); // expected-warning {{Argument to free() is the address of the local variable 'a', which is not memory allocated by malloc()}}
+}
+
+void t3 () {
+  static int a[] = { 1 };
+  free(a); // expected-warning {{Argument to free() is the address of the static variable 'a', which is not memory allocated by malloc()}}
+}
+
+void t4 (char *x) {
+  free(x); // no-warning
+}
+
+void t5 () {
+  extern char *ptr();
+  free(ptr()); // no-warning
+}
+
+void t6 () {
+  free((void*)1000); // expected-warning {{Argument to free() is a constant address (1000), which is not memory allocated by malloc()}}
+}
+
+void t7 (char **x) {
+  free(*x); // no-warning
+}
+
+void t8 (char **x) {
+  // ugh
+  free((*x)+8); // no-warning
+}
+
+void t9 () {
+label:
+  free(&&label); // expected-warning {{Argument to free() is the address of the label 'label', which is not memory allocated by malloc()}}
+}
+
+void t10 () {
+  free((void*)&t10); // expected-warning {{Argument to free() is the address of the function 't10', which is not memory allocated by malloc()}}
+}
+
+void t11 () {
+  char *p = (char*)__builtin_alloca(2);
+  free(p); // expected-warning {{Argument to free() was allocated by alloca(), not malloc()}}
+}
+
+void t12 () {
+  free(^{return;}); // expected-warning {{Argument to free() is a block, which is not memory allocated by malloc()}}
+}
+
+void t13 (char a) {
+  free(&a); // expected-warning {{Argument to free() is the address of the parameter 'a', which is not memory allocated by malloc()}}
+}
+
+static int someGlobal[2];
+void t14 () {
+  free(someGlobal); // expected-warning {{Argument to free() is the address of the global variable 'someGlobal', which is not memory allocated by malloc()}}
+}
+
+void t15 (char **x, int offset) {
+  // Unknown value
+  free(x[offset]); // no-warning
+}