[analyzer] This implements potential undefbehavior.ZeroAllocDereference checker.

TODO: support realloc(). Currently it is not possible due to the present realloc() handling. Currently RegionState is not being attached to realloc() in case of a zero Size argument.
llvm-svn: 234889

diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 70e1768..0cf0094 100644 (file)
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -50,6 +50,8 @@ enum AllocationFamily {
 class RefState {
   enum Kind { // Reference to allocated memory.
               Allocated,
+              // Reference to zero-allocated memory.
+              AllocatedOfSizeZero,
               // Reference to released/freed memory.
               Released,
               // The responsibility for freeing resources has transferred from
@@ -62,8 +64,8 @@ class RefState {
   };
 
   const Stmt *S;
-  unsigned K : 2; // Kind enum, but stored as a bitfield.
-  unsigned Family : 30; // Rest of 32-bit word, currently just an allocation 
+  unsigned K : 3; // Kind enum, but stored as a bitfield.
+  unsigned Family : 29; // Rest of 32-bit word, currently just an allocation 
                         // family.
 
   RefState(Kind k, const Stmt *s, unsigned family) 
@@ -72,6 +74,7 @@ class RefState {
   }
 public:
   bool isAllocated() const { return K == Allocated; }
+  bool isAllocatedOfSizeZero() const { return K == AllocatedOfSizeZero; }
   bool isReleased() const { return K == Released; }
   bool isRelinquished() const { return K == Relinquished; }
   bool isEscaped() const { return K == Escaped; }
@@ -87,6 +90,10 @@ public:
   static RefState getAllocated(unsigned family, const Stmt *s) {
     return RefState(Allocated, s, family);
   }
+  static RefState getAllocatedOfSizeZero(const RefState *RS) {
+    return RefState(AllocatedOfSizeZero, RS->getStmt(),
+                    RS->getAllocationFamily());
+  }
   static RefState getReleased(unsigned family, const Stmt *s) { 
     return RefState(Released, s, family);
   }
@@ -107,6 +114,7 @@ public:
     switch (static_cast<Kind>(K)) {
 #define CASE(ID) case ID: OS << #ID; break;
     CASE(Allocated)
+    CASE(AllocatedOfSizeZero)
     CASE(Released)
     CASE(Relinquished)
     CASE(Escaped)
@@ -222,6 +230,7 @@ private:
   mutable std::unique_ptr<BugType> BT_FreeAlloca[CK_NumCheckKinds];
   mutable std::unique_ptr<BugType> BT_MismatchedDealloc;
   mutable std::unique_ptr<BugType> BT_OffsetFree[CK_NumCheckKinds];
+  mutable std::unique_ptr<BugType> BT_UseZerroAllocated[CK_NumCheckKinds];
   mutable IdentifierInfo *II_alloca, *II_malloc, *II_free, *II_realloc,
                          *II_calloc, *II_valloc, *II_reallocf, *II_strndup,
                          *II_strdup, *II_kmalloc, *II_if_nameindex,
@@ -257,6 +266,12 @@ private:
                       MemoryOperationKind MemKind) const;
   bool isStandardNewDelete(const FunctionDecl *FD, ASTContext &C) const;
   ///@}
+
+  /// \brief Perform a zero-allocation check.
+  ProgramStateRef ProcessZeroAllocation(CheckerContext &C, const Expr *E,
+                                        const unsigned AllocationSizeArg,
+                                        ProgramStateRef State) const;
+
   ProgramStateRef MallocMemReturnsAttr(CheckerContext &C,
                                        const CallExpr *CE,
                                        const OwnershipAttr* Att,
@@ -307,6 +322,9 @@ private:
 
   bool checkUseAfterFree(SymbolRef Sym, CheckerContext &C, const Stmt *S) const;
 
+  void checkUseZeroAllocated(SymbolRef Sym, CheckerContext &C, 
+                             const Stmt *S) const;
+
   bool checkDoubleDelete(SymbolRef Sym, CheckerContext &C) const;
 
   /// Check if the function is known free memory, or if it is
@@ -361,6 +379,9 @@ private:
 
   void ReportDoubleDelete(CheckerContext &C, SymbolRef Sym) const;
 
+  void ReportUseZeroAllocated(CheckerContext &C, SourceRange Range,
+                              SymbolRef Sym) const;
+
   /// Find the location of the allocation for Sym on the path leading to the
   /// exploded node N.
   LeakInfo getAllocationSite(const ExplodedNode *N, SymbolRef Sym,
@@ -405,7 +426,9 @@ private:
                             const Stmt *Stmt) {
       // Did not track -> allocated. Other state (released) -> allocated.
       return (Stmt && (isa<CallExpr>(Stmt) || isa<CXXNewExpr>(Stmt)) &&
-              (S && S->isAllocated()) && (!SPrev || !SPrev->isAllocated()));
+              (S && (S->isAllocated() || S->isAllocatedOfSizeZero())) && 
+              (!SPrev || !(SPrev->isAllocated() || 
+                           SPrev->isAllocatedOfSizeZero())));
     }
 
     inline bool isReleased(const RefState *S, const RefState *SPrev,
@@ -431,7 +454,9 @@ private:
       // check. If we have to handle more cases here, it might be cleaner just
       // to track this extra bit in the state itself.
       return ((!Stmt || !isa<CallExpr>(Stmt)) &&
-              (S && S->isAllocated()) && (SPrev && !SPrev->isAllocated()));
+              (S && (S->isAllocated() || S->isAllocatedOfSizeZero())) &&
+              (SPrev && !(SPrev->isAllocated() ||
+                          SPrev->isAllocatedOfSizeZero())));
     }
 
     PathDiagnosticPiece *VisitNode(const ExplodedNode *N,
@@ -730,6 +755,8 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
         return;
       if (CE->getNumArgs() < 3) {
         State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
+        if (CE->getNumArgs() == 1)
+          State = ProcessZeroAllocation(C, CE, 0, State);
       } else if (CE->getNumArgs() == 3) {
         llvm::Optional<ProgramStateRef> MaybeState =
           performKernelMalloc(CE, C, State);
@@ -749,12 +776,17 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
       if (CE->getNumArgs() < 1)
         return;
       State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
+      State = ProcessZeroAllocation(C, CE, 0, State);
     } else if (FunI == II_realloc) {
       State = ReallocMem(C, CE, false, State);
+      State = ProcessZeroAllocation(C, CE, 1, State);
     } else if (FunI == II_reallocf) {
       State = ReallocMem(C, CE, true, State);
+      State = ProcessZeroAllocation(C, CE, 1, State);
     } else if (FunI == II_calloc) {
       State = CallocMem(C, CE, State);
+      State = ProcessZeroAllocation(C, CE, 0, State);
+      State = ProcessZeroAllocation(C, CE, 1, State);
     } else if (FunI == II_free) {
       State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);
     } else if (FunI == II_strdup) {
@@ -764,18 +796,23 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
     } else if (FunI == II_alloca) {
       State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State,
                            AF_Alloca);
+      State = ProcessZeroAllocation(C, CE, 0, State);
     } else if (isStandardNewDelete(FD, C.getASTContext())) {
       // Process direct calls to operator new/new[]/delete/delete[] functions
       // as distinct from new/new[]/delete/delete[] expressions that are 
       // processed by the checkPostStmt callbacks for CXXNewExpr and 
       // CXXDeleteExpr.
       OverloadedOperatorKind K = FD->getOverloadedOperator();
-      if (K == OO_New)
+      if (K == OO_New) {
         State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State,
                              AF_CXXNew);
-      else if (K == OO_Array_New)
+        State = ProcessZeroAllocation(C, CE, 0, State);
+      }
+      else if (K == OO_Array_New) {
         State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State,
                              AF_CXXNewArray);
+        State = ProcessZeroAllocation(C, CE, 0, State);
+      }
       else if (K == OO_Delete || K == OO_Array_Delete)
         State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);
       else
