[Bug analyzer/107851] New: Issues with -Wanalyzer-allocation-size messages

[Bug analyzer/107851] New: Issues with -Wanalyzer-allocation-size messages

[dmalcolm at gcc dot gnu.org]

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

            Bug ID: 107851
           Summary: Issues with -Wanalyzer-allocation-size messages
           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: ---

Taken from gcc.dg/analyzer/allocation-size-2.c


#include <stdlib.h>
#include <stdint.h>

void test_2 (int32_t n)
{
  int32_t *ptr = malloc (n * sizeof (int16_t));
  free (ptr);
}

With "-fanalyzer" (https://godbolt.org/z/fKcdrrh3z) we get:

<source>: In function 'test_2':
<source>:6:18: warning: allocated buffer size is not a multiple of the
pointee's size [CWE-131] [-Wanalyzer-allocation-size]
    6 |   int32_t *ptr = malloc (n * sizeof (int16_t));
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  'test_2': event 1
    |
    |    6 |   int32_t *ptr = malloc (n * sizeof (int16_t));
    |      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                  |
    |      |                  (1) allocated '(long unsigned int)n * 2' bytes
and assigned to 'int32_t *' {aka 'int *'} here; 'sizeof (int32_t {aka int})' is
'4'
    |
<source>:6:18: warning: allocated buffer size is not a multiple of the
pointee's size [CWE-131] [-Wanalyzer-allocation-size]
  'test_2': events 1-3
    |
    |    6 |   int32_t *ptr = malloc (n * sizeof (int16_t));
    |      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                  |
    |      |                  (1) allocated '(long unsigned int)n * 2' bytes
here
    |      |                  (2) allocated '(long unsigned int)n * 2' bytes
here
    |      |                  (3) assigned to 'int32_t *' {aka 'int *'} here;
'sizeof (int32_t {aka int})' is '4'
    |
Compiler returned: 0

With "-fanalyzer -fanalyzer-fine-grained" ( https://godbolt.org/z/3fbvofPje )
we get:
<source>: In function 'test_2':
<source>:6:18: warning: allocated buffer size is not a multiple of the
pointee's size [CWE-131] [-Wanalyzer-allocation-size]
    6 |   int32_t *ptr = malloc (n * sizeof (int16_t));
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  'test_2': events 1-3
    |
    |    6 |   int32_t *ptr = malloc (n * sizeof (int16_t));
    |      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                  |
    |      |                  (1) allocated '(long unsigned int)n * 2' bytes
here
    |      |                  (2) allocated '(long unsigned int)n * 2' bytes
here
    |      |                  (3) assigned to 'int32_t *' {aka 'int *'} here;
'sizeof (int32_t {aka int})' is '4'
    |
Compiler returned: 0

Issues:
* note how the "allocated '(long unsigned int)n * 2' bytes here" message is
repeated
* note how we get a duplicate diagnostic, which goes away with
-fanalyzer-fine-grained

[Bug analyzer/107851] Issues with -Wanalyzer-allocation-size messages

[cvs-commit at gcc dot gnu.org]

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

--- Comment #1 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:f5758fe5b430ef3447fbab947fcea32a1d995f36

commit r13-4471-gf5758fe5b430ef3447fbab947fcea32a1d995f36
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Fri Dec 2 16:30:51 2022 -0500

    analyzer: fixes to region creation messages [PR107851]

    In r13-2573-gc81b60b8c6ff3d I split up the analyzer's region-creation
    events to describe the memory space and capacity of the region as two
    separate events to avoid combinatorial explosion of message wordings.

    However I didn't take into account r13-1405-ge6c3bb379f515b which
    added a pending_diagnostic::describe_region_creation_event vfunc which
    could change the wording of region creation events.

    Hence for:

    #include <stdlib.h>
    #include <stdint.h>

    void test ()
    {
      int32_t *ptr = malloc (1);
      free (ptr);
    }

    trunk currently emits:

      Compiler Explorer (x86_64 trunk): https://godbolt.org/z/e3Td7c9s5:

    <source>: In function 'test':
    <source>:6:18: warning: allocated buffer size is not a multiple of the
pointee's size [CWE-131] [-Wanalyzer-allocation-size]
        6 |   int32_t *ptr = malloc (1);
          |                  ^~~~~~~~~~
      'test': events 1-3
        |
        |    6 |   int32_t *ptr = malloc (1);
        |      |                  ^~~~~~~~~~
        |      |                  |
        |      |                  (1) allocated 1 bytes here
        |      |                  (2) allocated 1 bytes here
        |      |                  (3) assigned to 'int32_t *' {aka 'int *'}
here; 'sizeof (int32_t {aka int})' is '4'
        |

    where events (1) and (2) are different region_creation_events that have
    had their wording overridden (also, with a "1 bytes" issue).

    This patch reorganizes region creation events so that each
    pending_diagnostic instead creates the events that is appropriate for it,
    and the events have responsibility for their own wording.

    With this patch, the above emits:

    <source>: In function 'test':
    <source>:6:18: warning: allocated buffer size is not a multiple of the
pointee's size [CWE-131] [-Wanalyzer-allocation-size]
        6 |   int32_t *ptr = malloc (1);
          |                  ^~~~~~~~~~
      'test': events 1-2
        |
        |    6 |   int32_t *ptr = malloc (1);
        |      |                  ^~~~~~~~~~
        |      |                  |
        |      |                  (1) allocated 1 byte here
        |      |                  (2) assigned to 'int32_t *' {aka 'int *'}
here; 'sizeof (int32_t {aka int})' is '4'
        |

    fixing the duplicate event, and fixing the singular/plural issue.

    gcc/analyzer/ChangeLog:
            PR analyzer/107851
            * analyzer.cc (make_label_text_n): Convert param "n" from int to
            unsigned HOST_WIDE_INT.
            * analyzer.h (make_label_text_n): Likewise for decl.
            * bounds-checking.cc: Include "analyzer/checker-event.h" and
            "analyzer/checker-path.h".
            (out_of_bounds::add_region_creation_events): New.
            (concrete_past_the_end::describe_region_creation_event): Replace
            with...
            (concrete_past_the_end::add_region_creation_events): ...this.
            (symbolic_past_the_end::describe_region_creation_event): Delete.
            * checker-event.cc (region_creation_event::region_creation_event):
            Update for dropping all member data.
            (region_creation_event::get_desc): Delete, splitting out into
            region_creation_event_memory_space::get_desc,
            region_creation_event_capacity::get_desc, and
            region_creation_event_debug::get_desc.
            (region_creation_event_memory_space::get_desc): New.
            (region_creation_event_capacity::get_desc): New.
            (region_creation_event_allocation_size::get_desc): New.
            (region_creation_event_debug::get_desc): New.
            * checker-event.h: Include "analyzer/program-state.h".
            (enum rce_kind): Delete.
            (class region_creation_event): Drop all member data.
            (region_creation_event::region_creation_event): Make protected.
            (region_creation_event::get_desc): Delete.
            (class region_creation_event_memory_space): New.
            (class region_creation_event_capacity): New.
            (class region_creation_event_allocation_size): New.
            (class region_creation_event_debug): New.
            * checker-path.cc (checker_path::add_region_creation_events): Add
            "pd" param.  Call pending_diangnostic::add_region_creation_events.
            Update for conversion of RCE_DEBUG to region_creation_event_debug.
            * checker-path.h (checker_path::add_region_creation_events): Add
            "pd" param.
            * diagnostic-manager.cc (diagnostic_manager::build_emission_path):
            Pass pending_diagnostic to
            emission_path::add_region_creation_events.
            (diagnostic_manager::build_emission_path): Pass path_builder to
            add_event_on_final_node.
            (diagnostic_manager::add_event_on_final_node): Add "pb" param.
            Pass pending_diagnostic to
            emission_path::add_region_creation_events.
            (diagnostic_manager::add_events_for_eedge): Pass
            pending_diagnostic to emission_path::add_region_creation_events.
            * diagnostic-manager.h
            (diagnostic_manager::add_event_on_final_node): Add "pb" param.
            * pending-diagnostic.cc
            (pending_diagnostic::add_region_creation_events): New.
            * pending-diagnostic.h (struct region_creation): Delete.
            (pending_diagnostic::describe_region_creation_event): Delete.
            (pending_diagnostic::add_region_creation_events): New vfunc.
            * region-model.cc: Include "analyzer/checker-event.h" and
            "analyzer/checker-path.h".
            (dubious_allocation_size::dubious_allocation_size): Initialize
            m_has_allocation_event.
            (dubious_allocation_size::describe_region_creation_event): Delete.
            (dubious_allocation_size::describe_final_event): Update for
            replacement of m_allocation_event with m_has_allocation_event.
            (dubious_allocation_size::add_region_creation_events): New.
            (dubious_allocation_size::m_allocation_event): Replace with...
            (dubious_allocation_size::m_has_allocation_event): ...this.

    gcc/testsuite/ChangeLog:
            PR analyzer/107851
            * gcc.dg/analyzer/allocation-size-4.c: Update expected wording.
            * gcc.dg/analyzer/allocation-size-multiline-1.c: New test.
            * gcc.dg/analyzer/allocation-size-multiline-2.c: New test.
            * gcc.dg/analyzer/out-of-bounds-multiline-1.c: Update expected
            wording.
            * gcc.dg/analyzer/out-of-bounds-multiline-2.c: New test.
            * gcc.dg/analyzer/out-of-bounds-read-char-arr.c: Update expected
            wording.
            * gcc.dg/analyzer/out-of-bounds-read-int-arr.c: Likewise.
            * gcc.dg/analyzer/out-of-bounds-write-char-arr.c: Likewise.
            * gcc.dg/analyzer/out-of-bounds-write-int-arr.c: Likewise.

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

[Bug analyzer/107851] Issues with -Wanalyzer-allocation-size messages

[dmalcolm at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-12-02

--- Comment #2 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
The above patch fixes the repeated events, but doesn't fix the repeated
diagnostics.

Keeping open to track fixing the latter; see:
  gcc/testsuite/gcc.dg/analyzer/allocation-size-multiline-2.c