public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [committed 1/7] analyzer: move bounds checking to a new bounds-checking.cc
@ 2022-12-01  2:41 David Malcolm
  2022-12-01  2:41 ` [committed 2/7] analyzer: fix wording of 'number of bad bytes' note [PR106626] David Malcolm
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: David Malcolm @ 2022-12-01  2:41 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

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

gcc/ChangeLog:
	* Makefile.in (ANALYZER_OBJS): Add analyzer/bounds-checking.o.

gcc/analyzer/ChangeLog:
	* bounds-checking.cc: New file, taken from region-model.cc.
	* region-model.cc (class out_of_bounds): Move to
	bounds-checking.cc.
	(class past_the_end): Likewise.
	(class buffer_overflow): Likewise.
	(class buffer_overread): Likewise.
	(class buffer_underflow): Likewise.
	(class buffer_underread): Likewise.
	(class symbolic_past_the_end): Likewise.
	(class symbolic_buffer_overflow): Likewise.
	(class symbolic_buffer_overread): Likewise.
	(region_model::check_symbolic_bounds): Likewise.
	(maybe_get_integer_cst_tree): Likewise.
	(region_model::check_region_bounds): Likewise.
	* region-model.h: Add comment.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>
---
 gcc/Makefile.in                 |   1 +
 gcc/analyzer/bounds-checking.cc | 695 ++++++++++++++++++++++++++++++++
 gcc/analyzer/region-model.cc    | 653 ------------------------------
 gcc/analyzer/region-model.h     |   2 +
 4 files changed, 698 insertions(+), 653 deletions(-)
 create mode 100644 gcc/analyzer/bounds-checking.cc

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index fa5e5b444bb..615a07089ee 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1255,6 +1255,7 @@ ANALYZER_OBJS = \
 	analyzer/analyzer-pass.o \
 	analyzer/analyzer-selftests.o \
 	analyzer/bar-chart.o \
+	analyzer/bounds-checking.o \
 	analyzer/call-info.o \
 	analyzer/call-string.o \
 	analyzer/call-summary.o \
diff --git a/gcc/analyzer/bounds-checking.cc b/gcc/analyzer/bounds-checking.cc
new file mode 100644
index 00000000000..19aaa51e6a8
--- /dev/null
+++ b/gcc/analyzer/bounds-checking.cc
@@ -0,0 +1,695 @@
+/* Bounds-checking of reads and writes to memory regions.
+   Copyright (C) 2019-2022 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it
+under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#include "config.h"
+#define INCLUDE_MEMORY
+#include "system.h"
+#include "coretypes.h"
+#include "make-unique.h"
+#include "tree.h"
+#include "function.h"
+#include "basic-block.h"
+#include "gimple.h"
+#include "gimple-iterator.h"
+#include "diagnostic-core.h"
+#include "diagnostic-metadata.h"
+#include "analyzer/analyzer.h"
+#include "analyzer/analyzer-logging.h"
+#include "analyzer/region-model.h"
+
+#if ENABLE_ANALYZER
+
+namespace ana {
+
+/* Abstract base class for all out-of-bounds warnings with concrete values.  */
+
+class out_of_bounds : public pending_diagnostic_subclass<out_of_bounds>
+{
+public:
+  out_of_bounds (const region *reg, tree diag_arg,
+		 byte_range out_of_bounds_range)
+  : m_reg (reg), m_diag_arg (diag_arg),
+    m_out_of_bounds_range (out_of_bounds_range)
+  {}
+
+  const char *get_kind () const final override
+  {
+    return "out_of_bounds_diagnostic";
+  }
+
+  bool operator== (const out_of_bounds &other) const
+  {
+    return m_reg == other.m_reg
+	   && m_out_of_bounds_range == other.m_out_of_bounds_range
+	   && pending_diagnostic::same_tree_p (m_diag_arg, other.m_diag_arg);
+  }
+
+  int get_controlling_option () const final override
+  {
+    return OPT_Wanalyzer_out_of_bounds;
+  }
+
+  void mark_interesting_stuff (interesting_t *interest) final override
+  {
+    interest->add_region_creation (m_reg);
+  }
+
+protected:
+  const region *m_reg;
+  tree m_diag_arg;
+  byte_range m_out_of_bounds_range;
+};
+
+/* Abstract subclass to complaing about out-of-bounds
+   past the end of the buffer.  */
+
+class past_the_end : public out_of_bounds
+{
+public:
+  past_the_end (const region *reg, tree diag_arg, byte_range range,
+		tree byte_bound)
+  : out_of_bounds (reg, diag_arg, range), m_byte_bound (byte_bound)
+  {}
+
+  bool operator== (const past_the_end &other) const
+  {
+    return out_of_bounds::operator== (other)
+	   && pending_diagnostic::same_tree_p (m_byte_bound,
+					       other.m_byte_bound);
+  }
+
+  label_text
+  describe_region_creation_event (const evdesc::region_creation &ev) final
+  override
+  {
+    if (m_byte_bound && TREE_CODE (m_byte_bound) == INTEGER_CST)
+      return ev.formatted_print ("capacity is %E bytes", m_byte_bound);
+
+    return label_text ();
+  }
+
+protected:
+  tree m_byte_bound;
+};
+
+/* Concrete subclass to complain about buffer overflows.  */
+
+class buffer_overflow : public past_the_end
+{
+public:
+  buffer_overflow (const region *reg, tree diag_arg,
+		   byte_range range, tree byte_bound)
+  : past_the_end (reg, diag_arg, range, byte_bound)
+  {}
+
+  bool emit (rich_location *rich_loc) final override
+  {
+    diagnostic_metadata m;
+    bool warned;
+    switch (m_reg->get_memory_space ())
+      {
+      default:
+	m.add_cwe (787);
+	warned = warning_meta (rich_loc, m, get_controlling_option (),
+			       "buffer overflow");
+	break;
+      case MEMSPACE_STACK:
+	m.add_cwe (121);
+	warned = warning_meta (rich_loc, m, get_controlling_option (),
+			       "stack-based buffer overflow");
+	break;
+      case MEMSPACE_HEAP:
+	m.add_cwe (122);
+	warned = warning_meta (rich_loc, m, get_controlling_option (),
+			       "heap-based buffer overflow");
+	break;
+      }
+
+    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);
+      }
+
+    return warned;
+  }
+
+  label_text describe_final_event (const evdesc::final_event &ev)
+  final override
+  {
+    byte_size_t start = m_out_of_bounds_range.get_start_byte_offset ();
+    byte_size_t end = m_out_of_bounds_range.get_last_byte_offset ();
+    char start_buf[WIDE_INT_PRINT_BUFFER_SIZE];
+    print_dec (start, start_buf, SIGNED);
+    char end_buf[WIDE_INT_PRINT_BUFFER_SIZE];
+    print_dec (end, end_buf, SIGNED);
+
+    if (start == end)
+      {
+	if (m_diag_arg)
+	  return ev.formatted_print ("out-of-bounds write at byte %s but %qE"
+				     " ends at byte %E", start_buf, m_diag_arg,
+							 m_byte_bound);
+	return ev.formatted_print ("out-of-bounds write at byte %s but region"
+				   " ends at byte %E", start_buf,
+						       m_byte_bound);
+      }
+    else
+      {
+	if (m_diag_arg)
+	  return ev.formatted_print ("out-of-bounds write from byte %s till"
+				     " byte %s but %qE ends at byte %E",
+				     start_buf, end_buf, m_diag_arg,
+				     m_byte_bound);
+	return ev.formatted_print ("out-of-bounds write from byte %s till"
+				   " byte %s but region ends at byte %E",
+				   start_buf, end_buf, m_byte_bound);
+      }
+  }
+};
+
+/* Concrete subclass to complain about buffer overreads.  */
+
+class buffer_overread : public past_the_end
+{
+public:
+  buffer_overread (const region *reg, tree diag_arg,
+		   byte_range range, tree byte_bound)
+  : past_the_end (reg, diag_arg, range, byte_bound)
+  {}
+
+  bool emit (rich_location *rich_loc) final override
+  {
+    diagnostic_metadata m;
+    m.add_cwe (126);
+    bool warned = warning_meta (rich_loc, m, get_controlling_option (),
+				"buffer overread");
+
+    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);
+      }
+
+    return warned;
+  }
+
+  label_text describe_final_event (const evdesc::final_event &ev)
+  final override
+  {
+    byte_size_t start = m_out_of_bounds_range.get_start_byte_offset ();
+    byte_size_t end = m_out_of_bounds_range.get_last_byte_offset ();
+    char start_buf[WIDE_INT_PRINT_BUFFER_SIZE];
+    print_dec (start, start_buf, SIGNED);
+    char end_buf[WIDE_INT_PRINT_BUFFER_SIZE];
+    print_dec (end, end_buf, SIGNED);
+
+    if (start == end)
+      {
+	if (m_diag_arg)
+	  return ev.formatted_print ("out-of-bounds read at byte %s but %qE"
+				     " ends at byte %E", start_buf, m_diag_arg,
+							 m_byte_bound);
+	return ev.formatted_print ("out-of-bounds read at byte %s but region"
+				   " ends at byte %E", start_buf,
+						       m_byte_bound);
+      }
+    else
+      {
+	if (m_diag_arg)
+	  return ev.formatted_print ("out-of-bounds read from byte %s till"
+				     " byte %s but %qE ends at byte %E",
+				     start_buf, end_buf, m_diag_arg,
+				     m_byte_bound);
+	return ev.formatted_print ("out-of-bounds read from byte %s till"
+				   " byte %s but region ends at byte %E",
+				   start_buf, end_buf, m_byte_bound);
+      }
+  }
+};
+
+/* Concrete subclass to complain about buffer underflows.  */
+
+class buffer_underflow : public out_of_bounds
+{
+public:
+  buffer_underflow (const region *reg, tree diag_arg, byte_range range)
+  : out_of_bounds (reg, diag_arg, range)
+  {}
+
+  bool emit (rich_location *rich_loc) final override
+  {
+    diagnostic_metadata m;
+    m.add_cwe (124);
+    return warning_meta (rich_loc, m, get_controlling_option (),
+			 "buffer underflow");
+  }
+
+  label_text describe_final_event (const evdesc::final_event &ev)
+  final override
+  {
+    byte_size_t start = m_out_of_bounds_range.get_start_byte_offset ();
+    byte_size_t end = m_out_of_bounds_range.get_last_byte_offset ();
+    char start_buf[WIDE_INT_PRINT_BUFFER_SIZE];
+    print_dec (start, start_buf, SIGNED);
+    char end_buf[WIDE_INT_PRINT_BUFFER_SIZE];
+    print_dec (end, end_buf, SIGNED);
+
+    if (start == end)
+      {
+	if (m_diag_arg)
+	  return ev.formatted_print ("out-of-bounds write at byte %s but %qE"
+				     " starts at byte 0", start_buf,
+							  m_diag_arg);
+	return ev.formatted_print ("out-of-bounds write at byte %s but region"
+				   " starts at byte 0", start_buf);
+      }
+    else
+      {
+	if (m_diag_arg)
+	  return ev.formatted_print ("out-of-bounds write from byte %s till"
+				     " byte %s but %qE starts at byte 0",
+				     start_buf, end_buf, m_diag_arg);
+	return ev.formatted_print ("out-of-bounds write from byte %s till"
+				   " byte %s but region starts at byte 0",
+				   start_buf, end_buf);;
+      }
+  }
+};
+
+/* Concrete subclass to complain about buffer underreads.  */
+
+class buffer_underread : public out_of_bounds
+{
+public:
+  buffer_underread (const region *reg, tree diag_arg, byte_range range)
+  : out_of_bounds (reg, diag_arg, range)
+  {}
+
+  bool emit (rich_location *rich_loc) final override
+  {
+    diagnostic_metadata m;
+    m.add_cwe (127);
+    return warning_meta (rich_loc, m, get_controlling_option (),
+			 "buffer underread");
+  }
+
+  label_text describe_final_event (const evdesc::final_event &ev)
+  final override
+  {
+    byte_size_t start = m_out_of_bounds_range.get_start_byte_offset ();
+    byte_size_t end = m_out_of_bounds_range.get_last_byte_offset ();
+    char start_buf[WIDE_INT_PRINT_BUFFER_SIZE];
+    print_dec (start, start_buf, SIGNED);
+    char end_buf[WIDE_INT_PRINT_BUFFER_SIZE];
+    print_dec (end, end_buf, SIGNED);
+
+    if (start == end)
+      {
+	if (m_diag_arg)
+	  return ev.formatted_print ("out-of-bounds read at byte %s but %qE"
+				     " starts at byte 0", start_buf,
+							  m_diag_arg);
+	return ev.formatted_print ("out-of-bounds read at byte %s but region"
+				  " starts at byte 0", start_buf);
+      }
+    else
+      {
+	if (m_diag_arg)
+	  return ev.formatted_print ("out-of-bounds read from byte %s till"
+				     " byte %s but %qE starts at byte 0",
+				     start_buf, end_buf, m_diag_arg);
+	return ev.formatted_print ("out-of-bounds read from byte %s till"
+				   " byte %s but region starts at byte 0",
+				   start_buf, end_buf);;
+      }
+  }
+};
+
+/* Abstract class to complain about out-of-bounds read/writes where
+   the values are symbolic.  */
+
+class symbolic_past_the_end
+  : public pending_diagnostic_subclass<symbolic_past_the_end>
+{
+public:
+  symbolic_past_the_end (const region *reg, tree diag_arg, tree offset,
+			 tree num_bytes, tree capacity)
+  : m_reg (reg), m_diag_arg (diag_arg), m_offset (offset),
+    m_num_bytes (num_bytes), m_capacity (capacity)
+  {}
+
+  const char *get_kind () const final override
+  {
+    return "symbolic_past_the_end";
+  }
+
+  bool operator== (const symbolic_past_the_end &other) const
+  {
+    return m_reg == other.m_reg
+	   && pending_diagnostic::same_tree_p (m_diag_arg, other.m_diag_arg)
+	   && pending_diagnostic::same_tree_p (m_offset, other.m_offset)
+	   && pending_diagnostic::same_tree_p (m_num_bytes, other.m_num_bytes)
+	   && pending_diagnostic::same_tree_p (m_capacity, other.m_capacity);
+  }
+
+  int get_controlling_option () const final override
+  {
+    return OPT_Wanalyzer_out_of_bounds;
+  }
+
+  void mark_interesting_stuff (interesting_t *interest) final override
+  {
+    interest->add_region_creation (m_reg);
+  }
+
+  label_text
+  describe_region_creation_event (const evdesc::region_creation &ev) final
+  override
+  {
+    if (m_capacity)
+      return ev.formatted_print ("capacity is %qE bytes", m_capacity);
+
+    return label_text ();
+  }
+
+  label_text
+  describe_final_event (const evdesc::final_event &ev) final override
+  {
+    const char *byte_str;
+    if (pending_diagnostic::same_tree_p (m_num_bytes, integer_one_node))
+      byte_str = "byte";
+    else
+      byte_str = "bytes";
+
+    if (m_offset)
+      {
+	if (m_num_bytes && TREE_CODE (m_num_bytes) == INTEGER_CST)
+	  {
+	    if (m_diag_arg)
+	      return ev.formatted_print ("%s of %E %s at offset %qE"
+					 " exceeds %qE", m_dir_str,
+					 m_num_bytes, byte_str,
+					 m_offset, m_diag_arg);
+	    else
+	      return ev.formatted_print ("%s of %E %s at offset %qE"
+					 " exceeds the buffer", m_dir_str,
+					 m_num_bytes, byte_str, m_offset);
+	  }
+	else if (m_num_bytes)
+	  {
+	    if (m_diag_arg)
+	      return ev.formatted_print ("%s of %qE %s at offset %qE"
+					 " exceeds %qE", m_dir_str,
+					 m_num_bytes, byte_str,
+					 m_offset, m_diag_arg);
+	    else
+	      return ev.formatted_print ("%s of %qE %s at offset %qE"
+					 " exceeds the buffer", m_dir_str,
+					 m_num_bytes, byte_str, m_offset);
+	  }
+	else
+	  {
+	    if (m_diag_arg)
+	      return ev.formatted_print ("%s at offset %qE exceeds %qE",
+					 m_dir_str, m_offset, m_diag_arg);
+	    else
+	      return ev.formatted_print ("%s at offset %qE exceeds the"
+					 " buffer", m_dir_str, m_offset);
+	  }
+      }
+    if (m_diag_arg)
+      return ev.formatted_print ("out-of-bounds %s on %qE",
+				 m_dir_str, m_diag_arg);
+    return ev.formatted_print ("out-of-bounds %s", m_dir_str);
+  }
+
+protected:
+  const region *m_reg;
+  tree m_diag_arg;
+  tree m_offset;
+  tree m_num_bytes;
+  tree m_capacity;
+  const char *m_dir_str;
+};
+
+/* Concrete subclass to complain about overflows with symbolic values.  */
+
+class symbolic_buffer_overflow : public symbolic_past_the_end
+{
+public:
+  symbolic_buffer_overflow (const region *reg, tree diag_arg, tree offset,
+			    tree num_bytes, tree capacity)
+  : symbolic_past_the_end (reg, diag_arg, offset, num_bytes, capacity)
+  {
+    m_dir_str = "write";
+  }
+
+  bool emit (rich_location *rich_loc) final override
+  {
+    diagnostic_metadata m;
+    switch (m_reg->get_memory_space ())
+      {
+      default:
+	m.add_cwe (787);
+	return warning_meta (rich_loc, m, get_controlling_option (),
+			     "buffer overflow");
+      case MEMSPACE_STACK:
+	m.add_cwe (121);
+	return warning_meta (rich_loc, m, get_controlling_option (),
+			     "stack-based buffer overflow");
+      case MEMSPACE_HEAP:
+	m.add_cwe (122);
+	return warning_meta (rich_loc, m, get_controlling_option (),
+			     "heap-based buffer overflow");
+      }
+  }
+};
+
+/* Concrete subclass to complain about overreads with symbolic values.  */
+
+class symbolic_buffer_overread : public symbolic_past_the_end
+{
+public:
+  symbolic_buffer_overread (const region *reg, tree diag_arg, tree offset,
+			    tree num_bytes, tree capacity)
+  : symbolic_past_the_end (reg, diag_arg, offset, num_bytes, capacity)
+  {
+    m_dir_str = "read";
+  }
+
+  bool emit (rich_location *rich_loc) final override
+  {
+    diagnostic_metadata m;
+    m.add_cwe (126);
+    return warning_meta (rich_loc, m, get_controlling_option (),
+			 "buffer overread");
+  }
+};
+
+/* Check whether an access is past the end of the BASE_REG.  */
+
+void
+region_model::check_symbolic_bounds (const region *base_reg,
+				     const svalue *sym_byte_offset,
+				     const svalue *num_bytes_sval,
+				     const svalue *capacity,
+				     enum access_direction dir,
+				     region_model_context *ctxt) const
+{
+  gcc_assert (ctxt);
+
+  const svalue *next_byte
+    = m_mgr->get_or_create_binop (num_bytes_sval->get_type (), PLUS_EXPR,
+				  sym_byte_offset, num_bytes_sval);
+
+  if (eval_condition (next_byte, GT_EXPR, capacity).is_true ())
+    {
+      tree diag_arg = get_representative_tree (base_reg);
+      tree offset_tree = get_representative_tree (sym_byte_offset);
+      tree num_bytes_tree = get_representative_tree (num_bytes_sval);
+      tree capacity_tree = get_representative_tree (capacity);
+      switch (dir)
+	{
+	default:
+	  gcc_unreachable ();
+	  break;
+	case DIR_READ:
+	  ctxt->warn (make_unique<symbolic_buffer_overread> (base_reg,
+							     diag_arg,
+							     offset_tree,
+							     num_bytes_tree,
+							     capacity_tree));
+	  break;
+	case DIR_WRITE:
+	  ctxt->warn (make_unique<symbolic_buffer_overflow> (base_reg,
+							     diag_arg,
+							     offset_tree,
+							     num_bytes_tree,
+							     capacity_tree));
+	  break;
+	}
+    }
+}
+
+static tree
+maybe_get_integer_cst_tree (const svalue *sval)
+{
+  tree cst_tree = sval->maybe_get_constant ();
+  if (cst_tree && TREE_CODE (cst_tree) == INTEGER_CST)
+    return cst_tree;
+
+  return NULL_TREE;
+}
+
+/* May complain when the access on REG is out-of-bounds.  */
+
+void
+region_model::check_region_bounds (const region *reg,
+				   enum access_direction dir,
+				   region_model_context *ctxt) const
+{
+  gcc_assert (ctxt);
+
+  /* Get the offset.  */
+  region_offset reg_offset = reg->get_offset (m_mgr);
+  const region *base_reg = reg_offset.get_base_region ();
+
+  /* Bail out on symbolic regions.
+     (e.g. because the analyzer did not see previous offsets on the latter,
+     it might think that a negative access is before the buffer).  */
+  if (base_reg->symbolic_p ())
+    return;
+
+  /* Find out how many bytes were accessed.  */
+  const svalue *num_bytes_sval = reg->get_byte_size_sval (m_mgr);
+  tree num_bytes_tree = maybe_get_integer_cst_tree (num_bytes_sval);
+  /* Bail out if 0 bytes are accessed.  */
+  if (num_bytes_tree && zerop (num_bytes_tree))
+    return;
+
+  /* Get the capacity of the buffer.  */
+  const svalue *capacity = get_capacity (base_reg);
+  tree cst_capacity_tree = maybe_get_integer_cst_tree (capacity);
+
+  /* The constant offset from a pointer is represented internally as a sizetype
+     but should be interpreted as a signed value here.  The statement below
+     converts the offset from bits to bytes and then to a signed integer with
+     the same precision the sizetype has on the target system.
+
+     For example, this is needed for out-of-bounds-3.c test1 to pass when
+     compiled with a 64-bit gcc build targeting 32-bit systems.  */
+  byte_offset_t offset;
+  if (!reg_offset.symbolic_p ())
+    offset = wi::sext (reg_offset.get_bit_offset () >> LOG2_BITS_PER_UNIT,
+		       TYPE_PRECISION (size_type_node));
+
+  /* If either the offset or the number of bytes accessed are symbolic,
+     we have to reason about symbolic values.  */
+  if (reg_offset.symbolic_p () || !num_bytes_tree)
+    {
+      const svalue* byte_offset_sval;
+      if (!reg_offset.symbolic_p ())
+	{
+	  tree offset_tree = wide_int_to_tree (integer_type_node, offset);
+	  byte_offset_sval
+	    = m_mgr->get_or_create_constant_svalue (offset_tree);
+	}
+      else
+	byte_offset_sval = reg_offset.get_symbolic_byte_offset ();
+      check_symbolic_bounds (base_reg, byte_offset_sval, num_bytes_sval,
+			     capacity, dir, ctxt);
+      return;
+    }
+
+  /* Otherwise continue to check with concrete values.  */
+  byte_range out (0, 0);
+  /* NUM_BYTES_TREE should always be interpreted as unsigned.  */
+  byte_offset_t num_bytes_unsigned = wi::to_offset (num_bytes_tree);
+  byte_range read_bytes (offset, num_bytes_unsigned);
+  /* If read_bytes has a subset < 0, we do have an underflow.  */
+  if (read_bytes.falls_short_of_p (0, &out))
+    {
+      tree diag_arg = get_representative_tree (base_reg);
+      switch (dir)
+	{
+	default:
+	  gcc_unreachable ();
+	  break;
+	case DIR_READ:
+	  ctxt->warn (make_unique<buffer_underread> (reg, diag_arg, out));
+	  break;
+	case DIR_WRITE:
+	  ctxt->warn (make_unique<buffer_underflow> (reg, diag_arg, out));
+	  break;
+	}
+    }
+
+  /* For accesses past the end, we do need a concrete capacity.  No need to
+     do a symbolic check here because the inequality check does not reason
+     whether constants are greater than symbolic values.  */
+  if (!cst_capacity_tree)
+    return;
+
+  byte_range buffer (0, wi::to_offset (cst_capacity_tree));
+  /* If READ_BYTES exceeds BUFFER, we do have an overflow.  */
+  if (read_bytes.exceeds_p (buffer, &out))
+    {
+      tree byte_bound = wide_int_to_tree (size_type_node,
+					  buffer.get_next_byte_offset ());
+      tree diag_arg = get_representative_tree (base_reg);
+
+      switch (dir)
+	{
+	default:
+	  gcc_unreachable ();
+	  break;
+	case DIR_READ:
+	  ctxt->warn (make_unique<buffer_overread> (reg, diag_arg,
+						    out, byte_bound));
+	  break;
+	case DIR_WRITE:
+	  ctxt->warn (make_unique<buffer_overflow> (reg, diag_arg,
+						    out, byte_bound));
+	  break;
+	}
+    }
+}
+
+} // namespace ana
+
+#endif /* #if ENABLE_ANALYZER */
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 7f2c0b6bd1a..91b868f7b16 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -1212,659 +1212,6 @@ region_model::on_stmt_pre (const gimple *stmt,
     }
 }
 
-/* Abstract base class for all out-of-bounds warnings with concrete values.  */
-
-class out_of_bounds : public pending_diagnostic_subclass<out_of_bounds>
-{
-public:
-  out_of_bounds (const region *reg, tree diag_arg,
-		 byte_range out_of_bounds_range)
-  : m_reg (reg), m_diag_arg (diag_arg),
-    m_out_of_bounds_range (out_of_bounds_range)
-  {}
-
-  const char *get_kind () const final override
-  {
-    return "out_of_bounds_diagnostic";
-  }
-
-  bool operator== (const out_of_bounds &other) const
-  {
-    return m_reg == other.m_reg
-	   && m_out_of_bounds_range == other.m_out_of_bounds_range
-	   && pending_diagnostic::same_tree_p (m_diag_arg, other.m_diag_arg);
-  }
-
-  int get_controlling_option () const final override
-  {
-    return OPT_Wanalyzer_out_of_bounds;
-  }
-
-  void mark_interesting_stuff (interesting_t *interest) final override
-  {
-    interest->add_region_creation (m_reg);
-  }
-
-protected:
-  const region *m_reg;
-  tree m_diag_arg;
-  byte_range m_out_of_bounds_range;
-};
-
-/* Abstract subclass to complaing about out-of-bounds
-   past the end of the buffer.  */
-
-class past_the_end : public out_of_bounds
-{
-public:
-  past_the_end (const region *reg, tree diag_arg, byte_range range,
-		tree byte_bound)
-  : out_of_bounds (reg, diag_arg, range), m_byte_bound (byte_bound)
-  {}
-
-  bool operator== (const past_the_end &other) const
-  {
-    return out_of_bounds::operator== (other)
-	   && pending_diagnostic::same_tree_p (m_byte_bound,
-					       other.m_byte_bound);
-  }
-
-  label_text
-  describe_region_creation_event (const evdesc::region_creation &ev) final
-  override
-  {
-    if (m_byte_bound && TREE_CODE (m_byte_bound) == INTEGER_CST)
-      return ev.formatted_print ("capacity is %E bytes", m_byte_bound);
-
-    return label_text ();
-  }
-
-protected:
-  tree m_byte_bound;
-};
-
-/* Concrete subclass to complain about buffer overflows.  */
-
-class buffer_overflow : public past_the_end
-{
-public:
-  buffer_overflow (const region *reg, tree diag_arg,
-		   byte_range range, tree byte_bound)
-  : past_the_end (reg, diag_arg, range, byte_bound)
-  {}
-
-  bool emit (rich_location *rich_loc) final override
-  {
-    diagnostic_metadata m;
-    bool warned;
-    switch (m_reg->get_memory_space ())
-      {
-      default:
-	m.add_cwe (787);
-	warned = warning_meta (rich_loc, m, get_controlling_option (),
-			       "buffer overflow");
-	break;
-      case MEMSPACE_STACK:
-	m.add_cwe (121);
-	warned = warning_meta (rich_loc, m, get_controlling_option (),
-			       "stack-based buffer overflow");
-	break;
-      case MEMSPACE_HEAP:
-	m.add_cwe (122);
-	warned = warning_meta (rich_loc, m, get_controlling_option (),
-			       "heap-based buffer overflow");
-	break;
-      }
-
-    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);
-      }
-
-    return warned;
-  }
-
-  label_text describe_final_event (const evdesc::final_event &ev)
-  final override
-  {
-    byte_size_t start = m_out_of_bounds_range.get_start_byte_offset ();
-    byte_size_t end = m_out_of_bounds_range.get_last_byte_offset ();
-    char start_buf[WIDE_INT_PRINT_BUFFER_SIZE];
-    print_dec (start, start_buf, SIGNED);
-    char end_buf[WIDE_INT_PRINT_BUFFER_SIZE];
-    print_dec (end, end_buf, SIGNED);
-
-    if (start == end)
-      {
-	if (m_diag_arg)
-	  return ev.formatted_print ("out-of-bounds write at byte %s but %qE"
-				     " ends at byte %E", start_buf, m_diag_arg,
-							 m_byte_bound);
-	return ev.formatted_print ("out-of-bounds write at byte %s but region"
-				   " ends at byte %E", start_buf,
-						       m_byte_bound);
-      }
-    else
-      {
-	if (m_diag_arg)
-	  return ev.formatted_print ("out-of-bounds write from byte %s till"
-				     " byte %s but %qE ends at byte %E",
-				     start_buf, end_buf, m_diag_arg,
-				     m_byte_bound);
-	return ev.formatted_print ("out-of-bounds write from byte %s till"
-				   " byte %s but region ends at byte %E",
-				   start_buf, end_buf, m_byte_bound);
-      }
-  }
-};
-
-/* Concrete subclass to complain about buffer overreads.  */
-
-class buffer_overread : public past_the_end
-{
-public:
-  buffer_overread (const region *reg, tree diag_arg,
-		   byte_range range, tree byte_bound)
-  : past_the_end (reg, diag_arg, range, byte_bound)
-  {}
-
-  bool emit (rich_location *rich_loc) final override
-  {
-    diagnostic_metadata m;
-    m.add_cwe (126);
-    bool warned = warning_meta (rich_loc, m, get_controlling_option (),
-				"buffer overread");
-
-    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);
-      }
-
-    return warned;
-  }
-
-  label_text describe_final_event (const evdesc::final_event &ev)
-  final override
-  {
-    byte_size_t start = m_out_of_bounds_range.get_start_byte_offset ();
-    byte_size_t end = m_out_of_bounds_range.get_last_byte_offset ();
-    char start_buf[WIDE_INT_PRINT_BUFFER_SIZE];
-    print_dec (start, start_buf, SIGNED);
-    char end_buf[WIDE_INT_PRINT_BUFFER_SIZE];
-    print_dec (end, end_buf, SIGNED);
-
-    if (start == end)
-      {
-	if (m_diag_arg)
-	  return ev.formatted_print ("out-of-bounds read at byte %s but %qE"
-				     " ends at byte %E", start_buf, m_diag_arg,
-							 m_byte_bound);
-	return ev.formatted_print ("out-of-bounds read at byte %s but region"
-				   " ends at byte %E", start_buf,
-						       m_byte_bound);
-      }
-    else
-      {
-	if (m_diag_arg)
-	  return ev.formatted_print ("out-of-bounds read from byte %s till"
-				     " byte %s but %qE ends at byte %E",
-				     start_buf, end_buf, m_diag_arg,
-				     m_byte_bound);
-	return ev.formatted_print ("out-of-bounds read from byte %s till"
-				   " byte %s but region ends at byte %E",
-				   start_buf, end_buf, m_byte_bound);
-      }
-  }
-};
-
-/* Concrete subclass to complain about buffer underflows.  */
-
-class buffer_underflow : public out_of_bounds
-{
-public:
-  buffer_underflow (const region *reg, tree diag_arg, byte_range range)
-  : out_of_bounds (reg, diag_arg, range)
-  {}
-
-  bool emit (rich_location *rich_loc) final override
-  {
-    diagnostic_metadata m;
-    m.add_cwe (124);
-    return warning_meta (rich_loc, m, get_controlling_option (),
-			 "buffer underflow");
-  }
-
-  label_text describe_final_event (const evdesc::final_event &ev)
-  final override
-  {
-    byte_size_t start = m_out_of_bounds_range.get_start_byte_offset ();
-    byte_size_t end = m_out_of_bounds_range.get_last_byte_offset ();
-    char start_buf[WIDE_INT_PRINT_BUFFER_SIZE];
-    print_dec (start, start_buf, SIGNED);
-    char end_buf[WIDE_INT_PRINT_BUFFER_SIZE];
-    print_dec (end, end_buf, SIGNED);
-
-    if (start == end)
-      {
-	if (m_diag_arg)
-	  return ev.formatted_print ("out-of-bounds write at byte %s but %qE"
-				     " starts at byte 0", start_buf,
-							  m_diag_arg);
-	return ev.formatted_print ("out-of-bounds write at byte %s but region"
-				   " starts at byte 0", start_buf);
-      }
-    else
-      {
-	if (m_diag_arg)
-	  return ev.formatted_print ("out-of-bounds write from byte %s till"
-				     " byte %s but %qE starts at byte 0",
-				     start_buf, end_buf, m_diag_arg);
-	return ev.formatted_print ("out-of-bounds write from byte %s till"
-				   " byte %s but region starts at byte 0",
-				   start_buf, end_buf);;
-      }
-  }
-};
-
-/* Concrete subclass to complain about buffer underreads.  */
-
-class buffer_underread : public out_of_bounds
-{
-public:
-  buffer_underread (const region *reg, tree diag_arg, byte_range range)
-  : out_of_bounds (reg, diag_arg, range)
-  {}
-
-  bool emit (rich_location *rich_loc) final override
-  {
-    diagnostic_metadata m;
-    m.add_cwe (127);
-    return warning_meta (rich_loc, m, get_controlling_option (),
-			 "buffer underread");
-  }
-
-  label_text describe_final_event (const evdesc::final_event &ev)
-  final override
-  {
-    byte_size_t start = m_out_of_bounds_range.get_start_byte_offset ();
-    byte_size_t end = m_out_of_bounds_range.get_last_byte_offset ();
-    char start_buf[WIDE_INT_PRINT_BUFFER_SIZE];
-    print_dec (start, start_buf, SIGNED);
-    char end_buf[WIDE_INT_PRINT_BUFFER_SIZE];
-    print_dec (end, end_buf, SIGNED);
-
-    if (start == end)
-      {
-	if (m_diag_arg)
-	  return ev.formatted_print ("out-of-bounds read at byte %s but %qE"
-				     " starts at byte 0", start_buf,
-							  m_diag_arg);
-	return ev.formatted_print ("out-of-bounds read at byte %s but region"
-				  " starts at byte 0", start_buf);
-      }
-    else
-      {
-	if (m_diag_arg)
-	  return ev.formatted_print ("out-of-bounds read from byte %s till"
-				     " byte %s but %qE starts at byte 0",
-				     start_buf, end_buf, m_diag_arg);
-	return ev.formatted_print ("out-of-bounds read from byte %s till"
-				   " byte %s but region starts at byte 0",
-				   start_buf, end_buf);;
-      }
-  }
-};
-
-/* Abstract class to complain about out-of-bounds read/writes where
-   the values are symbolic.  */
-
-class symbolic_past_the_end
-  : public pending_diagnostic_subclass<symbolic_past_the_end>
-{
-public:
-  symbolic_past_the_end (const region *reg, tree diag_arg, tree offset,
-			 tree num_bytes, tree capacity)
-  : m_reg (reg), m_diag_arg (diag_arg), m_offset (offset),
-    m_num_bytes (num_bytes), m_capacity (capacity)
-  {}
-
-  const char *get_kind () const final override
-  {
-    return "symbolic_past_the_end";
-  }
-
-  bool operator== (const symbolic_past_the_end &other) const
-  {
-    return m_reg == other.m_reg
-	   && pending_diagnostic::same_tree_p (m_diag_arg, other.m_diag_arg)
-	   && pending_diagnostic::same_tree_p (m_offset, other.m_offset)
-	   && pending_diagnostic::same_tree_p (m_num_bytes, other.m_num_bytes)
-	   && pending_diagnostic::same_tree_p (m_capacity, other.m_capacity);
-  }
-
-  int get_controlling_option () const final override
-  {
-    return OPT_Wanalyzer_out_of_bounds;
-  }
-
-  void mark_interesting_stuff (interesting_t *interest) final override
-  {
-    interest->add_region_creation (m_reg);
-  }
-
-  label_text
-  describe_region_creation_event (const evdesc::region_creation &ev) final
-  override
-  {
-    if (m_capacity)
-      return ev.formatted_print ("capacity is %qE bytes", m_capacity);
-
-    return label_text ();
-  }
-
-  label_text
-  describe_final_event (const evdesc::final_event &ev) final override
-  {
-    const char *byte_str;
-    if (pending_diagnostic::same_tree_p (m_num_bytes, integer_one_node))
-      byte_str = "byte";
-    else
-      byte_str = "bytes";
-
-    if (m_offset)
-      {
-	if (m_num_bytes && TREE_CODE (m_num_bytes) == INTEGER_CST)
-	  {
-	    if (m_diag_arg)
-	      return ev.formatted_print ("%s of %E %s at offset %qE"
-					 " exceeds %qE", m_dir_str,
-					 m_num_bytes, byte_str,
-					 m_offset, m_diag_arg);
-	    else
-	      return ev.formatted_print ("%s of %E %s at offset %qE"
-					 " exceeds the buffer", m_dir_str,
-					 m_num_bytes, byte_str, m_offset);
-	  }
-	else if (m_num_bytes)
-	  {
-	    if (m_diag_arg)
-	      return ev.formatted_print ("%s of %qE %s at offset %qE"
-					 " exceeds %qE", m_dir_str,
-					 m_num_bytes, byte_str,
-					 m_offset, m_diag_arg);
-	    else
-	      return ev.formatted_print ("%s of %qE %s at offset %qE"
-					 " exceeds the buffer", m_dir_str,
-					 m_num_bytes, byte_str, m_offset);
-	  }
-	else
-	  {
-	    if (m_diag_arg)
-	      return ev.formatted_print ("%s at offset %qE exceeds %qE",
-					 m_dir_str, m_offset, m_diag_arg);
-	    else
-	      return ev.formatted_print ("%s at offset %qE exceeds the"
-					 " buffer", m_dir_str, m_offset);
-	  }
-      }
-    if (m_diag_arg)
-      return ev.formatted_print ("out-of-bounds %s on %qE",
-				 m_dir_str, m_diag_arg);
-    return ev.formatted_print ("out-of-bounds %s", m_dir_str);
-  }
-
-protected:
-  const region *m_reg;
-  tree m_diag_arg;
-  tree m_offset;
-  tree m_num_bytes;
-  tree m_capacity;
-  const char *m_dir_str;
-};
-
-/* Concrete subclass to complain about overflows with symbolic values.  */
-
-class symbolic_buffer_overflow : public symbolic_past_the_end
-{
-public:
-  symbolic_buffer_overflow (const region *reg, tree diag_arg, tree offset,
-			    tree num_bytes, tree capacity)
-  : symbolic_past_the_end (reg, diag_arg, offset, num_bytes, capacity)
-  {
-    m_dir_str = "write";
-  }
-
-  bool emit (rich_location *rich_loc) final override
-  {
-    diagnostic_metadata m;
-    switch (m_reg->get_memory_space ())
-      {
-      default:
-	m.add_cwe (787);
-	return warning_meta (rich_loc, m, get_controlling_option (),
-			     "buffer overflow");
-      case MEMSPACE_STACK:
-	m.add_cwe (121);
-	return warning_meta (rich_loc, m, get_controlling_option (),
-			     "stack-based buffer overflow");
-      case MEMSPACE_HEAP:
-	m.add_cwe (122);
-	return warning_meta (rich_loc, m, get_controlling_option (),
-			     "heap-based buffer overflow");
-      }
-  }
-};
-
-/* Concrete subclass to complain about overreads with symbolic values.  */
-
-class symbolic_buffer_overread : public symbolic_past_the_end
-{
-public:
-  symbolic_buffer_overread (const region *reg, tree diag_arg, tree offset,
-			    tree num_bytes, tree capacity)
-  : symbolic_past_the_end (reg, diag_arg, offset, num_bytes, capacity)
-  {
-    m_dir_str = "read";
-  }
-
-  bool emit (rich_location *rich_loc) final override
-  {
-    diagnostic_metadata m;
-    m.add_cwe (126);
-    return warning_meta (rich_loc, m, get_controlling_option (),
-			 "buffer overread");
-  }
-};
-
-/* Check whether an access is past the end of the BASE_REG.  */
-
-void
-region_model::check_symbolic_bounds (const region *base_reg,
-				     const svalue *sym_byte_offset,
-				     const svalue *num_bytes_sval,
-				     const svalue *capacity,
-				     enum access_direction dir,
-				     region_model_context *ctxt) const
-{
-  gcc_assert (ctxt);
-
-  const svalue *next_byte
-    = m_mgr->get_or_create_binop (num_bytes_sval->get_type (), PLUS_EXPR,
-				  sym_byte_offset, num_bytes_sval);
-
-  if (eval_condition (next_byte, GT_EXPR, capacity).is_true ())
-    {
-      tree diag_arg = get_representative_tree (base_reg);
-      tree offset_tree = get_representative_tree (sym_byte_offset);
-      tree num_bytes_tree = get_representative_tree (num_bytes_sval);
-      tree capacity_tree = get_representative_tree (capacity);
-      switch (dir)
-	{
-	default:
-	  gcc_unreachable ();
-	  break;
-	case DIR_READ:
-	  ctxt->warn (make_unique<symbolic_buffer_overread> (base_reg,
-							     diag_arg,
-							     offset_tree,
-							     num_bytes_tree,
-							     capacity_tree));
-	  break;
-	case DIR_WRITE:
-	  ctxt->warn (make_unique<symbolic_buffer_overflow> (base_reg,
-							     diag_arg,
-							     offset_tree,
-							     num_bytes_tree,
-							     capacity_tree));
-	  break;
-	}
-    }
-}
-
-static tree
-maybe_get_integer_cst_tree (const svalue *sval)
-{
-  tree cst_tree = sval->maybe_get_constant ();
-  if (cst_tree && TREE_CODE (cst_tree) == INTEGER_CST)
-    return cst_tree;
-
-  return NULL_TREE;
-}
-
-/* May complain when the access on REG is out-of-bounds.  */
-
-void
-region_model::check_region_bounds (const region *reg,
-				   enum access_direction dir,
-				   region_model_context *ctxt) const
-{
-  gcc_assert (ctxt);
-
-  /* Get the offset.  */
-  region_offset reg_offset = reg->get_offset (m_mgr);
-  const region *base_reg = reg_offset.get_base_region ();
-
-  /* Bail out on symbolic regions.
-     (e.g. because the analyzer did not see previous offsets on the latter,
-     it might think that a negative access is before the buffer).  */
-  if (base_reg->symbolic_p ())
-    return;
-
-  /* Find out how many bytes were accessed.  */
-  const svalue *num_bytes_sval = reg->get_byte_size_sval (m_mgr);
-  tree num_bytes_tree = maybe_get_integer_cst_tree (num_bytes_sval);
-  /* Bail out if 0 bytes are accessed.  */
-  if (num_bytes_tree && zerop (num_bytes_tree))
-    return;
-
-  /* Get the capacity of the buffer.  */
-  const svalue *capacity = get_capacity (base_reg);
-  tree cst_capacity_tree = maybe_get_integer_cst_tree (capacity);
-
-  /* The constant offset from a pointer is represented internally as a sizetype
-     but should be interpreted as a signed value here.  The statement below
-     converts the offset from bits to bytes and then to a signed integer with
-     the same precision the sizetype has on the target system.
-
-     For example, this is needed for out-of-bounds-3.c test1 to pass when
-     compiled with a 64-bit gcc build targeting 32-bit systems.  */
-  byte_offset_t offset;
-  if (!reg_offset.symbolic_p ())
-    offset = wi::sext (reg_offset.get_bit_offset () >> LOG2_BITS_PER_UNIT,
-		       TYPE_PRECISION (size_type_node));
-
-  /* If either the offset or the number of bytes accessed are symbolic,
-     we have to reason about symbolic values.  */
-  if (reg_offset.symbolic_p () || !num_bytes_tree)
-    {
-      const svalue* byte_offset_sval;
-      if (!reg_offset.symbolic_p ())
-	{
-	  tree offset_tree = wide_int_to_tree (integer_type_node, offset);
-	  byte_offset_sval
-	    = m_mgr->get_or_create_constant_svalue (offset_tree);
-	}
-      else
-	byte_offset_sval = reg_offset.get_symbolic_byte_offset ();
-      check_symbolic_bounds (base_reg, byte_offset_sval, num_bytes_sval,
-			     capacity, dir, ctxt);
-      return;
-    }
-
-  /* Otherwise continue to check with concrete values.  */
-  byte_range out (0, 0);
-  /* NUM_BYTES_TREE should always be interpreted as unsigned.  */
-  byte_offset_t num_bytes_unsigned = wi::to_offset (num_bytes_tree);
-  byte_range read_bytes (offset, num_bytes_unsigned);
-  /* If read_bytes has a subset < 0, we do have an underflow.  */
-  if (read_bytes.falls_short_of_p (0, &out))
-    {
-      tree diag_arg = get_representative_tree (base_reg);
-      switch (dir)
-	{
-	default:
-	  gcc_unreachable ();
-	  break;
-	case DIR_READ:
-	  ctxt->warn (make_unique<buffer_underread> (reg, diag_arg, out));
-	  break;
-	case DIR_WRITE:
-	  ctxt->warn (make_unique<buffer_underflow> (reg, diag_arg, out));
-	  break;
-	}
-    }
-
-  /* For accesses past the end, we do need a concrete capacity.  No need to
-     do a symbolic check here because the inequality check does not reason
-     whether constants are greater than symbolic values.  */
-  if (!cst_capacity_tree)
-    return;
-
-  byte_range buffer (0, wi::to_offset (cst_capacity_tree));
-  /* If READ_BYTES exceeds BUFFER, we do have an overflow.  */
-  if (read_bytes.exceeds_p (buffer, &out))
-    {
-      tree byte_bound = wide_int_to_tree (size_type_node,
-					  buffer.get_next_byte_offset ());
-      tree diag_arg = get_representative_tree (base_reg);
-
-      switch (dir)
-	{
-	default:
-	  gcc_unreachable ();
-	  break;
-	case DIR_READ:
-	  ctxt->warn (make_unique<buffer_overread> (reg, diag_arg,
-						    out, byte_bound));
-	  break;
-	case DIR_WRITE:
-	  ctxt->warn (make_unique<buffer_overflow> (reg, diag_arg,
-						    out, byte_bound));
-	  break;
-	}
-    }
-}
-
 /* Ensure that all arguments at the call described by CD are checked
    for poisoned values, by calling get_rvalue on each argument.  */
 
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index 86c42a24ff2..86bfb4ff8a0 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -606,6 +606,8 @@ private:
 			      region_model_context *ctxt) const;
   void check_region_size (const region *lhs_reg, const svalue *rhs_sval,
 			  region_model_context *ctxt) const;
+
+  /* Implemented in bounds-checking.cc  */
   void check_symbolic_bounds (const region *base_reg,
 			      const svalue *sym_byte_offset,
 			      const svalue *num_bytes_sval,
-- 
2.26.3


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [committed 2/7] analyzer: fix wording of 'number of bad bytes' note [PR106626]
  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 ` David Malcolm
  2022-12-01  2:41 ` [committed 3/7] analyzer: add note about valid subscripts [PR106626] David Malcolm
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: David Malcolm @ 2022-12-01  2:41 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

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".

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

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>
---
 gcc/analyzer/bounds-checking.cc               | 66 ++++++++++++-------
 .../analyzer/out-of-bounds-read-char-arr.c    | 11 +---
 .../analyzer/out-of-bounds-read-int-arr.c     |  8 +--
 .../analyzer/out-of-bounds-write-char-arr.c   | 11 +---
 .../analyzer/out-of-bounds-write-int-arr.c    |  8 +--
 5 files changed, 56 insertions(+), 48 deletions(-)

diff --git a/gcc/analyzer/bounds-checking.cc b/gcc/analyzer/bounds-checking.cc
index 19aaa51e6a8..ad7f431ea2f 100644
--- 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 61cbfc75c11..2b43000f833 100644
--- 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 0bb30d24e9f..0dc95563620 100644
--- 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 47fbc5207ee..3564476c322 100644
--- 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 bf9760ee978..24a9a6bfa18 100644
--- 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 } */
 }
-- 
2.26.3


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [committed 3/7] analyzer: add note about valid subscripts [PR106626]
  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 ` David Malcolm
  2022-12-01  2:41 ` [committed 4/7] analyzer: more bounds-checking wording tweaks [PR106626] David Malcolm
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: David Malcolm @ 2022-12-01  2:41 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

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

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

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>
---
 gcc/analyzer/bounds-checking.cc               | 46 +++++++++++++++++--
 .../gcc.dg/analyzer/call-summaries-2.c        |  1 +
 .../gcc.dg/analyzer/out-of-bounds-1.c         | 16 ++++---
 .../analyzer/out-of-bounds-write-char-arr.c   |  6 +++
 .../analyzer/out-of-bounds-write-int-arr.c    |  6 +++
 5 files changed, 64 insertions(+), 11 deletions(-)

diff --git a/gcc/analyzer/bounds-checking.cc b/gcc/analyzer/bounds-checking.cc
index ad7f431ea2f..b02bc79a926 100644
--- a/gcc/analyzer/bounds-checking.cc
+++ b/gcc/analyzer/bounds-checking.cc
@@ -71,6 +71,34 @@ public:
   }
 
 protected:
+  /* Potentially add a note about valid ways to index this array, such
+     as (given "int arr[10];"):
+       note: valid subscripts for 'arr' are '[0]' to '[9]'
+     We print the '[' and ']' characters so as to express the valid
+     subscripts using C syntax, rather than just as byte ranges,
+     which hopefully is more clear to the user.  */
+  void
+  maybe_describe_array_bounds (location_t loc) const
+  {
+    if (!m_diag_arg)
+      return;
+    tree t = TREE_TYPE (m_diag_arg);
+    if (!t)
+      return;
+    if (TREE_CODE (t) != ARRAY_TYPE)
+      return;
+    tree domain = TYPE_DOMAIN (t);
+    if (!domain)
+      return;
+    tree max_idx = TYPE_MAX_VALUE (domain);
+    if (!max_idx)
+      return;
+    tree min_idx = TYPE_MIN_VALUE (domain);
+    inform (loc,
+	    "valid subscripts for %qE are %<[%E]%> to %<[%E]%>",
+	    m_diag_arg, min_idx, max_idx);
+  }
+
   const region *m_reg;
   tree m_diag_arg;
   byte_range m_out_of_bounds_range;
@@ -165,6 +193,8 @@ public:
 	  inform (rich_loc->get_loc (),
 		  "write to beyond the end of %qE",
 		  m_diag_arg);
+
+	maybe_describe_array_bounds (rich_loc->get_loc ());
       }
 
     return warned;
@@ -245,6 +275,8 @@ public:
 	  inform (rich_loc->get_loc (),
 		  "read from after the end of %qE",
 		  m_diag_arg);
+
+	maybe_describe_array_bounds (rich_loc->get_loc ());
       }
 
     return warned;
@@ -297,8 +329,11 @@ public:
   {
     diagnostic_metadata m;
     m.add_cwe (124);
-    return warning_meta (rich_loc, m, get_controlling_option (),
-			 "buffer underflow");
+    bool warned = warning_meta (rich_loc, m, get_controlling_option (),
+				"buffer underflow");
+    if (warned)
+      maybe_describe_array_bounds (rich_loc->get_loc ());
+    return warned;
   }
 
   label_text describe_final_event (const evdesc::final_event &ev)
@@ -346,8 +381,11 @@ public:
   {
     diagnostic_metadata m;
     m.add_cwe (127);
-    return warning_meta (rich_loc, m, get_controlling_option (),
-			 "buffer underread");
+    bool warned = warning_meta (rich_loc, m, get_controlling_option (),
+				"buffer underread");
+    if (warned)
+      maybe_describe_array_bounds (rich_loc->get_loc ());
+    return warned;
   }
 
   label_text describe_final_event (const evdesc::final_event &ev)
diff --git a/gcc/testsuite/gcc.dg/analyzer/call-summaries-2.c b/gcc/testsuite/gcc.dg/analyzer/call-summaries-2.c
index 953cbd32f5a..a7a17dbd358 100644
--- a/gcc/testsuite/gcc.dg/analyzer/call-summaries-2.c
+++ b/gcc/testsuite/gcc.dg/analyzer/call-summaries-2.c
@@ -330,6 +330,7 @@ int test_returns_element_ptr (int j)
   __analyzer_eval (*returns_element_ptr (1) == 8); /* { dg-warning "TRUE" } */
   __analyzer_eval (*returns_element_ptr (2) == 9); /* { dg-warning "TRUE" } */
   return *returns_element_ptr (3); /* { dg-warning "buffer overread" } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[2\\\]'" "valid subscript note" { target *-*-* } .-1 } */
 }
 
 int returns_offset (int arr[3], int i)
diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-1.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-1.c
index 9f3cda6e02b..dc4de9b28a6 100644
--- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-1.c
@@ -25,8 +25,9 @@ void test1 (void)
   id_sequence[2] = 345;
   id_sequence[3] = 456; /* { dg-line test1 } */
 
-  /* { dg-warning "overflow" "warning" { target *-*-* } test1 } */
-  /* { dg-message "" "note" { target *-*-* } test1 } */
+  /* { dg-warning "stack-based buffer overflow" "warning" { target *-*-* } test1 } */
+  /* { dg-message "write of 4 bytes to beyond the end of 'id_sequence'" "num bad bytes note" { target *-*-* } test1 } */
+  /* { dg-message "valid subscripts for 'id_sequence' are '\\\[0\\\]' to '\\\[2\\\]'" "valid subscript note" { target *-*-* } test1 } */
 }
 
 void test2 (void)
@@ -46,8 +47,9 @@ void test3 (void)
   for (int i = n; i >= 0; i--)
     arr[i] = i; /* { dg-line test3 } */
 
-  /* { dg-warning "overflow" "warning" { target *-*-* } test3 } */
-  /* { dg-message "" "note" { target *-*-* } test3 } */
+  /* { dg-warning "stack-based buffer overflow" "warning" { target *-*-* } test3 } */
+  /* { dg-message "write of 4 bytes to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } test3 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[3\\\]'" "valid subscript note" { target *-*-* } test3 } */
 }
 
 void test4 (void)
@@ -72,7 +74,7 @@ void test5 (void)
   *last_el = 4; /* { dg-line test5 } */
 
   free (arr);
-  /* { dg-warning "overflow" "warning" { target *-*-* } test5 } */
+  /* { dg-warning "heap-based buffer overflow" "warning" { target *-*-* } test5 } */
   /* { dg-message "" "note" { target *-*-* } test5 } */
 }
 
@@ -89,9 +91,9 @@ void test6 (void)
     printf ("x=%d y=%d *p=%d *q=%d\n" , x, y, *p, *q);  /* { dg-line test6c } */
   }
 
-  /* { dg-warning "overflow" "warning" { target *-*-* } test6b } */
+  /* { dg-warning "buffer overflow" "warning" { target *-*-* } test6b } */
   /* { dg-message "" "note" { target *-*-* } test6b } */
-  /* { dg-warning "overread" "warning" { target *-*-* } test6c } */
+  /* { dg-warning "buffer overread" "warning" { target *-*-* } test6c } */
   /* { dg-message "" "note" { target *-*-* } test6c } */
 }
 
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 3564476c322..739ebb11590 100644
--- 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
@@ -4,18 +4,21 @@ void int_arr_write_element_before_start_far(char x)
 {
   arr[-100] = x; /* { dg-warning "buffer underflow" "warning" } */
   /* { dg-message "out-of-bounds write at byte -100 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
 }
 
 void int_arr_write_element_before_start_near(char x)
 {
   arr[-2] = x; /* { dg-warning "buffer underflow" "warning" } */
   /* { dg-message "out-of-bounds write at byte -2 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
 }
 
 void int_arr_write_element_before_start_off_by_one(char x)
 {
   arr[-1] = x; /* { dg-warning "buffer underflow" "warning" } */
   /* { dg-message "out-of-bounds write at byte -1 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
 }
 
 void int_arr_write_element_at_start(char x)
@@ -33,6 +36,7 @@ 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 of 1 byte to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */
 }
 
 void int_arr_write_element_after_end_near(char x)
@@ -40,6 +44,7 @@ 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 of 1 byte to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */
 }
 
 void int_arr_write_element_after_end_far(char x)
@@ -47,4 +52,5 @@ 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 of 1 byte to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */
 }
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 24a9a6bfa18..b2b37b92e01 100644
--- 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
@@ -6,18 +6,21 @@ void int_arr_write_element_before_start_far(int32_t x)
 {
   arr[-100] = x; /* { dg-warning "buffer underflow" "warning" } */
   /* { dg-message "out-of-bounds write from byte -400 till byte -397 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
 }
 
 void int_arr_write_element_before_start_near(int32_t x)
 {
   arr[-2] = x; /* { dg-warning "buffer underflow" "warning" } */
   /* { dg-message "out-of-bounds write from byte -8 till byte -5 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
 }
 
 void int_arr_write_element_before_start_off_by_one(int32_t x)
 {
   arr[-1] = x; /* { dg-warning "buffer underflow" "warning" } */
   /* { dg-message "out-of-bounds write from byte -4 till byte -1 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
 }
 
 void int_arr_write_element_at_start(int32_t x)
@@ -35,6 +38,7 @@ 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 of 4 bytes to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */
 }
 
 void int_arr_write_element_after_end_near(int32_t x)
@@ -42,6 +46,7 @@ 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 of 4 bytes to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */
 }
 
 void int_arr_write_element_after_end_far(int32_t x)
@@ -49,4 +54,5 @@ 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 of 4 bytes to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */
 }
-- 
2.26.3


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [committed 4/7] analyzer: more bounds-checking wording tweaks [PR106626]
  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 ` David Malcolm
  2022-12-01  2:41 ` [committed 5/7] diagnostics: tweak diagnostic_path::interprocedural_p [PR106626] David Malcolm
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: David Malcolm @ 2022-12-01  2:41 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

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"

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

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>
---
 gcc/analyzer/bounds-checking.cc               | 133 +++++++++++++-----
 .../gcc.dg/analyzer/call-summaries-2.c        |   2 +-
 .../gcc.dg/analyzer/out-of-bounds-1.c         |   4 +-
 .../gcc.dg/analyzer/out-of-bounds-2.c         |  15 +-
 .../gcc.dg/analyzer/out-of-bounds-3.c         |  27 ++--
 .../gcc.dg/analyzer/out-of-bounds-4.c         |  15 +-
 .../gcc.dg/analyzer/out-of-bounds-5.c         |  20 +--
 .../analyzer/out-of-bounds-container_of.c     |   4 +-
 .../analyzer/out-of-bounds-read-char-arr.c    |  34 +++--
 .../analyzer/out-of-bounds-read-int-arr.c     |  18 ++-
 .../analyzer/out-of-bounds-read-struct-arr.c  |  65 +++++++++
 .../analyzer/out-of-bounds-write-char-arr.c   |  22 +--
 .../analyzer/out-of-bounds-write-int-arr.c    |   6 +-
 .../analyzer/out-of-bounds-write-struct-arr.c |  65 +++++++++
 gcc/testsuite/gcc.dg/analyzer/pr101962.c      |   2 +-
 gcc/testsuite/gcc.dg/analyzer/realloc-5.c     |   2 +-
 gcc/testsuite/gcc.dg/analyzer/zlib-3.c        |   2 +-
 17 files changed, 327 insertions(+), 109 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-struct-arr.c
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-struct-arr.c

diff --git a/gcc/analyzer/bounds-checking.cc b/gcc/analyzer/bounds-checking.cc
index b02bc79a926..bc7d2dd17ae 100644
--- a/gcc/analyzer/bounds-checking.cc
+++ b/gcc/analyzer/bounds-checking.cc
@@ -71,6 +71,11 @@ public:
   }
 
 protected:
+  enum memory_space get_memory_space () const
+  {
+    return m_reg->get_memory_space ();
+  }
+
   /* Potentially add a note about valid ways to index this array, such
      as (given "int arr[10];"):
        note: valid subscripts for 'arr' are '[0]' to '[9]'
@@ -150,7 +155,7 @@ public:
   {
     diagnostic_metadata m;
     bool warned;
-    switch (m_reg->get_memory_space ())
+    switch (get_memory_space ())
       {
       default:
 	m.add_cwe (787);
@@ -234,22 +239,36 @@ public:
   }
 };
 
-/* Concrete subclass to complain about buffer overreads.  */
+/* Concrete subclass to complain about buffer over-reads.  */
 
-class buffer_overread : public past_the_end
+class buffer_over_read : public past_the_end
 {
 public:
-  buffer_overread (const region *reg, tree diag_arg,
-		   byte_range range, tree byte_bound)
+  buffer_over_read (const region *reg, tree diag_arg,
+		    byte_range range, tree byte_bound)
   : past_the_end (reg, diag_arg, range, byte_bound)
   {}
 
   bool emit (rich_location *rich_loc) final override
   {
     diagnostic_metadata m;
+    bool warned;
     m.add_cwe (126);
-    bool warned = warning_meta (rich_loc, m, get_controlling_option (),
-				"buffer overread");
+    switch (get_memory_space ())
+      {
+      default:
+	warned = warning_meta (rich_loc, m, get_controlling_option (),
+			       "buffer over-read");
+	break;
+      case MEMSPACE_STACK:
+	warned = warning_meta (rich_loc, m, get_controlling_option (),
+			       "stack-based buffer over-read");
+	break;
+      case MEMSPACE_HEAP:
+	warned = warning_meta (rich_loc, m, get_controlling_option (),
+			       "heap-based buffer over-read");
+	break;
+      }
 
     if (warned)
       {
@@ -316,21 +335,35 @@ public:
   }
 };
 
-/* Concrete subclass to complain about buffer underflows.  */
+/* Concrete subclass to complain about buffer underwrites.  */
 
-class buffer_underflow : public out_of_bounds
+class buffer_underwrite : public out_of_bounds
 {
 public:
-  buffer_underflow (const region *reg, tree diag_arg, byte_range range)
+  buffer_underwrite (const region *reg, tree diag_arg, byte_range range)
   : out_of_bounds (reg, diag_arg, range)
   {}
 
   bool emit (rich_location *rich_loc) final override
   {
     diagnostic_metadata m;
+    bool warned;
     m.add_cwe (124);
-    bool warned = warning_meta (rich_loc, m, get_controlling_option (),
-				"buffer underflow");
+    switch (get_memory_space ())
+      {
+      default:
+	warned = warning_meta (rich_loc, m, get_controlling_option (),
+			       "buffer underwrite");
+	break;
+      case MEMSPACE_STACK:
+	warned = warning_meta (rich_loc, m, get_controlling_option (),
+			       "stack-based buffer underwrite");
+	break;
+      case MEMSPACE_HEAP:
+	warned = warning_meta (rich_loc, m, get_controlling_option (),
+			       "heap-based buffer underwrite");
+	break;
+      }
     if (warned)
       maybe_describe_array_bounds (rich_loc->get_loc ());
     return warned;
@@ -368,21 +401,35 @@ public:
   }
 };
 
-/* Concrete subclass to complain about buffer underreads.  */
+/* Concrete subclass to complain about buffer under-reads.  */
 
-class buffer_underread : public out_of_bounds
+class buffer_under_read : public out_of_bounds
 {
 public:
-  buffer_underread (const region *reg, tree diag_arg, byte_range range)
+  buffer_under_read (const region *reg, tree diag_arg, byte_range range)
   : out_of_bounds (reg, diag_arg, range)
   {}
 
   bool emit (rich_location *rich_loc) final override
   {
     diagnostic_metadata m;
+    bool warned;
     m.add_cwe (127);
-    bool warned = warning_meta (rich_loc, m, get_controlling_option (),
-				"buffer underread");
+    switch (get_memory_space ())
+      {
+      default:
+	warned = warning_meta (rich_loc, m, get_controlling_option (),
+			       "buffer under-read");
+	break;
+      case MEMSPACE_STACK:
+	warned = warning_meta (rich_loc, m, get_controlling_option (),
+			       "stack-based buffer under-read");
+	break;
+      case MEMSPACE_HEAP:
+	warned = warning_meta (rich_loc, m, get_controlling_option (),
+			       "heap-based buffer under-read");
+	break;
+      }
     if (warned)
       maybe_describe_array_bounds (rich_loc->get_loc ());
     return warned;
@@ -519,6 +566,11 @@ public:
   }
 
 protected:
+  enum memory_space get_memory_space () const
+  {
+    return m_reg->get_memory_space ();
+  }
+
   const region *m_reg;
   tree m_diag_arg;
   tree m_offset;
@@ -542,7 +594,7 @@ public:
   bool emit (rich_location *rich_loc) final override
   {
     diagnostic_metadata m;
-    switch (m_reg->get_memory_space ())
+    switch (get_memory_space ())
       {
       default:
 	m.add_cwe (787);
@@ -560,13 +612,13 @@ public:
   }
 };
 
-/* Concrete subclass to complain about overreads with symbolic values.  */
+/* Concrete subclass to complain about over-reads with symbolic values.  */
 
-class symbolic_buffer_overread : public symbolic_past_the_end
+class symbolic_buffer_over_read : public symbolic_past_the_end
 {
 public:
-  symbolic_buffer_overread (const region *reg, tree diag_arg, tree offset,
-			    tree num_bytes, tree capacity)
+  symbolic_buffer_over_read (const region *reg, tree diag_arg, tree offset,
+			     tree num_bytes, tree capacity)
   : symbolic_past_the_end (reg, diag_arg, offset, num_bytes, capacity)
   {
     m_dir_str = "read";
@@ -576,8 +628,21 @@ public:
   {
     diagnostic_metadata m;
     m.add_cwe (126);
-    return warning_meta (rich_loc, m, get_controlling_option (),
-			 "buffer overread");
+    switch (get_memory_space ())
+      {
+      default:
+	m.add_cwe (787);
+	return warning_meta (rich_loc, m, get_controlling_option (),
+			     "buffer over-read");
+      case MEMSPACE_STACK:
+	m.add_cwe (121);
+	return warning_meta (rich_loc, m, get_controlling_option (),
+			     "stack-based buffer over-read");
+      case MEMSPACE_HEAP:
+	m.add_cwe (122);
+	return warning_meta (rich_loc, m, get_controlling_option (),
+			     "heap-based buffer over-read");
+      }
   }
 };
 
@@ -609,11 +674,11 @@ region_model::check_symbolic_bounds (const region *base_reg,
 	  gcc_unreachable ();
 	  break;
 	case DIR_READ:
-	  ctxt->warn (make_unique<symbolic_buffer_overread> (base_reg,
-							     diag_arg,
-							     offset_tree,
-							     num_bytes_tree,
-							     capacity_tree));
+	  ctxt->warn (make_unique<symbolic_buffer_over_read> (base_reg,
+							      diag_arg,
+							      offset_tree,
+							      num_bytes_tree,
+							      capacity_tree));
 	  break;
 	case DIR_WRITE:
 	  ctxt->warn (make_unique<symbolic_buffer_overflow> (base_reg,
@@ -701,7 +766,7 @@ region_model::check_region_bounds (const region *reg,
   /* NUM_BYTES_TREE should always be interpreted as unsigned.  */
   byte_offset_t num_bytes_unsigned = wi::to_offset (num_bytes_tree);
   byte_range read_bytes (offset, num_bytes_unsigned);
-  /* If read_bytes has a subset < 0, we do have an underflow.  */
+  /* If read_bytes has a subset < 0, we do have an underwrite.  */
   if (read_bytes.falls_short_of_p (0, &out))
     {
       tree diag_arg = get_representative_tree (base_reg);
@@ -711,10 +776,10 @@ region_model::check_region_bounds (const region *reg,
 	  gcc_unreachable ();
 	  break;
 	case DIR_READ:
-	  ctxt->warn (make_unique<buffer_underread> (reg, diag_arg, out));
+	  ctxt->warn (make_unique<buffer_under_read> (reg, diag_arg, out));
 	  break;
 	case DIR_WRITE:
-	  ctxt->warn (make_unique<buffer_underflow> (reg, diag_arg, out));
+	  ctxt->warn (make_unique<buffer_underwrite> (reg, diag_arg, out));
 	  break;
 	}
     }
@@ -739,8 +804,8 @@ region_model::check_region_bounds (const region *reg,
 	  gcc_unreachable ();
 	  break;
 	case DIR_READ:
-	  ctxt->warn (make_unique<buffer_overread> (reg, diag_arg,
-						    out, byte_bound));
+	  ctxt->warn (make_unique<buffer_over_read> (reg, diag_arg,
+						     out, byte_bound));
 	  break;
 	case DIR_WRITE:
 	  ctxt->warn (make_unique<buffer_overflow> (reg, diag_arg,
diff --git a/gcc/testsuite/gcc.dg/analyzer/call-summaries-2.c b/gcc/testsuite/gcc.dg/analyzer/call-summaries-2.c
index a7a17dbd358..22ca475b2ed 100644
--- a/gcc/testsuite/gcc.dg/analyzer/call-summaries-2.c
+++ b/gcc/testsuite/gcc.dg/analyzer/call-summaries-2.c
@@ -329,7 +329,7 @@ int test_returns_element_ptr (int j)
   __analyzer_eval (*returns_element_ptr (0) == 7); /* { dg-warning "TRUE" } */
   __analyzer_eval (*returns_element_ptr (1) == 8); /* { dg-warning "TRUE" } */
   __analyzer_eval (*returns_element_ptr (2) == 9); /* { dg-warning "TRUE" } */
-  return *returns_element_ptr (3); /* { dg-warning "buffer overread" } */
+  return *returns_element_ptr (3); /* { dg-warning "buffer over-read" } */
   /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[2\\\]'" "valid subscript note" { target *-*-* } .-1 } */
 }
 
diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-1.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-1.c
index dc4de9b28a6..977476ed2fb 100644
--- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-1.c
@@ -93,7 +93,7 @@ void test6 (void)
 
   /* { dg-warning "buffer overflow" "warning" { target *-*-* } test6b } */
   /* { dg-message "" "note" { target *-*-* } test6b } */
-  /* { dg-warning "buffer overread" "warning" { target *-*-* } test6c } */
+  /* { dg-warning "buffer over-read" "warning" { target *-*-* } test6c } */
   /* { dg-message "" "note" { target *-*-* } test6c } */
 }
 
@@ -116,7 +116,7 @@ void test7 (void)
   fn (destBuf, srcBuf, returnChunkSize (destBuf)); /* { dg-line test7 } */
 
   // TODO: Should we handle widening_svalues as a follow-up?
-  /* { dg-warning "overread" "warning" { xfail *-*-* } test7 } */
+  /* { dg-warning "over-read" "warning" { xfail *-*-* } test7 } */
   /* { dg-warning "overflow" "warning" { xfail *-*-* } test7 } */
   /* { dg-message "" "note" { xfail *-*-* } test7 } */
 }
diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-2.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-2.c
index 0df9364c5c1..1330090f348 100644
--- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-2.c
+++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-2.c
@@ -3,7 +3,7 @@
 #include <stdio.h>
 #include <stdint.h>
 
-/* Wanalyzer-out-of-bounds tests for buffer overreads.  */
+/* Wanalyzer-out-of-bounds tests for buffer over-reads.  */
 
 /* Avoid folding of memcpy.  */
 typedef void * (*memcpy_t) (void *dst, const void *src, size_t n);
@@ -21,8 +21,9 @@ void test1 (void)
   memset (id_sequence, 0, 3 * sizeof(int));
   printf ("%i", id_sequence[3]); /* { dg-line test1 } */
 
-  /* { dg-warning "overread" "warning" { target *-*-* } test1 } */
-  /* { dg-message "" "note" { target *-*-* } test1 } */
+  /* { dg-warning "stack-based buffer over-read" "warning" { target *-*-* } test1 } */
+  /* { dg-message "read of 4 bytes from after the end of 'id_sequence'" "num bad bytes note" { target *-*-* } test1 } */
+  /* { dg-message "valid subscripts for 'id_sequence' are '\\\[0\\\]' to '\\\[2\\\]'" "valid subscript note" { target *-*-* } test1 } */
 }
 
 void test2 (void)
@@ -46,7 +47,7 @@ void test3 (void)
   for (int i = n; i > 0; i--)
     sum += arr[i]; /* { dg-line test3 } */
 
-  /* { dg-warning "overread" "warning" { target *-*-* } test3 } */
+  /* { dg-warning "stack-based buffer over-read" "warning" { target *-*-* } test3 } */
   /* { dg-message "" "note" { target *-*-* } test3 } */
 }
 
@@ -78,6 +79,8 @@ void test5 (void)
     sum += *(arr + i); /* { dg-line test5 } */
 
   free (arr);
-  /* { dg-warning "overread" "warning" { target *-*-* } test5 } */
-  /* { dg-message "" "note" { target *-*-* } test5 } */
+  /* { dg-warning "heap-based buffer over-read" "bounds warning" { target *-*-* } test5 } */
+  /* { dg-message "read of 4 bytes from after the end of the region" "num bad bytes note" { target *-*-* } test5 } */
+
+  /* { dg-warning "use of uninitialized value" "uninit warning" { target *-*-* } test5 } */
 }
diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-3.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-3.c
index 7446b182e48..5fd9cc3c6b1 100644
--- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-3.c
+++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-3.c
@@ -2,7 +2,7 @@
 #include <string.h>
 #include <stdint.h>
 
-/* Wanalyzer-out-of-bounds tests for buffer underreads and writes.  */
+/* Wanalyzer-out-of-bounds tests for buffer under-reads and underwrites.  */
 
 /* Avoid folding of memcpy.  */
 typedef void * (*memcpy_t) (void *dst, const void *src, size_t n);
@@ -19,8 +19,9 @@ void test1 (void)
   int *e = buf - 1;
   *e = 42; /* { dg-line test1 } */
 
-  /* { dg-warning "underflow" "warning" { target *-*-* } test1 } */
-  /* { dg-message "" "note" { target *-*-* } test1 } */
+  /* { dg-warning "stack-based buffer underwrite" "warning" { target *-*-* } test1 } */
+  /* { dg-message "out-of-bounds write from byte -4 till byte -1 but 'buf' starts at byte 0" "final event" { target *-*-* } test1 } */
+  /* { dg-message "valid subscripts for 'buf' are '\\\[0\\\]' to '\\\[3\\\]'" "valid subscript note" { target *-*-* } test1 } */
 }
 
 void test2 (void)
@@ -38,8 +39,9 @@ void test3 (void)
   *e = 123;
   *(e - 2) = 321; /* { dg-line test3 } */
 
-  /* { dg-warning "underflow" "warning" { target *-*-* } test3 } */
-  /* { dg-message "" "note" { target *-*-* } test3 } */
+  /* { dg-warning "stack-based buffer underwrite" "warning" { target *-*-* } test3 } */
+  /* { dg-message "out-of-bounds write from byte -4 till byte -1 but 'buf' starts at byte 0" "final event" { target *-*-* } test3 } */
+  /* { dg-message "valid subscripts for 'buf' are '\\\[0\\\]' to '\\\[3\\\]'" "valid subscript note" { target *-*-* } test3 } */
 }
 
 void test4 (void)
@@ -50,8 +52,9 @@ void test4 (void)
   int n = -4;
   fn (&(buf[n]), buf, sizeof (int));  /* { dg-line test4 } */
 
-  /* { dg-warning "underflow" "warning" { target *-*-* } test4 } */
-  /* { dg-message "" "note" { target *-*-* } test4 } */
+  /* { dg-warning "stack-based buffer underwrite" "warning" { target *-*-* } test4 } */
+  /* { dg-message "out-of-bounds write from byte -16 till byte -13 but 'buf' starts at byte 0" "final event" { target *-*-* } test4 } */
+  /* { dg-message "valid subscripts for 'buf' are '\\\[0\\\]' to '\\\[3\\\]'" "valid subscript note" { target *-*-* } test4 } */
 }
 
 void test5 (void)
@@ -63,8 +66,9 @@ void test5 (void)
   for (int i = 4; i >= 0; i++)
     sum += *(buf - i); /* { dg-line test5 } */
 
-  /* { dg-warning "underread" "warning" { target *-*-* } test5 } */
-  /* { dg-message "" "note" { target *-*-* } test5 } */
+  /* { dg-warning "stack-based buffer under-read" "warning" { target *-*-* } test5 } */
+  /* { dg-message "out-of-bounds read from byte -16 till byte -13 but 'buf' starts at byte 0" "final event" { target *-*-* } test5 } */
+  /* { dg-message "valid subscripts for 'buf' are '\\\[0\\\]' to '\\\[3\\\]'" "valid subscript note" { target *-*-* } test5 } */
 }
 
 void test6 (void)
@@ -86,6 +90,7 @@ void test8 (void)
   int n = -4;
   fn (buf, &(buf[n]), sizeof (int));  /* { dg-line test8 } */
 
-  /* { dg-warning "underread" "warning" { target *-*-* } test8 } */
-  /* { dg-message "" "note" { target *-*-* } test8 } */
+  /* { dg-warning "stack-based buffer under-read" "warning" { target *-*-* } test8 } */
+  /* { dg-message "out-of-bounds read from byte -16 till byte -13 but 'buf' starts at byte 0" "note" { target *-*-* } test8 } */
+  /* { dg-message "valid subscripts for 'buf' are '\\\[0\\\]' to '\\\[3\\\]'" "valid subscript note" { target *-*-* } test8 } */
 }
diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-4.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-4.c
index 46f600de658..9cd8bda76c3 100644
--- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-4.c
+++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-4.c
@@ -11,8 +11,9 @@ void test1 (void)
   char dst[5];
   strcpy (dst, "Hello"); /* { dg-line test1 } */
 
-  /* { dg-warning "overflow" "warning" { target *-*-* } test1 } */
-  /* { dg-message "dst" "note" { target *-*-* } test1 } */
+  /* { dg-warning "stack-based buffer overflow" "warning" { target *-*-* } test1 } */
+  /* { dg-message "write of 1 byte to beyond the end of 'dst'" "num bad bytes note" { target *-*-* } test1 } */
+  /* { dg-message "valid subscripts for 'dst' are '\\\[0\\\]' to '\\\[4\\\]'" "valid subscript note" { target *-*-* } test1 } */
 }
 
 void test2 (void)
@@ -27,8 +28,9 @@ void test3 (void)
   char dst[5];
   strcpy (dst, src); /* { dg-line test3 } */
 
-  /* { dg-warning "overflow" "warning" { target *-*-* } test3 } */
-  /* { dg-message "dst" "note" { target *-*-* } test3 } */
+  /* { dg-warning "stack-based buffer overflow" "warning" { target *-*-* } test3 } */
+  /* { dg-message "write of 1 byte to beyond the end of 'dst'" "num bad bytes note" { target *-*-* } test3 } */
+  /* { dg-message "valid subscripts for 'dst' are '\\\[0\\\]' to '\\\[4\\\]'" "valid subscript note" { target *-*-* } test3 } */
 }
 
 void test4 (void)
@@ -51,8 +53,9 @@ void test5 (void)
   char dst[5];
   strcpy (dst, str); /* { dg-line test5 } */
 
-  /* { dg-warning "overflow" "warning" { target *-*-* } test5 } */
-  /* { dg-message "dst" "note" { target *-*-* } test5 } */
+  /* { dg-warning "stack-based buffer overflow" "warning" { target *-*-* } test5 } */
+  /* { dg-message "write of 1 byte to beyond the end of 'dst'" "num bad bytes note" { target *-*-* } test5 } */
+  /* { dg-message "valid subscripts for 'dst' are '\\\[0\\\]' to '\\\[4\\\]'" "valid subscript note" { target *-*-* } test5 } */
 }
 
 void test6 (void)
diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-5.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-5.c
index 7dc0bc5bf18..52fea79152e 100644
--- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-5.c
+++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-5.c
@@ -13,7 +13,7 @@ void test1 (size_t size)
   char *buf = __builtin_malloc (size);
   if (!buf) return;
 
-  buf[size] = '\0'; /* { dg-warning "overflow" } */
+  buf[size] = '\0'; /* { dg-warning "heap-based buffer overflow" } */
   free (buf);
 }
 
@@ -22,7 +22,7 @@ void test2 (size_t size)
   char *buf = __builtin_malloc (size);
   if (!buf) return;
 
-  buf[size + 1] = '\0'; /* { dg-warning "overflow" } */
+  buf[size + 1] = '\0'; /* { dg-warning "heap-based buffer overflow" } */
   free (buf);
 }
 
@@ -31,33 +31,33 @@ void test3 (size_t size, size_t op)
   char *buf = __builtin_malloc (size);
   if (!buf) return;
 
-  buf[size + op] = '\0'; /* { dg-warning "overflow" } */
+  buf[size + op] = '\0'; /* { dg-warning "heap-based buffer overflow" } */
   free (buf);
 }
 
 void test4 (size_t size, unsigned short s)
 {
   char *buf = __builtin_alloca (size);
-  buf[size + s] = '\0'; /* { dg-warning "overflow" } */
+  buf[size + s] = '\0'; /* { dg-warning "stack-based buffer overflow" } */
 }
 
 void test5 (size_t size)
 {
   int32_t *buf = __builtin_alloca (4 * size);
-  buf[size] = 42; /* { dg-warning "overflow" } */
+  buf[size] = 42; /* { dg-warning "stack-based buffer overflow" } */
 }
 
 void test6 (size_t size)
 {
   int32_t *buf = __builtin_alloca (4 * size);
   memset (buf, 0, 4 * size);
-  int32_t last = *(buf + 4 * size); /* { dg-warning "overread" } */
+  int32_t last = *(buf + 4 * size); /* { dg-warning "stack-based buffer over-read" } */
 }
 
 void test7 (size_t size)
 {
   int32_t *buf = __builtin_alloca (4 * size + 3); /* { dg-warning "allocated buffer size is not a multiple of the pointee's size" } */
-  buf[size] = 42; /* { dg-warning "overflow" } */
+  buf[size] = 42; /* { dg-warning "stack-based buffer overflow" } */
 }
 
 /* Test where the offset itself is not out-of-bounds
@@ -68,7 +68,7 @@ void test8 (size_t size, size_t offset)
   char src[size];
   char dst[size];
   memcpy (dst, src, size + offset); /* { dg-line test8 } */
-  /* { dg-warning "overread" "warning" { target *-*-* } test8 } */
+  /* { dg-warning "over-read" "warning" { target *-*-* } test8 } */
   /* { dg-warning "overflow" "warning" { target *-*-* } test8 } */
 }
 
@@ -77,7 +77,7 @@ void test9 (size_t size, size_t offset)
   int32_t src[size];
   int32_t dst[size];
   memcpy (dst, src, 4 * size + 1); /* { dg-line test9 } */
-  /* { dg-warning "overread" "warning" { target *-*-* } test9 } */
+  /* { dg-warning "over-read" "warning" { target *-*-* } test9 } */
   /* { dg-warning "overflow" "warning" { target *-*-* } test9 } */
 }
 
@@ -151,6 +151,6 @@ char *test99 (const char *x, const char *y)
   __builtin_memcpy (result, x, len_x);
   __builtin_memcpy (result + len_x, y, len_y);
   /* BUG (symptom): off-by-one out-of-bounds write to heap.  */
-  result[len_x + len_y] = '\0'; /* { dg-warning "overflow" } */
+  result[len_x + len_y] = '\0'; /* { dg-warning "heap-based buffer overflow" } */
   return result;
 }
diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-container_of.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-container_of.c
index 172ec474c42..ef460f4e772 100644
--- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-container_of.c
+++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-container_of.c
@@ -44,8 +44,8 @@ int test (struct outer *outer_p, struct inner *inner_p)
   sum += o->i; /* { dg-line testB } */
 
   return sum;
-  /* { dg-warning "underread" "warning" { target *-*-* } testA } */
+  /* { dg-warning "stack-based buffer under-read" "warning" { target *-*-* } testA } */
   /* { dg-message "" "note" { target *-*-* } testA } */
-  /* { dg-warning "underread" "warning" { target *-*-* } testB } */
+  /* { dg-warning "stack-based buffer under-read" "warning" { target *-*-* } testB } */
   /* { dg-message "" "note" { target *-*-* } testB } */
 }
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 2b43000f833..f6d0dda9fb9 100644
--- 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
@@ -1,50 +1,56 @@
 char arr[10]; /* { dg-message "capacity is 10 bytes" } */
 
-char int_arr_read_element_before_start_far(void)
+char char_arr_read_element_before_start_far(void)
 {
-  return arr[-100]; /* { dg-warning "buffer underread" "warning" } */
+  return arr[-100]; /* { dg-warning "buffer under-read" "warning" } */
   /* { dg-message "out-of-bounds read at byte -100 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
 }
 
-char int_arr_read_element_before_start_near(void)
+char char_arr_read_element_before_start_near(void)
 {
-  return arr[-2]; /* { dg-warning "buffer underread" "warning" } */
+  return arr[-2]; /* { dg-warning "buffer under-read" "warning" } */
   /* { dg-message "out-of-bounds read at byte -2 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
 }
 
-char int_arr_read_element_before_start_off_by_one(void)
+char char_arr_read_element_before_start_off_by_one(void)
 {
-  return arr[-1]; /* { dg-warning "buffer underread" "warning" } */
+  return arr[-1]; /* { dg-warning "buffer under-read" "warning" } */
   /* { dg-message "out-of-bounds read at byte -1 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
 }
 
-char int_arr_read_element_at_start(void)
+char char_arr_read_element_at_start(void)
 {
   return arr[0];
 }
 
-char int_arr_read_element_at_end(void)
+char char_arr_read_element_at_end(void)
 {
   return arr[9];
 }
 
-char int_arr_read_element_after_end_off_by_one(void)
+char char_arr_read_element_after_end_off_by_one(void)
 {
-  return arr[10]; /* { dg-warning "buffer overread" "warning" } */
+  return arr[10]; /* { dg-warning "buffer over-read" "warning" } */
   /* { dg-message "out-of-bounds read at byte 10 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */
   /* { dg-message "read of 1 byte from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */
 }
 
-char int_arr_read_element_after_end_near(void)
+char char_arr_read_element_after_end_near(void)
 {
-  return arr[11]; /* { dg-warning "buffer overread" "warning" } */
+  return arr[11]; /* { dg-warning "buffer over-read" "warning" } */
   /* { dg-message "out-of-bounds read at byte 11 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */
   /* { dg-message "read of 1 byte from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */
 }
 
-char int_arr_read_element_after_end_far(void)
+char char_arr_read_element_after_end_far(void)
 {
-  return arr[100]; /* { dg-warning "buffer overread" "warning" } */
+  return arr[100]; /* { dg-warning "buffer over-read" "warning" } */
   /* { dg-message "out-of-bounds read at byte 100 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */
   /* { dg-message "read of 1 byte from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */
 }
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 0dc95563620..f1b6e119777 100644
--- 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
@@ -4,20 +4,23 @@ int32_t arr[10]; /* { dg-message "capacity is 40 bytes" } */
 
 int32_t int_arr_read_element_before_start_far(void)
 {
-  return arr[-100]; /* { dg-warning "buffer underread" "warning" } */
+  return arr[-100]; /* { dg-warning "buffer under-read" "warning" } */
   /* { dg-message "out-of-bounds read from byte -400 till byte -397 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
 }
 
 int32_t int_arr_read_element_before_start_near(void)
 {
-  return arr[-2]; /* { dg-warning "buffer underread" "warning" } */
+  return arr[-2]; /* { dg-warning "buffer under-read" "warning" } */
   /* { dg-message "out-of-bounds read from byte -8 till byte -5 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
 }
 
 int32_t int_arr_read_element_before_start_off_by_one(void)
 {
-  return arr[-1]; /* { dg-warning "buffer underread" "warning" } */
+  return arr[-1]; /* { dg-warning "buffer under-read" "warning" } */
   /* { dg-message "out-of-bounds read from byte -4 till byte -1 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
 }
 
 int32_t int_arr_read_element_at_start(void)
@@ -32,21 +35,24 @@ int32_t int_arr_read_element_at_end(void)
 
 int32_t int_arr_read_element_after_end_off_by_one(void)
 {
-  return arr[10]; /* { dg-warning "buffer overread" "warning" } */
+  return arr[10]; /* { dg-warning "buffer over-read" "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 of 4 bytes from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */
 }
 
 int32_t int_arr_read_element_after_end_near(void)
 {
-  return arr[11]; /* { dg-warning "buffer overread" "warning" } */
+  return arr[11]; /* { dg-warning "buffer over-read" "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 of 4 bytes from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */
 }
 
 int32_t int_arr_read_element_after_end_far(void)
 {
-  return arr[100]; /* { dg-warning "buffer overread" "warning" } */
+  return arr[100]; /* { dg-warning "buffer over-read" "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 of 4 bytes from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */
 }
diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-struct-arr.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-struct-arr.c
new file mode 100644
index 00000000000..0f50bb92290
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-struct-arr.c
@@ -0,0 +1,65 @@
+#include <stdint.h>
+
+struct st
+{
+  char buf[16];
+  int32_t x;
+  int32_t y;
+};
+
+struct st arr[10];
+
+int32_t struct_arr_read_x_element_before_start_far(void)
+{
+  return arr[-100].x; /* { dg-warning "buffer under-read" "warning" } */
+  /* { dg-message "out-of-bounds read from byte -2384 till byte -2381 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
+}
+
+int32_t struct_arr_read_x_element_before_start_near(void)
+{
+  return arr[-2].x; /* { dg-warning "buffer under-read" "warning" } */
+  /* { dg-message "out-of-bounds read from byte -32 till byte -29 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
+}
+
+int32_t struct_arr_read_x_element_before_start_off_by_one(void)
+{
+  return arr[-1].x; /* { dg-warning "buffer under-read" "warning" } */
+  /* { dg-message "out-of-bounds read from byte -8 till byte -5 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
+}
+
+int32_t struct_arr_read_x_element_at_start(void)
+{
+  return arr[0].x;
+}
+
+int32_t struct_arr_read_x_element_at_end(void)
+{
+  return arr[9].x;
+}
+
+int32_t struct_arr_read_x_element_after_end_off_by_one(void)
+{
+  return arr[10].x; /* { dg-warning "buffer over-read" "warning" } */
+  /* { dg-message "out-of-bounds read from byte 256 till byte 259 but 'arr' ends at byte 240" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "read of 4 bytes from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */
+}
+
+int32_t struct_arr_read_x_element_after_end_near(void)
+{
+  return arr[11].x; /* { dg-warning "buffer over-read" "warning" } */
+  /* { dg-message "out-of-bounds read from byte 280 till byte 283 but 'arr' ends at byte 240" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "read of 4 bytes from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */
+}
+
+int32_t struct_arr_read_x_element_after_end_far(void)
+{
+  return arr[100].x; /* { dg-warning "buffer over-read" "warning" } */
+  /* { dg-message "out-of-bounds read from byte 2416 till byte 2419 but 'arr' ends at byte 240" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "read of 4 bytes from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */
+}
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 739ebb11590..2f3bbfa2dc2 100644
--- 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
@@ -1,37 +1,37 @@
 char arr[10]; /* { dg-message "capacity is 10 bytes" } */
 
-void int_arr_write_element_before_start_far(char x)
+void char_arr_write_element_before_start_far(char x)
 {
-  arr[-100] = x; /* { dg-warning "buffer underflow" "warning" } */
+  arr[-100] = x; /* { dg-warning "buffer underwrite" "warning" } */
   /* { dg-message "out-of-bounds write at byte -100 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
   /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
 }
 
-void int_arr_write_element_before_start_near(char x)
+void char_arr_write_element_before_start_near(char x)
 {
-  arr[-2] = x; /* { dg-warning "buffer underflow" "warning" } */
+  arr[-2] = x; /* { dg-warning "buffer underwrite" "warning" } */
   /* { dg-message "out-of-bounds write at byte -2 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
   /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
 }
 
-void int_arr_write_element_before_start_off_by_one(char x)
+void char_arr_write_element_before_start_off_by_one(char x)
 {
-  arr[-1] = x; /* { dg-warning "buffer underflow" "warning" } */
+  arr[-1] = x; /* { dg-warning "buffer underwrite" "warning" } */
   /* { dg-message "out-of-bounds write at byte -1 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
   /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
 }
 
-void int_arr_write_element_at_start(char x)
+void char_arr_write_element_at_start(char x)
 {
   arr[0] = x;
 }
 
-void int_arr_write_element_at_end(char x)
+void char_arr_write_element_at_end(char x)
 {
   arr[9] = x;
 }
 
-void int_arr_write_element_after_end_off_by_one(char x)
+void char_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 } */
@@ -39,7 +39,7 @@ void int_arr_write_element_after_end_off_by_one(char x)
   /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */
 }
 
-void int_arr_write_element_after_end_near(char x)
+void char_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 } */
@@ -47,7 +47,7 @@ void int_arr_write_element_after_end_near(char x)
   /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */
 }
 
-void int_arr_write_element_after_end_far(char x)
+void char_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 } */
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 b2b37b92e01..0adb7899641 100644
--- 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
@@ -4,21 +4,21 @@ int32_t arr[10]; /* { dg-message "capacity is 40 bytes" } */
 
 void int_arr_write_element_before_start_far(int32_t x)
 {
-  arr[-100] = x; /* { dg-warning "buffer underflow" "warning" } */
+  arr[-100] = x; /* { dg-warning "buffer underwrite" "warning" } */
   /* { dg-message "out-of-bounds write from byte -400 till byte -397 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
   /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
 }
 
 void int_arr_write_element_before_start_near(int32_t x)
 {
-  arr[-2] = x; /* { dg-warning "buffer underflow" "warning" } */
+  arr[-2] = x; /* { dg-warning "buffer underwrite" "warning" } */
   /* { dg-message "out-of-bounds write from byte -8 till byte -5 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
   /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
 }
 
 void int_arr_write_element_before_start_off_by_one(int32_t x)
 {
-  arr[-1] = x; /* { dg-warning "buffer underflow" "warning" } */
+  arr[-1] = x; /* { dg-warning "buffer underwrite" "warning" } */
   /* { dg-message "out-of-bounds write from byte -4 till byte -1 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
   /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
 }
diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-struct-arr.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-struct-arr.c
new file mode 100644
index 00000000000..cf6b458f44b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-struct-arr.c
@@ -0,0 +1,65 @@
+#include <stdint.h>
+
+struct st
+{
+  char buf[16];
+  int32_t x;
+  int32_t y;
+};
+
+struct st arr[10];
+
+void struct_arr_write_x_element_before_start_far(int32_t x)
+{
+  arr[-100].x = x; /* { dg-warning "buffer underwrite" "warning" } */
+  /* { dg-message "out-of-bounds write from byte -2384 till byte -2381 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
+}
+
+void struct_arr_write_x_element_before_start_near(int32_t x)
+{
+  arr[-2].x = x; /* { dg-warning "buffer underwrite" "warning" } */
+  /* { dg-message "out-of-bounds write from byte -32 till byte -29 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
+}
+
+void struct_arr_write_x_element_before_start_off_by_one(int32_t x)
+{
+  arr[-1].x = x; /* { dg-warning "buffer underwrite" "warning" } */
+  /* { dg-message "out-of-bounds write from byte -8 till byte -5 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */
+}
+
+void struct_arr_write_x_element_at_start(int32_t x)
+{
+  arr[0].x = x;
+}
+
+void struct_arr_write_x_element_at_end(int32_t x)
+{
+  arr[9].x = x;
+}
+
+void struct_arr_write_x_element_after_end_off_by_one(int32_t x)
+{
+  arr[10].x = x; /* { dg-warning "buffer overflow" "warning" } */
+  /* { dg-message "out-of-bounds write from byte 256 till byte 259 but 'arr' ends at byte 240" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "write of 4 bytes to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */
+}
+
+void struct_arr_write_x_element_after_end_near(int32_t x)
+{
+  arr[11].x = x; /* { dg-warning "buffer overflow" "warning" } */
+  /* { dg-message "out-of-bounds write from byte 280 till byte 283 but 'arr' ends at byte 240" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "write of 4 bytes to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */
+}
+
+void struct_arr_write_x_element_after_end_far(int32_t x)
+{
+  arr[100].x = x; /* { dg-warning "buffer overflow" "warning" } */
+  /* { dg-message "out-of-bounds write from byte 2416 till byte 2419 but 'arr' ends at byte 240" "final event" { target *-*-* } .-1 } */
+  /* { dg-message "write of 4 bytes to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */
+  /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr101962.c b/gcc/testsuite/gcc.dg/analyzer/pr101962.c
index cf0041b2fcd..08c0aba5cbf 100644
--- a/gcc/testsuite/gcc.dg/analyzer/pr101962.c
+++ b/gcc/testsuite/gcc.dg/analyzer/pr101962.c
@@ -25,7 +25,7 @@ test_1 (void)
   return *a; /* { dg-line test_1 } */
 
   /* { dg-warning "use of uninitialized value '\\*a'" "warning" { target *-*-* } test_1 } */
-  /* { dg-warning "overread" "warning" { target *-*-* } test_1 } */
+  /* { dg-warning "stack-based buffer over-read" "warning" { target *-*-* } test_1 } */
 }
 
 static const char * __attribute__((noinline))
diff --git a/gcc/testsuite/gcc.dg/analyzer/realloc-5.c b/gcc/testsuite/gcc.dg/analyzer/realloc-5.c
index 2efe3371e12..75f0b70a996 100644
--- a/gcc/testsuite/gcc.dg/analyzer/realloc-5.c
+++ b/gcc/testsuite/gcc.dg/analyzer/realloc-5.c
@@ -37,7 +37,7 @@ void test_1 ()
       __analyzer_eval (q[8] == 1); /* { dg-line eval } */
     
       /* { dg-warning "UNKNOWN" "warning" { target *-*-* } eval } */
-      /* { dg-warning "overread" "warning" { target *-*-* } eval } */
+      /* { dg-warning "heap-based buffer over-read" "warning" { target *-*-* } eval } */
       /* { dg-warning "use of uninitialized value" "warning" { target *-*-* } eval } */
     }
 
diff --git a/gcc/testsuite/gcc.dg/analyzer/zlib-3.c b/gcc/testsuite/gcc.dg/analyzer/zlib-3.c
index b05b8629a7f..def83006a71 100644
--- a/gcc/testsuite/gcc.dg/analyzer/zlib-3.c
+++ b/gcc/testsuite/gcc.dg/analyzer/zlib-3.c
@@ -184,7 +184,7 @@ static int huft_build(uInt *b, uInt n, uInt s, const uInt *d, const uInt *e,
       mask = (1 << w) - 1;
       /* The analyzer thinks that h can be -1 here.
          This is probably a false positive. */
-      while ((i & mask) !=  x[h]) { /* { dg-bogus "underread" "" { xfail *-*-* } } */
+      while ((i & mask) !=  x[h]) { /* { dg-bogus "under-read" "" { xfail *-*-* } } */
         h--;
         w -= l;
         mask = (1 << w) - 1;
-- 
2.26.3


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [committed 5/7] diagnostics: tweak diagnostic_path::interprocedural_p [PR106626]
  2022-12-01  2:41 [committed 1/7] analyzer: move bounds checking to a new bounds-checking.cc David Malcolm
                   ` (2 preceding siblings ...)
  2022-12-01  2:41 ` [committed 4/7] analyzer: more bounds-checking wording tweaks [PR106626] David Malcolm
@ 2022-12-01  2:41 ` David Malcolm
  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
  5 siblings, 0 replies; 7+ messages in thread
