[Bug analyzer/96651] New: -fanalyzer switch

[Bug analyzer/96651] New: -fanalyzer switch

[uso.cosmo.ray at gmail dot com]

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

            Bug ID: 96651
           Summary: -fanalyzer switch
           Product: gcc
           Version: 10.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: uso.cosmo.ray at gmail dot com
  Target Milestone: ---

When compiling this code with -fanalyzer:

static int a;

int main(void)
{
        char *src = NULL;
        char buf[128];

        switch (a) {
        case 1:
                strcpy(buf, src);
                break;
        case 0:
                strcpy(buf, "hello");
        }
        printf("%s\n", buf);
}

GCC seems to think the code can enter case 1 and use strcpy with a NULL value,
but it can't because a is initialize to 0, and isn't touch anywhere.

It also find have the same error if a isn't static.

Note: I've create a small snippet of code that allow to reproduce the error,
I've actually encounter the error here: https://github.com/curl/curl/pull/5815
in sws.c

[Bug analyzer/96651] gcc 10 -fanalyzer fail to track (static) global variable in a switch

[dmalcolm at gcc dot gnu.org]

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

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2020-08-17

--- Comment #1 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Thanks for filing this.

Note that the analyzer is still experimental.

The issue is that the analyzer is treating the values of globals as arbitrary
unknown values, ignoring their initialization values, even for code paths from
the entrypoint of "main".

I'm looking at patching it so that code paths from "main" can assume these
initial values (until a call to code outside the TU occurs on the path), so
that we can assume that a is 0 at the start of main.

[Bug analyzer/96651] gcc 10 -fanalyzer fail to track (static) global variable in a switch

[cvs-commit at gcc dot gnu.org]

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

--- Comment #2 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:623bc0276849d48ada5a7a2e3e94bd79de42c3db

commit r11-2754-g623bc0276849d48ada5a7a2e3e94bd79de42c3db
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Mon Aug 17 16:35:10 2020 -0400

    analyzer: consider initializers for globals [PR96651]

    PR analyzer/96651 reports a false positive in which a global
    that can't have been touched yet is checked in "main".  The analyzer
    fails to reject code paths in which the initial value of the global
    makes the path condition impossible.

    This patch detects cases where the code path begins at the entrypoint
    of "main", and extracts values from initializers for globals that
    can't have been touched yet, rather than using a symbolic
    "INIT_VAL(REG)", fixing the false positive.

    gcc/analyzer/ChangeLog:
            PR analyzer/96651
            * region-model.cc (region_model::called_from_main_p): New.
            (region_model::get_store_value): Move handling for globals into...
            (region_model::get_initial_value_for_global): ...this new
            function, and add logic for extracting values from decl
            initializers.
            * region-model.h (decl_region::get_svalue_for_constructor): New
            decl.
            (decl_region::get_svalue_for_initializer): New decl.
            (region_model::called_from_main_p): New decl.
            (region_model::get_initial_value_for_global): New.
            * region.cc (decl_region::maybe_get_constant_value): Move logic
            for getting an svalue from a CONSTRUCTOR node to...
            (decl_region::get_svalue_for_constructor): ...this new function.
            (decl_region::get_svalue_for_initializer): New.
            * store.cc (get_svalue_for_ctor_val): Rewrite in terms of
            region_model::get_rvalue.
            * store.h (binding_cluster::get_map): New accessor.

    gcc/testsuite/ChangeLog:
            PR analyzer/96651
            * gcc.dg/analyzer/pr96651-1.c: New test.
            * gcc.dg/analyzer/pr96651-2.c: New test.

[Bug analyzer/96651] gcc 10 -fanalyzer fail to track (static) global variable in a switch

[dmalcolm at gcc dot gnu.org]

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

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #3 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Should be fixed by the above patch; marking as resolved.

That said, does this fix the false positives from curl?  In the first example
in https://github.com/curl/curl/pull/5815 I see various function calls on the
path before the "switch", and if those are in a different source file the
analyzer ought to conservatively assume that non-static globals could get
written to.