[gcc r13-5745] analyzer: fix overzealous state purging with on-stack structs [PR108704]

[gcc r13-5745] analyzer: fix overzealous state purging with on-stack structs [PR108704]

[David Malcolm]

https://gcc.gnu.org/g:77bb54b1b07add45007c664724b726541d672ef3

commit r13-5745-g77bb54b1b07add45007c664724b726541d672ef3
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Wed Feb 8 13:47:36 2023 -0500

    analyzer: fix overzealous state purging with on-stack structs [PR108704]
    
    PR analyzer/108704 reports many false positives seen from
    -Wanalyzer-use-of-uninitialized-value on qemu's softfloat.c on code like
    the following:
    
       struct st s;
       s = foo ();
       s = bar (s); // bogusly reports that s is uninitialized here
    
    where e.g. "struct st" is "floatx80" in the qemu examples.
    
    The root cause is overzealous purging of on-stack structs in the code I
    added in r12-7718-gfaacafd2306ad7, where at:
    
            s = bar (s);
    
    state_purge_per_decl::process_point_backwards "sees" the assignment to 's'
    and stops processing, effectively treating 's' as unneeded before this
    stmt, not noticing the use of 's' in the argument.
    
    Fixed thusly.
    
    The patch greatly reduces the number of
    -Wanalyzer-use-of-uninitialized-value warnings from my integration tests:
      ImageMagick-7.1.0-57:  10 ->  6   (-4)
                  qemu-7.2: 858 -> 87 (-771)
             haproxy-2.7.1:   1 ->  0   (-1)
    All of the above that I've examined appear to be false positives.
    
    gcc/analyzer/ChangeLog:
            PR analyzer/108704
            * state-purge.cc (state_purge_per_decl::process_point_backwards):
            Don't stop processing the decl if it's fully overwritten by
            this stmt if it's also used by this stmt.
    
    gcc/testsuite/ChangeLog:
            PR analyzer/108704
            * gcc.dg/analyzer/uninit-7.c: New test.
            * gcc.dg/analyzer/uninit-pr108704.c: New test.
    
    Signed-off-by: David Malcolm <dmalcolm@redhat.com>

Diff:
---
 gcc/analyzer/state-purge.cc                     |  15 ++-
 gcc/testsuite/gcc.dg/analyzer/uninit-7.c        | 127 ++++++++++++++++++++++++
 gcc/testsuite/gcc.dg/analyzer/uninit-pr108704.c |  29 ++++++
 3 files changed, 170 insertions(+), 1 deletion(-)

diff --git a/gcc/analyzer/state-purge.cc b/gcc/analyzer/state-purge.cc
index 5fa596d98fb..5f2d1f7fefa 100644
--- a/gcc/analyzer/state-purge.cc
+++ b/gcc/analyzer/state-purge.cc
@@ -922,7 +922,20 @@ process_point_backwards (const function_point &point,
       {
 	/* This is somewhat equivalent to how the SSA case handles
 	   def-stmts.  */
-	if (fully_overwrites_p (point.get_stmt (), m_decl, model))
+	if (fully_overwrites_p (point.get_stmt (), m_decl, model)
+	    /* ...but we mustn't be at a point that also consumes the
+	       current value of the decl when it's generating the new
+	       value, for cases such as
+		  struct st s;
+		  s = foo ();
+		  s = bar (s);
+	       where we want to make sure that we don't stop at the:
+		  s = bar (s);
+	       since otherwise we would erroneously purge the state of "s"
+	       after:
+		  s = foo ();
+	    */
+	    && !m_points_needing_decl.contains (point))
 	  {
 	    if (logger)
 	      logger->log ("stmt fully overwrites %qE; terminating", m_decl);
diff --git a/gcc/testsuite/gcc.dg/analyzer/uninit-7.c b/gcc/testsuite/gcc.dg/analyzer/uninit-7.c
new file mode 100644
index 00000000000..cb3e29abe8f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/uninit-7.c
@@ -0,0 +1,127 @@
+typedef struct st
+{
+  char buf[16];
+} st;
+
+extern st foo (st);
+extern st bar (st *);
+extern char baz (st);
+
+void test_1 (st a)
+{
+  st b, c, d, e;
+
+  b = a;
+  c = foo(a);
+  d = bar(&a);
+  c = foo(e); /* { dg-warning "use of uninitialized value 'e'" } */
+}
+
+void test_2 (st a)
+{
+  a = a;
+}
+
+st test_2a (void)
+{
+  st a;
+  a = a; /* { dg-warning "use of uninitialized value 'a'" } */
+  return a;
+}
+
+void test_3 (st a)
+{
+  a = foo (a);
+}
+
+st test_3a (void)
+{
+  st a;
+  a = foo (a); /* { dg-warning "use of uninitialized value 'a'" } */
+  return a;
+}
+
+void test_3b (st a, st b)
+{
+  a = foo (a);
+  foo (b);
+  a = foo (a);
+  foo (b);
+  a = foo (a);
+  foo (b);
+}
+
+void test_4 (st a)
+{
+  a = bar (&a);
+}
+
+st test_4a (void)
+{
+  st a;
+  a = bar (&a);
+  return a;
+}
+
+void test_5 (st a)
+{
+  st b;
+  a = bar (&a);
+  b = b; /* { dg-warning "use of uninitialized value 'b'" } */
+}
+
+st test_6 (st a)
+{
+  st b;
+  a = bar (&b);
+  b = b;
+  return b;
+}
+
+void test_6a (st a)
+{
+  st b;
+  a = bar (&b);
+  b = b;
+}
+
+st test_7 (st a)
+{
+  st b;
+  b = bar (&a);
+  return b;
+}
+
+void test_7a (st a)
+{
+  st b;
+  b = bar (&a);
+}
+
+st test_8 (void)
+{
+  st b;
+  b = bar (&b);
+  return b;
+}
+
+void test_8a (void)
+{
+  st b;
+  b = bar (&b);
+}
+
+char test_9 (st a)
+{
+  char c;
+  c = baz (a);
+  return c;
+}
+
+char test_10 (st a)
+{
+  char c;
+  a = foo (a);
+  c = baz (a);
+  return c;
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/uninit-pr108704.c b/gcc/testsuite/gcc.dg/analyzer/uninit-pr108704.c
new file mode 100644
index 00000000000..ebf8151e58f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/uninit-pr108704.c
@@ -0,0 +1,29 @@
+typedef unsigned short int __uint16_t;
+typedef unsigned int __uint32_t;
+typedef unsigned long int __uint64_t;
+typedef __uint16_t uint16_t;
+typedef __uint32_t uint32_t;
+typedef __uint64_t uint64_t;
+
+typedef uint32_t float32;
+typedef struct
+{
+  uint64_t low;
+  uint16_t high;
+} floatx80;
+
+extern floatx80
+float32_to_floatx80(float32);
+
+extern floatx80
+floatx80_add(floatx80, floatx80);
+
+floatx80
+test (floatx80 a)
+{
+  floatx80 fp0;
+
+  fp0 = a;
+  fp0 = floatx80_add(fp0, float32_to_floatx80((0x3F800000))); /* { dg-bogus "use of uninitialized value 'fp0'" } */
+  return fp0;
+}