From: David Malcolm @ 2022-12-01  2:41 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [committed 6/7] analyzer: unify bounds-checking class hierarchies
  2022-12-01  2:41 [committed 1/7] analyzer: move bounds checking to a new bounds-checking.cc David Malcolm
                   ` (3 preceding siblings ...)
  2022-12-01  2:41 ` [committed 5/7] diagnostics: tweak diagnostic_path::interprocedural_p [PR106626] David Malcolm
@ 2022-12-01  2:41 ` David Malcolm
  2022-12-01  2:42 ` [committed 7/7] analyzer: fix i18n issues in symbolic out-of-bounds [PR106626] David Malcolm
  5 siblings, 0 replies; 7+ messages in thread
From: David Malcolm @ 2022-12-01  2:41 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

Convert out-of-bounds class hierarchy from:

  pending_diagnostic
    out_of_bounds
      past_the_end
        buffer_overflow (*)
        buffer_over_read (*)
      buffer_underwrite (*)
      buffer_under_read (*)
    symbolic_past_the_end
      symbolic_buffer_overflow (*)
      symbolic_buffer_over_read (*)

to:

  pending_diagnostic
    out_of_bounds
      concrete_out_of_bounds
        concrete_past_the_end
          concrete_buffer_overflow (*)
          concrete_buffer_over_read (*)
        concrete_buffer_underwrite (*)
        concrete_buffer_under_read (*)
      symbolic_past_the_end
        symbolic_buffer_overflow (*)
        symbolic_buffer_over_read (*)

