analyzer: remove -Wanalyzer-use-of-uninitialized-value for GCC 10

From what I can tell -Wanalyzer-use-of-uninitialized-value has not yet found a true diagnostic in real-world code, and seems to be particularly susceptible to false positives. These relate to bugs in the region_model code. For GCC 10 it seems best to remove this warning, which this patch does. Internally it also removes POISON_KIND_UNINIT. I'm working on a rewrite of the region_model code for GCC 11 that I hope will fix these issues, and allow this warning to be reintroduced. gcc/analyzer/ChangeLog: PR analyzer/94447 PR analyzer/94639 PR analyzer/94732 PR analyzer/94754 * analyzer.opt (Wanalyzer-use-of-uninitialized-value): Delete. * program-state.cc (selftest::test_program_state_dumping): Update expected dump result for removal of "uninit". * region-model.cc (poison_kind_to_str): Delete POISON_KIND_UNINIT case. (root_region::ensure_stack_region): Initialize stack with null svalue_id rather than with a typeless POISON_KIND_UNINIT value. (root_region::ensure_heap_region): Likewise for the heap. (region_model::dump_summary_of_rep_path_vars): Remove summarization of uninit values. (region_model::validate): Remove check that the stack has a POISON_KIND_UNINIT value. (poisoned_value_diagnostic::emit): Remove POISON_KIND_UNINIT case. (poisoned_value_diagnostic::describe_final_event): Likewise. (selftest::test_dump): Update expected dump result for removal of "uninit". (selftest::test_svalue_equality): Remove "uninit" and "freed". * region-model.h (enum poison_kind): Remove POISON_KIND_UNINIT. gcc/ChangeLog: PR analyzer/94447 PR analyzer/94639 PR analyzer/94732 PR analyzer/94754 * doc/invoke.texi (Static Analyzer Options): Remove -Wanalyzer-use-of-uninitialized-value. (-Wno-analyzer-use-of-uninitialized-value): Remove item. gcc/testsuite/ChangeLog: PR analyzer/94447 PR analyzer/94639 PR analyzer/94732 PR analyzer/94754 * gcc.dg/analyzer/data-model-1.c: Mark "use of uninitialized value" warnings as xfail for now. * gcc.dg/analyzer/data-model-5b.c: Remove uninitialized warning. * gcc.dg/analyzer/pr94099.c: Mark "uninitialized" warning as xfail for now. * gcc.dg/analyzer/pr94447.c: New test. * gcc.dg/analyzer/pr94639.c: New test. * gcc.dg/analyzer/pr94732.c: New test. * gcc.dg/analyzer/pr94754.c: New test. * gcc.dg/analyzer/zlib-6.c: Mark "uninitialized" warning as xfail for now.
2020-04-23 21:31:22 -04:00 · 2020-04-23 21:31:22 -04:00 · 78b9783774
commit 78b9783774
parent 5cbf892543
16 changed files with 129 additions and 99 deletions
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@ -1,3 +1,13 @@
+2020-04-28  David Malcolm  <dmalcolm@redhat.com>
+
+	PR analyzer/94447
+	PR analyzer/94639
+	PR analyzer/94732
+	PR analyzer/94754
+	* doc/invoke.texi (Static Analyzer Options): Remove
+	-Wanalyzer-use-of-uninitialized-value.
+	(-Wno-analyzer-use-of-uninitialized-value): Remove item.
+
 2020-04-28  Jakub Jelinek  <jakub@redhat.com>

 	PR tree-optimization/94809
--- a/gcc/analyzer/ChangeLog
+++ b/gcc/analyzer/ChangeLog
@ -1,3 +1,29 @@
+2020-04-28  David Malcolm  <dmalcolm@redhat.com>
+
+	PR analyzer/94447
+	PR analyzer/94639
+	PR analyzer/94732
+	PR analyzer/94754
+	* analyzer.opt (Wanalyzer-use-of-uninitialized-value): Delete.
+	* program-state.cc (selftest::test_program_state_dumping): Update
+	expected dump result for removal of "uninit".
+	* region-model.cc (poison_kind_to_str): Delete POISON_KIND_UNINIT
+	case.
+	(root_region::ensure_stack_region): Initialize stack with null
+	svalue_id rather than with a typeless POISON_KIND_UNINIT value.
+	(root_region::ensure_heap_region): Likewise for the heap.
+	(region_model::dump_summary_of_rep_path_vars): Remove
+	summarization of uninit values.
+	(region_model::validate): Remove check that the stack has a
+	POISON_KIND_UNINIT value.
+	(poisoned_value_diagnostic::emit): Remove POISON_KIND_UNINIT
+	case.
+	(poisoned_value_diagnostic::describe_final_event): Likewise.
+	(selftest::test_dump): Update expected dump result for removal of
+	"uninit".
+	(selftest::test_svalue_equality): Remove "uninit" and "freed".
+	* region-model.h (enum poison_kind): Remove POISON_KIND_UNINIT.
+
 2020-04-01  David Malcolm  <dmalcolm@redhat.com>

 	PR analyzer/94378
