Bug 94713 - Analyzer is buggy on uninitialized pointer
Summary: Analyzer is buggy on uninitialized pointer
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: analyzer (show other bugs)
Version: 10.0
: P3 normal
Target Milestone: ---
Assignee: David Malcolm
URL:
Keywords:
Depends on:
Blocks: analyzer-uninit
  Show dependency treegraph
 
Reported: 2020-04-22 13:06 UTC by Vincent Lefèvre
Modified: 2021-07-15 19:35 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2020-04-23 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Vincent Lefèvre 2020-04-22 13:06:27 UTC
Test with: gcc-10 (Debian 10-20200418-1) 10.0.1 20200418 (experimental) [master revision 27c171775ab:4c277008be0:c5bac7d127f288fd2f8a1f15c3f30da5903141c6]

Consider:

void f1 (int *);
void f2 (int);

int foo (void)
{
  int *p;

  f1 (p);
  f2 (p[0]);
  return 0;
}

zira% gcc-10 -Wall tst2.c -O3 -c -fanalyzer
tst2.c: In function ‘foo’:
tst2.c:8:3: warning: ‘p’ is used uninitialized in this function [-Wuninitialize]
    8 |   f1 (p);
      |   ^~~~~~
tst2.c:9:3: warning: use of uninitialized value ‘p’ [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
    9 |   f2 (p[0]);
      |   ^~~~~~~~~
  ‘foo’: event 1
    |
    |

-Wuninitialize works as expected, but -Wanalyzer-use-of-uninitialized-value outputs the warning message on p[0], though the ‘p’ in the warning message is correct.

If I comment out the "f2 (p[0]);" line, I no longer get the warning from the analyzer, which means that it is the p[0] that triggers the warning instead of p.
Comment 1 Dmitry G. Dyachenko 2021-02-11 15:06:14 UTC
gcc version 11.0.0 20210210 (experimental) [master revision bd0e37f68a3:deed5164277:72932511053596091ad291539022b51d9f2ba418]

PASS for me
Comment 2 Vincent Lefèvre 2021-02-12 08:37:34 UTC
This also works with: gcc-10 (Debian 10.2.1-6) 10.2.1 20210110
Comment 3 GCC Commits 2021-07-15 19:09:16 UTC
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:

https://gcc.gnu.org/g:33255ad3ac14e3953750fe0f2d82b901c2852ff6

commit r12-2337-g33255ad3ac14e3953750fe0f2d82b901c2852ff6
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Thu Jul 15 15:07:07 2021 -0400

    analyzer: reimplement -Wanalyzer-use-of-uninitialized-value [PR95006 et al]
    
    The initial gcc 10 era commit of the analyzer (in
    757bf1dff5e8cee34c0a75d06140ca972bfecfa7) had an implementation of
    -Wanalyzer-use-of-uninitialized-value, but was sufficiently buggy
    that I removed it in 78b9783774bfd3540f38f5b1e3c7fc9f719653d7 before
    the release of gcc 10.1
    
    This patch reintroduces the warning, heavily rewritten, with (I hope)
    a less buggy implementation this time, for GCC 12.
    
    gcc/analyzer/ChangeLog:
            PR analyzer/95006
            PR analyzer/94713
            PR analyzer/94714
            * analyzer.cc (maybe_reconstruct_from_def_stmt): Split out
            GIMPLE_ASSIGN case into...
            (get_diagnostic_tree_for_gassign_1): New.
            (get_diagnostic_tree_for_gassign): New.
            * analyzer.h (get_diagnostic_tree_for_gassign): New decl.
            * analyzer.opt (Wanalyzer-write-to-string-literal): New.
            * constraint-manager.cc (class svalue_purger): New.
            (constraint_manager::purge_state_involving): New.
            * constraint-manager.h
            (constraint_manager::purge_state_involving): New.
            * diagnostic-manager.cc (saved_diagnostic::supercedes_p): New.
            (dedupe_winners::handle_interactions): New.
            (diagnostic_manager::emit_saved_diagnostics): Call it.
            * diagnostic-manager.h (saved_diagnostic::supercedes_p): New decl.
            * engine.cc (impl_region_model_context::warn): Convert return type
            to bool.  Return false if the diagnostic isn't saved.
            (impl_region_model_context::purge_state_involving): New.
            (impl_sm_context::get_state): Use NULL ctxt when querying old
            rvalue.
            (impl_sm_context::set_next_state): Use new sval when querying old
            state.
            (class dump_path_diagnostic): Move to region-model.cc
            (exploded_node::on_stmt): Move to on_stmt_pre and on_stmt_post.
            Remove call to purge_state_involving.
            (exploded_node::on_stmt_pre): New, based on the above.  Move most
            of it to region_model::on_stmt_pre.
            (exploded_node::on_stmt_post): Likewise, moving to
            region_model::on_stmt_post.
            (class stale_jmp_buf): Fix parent class to use curiously recurring
            template pattern.
            (feasibility_state::maybe_update_for_edge): Call on_call_pre and
            on_call_post on gcalls.
            * exploded-graph.h (impl_region_model_context::warn): Return bool.
            (impl_region_model_context::purge_state_involving): New decl.
            (exploded_node::on_stmt_pre): New decl.
            (exploded_node::on_stmt_post): New decl.
            * pending-diagnostic.h (pending_diagnostic::use_of_uninit_p): New.
            (pending_diagnostic::supercedes_p): New.
            * program-state.cc (sm_state_map::get_state): Inherit state for
            conjured_svalue as well as initial_svalue.
            (sm_state_map::purge_state_involving): Also support SK_CONJURED.
            * region-model-impl-calls.cc (call_details::get_uncertainty):
            Handle m_ctxt being NULL.
            (call_details::get_or_create_conjured_svalue): New.
            (region_model::impl_call_fgets): New.
            (region_model::impl_call_fread): New.
            * region-model-manager.cc
            (region_model_manager::get_or_create_initial_value): Return an
            uninitialized poisoned value for regions that can't have initial
            values.
            * region-model-reachability.cc
            (reachable_regions::mark_escaped_clusters): Handle ctxt being
            NULL.
            * region-model.cc (region_to_value_map::purge_state_involving): New.
            (poisoned_value_diagnostic::use_of_uninit_p): New.
            (poisoned_value_diagnostic::emit): Handle POISON_KIND_UNINIT.
            (poisoned_value_diagnostic::describe_final_event): Likewise.
            (region_model::check_for_poison): New.
            (region_model::on_assignment): Call it.
            (class dump_path_diagnostic): Move here from engine.cc.
            (region_model::on_stmt_pre): New, based on exploded_node::on_stmt.
            (region_model::on_call_pre): Move the setting of the LHS to a
            conjured svalue to before the checks for specific functions.
            Handle "fgets", "fgets_unlocked", and "fread".
            (region_model::purge_state_involving): New.
            (region_model::handle_unrecognized_call): Handle ctxt being NULL.
            (region_model::get_rvalue): Call check_for_poison.
            (selftest::test_stack_frames): Use NULL for context when getting
            uninitialized rvalue.
            (selftest::test_alloca): Likewise.
            * region-model.h (region_to_value_map::purge_state_involving): New
            decl.
            (call_details::get_or_create_conjured_svalue): New decl.
            (region_model::on_stmt_pre): New decl.
            (region_model::purge_state_involving): New decl.
            (region_model::impl_call_fgets): New decl.
            (region_model::impl_call_fread): New decl.
            (region_model::check_for_poison): New decl.
            (region_model_context::warn): Return bool.
            (region_model_context::purge_state_involving): New.
            (noop_region_model_context::warn): Return bool.
            (noop_region_model_context::purge_state_involving): New.
            (test_region_model_context:: warn): Return bool.
            * region.cc (region::get_memory_space): New.
            (region::can_have_initial_svalue_p): New.
            (region::involves_p): New.
            * region.h (enum memory_space): New.
            (region::get_memory_space): New decl.
            (region::can_have_initial_svalue_p): New decl.
            (region::involves_p): New decl.
            * sm-malloc.cc (use_after_free::supercedes_p): New.
            * store.cc (binding_cluster::purge_state_involving): New.
            (store::purge_state_involving): New.
            * store.h (class symbolic_binding): New forward decl.
            (binding_key::dyn_cast_symbolic_binding): New.
            (symbolic_binding::dyn_cast_symbolic_binding): New.
            (binding_cluster::purge_state_involving): New.
            (store::purge_state_involving): New.
            * svalue.cc (svalue::can_merge_p): Reject attempts to merge
            poisoned svalues with other svalues, so that we identify
            paths in which a variable is conditionally uninitialized.
            (involvement_visitor::visit_conjured_svalue): New.
            (svalue::involves_p): Also handle SK_CONJURED.
            (poison_kind_to_str): Handle POISON_KIND_UNINIT.
            (poisoned_svalue::maybe_fold_bits_within): New.
            * svalue.h (enum poison_kind): Add POISON_KIND_UNINIT.
            (poisoned_svalue::maybe_fold_bits_within): New decl.
    
    gcc/ChangeLog:
            PR analyzer/95006
            PR analyzer/94713
            PR analyzer/94714
            * doc/invoke.texi: Add -Wanalyzer-use-of-uninitialized-value.
    
    gcc/testsuite/ChangeLog:
            PR analyzer/95006
            PR analyzer/94713
            PR analyzer/94714
            * g++.dg/analyzer/pr93212.C: Update location of warning.
            * g++.dg/analyzer/pr94011.C: Add
            -Wno-analyzer-use-of-uninitialized-value.
            * g++.dg/analyzer/pr94503.C: Likewise.
            * gcc.dg/analyzer/clobbers-1.c: Convert "f" from a local to a
            param to avoid uninitialized warning.
            * gcc.dg/analyzer/data-model-1.c (test_12): Add test for
            uninitialized value on result of alloca.
            (test_12a): Add expected warning.
            (test_12c): Likewise.
            (test_19): Likewise.
            (test_29b): Likewise.
            (test_29c): Likewise.
            (test_37): Remove xfail.
            (test_37a): Likewise.
            * gcc.dg/analyzer/data-model-20.c: Add warning about leak.
            * gcc.dg/analyzer/explode-2.c: Remove params; add
            -Wno-analyzer-too-complex, -Wno-analyzer-malloc-leak, and xfails.
            Initialize the locals.
            * gcc.dg/analyzer/explode-2a.c: Initialize the locals.  Add
            expected leak.
            * gcc.dg/analyzer/fgets-1.c: New test.
            * gcc.dg/analyzer/fread-1.c: New test.
            * gcc.dg/analyzer/malloc-1.c (test_16): Add expected warning.
            (test_40): Likewise.
            * gcc.dg/analyzer/memset-CVE-2017-18549-1.c: Check for
            uninitialized padding.
            * gcc.dg/analyzer/pr93355-localealias-feasibility.c (fread): New
            decl.
            (read_alias_file): Call it.
            * gcc.dg/analyzer/pr94047.c: Add expected warnings.
            * gcc.dg/analyzer/pr94851-2.c: Likewise.
            * gcc.dg/analyzer/pr96841.c: Convert local to a param.
            * gcc.dg/analyzer/pr98628.c: Likewise.
            * gcc.dg/analyzer/pr99042.c: Updated expected location of leak
            diagnostics.
            * gcc.dg/analyzer/symbolic-1.c: Add expected warnings.
            * gcc.dg/analyzer/symbolic-7.c: Likewise.
            * gcc.dg/analyzer/torture/pr93649.c: Add expected warning.  Skip
            with -fno-fat-lto-objects.
            * gcc.dg/analyzer/uninit-1.c: New test.
            * gcc.dg/analyzer/uninit-2.c: New test.
            * gcc.dg/analyzer/uninit-3.c: New test.
            * gcc.dg/analyzer/uninit-4.c: New test.
            * gcc.dg/analyzer/uninit-pr94713.c: New test.
            * gcc.dg/analyzer/uninit-pr94714.c: New test.
            * gcc.dg/analyzer/use-after-free-2.c: New test.
            * gcc.dg/analyzer/use-after-free-3.c: New test.
            * gcc.dg/analyzer/zlib-3.c: Add expected warning.
            * gcc.dg/analyzer/zlib-6.c: Convert locals to params to avoid
            uninitialized warnings.  Remove xfail.
            * gcc.dg/analyzer/zlib-6a.c: New test, based on the old version
            of the above.
            * gfortran.dg/analyzer/pr97668.f: Add
            -Wno-analyzer-use-of-uninitialized-value and
            -Wno-analyzer-too-complex.
    
    Signed-off-by: David Malcolm <dmalcolm@redhat.com>
Comment 4 David Malcolm 2021-07-15 19:35:00 UTC
Should be fixed on trunk for gcc 12 by the above patch.