[analyzer] Make realloc(ptr, 0) handling equivalent to malloc(0).

Currently realloc(ptr, 0) is treated as free() which seems to be not correct. C
standard (N1570) establishes equivalent behavior for malloc(0) and realloc(ptr,
0): "7.22.3 Memory management functions calloc, malloc, realloc: If the size of
the space requested is zero, the behavior is implementation-defined: either a
null pointer is  returned, or the behavior is as if the size were some nonzero
value, except that the returned pointer shall not be used to access an object."
The patch equalizes the processing of malloc(0) and realloc(ptr,0). The patch
also enables unix.Malloc checker to detect references to zero-allocated memory
returned by realloc(ptr,0) ("Use of zero-allocated memory" warning).

A patch by Антон Ярцев!

Differential Revision: http://reviews.llvm.org/D9040

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

lib/StaticAnalyzer/Checkers/MallocChecker.cpp

@@ -508,6 +508,7 @@ private:
 
 REGISTER_MAP_WITH_PROGRAMSTATE(RegionState, SymbolRef, RefState)
 REGISTER_MAP_WITH_PROGRAMSTATE(ReallocPairs, SymbolRef, ReallocPair)
+REGISTER_SET_WITH_PROGRAMSTATE(ReallocSizeZeroSymbols, SymbolRef)
 
 // A map from the freed symbol to the symbol representing the return value of
 // the free function.
@@ -891,15 +892,19 @@ ProgramStateRef MallocChecker::ProcessZeroAllocation(CheckerContext &C,
       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));
+    if (RS) {
+      if (RS->isAllocated())
+        return TrueState->set<RegionState>(Sym,
+                                          RefState::getAllocatedOfSizeZero(RS));
+      else
+        return State;
+    } else {
+      // Case of zero-size realloc. Historically 'realloc(ptr, 0)' is treated as
+      // 'free(ptr)' and the returned value from 'realloc(ptr, 0)' is not
+      // tracked. Add zero-reallocated Sym to the state to catch references
+      // to zero-allocated memory.
+      return TrueState->add<ReallocSizeZeroSymbols>(Sym);
+    }
   }
 
   // Assume the value is non-zero going forward.
@@ -1487,6 +1492,9 @@ MallocChecker::getCheckIfTracked(CheckerContext &C,
 Optional<MallocChecker::CheckKind>
 MallocChecker::getCheckIfTracked(CheckerContext &C, SymbolRef Sym,
                                  bool IsALeakCheck) const {
+  if (C.getState()->contains<ReallocSizeZeroSymbols>(Sym))
+    return CK_MallocChecker;
+
   const RefState *RS = C.getState()->get<RegionState>(Sym);
   assert(RS);
   return getCheckIfTracked(RS->getAllocationFamily(), IsALeakCheck);
@@ -1929,7 +1937,7 @@ ProgramStateRef MallocChecker::ReallocMem(CheckerContext &C,
   }
 
   if (PrtIsNull && SizeIsZero)
-    return nullptr;
+    return State;
 
   // Get the from and to pointer symbols as in toPtr = realloc(fromPtr, size).
   assert(!PrtIsNull);
@@ -2293,10 +2301,14 @@ bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext &C,
 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);
+  if (const RefState *RS = C.getState()->get<RegionState>(Sym)) {
+    if (RS->isAllocatedOfSizeZero())
+      ReportUseZeroAllocated(C, RS->getStmt()->getSourceRange(), Sym);
+  }
+  else if (C.getState()->contains<ReallocSizeZeroSymbols>(Sym)) {
+    ReportUseZeroAllocated(C, S->getSourceRange(), Sym);
+  }
 }
 
 bool MallocChecker::checkDoubleDelete(SymbolRef Sym, CheckerContext &C) const {

test/Analysis/malloc.c

@@ -263,21 +263,21 @@ void CheckUseZeroAllocated6() {
 
 void CheckUseZeroAllocated7() {
   int *p = realloc(0, 0);
-  *p = 1; //TODO: warn about use of zero-allocated memory
+  *p = 1; // expected-warning {{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
+  *q = 1; // expected-warning {{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
+  *q = 1; // expected-warning {{Use of zero-allocated memory}}
   free(q);
 }
 
@@ -307,6 +307,34 @@ void CheckUseZeroAllocatedPathWarn(_Bool b) {
   free(p);
 }
 
+void CheckUseZeroReallocatedPathNoWarn(_Bool b) {
+  int s = 0;
+  if (b)
+    s= 10;
+
+  char *p = malloc(8);
+  char *q = realloc(p, s);
+
+  if (b)
+    *q = 1; // no warning
+
+  free(q);
+}
+
+void CheckUseZeroReallocatedPathWarn(_Bool b) {
+  int s = 10;
+  if (b)
+    s= 0;
+
+  char *p = malloc(8);
+  char *q = realloc(p, s);
+
+  if (b)
+    *q = 1; // expected-warning {{Use of zero-allocated memory}}
+
+  free(q);
+}
+
 // 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
 // or inter-procedural analysis, this is a conservative answer.