From 78b9783774bfd3540f38f5b1e3c7fc9f719653d7 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Thu, 23 Apr 2020 21:31:22 -0400 Subject: [PATCH] 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. --- gcc/ChangeLog | 10 +++ gcc/analyzer/ChangeLog | 26 +++++++ gcc/analyzer/analyzer.opt | 4 - gcc/analyzer/program-state.cc | 14 ++-- gcc/analyzer/region-model.cc | 75 ++----------------- gcc/analyzer/region-model.h | 3 - gcc/doc/invoke.texi | 10 --- gcc/testsuite/ChangeLog | 18 +++++ gcc/testsuite/gcc.dg/analyzer/data-model-1.c | 4 +- gcc/testsuite/gcc.dg/analyzer/data-model-5b.c | 3 +- gcc/testsuite/gcc.dg/analyzer/pr94099.c | 2 +- gcc/testsuite/gcc.dg/analyzer/pr94447.c | 10 +++ gcc/testsuite/gcc.dg/analyzer/pr94639.c | 14 ++++ gcc/testsuite/gcc.dg/analyzer/pr94732.c | 13 ++++ gcc/testsuite/gcc.dg/analyzer/pr94754.c | 20 +++++ gcc/testsuite/gcc.dg/analyzer/zlib-6.c | 2 +- 16 files changed, 129 insertions(+), 99 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr94447.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr94639.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr94732.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr94754.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index e7a4edb6093..dc64c76be48 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,13 @@ +2020-04-28 David Malcolm + + 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 PR tree-optimization/94809 diff --git a/gcc/analyzer/ChangeLog b/gcc/analyzer/ChangeLog index 3f136f45c20..3c8f45883a4 100644 --- a/gcc/analyzer/ChangeLog +++ b/gcc/analyzer/ChangeLog @@ -1,3 +1,29 @@ +2020-04-28 David Malcolm + + 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 PR analyzer/94378 diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt index 22cf4b0ad3b..3133cdd4106 100644 --- 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. diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc index 43396c65827..1a5843be16d 100644 --- 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. */ diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index 30fe72b90e9..22049a34d29 100644 --- 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 unknown_trees; - auto_vec 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 (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 % 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; } diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h index 2c9ee39a38d..ad3dd1d13ef 100644 --- 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, diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index a101928eabb..04b84e3a10e 100644 --- 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: diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index e94c09cef7b..a1ee87e4523 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,21 @@ +2020-04-28 David Malcolm + + 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 PR tree-optimization/94809 diff --git a/gcc/testsuite/gcc.dg/analyzer/data-model-1.c b/gcc/testsuite/gcc.dg/analyzer/data-model-1.c index e2bd1f9cd36..1db99133d50 100644 --- 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 diff --git a/gcc/testsuite/gcc.dg/analyzer/data-model-5b.c b/gcc/testsuite/gcc.dg/analyzer/data-model-5b.c index 11b56719a66..76e351d86cb 100644 --- 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 ''" "" { xfail *-*-* } } */ - // TODO (xfail): not sure what's going on here + obj->str_base.ob_type->tp_dealloc ((base_obj *)obj); } } diff --git a/gcc/testsuite/gcc.dg/analyzer/pr94099.c b/gcc/testsuite/gcc.dg/analyzer/pr94099.c index 0a34f561821..a116c49f105 100644 --- 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); } } diff --git a/gcc/testsuite/gcc.dg/analyzer/pr94447.c b/gcc/testsuite/gcc.dg/analyzer/pr94447.c new file mode 100644 index 00000000000..1aecebba4ef --- /dev/null +++ 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; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/pr94639.c b/gcc/testsuite/gcc.dg/analyzer/pr94639.c new file mode 100644 index 00000000000..2dbb57e39ec --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/pr94639.c @@ -0,0 +1,14 @@ +#include + +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; + } + } +} diff --git a/gcc/testsuite/gcc.dg/analyzer/pr94732.c b/gcc/testsuite/gcc.dg/analyzer/pr94732.c new file mode 100644 index 00000000000..1aa154fc739 --- /dev/null +++ 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); +} diff --git a/gcc/testsuite/gcc.dg/analyzer/pr94754.c b/gcc/testsuite/gcc.dg/analyzer/pr94754.c new file mode 100644 index 00000000000..3fae20c33d0 --- /dev/null +++ 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; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/zlib-6.c b/gcc/testsuite/gcc.dg/analyzer/zlib-6.c index d68c387fa53..0d814c0c096 100644 --- 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; } } -- 2.30.2