public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: David Malcolm <dmalcolm@redhat.com>
To: gcc-patches@gcc.gnu.org
Cc: David Malcolm <dmalcolm@redhat.com>
Subject: [committed 5/7] diagnostics: tweak diagnostic_path::interprocedural_p [PR106626]
Date: Wed, 30 Nov 2022 21:41:58 -0500	[thread overview]
Message-ID: <20221201024200.3722982-5-dmalcolm@redhat.com> (raw)
In-Reply-To: <20221201024200.3722982-1-dmalcolm@redhat.com>

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]'

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r13-4429-g1d86af242bc4a8.

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>
---
 gcc/diagnostic-path.h                         |  3 ++
 gcc/diagnostic.cc                             | 37 +++++++++++++++++--
 .../analyzer/out-of-bounds-multiline-1.c      | 37 +++++++++++++++++++
 3 files changed, 74 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/out-of-bounds-multiline-1.c

diff --git a/gcc/diagnostic-path.h b/gcc/diagnostic-path.h
index 8ce4ff763d4..aa5cda8c23a 100644
--- a/gcc/diagnostic-path.h
+++ b/gcc/diagnostic-path.h
@@ -167,6 +167,9 @@ class diagnostic_path
   virtual const diagnostic_event & get_event (int idx) const = 0;
 
   bool interprocedural_p () const;
+
+private:
+  bool get_first_event_in_a_function (unsigned *out_idx) const;
 };
 
 /* Concrete subclasses.  */
diff --git a/gcc/diagnostic.cc b/gcc/diagnostic.cc
index a9562a815b1..322515b3242 100644
--- a/gcc/diagnostic.cc
+++ b/gcc/diagnostic.cc
@@ -939,18 +939,49 @@ diagnostic_event::meaning::maybe_get_property_str (enum property p)
 
 /* class diagnostic_path.  */
 
+/* Subroutint of diagnostic_path::interprocedural_p.
+   Look for the first event in this path that is within a function
+   i.e. has a non-NULL fndecl, and a non-zero stack depth.
+   If found, write its index to *OUT_IDX and return true.
+   Otherwise return false.  */
+
+bool
+diagnostic_path::get_first_event_in_a_function (unsigned *out_idx) const
+{
+  const unsigned num = num_events ();
+  for (unsigned i = 0; i < num; i++)
+    {
+      if (!(get_event (i).get_fndecl () == NULL
+	    && get_event (i).get_stack_depth () == 0))
+	{
+	  *out_idx = i;
+	  return true;
+	}
+    }
+  return false;
+}
+
 /* Return true if the events in this path involve more than one
    function, or false if it is purely intraprocedural.  */
 
 bool
 diagnostic_path::interprocedural_p () const
 {
+  /* Ignore leading events that are outside of any function.  */
+  unsigned first_fn_event_idx;
+  if (!get_first_event_in_a_function (&first_fn_event_idx))
+    return false;
+
+  const diagnostic_event &first_fn_event = get_event (first_fn_event_idx);
+  tree first_fndecl = first_fn_event.get_fndecl ();
+  int first_fn_stack_depth = first_fn_event.get_stack_depth ();
+
   const unsigned num = num_events ();
-  for (unsigned i = 0; i < num; i++)
+  for (unsigned i = first_fn_event_idx + 1; i < num; i++)
     {
-      if (get_event (i).get_fndecl () != get_event (0).get_fndecl ())
+      if (get_event (i).get_fndecl () != first_fndecl)
 	return true;
-      if (get_event (i).get_stack_depth () != get_event (0).get_stack_depth ())
+      if (get_event (i).get_stack_depth () != first_fn_stack_depth)
 	return true;
     }
   return false;
diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-multiline-1.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-multiline-1.c
new file mode 100644
index 00000000000..25301e9e2ff
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-multiline-1.c
@@ -0,0 +1,37 @@
+/* Integration test of how the execution path looks for
+   -Wanalyzer-out-of-bounds.  */
+
+/* { dg-additional-options "-fdiagnostics-show-path-depths" } */
+/* { dg-additional-options "-fdiagnostics-path-format=inline-events -fdiagnostics-show-caret" } */
+
+
+#include <stdint.h>
+
+int32_t arr[10];
+
+void int_arr_write_element_after_end_off_by_one(int32_t x)
+{
+  arr[10] = x;  /* { dg-line line } */
+}
+/* { dg-warning "buffer overflow" "warning" { target *-*-* } line } */
+/* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } line } */
+
+
+/* { dg-begin-multiline-output "" }
+   arr[10] = x;
+   ~~~~~~~~^~~
+  event 1 (depth 0)
+    |
+    | int32_t arr[10];
+    |         ^~~
+    |         |
+    |         (1) capacity is 40 bytes
+    |
+    +--> 'int_arr_write_element_after_end_off_by_one': event 2 (depth 1)
+           |
+           |   arr[10] = x;
+           |   ~~~~~~~~^~~
+           |           |
+           |           (2) out-of-bounds write from byte 40 till byte 43 but 'arr' ends at byte 40
+           |
+   { dg-end-multiline-output "" } */
-- 
2.26.3


  parent reply	other threads:[~2022-12-01  2:42 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-01  2:41 [committed 1/7] analyzer: move bounds checking to a new bounds-checking.cc David Malcolm
2022-12-01  2:41 ` [committed 2/7] analyzer: fix wording of 'number of bad bytes' note [PR106626] David Malcolm
2022-12-01  2:41 ` [committed 3/7] analyzer: add note about valid subscripts [PR106626] David Malcolm
2022-12-01  2:41 ` [committed 4/7] analyzer: more bounds-checking wording tweaks [PR106626] David Malcolm
2022-12-01  2:41 ` David Malcolm [this message]
2022-12-01  2:41 ` [committed 6/7] analyzer: unify bounds-checking class hierarchies David Malcolm
2022-12-01  2:42 ` [committed 7/7] analyzer: fix i18n issues in symbolic out-of-bounds [PR106626] David Malcolm

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221201024200.3722982-5-dmalcolm@redhat.com \
    --to=dmalcolm@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).