analyzer: further false leak fixes due to overzealous state merging [PR103217]

Commit r12-5424-gf573d35147ca8433c102e1721d8c99fc432cb44b fixed a false
positive from -Wanalyzer-malloc-leak due to overzealous state merging,
erroneously merging two different svalues bound to a particular part
of the store when one has sm-state.

A further case was discovered by the reporter of PR analyzer/103217,
which this patch fixes.  In this variant, different states have set
different fields of a struct, and on attempting to merge them, the
states have a different set of binding keys, leading to one state
having an svalue with sm-state, and its peer state having a NULL value
for that binding key.  The state merger code was erroneously treating
them as mergeable to "UNKNOWN".  This followup patch fixes things by
rejecting such mergers if the non-NULL svalue is not mergeable with
"UNKNOWN".

gcc/analyzer/ChangeLog:
	PR analyzer/103217
	* store.cc (binding_cluster::can_merge_p): For the "key is bound"
	vs "key is not bound" merger case, check that the bound svalue
	is mergeable before merging it to "unknown", rejecting the merger
	otherwise.

gcc/testsuite/ChangeLog:
	PR analyzer/103217
	* gcc.dg/analyzer/pr103217-2.c: New test.
	* gcc.dg/analyzer/pr103217-3.c: New test.
	* gcc.dg/analyzer/pr103217-4.c: New test.
	* gcc.dg/analyzer/pr103217-5.c: New test.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>

