[Bug analyzer/109014] New: -Wanalyzer-use-of-uninitialized-value seen in pcre2-10.42's pcre2test.c

[Bug analyzer/109014] New: -Wanalyzer-use-of-uninitialized-value seen in pcre2-10.42's pcre2test.c

[dmalcolm at gcc dot gnu.org]

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

            Bug ID: 109014
           Summary: -Wanalyzer-use-of-uninitialized-value seen in
                    pcre2-10.42's pcre2test.c
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: dmalcolm at gcc dot gnu.org
  Target Milestone: ---

Created attachment 54579
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54579&action=edit
Partially reducer reproducer

I'm about to commit a patch that provides a prototype implementation of
sprintf; this leads to a new false positive on the attached:


pcre2test.c: In function 'format_list_item':
pcre2test.c:4448:21: warning: use of uninitialized value '*ff_44(D) + _2'
[CWE-457] [-Wanalyzer-use-of-uninitialized-value]
 4448 |   for (count = 0; ff[count] >= 0; count++) {
      |                   ~~^~~~~~~
  'main': events 1-12
    |
    | 4565 | main(int argc, char** argv)
    |      | ^~~~
    |      | |
    |      | (1) entry to 'main'
    |......
    | 4579 |   while (argc > 1 && argv[op][0] == '-' && argv[op][1] != 0) {
    |      |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                                         |
    |      |                                         (2) following 'true'
branch...
    |      |                                         (8) following 'true'
branch...
    | 4580 |     /* [...snip...] */
    | 4581 |     char* arg = argv[op];
    |      |                     ~
    |      |                     |
    |      |                     (3) ...to here
    |      |                     (9) ...to here
    |......
    | 4585 |     if (strcmp(arg, "-LP") == 0) {
    |      |        ~
    |      |        |
    |      |        (4) following 'false' branch (when the strings are
non-equal)...
    |      |        (10) following 'true' branch (when the strings are
equal)...
    | 4586 |       display_properties(0);
    |      |       ~~~~~~~~~~~~~~~~~~~~~
    |      |       |
    |      |       (11) ...to here
    |      |       (12) calling 'display_properties' from 'main'
    |......
    | 4592 |     if (strcmp(arg, "-8") == 0) {
    |      |        ~~~~~~~~~~~~~~~~~~
    |      |        ||
    |      |        |(5) ...to here
    |      |        (6) following 'true' branch (when the strings are equal)...
    | 4593 |       test_mode = 8;
    |      |       ~~~~~~~~~~~~~
    |      |                 |
    |      |                 (7) ...to here
    |
    +--> 'display_properties': events 13-25
           |
           | 4481 | display_properties(BOOL wantscripts)
           |      | ^~~~~~~~~~~~~~~~~~
           |      | |
           |      | (13) entry to 'display_properties'
           |......
           | 4487 |   int16_t found[256][5 + 1];
           |      |           ~~~~~
           |      |           |
           |      |           (14) region created on stack here
           |......
           | 4492 |   if (wantscripts) {
           |      |      ~
           |      |      |
           |      |      (15) following 'false' branch (when 'wantscripts ==
0')...
           |......
           | 4496 |     n = ucp_Bprop_Count;
           |      |     ~~~~~~~~~~~~~~~~~~~
           |      |       |
           |      |       (16) ...to here
           |......
           | 4500 |   for (size_t i = 0; i < utt_size; i++) {
           |      |                      ~~~~~~~~~~~~
           |      |                        |
           |      |                        (17) following 'true' branch...
           | 4501 |     int k;
           | 4502 |     int m = 0;
           |      |         ~
           |      |         |
           |      |         (18) ...to here
           |......
           | 4507 |     if (wantscripts) {
           |      |        ~
           |      |        |
           |      |        (19) following 'false' branch (when 'wantscripts ==
0')...
           |......
           | 4511 |       if (t->type != 13)
           |      |          ~~~~~~~~
           |      |          | |
           |      |          | (20) ...to here
           |      |          (21) following 'true' branch...
           |      |          (22) ...to here
           |......
           | 4544 |   for (int k = 0; k < (n + 1) / 2; k++) {
           |      |                   ~~~~~~~~~~~~~~~
           |      |                     |
           |      |                     (23) following 'true' branch...
           |......
           | 4549 |     format_list_item(found[k], buff1, wantscripts);
           |      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |     |                     |
           |      |     |                     (24) ...to here
           |      |     (25) calling 'format_list_item' from
'display_properties'
           |
           +--> 'format_list_item': events 26-27
                  |
                  | 4441 | format_list_item(int16_t* ff, char* buff, BOOL
isscript)
                  |      | ^~~~~~~~~~~~~~~~
                  |      | |
                  |      | (26) entry to 'format_list_item'
                  |......
                  | 4448 |   for (count = 0; ff[count] >= 0; count++) {
                  |      |                   ~~~~~~~~~
                  |      |                     |
                  |      |                     (27) use of uninitialized value
'*ff_44(D) + _2' here
                  |

[Bug analyzer/109014] -Wanalyzer-use-of-uninitialized-value seen in pcre2-10.42's pcre2test.c

[dmalcolm at gcc dot gnu.org]

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

--- Comment #1 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
I believe the issue here is that:

* display_properties partially initializes the "found" buffer, writing a -1
terminator at the end of the initialized part at:

    fv[m] = -1;

* display_properties then calls format_list_item, which tries to find the
terminator with:

 for (count = 0; ff[count] >= 0; count++) {

* -fanalyzer isn't smart enough to know that a -1 terminator has been written,
and simulates iterating past the end of the initialized region

and this is currently masked by the "sprintf" call, which in the absence of a
known_function implementation is currently assumed to potentially write to fv.

[Bug analyzer/109014] -Wanalyzer-use-of-uninitialized-value seen in pcre2-10.42's pcre2test.c

[cvs-commit at gcc dot gnu.org]

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

--- 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:56572a08ec4a0fc1a7802d3737cd7f7cc9089c4b

commit r13-6466-g56572a08ec4a0fc1a7802d3737cd7f7cc9089c4b
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Fri Mar 3 17:59:21 2023 -0500

    analyzer: provide placeholder implementation of sprintf

    Previously, the analyzer lacked a known_function implementation of
    sprintf, and thus would handle calls to sprintf with the "anything could
    happen" fallback.

    Whilst working on PR analyzer/107565 I noticed that this was preventing
    a lot of genuine memory leaks from being reported for Doom; fixing
    thusly.

    Integration testing of the effect of the patch shows a big increase in
    true positives due to the case mentioned in Doom, and one new false
    positive (in pcre2), which I'm tracking as PR analyzer/109014.

    Comparison:
      GOOD:  67 -> 123 (+56); 10.91% -> 18.33%
       BAD: 547 -> 548 (+1)

    where the affected warnings/projects are:

      -Wanalyzer-malloc-leak:
        GOOD:  0 -> 56 (+56);  0.00% -> 41.48%
         BAD: 79
          True positives: 0 -> 56 (+56)
            (all in Doom)

      -Wanalyzer-use-of-uninitialized-value:
        GOOD: 0;  0.00%
         BAD: 80 -> 81 (+1)
          False positives:
            pcre2-10.42: 0 -> 1 (+1)

    gcc/analyzer/ChangeLog:
            * kf.cc (class kf_sprintf): New.
            (register_known_functions): Register it.

    gcc/testsuite/ChangeLog:
            * gcc.dg/analyzer/doom-d_main-IdentifyVersion.c: New test.
            * gcc.dg/analyzer/sprintf-1.c: New test.
            * gcc.dg/analyzer/sprintf-concat.c: New test.

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

[Bug analyzer/109014] [13/14 Regression] -Wanalyzer-use-of-uninitialized-value seen in pcre2-10.42's pcre2test.c

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.3

[Bug analyzer/109014] [13/14 Regression] -Wanalyzer-use-of-uninitialized-value seen in pcre2-10.42's pcre2test.c

[law at gcc dot gnu.org]

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

Jeffrey A. Law <law at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |law at gcc dot gnu.org
           Priority|P3                          |P2

[Bug analyzer/109014] [13/14/15 Regression] -Wanalyzer-use-of-uninitialized-value seen in pcre2-10.42's pcre2test.c

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|13.3                        |13.4

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 13.3 is being released, retargeting bugs to GCC 13.4.