public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r13-4471] analyzer: fixes to region creation messages [PR107851]
@ 2022-12-02 21:32 David Malcolm
  0 siblings, 0 replies; only message in thread
From: David Malcolm @ 2022-12-02 21:32 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:f5758fe5b430ef3447fbab947fcea32a1d995f36

commit r13-4471-gf5758fe5b430ef3447fbab947fcea32a1d995f36
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Fri Dec 2 16:30:51 2022 -0500

    analyzer: fixes to region creation messages [PR107851]
    
    In r13-2573-gc81b60b8c6ff3d I split up the analyzer's region-creation
    events to describe the memory space and capacity of the region as two
    separate events to avoid combinatorial explosion of message wordings.
    
    However I didn't take into account r13-1405-ge6c3bb379f515b which
    added a pending_diagnostic::describe_region_creation_event vfunc which
    could change the wording of region creation events.
    
    Hence for:
    
    #include <stdlib.h>
    #include <stdint.h>
    
    void test ()
    {
      int32_t *ptr = malloc (1);
      free (ptr);
    }
    
    trunk currently emits:
    
      Compiler Explorer (x86_64 trunk): https://godbolt.org/z/e3Td7c9s5:
    
    <source>: In function 'test':
    <source>:6:18: warning: allocated buffer size is not a multiple of the pointee's size [CWE-131] [-Wanalyzer-allocation-size]
        6 |   int32_t *ptr = malloc (1);
          |                  ^~~~~~~~~~
      'test': events 1-3
        |
        |    6 |   int32_t *ptr = malloc (1);
        |      |                  ^~~~~~~~~~
        |      |                  |
        |      |                  (1) allocated 1 bytes here
        |      |                  (2) allocated 1 bytes here
        |      |                  (3) assigned to 'int32_t *' {aka 'int *'} here; 'sizeof (int32_t {aka int})' is '4'
        |
    
    where events (1) and (2) are different region_creation_events that have
    had their wording overridden (also, with a "1 bytes" issue).
    
    This patch reorganizes region creation events so that each
    pending_diagnostic instead creates the events that is appropriate for it,
    and the events have responsibility for their own wording.
    
    With this patch, the above emits:
    
    <source>: In function 'test':
    <source>:6:18: warning: allocated buffer size is not a multiple of the pointee's size [CWE-131] [-Wanalyzer-allocation-size]
        6 |   int32_t *ptr = malloc (1);
          |                  ^~~~~~~~~~
      'test': events 1-2
        |
        |    6 |   int32_t *ptr = malloc (1);
        |      |                  ^~~~~~~~~~
        |      |                  |
        |      |                  (1) allocated 1 byte here
        |      |                  (2) assigned to 'int32_t *' {aka 'int *'} here; 'sizeof (int32_t {aka int})' is '4'
        |
    
    fixing the duplicate event, and fixing the singular/plural issue.
    
    gcc/analyzer/ChangeLog:
            PR analyzer/107851
            * analyzer.cc (make_label_text_n): Convert param "n" from int to
            unsigned HOST_WIDE_INT.
            * analyzer.h (make_label_text_n): Likewise for decl.
            * bounds-checking.cc: Include "analyzer/checker-event.h" and
            "analyzer/checker-path.h".
            (out_of_bounds::add_region_creation_events): New.
            (concrete_past_the_end::describe_region_creation_event): Replace
            with...
            (concrete_past_the_end::add_region_creation_events): ...this.
            (symbolic_past_the_end::describe_region_creation_event): Delete.
            * checker-event.cc (region_creation_event::region_creation_event):
            Update for dropping all member data.
            (region_creation_event::get_desc): Delete, splitting out into
            region_creation_event_memory_space::get_desc,
            region_creation_event_capacity::get_desc, and
            region_creation_event_debug::get_desc.
            (region_creation_event_memory_space::get_desc): New.
            (region_creation_event_capacity::get_desc): New.
            (region_creation_event_allocation_size::get_desc): New.
            (region_creation_event_debug::get_desc): New.
            * checker-event.h: Include "analyzer/program-state.h".
            (enum rce_kind): Delete.
            (class region_creation_event): Drop all member data.
            (region_creation_event::region_creation_event): Make protected.
            (region_creation_event::get_desc): Delete.
            (class region_creation_event_memory_space): New.
            (class region_creation_event_capacity): New.
            (class region_creation_event_allocation_size): New.
            (class region_creation_event_debug): New.
            * checker-path.cc (checker_path::add_region_creation_events): Add
            "pd" param.  Call pending_diangnostic::add_region_creation_events.
            Update for conversion of RCE_DEBUG to region_creation_event_debug.
            * checker-path.h (checker_path::add_region_creation_events): Add
            "pd" param.
            * diagnostic-manager.cc (diagnostic_manager::build_emission_path):
            Pass pending_diagnostic to
            emission_path::add_region_creation_events.
            (diagnostic_manager::build_emission_path): Pass path_builder to
            add_event_on_final_node.
            (diagnostic_manager::add_event_on_final_node): Add "pb" param.
            Pass pending_diagnostic to
            emission_path::add_region_creation_events.
            (diagnostic_manager::add_events_for_eedge): Pass
            pending_diagnostic to emission_path::add_region_creation_events.
            * diagnostic-manager.h
            (diagnostic_manager::add_event_on_final_node): Add "pb" param.
            * pending-diagnostic.cc
            (pending_diagnostic::add_region_creation_events): New.
            * pending-diagnostic.h (struct region_creation): Delete.
            (pending_diagnostic::describe_region_creation_event): Delete.
            (pending_diagnostic::add_region_creation_events): New vfunc.
            * region-model.cc: Include "analyzer/checker-event.h" and
            "analyzer/checker-path.h".
            (dubious_allocation_size::dubious_allocation_size): Initialize
            m_has_allocation_event.
            (dubious_allocation_size::describe_region_creation_event): Delete.
            (dubious_allocation_size::describe_final_event): Update for
            replacement of m_allocation_event with m_has_allocation_event.
            (dubious_allocation_size::add_region_creation_events): New.
            (dubious_allocation_size::m_allocation_event): Replace with...
            (dubious_allocation_size::m_has_allocation_event): ...this.
    
    gcc/testsuite/ChangeLog:
            PR analyzer/107851
            * gcc.dg/analyzer/allocation-size-4.c: Update expected wording.
            * gcc.dg/analyzer/allocation-size-multiline-1.c: New test.
            * gcc.dg/analyzer/allocation-size-multiline-2.c: New test.
            * gcc.dg/analyzer/out-of-bounds-multiline-1.c: Update expected
            wording.
            * gcc.dg/analyzer/out-of-bounds-multiline-2.c: New test.
            * gcc.dg/analyzer/out-of-bounds-read-char-arr.c: Update expected
            wording.
            * gcc.dg/analyzer/out-of-bounds-read-int-arr.c: Likewise.
            * gcc.dg/analyzer/out-of-bounds-write-char-arr.c: Likewise.
            * gcc.dg/analyzer/out-of-bounds-write-int-arr.c: Likewise.
    
    Signed-off-by: David Malcolm <dmalcolm@redhat.com>

