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