where the concrete classes (i.e. the instantiable ones) are marked
with a (*).

Doing so undercovered a bug where, for CWE-131-examples.c, we were
emitting an extra:
  warning: heap-based buffer over-read [CWE-122] [-Wanalyzer-out-of-bounds]
at the:
  WidgetList[numWidgets] = NULL;
The issue was that within set_next_state we get the rvalue for the LHS,
which looks like a read to the bounds-checker.  The patch fixes this by
passing NULL as the region_model_context * for such accesses.

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

gcc/analyzer/ChangeLog:
	* bounds-checking.cc (class out_of_bounds): Split out from...
	(class concrete_out_of_bounds): New abstract subclass.
	(class past_the_end): Rename to...
	(class concrete_past_the_end): ...this, and make a subclass of
	concrete_out_of_bounds.
	(class buffer_overflow): Rename to...
	(class concrete_buffer_overflow): ...this, and make a subclass of
	concrete_past_the_end.
	(class buffer_over_read): Rename to...
	(class concrete_buffer_over_read): ...this, and make a subclass of
	concrete_past_the_end.
	(class buffer_underwrite): Rename to...
	(class concrete_buffer_underwrite): ...this, and make a subclass
	of concrete_out_of_bounds.
	(class buffer_under_read): Rename to...
	(class concrete_buffer_under_read): ...this, and make a subclass
	of concrete_out_of_bounds.
	(class symbolic_past_the_end): Convert to a subclass of
	out_of_bounds.
	(symbolic_buffer_overflow::get_kind): New.
	(symbolic_buffer_over_read::get_kind): New.
	(region_model::check_region_bounds): Update for renamings.
	* engine.cc (impl_sm_context::set_next_state): Eliminate
	"new_ctxt", passing NULL to get_rvalue instead.
	(impl_sm_context::warn): Likewise.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>