Diff:
---
 gcc/analyzer/analyzer.cc                           |   2 +-
 gcc/analyzer/analyzer.h                            |   3 +-
 gcc/analyzer/bounds-checking.cc                    |  40 ++++---
 gcc/analyzer/checker-event.cc                      | 115 ++++++++++-----------
 gcc/analyzer/checker-event.h                       | 101 ++++++++++++++----
 gcc/analyzer/checker-path.cc                       |  14 +--
 gcc/analyzer/checker-path.h                        |   3 +-
 gcc/analyzer/diagnostic-manager.cc                 |  18 ++--
 gcc/analyzer/diagnostic-manager.h                  |   3 +-
 gcc/analyzer/pending-diagnostic.cc                 |  20 ++++
 gcc/analyzer/pending-diagnostic.h                  |  37 ++-----
 gcc/analyzer/region-model.cc                       |  47 ++++-----
 gcc/testsuite/gcc.dg/analyzer/allocation-size-4.c  |   2 +-
 .../gcc.dg/analyzer/allocation-size-multiline-1.c  |  59 +++++++++++
 .../gcc.dg/analyzer/allocation-size-multiline-2.c  |  62 +++++++++++
 .../gcc.dg/analyzer/out-of-bounds-multiline-1.c    |   2 +-
 .../gcc.dg/analyzer/out-of-bounds-multiline-2.c    |  32 ++++++
 .../gcc.dg/analyzer/out-of-bounds-read-char-arr.c  |   2 +-
 .../gcc.dg/analyzer/out-of-bounds-read-int-arr.c   |   2 +-
 .../gcc.dg/analyzer/out-of-bounds-write-char-arr.c |   2 +-
 .../gcc.dg/analyzer/out-of-bounds-write-int-arr.c  |   2 +-
 21 files changed, 396 insertions(+), 172 deletions(-)

diff --git a/gcc/analyzer/analyzer.cc b/gcc/analyzer/analyzer.cc
index 96497dccfa1..77d622d19e6 100644
--- a/gcc/analyzer/analyzer.cc
+++ b/gcc/analyzer/analyzer.cc
@@ -449,7 +449,7 @@ make_label_text (bool can_colorize, const char *fmt, ...)
 /* As above, but with singular vs plural.  */
 
 label_text
-make_label_text_n (bool can_colorize, int n,
+make_label_text_n (bool can_colorize, unsigned HOST_WIDE_INT n,
 		   const char *singular_fmt,
 		   const char *plural_fmt, ...)
 {
diff --git a/gcc/analyzer/analyzer.h b/gcc/analyzer/analyzer.h
index 35c71f3d69c..e0fdbad61a7 100644
--- a/gcc/analyzer/analyzer.h
+++ b/gcc/analyzer/analyzer.h
@@ -359,7 +359,8 @@ extern const char *get_user_facing_name (const gcall *call);
 extern void register_analyzer_pass ();
 
 extern label_text make_label_text (bool can_colorize, const char *fmt, ...);
-extern label_text make_label_text_n (bool can_colorize, int n,
+extern label_text make_label_text_n (bool can_colorize,
+				     unsigned HOST_WIDE_INT n,
 				     const char *singular_fmt,
 				     const char *plural_fmt, ...);
 
diff --git a/gcc/analyzer/bounds-checking.cc b/gcc/analyzer/bounds-checking.cc
index 1c44790f86d..17f183fea21 100644
--- a/gcc/analyzer/bounds-checking.cc
+++ b/gcc/analyzer/bounds-checking.cc
@@ -32,6 +32,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "analyzer/analyzer.h"
 #include "analyzer/analyzer-logging.h"
 #include "analyzer/region-model.h"
+#include "analyzer/checker-event.h"
+#include "analyzer/checker-path.h"
 
 #if ENABLE_ANALYZER
 
@@ -64,6 +66,20 @@ public:
     interest->add_region_creation (m_reg);
   }
 
+  void add_region_creation_events (const region *,
+				   tree capacity,
+				   location_t loc,
+				   tree fndecl, int depth,
+				   checker_path &emission_path) override
+  {
+    /* The memory space is described in the diagnostic message itself,
+       so we don't need an event for that.  */
+    if (capacity)
+      emission_path.add_event
+	(make_unique<region_creation_event_capacity> (capacity,
+						      loc, fndecl, depth));
+  }
+
 protected:
   enum memory_space get_memory_space () const
   {
@@ -147,14 +163,16 @@ public:
 						other.m_byte_bound));
   }
 
