[Bug analyzer/106626] New: Improvements to wording of -Wanalyzer-out-of-bounds

[Bug analyzer/106626] New: Improvements to wording of -Wanalyzer-out-of-bounds

[dmalcolm at gcc dot gnu.org]

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

            Bug ID: 106626
           Summary: Improvements to wording of -Wanalyzer-out-of-bounds
           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
                CC: tlange at gcc dot gnu.org
  Target Milestone: ---

During the patch review of -Wanalyzer-out-of-bounds we decided to focus on
getting the feature implemented in trunk first, and defer coming up with the
precise wording, to avoid holding up the feature.

I'm filing this bug to track coming up with the precise wording for
-Wanalyzer-out-of-bounds.

Current status quo:

Given e.g.:

int arr[10];

int
test (void)
{
    return arr[10];
}

https://godbolt.org/z/EPrqGTj8s

we report:

<source>: In function 'test':
<source>:6:15: warning: buffer overread [CWE-126] [-Wanalyzer-out-of-bounds]
    6 |     return arr[10];
      |            ~~~^~~~
  event 1
    |
    |    1 | int arr[10];
    |      |     ^~~
    |      |     |
    |      |     (1) capacity is 40 bytes
    |
    +--> 'test': events 2-3
           |
           |    4 | test (void)
           |      | ^~~~
           |      | |
           |      | (2) entry to 'test'
           |    5 | {
           |    6 |     return arr[10];
           |      |            ~~~~~~~
           |      |               |
           |      |               (3) out-of-bounds read from byte 40 till byte
43 but 'arr' ends at byte 40
           |
<source>:6:15: note: write is 4 bytes past the end of 'arr'
    6 |     return arr[10];
      |            ~~~^~~~

The note erroneously says "write" due to a copy&paste which I plan to fix
shortly.


Goals:

I'd like the diagnostics to (somehow) convey the following information to the
user:

* what was the expression (if available) responsible for the bad access, since
we can't always underline exactly the problematic subexpression in a compound
expression (or macros could be involved, obscuring the user's view of what the
analyzer is "seeing").  e.g. in the above example 'arr[10]'
* direction of the access: read vs write?  e.g. in the above example: read
* boundary being violated: is the access before or after the buffer?  e.g. in
the above example: after the buffer
,* location: where is the invalid access?  heap vs stack vs elsewhere (since
this can affect the impact of a vulnerability): e.g. in the above example:
stack
* magnitude: how far beyond the boundary is the invalid access (consider e.g.
the cases of immediately beyond ("off-by-one"), vs near (a few bytes or
elements), vs far); e.g. in the above example: 0-3 bytes beyond the boundary, 0
elements beyond when expressed as array index
* data size: how much data beyond the boundary is accessed; e.g. in the above
example 4 bytes, or 1 element.

Doing so is likely to avoid a combinatorial explosion (due to the need for
i18n), so to tame this, some of this may need to be split between different
parts of the diagnostic (the initial warning, events in the warning's
diagnostic path, and any notes after the warning).

The above list is taken in part from the the Bug Framework's deprecated Buffer
Overflow (BOF) Class:
  https://samate.nist.gov/BF/Old/BOFClass.html
I'm not sure why the Bug Framework deprecated "BOF" (presumably in favor of the
"BF Memory Model",
  https://samate.nist.gov/BF/Classes/MEM/MEMModel.html
and its "Memory Use Bugs (MUS) Class":
  https://samate.nist.gov/BF/Classes/MEM/MUS.html
), but the BOF attributes seem to me to be pertinent information for the user,
and good things to think about in test cases.

Currently -Wanalyzer-out-of-bounds only warns when the size and offset of the
access are constant (not symbolic), and the capacity of the underlying region
is constant (not symbolic).  I'd like to eventually generalize that (see PR
106625), so ideally whatever scheme we come up with should support that.

Ideally these should be reported in terms of the user's source code.  In the
above example, the messages talk about bytes, but we should probably *also*
talk about array indices (e.g. that indices 0 through 9 are valid, and 10 is
one beyond).

The wording should also be clear about inclusive vs exclusive ranges - I feel
  'arr' ends at byte 40"
is unclear.  Perhaps something like:
  'arr' has 10 elements, so valid indices are '[0]' to '[9]'  (bytes 0-39)
or somesuch.

I'm not sure what a correct solution here is, but am filing this now to try to
capture the things we might try to design for.

Tim: I'm CCing you in case you want to work on this; otherwise I'd like to have
a go at this in a few weeks.

[Bug analyzer/106626] Improvements to wording of -Wanalyzer-out-of-bounds

