[analyzer] Indirect invalidation counts as an escape for leak checkers.

Consider this example:

  char *p = malloc(sizeof(char));
  systemFunction(&p);
  free(p);

In this case, when we call systemFunction, we know (because it's a system
function) that it won't free 'p'. However, we /don't/ know whether or not
it will /change/ 'p', so the analyzer is forced to invalidate 'p', wiping
out any bindings it contains. But now the malloc'd region looks like a
leak, since there are no more bindings pointing to it, and we'll get a
spurious leak warning.

The fix for this is to notice when something is becoming inaccessible due
to invalidation (i.e. an imperfect model, as opposed to being explicitly
overwritten) and stop tracking it at that point. Currently, the best way
to determine this for a call is the "indirect escape" pointer-escape kind.

In practice, all the patch does is take the "system functions don't free
memory" special case and limit it to direct parameters, i.e. just the
arguments to a call and not other regions accessible to them. This is a
conservative change that should only cause us to escape regions more
eagerly, which means fewer leak warnings.

This isn't perfect for several reasons, the main one being that this
example is treated the same as the one above:

  char **p = malloc(sizeof(char *));
  systemFunction(p + 1);
  // leak

Currently, "addresses accessible by offsets of the starting region" and
"addresses accessible through bindings of the starting region" are both
considered "indirect" regions, hence this uniform treatment.

Another issue is our longstanding problem of not distinguishing const and
non-const bindings; if in the first example systemFunction's parameter were
a char * const *, we should know that the function will not overwrite 'p',
and thus we can safely report the leak.

<rdar://problem/13758386>

llvm-svn: 181607

1 files changed, 34 insertions, 8 deletions

diff --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c
index 6071d1d4358..76f59fda199 100644
--- a/clang/test/Analysis/malloc.c
+++ b/clang/test/Analysis/malloc.c
@@ -1057,12 +1057,6 @@ void testPassConstPointerIndirectly() {
   return; // expected-warning {{leak}}
 }
 
-void testPassToSystemHeaderFunctionIndirectly() {
-  int *p = malloc(4);
-  p++;
-  fakeSystemHeaderCallInt(p);
-} // expected-warning {{leak}}
-
 void testPassConstPointerIndirectlyStruct() {
   struct HasPtr hp;
   hp.p = malloc(10);
@@ -1073,8 +1067,32 @@ void testPassConstPointerIndirectlyStruct() {
 void testPassToSystemHeaderFunctionIndirectlyStruct() {
   SomeStruct ss;
   ss.p = malloc(1);
-  fakeSystemHeaderCall(&ss);
-} // expected-warning {{Potential leak of memory pointed to by 'ss.p'}}
+  fakeSystemHeaderCall(&ss); // invalidates ss, making ss.p unreachable
+  // Technically a false negative here -- we know the system function won't free
+  // ss.p, but nothing else will either!
+} // no-warning
+
+void testPassToSystemHeaderFunctionIndirectlyStructFree() {
+  SomeStruct ss;
+  ss.p = malloc(1);
+  fakeSystemHeaderCall(&ss); // invalidates ss, making ss.p unreachable
+  free(ss.p);
+} // no-warning
+
+void testPassToSystemHeaderFunctionIndirectlyArray() {
+  int *p[1];
+  p[0] = malloc(sizeof(int));
+  fakeSystemHeaderCallIntPtr(p); // invalidates p, making p[0] unreachable
+  // Technically a false negative here -- we know the system function won't free
+  // p[0], but nothing else will either!
+} // no-warning
+
+void testPassToSystemHeaderFunctionIndirectlyArrayFree() {
+  int *p[1];
+  p[0] = malloc(sizeof(int));
+  fakeSystemHeaderCallIntPtr(p); // invalidates p, making p[0] unreachable
+  free(p[0]);
+} // no-warning
 
 int *testOffsetAllocate(size_t size) {
   int *memoryBlock = (int *)malloc(size + sizeof(int));
@@ -1200,3 +1218,11 @@ void testMallocWithParam(int **p) {
 void testMallocWithParam_2(int **p) {
   *p = (int*) malloc(sizeof(int)); // no-warning
 }
+
+void testPassToSystemHeaderFunctionIndirectly() {
+  int *p = malloc(4);
+  p++;
+  fakeSystemHeaderCallInt(p);
+  // FIXME: This is a leak: if we think a system function won't free p, it
+  // won't free (p-1) either.
+}