diff mbox series

[pushed] analyzer: fix overzealous state purging with on-stack structs [PR108704]

Message ID 20230208185021.1281451-1-dmalcolm@redhat.com
State New
Headers show
Series [pushed] analyzer: fix overzealous state purging with on-stack structs [PR108704] | expand

Commit Message

David Malcolm Feb. 8, 2023, 6:50 p.m. UTC
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.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r13-5745-g77bb54b1b07add.

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                   |  15 ++-
 gcc/testsuite/gcc.dg/analyzer/uninit-7.c      | 127 ++++++++++++++++++
 .../gcc.dg/analyzer/uninit-pr108704.c         |  29 ++++
 3 files changed, 170 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/uninit-7.c
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/uninit-pr108704.c
diff mbox series

Patch

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;
+}