[cvs-commit at gcc dot gnu.org]

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

--- 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:23e8c0b0d99f585499baddda70af6a8b26f49bea

commit r13-2054-g23e8c0b0d99f585499baddda70af6a8b26f49bea
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Mon Aug 15 14:47:03 2022 -0400

    analyzer: fix direction of -Wanalyzer-out-of-bounds note [PR106626]

    Fix a read/write typo.

    Also, add more test coverage of -Wanalyzer-out-of-bounds to help
    establish a baseline for experiments on tweaking the wording of
    the warning (PR analyzer/106626).

    gcc/analyzer/ChangeLog:
            PR analyzer/106626
            * region-model.cc (buffer_overread::emit): Fix copy&paste error in
            direction of the access in the note.

    gcc/testsuite/ChangeLog:
            PR analyzer/106626
            * gcc.dg/analyzer/out-of-bounds-read-char-arr.c: New test.
            * gcc.dg/analyzer/out-of-bounds-read-int-arr.c: New test.
            * gcc.dg/analyzer/out-of-bounds-write-char-arr.c: New test.
            * gcc.dg/analyzer/out-of-bounds-write-int-arr.c: New test.

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

[Bug analyzer/106626] Improvements to wording of -Wanalyzer-out-of-bounds

[cvs-commit at gcc dot gnu.org]

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

--- 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:d69a95c12cc91ec10d6a8c78f401bf6720b08fce

commit r13-4426-gd69a95c12cc91ec10d6a8c78f401bf6720b08fce
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Wed Nov 30 21:26:42 2022 -0500

    analyzer: fix wording of 'number of bad bytes' note [PR106626]

    Consider -fanalyzer on:

    #include <stdint.h>

    int32_t arr[10];

    void int_arr_write_element_after_end_far(int32_t x)
    {
      arr[100] = x;
    }

    Trunk x86_64: https://godbolt.org/z/7GqEcYGq6

    Currently we emit:

    <source>: In function 'int_arr_write_element_after_end_far':
    <source>:7:12: warning: buffer overflow [CWE-787]
[-Wanalyzer-out-of-bounds]
        7 |   arr[100] = x;
          |   ~~~~~~~~~^~~
      event 1
        |
        |    3 | int32_t arr[10];
        |      |         ^~~
        |      |         |
        |      |         (1) capacity is 40 bytes
        |
        +--> 'int_arr_write_element_after_end_far': events 2-3
               |
               |    5 | void int_arr_write_element_after_end_far(int32_t x)
               |      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
               |      |      |
               |      |      (2) entry to 'int_arr_write_element_after_end_far'
               |    6 | {
               |    7 |   arr[100] = x;
               |      |   ~~~~~~~~~~~~
               |      |            |
               |      |            (3) out-of-bounds write from byte 400 till
byte 403 but 'arr' ends at byte 40
               |
    <source>:7:12: note: write is 4 bytes past the end of 'arr'
        7 |   arr[100] = x;
          |   ~~~~~~~~~^~~

    The wording of the final note:
      "write is 4 bytes past the end of 'arr'"
    reads to me as if the "4 bytes past" is describing where the access
    occurs, which seems wrong, as the write is far beyond the end of the
    array.  Looking at the implementation, it's actually describing the
    number of bytes within the access that are beyond the bounds of the
    buffer.

    This patch updates the wording so that the final note reads
      "write of 4 bytes to beyond the end of 'arr'"
    which more clearly expresses that it's the size of the access
    being described.

    The patch also uses inform_n to avoid emitting "1 bytes".

    gcc/analyzer/ChangeLog:
            PR analyzer/106626
            * bounds-checking.cc (buffer_overflow::emit): Use inform_n.
            Update wording to clarify that we're talking about the size of
            the bad access, rather than its position.
            (buffer_overread::emit): Likewise.

    gcc/testsuite/ChangeLog:
            PR analyzer/106626
            * gcc.dg/analyzer/out-of-bounds-read-char-arr.c: Update for
            changes to 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/106626] Improvements to wording of -Wanalyzer-out-of-bounds

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 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:7c655699ed51b0c987e5472767db48b19044ae05

commit r13-4427-g7c655699ed51b0c987e5472767db48b19044ae05
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Wed Nov 30 21:26:42 2022 -0500

    analyzer: add note about valid subscripts [PR106626]

    Consider -fanalyzer on:

    #include <stdint.h>

    int32_t arr[10];

    void int_arr_write_element_after_end_off_by_one(int32_t x)
    {
      arr[10] = x;
    }

    Trunk x86_64: https://godbolt.org/z/17zn3qYY4

    Currently we emit:

    <source>: In function 'int_arr_write_element_after_end_off_by_one':
    <source>:7:11: warning: buffer overflow [CWE-787]
