[analyzer] MallocChecker Cleanup - harden against crashes, fix an error

(use of return instead of continue), wording.

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

3 files changed, 43 insertions, 20 deletions

diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 08c0b1cde1..8b6964bff7 100644
--- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -255,7 +255,7 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
       if (A->getType().getTypePtr()->isAnyPointerType()) {
         SymbolRef Sym = State->getSVal(A, C.getLocationContext()).getAsSymbol();
         if (!Sym)
-          return;
+          continue;
         checkEscape(Sym, A, C);
         checkUseAfterFree(Sym, C, A);
       }
@@ -299,7 +299,11 @@ ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C,
   state = state->bindDefault(retVal, Init);
 
   // Set the region's extent equal to the Size parameter.
-  const SymbolicRegion *R = cast<SymbolicRegion>(retVal.getAsRegion());
+  const SymbolicRegion *R =
+      dyn_cast_or_null<SymbolicRegion>(retVal.getAsRegion());
+  if (!R || !isa<DefinedOrUnknownSVal>(Size))
+    return 0;
+
   DefinedOrUnknownSVal Extent = R->getExtent(svalBuilder);
   DefinedOrUnknownSVal DefinedSize = cast<DefinedOrUnknownSVal>(Size);
   DefinedOrUnknownSVal extentMatchesSize =
@@ -338,13 +342,14 @@ void MallocChecker::FreeMemAttr(CheckerContext &C, const CallExpr *CE,
 }
 
 ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
