analyzer: fix overzealous state purging with on-stack structs [PR108704]
authorDavid Malcolm <dmalcolm@redhat.com>
Wed, 8 Feb 2023 18:47:36 +0000 (13:47 -0500)
committerDavid Malcolm <dmalcolm@redhat.com>
Wed, 8 Feb 2023 18:47:36 +0000 (13:47 -0500)
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>
gcc/analyzer/state-purge.cc
gcc/testsuite/gcc.dg/analyzer/uninit-7.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/analyzer/uninit-pr108704.c [new file with mode: 0644]

index 5fa596d..5f2d1f7 100644 (file)
@@ -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 (file)
index 0000000..cb3e29a
--- /dev/null
@@ -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 (file)
index 0000000..ebf8151
--- /dev/null
@@ -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;
+}