[-Wanalyzer-out-of-bounds]
        7 |   arr[10] = x;
          |   ~~~~~~~~^~~
      event 1
        |
        |    3 | int32_t arr[10];
        |      |         ^~~
        |      |         |
        |      |         (1) capacity is 40 bytes
        |
        +--> 'int_arr_write_element_after_end_off_by_one': events 2-3
               |
               |    5 | void int_arr_write_element_after_end_off_by_one(int32_t
x)
               |      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
               |      |      |
               |      |      (2) entry to
'int_arr_write_element_after_end_off_by_one'
               |    6 | {
               |    7 |   arr[10] = x;
               |      |   ~~~~~~~~~~~
               |      |           |
               |      |           (3) out-of-bounds write from byte 40 till
byte 43 but 'arr' ends at byte 40
               |
    <source>:7:11: note: write of 4 bytes to beyond the end of 'arr'
        7 |   arr[10] = x;
          |   ~~~~~~~~^~~

    This is worded in terms of bytes, due to the way -Wanalyzer-out-of-bounds
    is implemented, but this isn't what the user wrote.

    This patch tries to get closer to the user's code by adding a note about
    array bounds when we're referring to an array.  In the above example it
    adds this trailing note:

      note: valid subscripts for 'arr' are '[0]' to '[9]'

    gcc/analyzer/ChangeLog:
            PR analyzer/106626
            * bounds-checking.cc (out_of_bounds::maybe_describe_array_bounds):
            New.
            (buffer_overflow::emit): Call maybe_describe_array_bounds.
            (buffer_overread::emit): Likewise.
            (buffer_underflow::emit): Likewise.
            (buffer_underread::emit): Likewise.

    gcc/testsuite/ChangeLog:
            PR analyzer/106626
            * gcc.dg/analyzer/call-summaries-2.c: Add dg-message for expected
            note about valid indexes.
            * gcc.dg/analyzer/out-of-bounds-1.c: Likewise, fixing up existing
            dg-message directives.
            * 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/106626] Improvements to wording of -Wanalyzer-out-of-bounds

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 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:df460cf51b25868564dd2628a399b8c874ef309c

commit r13-4428-gdf460cf51b25868564dd2628a399b8c874ef309c
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Wed Nov 30 21:26:42 2022 -0500

    analyzer: more bounds-checking wording tweaks [PR106626]

    This patch tweaks the wording of -Wanalyzer-out-of-bounds:

    * use the spellings/terminology of CWE:
      * replace "underread" with "under-read", as per:
         https://cwe.mitre.org/data/definitions/127.html
      * replace "overread" with "over-read" as per:
         https://cwe.mitre.org/data/definitions/126.html
      * replace "underflow" with "underwrite" as per:
        https://cwe.mitre.org/data/definitions/124.html

    * wherever known, specify the memory region of the bad access,
    so that it says e.g. "heap-based buffer over-read"
    or "stack-based buffer over-read"

    gcc/analyzer/ChangeLog:
            PR analyzer/106626
            * bounds-checking.cc (out_of_bounds::get_memory_space): New.
            (buffer_overflow::emit): Use it.
            (class buffer_overread): Rename to...
            (class buffer_over_read): ...this.
            (buffer_over_read::emit): Specify which memory space the read is
            from, where known.  Change "overread" to "over-read".
            (class buffer_underflow): Rename to...
            (class buffer_underwrite): ...this.
            (buffer_underwrite::emit): Specify which memory space the write is
            to, where known.  Change "underflow" to "underwrite".
            (class buffer_underread): Rename to...
            (class buffer_under_read): Rename to...
            (buffer_under_read::emit): Specify which memory space the read is
            from, where known.  Change "underread" to "under-read".
            (symbolic_past_the_end::get_memory_space): New.
            (symbolic_buffer_overflow::emit): Use it.
            (class symbolic_buffer_overread): Rename to...
            (class symbolic_buffer_over_read): ...this.
            (symbolic_buffer_over_read::emit): Specify which memory space the
            read is from, where known.  Change "overread" to "over-read".
            (region_model::check_symbolic_bounds): Update for class renaming.
            (region_model::check_region_bounds): Likewise.

    gcc/testsuite/ChangeLog:
            PR analyzer/106626
            * gcc.dg/analyzer/call-summaries-2.c: Update expected results.
            * gcc.dg/analyzer/out-of-bounds-1.c: Likewise.
            * gcc.dg/analyzer/out-of-bounds-2.c: Likewise.
            * gcc.dg/analyzer/out-of-bounds-3.c: Likewise.
            * gcc.dg/analyzer/out-of-bounds-4.c: Likewise.
            * gcc.dg/analyzer/out-of-bounds-5.c: Likewise.
            * gcc.dg/analyzer/out-of-bounds-container_of.c: Likewise.
            * gcc.dg/analyzer/out-of-bounds-read-char-arr.c: Likewise.  Rename
            functions from "int_arr_" to "char_arr_".
            * gcc.dg/analyzer/out-of-bounds-read-int-arr.c: Update expected
            results.
            * gcc.dg/analyzer/out-of-bounds-read-struct-arr.c: New test.
            * gcc.dg/analyzer/out-of-bounds-write-char-arr.c: Update expected
            results.  Rename functions from "int_arr_" to "char_arr_".
            * gcc.dg/analyzer/out-of-bounds-write-int-arr.c: Update expected
            results.
            * gcc.dg/analyzer/out-of-bounds-write-struct-arr.c: New test.
            * gcc.dg/analyzer/pr101962.c: Update expected results.
            * gcc.dg/analyzer/realloc-5.c: Update expected results.
            * gcc.dg/analyzer/zlib-3.c: Update expected results.

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

