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>

diff --git a/gcc/analyzer/bounds-checking.cc b/gcc/analyzer/bounds-checking.cc
index 19aaa51e6a820fbc8d25241129291f748c87014b..ad7f431ea2f0189b5b901b501d31fa49f105e238 100644 (file)
--- a/gcc/analyzer/bounds-checking.cc
+++ b/gcc/analyzer/bounds-checking.cc
@@ -143,17 +143,28 @@ public:
 
     if (warned)
       {
-       char num_bytes_past_buf[WIDE_INT_PRINT_BUFFER_SIZE];
-       print_dec (m_out_of_bounds_range.m_size_in_bytes,
-                  num_bytes_past_buf, UNSIGNED);
-       if (m_diag_arg)
-         inform (rich_loc->get_loc (), "write is %s bytes past the end"
-                                       " of %qE", num_bytes_past_buf,
-                                                  m_diag_arg);
-       else
-         inform (rich_loc->get_loc (), "write is %s bytes past the end"
-                                       "of the region",
-                                       num_bytes_past_buf);
+       if (wi::fits_uhwi_p (m_out_of_bounds_range.m_size_in_bytes))
+         {
+           unsigned HOST_WIDE_INT num_bad_bytes
+             = m_out_of_bounds_range.m_size_in_bytes.to_uhwi ();
+           if (m_diag_arg)
+             inform_n (rich_loc->get_loc (),
+                       num_bad_bytes,
+                       "write of %wu byte to beyond the end of %qE",
+                       "write of %wu bytes to beyond the end of %qE",
+                       num_bad_bytes,
+                       m_diag_arg);
+           else
+             inform_n (rich_loc->get_loc (),
+                       num_bad_bytes,
+                       "write of %wu byte to beyond the end of the region",
+                       "write of %wu bytes to beyond the end of the region",
+                       num_bad_bytes);
+         }
+       else if (m_diag_arg)
+         inform (rich_loc->get_loc (),
+                 "write to beyond the end of %qE",
+                 m_diag_arg);
       }
 
     return warned;
@@ -212,17 +223,28 @@ public:
 
     if (warned)
       {
-       char num_bytes_past_buf[WIDE_INT_PRINT_BUFFER_SIZE];
-       print_dec (m_out_of_bounds_range.m_size_in_bytes,
-                  num_bytes_past_buf, UNSIGNED);
-       if (m_diag_arg)
-         inform (rich_loc->get_loc (), "read is %s bytes past the end"
-                                       " of %qE", num_bytes_past_buf,
-                                                   m_diag_arg);
-       else
-         inform (rich_loc->get_loc (), "read is %s bytes past the end"
-                                       "of the region",
-                                       num_bytes_past_buf);
+       if (wi::fits_uhwi_p (m_out_of_bounds_range.m_size_in_bytes))
+         {
+           unsigned HOST_WIDE_INT num_bad_bytes
+             = m_out_of_bounds_range.m_size_in_bytes.to_uhwi ();
+           if (m_diag_arg)
+             inform_n (rich_loc->get_loc (),
+                       num_bad_bytes,
+                       "read of %wu byte from after the end of %qE",
+                       "read of %wu bytes from after the end of %qE",
+                       num_bad_bytes,
+                       m_diag_arg);
+           else
+             inform_n (rich_loc->get_loc (),
+                       num_bad_bytes,
+                       "read of %wu byte from after the end of the region",
+                       "read of %wu bytes from after the end of the region",
+                       num_bad_bytes);
+         }
+       else if (m_diag_arg)
+         inform (rich_loc->get_loc (),
+                 "read from after the end of %qE",
+                 m_diag_arg);
       }
 
     return warned;

diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-char-arr.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-char-arr.c
index 61cbfc75c11e5a7793dce92b56478ca48e1bc00d..2b43000f8334871a3f4463bb5187fb73aa05c143 100644 (file)
--- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-char-arr.c
+++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-char-arr.c
@@ -32,24 +32,19 @@ char int_arr_read_element_after_end_off_by_one(void)
 {
   return arr[10]; /* { dg-warning "buffer overread" "warning" } */
   /* { dg-message "out-of-bounds read at byte 10 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */
-  /* { dg-message "read is 1 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */
-  // FIXME(PR 106626): "1 bytes"
+  /* { dg-message "read of 1 byte from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
 }
 
 char int_arr_read_element_after_end_near(void)
 {
   return arr[11]; /* { dg-warning "buffer overread" "warning" } */
   /* { dg-message "out-of-bounds read at byte 11 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */
-  /* { dg-message "read is 1 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */
-  // FIXME(PR 106626): is the note correct?
-  // FIXME(PR 106626): "1 bytes"
+  /* { dg-message "read of 1 byte from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
 }
 
 char int_arr_read_element_after_end_far(void)
 {
   return arr[100]; /* { dg-warning "buffer overread" "warning" } */
   /* { dg-message "out-of-bounds read at byte 100 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */
-  /* { dg-message "read is 1 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */
-  // FIXME(PR 106626): the note seems incorrect (size of access is 1 byte, but magnitude beyond boundary is 90)
-  // FIXME(PR 106626): "1 bytes"
+  /* { dg-message "read of 1 byte from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
 }

diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-int-arr.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-int-arr.c
index 0bb30d24e9f9a001b97adf22d8c739540bcf1475..0dc95563620c1b487f3473b7e7f06aa0d0b94267 100644 (file)
--- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-int-arr.c
+++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-int-arr.c
@@ -34,21 +34,19 @@ int32_t int_arr_read_element_after_end_off_by_one(void)
 {
   return arr[10]; /* { dg-warning "buffer overread" "warning" } */
   /* { dg-message "out-of-bounds read from byte 40 till byte 43 but 'arr' ends at byte 40" "final event" { target *-*-* } .-1 } */
-  /* { dg-message "read is 4 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */
+  /* { dg-message "read of 4 bytes from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
 }
 
 int32_t int_arr_read_element_after_end_near(void)
 {
   return arr[11]; /* { dg-warning "buffer overread" "warning" } */
   /* { dg-message "out-of-bounds read from byte 44 till byte 47 but 'arr' ends at byte 40" "final event" { target *-*-* } .-1 } */
-  /* { dg-message "read is 4 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */
-  // FIXME(PR 106626): is the note correct?
+  /* { dg-message "read of 4 bytes from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
 }
 
 int32_t int_arr_read_element_after_end_far(void)
 {
   return arr[100]; /* { dg-warning "buffer overread" "warning" } */
   /* { dg-message "out-of-bounds read from byte 400 till byte 403 but 'arr' ends at byte 40" "final event" { target *-*-* } .-1 } */
-  /* { dg-message "read is 4 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */
-  // FIXME(PR 106626): the note seems incorrect (size of access is 4 bytes, but magnitude beyond boundary is 390-393)
+  /* { dg-message "read of 4 bytes from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
 }

diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-char-arr.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-char-arr.c
index 47fbc5207eedf285dc947f188aeb5d530d2c91d2..3564476c322b95be3b021483a4583e81b69f7e59 100644 (file)
--- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-char-arr.c
+++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-char-arr.c
@@ -32,24 +32,19 @@ void int_arr_write_element_after_end_off_by_one(char x)
 {
   arr[10] = x; /* { dg-warning "buffer overflow" "warning" } */
   /* { dg-message "out-of-bounds write at byte 10 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */
-  /* { dg-message "write is 1 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */
-  // FIXME(PR 106626): "1 bytes"
+  /* { dg-message "write of 1 byte to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
 }
 
 void int_arr_write_element_after_end_near(char x)
 {
   arr[11] = x; /* { dg-warning "buffer overflow" "warning" } */
   /* { dg-message "out-of-bounds write at byte 11 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */
-  /* { dg-message "write is 1 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */
-  // FIXME(PR 106626): is the note correct?
-  // FIXME(PR 106626): "1 bytes"
+  /* { dg-message "write of 1 byte to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
 }
 
 void int_arr_write_element_after_end_far(char x)
 {
   arr[100] = x; /* { dg-warning "buffer overflow" "warning" } */
   /* { dg-message "out-of-bounds write at byte 100 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */
-  /* { dg-message "write is 1 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */
-  // FIXME(PR 106626): the note seems incorrect (size of access is 1 byte, but magnitude beyond boundary is 90)
-  // FIXME(PR 106626): "1 bytes"
+  /* { dg-message "write of 1 byte to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
 }

diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-int-arr.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-int-arr.c
index bf9760ee978f25f30e7a518a65deb3bdd24bcf82..24a9a6bfa18cac1d5c80a7f1ae7f4e80dd2bf116 100644 (file)
--- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-int-arr.c
+++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-int-arr.c
@@ -34,21 +34,19 @@ void int_arr_write_element_after_end_off_by_one(int32_t x)
 {
   arr[10] = x; /* { dg-warning "buffer overflow" "warning" } */
   /* { dg-message "out-of-bounds write from byte 40 till byte 43 but 'arr' ends at byte 40" "final event" { target *-*-* } .-1 } */
-  /* { dg-message "write is 4 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */
+  /* { dg-message "write of 4 bytes to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
 }
 
 void int_arr_write_element_after_end_near(int32_t x)
 {
   arr[11] = x; /* { dg-warning "buffer overflow" "warning" } */
   /* { dg-message "out-of-bounds write from byte 44 till byte 47 but 'arr' ends at byte 40" "final event" { target *-*-* } .-1 } */
-  /* { dg-message "write is 4 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */
-  // FIXME(PR 106626): is the note correct?
+  /* { dg-message "write of 4 bytes to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
 }
 
 void int_arr_write_element_after_end_far(int32_t x)
 {
   arr[100] = x; /* { dg-warning "buffer overflow" "warning" } */
   /* { dg-message "out-of-bounds write from byte 400 till byte 403 but 'arr' ends at byte 40" "final event" { target *-*-* } .-1 } */
-  /* { dg-message "write is 4 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */
-  // FIXME(PR 106626): the note seems incorrect (size of access is 4 bytes, but magnitude beyond boundary is 390-393)
+  /* { dg-message "write of 4 bytes to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
 }