@@ -809,6 +846,68 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
   C.addTransition(State);
 }
 
+// Performs a 0-sized allocations check.
+ProgramStateRef MallocChecker::ProcessZeroAllocation(CheckerContext &C,
+                                               const Expr *E,
+                                               const unsigned AllocationSizeArg,
+                                               ProgramStateRef State) const {
+  if (!State)
+    return nullptr;
+
+  const Expr *Arg = nullptr;
+
+  if (const CallExpr *CE = dyn_cast<CallExpr>(E)) {
+    Arg = CE->getArg(AllocationSizeArg);
+  }
+  else if (const CXXNewExpr *NE = dyn_cast<CXXNewExpr>(E)) {
+    if (NE->isArray())
+      Arg = NE->getArraySize();
+    else
+      return State;
+  }
+  else
+    llvm_unreachable("not a CallExpr or CXXNewExpr");
+
+  assert(Arg);
+
+  Optional<DefinedSVal> DefArgVal = 
+      State->getSVal(Arg, C.getLocationContext()).getAs<DefinedSVal>();
+
+  if (!DefArgVal)
+    return State;
+
+  // Check if the allocation size is 0.
+  ProgramStateRef TrueState, FalseState;
+  SValBuilder &SvalBuilder = C.getSValBuilder();
+  DefinedSVal Zero =
+      SvalBuilder.makeZeroVal(Arg->getType()).castAs<DefinedSVal>();
+
+  std::tie(TrueState, FalseState) = 
+      State->assume(SvalBuilder.evalEQ(State, *DefArgVal, Zero));
+
+  if (TrueState && !FalseState) {
+    SVal retVal = State->getSVal(E, C.getLocationContext());
+    SymbolRef Sym = retVal.getAsLocSymbol();
+    if (!Sym)
+      return State;
+
+    const RefState *RS = State->get<RegionState>(Sym);
+    if (!RS)
+      return State; // TODO: change to assert(RS); after realloc() will 
+                    // guarantee have a RegionState attached.
+
+    if (!RS->isAllocated())
+      return State;
+
+    return TrueState->set<RegionState>(Sym,
+                                       RefState::getAllocatedOfSizeZero(RS));
+  }
+
+  // Assume the value is non-zero going forward.
+  assert(FalseState);
+  return FalseState;
+}
+
 static QualType getDeepPointeeType(QualType T) {
   QualType Result = T, PointeeType = T->getPointeeType();
   while (!PointeeType.isNull()) {
@@ -868,6 +967,7 @@ void MallocChecker::checkPostStmt(const CXXNewExpr *NE,
   // existing binding.
   State = MallocUpdateRefState(C, NE, State, NE->isArray() ? AF_CXXNewArray 
                                                            : AF_CXXNew);
+  State = ProcessZeroAllocation(C, NE, 0, State);
   C.addTransition(State);
 }
 
@@ -1291,7 +1391,8 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
 
     // If the pointer is allocated or escaped, but we are now trying to free it,
     // check that the call to free is proper.
-    } else if (RsBase->isAllocated() || RsBase->isEscaped()) {
+    } else if (RsBase->isAllocated() || RsBase->isAllocatedOfSizeZero() || 
+               RsBase->isEscaped()) {
 
       // Check if an expected deallocation function matches the real one.
       bool DeallocMatchesAlloc =
@@ -1316,7 +1417,8 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
     }
   }
 