-  label_text
-  describe_region_creation_event (const evdesc::region_creation &ev) final
-  override
+  void add_region_creation_events (const region *,
+				   tree,
+				   location_t loc,
+				   tree fndecl, int depth,
+				   checker_path &emission_path) 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 ();
+      emission_path.add_event
+	(make_unique<region_creation_event_capacity> (m_byte_bound,
+						      loc, fndecl, depth));
   }
 
 protected:
@@ -534,16 +552,6 @@ public:
 	    && pending_diagnostic::same_tree_p (m_capacity, other.m_capacity));
   }
 
-  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 ();
-  }
-
 protected:
   tree m_offset;
   tree m_num_bytes;
diff --git a/gcc/analyzer/checker-event.cc b/gcc/analyzer/checker-event.cc
index a3e043333fe..98f1053da8f 100644
--- a/gcc/analyzer/checker-event.cc
+++ b/gcc/analyzer/checker-event.cc
@@ -293,84 +293,77 @@ statement_event::get_desc (bool) const
 
 /* class region_creation_event : public checker_event.  */
 
-region_creation_event::region_creation_event (const region *reg,
-					      tree capacity,
-					      enum rce_kind kind,
-					      location_t loc,
+region_creation_event::region_creation_event (location_t loc,
 					      tree fndecl,
 					      int depth)
-: checker_event (EK_REGION_CREATION, loc, fndecl, depth),
-  m_reg (reg),
-  m_capacity (capacity),
-  m_rce_kind (kind)
+: checker_event (EK_REGION_CREATION, loc, fndecl, depth)
 {
-  if (m_rce_kind == RCE_CAPACITY)
-    gcc_assert (capacity);
 }
 
-/* Implementation of diagnostic_event::get_desc vfunc for
-   region_creation_event.
-   There are effectively 3 kinds of region_region_event, to
-   avoid combinatorial explosion by trying to convy the
-   information in a single message.  */
+/* The various region_creation_event subclasses' get_desc
+   implementations.  */
 
 label_text
-region_creation_event::get_desc (bool can_colorize) const
+region_creation_event_memory_space::get_desc (bool) const
 {
-  if (m_pending_diagnostic)
+  switch (m_mem_space)
     {
-      label_text custom_desc
-	    = m_pending_diagnostic->describe_region_creation_event
-		(evdesc::region_creation (can_colorize, m_reg));
-      if (custom_desc.get ())
-	return custom_desc;
+    default:
+      return label_text::borrow ("region created here");
+    case MEMSPACE_STACK:
+      return label_text::borrow ("region created on stack here");
+    case MEMSPACE_HEAP:
+      return label_text::borrow ("region created on heap here");
     }
+}
 
-  switch (m_rce_kind)
+label_text
+region_creation_event_capacity::get_desc (bool can_colorize) const
+{
+  gcc_assert (m_capacity);
+  if (TREE_CODE (m_capacity) == INTEGER_CST)
     {
-    default:
-      gcc_unreachable ();
-
-    case RCE_MEM_SPACE:
-      switch (m_reg->get_memory_space ())
-	{
-	default:
-	  return label_text::borrow ("region created here");
-	case MEMSPACE_STACK:
-	  return label_text::borrow ("region created on stack here");
-	case MEMSPACE_HEAP:
-	  return label_text::borrow ("region created on heap here");
-	}
-      break;
+      unsigned HOST_WIDE_INT hwi = tree_to_uhwi (m_capacity);
+      return make_label_text_n (can_colorize,
+				hwi,
+				"capacity: %wu byte",
+				"capacity: %wu bytes",
+				hwi);
+    }
+  else
+    return make_label_text (can_colorize,
+			    "capacity: %qE bytes", m_capacity);
+}
 
-    case RCE_CAPACITY:
-      gcc_assert (m_capacity);
+label_text
+region_creation_event_allocation_size::get_desc (bool can_colorize) const
+{
+  if (m_capacity)
+    {
       if (TREE_CODE (m_capacity) == INTEGER_CST)
-	{
-	  unsigned HOST_WIDE_INT hwi = tree_to_uhwi (m_capacity);
-	  if (hwi == 1)
-	    return make_label_text (can_colorize,
-				    "capacity: %wu byte", hwi);
-	  else
-	    return make_label_text (can_colorize,
-				    "capacity: %wu bytes", hwi);
-	}
+	return make_label_text_n (can_colorize,
+				  tree_to_uhwi (m_capacity),
+				  "allocated %E byte here",
+				  "allocated %E bytes here",
+				  m_capacity);
       else
 	return make_label_text (can_colorize,
-				"capacity: %qE bytes", m_capacity);
-
-    case RCE_DEBUG:
-      {
-	pretty_printer pp;
-	pp_format_decoder (&pp) = default_tree_printer;
-	pp_string (&pp, "region creation: ");
-	m_reg->dump_to_pp (&pp, true);
-	if (m_capacity)
-	  pp_printf (&pp, " capacity: %qE", m_capacity);
-	return label_text::take (xstrdup (pp_formatted_text (&pp)));
-      }
-      break;
+				"allocated %qE bytes here",
+				m_capacity);
     }
+  return make_label_text (can_colorize, "allocated here");
+}
+
+label_text
+region_creation_event_debug::get_desc (bool) const
+{
+  pretty_printer pp;
+  pp_format_decoder (&pp) = default_tree_printer;
+  pp_string (&pp, "region creation: ");
+  m_reg->dump_to_pp (&pp, true);
+  if (m_capacity)
+    pp_printf (&pp, " capacity: %qE", m_capacity);
+  return label_text::take (xstrdup (pp_formatted_text (&pp)));
 }
 
 /* class function_entry_event : public checker_event.  */
