[clang] 2e5e42d - [analyzer][MallocChecker] When modeling realloc-like functions, don't early return if the argument is symbolic

[Kirstóf Umann via cfe-commits]

Author: Kirstóf Umann
Date: 2020-05-19T13:59:29+02:00
New Revision: 2e5e42d4aeab98636346db558e89ab9b122c9dc3

URL: https://github.com/llvm/llvm-project/commit/2e5e42d4aeab98636346db558e89ab9b122c9dc3
DIFF: https://github.com/llvm/llvm-project/commit/2e5e42d4aeab98636346db558e89ab9b122c9dc3.diff

LOG: [analyzer][MallocChecker] When modeling realloc-like functions, don't early return if the argument is symbolic

The very essence of MallocChecker lies in 2 overload sets: the FreeMemAux
functions and the MallocMemAux functions. The former houses most of the error
checking as well (aside from leaks), such as incorrect deallocation. There, we
check whether the argument's MemSpaceRegion is the heap or unknown, and if it
isn't, we know we encountered a bug (aside from a corner case patched by
@balazske in D76830), as specified by MEM34-C.

In ReallocMemAux, which really is the combination of  FreeMemAux and
MallocMemAux, we incorrectly early returned if the memory argument of realloc is
non-symbolic. The problem is, one of the cases where this happens when we know
precisely what the region is, like an array, as demonstrated in the test file.
So, lets get rid of this false negative :^)

Side note, I dislike the warning message and the associated checker name, but
I'll address it in a later patch.

Differential Revision: https://reviews.llvm.org/D79415

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
    clang/test/Analysis/malloc.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 2bff317873c1..d108f88776d4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2371,13 +2371,7 @@ MallocChecker::ReallocMemAux(CheckerContext &C, const CallExpr *CE,
   if (PrtIsNull && SizeIsZero)
     return State;
 
-  // Get the from and to pointer symbols as in toPtr = realloc(fromPtr, size).
   assert(!PrtIsNull);
-  SymbolRef FromPtr = arg0Val.getAsSymbol();
-  SVal RetVal = C.getSVal(CE);
-  SymbolRef ToPtr = RetVal.getAsSymbol();
-  if (!FromPtr || !ToPtr)
-    return nullptr;
 
   bool IsKnownToBeAllocated = false;
 
@@ -2406,6 +2400,14 @@ MallocChecker::ReallocMemAux(CheckerContext &C, const CallExpr *CE,
     else if (!IsKnownToBeAllocated)
       Kind = OAR_DoNotTrackAfterFailure;
 
+    // Get the from and to pointer symbols as in toPtr = realloc(fromPtr, size).
+    SymbolRef FromPtr = arg0Val.getAsSymbol();
+    SVal RetVal = C.getSVal(CE);
+    SymbolRef ToPtr = RetVal.getAsSymbol();
+    assert(FromPtr && ToPtr &&
+           "By this point, FreeMemAux and MallocMemAux should have checked "
+           "whether the argument or the return value is symbolic!");
+
     // Record the info about the reallocated symbol so that we could properly
     // process failed reallocation.
     stateRealloc = stateRealloc->set<ReallocPairs>(ToPtr,

diff  --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c
index 5288e21a2821..b7a29db274b4 100644
--- a/clang/test/Analysis/malloc.c
+++ b/clang/test/Analysis/malloc.c
@@ -1828,6 +1828,21 @@ void testCStyleListItems(struct ListInfo *list) {
   list_add(list, &x->li); // will free 'x'.
 }
 
+// MEM34-C. Only free memory allocated dynamically
+// Second non-compliant example.
+// https://wiki.sei.cmu.edu/confluence/display/c/MEM34-C.+Only+free+memory+allocated+dynamically
+enum { BUFSIZE = 256 };
+
+void MEM34_C(void) {
+  char buf[BUFSIZE];
+  char *p = (char *)realloc(buf, 2 * BUFSIZE);
+  // expected-warning at -1{{Argument to realloc() is the address of the local \
+variable 'buf', which is not memory allocated by malloc() [unix.Malloc]}}
+  if (p == NULL) {
+    /* Handle error */
+  }
+}
+
 // ----------------------------------------------------------------------------
 // False negatives.