diff options
author | David Malcolm <dmalcolm@redhat.com> | 2022-03-25 16:50:51 -0400 |
---|---|---|
committer | David Malcolm <dmalcolm@redhat.com> | 2022-03-26 09:05:30 -0400 |
commit | 8c8993c75309901e03418eba1d6239b9a39a43b7 (patch) | |
tree | 2365a0f835c6b1a8ad5ae48c0de9c4f044bb8db7 | |
parent | f0fdd92e9dae17b543b089dac753211298b04c78 (diff) | |
download | gcc-8c8993c75309901e03418eba1d6239b9a39a43b7.zip gcc-8c8993c75309901e03418eba1d6239b9a39a43b7.tar.gz gcc-8c8993c75309901e03418eba1d6239b9a39a43b7.tar.bz2 |
analyzer: fix ICE on memset of untracked region [PR105057]
In r12-7809-g5f6197d7c197f9d2b7fb2e1a19dac39a023755e8 I added an
optimization to avoid tracking the state of certain memory regions
in the store.
Unfortunately, I didn't cover every way in which
store::get_or_create_cluster can be called for a base region, leading
to assertion failure ICEs in -fanalyzer on certain function calls
with certain params.
I've worked through all uses of store::get_or_create_cluster and found
four places where the assertion could fire.
This patch fixes them, and adds regression tests where possible.
gcc/analyzer/ChangeLog:
PR analyzer/105057
* store.cc (binding_cluster::make_unknown_relative_to): Reject
attempts to create a cluster for untracked base regions.
(store::set_value): Likewise.
(store::fill_region): Likewise.
(store::mark_region_as_unknown): Likewise.
gcc/testsuite/ChangeLog:
PR analyzer/105057
* gcc.dg/analyzer/fread-2.c: New test, as a regression test for
ICE in store::set_value on untracked base region.
* gcc.dg/analyzer/memset-2.c: Likewise, for ICE in
store::fill_region.
* gcc.dg/analyzer/strcpy-2.c: Likewise, for ICE in
store::mark_region_as_unknown.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
-rw-r--r-- | gcc/analyzer/store.cc | 17 | ||||
-rw-r--r-- | gcc/testsuite/gcc.dg/analyzer/fread-2.c | 31 | ||||
-rw-r--r-- | gcc/testsuite/gcc.dg/analyzer/memset-2.c | 27 | ||||
-rw-r--r-- | gcc/testsuite/gcc.dg/analyzer/strcpy-2.c | 27 |
4 files changed, 98 insertions, 4 deletions
diff --git a/gcc/analyzer/store.cc b/gcc/analyzer/store.cc index 62733b9..9aa7d69 100644 --- a/gcc/analyzer/store.cc +++ b/gcc/analyzer/store.cc @@ -1823,7 +1823,8 @@ binding_cluster::make_unknown_relative_to (const binding_cluster *other, { const region *base_reg = region_sval->get_pointee ()->get_base_region (); - if (!base_reg->symbolic_for_unknown_ptr_p ()) + if (base_reg->tracked_p () + && !base_reg->symbolic_for_unknown_ptr_p ()) { binding_cluster *c = out_store->get_or_create_cluster (base_reg); c->mark_as_escaped (); @@ -2384,11 +2385,17 @@ store::set_value (store_manager *mgr, const region *lhs_reg, mark_as_escaped (ptr_base_reg); } } - else + else if (lhs_base_reg->tracked_p ()) { lhs_cluster = get_or_create_cluster (lhs_base_reg); lhs_cluster->bind (mgr, lhs_reg, rhs_sval); } + else + { + /* Reject attempting to bind values into an untracked region; + merely invalidate values below. */ + lhs_cluster = NULL; + } /* Bindings to a cluster can affect other clusters if a symbolic base region is involved. @@ -2564,7 +2571,8 @@ void store::fill_region (store_manager *mgr, const region *reg, const svalue *sval) { const region *base_reg = reg->get_base_region (); - if (base_reg->symbolic_for_unknown_ptr_p ()) + if (base_reg->symbolic_for_unknown_ptr_p () + || !base_reg->tracked_p ()) return; binding_cluster *cluster = get_or_create_cluster (base_reg); cluster->fill_region (mgr, reg, sval); @@ -2587,7 +2595,8 @@ store::mark_region_as_unknown (store_manager *mgr, const region *reg, uncertainty_t *uncertainty) { const region *base_reg = reg->get_base_region (); - if (base_reg->symbolic_for_unknown_ptr_p ()) + if (base_reg->symbolic_for_unknown_ptr_p () + || !base_reg->tracked_p ()) return; binding_cluster *cluster = get_or_create_cluster (base_reg); cluster->mark_region_as_unknown (mgr, reg, uncertainty); diff --git a/gcc/testsuite/gcc.dg/analyzer/fread-2.c b/gcc/testsuite/gcc.dg/analyzer/fread-2.c new file mode 100644 index 0000000..02a5e31 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/fread-2.c @@ -0,0 +1,31 @@ +/* { dg-additional-options "-fdump-analyzer-untracked" } */ + +#include "analyzer-decls.h" + +struct S +{ + int i; +}; + +typedef __SIZE_TYPE__ size_t; + +extern size_t fread (void *, size_t, size_t, void *); + +/* fread of a static struct that never gets used. */ + +void +test_1 (void *fp) +{ + static struct S s; /* { dg-warning "track 's': no" } */ + fread (&s, sizeof (s), 1, fp); +} + +/* fread of a static struct that later gets used. */ + +int +test_2 (void *fp) +{ + static struct S s; /* { dg-warning "track 's': yes" } */ + fread (&s, sizeof (s), 1, fp); + return s.i; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/memset-2.c b/gcc/testsuite/gcc.dg/analyzer/memset-2.c new file mode 100644 index 0000000..de7c973 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/memset-2.c @@ -0,0 +1,27 @@ +/* { dg-additional-options "-fdump-analyzer-untracked" } */ + +#include "analyzer-decls.h" + +struct S +{ + int i; +}; + +/* memset of a static struct that never gets used. */ + +void +test_1 (void) +{ + static struct S s; /* { dg-warning "track 's': no" } */ + __builtin_memset (&s, 0, sizeof (s)); +} + +/* memset of a static struct that later gets used. */ + +void +test_2 (void) +{ + static struct S s; /* { dg-warning "track 's': yes" } */ + __builtin_memset (&s, 0, sizeof (s)); + __analyzer_eval (s.i == 0); /* { dg-warning "TRUE" } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/strcpy-2.c b/gcc/testsuite/gcc.dg/analyzer/strcpy-2.c new file mode 100644 index 0000000..e4e6c97 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/strcpy-2.c @@ -0,0 +1,27 @@ +/* { dg-additional-options "-fdump-analyzer-untracked" } */ + +#include "analyzer-decls.h" + +struct S +{ + char buf[10]; +}; + +/* strcpy to a static struct that never gets used. */ + +void +test_1 (const char *src) +{ + static struct S s; /* { dg-warning "track 's': no" } */ + __builtin_strcpy (s.buf, src); +} + +/* strcpy to a static struct that later gets used. */ + +const char * +test_2 (const char *src) +{ + static struct S s; /* { dg-warning "track 's': yes" } */ + __builtin_strcpy (s.buf, src); + return s.buf; +} |