[Bug analyzer/105264] New: -Wanalyzer-use-of-uninitialized-value gets confused about var + i v.s. &var[i]

[Bug analyzer/105264] New: -Wanalyzer-use-of-uninitialized-value gets confused about var + i v.s. &var[i]

[avarab at gmail dot com]

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

            Bug ID: 105264
           Summary: -Wanalyzer-use-of-uninitialized-value gets confused
                    about var + i v.s. &var[i]
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: avarab at gmail dot com
  Target Milestone: ---

After reading
https://developers.redhat.com/articles/2022/04/12/state-static-analysis-gcc-12-compiler
I tried out -fanalyzer on GCC 12 (built from c1ff207af66 (ppc: testsuite: skip
pr60203 on no ldbl128, 2022-04-12)) against git.git, and discover what seems to
be a bug.

When compiling git (https://github.com/git/git/) as:

    $ make CC=gcc CFLAGS=-fanalyzer builtin/merge-file.o

It will complain about:

builtin/merge-file.c:86:28: error: use of uninitialized value ‘mmfs[i].size’
[CWE-457] [-Werror=analyzer-use-of-uninitialized-value]
   86 |                 if (mmfs[i].size > MAX_XDIFF_SIZE ||
[...]

The basic control flow is:

mmfile_t mmfs[3];
[...]
for-loop
[...]
ret = read_mmfile(mmfs + i, fname);
[...]

Where read_mmfile() function is always either returning -1 or populating
mmfs[i] structure, in the case of -1 we can't reach the code -fanalyzer raises
an issue about.

The warning will go away if I apply:

diff --git a/builtin/merge-file.c b/builtin/merge-file.c
index e695867ee54..0ca3580b27d 100644
--- a/builtin/merge-file.c
+++ b/builtin/merge-file.c
@@ -77,7 +77,7 @@ int cmd_merge_file(int argc, const char **argv, const char
*prefix)
                        names[i] = argv[i];

                fname = prefix_filename(prefix, argv[i]);
-               ret = read_mmfile(mmfs + i, fname);
+               ret = read_mmfile(&mmfs[i], fname);
                free(fname);
                if (ret)
                        return -1;


Which to me suggests a bug in the analyzer, that's not the most obvious code in
the world and probably could use that patch in any case, but the analyzer
should understand that mmfs+i and &mmfs[i] yield the same pointer.



analyzer-use-of-uninitialized-value

[Bug analyzer/105264] -Wanalyzer-use-of-uninitialized-value gets confused about var + i v.s. &var[i]

[dmalcolm at gcc dot gnu.org]

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

--- Comment #1 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Thanks for filing this bug.  I suspect the analyzer is getting confused about
the loop index on successive iterations (and state relating to this).

Please can you:
(a) specify exactly which compilation flags you use to reproduce the false
positive, and
(b) attach the preprocessed source.  You can get this via the -E option to gcc.

Thanks!

[Bug analyzer/105264] -Wanalyzer-use-of-uninitialized-value gets confused about var + i v.s. &var[i]

[avarab at gmail dot com]

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

--- Comment #2 from Ævar Arnfjörð Bjarmason <avarab at gmail dot com> ---
I think I can do one better. Here's a stand-alone reproducible test case
without any headers except standard headers, I've expanded the gcc -E version
of that too, but presumably you won't need it.

It's screaming bloddy murder about various other things, since I did a rather
bad hatchet job of copy/pasting various structs I needed from local headers
into the testcase, and deleted some code I could do away with, but as noted in
the now-added comment in the code:

1. "mmfs + i" to "&mmfs[i]" makes it go away, so if it's a loop-iterator
problem wouldn't that apply to both?

2. I tried copying the relevant function into the file and that makes it go
away too, so some cross-file analysis limitation?

Will follow-up and attach both the testcase & the gcc -E version, tested with:

gcc -o testcase -fanalyzer testcase.c

[Bug analyzer/105264] -Wanalyzer-use-of-uninitialized-value gets confused about var + i v.s. &var[i]

[avarab at gmail dot com]

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

Ævar Arnfjörð Bjarmason <avarab at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |avarab at gmail dot com

--- Comment #3 from Ævar Arnfjörð Bjarmason <avarab at gmail dot com> ---
Created attachment 52805
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52805&action=edit
testcase.c

A stand-alone testcase of builtin/merge-file.c from git.git

[Bug analyzer/105264] -Wanalyzer-use-of-uninitialized-value gets confused about var + i v.s. &var[i]

[avarab at gmail dot com]

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

--- Comment #4 from Ævar Arnfjörð Bjarmason <avarab at gmail dot com> ---
Created attachment 52806
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52806&action=edit
testcase-full.c (gcc -E of testcase.c)

The gcc -E version of testcase.c, probably useless since it only uses headers
from glibc, but just in case...

[Bug analyzer/105264] -Wanalyzer-use-of-uninitialized-value gets confused about var + i v.s. &var[i]

[dmalcolm at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2022-04-14

--- Comment #5 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
(In reply to Ævar Arnfjörð Bjarmason from comment #2)
> I think I can do one better. Here's a stand-alone reproducible test case
> without any headers except standard headers, I've expanded the gcc -E
> version of that too, but presumably you won't need it.

Thanks - I'm poking at -fanalyzer on both attachments now.

FWIW, the -E version can sometimes be very helpful, for the case where we have
different headers (e.g. glibc sometimes adds attributes to decls, which can
affect things)

[Bug analyzer/105264] -Wanalyzer-use-of-uninitialized-value gets confused about var + i v.s. &var[i]

[dmalcolm at gcc dot gnu.org]

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

--- Comment #6 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
There are some fiddly issues where the analyzer fails to figure out that ptr +
i and &ptr[i] refer to the same memory, for certain symbolic values of i.

I'm testing a partial fix for GCC 12, which at least seems to fix the
-Wanalyzer-use-of-uninitialized-value false positives identified in the
reproducer.  That said, I think for a deeper fix it's probably best to wait
until GCC 13.

[Bug analyzer/105264] -Wanalyzer-use-of-uninitialized-value gets confused about var + i v.s. &var[i]

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 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:a358e4b60815b41e27f3508014ceb592f86b9b45

commit r12-8169-ga358e4b60815b41e27f3508014ceb592f86b9b45
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Thu Apr 14 09:52:00 2022 -0400

    analyzer: fix escaping of pointer arithmetic [PR105264]

    PR analyzer/105264 reports that the analyzer can fail to treat
    (PTR + IDX) and PTR[IDX] as referring to the same memory under
    some situations.

    There are various ways in which this can happen when IDX is a
    symbolic value, due to having several ways in which such memory
    regions can be referred to symbolically.  I attempted to fix this by
    being smarter when folding svalues and regions, but this fix
    seems too fiddly to attempt in stage 4.

    Instead, this less ambitious patch fixes a false positive from
    -Wanalyzer-use-of-uninitialized-value by making the analyzer's escape
    analysis smarter, so that it treats *PTR as escaping when
    (PTR + OFFSET) is passed to an external function, and thus
    it treats *PTR as possibly-initialized (the "passing &PTR[IDX]" case
    was already working).

    gcc/analyzer/ChangeLog:
            PR analyzer/105264
            * region-model-reachability.cc (reachable_regions::handle_parm):
            Use maybe_get_deref_base_region rather than just region_svalue, to
            handle pointer arithmetic also.
            * svalue.cc (svalue::maybe_get_deref_base_region): New.
            * svalue.h (svalue::maybe_get_deref_base_region): New decl.

    gcc/testsuite/ChangeLog:
            PR analyzer/105264
            * gcc.dg/analyzer/torture/symbolic-10.c: New test.

    Signed-off-by: David Malcolm <dmalcolm@redhat.com>

[Bug analyzer/105264] -Wanalyzer-use-of-uninitialized-value gets confused about var + i v.s. &var[i]

[dmalcolm at gcc dot gnu.org]

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

--- Comment #8 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
The above patch hopefully fixes the false positive you're seeing, but as noted,
there are some deeper issues that it doesn't fix; keeping this bug open.

[Bug analyzer/105264] -Wanalyzer-use-of-uninitialized-value gets confused about var + i v.s. &var[i]

[avarab at gmail dot com]

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

--- Comment #9 from Ævar Arnfjörð Bjarmason <avarab at gmail dot com> ---
Thanks a lot, I can confirm that this fixes the issue in builtin/merge-file.c
in git.git.