From: Jordan Rose <jordan_rose@apple.com>
Date: Wed, 20 Mar 2013 20:35:57 +0000 (+0000)
Subject: [analyzer] Track malloc'd memory into struct fields.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=74f6982232c25ae723b1cc5abc59665a10867f21;p=clang

[analyzer] Track malloc'd memory into struct fields.

Due to improper modelling of copy constructors (specifically, their
const reference arguments), we were producing spurious leak warnings
for allocated memory stored in structs. In order to silence this, we
decided to consider storing into a struct to be the same as escaping.
However, the previous commit has fixed this issue and we can now properly
distinguish leaked memory that happens to be in a struct from a buffer
that escapes within a struct wrapper.

Originally applied in r161511, reverted in r174468.
<rdar://problem/12945937>

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

diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 53cea0f9a2..b619c5e024 100644
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1692,12 +1692,6 @@ ProgramStateRef ExprEngine::processPointerEscapedOnBind(ProgramStateRef State,
       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
diff --git a/test/Analysis/malloc-annotations.c b/test/Analysis/malloc-annotations.c
index bdd50c6be5..3a260c3aef 100644
--- a/test/Analysis/malloc-annotations.c
+++ b/test/Analysis/malloc-annotations.c
@@ -70,6 +70,11 @@ void af1_c() {
   myglobalpointer = my_malloc(12); // no-warning
 }
 
+void af1_d() {
+  struct stuff mystuff;
+  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.
 void af1_e(void **pp) {
   *pp = my_malloc(42); // no-warning
@@ -262,14 +267,3 @@ void testMultipleFreeAnnotations() {
   my_freeBoth(p, q);
 }
 
-// ----------------------------------------------------------------------------
-
-// False negatives.
-
-// Pending on removal of the escaping on assignment to struct fields.
-void af1_d() {
-  struct stuff mystuff;
-  mystuff.somefield = my_malloc(12);
-} // missing warning
-
-
diff --git a/test/Analysis/malloc.c b/test/Analysis/malloc.c
index e8b4ad3b4d..7790b32b07 100644
--- a/test/Analysis/malloc.c
+++ b/test/Analysis/malloc.c
@@ -531,6 +531,12 @@ int *testMalloc3() {
   return y; // no-warning
 }
 
+void testStructLeak() {
+  StructWithPtr St;
+  St.memP = malloc(12);
+  return; // expected-warning {{Memory is never released; potential leak of memory pointed to by 'St.memP'}}
+}
+
 void testElemRegion1() {
   char *x = (void*)malloc(2);
   int *ix = (int*)x;
@@ -929,6 +935,18 @@ int cmpHeapAllocationToUnknown() {
   return 0;
 }
 
+void localArrayTest() {
+  char *p = (char*)malloc(12);
+  char *ArrayL[12];
+  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}}
+
 #ifdef __INTPTR_TYPE__
 // Test double assignment through integers.
 typedef __INTPTR_TYPE__ intptr_t;
@@ -1045,50 +1063,18 @@ void testPassToSystemHeaderFunctionIndirectly() {
   fakeSystemHeaderCallInt(p);
 } // expected-warning {{leak}}
 
-// ----------------------------------------------------------------------------
-// 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));
-}
-
-// Pending on removal of the escaping on assignment to struct fields.
-void testStructLeak() {
-  StructWithPtr St;
-  St.memP = malloc(12);
-  return; // missing warning
-}
-
-void localArrayTest() {
-  char *p = (char*)malloc(12);
-  char *ArrayL[12];
-  ArrayL[0] = p;
-} // missing warning
-
-void localStructTest() {
-  StructWithPtr St;
-  StructWithPtr *pSt = &St;
-  pSt->memP = malloc(12);
-} // missing warning
-
 void testPassConstPointerIndirectlyStruct() {
   struct HasPtr hp;
   hp.p = malloc(10);
   memcmp(&hp, &hp, sizeof(hp));
-  return; // missing leak
+  return; // expected-warning {{Memory is never released; potential leak of memory pointed to by 'hp.p'}}
 }
 
 void testPassToSystemHeaderFunctionIndirectlyStruct() {
   SomeStruct ss;
   ss.p = malloc(1);
   fakeSystemHeaderCall(&ss);
-} // missing leak
+} // expected-warning {{Memory is never released; potential leak of memory pointed to by 'ss.p'}}
 
 int *testOffsetAllocate(size_t size) {
   int *memoryBlock = (int *)malloc(size + sizeof(int));
@@ -1202,3 +1188,15 @@ void freeMemory() {
     poolFreeC(_vectorSegments[_nVectorSegments++]);
   }
 }
+
+// ----------------------------------------------------------------------------
+// False negatives.
+
+void testMallocWithParam(int **p) {
+  *p = (int*) malloc(sizeof(int));
+  *p = 0; // FIXME: should warn here
+}
+
+void testMallocWithParam_2(int **p) {
+  *p = (int*) malloc(sizeof(int)); // no-warning
+}
diff --git a/test/Analysis/simple-stream-checks.c b/test/Analysis/simple-stream-checks.c
index 1fb6de3ec1..ce57fa7ac3 100644
--- a/test/Analysis/simple-stream-checks.c
+++ b/test/Analysis/simple-stream-checks.c
@@ -88,4 +88,4 @@ void testPassToSystemHeaderFunctionIndirectly() {
   FileStruct fs;
   fs.p = fopen("myfile.txt", "w");
   fakeSystemHeaderCall(&fs);
-} // expected leak warning
+}  // expected-warning {{Opened file is never closed; potential resource leak}}