[Bug analyzer/93355] Missing diagnostic for missing fclose in intl/localealias.c

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93355

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:

https://gcc.gnu.org/g:8a2750086d57d1a2251d9239fa4e6c2dc9ec3a86

commit r11-7029-g8a2750086d57d1a2251d9239fa4e6c2dc9ec3a86
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Mon Feb 1 21:54:11 2021 -0500

    analyzer: directly explore within static functions [PR93355,PR96374]

    PR analyzer/93355 tracks that -fanalyzer fails to report the FILE *
    leak in read_alias_file in intl/localealias.c.

    One reason for the failure is that read_alias_file is marked as
    "static", and the path leading to the single call of
    read_alias_file is falsely rejected as infeasible due to
    PR analyzer/96374.  I have been attempting to fix that bug, but
    don't have a good solution yet.

    Previously, -fanalyzer only directly explored "static" functions
    if they were needed for call summaries, instead forcing them to
    be indirectly explored, but if we have a feasibility bug like
    above, we will fail to report any issues in a function that's
    only called by such a falsely infeasible path.

    It now seems wrong to me to reject directly exploring static
    functions: even if there is currently no way to call a function,
    it seems reasonable to warn about bugs within them, since
    otherwise these latent bugs are a timebomb in the code.

    Hence this patch reworks toplevel_function_p to directly explore
    almost all functions, working around these feasiblity issues.
    It introduces a naming convention that "__analyzer_"-prefixed
    function names don't get directly explored, since this is
    useful in the analyzer's DejaGnu-based tests.

    This workaround gets PR analyzer/93355 closer to working, but
    unfortunately there is a second instance of PR analyzer/96374
    within read_alias_file itself which means even with this patch
    -fanalyzer falsely rejects the path as infeasible.

    Still, this ought to help in other cases, and simplifies the
    implementation.

    gcc/analyzer/ChangeLog:
            PR analyzer/93355
            PR analyzer/96374
            * engine.cc (toplevel_function_p): Simplify so that
            we only reject functions with a "__analyzer_" prefix.
            (add_any_callbacks): Delete.
            (exploded_graph::build_initial_worklist): Update for
            dropped param of toplevel_function_p.
            (exploded_graph::build_initial_worklist): Don't bother
            looking for callbacks that are reachable from global
            initializers.

    gcc/testsuite/ChangeLog:
            PR analyzer/93355
            PR analyzer/96374
            * gcc.dg/analyzer/conditionals-3.c: Add "__analyzer_"
            prefix to support subroutines where necessary.
            * gcc.dg/analyzer/data-model-1.c: Likewise.
            * gcc.dg/analyzer/feasibility-1.c (called_by_test_6a): New.
            (test_6a): New.
            * gcc.dg/analyzer/params.c: Add "__analyzer_" prefix to support
            subroutines where necessary.
            * gcc.dg/analyzer/pr96651-2.c: Likewise.
            * gcc.dg/analyzer/signal-4b.c: Likewise.
            * gcc.dg/analyzer/single-field.c: Likewise.
            * gcc.dg/analyzer/torture/conditionals-2.c: Likewise.