--- a/gcc/analyzer/analyzer.opt
+++ b/gcc/analyzer/analyzer.opt
@ -98,10 +98,6 @@ Wanalyzer-use-of-pointer-in-stale-stack-frame
 Common Var(warn_analyzer_use_of_pointer_in_stale_stack_frame) Init(1) Warning
 Warn about code paths in which a pointer to a stale stack frame is used.

-Wanalyzer-use-of-uninitialized-value
-Common Var(warn_analyzer_use_of_uninitialized_value) Init(1) Warning
-Warn about code paths in which an uninitialized value is used.
-
 Wanalyzer-too-complex
 Common Var(warn_analyzer_too_complex) Init(0) Warning
 Warn if the code is too complicated for the analyzer to fully explore.
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@ -1449,23 +1449,21 @@ test_program_state_dumping ()
  ASSERT_DUMP_EQ
    (s, ext_state, false,
     "rmodel: r0: {kind: `root', parent: null, sval: null}\n"
-     "|-heap: r1: {kind: `heap', parent: r0, sval: sv0}\n"
-     "|  |: sval: sv0: {poisoned: uninit}\n"
+     "|-heap: r1: {kind: `heap', parent: r0, sval: null}\n"
     "|  `-r2: {kind: `symbolic', parent: r1, sval: null, possibly_null: true}\n"
     "`-globals: r3: {kind: `globals', parent: r0, sval: null, map: {`p': r4}}\n"
-     "  `-`p': r4: {kind: `primitive', parent: r3, sval: sv1, type: `void *'}\n"
-     "    |: sval: sv1: {type: `void *', &r2}\n"
+     "  `-`p': r4: {kind: `primitive', parent: r3, sval: sv0, type: `void *'}\n"
+     "    |: sval: sv0: {type: `void *', &r2}\n"
     "    |: type: `void *'\n"
     "svalues:\n"
-     "  sv0: {poisoned: uninit}\n"
-     "  sv1: {type: `void *', &r2}\n"
+     "  sv0: {type: `void *', &r2}\n"
     "constraint manager:\n"
     "  equiv classes:\n"
     "  constraints:\n"
-     "malloc: {sv1: unchecked (`p')}\n");
+     "malloc: {sv0: unchecked (`p')}\n");

  ASSERT_DUMP_EQ (s, ext_state, true,
-		  "rmodel: p: &r2 malloc: {sv1: unchecked (`p')}");
+		  "rmodel: p: &r2 malloc: {sv0: unchecked (`p')}");
 }

 /* Verify that program_state::dump_to_pp works for string literals.  */
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@ -788,8 +788,6 @@ poison_kind_to_str (enum poison_kind kind)
    {
    default:
      gcc_unreachable ();
-    case POISON_KIND_UNINIT:
-      return "uninit";
    case POISON_KIND_FREED:
      return "freed";
    case POISON_KIND_POPPED_STACK:
@ -3204,12 +3202,9 @@ root_region::ensure_stack_region (region_model *model)
 {
  if (m_stack_rid.null_p ())
    {
-      svalue_id uninit_sid
-	= model->add_svalue (new poisoned_svalue (POISON_KIND_UNINIT,
-						  NULL_TREE));
      m_stack_rid
 	= model->add_region (new stack_region (model->get_root_rid (),
-					       uninit_sid));
+					       svalue_id::null ()));
    }
  return m_stack_rid;
 }
@ -3270,12 +3265,9 @@ root_region::ensure_heap_region (region_model *model)
 {
  if (m_heap_rid.null_p ())
    {
-      svalue_id uninit_sid
-	= model->add_svalue (new poisoned_svalue (POISON_KIND_UNINIT,
-						  NULL_TREE));
      m_heap_rid
 	= model->add_region (new heap_region (model->get_root_rid (),
-					      uninit_sid));
+					      svalue_id::null ()));
    }
  return m_heap_rid;
 }