---
 gcc/analyzer/bounds-checking.cc | 185 +++++++++++++++++++-------------
 gcc/analyzer/engine.cc          |  24 +----
 2 files changed, 115 insertions(+), 94 deletions(-)

diff --git a/gcc/analyzer/bounds-checking.cc b/gcc/analyzer/bounds-checking.cc
index bc7d2dd17ae..aaf3f22109b 100644
--- a/gcc/analyzer/bounds-checking.cc
+++ b/gcc/analyzer/bounds-checking.cc
@@ -37,27 +37,21 @@ along with GCC; see the file COPYING3.  If not see
 
 namespace ana {
 
-/* Abstract base class for all out-of-bounds warnings with concrete values.  */
+/* Abstract base class for all out-of-bounds warnings.  */
 
-class out_of_bounds : public pending_diagnostic_subclass<out_of_bounds>
+class out_of_bounds : public pending_diagnostic
 {
 public:
-  out_of_bounds (const region *reg, tree diag_arg,
-		 byte_range out_of_bounds_range)
-  : m_reg (reg), m_diag_arg (diag_arg),
-    m_out_of_bounds_range (out_of_bounds_range)
+  out_of_bounds (const region *reg, tree diag_arg)
+  : m_reg (reg), m_diag_arg (diag_arg)
   {}
 
-  const char *get_kind () const final override
-  {
-    return "out_of_bounds_diagnostic";
-  }
-
-  bool operator== (const out_of_bounds &other) const
+  bool subclass_equal_p (const pending_diagnostic &base_other) const override
   {
-    return m_reg == other.m_reg
-	   && m_out_of_bounds_range == other.m_out_of_bounds_range
-	   && pending_diagnostic::same_tree_p (m_diag_arg, other.m_diag_arg);
+    const out_of_bounds &other
+      (static_cast <const out_of_bounds &>(base_other));
+    return (m_reg == other.m_reg
+	    && pending_diagnostic::same_tree_p (m_diag_arg, other.m_diag_arg));
   }
 
   int get_controlling_option () const final override
@@ -106,25 +100,51 @@ protected:
 
   const region *m_reg;
   tree m_diag_arg;
+};
+
+/* Abstract base class for all out-of-bounds warnings where the
+   out-of-bounds range is concrete.  */
+
+class concrete_out_of_bounds : public out_of_bounds
+{
+public:
+  concrete_out_of_bounds (const region *reg, tree diag_arg,
+			  byte_range out_of_bounds_range)
+  : out_of_bounds (reg, diag_arg),
+    m_out_of_bounds_range (out_of_bounds_range)
+  {}
+
+  bool subclass_equal_p (const pending_diagnostic &base_other) const override
+  {
+    const concrete_out_of_bounds &other
+      (static_cast <const concrete_out_of_bounds &>(base_other));
+    return (out_of_bounds::subclass_equal_p (other)
+	    && m_out_of_bounds_range == other.m_out_of_bounds_range);
+  }
+
+protected:
   byte_range m_out_of_bounds_range;
 };
 
-/* Abstract subclass to complaing about out-of-bounds
+/* Abstract subclass to complaing about concrete out-of-bounds
    past the end of the buffer.  */
 
-class past_the_end : public out_of_bounds
+class concrete_past_the_end : public concrete_out_of_bounds
 {
 public:
-  past_the_end (const region *reg, tree diag_arg, byte_range range,
-		tree byte_bound)
-  : out_of_bounds (reg, diag_arg, range), m_byte_bound (byte_bound)
+  concrete_past_the_end (const region *reg, tree diag_arg, byte_range range,
+			 tree byte_bound)
+  : concrete_out_of_bounds (reg, diag_arg, range), m_byte_bound (byte_bound)
   {}
 
-  bool operator== (const past_the_end &other) const
+  bool
+  subclass_equal_p (const pending_diagnostic &base_other) const final override
   {
-    return out_of_bounds::operator== (other)
-	   && pending_diagnostic::same_tree_p (m_byte_bound,
-					       other.m_byte_bound);
+    const concrete_past_the_end &other
+      (static_cast <const concrete_past_the_end &>(base_other));
+    return (concrete_out_of_bounds::subclass_equal_p (other)
+	    && pending_diagnostic::same_tree_p (m_byte_bound,
+						other.m_byte_bound));
   }
 
   label_text
@@ -143,14 +163,19 @@ protected:
 
 /* Concrete subclass to complain about buffer overflows.  */
 
-class buffer_overflow : public past_the_end
+class concrete_buffer_overflow : public concrete_past_the_end
 {
 public:
-  buffer_overflow (const region *reg, tree diag_arg,
+  concrete_buffer_overflow (const region *reg, tree diag_arg,
 		   byte_range range, tree byte_bound)
-  : past_the_end (reg, diag_arg, range, byte_bound)
+  : concrete_past_the_end (reg, diag_arg, range, byte_bound)
   {}
 
+  const char *get_kind () const final override
+  {
+    return "concrete_buffer_overflow";
+  }
+
   bool emit (rich_location *rich_loc) final override
   {
     diagnostic_metadata m;
@@ -241,14 +266,19 @@ public:
 
 /* Concrete subclass to complain about buffer over-reads.  */
 
-class buffer_over_read : public past_the_end
+class concrete_buffer_over_read : public concrete_past_the_end
 {
 public:
-  buffer_over_read (const region *reg, tree diag_arg,
-		    byte_range range, tree byte_bound)
-  : past_the_end (reg, diag_arg, range, byte_bound)
+  concrete_buffer_over_read (const region *reg, tree diag_arg,
+			     byte_range range, tree byte_bound)
+  : concrete_past_the_end (reg, diag_arg, range, byte_bound)
   {}
 
+  const char *get_kind () const final override
+  {
+    return "concrete_buffer_over_read";
+  }
+
   bool emit (rich_location *rich_loc) final override
   {
     diagnostic_metadata m;
@@ -337,13 +367,19 @@ public:
 
 /* Concrete subclass to complain about buffer underwrites.  */
 
-class buffer_underwrite : public out_of_bounds
+class concrete_buffer_underwrite : public concrete_out_of_bounds
 {
 public:
-  buffer_underwrite (const region *reg, tree diag_arg, byte_range range)
-  : out_of_bounds (reg, diag_arg, range)
+  concrete_buffer_underwrite (const region *reg, tree diag_arg,
+			      byte_range range)
+  : concrete_out_of_bounds (reg, diag_arg, range)
   {}
 
+  const char *get_kind () const final override
+  {
+    return "concrete_buffer_underwrite";
+  }
+
   bool emit (rich_location *rich_loc) final override
   {
     diagnostic_metadata m;
@@ -403,13 +439,19 @@ public:
 
 /* Concrete subclass to complain about buffer under-reads.  */
 
-class buffer_under_read : public out_of_bounds
+class concrete_buffer_under_read : public concrete_out_of_bounds
 {
 public:
-  buffer_under_read (const region *reg, tree diag_arg, byte_range range)
-  : out_of_bounds (reg, diag_arg, range)
+  concrete_buffer_under_read (const region *reg, tree diag_arg,
+			      byte_range range)
+  : concrete_out_of_bounds (reg, diag_arg, range)
   {}
 
+  const char *get_kind () const final override
+  {
+    return "concrete_buffer_under_read";
+  }
+
   bool emit (rich_location *rich_loc) final override
   {
     diagnostic_metadata m;
@@ -470,38 +512,26 @@ public:
 /* Abstract class to complain about out-of-bounds read/writes where
    the values are symbolic.  */
 
-class symbolic_past_the_end
-  : public pending_diagnostic_subclass<symbolic_past_the_end>
+class symbolic_past_the_end : public out_of_bounds
 {
 public:
   symbolic_past_the_end (const region *reg, tree diag_arg, tree offset,
 			 tree num_bytes, tree capacity)
-  : m_reg (reg), m_diag_arg (diag_arg), m_offset (offset),
-    m_num_bytes (num_bytes), m_capacity (capacity)
+  : out_of_bounds (reg, diag_arg),
+    m_offset (offset),
+    m_num_bytes (num_bytes),
+    m_capacity (capacity)
   {}
 
-  const char *get_kind () const final override
-  {
-    return "symbolic_past_the_end";
-  }
-
-  bool operator== (const symbolic_past_the_end &other) const
-  {
-    return m_reg == other.m_reg
-	   && pending_diagnostic::same_tree_p (m_diag_arg, other.m_diag_arg)
-	   && pending_diagnostic::same_tree_p (m_offset, other.m_offset)
-	   && pending_diagnostic::same_tree_p (m_num_bytes, other.m_num_bytes)
-	   && pending_diagnostic::same_tree_p (m_capacity, other.m_capacity);
-  }
-
-  int get_controlling_option () const final override
-  {
-    return OPT_Wanalyzer_out_of_bounds;
-  }
-
-  void mark_interesting_stuff (interesting_t *interest) final override
+  bool
+  subclass_equal_p (const pending_diagnostic &base_other) const final override
   {
-    interest->add_region_creation (m_reg);
+    const symbolic_past_the_end &other
+      (static_cast <const symbolic_past_the_end &>(base_other));
+    return (out_of_bounds::subclass_equal_p (other)
+	    && pending_diagnostic::same_tree_p (m_offset, other.m_offset)
+	    && pending_diagnostic::same_tree_p (m_num_bytes, other.m_num_bytes)
+	    && pending_diagnostic::same_tree_p (m_capacity, other.m_capacity));
   }
 
   label_text
@@ -566,13 +596,6 @@ public:
   }
 
 protected:
-  enum memory_space get_memory_space () const
-  {
-    return m_reg->get_memory_space ();
-  }
-
-  const region *m_reg;
-  tree m_diag_arg;
   tree m_offset;
   tree m_num_bytes;
   tree m_capacity;
@@ -591,6 +614,11 @@ public:
     m_dir_str = "write";
   }
 
+  const char *get_kind () const final override
+  {
+    return "symbolic_buffer_overflow";
+  }
+
   bool emit (rich_location *rich_loc) final override
   {
     diagnostic_metadata m;
@@ -624,6 +652,11 @@ public:
     m_dir_str = "read";
   }
 
+  const char *get_kind () const final override
+  {
+    return "symbolic_buffer_over_read";
+  }
+
   bool emit (rich_location *rich_loc) final override
   {
     diagnostic_metadata m;
@@ -776,10 +809,12 @@ region_model::check_region_bounds (const region *reg,
 	  gcc_unreachable ();
 	  break;
 	case DIR_READ:
-	  ctxt->warn (make_unique<buffer_under_read> (reg, diag_arg, out));
+	  ctxt->warn (make_unique<concrete_buffer_under_read> (reg, diag_arg,
+							       out));
 	  break;
 	case DIR_WRITE:
-	  ctxt->warn (make_unique<buffer_underwrite> (reg, diag_arg, out));
+	  ctxt->warn (make_unique<concrete_buffer_underwrite> (reg, diag_arg,
+							       out));
 	  break;
 	}
     }
@@ -804,12 +839,12 @@ region_model::check_region_bounds (const region *reg,
 	  gcc_unreachable ();
 	  break;
 	case DIR_READ:
-	  ctxt->warn (make_unique<buffer_over_read> (reg, diag_arg,
-						     out, byte_bound));
+	  ctxt->warn (make_unique<concrete_buffer_over_read> (reg, diag_arg,
+							      out, byte_bound));
 	  break;
 	case DIR_WRITE:
-	  ctxt->warn (make_unique<buffer_overflow> (reg, diag_arg,
-						    out, byte_bound));
+	  ctxt->warn (make_unique<concrete_buffer_overflow> (reg, diag_arg,
+							     out, byte_bound));
 	  break;
 	}
     }
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 0c49bb26872..991b592b828 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -310,21 +310,17 @@ public:
   }
 
 
-  void set_next_state (const gimple *stmt,
+  void set_next_state (const gimple *,
 		       tree var,
 		       state_machine::state_t to,
 		       tree origin) final override
   {
     logger * const logger = get_logger ();
     LOG_FUNC (logger);
-    impl_region_model_context new_ctxt (m_eg, m_enode_for_diag,
-					m_old_state, m_new_state,
-					NULL, NULL,
-					stmt);
     const svalue *var_new_sval
-      = m_new_state->m_region_model->get_rvalue (var, &new_ctxt);
+      = m_new_state->m_region_model->get_rvalue (var, NULL);
     const svalue *origin_new_sval
-      = m_new_state->m_region_model->get_rvalue (origin, &new_ctxt);
+      = m_new_state->m_region_model->get_rvalue (origin, NULL);
 
     /* We use the new sval here to avoid issues with uninitialized values.  */
     state_machine::state_t current
@@ -350,12 +346,8 @@ public:
       (m_eg, m_enode_for_diag, NULL, NULL, NULL/*m_enode->get_state ()*/,
        NULL, stmt);
 
-    impl_region_model_context new_ctxt (m_eg, m_enode_for_diag,
-					m_old_state, m_new_state,
-					NULL, NULL,
-					stmt);
     const svalue *origin_new_sval
-      = m_new_state->m_region_model->get_rvalue (origin, &new_ctxt);
+      = m_new_state->m_region_model->get_rvalue (origin, NULL);
 
     state_machine::state_t current
       = m_old_smap->get_state (sval, m_eg.get_ext_state ());
@@ -380,11 +372,8 @@ public:
   {
     LOG_FUNC (get_logger ());
     gcc_assert (d);
-    impl_region_model_context old_ctxt
-      (m_eg, m_enode_for_diag, m_old_state, m_new_state, NULL, NULL, NULL);
-
     const svalue *var_old_sval
-      = m_old_state->m_region_model->get_rvalue (var, &old_ctxt);
+      = m_old_state->m_region_model->get_rvalue (var, NULL);
     state_machine::state_t current
       = (var
 	 ? m_old_smap->get_state (var_old_sval, m_eg.get_ext_state ())
@@ -400,9 +389,6 @@ public:
   {
     LOG_FUNC (get_logger ());
     gcc_assert (d);
-    impl_region_model_context old_ctxt
-      (m_eg, m_enode_for_diag, m_old_state, m_new_state, NULL, NULL, NULL);
-
     state_machine::state_t current
       = (sval
 	 ? m_old_smap->get_state (sval, m_eg.get_ext_state ())
-- 
2.26.3


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [committed 7/7] analyzer: fix i18n issues in symbolic out-of-bounds [PR106626]
  2022-12-01  2:41 [committed 1/7] analyzer: move bounds checking to a new bounds-checking.cc David Malcolm
                   ` (4 preceding siblings ...)
  2022-12-01  2:41 ` [committed 6/7] analyzer: unify bounds-checking class hierarchies David Malcolm
@ 2022-12-01  2:42 ` David Malcolm
  5 siblings, 0 replies; 7+ messages in thread
From: David Malcolm @ 2022-12-01  2:42 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

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

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>
---
 gcc/analyzer/bounds-checking.cc | 192 +++++++++++++++++++++++---------
 1 file changed, 138 insertions(+), 54 deletions(-)

diff --git a/gcc/analyzer/bounds-checking.cc b/gcc/analyzer/bounds-checking.cc
index aaf3f22109b..1c44790f86d 100644
--- a/gcc/analyzer/bounds-checking.cc
+++ b/gcc/analyzer/bounds-checking.cc
@@ -544,62 +544,10 @@ public:
     return label_text ();
   }
 
-  label_text
-  describe_final_event (const evdesc::final_event &ev) final override
-  {
-    const char *byte_str;
-    if (pending_diagnostic::same_tree_p (m_num_bytes, integer_one_node))
-      byte_str = "byte";
-    else
-      byte_str = "bytes";
-
-    if (m_offset)
-      {
-	if (m_num_bytes && TREE_CODE (m_num_bytes) == INTEGER_CST)
-	  {
-	    if (m_diag_arg)
-	      return ev.formatted_print ("%s of %E %s at offset %qE"
-					 " exceeds %qE", m_dir_str,
-					 m_num_bytes, byte_str,
-					 m_offset, m_diag_arg);
-	    else
-	      return ev.formatted_print ("%s of %E %s at offset %qE"
-					 " exceeds the buffer", m_dir_str,
-					 m_num_bytes, byte_str, m_offset);
-	  }
-	else if (m_num_bytes)
-	  {
-	    if (m_diag_arg)
-	      return ev.formatted_print ("%s of %qE %s at offset %qE"
-					 " exceeds %qE", m_dir_str,
-					 m_num_bytes, byte_str,
-					 m_offset, m_diag_arg);
-	    else
-	      return ev.formatted_print ("%s of %qE %s at offset %qE"
-					 " exceeds the buffer", m_dir_str,
-					 m_num_bytes, byte_str, m_offset);
-	  }
-	else
-	  {
-	    if (m_diag_arg)
-	      return ev.formatted_print ("%s at offset %qE exceeds %qE",
-					 m_dir_str, m_offset, m_diag_arg);
-	    else
-	      return ev.formatted_print ("%s at offset %qE exceeds the"
-					 " buffer", m_dir_str, m_offset);
-	  }
-      }
-    if (m_diag_arg)
-      return ev.formatted_print ("out-of-bounds %s on %qE",
-				 m_dir_str, m_diag_arg);
-    return ev.formatted_print ("out-of-bounds %s", m_dir_str);
-  }
-
 protected:
   tree m_offset;
   tree m_num_bytes;
   tree m_capacity;
-  const char *m_dir_str;
 };
 
 /* Concrete subclass to complain about overflows with symbolic values.  */
@@ -611,7 +559,6 @@ public:
 			    tree num_bytes, tree capacity)
   : symbolic_past_the_end (reg, diag_arg, offset, num_bytes, capacity)
   {
-    m_dir_str = "write";
   }
 
   const char *get_kind () const final override
@@ -638,6 +585,75 @@ public:
 			     "heap-based buffer overflow");
       }
   }
+
+  label_text
+  describe_final_event (const evdesc::final_event &ev) final override
+  {
+    if (m_offset)
+      {
+	/* Known offset.  */
+	if (m_num_bytes)
+	  {
+	    /* Known offset, known size.  */
+	    if (TREE_CODE (m_num_bytes) == INTEGER_CST)
+	      {
+		/* Known offset, known constant size.  */
+		if (pending_diagnostic::same_tree_p (m_num_bytes,
+						     integer_one_node))
+		  {
+		    /* Singular m_num_bytes.  */
+		    if (m_diag_arg)
+		      return ev.formatted_print
+			("write of %E byte at offset %qE exceeds %qE",
+			 m_num_bytes, m_offset, m_diag_arg);
+		    else
+		      return ev.formatted_print
+			("write of %E byte at offset %qE exceeds the buffer",
+			 m_num_bytes, m_offset);
+		  }
+		else
+		  {
+		    /* Plural m_num_bytes.  */
+		    if (m_diag_arg)
+		      return ev.formatted_print
+			("write of %E bytes at offset %qE exceeds %qE",
+			 m_num_bytes, m_offset, m_diag_arg);
+		    else
+		      return ev.formatted_print
+			("write of %E bytes at offset %qE exceeds the buffer",
+			 m_num_bytes, m_offset);
+		  }
+	      }
+	    else
+	      {
+		/* Known offset, known symbolic size.  */
+		if (m_diag_arg)
+		  return ev.formatted_print
+		    ("write of %qE bytes at offset %qE exceeds %qE",
+		     m_num_bytes, m_offset, m_diag_arg);
+		else
+		  return ev.formatted_print
+		    ("write of %qE bytes at offset %qE exceeds the buffer",
+		     m_num_bytes, m_offset);
+	      }
+	  }
+	else
+	  {
+	    /* Known offset, unknown size.  */
+	    if (m_diag_arg)
+	      return ev.formatted_print ("write at offset %qE exceeds %qE",
+					 m_offset, m_diag_arg);
+	    else
+	      return ev.formatted_print ("write at offset %qE exceeds the"
+					 " buffer", m_offset);
+	  }
+      }
+    /* Unknown offset.  */
+    if (m_diag_arg)
+      return ev.formatted_print ("out-of-bounds write on %qE",
+				 m_diag_arg);
+    return ev.formatted_print ("out-of-bounds write");
+  }
 };
 
 /* Concrete subclass to complain about over-reads with symbolic values.  */
@@ -649,7 +665,6 @@ public:
 			     tree num_bytes, tree capacity)
   : symbolic_past_the_end (reg, diag_arg, offset, num_bytes, capacity)
   {
-    m_dir_str = "read";
   }
 
   const char *get_kind () const final override
@@ -677,6 +692,75 @@ public:
 			     "heap-based buffer over-read");
       }
   }
+
+  label_text
+  describe_final_event (const evdesc::final_event &ev) final override
+  {
+    if (m_offset)
+      {
+	/* Known offset.  */
+	if (m_num_bytes)
+	  {
+	    /* Known offset, known size.  */
+	    if (TREE_CODE (m_num_bytes) == INTEGER_CST)
+	      {
+		/* Known offset, known constant size.  */
+		if (pending_diagnostic::same_tree_p (m_num_bytes,
+						     integer_one_node))
+		  {
+		    /* Singular m_num_bytes.  */
+		    if (m_diag_arg)
+		      return ev.formatted_print
+			("read of %E byte at offset %qE exceeds %qE",
+			 m_num_bytes, m_offset, m_diag_arg);
+		    else
+		      return ev.formatted_print
+			("read of %E byte at offset %qE exceeds the buffer",
+			 m_num_bytes, m_offset);
+		  }
+		else
+		  {
+		    /* Plural m_num_bytes.  */
+		    if (m_diag_arg)
+		      return ev.formatted_print
+			("read of %E bytes at offset %qE exceeds %qE",
+			 m_num_bytes, m_offset, m_diag_arg);
+		    else
+		      return ev.formatted_print
+			("read of %E bytes at offset %qE exceeds the buffer",
+			 m_num_bytes, m_offset);
+		  }
+	      }
+	    else
+	      {
+		/* Known offset, known symbolic size.  */
+		if (m_diag_arg)
+		  return ev.formatted_print
+		    ("read of %qE bytes at offset %qE exceeds %qE",
+		     m_num_bytes, m_offset, m_diag_arg);
+		else
+		  return ev.formatted_print
+		    ("read of %qE bytes at offset %qE exceeds the buffer",
+		     m_num_bytes, m_offset);
+	      }
+	  }
+	else
+	  {
+	    /* Known offset, unknown size.  */
+	    if (m_diag_arg)
+	      return ev.formatted_print ("read at offset %qE exceeds %qE",
+					 m_offset, m_diag_arg);
+	    else
+	      return ev.formatted_print ("read at offset %qE exceeds the"
+					 " buffer", m_offset);
+	  }
+      }
+    /* Unknown offset.  */
+    if (m_diag_arg)
+      return ev.formatted_print ("out-of-bounds read on %qE",
+				 m_diag_arg);
+    return ev.formatted_print ("out-of-bounds read");
+  }
 };
 
 /* Check whether an access is past the end of the BASE_REG.  */
-- 
2.26.3


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-12-01  2:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` [committed 5/7] diagnostics: tweak diagnostic_path::interprocedural_p [PR106626] David Malcolm
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

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).