* [Bug analyzer/106626] Improvements to wording of -Wanalyzer-out-of-bounds
2022-08-15 13:08 [Bug analyzer/106626] New: Improvements to wording of -Wanalyzer-out-of-bounds dmalcolm at gcc dot gnu.org
@ 2022-08-15 18:48 ` cvs-commit at gcc dot gnu.org
2022-12-01 2:31 ` cvs-commit at gcc dot gnu.org
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-08-15 18:48 UTC (permalink / raw)
To: gcc-bugs
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>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug analyzer/106626] Improvements to wording of -Wanalyzer-out-of-bounds
2022-08-15 13:08 [Bug analyzer/106626] New: Improvements to wording of -Wanalyzer-out-of-bounds dmalcolm at gcc dot gnu.org
2022-08-15 18:48 ` [Bug analyzer/106626] " cvs-commit at gcc dot gnu.org
@ 2022-12-01 2:31 ` cvs-commit at gcc dot gnu.org
2022-12-01 2:31 ` cvs-commit at gcc dot gnu.org
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-12-01 2:31 UTC (permalink / raw)
To: gcc-bugs
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>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug analyzer/106626] Improvements to wording of -Wanalyzer-out-of-bounds
2022-08-15 13:08 [Bug analyzer/106626] New: Improvements to wording of -Wanalyzer-out-of-bounds dmalcolm at gcc dot gnu.org
2022-08-15 18:48 ` [Bug analyzer/106626] " cvs-commit at gcc dot gnu.org
2022-12-01 2:31 ` cvs-commit at gcc dot gnu.org
@ 2022-12-01 2:31 ` cvs-commit at gcc dot gnu.org
2022-12-01 2:31 ` cvs-commit at gcc dot gnu.org
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-12-01 2:31 UTC (permalink / raw)
To: gcc-bugs
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>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug analyzer/106626] Improvements to wording of -Wanalyzer-out-of-bounds
2022-08-15 13:08 [Bug analyzer/106626] New: Improvements to wording of -Wanalyzer-out-of-bounds dmalcolm at gcc dot gnu.org
` (2 preceding siblings ...)
2022-12-01 2:31 ` cvs-commit at gcc dot gnu.org
@ 2022-12-01 2:31 ` cvs-commit at gcc dot gnu.org
2022-12-01 2:31 ` cvs-commit at gcc dot gnu.org
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-12-01 2:31 UTC (permalink / raw)
To: gcc-bugs
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>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug analyzer/106626] Improvements to wording of -Wanalyzer-out-of-bounds
2022-08-15 13:08 [Bug analyzer/106626] New: Improvements to wording of -Wanalyzer-out-of-bounds dmalcolm at gcc dot gnu.org
` (3 preceding siblings ...)
2022-12-01 2:31 ` cvs-commit at gcc dot gnu.org
@ 2022-12-01 2:31 ` cvs-commit at gcc dot gnu.org
2022-12-01 2:31 ` cvs-commit at gcc dot gnu.org
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-12-01 2:31 UTC (permalink / raw)
To: gcc-bugs
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>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug analyzer/106626] Improvements to wording of -Wanalyzer-out-of-bounds
2022-08-15 13:08 [Bug analyzer/106626] New: Improvements to wording of -Wanalyzer-out-of-bounds dmalcolm at gcc dot gnu.org
` (4 preceding siblings ...)
2022-12-01 2:31 ` cvs-commit at gcc dot gnu.org
@ 2022-12-01 2:31 ` cvs-commit at gcc dot gnu.org
2023-04-07 12:36 ` dmalcolm at gcc dot gnu.org
2023-06-22 2:06 ` cvs-commit at gcc dot gnu.org
7 siblings, 0 replies; 9+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-12-01 2:31 UTC (permalink / raw)
To: gcc-bugs
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>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug analyzer/106626] Improvements to wording of -Wanalyzer-out-of-bounds
2022-08-15 13:08 [Bug analyzer/106626] New: Improvements to wording of -Wanalyzer-out-of-bounds dmalcolm at gcc dot gnu.org
` (5 preceding siblings ...)
2022-12-01 2:31 ` cvs-commit at gcc dot gnu.org
@ 2023-04-07 12:36 ` dmalcolm at gcc dot gnu.org
2023-06-22 2:06 ` cvs-commit at gcc dot gnu.org
7 siblings, 0 replies; 9+ messages in thread
From: dmalcolm at gcc dot gnu.org @ 2023-04-07 12:36 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug analyzer/106626] Improvements to wording of -Wanalyzer-out-of-bounds
2022-08-15 13:08 [Bug analyzer/106626] New: Improvements to wording of -Wanalyzer-out-of-bounds dmalcolm at gcc dot gnu.org
` (6 preceding siblings ...)
2023-04-07 12:36 ` dmalcolm at gcc dot gnu.org
@ 2023-06-22 2:06 ` cvs-commit at gcc dot gnu.org
7 siblings, 0 replies; 9+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-06-22 2:06 UTC (permalink / raw)
To: gcc-bugs
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>
^ permalink raw reply [flat|nested] 9+ messages in thread