public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r12-5585] analyzer: further false leak fixes due to overzealous state merging [PR103217]
@ 2021-11-29 23:51 David Malcolm
0 siblings, 0 replies; only message in thread
From: David Malcolm @ 2021-11-29 23:51 UTC (permalink / raw)
To: gcc-cvs
https://gcc.gnu.org/g:132902177138c09803d639e12b1daebf2b9edddc
commit r12-5585-g132902177138c09803d639e12b1daebf2b9edddc
Author: David Malcolm <dmalcolm@redhat.com>
Date: Mon Nov 29 11:47:47 2021 -0500
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:
---
gcc/analyzer/store.cc | 14 ++++++--
gcc/testsuite/gcc.dg/analyzer/pr103217-2.c | 52 ++++++++++++++++++++++++++++++
gcc/testsuite/gcc.dg/analyzer/pr103217-3.c | 52 ++++++++++++++++++++++++++++++
gcc/testsuite/gcc.dg/analyzer/pr103217-4.c | 52 ++++++++++++++++++++++++++++++
gcc/testsuite/gcc.dg/analyzer/pr103217-5.c | 47 +++++++++++++++++++++++++++
5 files changed, 215 insertions(+), 2 deletions(-)
diff --git a/gcc/analyzer/store.cc b/gcc/analyzer/store.cc
index 3760858c26d..5eecbe6cf18 100644
--- 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
index 00000000000..3a9c4145848
--- /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
index 00000000000..b103a121650
--- /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
index 00000000000..516e1f4f997
--- /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
index 00000000000..d916d92eeec
--- /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;
+}
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2021-11-29 23:51 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-29 23:51 [gcc r12-5585] analyzer: further false leak fixes due to overzealous state merging [PR103217] David Malcolm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).