diff --git a/gcc/analyzer/store.cc b/gcc/analyzer/store.cc
index 3760858..5eecbe6 100644 (file)
--- a/gcc/analyzer/store.cc
+++ b/gcc/analyzer/store.cc
@@ -1729,6 +1729,7 @@ binding_cluster::can_merge_p (const binding_cluster *cluster_a,
   for (hash_set<const binding_key *>::iterator iter = keys.begin ();
        iter != keys.end (); ++iter)
     {
+      region_model_manager *sval_mgr = mgr->get_svalue_manager ();
       const binding_key *key = *iter;
       const svalue *sval_a = cluster_a->get_any_value (key);
       const svalue *sval_b = cluster_b->get_any_value (key);
@@ -1746,7 +1747,6 @@ binding_cluster::can_merge_p (const binding_cluster *cluster_a,
        }
       else if (sval_a && sval_b)
        {
-         region_model_manager *sval_mgr = mgr->get_svalue_manager ();
          if (const svalue *merged_sval
              = sval_a->can_merge_p (sval_b, sval_mgr, merger))
            {
@@ -1760,9 +1760,19 @@ binding_cluster::can_merge_p (const binding_cluster *cluster_a,
       /* If we get here, then one cluster binds this key and the other
         doesn't; merge them as "UNKNOWN".  */
       gcc_assert (sval_a || sval_b);
-      tree type = sval_a ? sval_a->get_type () : sval_b->get_type ();
+
+      const svalue *bound_sval = sval_a ? sval_a : sval_b;
+      tree type = bound_sval->get_type ();
       const svalue *unknown_sval
        = mgr->get_svalue_manager ()->get_or_create_unknown_svalue (type);
+
+      /* ...but reject the merger if this sval shouldn't be mergeable
+        (e.g. reject merging svalues that have non-purgable sm-state,
+        to avoid falsely reporting memory leaks by merging them
+        with something else).  */
+      if (!bound_sval->can_merge_p (unknown_sval, sval_mgr, merger))
+       return false;
+
       out_cluster->m_map.put (key, unknown_sval);
     }
 

diff --git a/gcc/testsuite/gcc.dg/analyzer/pr103217-2.c b/gcc/testsuite/gcc.dg/analyzer/pr103217-2.c
new file mode 100644 (file)
index 0000000..3a9c414
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/pr103217-2.c
@@ -0,0 +1,52 @@
+typedef __SIZE_TYPE__ size_t;
+
+extern void *calloc (size_t __nmemb, size_t __size)
+  __attribute__ ((__nothrow__ , __leaf__, __malloc__, __alloc_size__ (1, 2)));
+
+extern char *strdup (const char *__s)
+  __attribute__ ((__nothrow__ , __leaf__, __malloc__, __nonnull__ (1)));
+
+extern void abort (void)
+  __attribute__ ((__nothrow__ , __leaf__, __noreturn__));
+
+extern int getopt (int ___argc, char *const *___argv, const char *__shortopts)
+  __attribute__ ((__nothrow__ , __leaf__, __nonnull__ (2, 3)));
+extern char *optarg;
+
+extern void free (void *__ptr)
+  __attribute__ ((__nothrow__ , __leaf__));
+
+char *xstrdup(const char *src) {
+       char *val = strdup(src);
+       if (!val)
+               abort();
+       return val;
+}
+
+struct test {
+       char *one, *two;
+};
+
+int main(int argc, char *argv[]) {
+       struct test *options = calloc(1, sizeof(*options));
+       int rc;
+       if (!options)
+               abort();
+
+       while ((rc = getopt(argc, argv, "a:b:")) != -1) {
+               switch (rc) {
+               case 'a':
+                       free(options->one);
+                       options->one = xstrdup(optarg);
+                       break;
+               case 'b':
+                       free(options->two);
+                       options->two = xstrdup(optarg);
+                       break;
+               }
+       }
+       free(options->one);
+       free(options->two);
+       free(options);
+       return 0;
+}

diff --git a/gcc/testsuite/gcc.dg/analyzer/pr103217-3.c b/gcc/testsuite/gcc.dg/analyzer/pr103217-3.c
new file mode 100644 (file)
index 0000000..b103a12
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/pr103217-3.c
@@ -0,0 +1,52 @@
+extern char *strdup (const char *__s)
+  __attribute__ ((__nothrow__ , __leaf__, __malloc__, __nonnull__ (1)));
+
+extern void abort (void)
+  __attribute__ ((__nothrow__ , __leaf__, __noreturn__));
+
+extern int getopt (int ___argc, char *const *___argv, const char *__shortopts)
+  __attribute__ ((__nothrow__ , __leaf__, __nonnull__ (2, 3)));
+extern char *optarg;
+
+extern void free (void *__ptr)
+  __attribute__ ((__nothrow__ , __leaf__));
+
+struct state {
+  const char *confpath;
+  const char *host;
+  const char *port;
+  const char *state_dir_prefix;
+};
+
+static inline char *xstrdup(const char *s) { 
+        char *val = strdup(s);
+        if (!val)
+                abort();
+        return val;
+}
+
+int config_init(struct state *config);
+
+int main(int argc, char *argv[]) { 
+        int rc;
+        struct state state = { 0 };
+
+        config_init(&state);
+
+        while ((rc = getopt(argc, argv, "H:p:")) != -1) { 
+                switch (rc) { 
+                case 'H':
+                        free((void*)state.host);
+                        state.host = xstrdup(optarg);
+                        break;
+                case 'p':
+                        free((void*)state.port);
+                        state.port = xstrdup(optarg);
+                        break;
+                } 
+        } 
+
+        free((void*)state.host);
+        free((void*)state.port);
+        return rc;
+}

diff --git a/gcc/testsuite/gcc.dg/analyzer/pr103217-4.c b/gcc/testsuite/gcc.dg/analyzer/pr103217-4.c
new file mode 100644 (file)
index 0000000..516e1f4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/pr103217-4.c
@@ -0,0 +1,52 @@
+extern char *strdup (const char *__s)
+  __attribute__ ((__nothrow__ , __leaf__, __malloc__, __nonnull__ (1)));
+
+extern void abort (void)
+  __attribute__ ((__nothrow__ , __leaf__, __noreturn__));
+
+extern int getopt (int ___argc, char *const *___argv, const char *__shortopts)
+  __attribute__ ((__nothrow__ , __leaf__, __nonnull__ (2, 3)));
+extern char *optarg;
+
+extern void free (void *__ptr)
+  __attribute__ ((__nothrow__ , __leaf__));
+
+struct state {
+  const char *confpath;
+  const char *host;
+  const char *port;
+  const char *state_dir_prefix;
+};
+
+static inline char *xstrdup(const char *s) { 
+        char *val = strdup(s);
+        if (!val)
+                abort();
+        return val;
+}
+
+int config_init(struct state *config);
+
+int main(int argc, char *argv[]) { 
+        int rc;
+        struct state state = { 0 };
+
+        config_init(&state);
+
+        if ((rc = getopt(argc, argv, "H:p:")) != -1) {
+                switch (rc) {
+                case 'H':
+                        free((void*)state.host);
+                        state.host = xstrdup(optarg);
+                        break;
+                case 'p':
+                        free((void*)state.port);
+                        state.port = xstrdup(optarg);
+                        break;
+                } 
+        } 
+
+        free((void*)state.host);
+        free((void*)state.port);
+        return rc;
+}

diff --git a/gcc/testsuite/gcc.dg/analyzer/pr103217-5.c b/gcc/testsuite/gcc.dg/analyzer/pr103217-5.c
new file mode 100644 (file)
index 0000000..d916d92
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/pr103217-5.c
@@ -0,0 +1,47 @@
+extern char *strdup (const char *__s)
+  __attribute__ ((__nothrow__ , __leaf__, __malloc__, __nonnull__ (1)));
+
+extern void abort (void)
+  __attribute__ ((__nothrow__ , __leaf__, __noreturn__));
+
+extern int getopt (int ___argc, char *const *___argv, const char *__shortopts)
+  __attribute__ ((__nothrow__ , __leaf__, __nonnull__ (2, 3)));
+extern char *optarg;
+
+extern void free (void *__ptr)
+  __attribute__ ((__nothrow__ , __leaf__));
+
+struct state {
+  const char *confpath;
+  const char *host;
+  const char *port;
+  const char *state_dir_prefix;
+};
+
+static inline char *xstrdup(const char *s) {
+        char *val = strdup(s);
+        if (!val)
+                abort();
+        return val;
+}
+
+int config_init(struct state *config);
+
+int main(int argc, char *argv[]) {
+  struct state state;
+
+  config_init(&state);
+
+  switch (getopt(argc, argv, "H:p:")) {
+  case 'H':
+    state.host = xstrdup(optarg);
+    break;
+  case 'p':
+    state.port = xstrdup(optarg);
+    break;
+  }
+
+  free((void*)state.host);
+  free((void*)state.port);
+  return 0;
+}