@ -3859,7 +3851,6 @@ region_model::dump_summary_of_rep_path_vars (pretty_printer *pp,
  unsigned i;
  path_var *pv;
  auto_vec<tree> unknown_trees;
-  auto_vec<tree> uninit_trees;
  FOR_EACH_VEC_ELT (*rep_path_vars, i, pv)
    {
      if (TREE_CODE (pv->m_tree) == STRING_CST)
@ -3908,14 +3899,9 @@ region_model::dump_summary_of_rep_path_vars (pretty_printer *pp,
 	  {
 	    poisoned_svalue *poisoned_sval = as_a <poisoned_svalue *> (sval);
 	    enum poison_kind pkind = poisoned_sval->get_poison_kind ();
-	    if (pkind == POISON_KIND_UNINIT)
-	      uninit_trees.safe_push (pv->m_tree);
-	    else
-	      {
-		dump_separator (pp, is_first);
-		dump_tree (pp, pv->m_tree);
-		pp_printf (pp, ": %s", poison_kind_to_str (pkind));
-	      }
+	    dump_separator (pp, is_first);
+	    dump_tree (pp, pv->m_tree);
+	    pp_printf (pp, ": %s", poison_kind_to_str (pkind));
 	  }
 	  break;
 	case SK_SETJMP:
@ -3928,7 +3914,6 @@ region_model::dump_summary_of_rep_path_vars (pretty_printer *pp,

  /* Print unknown and uninitialized values in consolidated form.  */
  dump_vec_of_tree (pp, is_first, unknown_trees, "unknown");
-  dump_vec_of_tree (pp, is_first, uninit_trees, "uninit");
 }

 /* Assert that this object is valid.  */
@ -3949,18 +3934,6 @@ region_model::validate () const
    r->validate (*this);

  // TODO: anything else?
-
-  /* Verify that the stack region (if any) has an "uninitialized" value.  */
-  region *stack_region = get_root_region ()->get_stack_region (this);
-  if (stack_region)
-    {
-      svalue_id stack_value_sid = stack_region->get_value_direct ();
-      svalue *stack_value = get_svalue (stack_value_sid);
-      gcc_assert (stack_value->get_kind () == SK_POISONED);
-      poisoned_svalue *subclass = stack_value->dyn_cast_poisoned_svalue ();
-      gcc_assert (subclass);
-      gcc_assert (subclass->get_poison_kind () == POISON_KIND_UNINIT);
-    }
 }

 /* Global data for use by svalue_id_cmp_by_constant_svalue.  */
@ -4087,16 +4060,6 @@ public:
      {
      default:
 	gcc_unreachable ();
-      case POISON_KIND_UNINIT:
-	{
-	  diagnostic_metadata m;
-	  m.add_cwe (457); /* "CWE-457: Use of Uninitialized Variable".  */
-	  return warning_meta (rich_loc, m,
-			       OPT_Wanalyzer_use_of_uninitialized_value,
-			       "use of uninitialized value %qE",
-			       m_expr);
-	}
-	break;
      case POISON_KIND_FREED:
 	{
 	  diagnostic_metadata m;
@ -4125,9 +4088,6 @@ public:
      {
      default:
 	gcc_unreachable ();
-      case POISON_KIND_UNINIT:
-	return ev.formatted_print ("use of uninitialized value %qE here",
-				   m_expr);
      case POISON_KIND_FREED:
 	return ev.formatted_print ("use after %<free%> of %qE here",
 				   m_expr);
@ -7578,14 +7538,10 @@ test_dump ()

  ASSERT_DUMP_EQ (model, false,
 		  "r0: {kind: `root', parent: null, sval: null}\n"
-		  "|-stack: r1: {kind: `stack', parent: r0, sval: sv0}\n"
-		  "|  |: sval: sv0: {poisoned: uninit}\n"
+		  "|-stack: r1: {kind: `stack', parent: r0, sval: null}\n"
 		  "|-globals: r2: {kind: `globals', parent: r0, sval: null, map: {}}\n"
-		  "`-heap: r3: {kind: `heap', parent: r0, sval: sv1}\n"
-		  "  |: sval: sv1: {poisoned: uninit}\n"
+		  "`-heap: r3: {kind: `heap', parent: r0, sval: null}\n"
 		  "svalues:\n"
-		  "  sv0: {poisoned: uninit}\n"
-		  "  sv1: {poisoned: uninit}\n"
 		  "constraint manager:\n"
 		  "  equiv classes:\n"
 		  "  constraints:\n");
@ -7798,15 +7754,6 @@ test_svalue_equality ()
  ASSERT_NE (cst_int_42->hash (), cst_int_0->hash ());
  ASSERT_NE (*cst_int_42, *cst_int_0);

-  svalue *uninit = new poisoned_svalue (POISON_KIND_UNINIT, NULL_TREE);
-  svalue *freed = new poisoned_svalue (POISON_KIND_FREED, NULL_TREE);
-
-  ASSERT_EQ (uninit->hash (), uninit->hash ());
-  ASSERT_EQ (*uninit, *uninit);
-
-  ASSERT_NE (uninit->hash (), freed->hash ());
-  ASSERT_NE (*uninit, *freed);
-
  svalue *unknown_0 = new unknown_svalue (ptr_type_node);
  svalue *unknown_1 = new unknown_svalue (ptr_type_node);
  ASSERT_EQ (unknown_0->hash (), unknown_0->hash ());
@ -7815,24 +7762,16 @@ test_svalue_equality ()

  /* Comparisons between different kinds of svalue.  */
  ASSERT_NE (*ptr_to_r0, *cst_int_42);
-  ASSERT_NE (*ptr_to_r0, *uninit);
  ASSERT_NE (*ptr_to_r0, *unknown_0);
  ASSERT_NE (*cst_int_42, *ptr_to_r0);
-  ASSERT_NE (*cst_int_42, *uninit);
  ASSERT_NE (*cst_int_42, *unknown_0);
-  ASSERT_NE (*uninit, *ptr_to_r0);
-  ASSERT_NE (*uninit, *cst_int_42);
-  ASSERT_NE (*uninit, *unknown_0);
  ASSERT_NE (*unknown_0, *ptr_to_r0);
  ASSERT_NE (*unknown_0, *cst_int_42);
-  ASSERT_NE (*unknown_0, *uninit);

  delete ptr_to_r0;
  delete ptr_to_r1;
  delete cst_int_42;
  delete cst_int_0;
-  delete uninit;
-  delete freed;
  delete unknown_0;
  delete unknown_1;
 }
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@ -684,9 +684,6 @@ public:

 enum poison_kind
 {
-  /* For use to describe uninitialized memory.  */
-  POISON_KIND_UNINIT,
-
  /* For use to describe freed memory.  */
  POISON_KIND_FREED,

--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@ -8256,7 +8256,6 @@ Enabling this option effectively enables the following warnings:
 -Wanalyzer-tainted-array-index @gol
 -Wanalyzer-unsafe-call-within-signal-handler @gol
 -Wanalyzer-use-after-free @gol
-Wanalyzer-use-of-uninitialized-value @gol
 -Wanalyzer-use-of-pointer-in-stale-stack-frame @gol
 }

@ -8429,15 +8428,6 @@ to disable it.
 This diagnostic warns for paths through the code in which a pointer
 is dereferenced that points to a variable in a stale stack frame.

-@item -Wno-analyzer-use-of-uninitialized-value
-@opindex Wanalyzer-use-of-uninitialized-value
-@opindex Wno-analyzer-use-of-uninitialized-value
-This warning requires @option{-fanalyzer}, which enables it; use
-@option{-Wno-analyzer-use-of-uninitialized-value} to disable it.
-
-This diagnostic warns for paths through the code in which an uninitialized
-value is used.
-
@end table

 Pertinent parameters for controlling the exploration are:
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@ -1,3 +1,21 @@
+2020-04-28  David Malcolm  <dmalcolm@redhat.com>
+
+	PR analyzer/94447
+	PR analyzer/94639
+	PR analyzer/94732
+	PR analyzer/94754
+	* gcc.dg/analyzer/data-model-1.c: Mark "use of uninitialized
+	value" warnings as xfail for now.
+	* gcc.dg/analyzer/data-model-5b.c: Remove uninitialized warning.
+	* gcc.dg/analyzer/pr94099.c: Mark "uninitialized" warning as xfail
+	for now.
+	* gcc.dg/analyzer/pr94447.c: New test.
+	* gcc.dg/analyzer/pr94639.c: New test.
+	* gcc.dg/analyzer/pr94732.c: New test.
+	* gcc.dg/analyzer/pr94754.c: New test.
+	* gcc.dg/analyzer/zlib-6.c: Mark "uninitialized" warning as xfail
+	for now.
+
 2020-04-28  Jakub Jelinek  <jakub@redhat.com>

 	PR tree-optimization/94809
--- a/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
@ -849,7 +849,7 @@ void test_36 (int i)
 int test_37 (void)
 {
  int *ptr;
-  return *ptr; /* { dg-warning "use of uninitialized value 'ptr'" } */
+  return *ptr; /* { dg-warning "use of uninitialized value 'ptr'" "uninit-warning-removed" { xfail *-*-* } } */
 }

 /* Write through uninitialized pointer.  */
@ -857,7 +857,7 @@ int test_37 (void)
 void test_37a (int i)
 {
  int *ptr;
-  *ptr = i; /* { dg-warning "use of uninitialized value 'ptr'" } */
+  *ptr = i; /* { dg-warning "use of uninitialized value 'ptr'" "uninit-warning-removed" { xfail *-*-* } } */
 }

 // TODO: the various other ptr deref poisonings
--- a/gcc/testsuite/gcc.dg/analyzer/data-model-5b.c
+++ b/gcc/testsuite/gcc.dg/analyzer/data-model-5b.c
@ -76,8 +76,7 @@ void unref (string_obj *obj)
  if (--obj->str_base.ob_refcnt == 0)
    {
      //__analyzer_dump();
-      obj->str_base.ob_type->tp_dealloc ((base_obj *)obj); /* { dg-bogus "use of uninitialized value '<unknown>'" "" { xfail *-*-* } } */
-      // TODO (xfail): not sure what's going on here
+      obj->str_base.ob_type->tp_dealloc ((base_obj *)obj);
    }
 }

--- a/gcc/testsuite/gcc.dg/analyzer/pr94099.c
+++ b/gcc/testsuite/gcc.dg/analyzer/pr94099.c
@ -21,7 +21,7 @@ pl (void)
  for (sc = 0; sc < 1; ++sc)
    {
      th.gk.hk = 0;
-      th.gk.bg[sc] = 0; /* { dg-warning "uninitialized" } */
+      th.gk.bg[sc] = 0; /* { dg-warning "uninitialized" "uninit-warning-removed" { xfail *-*-* } } */
      l3 (&th);
    }
 }
--- a/gcc/testsuite/gcc.dg/analyzer/pr94447.c
+++ b/gcc/testsuite/gcc.dg/analyzer/pr94447.c
@ -0,0 +1,10 @@
+struct foo
+{
+  int *v;
+};
+
+int test (void)
+{
+  struct foo f = {};
+  return *f.v;
+}
--- a/gcc/testsuite/gcc.dg/analyzer/pr94639.c
+++ b/gcc/testsuite/gcc.dg/analyzer/pr94639.c
@ -0,0 +1,14 @@
+#include <string.h>
+
+void validatedatetime(const char *str)
+{
+  const char *templates[] = {"dddd-dd-dd dd:dd", "dddd-dd-dd"};
+
+  size_t len = strlen(str);
+
+  for (unsigned t = 0; t < 2; t++) {
+    if (len != strlen(templates[t])) {
+      continue;
+    }
+  }
+}
--- a/gcc/testsuite/gcc.dg/analyzer/pr94732.c
+++ b/gcc/testsuite/gcc.dg/analyzer/pr94732.c
@ -0,0 +1,13 @@
+typedef struct { int *a; } S;
+int *f (void);
+static void g (S *x)
+{
+  int *p = x->a;
+  p[0] = 0;
+}
+void h (void)
+{
+  S x[1];
+  x->a = f ();
+  g (x);
+}
--- a/gcc/testsuite/gcc.dg/analyzer/pr94754.c
+++ b/gcc/testsuite/gcc.dg/analyzer/pr94754.c
@ -0,0 +1,20 @@
+[[gnu::nonnull]]
+static
+void init_x(int cond, int **x, int *y)
+{
+  if (!cond)
+    return;
+  *x = y;
+}
+
+int foo(int cond)
+{
+  int *x;
+  int y = 7;
+
+  if (cond < 2)
+    return -1;
+  init_x(cond, &x, &y);
+
+  return *x;
+}
--- a/gcc/testsuite/gcc.dg/analyzer/zlib-6.c
+++ b/gcc/testsuite/gcc.dg/analyzer/zlib-6.c
@ -41,7 +41,7 @@ int inflate_blocks(inflate_blocks_statef *s, z_stream *z, int r) {
        return inflate_flush(s, z, r);
      }
    };
-    b |= ((uLong)(n--, *p++)) << k; /* { dg-warning "use of uninitialized value" } */
+    b |= ((uLong)(n--, *p++)) << k; /* { dg-warning "use of uninitialized value" "uninit-warning-removed" { xfail *-*-* } } */
    k += 8;
  }
 }