public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r12-7827] analyzer: fix ICE on memset of untracked region [PR105057]
@ 2022-03-26 13:07 David Malcolm
  0 siblings, 0 replies; only message in thread
From: David Malcolm @ 2022-03-26 13:07 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:8c8993c75309901e03418eba1d6239b9a39a43b7

commit r12-7827-g8c8993c75309901e03418eba1d6239b9a39a43b7
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Fri Mar 25 16:50:51 2022 -0400

    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>

Diff:
---
 gcc/analyzer/store.cc                    | 17 +++++++++++++----
 gcc/testsuite/gcc.dg/analyzer/fread-2.c  | 31 +++++++++++++++++++++++++++++++
 gcc/testsuite/gcc.dg/analyzer/memset-2.c | 27 +++++++++++++++++++++++++++
 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 62733b9777f..9aa7d690b04 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 00000000000..02a5e31cec6
--- /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 00000000000..de7c973b1fd
--- /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 00000000000..e4e6c975d14
--- /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;
+}


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-03-26 13:07 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-26 13:07 [gcc r12-7827] analyzer: fix ICE on memset of untracked region [PR105057] 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).