[cfe-commits] r151121 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/MallocChecker.cpp test/Analysis/malloc.c

[Anna Zaks]

Author: zaks
Date: Tue Feb 21 20:36:01 2012
New Revision: 151121

URL: http://llvm.org/viewvc/llvm-project?rev=151121&view=rev
Log:
[analyzer] Malloc: fix another false positive.
, when we return a symbol reachable to the malloced one via pointer
arithmetic.

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

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=151121&r1=151120&r2=151121&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Tue Feb 21 20:36:01 2012
@@ -881,7 +881,17 @@
     return;
 
   // Check if we are returning a symbol.
-  SymbolRef Sym = C.getState()->getSVal(E, C.getLocationContext()).getAsSymbol();
+  SVal RetVal = C.getState()->getSVal(E, C.getLocationContext());
+  SymbolRef Sym = RetVal.getAsSymbol();
+  if (!Sym)
+    // If we are returning a field of the allocated struct or an array element,
+    // the callee could still free the memory.
+    // TODO: This logic should be a part of generic symbol escape callback.
+    if (const MemRegion *MR = RetVal.getAsRegion())
+      if (isa<FieldRegion>(MR) || isa<ElementRegion>(MR))
+        if (const SymbolicRegion *BMR =
+              dyn_cast<SymbolicRegion>(MR->getBaseRegion()))
+          Sym = BMR->getSymbol();
   if (!Sym)
     return;
 

Modified: cfe/trunk/test/Analysis/malloc.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc.c?rev=151121&r1=151120&r2=151121&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/malloc.c (original)
+++ cfe/trunk/test/Analysis/malloc.c Tue Feb 21 20:36:01 2012
@@ -631,6 +631,27 @@
   p = ((__builtin_object_size (p, 0) != (size_t) -1) ? __builtin___strcpy_chk (p, s, __builtin_object_size (p, 2 > 1)) : __inline_strcpy_chk (p, s));
   free(p);
 }
+
+// Here we are returning a pointer one past the allocated value. An idiom which
+// can be used for implementing special malloc. The correct uses of this might
+// be rare enough so that we could keep this as a warning.
+static void *specialMalloc(int n){
+  int *p;
+  p = malloc( n+8 );
+  if( p ){
+    p[0] = n;
+    p++;
+  }
+  return p;
+}
+
+// Potentially, the user could free the struct by performing pointer arithmetic on the return value.
+// This is a variation of the specialMalloc issue, though probably would be more rare in correct code.
+int *specialMallocWithStruct() {
+  struct StructWithInt *px= malloc(sizeof(struct StructWithInt));
+  return &(px->g);
+}
+
 // Below are the known false positives.
 
 // TODO: There should be no warning here. This one might be difficult to get rid of.
@@ -650,20 +671,6 @@
   return;
 }
 
-// TODO: Should this be a warning?
-// Here we are returning a pointer one past the allocated value. An idiom which
-// can be used for implementing special malloc. The correct uses of this might
-// be rare enough so that we could keep this as a warning.
-static void *specialMalloc(int n){
-  int *p;
-  p = malloc( n+8 );
-  if( p ){
-    p[0] = n;
-    p++;
-  }
-  return p;// expected-warning {{Memory is never released; potential memory leak}}
-}
-
 // False negatives.
 
 // TODO: This requires tracking symbols stored inside the structs/arrays.
@@ -673,6 +680,16 @@
   pSt->memP = malloc(12);
 }
 
+// TODO: This is another false negative.
+void testMallocWithParam(int **p) {
+  *p = (int*) malloc(sizeof(int));
+  *p = 0;
+}
+
+void testMallocWithParam_2(int **p) {
+  *p = (int*) malloc(sizeof(int));
+}
+
 // TODO: This should produce a warning, similar to the previous issue.
 void localArrayTest() {
   char *p = (char*)malloc(12);