[Bug analyzer/113333] New: analyzer: False positives with calloc()

[Bug analyzer/113333] New: analyzer: False positives with calloc()

[buczek at molgen dot mpg.de]

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

            Bug ID: 113333
           Summary: analyzer: False positives with calloc()
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: buczek at molgen dot mpg.de
  Target Milestone: ---

Analyzer assumen that a pointer allocated by calloc() can be != NULL.


** Code:

#include <stdlib.h>
char **f(void) {
    char **vec = calloc(1, sizeof(char *));
    if (vec)
        for (char **p=vec ; *p ; p++);
    return vec;
}

** Result:

<source>: In function 'f':
<source>:5:29: warning: heap-based buffer over-read [CWE-126]
[-Wanalyzer-out-of-bounds]
    5 |         for (char **p=vec ; *p ; p++);
      |                             ^~
  'f': events 1-6
    |
    |    3 |     char **vec = calloc(1, sizeof(char *));
    |      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                  |
    |      |                  (1) capacity: 8 bytes
    |    4 |     if (vec)
    |      |        ~          
    |      |        |
    |      |        (2) following 'true' branch (when 'vec' is non-NULL)...
    |    5 |         for (char **p=vec ; *p ; p++);
    |      |                     ~       ~~   ~~~
    |      |                     |       |     |
    |      |                     |       |     (5) ...to here
    |      |                     |       (4) following 'true' branch...
    |      |                     |       (6) out-of-bounds read from byte 8
till byte 15 but region ends at byte 8
    |      |                     (3) ...to here
    |
<source>:5:29: note: read of 8 bytes from after the end of the region
    5 |         for (char **p=vec ; *p ; p++);
      |                             ^~
<source>:5:29: warning: use of uninitialized value '*p' [CWE-457]
[-Wanalyzer-use-of-uninitialized-value]
  'f': events 1-6
    |
    |    3 |     char **vec = calloc(1, sizeof(char *));
    |      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                  |
    |      |                  (1) region created on heap here
    |    4 |     if (vec)
    |      |        ~          
    |      |        |
    |      |        (2) following 'true' branch (when 'vec' is non-NULL)...
    |    5 |         for (char **p=vec ; *p ; p++);
    |      |                     ~       ~~   ~~~
    |      |                     |       |     |
    |      |                     |       |     (5) ...to here
    |      |                     |       (4) following 'true' branch...
    |      |                     |       (6) use of uninitialized value '*p'
here
    |      |                     (3) ...to here
    |
Compiler returned: 0

https://gcc.godbolt.org/z/h6bPeYc3T

[Bug analyzer/113333] analyzer: False positives with calloc()

[dmalcolm at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2024-01-11
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1

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

Looking at trunk with:

extern void __analyzer_describe (int verbosity, ...);
extern void __analyzer_eval (int);

#include <stdlib.h>
char **f(void) {
    char **vec = calloc(1, sizeof(char *));
    if (vec)
        {
           char **p=vec;       
          __analyzer_describe (0, p);
          __analyzer_describe (0, *p);
          __analyzer_eval (*p == 0);
        }
    return vec;
}

https://gcc.godbolt.org/z/z3vnxbTaT

source>: In function 'f':
<source>:10:11: warning: svalue: '&HEAP_ALLOCATED_REGION(14)'
   10 |           __analyzer_describe (0, p);
      |           ^~~~~~~~~~~~~~~~~~~~~~~~~~
<source>:11:11: warning: svalue: 'CAST(char *, REPEATED(outer_size: (long
unsigned int)8, inner_val: (char)0))'
   11 |           __analyzer_describe (0, *p);
      |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~
<source>:12:11: warning: UNKNOWN
   12 |           __analyzer_eval (*p == 0);
      |           ^~~~~~~~~~~~~~~~~~~~~~~~~

i.e. the analyzer "sees" that *p is the 0-byte repeated 8 times, cast to a char
*, but doesn't simplify that to just a NULL pointer.

I'm looking at a fix.

[Bug analyzer/113333] analyzer: False positives with calloc()

[cvs-commit at gcc dot gnu.org]

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

--- Comment #2 from GCC 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:d235bf2e807c5f7e959ca5f3f8d92936801f5b80

commit r14-7265-gd235bf2e807c5f7e959ca5f3f8d92936801f5b80
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Mon Jan 15 19:01:16 2024 -0500

    analyzer: casting all zeroes should give all zeroes [PR113333]

    In particular, accessing the result of *calloc (1, SZ) (if non-NULL)
    should be known to be all zeroes.

    gcc/analyzer/ChangeLog:
            PR analyzer/113333
            * region-model-manager.cc
            (region_model_manager::maybe_fold_unaryop): Casting all zeroes
            should give all zeroes.

    gcc/testsuite/ChangeLog:
            PR analyzer/113333
            * c-c++-common/analyzer/calloc-1.c: Add tests.
            * c-c++-common/analyzer/pr96639.c: Update expected results.
            * gcc.dg/analyzer/data-model-9.c: Likewise.

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

[Bug analyzer/113333] analyzer: False positives with calloc()

[dmalcolm at gcc dot gnu.org]

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

--- Comment #3 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Should be fixed on trunk for GCC 14 by the above patch.

Still affects GCC 13 and earlier; keeping open to track backporting.

[Bug analyzer/113333] analyzer: False positives with calloc()

[buczek at molgen dot mpg.de]

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

--- Comment #4 from Donald Buczek <buczek at molgen dot mpg.de> ---
Great, thank you!

I wonder, if the related missed optimization opportunity should also be
reported.

#include <stdlib.h>
int f(void) {
    char **vec = calloc(1, sizeof(char *));
    if (vec) {
        // *vec = NULL;
        if (*vec)
            return 1;
    }
    return 0;
}

This resolves to nothing only if the commented-out line is added.

https://gcc.godbolt.org/z/WTar6zdne

Treat: clang gets it right.

[Bug analyzer/113333] [11/12/13 Regression] analyzer: False positives with calloc()

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.5

[Bug analyzer/113333] [11/12/13 Regression] analyzer: False positives with calloc()

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by David Malcolm
<dmalcolm@gcc.gnu.org>:

https://gcc.gnu.org/g:132eb1a210bc7806c4cf188ecac6c08339c94384

commit r13-8752-g132eb1a210bc7806c4cf188ecac6c08339c94384
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Thu May 9 13:09:29 2024 -0400

    analyzer: casting all zeroes should give all zeroes [PR113333]

    In particular, accessing the result of *calloc (1, SZ) (if non-NULL)
    should be known to be all zeroes.

    (backported from commit r14-7265-gd235bf2e807c5f)

    gcc/analyzer/ChangeLog:
            PR analyzer/113333
            * region-model-manager.cc
            (region_model_manager::maybe_fold_unaryop): Casting all zeroes
            should give all zeroes.

    gcc/testsuite/ChangeLog:
            PR analyzer/113333
            * gcc.dg/analyzer/calloc-1.c: Add tests.
            * gcc.dg/analyzer/data-model-9.c: Update expected results.
            * gcc.dg/analyzer/pr96639.c: Update expected results.

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

[Bug analyzer/113333] [11/12 Regression] analyzer: False positives with calloc()

[dmalcolm at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[11/12/13 Regression]       |[11/12 Regression]
                   |analyzer: False positives   |analyzer: False positives
                   |with calloc()               |with calloc()

--- Comment #6 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Should be fixed for GCC 13 (for the upcoming GCC 13.3) by the above patch.

Keeping open to track backporting to older branches.