diff --git a/gcc/analyzer/checker-event.h b/gcc/analyzer/checker-event.h
index 18c44e600c8..f9885f5cfc3 100644
--- a/gcc/analyzer/checker-event.h
+++ b/gcc/analyzer/checker-event.h
@@ -22,6 +22,7 @@ along with GCC; see the file COPYING3.  If not see
 #define GCC_ANALYZER_CHECKER_EVENT_H
 
 #include "tree-logical-location.h"
+#include "analyzer/program-state.h"
 
 namespace ana {
 
@@ -211,43 +212,105 @@ public:
   const program_state m_dst_state;
 };
 
-/* There are too many combinations to express region creation in one message,
+/* An abstract event subclass describing the creation of a region that
+   is significant for a diagnostic.
+
+   There are too many combinations to express region creation in one message,
    so we emit multiple region_creation_event instances when each pertinent
    region is created.
 
-   This enum distinguishes between the different messages.  */
+   The events are created by pending_diagnostic's add_region_creation_events
+   vfunc, which by default creates a region_creation_event_memory_space, and
+   if a capacity is known, a region_creation_event_capacity, giving e.g.:
+     (1) region created on stack here
+     (2) capacity: 100 bytes
+   but this vfunc can be overridden to create other events if other wordings
+   are more appropriate foa a given pending_diagnostic.  */
 
-enum rce_kind
+class region_creation_event : public checker_event
 {
-  /* Generate a message based on the memory space of the region
-     e.g. "region created on stack here".  */
-  RCE_MEM_SPACE,
+protected:
+  region_creation_event (location_t loc, tree fndecl, int depth);
+};
+
+/* Concrete subclass of region_creation_event.
+   Generates a message based on the memory space of the region
+   e.g. "region created on stack here".  */
 
-  /* Generate a message based on the capacity of the region
-     e.g. "capacity: 100 bytes".  */
-  RCE_CAPACITY,
+class region_creation_event_memory_space : public region_creation_event
+{
+public:
+  region_creation_event_memory_space (enum memory_space mem_space,
+				      location_t loc, tree fndecl, int depth)
+  : region_creation_event (loc, fndecl, depth),
+    m_mem_space (mem_space)
+  {
+  }
 
-  /* Generate a debug message.  */
-  RCE_DEBUG
+  label_text get_desc (bool can_colorize) const final override;
+
+private:
+  enum memory_space m_mem_space;
 };
 
-/* A concrete event subclass describing the creation of a region that
-   is significant for a diagnostic.  */
+/* Concrete subclass of region_creation_event.
+   Generates a message based on the capacity of the region
+   e.g. "capacity: 100 bytes".  */
 
-class region_creation_event : public checker_event
+class region_creation_event_capacity : public region_creation_event
 {
 public:
-  region_creation_event (const region *reg,
-			 tree capacity,
-			 enum rce_kind kind,
-			 location_t loc, tree fndecl, int depth);
+  region_creation_event_capacity (tree capacity,
+				  location_t loc, tree fndecl, int depth)
+  : region_creation_event (loc, fndecl, depth),
+    m_capacity (capacity)
+  {
+    gcc_assert (m_capacity);
+  }
+
+  label_text get_desc (bool can_colorize) const final override;
+
+private:
+  tree m_capacity;
+};
+
+/* Concrete subclass of region_creation_event.
+   Generates a message based on the capacity of the region
+   e.g. "allocated 100 bytes here".  */
+
+class region_creation_event_allocation_size : public region_creation_event
+{
+public:
+  region_creation_event_allocation_size (tree capacity,
+					 location_t loc, tree fndecl, int depth)
+  : region_creation_event (loc, fndecl, depth),
+    m_capacity (capacity)
+  {}
+
+  label_text get_desc (bool can_colorize) const final override;
+
+private:
+  tree m_capacity;
+};
+
+/* Concrete subclass of region_creation_event.
+   Generates a debug message intended for analyzer developers.  */
+
+class region_creation_event_debug : public region_creation_event
+{
+public:
+  region_creation_event_debug (const region *reg, tree capacity,
+			       location_t loc, tree fndecl, int depth)
+  : region_creation_event (loc, fndecl, depth),
+    m_reg (reg), m_capacity (capacity)
+  {
+  }
 
   label_text get_desc (bool can_colorize) const final override;
 
 private:
   const region *m_reg;
   tree m_capacity;
-  enum rce_kind m_rce_kind;
 };
 
 /* An event subclass describing the entry to a function.  */
diff --git a/gcc/analyzer/checker-path.cc b/gcc/analyzer/checker-path.cc
index 221042ee010..0cc0b2bf81f 100644
--- a/gcc/analyzer/checker-path.cc
+++ b/gcc/analyzer/checker-path.cc
@@ -141,7 +141,8 @@ checker_path::debug () const
    If DEBUG is true, also create an RCE_DEBUG event.  */
 
 void