-                                              const CallExpr *CE,
-                                              ProgramStateRef state,
-                                              unsigned Num,
-                                              bool Hold) const {
+                                          const CallExpr *CE,
+                                          ProgramStateRef state,
+                                          unsigned Num,
+                                          bool Hold) const {
   const Expr *ArgExpr = CE->getArg(Num);
   SVal ArgVal = state->getSVal(ArgExpr, C.getLocationContext());
-
+  if (!isa<DefinedOrUnknownSVal>(ArgVal))
+    return 0;
   DefinedOrUnknownSVal location = cast<DefinedOrUnknownSVal>(ArgVal);
 
   // Check for null dereferences.
@@ -565,8 +570,10 @@ void MallocChecker::ReallocMem(CheckerContext &C, const CallExpr *CE) const {
   ProgramStateRef state = C.getState();
   const Expr *arg0Expr = CE->getArg(0);
   const LocationContext *LCtx = C.getLocationContext();
-  DefinedOrUnknownSVal arg0Val 
-    = cast<DefinedOrUnknownSVal>(state->getSVal(arg0Expr, LCtx));
+  SVal Arg0Val = state->getSVal(arg0Expr, LCtx);
+  if (!isa<DefinedOrUnknownSVal>(Arg0Val))
+    return;
+  DefinedOrUnknownSVal arg0Val = cast<DefinedOrUnknownSVal>(Arg0Val);
 
   SValBuilder &svalBuilder = C.getSValBuilder();
 
@@ -579,8 +586,10 @@ void MallocChecker::ReallocMem(CheckerContext &C, const CallExpr *CE) const {
     return;
 
   // Get the value of the size argument.
-  DefinedOrUnknownSVal Arg1Val = 
-    cast<DefinedOrUnknownSVal>(state->getSVal(Arg1, LCtx));
+  SVal Arg1ValG = state->getSVal(Arg1, LCtx);
+  if (!isa<DefinedOrUnknownSVal>(Arg1ValG))
+    return;
+  DefinedOrUnknownSVal Arg1Val = cast<DefinedOrUnknownSVal>(Arg1ValG);
 
   // Compare the size argument to 0.
   DefinedOrUnknownSVal SizeZero =
@@ -749,7 +758,7 @@ bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext &C,
   if (RS && RS->isReleased()) {
     if (ExplodedNode *N = C.addTransition()) {
       if (!BT_UseFree)
-        BT_UseFree.reset(new BuiltinBug("Use dynamically allocated memory "
+        BT_UseFree.reset(new BuiltinBug("Use of dynamically allocated memory "
             "after it is freed."));
 
       BugReport *R = new BugReport(*BT_UseFree, BT_UseFree->getDescription(),N);
@@ -779,6 +788,8 @@ void MallocChecker::checkBind(SVal location, SVal val,
   // structure does not transfer ownership.
 
   ProgramStateRef state = C.getState();
+  if (!isa<DefinedOrUnknownSVal>(location))
+    return;
   DefinedOrUnknownSVal l = cast<DefinedOrUnknownSVal>(location);
 
   // Check for null dereferences.
diff --git a/test/Analysis/malloc-annotations.c b/test/Analysis/malloc-annotations.c
index 0d7e402c8a..7890cfc176 100644
--- a/test/Analysis/malloc-annotations.c
+++ b/test/Analysis/malloc-annotations.c
@@ -199,13 +199,13 @@ void pr6293() {
 void f7() {
   char *x = (char*) malloc(4);
   free(x);
-  x[0] = 'a'; // expected-warning{{Use dynamically allocated memory after it is freed.}}
+  x[0] = 'a'; // expected-warning{{Use of dynamically allocated memory after it is freed.}}
 }
 
 void f7_realloc() {
   char *x = (char*) malloc(4);
   realloc(x,0);
-  x[0] = 'a'; // expected-warning{{Use dynamically allocated memory after it is freed.}}
+  x[0] = 'a'; // expected-warning{{Use of dynamically allocated memory after it is freed.}}
 }
 
 void PR6123() {
diff --git a/test/Analysis/malloc.c b/test/Analysis/malloc.c
index 7ffc9a1ec0..8d6295609b 100644
--- a/test/Analysis/malloc.c
+++ b/test/Analysis/malloc.c
@@ -84,13 +84,13 @@ void pr6293() {
 void f7() {
   char *x = (char*) malloc(4);
   free(x);
-  x[0] = 'a'; // expected-warning{{Use dynamically allocated memory after it is freed.}}
+  x[0] = 'a'; // expected-warning{{Use of dynamically allocated memory after it is freed.}}
 }
 
 void f7_realloc() {
   char *x = (char*) malloc(4);
   realloc(x,0);
-  x[0] = 'a'; // expected-warning{{Use dynamically allocated memory after it is freed.}}
+  x[0] = 'a'; // expected-warning{{Use of dynamically allocated memory after it is freed.}}
 }
 
 void PR6123() {
@@ -186,7 +186,7 @@ void mallocEscapeFreeUse() {
   int *p = malloc(12);
   myfoo(p);
   free(p);
-  myfoo(p); // expected-warning{{Use dynamically allocated memory after it is freed.}}
+  myfoo(p); // expected-warning{{Use of dynamically allocated memory after it is freed.}}
 }
 
 int *myalloc();
@@ -212,7 +212,7 @@ void mallocBindFreeUse() {
   int *x = malloc(12);
   int *y = x;
   free(y);
-  myfoo(x); // expected-warning{{Use dynamically allocated memory after it is freed.}}
+  myfoo(x); // expected-warning{{Use of dynamically allocated memory after it is freed.}}
 }
 
 void mallocEscapeMalloc() {
@@ -236,8 +236,8 @@ void mallocFreeMalloc() {
 void mallocFreeUse_params() {
   int *p = malloc(12);
   free(p);
-  myfoo(p); //expected-warning{{Use dynamically allocated memory after it is freed}}
-  myfooint(*p); //expected-warning{{Use dynamically allocated memory after it is freed}}
+  myfoo(p); //expected-warning{{Use of dynamically allocated memory after it is freed}}
+  myfooint(*p); //expected-warning{{Use of dynamically allocated memory after it is freed}}
 }
 
 void mallocFailedOrNot() {
@@ -248,6 +248,18 @@ void mallocFailedOrNot() {
     free(p);
 }
 
+struct StructWithInt {
+  int g;
+};
+void nonSymbolAsFirstArg(int *pp, struct StructWithInt *p);
+
+void mallocEscapeFooNonSymbolArg() {
+  struct StructWithInt *p = malloc(sizeof(struct StructWithInt));
+  nonSymbolAsFirstArg(&p->g, p);
+  return; // no warning
+}
+
+
 int *Gl;
 struct GlStTy {
   int *x;