[Bug analyzer/106626] Improvements to wording of -Wanalyzer-out-of-bounds

[cvs-commit at gcc dot gnu.org]

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

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

commit r13-4429-g1d86af242bc4a8e68aebf1f3b8c985f2d17fa791
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Wed Nov 30 21:26:43 2022 -0500

    diagnostics: tweak diagnostic_path::interprocedural_p [PR106626]

    The region-creation event at the start of...

    <source>: In function 'int_arr_write_element_after_end_off_by_one':
    <source>:14:11: warning: buffer overflow [CWE-787]
[-Wanalyzer-out-of-bounds]
       14 |   arr[10] = x;
          |   ~~~~~~~~^~~
      event 1
        |
        |   10 | int32_t arr[10];
        |      |         ^~~
        |      |         |
        |      |         (1) capacity is 40 bytes
        |
        +--> 'int_arr_write_element_after_end_off_by_one': events 2-3
               |
               |   12 | void int_arr_write_element_after_end_off_by_one(int32_t
x)
               |      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
               |      |      |
               |      |      (2) entry to
'int_arr_write_element_after_end_off_by_one'
               |   13 | {
               |   14 |   arr[10] = x;  /* { dg-line line } */
               |      |   ~~~~~~~~~~~
               |      |           |
               |      |           (3) out-of-bounds write from byte 40 till
byte 43 but 'arr' ends at byte 40
               |
    <source>:14:11: note: write of 4 bytes to beyond the end of 'arr'
       14 |   arr[10] = x;
          |   ~~~~~~~~^~~
    <source>:14:11: note: valid subscripts for 'arr' are '[0]' to '[9]'

    ...makes diagnostic_manager::finish_pruning consider the path to be
    interprocedural, and so it doesn't prune the function entry event.

    This patch tweaks diagnostic_path::interprocedural_p to ignore
    leading events outside of any function, so that it considers the
    path to be intraprocedural, and thus diagnostic_manager::finish_pruning
    prunes the function entry event, leading to this simpler output:

    <source>: In function 'int_arr_write_element_after_end_off_by_one':
    <source>:14:11: warning: buffer overflow [CWE-787]
[-Wanalyzer-out-of-bounds]
       14 |   arr[10] = x;
          |   ~~~~~~~~^~~
      event 1
        |
        |   10 | int32_t arr[10];
        |      |         ^~~
        |      |         |
        |      |         (1) capacity is 40 bytes
        |
        +--> 'int_arr_write_element_after_end_off_by_one': event 2
               |
               |   14 |   arr[10] = x;
               |      |   ~~~~~~~~^~~
               |      |           |
               |      |           (2) out-of-bounds write from byte 40 till
byte 43 but 'arr' ends at byte 40
               |
    <source>:14:11: note: write of 4 bytes to beyond the end of 'arr'
    <source>:14:11: note: valid subscripts for 'arr' are '[0]' to '[9]'

    gcc/ChangeLog:
            PR analyzer/106626
            * diagnostic-path.h
            (diagnostic_path::get_first_event_in_a_function): New decl.
            * diagnostic.cc (diagnostic_path::get_first_event_in_a_function):
            New.
            (diagnostic_path::interprocedural_p): Ignore leading events that
            are outside of any function.

    gcc/testsuite/ChangeLog:
            PR analyzer/106626
            * gcc.dg/analyzer/out-of-bounds-multiline-1.c: New test.

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

[Bug analyzer/106626] Improvements to wording of -Wanalyzer-out-of-bounds

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 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:eaaf97b6147095cc19f7efdefaf55c8ebe7a94e8

commit r13-4431-geaaf97b6147095cc19f7efdefaf55c8ebe7a94e8
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Wed Nov 30 21:26:43 2022 -0500

    analyzer: fix i18n issues in symbolic out-of-bounds [PR106626]

    gcc/analyzer/ChangeLog:
            PR analyzer/106626
            * bounds-checking.cc
            (symbolic_past_the_end::describe_final_event): Delete, moving to
            symbolic_buffer_overflow::describe_final_event and
            symbolic_buffer_over_read::describe_final_event, eliminating
            composition of text strings via "byte_str" and "m_dir_str".
            (symbolic_past_the_end::m_dir_str): Delete field.
            (symbolic_buffer_overflow::symbolic_buffer_overflow): Drop
            m_dir_str.
            (symbolic_buffer_overflow::describe_final_event): New, as noted
            above.
            (symbolic_buffer_over_read::symbolic_buffer_overflow): Drop
            m_dir_str.
            (symbolic_buffer_over_read::describe_final_event): New, as noted
            above.

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

[Bug analyzer/106626] Improvements to wording of -Wanalyzer-out-of-bounds

[dmalcolm at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2023-04-07

--- Comment #7 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
I'm experimenting with some new approaches to this (for GCC 14); marking as
ASSIGNED.

[Bug analyzer/106626] Improvements to wording of -Wanalyzer-out-of-bounds

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 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:0e466e978c728697f18c67c4eace9ba4633f9ef5

commit r14-2029-g0e466e978c728697f18c67c4eace9ba4633f9ef5
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Wed Jun 21 21:49:00 2023 -0400

    analyzer: add text-art visualizations of out-of-bounds accesses [PR106626]

    This patch extends -Wanalyzer-out-of-bounds so that, where possible, it
    will emit a text art diagram visualizing the spatial relationship between
    (a) the memory region that the analyzer predicts would be accessed, versus
    (b) the range of memory that is valid to access - whether they overlap,
    are touching, are close or far apart; which one is before or after in
    memory, the relative sizes involved, the direction of the access (read vs
    write), and, in some cases, the values of data involved.  This diagram
    can be suppressed using -fdiagnostics-text-art-charset=none.

    For example, given:

    int32_t arr[10];

    int32_t int_arr_read_element_before_start_far(void)
    {
      return arr[-100];
    }

    it emits:

    demo-1.c: In function âint_arr_read_element_before_start_farâ:
    demo-1.c:7:13: warning: buffer under-read [CWE-127]
[-Wanalyzer-out-of-bounds]
        7 |   return arr[-100];
          |          ~~~^~~~~~
      âint_arr_read_element_before_start_farâ: event 1
        |
        |    7 |   return arr[-100];
        |      |          ~~~^~~~~~
        |      |             |
        |      |             (1) out-of-bounds read from byte -400 till byte
-397 but âarrâ starts at byte 0
        |
    demo-1.c:7:13: note: valid subscripts for âarrâ are â[0]â to
â[9]â

     
âââââââââââââââââââââââââââââ
      âread of âint32_tâ (4 bytes)â
     
âââââââââââââââââââââââââââââ
                    ^
                    â
                    â
     
âââââââââââââââââââââââââââââ
            
ââââââââââ¬âââââââââ¬ââââââââââ
      â                           â              â  [0]   â  ...   â 
 [9]   â
      â    before valid range     â             
ââââââââââ´âââââââââ´ââââââââââ¤
      â                           â              ââarrâ (type:
âint32_t[10]â)â
     
âââââââââââââââââââââââââââââ
            
âââââââââââââââââââââââââââââ
     
âââââââââââââââ¬ââââââââââââââ¤âââââââ¬âââââââ¤âââââââââââââââ¬ââââââââââââââ¤
                    â                    â                     â
      
â­âââââââââââââ´ââââââââââââ® 
 â­âââââ´âââââ®       
â­ââââââââ´âââââââ®
       ââ ï¸  under-read of 4 bytesâ   â396 bytesâ        âsize: 40
bytesâ
      
â°ââââââââââââââââââââââââ⯠
 â°âââââââââ⯠      
â°âââââââââââââââ¯

    and given:

      #include <string.h>

      void
      test_non_ascii ()
      {
        char buf[5];
        strcpy (buf, "æå­åã");
      }

    it emits:

    demo-2.c: In function âtest_non_asciiâ:
    demo-2.c:7:3: warning: stack-based buffer overflow [CWE-121]
[-Wanalyzer-out-of-bounds]
        7 |   strcpy (buf, "æå­åã");
          |   ^~~~~~~~~~~~~~~~~~~~~~~~
      âtest_non_asciiâ: events 1-2
        |
        |    6 |   char buf[5];
        |      |        ^~~
        |      |        |
        |      |        (1) capacity: 5 bytes
        |    7 |   strcpy (buf, "æå­åã");
        |      |   ~~~~~~~~~~~~~~~~~~~~~~~~
        |      |   |
        |      |   (2) out-of-bounds write from byte 5 till byte 12 but
âbufâ ends at byte 5
        |
    demo-2.c:7:3: note: write of 8 bytes to beyond the end of âbufâ
        7 |   strcpy (buf, "æå­åã");
          |   ^~~~~~~~~~~~~~~~~~~~~~~~
    demo-2.c:7:3: note: valid subscripts for âbufâ are â[0]â to
â[4]â

     
âââââââ¬ââââââ¬ââââââ¬âââââ¬âââââââââââ¬âââââ¬âââââ¬âââââ¬âââââ¬âââââ¬âââââ¬âââââââ
      â [0] â [1] â [2] â[3] â[4] ââ[5] â[6] â[7] â[8]
â[9] â[10]â[11]â [12] â
     
âââââââ¼ââââââ¼ââââââ¼âââââ¼âââââ¤ââââââ¼âââââ¼âââââ¼âââââ¼âââââ¼âââââ¼âââââ¼âââââââ¤
      â0xe6 â0x96 â0x87
â0xe5â0xadââ0x97â0xe5â0x8câ0x96â0xe3â0x81â0x91â 0x00 â
     
âââââââ´ââââââ´ââââââ¼âââââ´âââââ´â´âââââ¼âââââ´âââââ´âââââ¼âââââ´âââââ´âââââ¼âââââââ¤
      â     U+6587      â    U+5b57     â    U+5316    â    U+3051   
âU+0000â
     
âââââââââââââââââââ¼ââââââââââââââââ¼âââââââââââââââ¼âââââââââââââââ¼âââââââ¤
      â       æ        â      å­       â      å      â      ã   
  â NUL  â
     
âââââââââââââââââââ´ââââââââââââââââ´âââââââââââââââ´âââââââââââââââ´âââââââ¤
      â                  string literal (type: âchar[13]â)               
   â
     
ââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââ
         â     â     â    â    â     â    â    â    â    â 
  â    â     â
         â     â     â    â    â     â    â    â    â    â 
  â    â     â
         v     v     v    v    v     v    v    v    v    v    v    v     v
     
âââââââ¬âââââââââââââââââ¬ââââââââââââââââââââââââââââââââââââââââââââââââ
      â [0] â      ...       â[4] ââ                                 
       â
     
âââââââ´âââââââââââââââââ´âââââ¤â
           after valid range            â
      â  âbufâ (type: âchar[5]â)  ââ                             
           â
     
ââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââ
     
âââââââââââââââ¬ââââââââââââââ¤ââââââââââââââââââââââ¬âââââââââââââââââââââ¤
                    â                                   â
           â­âââââââââ´âââââââââ®           
  â­ââââââââââââ´âââââââââââ®
           âcapacity: 5 bytesâ              ââ ï¸  overflow of 8
bytesâ
           â°âââââââââââââââââ⯠          
  â°âââââââââââââââââââââââ¯

    showing that the overflow occurs partway through the UTF-8 encoding of
    the U+5b57 code point.

    There are lots more examples in the test suite.

    It doesn't show up in this email, but the above diagrams are colorized
    to constrast the valid and invalid access ranges.

    gcc/ChangeLog:
            PR analyzer/106626
            * Makefile.in (ANALYZER_OBJS): Add analyzer/access-diagram.o.
            * doc/invoke.texi (Wanalyzer-out-of-bounds): Add description of
            text art.
            (fanalyzer-debug-text-art): New.

    gcc/analyzer/ChangeLog:
            PR analyzer/106626
            * access-diagram.cc: New file.
            * access-diagram.h: New file.
            * analyzer.h (class region_offset): Add default ctor.
            (region_offset::make_byte_offset): New decl.
            (region_offset::concrete_p): New.
            (region_offset::get_concrete_byte_offset): New.
            (region_offset::calc_symbolic_bit_offset): New decl.
            (region_offset::calc_symbolic_byte_offset): New decl.
            (region_offset::dump_to_pp): New decl.
            (region_offset::dump): New decl.
            (operator<, operator<=, operator>, operator>=): New decls for
            region_offset.
            * analyzer.opt
            (-param=analyzer-text-art-string-ellipsis-threshold=): New.
            (-param=analyzer-text-art-string-ellipsis-head-len=): New.
            (-param=analyzer-text-art-string-ellipsis-tail-len=): New.
            (-param=analyzer-text-art-ideal-canvas-width=): New.
            (fanalyzer-debug-text-art): New.
            * bounds-checking.cc: Include "intl.h", "diagnostic-diagram.h",
            and "analyzer/access-diagram.h".
            (class out_of_bounds::oob_region_creation_event_capacity): New.
            (out_of_bounds::out_of_bounds): Add "model" and "sval_hint"
            params.
            (out_of_bounds::mark_interesting_stuff): Use the base region.
            (out_of_bounds::add_region_creation_events): Use
            oob_region_creation_event_capacity.
            (out_of_bounds::get_dir): New pure vfunc.
            (out_of_bounds::maybe_show_notes): New.
            (out_of_bounds::maybe_show_diagram): New.
            (out_of_bounds::make_access_diagram): New.
            (out_of_bounds::m_model): New field.
            (out_of_bounds::m_sval_hint): New field.
            (out_of_bounds::m_region_creation_event_id): New field.
            (concrete_out_of_bounds::concrete_out_of_bounds): Update for new
            fields.
            (concrete_past_the_end::concrete_past_the_end): Likewise.
            (concrete_past_the_end::add_region_creation_events): Use
            oob_region_creation_event_capacity.
            (concrete_buffer_overflow::concrete_buffer_overflow): Update for
            new fields.
            (concrete_buffer_overflow::emit): Replace call to
            maybe_describe_array_bounds with maybe_show_notes.
            (concrete_buffer_overflow::get_dir): New.
            (concrete_buffer_over_read::concrete_buffer_over_read): Update for
            new fields.
            (concrete_buffer_over_read::emit): Replace call to
            maybe_describe_array_bounds with maybe_show_notes.
            (concrete_buffer_overflow::get_dir): New.
            (concrete_buffer_underwrite::concrete_buffer_underwrite): Update
            for new fields.
            (concrete_buffer_underwrite::emit): Replace call to
            maybe_describe_array_bounds with maybe_show_notes.
            (concrete_buffer_underwrite::get_dir): New.
            (concrete_buffer_under_read::concrete_buffer_under_read): Update
            for new fields.
            (concrete_buffer_under_read::emit): Replace call to
            maybe_describe_array_bounds with maybe_show_notes.
            (concrete_buffer_under_read::get_dir): New.
            (symbolic_past_the_end::symbolic_past_the_end): Update for new
            fields.
            (symbolic_buffer_overflow::symbolic_buffer_overflow): Likewise.
            (symbolic_buffer_overflow::emit): Call maybe_show_notes.
            (symbolic_buffer_overflow::get_dir): New.
            (symbolic_buffer_over_read::symbolic_buffer_over_read): Update for
            new fields.
            (symbolic_buffer_over_read::emit): Call maybe_show_notes.
            (symbolic_buffer_over_read::get_dir): New.
            (region_model::check_symbolic_bounds): Add "sval_hint" param.  Pass
            it and sized_offset_reg to diagnostics.
            (region_model::check_region_bounds): Add "sval_hint" param, passing
            it to diagnostics.
            * diagnostic-manager.cc
            (diagnostic_manager::emit_saved_diagnostic): Pass logger to
            pending_diagnostic::emit.
            * engine.cc: Add logger param to pending_diagnostic::emit
            implementations.
            * infinite-recursion.cc: Likewise.
            * kf-analyzer.cc: Likewise.
            * kf.cc: Likewise.  Add nullptr for new param of
            check_region_for_write.
            * pending-diagnostic.h: Likewise in decl.
            * region-model-manager.cc
            (region_model_manager::get_or_create_int_cst): Convert param from
            poly_int64 to const poly_wide_int_ref &.
            (region_model_manager::maybe_fold_binop): Support type being NULL
            when checking for floating-point types.
            Check for (X + Y) - X => Y.  Be less strict about types when
folding
            associative ops.  Check for (X + Y) * CST => (X * CST) + (Y * CST).
            * region-model-manager.h
            (region_model_manager::get_or_create_int_cst): Convert param from
            poly_int64 to const poly_wide_int_ref &.
            * region-model.cc: Add logger param to pending_diagnostic::emit
            implementations.
            (region_model::check_external_function_for_access_attr): Update
            for new param of check_region_for_write.
            (region_model::deref_rvalue): Use nullptr rather than NULL.
            (region_model::get_capacity): Handle RK_STRING.
            (region_model::check_region_access): Add "sval_hint" param; pass it
to
            check_region_bounds.
            (region_model::check_region_for_write): Add "sval_hint" param;
            pass it to check_region_access.
            (region_model::check_region_for_read): Add NULL for new param to
            check_region_access.
            (region_model::set_value): Pass rhs_sval to
            check_region_for_write.
            (region_model::get_representative_path_var_1): Handle SK_CONSTANT
            in the check for infinite recursion.
            * region-model.h (region_model::check_region_for_write): Add
            "sval_hint" param.
            (region_model::check_region_access): Likewise.
            (region_model::check_symbolic_bounds): Likewise.
            (region_model::check_region_bounds): Likewise.
            * region.cc (region_offset::make_byte_offset): New.
            (region_offset::calc_symbolic_bit_offset): New.
            (region_offset::calc_symbolic_byte_offset): New.
            (region_offset::dump_to_pp): New.
            (region_offset::dump): New.
            (struct linear_op): New.
            (operator<, operator<=, operator>, operator>=): New, for
            region_offset.
            (region::get_next_offset): New.
            (region::get_relative_symbolic_offset): Use ptrdiff_type_node.
            (field_region::get_relative_symbolic_offset): Likewise.
            (element_region::get_relative_symbolic_offset): Likewise.
            (bit_range_region::get_relative_symbolic_offset): Likewise.
            * region.h (region::get_next_offset): New decl.
            * sm-fd.cc: Add logger param to pending_diagnostic::emit
            implementations.
            * sm-file.cc: Likewise.
            * sm-malloc.cc: Likewise.
            * sm-pattern-test.cc: Likewise.
            * sm-sensitive.cc: Likewise.
            * sm-signal.cc: Likewise.
            * sm-taint.cc: Likewise.
            * store.cc (bit_range::contains_p): Allow "out" to be null.
            * store.h (byte_range::get_start_bit_offset): New.
            (byte_range::get_next_bit_offset): New.
            * varargs.cc: Add logger param to pending_diagnostic::emit
            implementations.

    gcc/testsuite/ChangeLog:
            PR analyzer/106626
            * gcc.dg/analyzer/data-model-1.c (test_16): Update for
            out-of-bounds working.
            * gcc.dg/analyzer/out-of-bounds-diagram-1-ascii.c: New test.
            * gcc.dg/analyzer/out-of-bounds-diagram-1-debug.c: New test.
            * gcc.dg/analyzer/out-of-bounds-diagram-1-emoji.c: New test.
            * gcc.dg/analyzer/out-of-bounds-diagram-1-json.c: New test.
            * gcc.dg/analyzer/out-of-bounds-diagram-1-sarif.c: New test.
            * gcc.dg/analyzer/out-of-bounds-diagram-1-unicode.c: New test.
            * gcc.dg/analyzer/out-of-bounds-diagram-10.c: New test.
            * gcc.dg/analyzer/out-of-bounds-diagram-11.c: New test.
            * gcc.dg/analyzer/out-of-bounds-diagram-12.c: New test.
            * gcc.dg/analyzer/out-of-bounds-diagram-13.c: New test.
            * gcc.dg/analyzer/out-of-bounds-diagram-14.c: New test.
            * gcc.dg/analyzer/out-of-bounds-diagram-15.c: New test.
            * gcc.dg/analyzer/out-of-bounds-diagram-2.c: New test.
            * gcc.dg/analyzer/out-of-bounds-diagram-3.c: New test.
            * gcc.dg/analyzer/out-of-bounds-diagram-4.c: New test.
            * gcc.dg/analyzer/out-of-bounds-diagram-5-ascii.c: New test.
            * gcc.dg/analyzer/out-of-bounds-diagram-5-unicode.c: New test.
            * gcc.dg/analyzer/out-of-bounds-diagram-6.c: New test.
            * gcc.dg/analyzer/out-of-bounds-diagram-7.c: New test.
            * gcc.dg/analyzer/out-of-bounds-diagram-8.c: New test.
            * gcc.dg/analyzer/out-of-bounds-diagram-9.c: New test.
            * gcc.dg/analyzer/pattern-test-2.c: Update expected results.
            * gcc.dg/analyzer/pr101962.c: Update expected results.
            * gcc.dg/plugin/analyzer_gil_plugin.c:  Add logger param to
            pending_diagnostic::emit implementations.

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