-checker_path::add_region_creation_events (const region *reg,
+checker_path::add_region_creation_events (pending_diagnostic *pd,
+					  const region *reg,
 					  const region_model *model,
 					  location_t loc,
 					  tree fndecl, int depth,
@@ -152,16 +153,11 @@ checker_path::add_region_creation_events (const region *reg,
     if (const svalue *capacity_sval = model->get_capacity (reg))
       capacity = model->get_representative_tree (capacity_sval);
 
-  add_event (make_unique<region_creation_event> (reg, capacity, RCE_MEM_SPACE,
-						 loc, fndecl, depth));
-
-  if (capacity)
-    add_event (make_unique<region_creation_event> (reg, capacity, RCE_CAPACITY,
-						   loc, fndecl, depth));
+  pd->add_region_creation_events (reg, capacity, loc, fndecl, depth, *this);
 
   if (debug)
-    add_event (make_unique<region_creation_event> (reg, capacity, RCE_DEBUG,
-						   loc, fndecl, depth));
+    add_event (make_unique<region_creation_event_debug> (reg, capacity,
+							 loc, fndecl, depth));
 }
 
 void
diff --git a/gcc/analyzer/checker-path.h b/gcc/analyzer/checker-path.h
index 55bf1e3e3b5..ba04aed069e 100644
--- a/gcc/analyzer/checker-path.h
+++ b/gcc/analyzer/checker-path.h
@@ -76,7 +76,8 @@ public:
     m_events[idx] = new_event;
   }
 
-  void add_region_creation_events (const region *reg,
+  void add_region_creation_events (pending_diagnostic *pd,
+				   const region *reg,
 				   const region_model *model,
 				   location_t loc,
 				   tree fndecl, int depth,
diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index 1b19e58201b..0574758be5a 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -1447,7 +1447,8 @@ diagnostic_manager::build_emission_path (const path_builder &pb,
 		  && DECL_SOURCE_LOCATION (decl) != UNKNOWN_LOCATION)
 		{
 		  emission_path->add_region_creation_events
-		    (reg, NULL,
+		    (pb.get_pending_diagnostic (),
+		     reg, NULL,
 		     DECL_SOURCE_LOCATION (decl),
 		     NULL_TREE,
 		     0,
@@ -1463,7 +1464,8 @@ diagnostic_manager::build_emission_path (const path_builder &pb,
       const exploded_edge *eedge = epath.m_edges[i];
       add_events_for_eedge (pb, *eedge, emission_path, &interest);
     }
-  add_event_on_final_node (epath.get_final_enode (), emission_path, &interest);
+  add_event_on_final_node (pb, epath.get_final_enode (),
+			   emission_path, &interest);
 }
 
 /* Emit a region_creation_event when requested on the last statement in
@@ -1475,7 +1477,8 @@ diagnostic_manager::build_emission_path (const path_builder &pb,
 */
 
 void
-diagnostic_manager::add_event_on_final_node (const exploded_node *final_enode,
+diagnostic_manager::add_event_on_final_node (const path_builder &pb,
+					     const exploded_node *final_enode,
 					     checker_path *emission_path,
 					     interesting_t *interest) const
 {
@@ -1512,7 +1515,8 @@ diagnostic_manager::add_event_on_final_node (const exploded_node *final_enode,
 		case RK_HEAP_ALLOCATED:
 		case RK_ALLOCA:
 		  emission_path->add_region_creation_events
-		    (reg,
+		    (pb.get_pending_diagnostic (),
+		     reg,
 		     dst_model,
 		     src_point.get_location (),
 		     src_point.get_fndecl (),
@@ -1940,7 +1944,8 @@ diagnostic_manager::add_events_for_eedge (const path_builder &pb,
 			    && DECL_SOURCE_LOCATION (decl) != UNKNOWN_LOCATION)
 			  {
 			    emission_path->add_region_creation_events
-			      (reg, dst_state.m_region_model,
+			      (pb.get_pending_diagnostic (),
+			       reg, dst_state.m_region_model,
 			       DECL_SOURCE_LOCATION (decl),
 			       dst_point.get_fndecl (),
 			       dst_stack_depth,
@@ -2036,7 +2041,8 @@ diagnostic_manager::add_events_for_eedge (const path_builder &pb,
 		  case RK_HEAP_ALLOCATED:
 		  case RK_ALLOCA:
 		    emission_path->add_region_creation_events
-		      (reg, dst_model,
+		      (pb.get_pending_diagnostic (),
+		       reg, dst_model,
 		       src_point.get_location (),
 		       src_point.get_fndecl (),
 		       src_stack_depth,
diff --git a/gcc/analyzer/diagnostic-manager.h b/gcc/analyzer/diagnostic-manager.h
index 4862cf47241..56a233b9fa3 100644
--- a/gcc/analyzer/diagnostic-manager.h
+++ b/gcc/analyzer/diagnostic-manager.h
@@ -148,7 +148,8 @@ private:
 			    const exploded_path &epath,
 			    checker_path *emission_path) const;
 
-  void add_event_on_final_node (const exploded_node *final_enode,
+  void add_event_on_final_node (const path_builder &pb,
+				const exploded_node *final_enode,
 				checker_path *emission_path,
 				interesting_t *interest) const;
 
diff --git a/gcc/analyzer/pending-diagnostic.cc b/gcc/analyzer/pending-diagnostic.cc
index 53cab2065dd..babefc5ad4e 100644
--- a/gcc/analyzer/pending-diagnostic.cc
+++ b/gcc/analyzer/pending-diagnostic.cc
@@ -199,6 +199,26 @@ pending_diagnostic::add_call_event (const exploded_edge &eedge,
 			      src_stack_depth));
 }
 
+/* Base implementation of pending_diagnostic::add_region_creation_events.
+   See the comment for class region_creation_event.  */
+
+void
+pending_diagnostic::add_region_creation_events (const region *reg,
+						tree capacity,
+						location_t loc,
+						tree fndecl, int depth,
+						checker_path &emission_path)
+{
+  emission_path.add_event
+    (make_unique<region_creation_event_memory_space> (reg->get_memory_space (),
+						      loc, fndecl, depth));
+
+  if (capacity)
+    emission_path.add_event
+      (make_unique<region_creation_event_capacity> (capacity,
+						    loc, fndecl, depth));
+}
+
 /* Base implementation of pending_diagnostic::add_final_event.
    Add a warning_event to the end of EMISSION_PATH.  */
 
diff --git a/gcc/analyzer/pending-diagnostic.h b/gcc/analyzer/pending-diagnostic.h
index 5d5d126b342..4bc3080c049 100644
--- a/gcc/analyzer/pending-diagnostic.h
+++ b/gcc/analyzer/pending-diagnostic.h
@@ -59,17 +59,6 @@ struct event_desc
   bool m_colorize;
 };
 
-/* For use by pending_diagnostic::describe_region_creation.  */
-
-struct region_creation : public event_desc
-{
-  region_creation (bool colorize, const region *reg)
-  : event_desc (colorize), m_reg (reg)
-  {}
-
-  const region *m_reg;
-};
-
 /* For use by pending_diagnostic::describe_state_change.  */
 
 struct state_change : public event_desc
@@ -219,23 +208,6 @@ class pending_diagnostic
      for the diagnostic, and FALSE for events in their paths.  */
   virtual location_t fixup_location (location_t loc, bool primary) const;
 
-  /* For greatest precision-of-wording, the various following "describe_*"
-     virtual functions give the pending diagnostic a way to describe events
-     in a diagnostic_path in terms that make sense for that diagnostic.
-
-     In each case, return a non-NULL label_text to give the event a custom
-     description; NULL otherwise (falling back on a more generic
-     description).  */
-
-  /* Precision-of-wording vfunc for describing a region creation event
-     triggered by the mark_interesting_stuff vfunc.  */
-  virtual label_text
-  describe_region_creation_event (const evdesc::region_creation &)
-  {
-    /* Default no-op implementation.  */
-    return label_text ();
-  }
-
   /* Precision-of-wording vfunc for describing a critical state change
      within the diagnostic_path.
 
@@ -338,6 +310,15 @@ class pending_diagnostic
   virtual void add_call_event (const exploded_edge &,
 			       checker_path *);
 
+  /* Vfunc for adding any events for the creation of regions identified
+     by the mark_interesting_stuff vfunc.
+     See the comment for class region_creation_event.  */
+  virtual void add_region_creation_events (const region *reg,
+					   tree capacity,
+					   location_t loc,
+					   tree fndecl, int depth,
+					   checker_path &emission_path);
+
   /* Vfunc for adding the final warning_event to a checker_path, so that e.g.
      the infinite recursion diagnostic can have its diagnostic appear at
      the callsite, but the final event in the path be at the entrypoint
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 4f623fd6ca3..c6486f315f4 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -74,6 +74,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "calls.h"
 #include "is-a.h"
 #include "gcc-rich-location.h"
+#include "analyzer/checker-event.h"
+#include "analyzer/checker-path.h"
 
 #if ENABLE_ANALYZER
 
@@ -2777,12 +2779,14 @@ class dubious_allocation_size
 {
 public:
   dubious_allocation_size (const region *lhs, const region *rhs)
-  : m_lhs (lhs), m_rhs (rhs), m_expr (NULL_TREE)
+  : m_lhs (lhs), m_rhs (rhs), m_expr (NULL_TREE),
+    m_has_allocation_event (false)
   {}
 
   dubious_allocation_size (const region *lhs, const region *rhs,
 			   tree expr)
-  : m_lhs (lhs), m_rhs (rhs), m_expr (expr)
+  : m_lhs (lhs), m_rhs (rhs), m_expr (expr),
+    m_has_allocation_event (false)
   {}
 
   const char *get_kind () const final override
@@ -2811,34 +2815,17 @@ public:
 			 " of the pointee's size");
   }
 
-  label_text
-  describe_region_creation_event (const evdesc::region_creation &ev) final
-  override
-  {
-    m_allocation_event = &ev;
-    if (m_expr)
-      {
-	if (TREE_CODE (m_expr) == INTEGER_CST)
-	  return ev.formatted_print ("allocated %E bytes here", m_expr);
-	else
-	  return ev.formatted_print ("allocated %qE bytes here", m_expr);
-      }
-
-    return ev.formatted_print ("allocated here");
-  }
-
   label_text describe_final_event (const evdesc::final_event &ev) final
   override
   {
     tree pointee_type = TREE_TYPE (m_lhs->get_type ());
-    if (m_allocation_event)
-      /* Fallback: Typically, we should always
-	 see an m_allocation_event before.  */
+    if (m_has_allocation_event)
       return ev.formatted_print ("assigned to %qT here;"
 				 " %<sizeof (%T)%> is %qE",
 				 m_lhs->get_type (), pointee_type,
 				 size_in_bytes (pointee_type));
-
+    /* Fallback: Typically, we should always see an allocation_event
+       before.  */
     if (m_expr)
       {
 	if (TREE_CODE (m_expr) == INTEGER_CST)
@@ -2859,6 +2846,20 @@ public:
 			       size_in_bytes (pointee_type));
   }
 
+  void
+  add_region_creation_events (const region *,
+			      tree capacity,
+			      location_t loc,
+			      tree fndecl, int depth,
+			      checker_path &emission_path) final override
+  {
+    emission_path.add_event
+      (make_unique<region_creation_event_allocation_size> (capacity,
+							   loc, fndecl, depth));
+
+    m_has_allocation_event = true;
+  }
+
   void mark_interesting_stuff (interesting_t *interest) final override
   {
     interest->add_region_creation (m_rhs);
@@ -2868,7 +2869,7 @@ private:
   const region *m_lhs;
   const region *m_rhs;
   const tree m_expr;
-  const evdesc::region_creation *m_allocation_event;
+  bool m_has_allocation_event;
 };
 
 /* Return true on dubious allocation sizes for constant sizes.  */
diff --git a/gcc/testsuite/gcc.dg/analyzer/allocation-size-4.c b/gcc/testsuite/gcc.dg/analyzer/allocation-size-4.c
index 235c156a25c..a56b25b4374 100644
--- a/gcc/testsuite/gcc.dg/analyzer/allocation-size-4.c
+++ b/gcc/testsuite/gcc.dg/analyzer/allocation-size-4.c
@@ -56,6 +56,6 @@ void test_5 (void)
   free (ptr);
 
   /* { dg-warning "allocated buffer size is not a multiple of the pointee's size \\\[CWE-131\\\]" "warning" { target *-*-* } malloc5 } */
-  /* { dg-message "1 bytes" "note" { target *-*-* } malloc5 } */
+  /* { dg-message "allocated 1 byte here" "note" { target *-*-* } malloc5 } */
   /* { dg-message "'struct base \\*' here; 'sizeof \\(struct base\\)' is '\\d+'" "note" { target *-*-* } malloc5 } */
 }
diff --git a/gcc/testsuite/gcc.dg/analyzer/allocation-size-multiline-1.c b/gcc/testsuite/gcc.dg/analyzer/allocation-size-multiline-1.c
new file mode 100644
index 00000000000..7251665105d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/allocation-size-multiline-1.c
@@ -0,0 +1,59 @@
+/* { dg-additional-options "-fdiagnostics-path-format=inline-events -fdiagnostics-show-caret" } */
+
+#include <stdint.h>
+
+void test_constant_1 (void)
+{
+  int32_t *ptr = __builtin_malloc (1); /* { dg-warning "allocated buffer size is not a multiple of the pointee's size" } */
+  __builtin_free (ptr);
+}
+
+/* { dg-begin-multiline-output "" }
+   int32_t *ptr = __builtin_malloc (1);
+                  ^~~~~~~~~~~~~~~~~~~~
+  'test_constant_1': events 1-2
+    |
+    |   int32_t *ptr = __builtin_malloc (1);
+    |                  ^~~~~~~~~~~~~~~~~~~~
+    |                  |
+    |                  (1) allocated 1 byte here
+    |                  (2) assigned to 'int32_t *'
+    |
+   { dg-end-multiline-output "" } */
+
+void test_constant_2 (void)
+{
+  int32_t *ptr = __builtin_malloc (2); /* { dg-warning "allocated buffer size is not a multiple of the pointee's size" } */
+  __builtin_free (ptr);
+}
+
+/* { dg-begin-multiline-output "" }
+   int32_t *ptr = __builtin_malloc (2);
+                  ^~~~~~~~~~~~~~~~~~~~
+  'test_constant_2': events 1-2
+    |
+    |   int32_t *ptr = __builtin_malloc (2);
+    |                  ^~~~~~~~~~~~~~~~~~~~
+    |                  |
+    |                  (1) allocated 2 bytes here
+    |                  (2) assigned to 'int32_t *'
+    |
+   { dg-end-multiline-output "" } */
+
+void test_symbolic (int n)
+{
+  int32_t *ptr = __builtin_malloc (n * 2); /* { dg-warning "allocated buffer size is not a multiple of the pointee's size" } */
+  __builtin_free (ptr);
+}
+
+/* { dg-begin-multiline-output "" }
+   int32_t *ptr = __builtin_malloc (n * 2);
+                  ^~~~~~~~~~~~~~~~~~~~~~~~
+  'test_symbolic': event 1
+    |
+    |   int32_t *ptr = __builtin_malloc (n * 2);
+    |                  ^~~~~~~~~~~~~~~~~~~~~~~~
+    |                  |
+    |                  (1) allocated 'n * 2' bytes and assigned to 'int32_t *'
+    |
+   { dg-end-multiline-output "" } */
diff --git a/gcc/testsuite/gcc.dg/analyzer/allocation-size-multiline-2.c b/gcc/testsuite/gcc.dg/analyzer/allocation-size-multiline-2.c
new file mode 100644
index 00000000000..7cadbb74751
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/allocation-size-multiline-2.c
@@ -0,0 +1,62 @@
+/* { dg-additional-options "-fdiagnostics-path-format=inline-events -fdiagnostics-show-caret -fanalyzer-fine-grained" } */
+/* { dg-require-effective-target alloca } */
+
+#include <stdint.h>
+
+void test_constant_1 (void)
+{
+  int32_t *ptr = __builtin_alloca (1); /* { dg-warning "allocated buffer size is not a multiple of the pointee's size" } */
+}
+
+/* { dg-begin-multiline-output "" }
+   int32_t *ptr = __builtin_alloca (1);
+                  ^~~~~~~~~~~~~~~~~~~~
+  'test_constant_1': events 1-2
+    |
+    |   int32_t *ptr = __builtin_alloca (1);
+    |                  ^~~~~~~~~~~~~~~~~~~~
+    |                  |
+    |                  (1) allocated 1 byte here
+    |                  (2) assigned to 'int32_t *'
+    |
+   { dg-end-multiline-output "" } */
+
+void test_constant_2 (void)
+{
+  int32_t *ptr = __builtin_alloca (2); /* { dg-warning "allocated buffer size is not a multiple of the pointee's size" } */
+}
+
+/* { dg-begin-multiline-output "" }
+   int32_t *ptr = __builtin_alloca (2);
+                  ^~~~~~~~~~~~~~~~~~~~
+  'test_constant_2': events 1-2
+    |
+    |   int32_t *ptr = __builtin_alloca (2);
+    |                  ^~~~~~~~~~~~~~~~~~~~
+    |                  |
+    |                  (1) allocated 2 bytes here
+    |                  (2) assigned to 'int32_t *'
+    |
+   { dg-end-multiline-output "" } */
+
+void test_symbolic (int n)
+{
+  int32_t *ptr = __builtin_alloca (n * 2); /* { dg-warning "allocated buffer size is not a multiple of the pointee's size" } */
+}
+
+/* { dg-begin-multiline-output "" }
+   int32_t *ptr = __builtin_alloca (n * 2);
+                  ^~~~~~~~~~~~~~~~~~~~~~~~
+  'test_symbolic': events 1-2
+    |
+    |   int32_t *ptr = __builtin_alloca (n * 2);
+    |                  ^~~~~~~~~~~~~~~~~~~~~~~~
+    |                  |
+    |                  (1) allocated 'n * 2' bytes here
+    |                  (2) assigned to 'int32_t *'
+    |
+   { dg-end-multiline-output "" } */
+
+/* FIXME: am getting a duplicate warning here for some reason
+   without -fanalyzer-fine-grained (PR PR analyzer/107851).  */
+
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
index 25301e9e2ff..ca5022d20cb 100644
--- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-multiline-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-multiline-1.c
@@ -25,7 +25,7 @@ void int_arr_write_element_after_end_off_by_one(int32_t x)
     | int32_t arr[10];
     |         ^~~
     |         |
-    |         (1) capacity is 40 bytes
+    |         (1) capacity: 40 bytes
     |
     +--> 'int_arr_write_element_after_end_off_by_one': event 2 (depth 1)
            |
diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-multiline-2.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-multiline-2.c
new file mode 100644
index 00000000000..660901ab782
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-multiline-2.c
@@ -0,0 +1,32 @@
+/* Integration test of how the execution path looks for
+   -Wanalyzer-out-of-bounds with a symbolic size.  */
+
+/* { dg-additional-options "-fdiagnostics-show-path-depths" } */
+/* { dg-additional-options "-fdiagnostics-path-format=inline-events -fdiagnostics-show-caret" } */
+
+#include <stdint.h>
+#include <stdlib.h>
+
+void int_vla_write_element_after_end_off_by_one(int32_t x, size_t n)
+{
+  int32_t arr[n];
+
+  arr[n] = x;  /* { dg-warning "stack-based buffer overflow" } */
+}
+
+/* { dg-begin-multiline-output "" }
+   arr[n] = x;
+   ~~~~~~~^~~
+  'int_vla_write_element_after_end_off_by_one': events 1-2 (depth 1)
+    |
+    |   int32_t arr[n];
+    |           ^~~
+    |           |
+    |           (1) capacity: 'n * 4' bytes
+    |
+    |   arr[n] = x;
+    |   ~~~~~~~~~~
+    |          |
+    |          (2) write of 4 bytes at offset 'n * 4' exceeds the buffer
+    |
+   { dg-end-multiline-output "" } */
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 f6d0dda9fb9..fa4b613550d 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,4 +1,4 @@
-char arr[10]; /* { dg-message "capacity is 10 bytes" } */
+char arr[10]; /* { dg-message "capacity: 10 bytes" } */
 
 char char_arr_read_element_before_start_far(void)
 {
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 f1b6e119777..c04cc19df93 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
@@ -1,6 +1,6 @@
 #include <stdint.h>
 
-int32_t arr[10]; /* { dg-message "capacity is 40 bytes" } */
+int32_t arr[10]; /* { dg-message "capacity: 40 bytes" } */
 
 int32_t int_arr_read_element_before_start_far(void)
 {
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 2f3bbfa2dc2..2bc707c8d03 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,4 +1,4 @@
-char arr[10]; /* { dg-message "capacity is 10 bytes" } */
+char arr[10]; /* { dg-message "capacity: 10 bytes" } */
 
 void char_arr_write_element_before_start_far(char x)
 {
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 0adb7899641..c6c0435cf58 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
@@ -1,6 +1,6 @@
 #include <stdint.h>
 
-int32_t arr[10]; /* { dg-message "capacity is 40 bytes" } */
+int32_t arr[10]; /* { dg-message "capacity: 40 bytes" } */
 
 void int_arr_write_element_before_start_far(int32_t x)
 {

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-12-02 21:32 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-02 21:32 [gcc r13-4471] analyzer: fixes to region creation messages [PR107851] 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).