[cfe-commits] r161511 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/MallocChecker.cpp lib/StaticAnalyzer/Core/ProgramState.cpp test/Analysis/malloc-annotations.c test/Analysis/malloc.c

Jordan Rose jordan_rose at apple.com
Wed Aug 8 11:23:32 PDT 2012


Author: jrose
Date: Wed Aug  8 13:23:31 2012
New Revision: 161511

URL: http://llvm.org/viewvc/llvm-project?rev=161511&view=rev
Log:
[analyzer] Track malloc'd regions stored in structs.

The main blocker on this (besides the previous commit) was that
ScanReachableSymbols was not looking through LazyCompoundVals.
Once that was fixed, it's easy enough to clear out malloc data on return,
just like we do when we bind to a global region.

<rdar://problem/10872635>

Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp
    cfe/trunk/test/Analysis/malloc-annotations.c
    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=161511&r1=161510&r2=161511&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Wed Aug  8 13:23:31 2012
@@ -1000,10 +1000,11 @@
   SmallString<200> buf;
   llvm::raw_svector_ostream os(buf);
   os << "Memory is never released; potential leak";
-  if (Region) {
+  // FIXME: Make all region pretty-printing nice enough to show.
+  if (Region && isa<VarRegion>(Region)) {
     os << " of memory pointed to by '";
     Region->dumpPretty(os);
-    os <<'\'';
+    os << '\'';
   }
 
   BugReport *R = new BugReport(*BT_Leak, os.str(), N, LocUsedForUniqueing);
@@ -1117,7 +1118,8 @@
     return;
 
   // Check if we are returning a symbol.
-  SVal RetVal = C.getState()->getSVal(E, C.getLocationContext());
+  ProgramStateRef State = C.getState();
+  SVal RetVal = State->getSVal(E, C.getLocationContext());
   SymbolRef Sym = RetVal.getAsSymbol();
   if (!Sym)
     // If we are returning a field of the allocated struct or an array element,
@@ -1128,16 +1130,18 @@
         if (const SymbolicRegion *BMR =
               dyn_cast<SymbolicRegion>(MR->getBaseRegion()))
           Sym = BMR->getSymbol();
-  if (!Sym)
-    return;
 
   // Check if we are returning freed memory.
-  if (checkUseAfterFree(Sym, C, E))
-    return;
+  if (Sym)
+    if (checkUseAfterFree(Sym, C, E))
+      return;
 
-  // If this function body is not inlined, check if the symbol is escaping.
-  if (C.getLocationContext()->getParent() == 0)
-    checkEscape(Sym, E, C);
+  // If this function body is not inlined, stop tracking any returned symbols.
+  if (C.getLocationContext()->getParent() == 0) {
+    State =
+      State->scanReachableSymbols<StopTrackingCallback>(RetVal).getState();
+    C.addTransition(State);
+  }
 }
 
 // TODO: Blocks should be either inlined or should call invalidate regions
@@ -1245,12 +1249,6 @@
       if (StoredVal != val)
         escapes = (state == (state->bindLoc(*regionLoc, val)));
     }
-    if (!escapes) {
-      // Case 4: We do not currently model what happens when a symbol is
-      // assigned to a struct field, so be conservative here and let the symbol
-      // go. TODO: This could definitely be improved upon.
-      escapes = !isa<VarRegion>(regionLoc->getRegion());
-    }
   }
 
   // If our store can represent the binding and we aren't storing to something

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp?rev=161511&r1=161510&r2=161511&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp Wed Aug  8 13:23:31 2012
@@ -542,6 +542,9 @@
   if (loc::MemRegionVal *X = dyn_cast<loc::MemRegionVal>(&val))
     return scan(X->getRegion());
 
+  if (nonloc::LazyCompoundVal *X = dyn_cast<nonloc::LazyCompoundVal>(&val))
+    return scan(X->getRegion());
+
   if (nonloc::LocAsInteger *X = dyn_cast<nonloc::LocAsInteger>(&val))
     return scan(X->getLoc());
 

Modified: cfe/trunk/test/Analysis/malloc-annotations.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc-annotations.c?rev=161511&r1=161510&r2=161511&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/malloc-annotations.c (original)
+++ cfe/trunk/test/Analysis/malloc-annotations.c Wed Aug  8 13:23:31 2012
@@ -70,10 +70,9 @@
   myglobalpointer = my_malloc(12); // no-warning
 }
 
-// TODO: We will be able to handle this after we add support for tracking allocations stored in struct fields.
 void af1_d() {
   struct stuff mystuff;
-  mystuff.somefield = my_malloc(12); // false negative
+  mystuff.somefield = my_malloc(12); // expected-warning{{Memory is never released; potential leak}}
 }
 
 // Test that we can pass out allocated memory via pointer-to-pointer.

Modified: cfe/trunk/test/Analysis/malloc.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc.c?rev=161511&r1=161510&r2=161511&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/malloc.c (original)
+++ cfe/trunk/test/Analysis/malloc.c Wed Aug  8 13:23:31 2012
@@ -513,20 +513,20 @@
   int *x = malloc(12);
   StructWithPtr St;
   St.memP = x;
-  arrOfStructs[0] = St;
+  arrOfStructs[0] = St; // no-warning
 }
 
 StructWithPtr testMalloc2() {
   int *x = malloc(12);
   StructWithPtr St;
   St.memP = x;
-  return St;
+  return St; // no-warning
 }
 
 int *testMalloc3() {
   int *x = malloc(12);
   int *y = x;
-  return y;
+  return y; // no-warning
 }
 
 void testElemRegion1() {
@@ -926,31 +926,16 @@
   return 0;
 }
 
-// ----------------------------------------------------------------------------
-// False negatives.
-
-// TODO: This requires tracking symbols stored inside the structs/arrays.
-void testMalloc5() {
-  StructWithPtr St;
-  StructWithPtr *pSt = &St;
-  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);
   char *ArrayL[12];
-  ArrayL[0] = p;
+  ArrayL[0] = p; // expected-warning {{leak}}
+}
+
+void localStructTest() {
+  StructWithPtr St;
+  StructWithPtr *pSt = &St;
+  pSt->memP = malloc(12); // expected-warning{{Memory is never released; potential leak}}
 }
 
 // Test double assignment through integers.
@@ -1019,3 +1004,16 @@
     return 0; // expected-warning{{Memory is never released; potential leak}}
   return a->p;
 }
+
+// ----------------------------------------------------------------------------
+// False negatives.
+
+// 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));
+}





More information about the cfe-commits mailing list