-  ReleasedAllocated = (RsBase != nullptr) && RsBase->isAllocated();
+  ReleasedAllocated = (RsBase != nullptr) && (RsBase->isAllocated() || 
+                                              RsBase->isAllocatedOfSizeZero());
 
   // Clean out the info on previous call to free return info.
   State = State->remove<FreeReturnValue>(SymBase);
@@ -1741,6 +1843,36 @@ void MallocChecker::ReportDoubleDelete(CheckerContext &C, SymbolRef Sym) const {
   }
 }
 
+void MallocChecker::ReportUseZeroAllocated(CheckerContext &C,
+                                           SourceRange Range,
+                                           SymbolRef Sym) const {
+
+  if (!ChecksEnabled[CK_MallocChecker] &&
+      !ChecksEnabled[CK_NewDeleteChecker])
+    return;
+
+  Optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(C, Sym);
+
+  if (!CheckKind.hasValue())
+    return;
+
+  if (ExplodedNode *N = C.generateSink()) {
+    if (!BT_UseZerroAllocated[*CheckKind])
+      BT_UseZerroAllocated[*CheckKind].reset(new BugType(
+          CheckNames[*CheckKind], "Use of zero allocated", "Memory Error"));
+
+    BugReport *R = new BugReport(*BT_UseZerroAllocated[*CheckKind],
+                                 "Use of zero-allocated memory", N);
+
+    R->addRange(Range);
+    if (Sym) {
+      R->markInteresting(Sym);
+      R->addVisitor(llvm::make_unique<MallocBugVisitor>(Sym));
+    }
+    C.emitReport(R);
+  }
+}
+
 ProgramStateRef MallocChecker::ReallocMem(CheckerContext &C,
                                           const CallExpr *CE,
                                           bool FreesOnFail,
@@ -1988,7 +2120,7 @@ void MallocChecker::checkDeadSymbols(SymbolReaper &SymReaper,
   SmallVector<SymbolRef, 2> Errors;
   for (RegionStateTy::iterator I = RS.begin(), E = RS.end(); I != E; ++I) {
     if (SymReaper.isDead(I->first)) {
-      if (I->second.isAllocated())
+      if (I->second.isAllocated() || I->second.isAllocatedOfSizeZero())
         Errors.push_back(I->first);
       // Remove the dead symbol from the map.
       RS = F.remove(RS, I->first);
@@ -2156,6 +2288,15 @@ bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext &C,
   return false;
 }
 
+void MallocChecker::checkUseZeroAllocated(SymbolRef Sym, CheckerContext &C,
+                                          const Stmt *S) const {
+  assert(Sym);
+  const RefState *RS = C.getState()->get<RegionState>(Sym);
+
+  if (RS && RS->isAllocatedOfSizeZero())
+    ReportUseZeroAllocated(C, RS->getStmt()->getSourceRange(), Sym);
+}
+
 bool MallocChecker::checkDoubleDelete(SymbolRef Sym, CheckerContext &C) const {
 
   if (isReleased(Sym, C)) {
@@ -2169,8 +2310,10 @@ bool MallocChecker::checkDoubleDelete(SymbolRef Sym, CheckerContext &C) const {
 void MallocChecker::checkLocation(SVal l, bool isLoad, const Stmt *S,
                                   CheckerContext &C) const {
   SymbolRef Sym = l.getLocSymbolInBase();
-  if (Sym)
+  if (Sym) {
     checkUseAfterFree(Sym, C, S);
+    checkUseZeroAllocated(Sym, C, S);
+  }
 }
 
 // If a symbolic region is assumed to NULL (or another constant), stop tracking
@@ -2414,7 +2557,8 @@ ProgramStateRef MallocChecker::checkPointerEscapeAux(ProgramStateRef State,
       continue;
     
     if (const RefState *RS = State->get<RegionState>(sym)) {
-      if (RS->isAllocated() && CheckRefState(RS)) {
+      if ((RS->isAllocated() || RS->isAllocatedOfSizeZero()) &&
+          CheckRefState(RS)) {
         State = State->remove<RegionState>(sym);
         State = State->set<RegionState>(sym, RefState::getEscaped(RS));
       }

diff --git a/clang/test/Analysis/Malloc+MismatchedDeallocator_intersections.cpp b/clang/test/Analysis/Malloc+MismatchedDeallocator_intersections.cpp
index 639790d..bca223b 100644 (file)
--- a/clang/test/Analysis/Malloc+MismatchedDeallocator_intersections.cpp
+++ b/clang/test/Analysis/Malloc+MismatchedDeallocator_intersections.cpp
@@ -24,5 +24,16 @@ void testNewDeleteNoWarn() {
 
   int *p4 = new int;
   delete p4;
-  int j = *p4; // no-warning  
+  int j = *p4; // no-warning
+}
+
+void testUseZeroAllocNoWarn() {
+  int *p1 = (int *)operator new(0);
+  *p1 = 1; // no-warning
+
+  int *p2 = (int *)operator new[](0);
+  p2[0] = 1; // no-warning
+
+  int *p3 = new int[0];
+  p3[0] = 1; // no-warning
 }

diff --git a/clang/test/Analysis/NewDelete-checker-test.cpp b/clang/test/Analysis/NewDelete-checker-test.cpp
index 3f28c2e..443cb2e 100644 (file)
--- a/clang/test/Analysis/NewDelete-checker-test.cpp
+++ b/clang/test/Analysis/NewDelete-checker-test.cpp
@@ -87,6 +87,30 @@ void testNewInvalidationPlacement(PtrWrapper *w) {
   new (w) PtrWrapper(new int); // no warn
 }
 
+//-----------------------------------------
+// check for usage of zero-allocated memory
+//-----------------------------------------
+
+void testUseZeroAlloc1() {
+  int *p = (int *)operator new(0);
+  *p = 1; // expected-warning {{Use of zero-allocated memory}}
+  delete p;
+}
+
+int testUseZeroAlloc2() {
+  int *p = (int *)operator new[](0);
+  return p[0]; // expected-warning {{Use of zero-allocated memory}}
+  delete[] p;
+}
+
+void f(int);
+
+void testUseZeroAlloc3() {
+  int *p = new int[0];
+  f(*p); // expected-warning {{Use of zero-allocated memory}}
+  delete[] p;
+}
+
 //---------------
 // other checks
 //---------------

diff --git a/clang/test/Analysis/NewDelete-intersections.mm b/clang/test/Analysis/NewDelete-intersections.mm
index 7b3fb6e..cde8122 100644 (file)
--- a/clang/test/Analysis/NewDelete-intersections.mm
+++ b/clang/test/Analysis/NewDelete-intersections.mm
@@ -43,6 +43,11 @@ void testDeleteMalloced() {
   delete p2; // no warn
 } 
 
+void testUseZeroAllocatedMalloced() {
+  int *p1 = (int *)malloc(0);
+  *p1 = 1; // no warn
+}
+
 //----- Test free standard new
 void testFreeOpNew() {
   void *p = operator new(0);

diff --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c
index 5762061..662df4c 100644 (file)
--- a/clang/test/Analysis/malloc.c
+++ b/clang/test/Analysis/malloc.c
@@ -200,6 +200,112 @@ void reallocfPtrZero1() {
   char *r = reallocf(0, 12);
 } // expected-warning {{Potential leak of memory pointed to by}}
 
+//------------------- Check usage of zero-allocated memory ---------------------
+void CheckUseZeroAllocatedNoWarn1() {
+  int *p = malloc(0);
+  free(p); // no warning
+}
+
+void CheckUseZeroAllocatedNoWarn2() {
+  int *p = alloca(0); // no warning
+}
+
+void CheckUseZeroAllocatedNoWarn3() {
+  int *p = malloc(0);
+  int *q = realloc(p, 8); // no warning
+  free(q);
+}
+
+void CheckUseZeroAllocatedNoWarn4() {
+  int *p = realloc(0, 8);
+  *p = 1; // no warning
+  free(p);
+}
+
+void CheckUseZeroAllocated1() {
+  int *p = malloc(0);
+  *p = 1; // expected-warning {{Use of zero-allocated memory}}
+  free(p);
+}
+
+char CheckUseZeroAllocated2() {
+  char *p = alloca(0);
+  return *p; // expected-warning {{Use of zero-allocated memory}}
+}
+
+void UseZeroAllocated(int *p) {
+  if (p)
+    *p = 7; // expected-warning {{Use of zero-allocated memory}}
+}
+void CheckUseZeroAllocated3() {
+  int *p = malloc(0);
+  UseZeroAllocated(p);
+}
+
+void f(char);
+void CheckUseZeroAllocated4() {
+  char *p = valloc(0);
+  f(*p); // expected-warning {{Use of zero-allocated memory}}
+  free(p);
+}
+
+void CheckUseZeroAllocated5() {
+  int *p = calloc(0, 2);
+  *p = 1; // expected-warning {{Use of zero-allocated memory}}
+  free(p);
+}
+
+void CheckUseZeroAllocated6() {
+  int *p = calloc(2, 0);
+  *p = 1; // expected-warning {{Use of zero-allocated memory}}
+  free(p);
+}
+
+void CheckUseZeroAllocated7() {
+  int *p = realloc(0, 0);
+  *p = 1; //TODO: warn about use of zero-allocated memory
+  free(p);
+}
+
+void CheckUseZeroAllocated8() {
+  int *p = malloc(8);
+  int *q = realloc(p, 0);
+  *q = 1; //TODO: warn about use of zero-allocated memory
+  free(q);
+}
+
+void CheckUseZeroAllocated9() {
+  int *p = realloc(0, 0);
+  int *q = realloc(p, 0);
+  *q = 1; //TODO: warn about use of zero-allocated memory
+  free(q);
+}
+
+void CheckUseZeroAllocatedPathNoWarn(_Bool b) {
+  int s = 0;
+  if (b)
+    s= 10;
+
+  char *p = malloc(s);
+
+  if (b)
+    *p = 1; // no warning
+
+  free(p);
+}
+
+void CheckUseZeroAllocatedPathWarn(_Bool b) {
+  int s = 10;
+  if (b)
+    s= 0;
+
+  char *p = malloc(s);
+
+  if (b)
+    *p = 1; // expected-warning {{Use of zero-allocated memory}}
+
+  free(p);
+}
 
 // This case tests that storing malloc'ed memory to a static variable which is
 // then returned is not leaked.  In the absence of known contracts for functions