public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r13-1405] analyzer: add allocation size checker [PR105900]
@ 2022-07-02 17:08 Tim Lange
0 siblings, 0 replies; only message in thread
From: Tim Lange @ 2022-07-02 17:08 UTC (permalink / raw)
To: gcc-cvs
https://gcc.gnu.org/g:e6c3bb379f515b27268d08e62b4b3e5d7200b437
commit r13-1405-ge6c3bb379f515b27268d08e62b4b3e5d7200b437
Author: Tim Lange <mail@tim-lange.me>
Date: Fri Jul 1 00:02:17 2022 +0200
analyzer: add allocation size checker [PR105900]
This patch adds an checker that warns about code paths in which a buffer
is assigned to a incompatible type, i.e. when the allocated buffer size
is not a multiple of the pointee's size.
Regression-tested on x86_64 Linux. Also compiled coreutils, curl, openssh and
httpd with the patch enabled.
2022-07-01 Tim Lange <mail@tim-lange.me>
gcc/analyzer/ChangeLog:
PR analyzer/105900
* analyzer.opt: Added Wanalyzer-allocation-size.
* checker-path.cc (region_creation_event::get_desc): Added call to new
virtual function pending_diagnostic::describe_region_creation_event.
* checker-path.h: Added region_creation_event::get_desc.
* diagnostic-manager.cc (diagnostic_manager::add_event_on_final_node):
New function.
* diagnostic-manager.h:
Added diagnostic_manager::add_event_on_final_node.
* pending-diagnostic.h (struct region_creation): New event_desc struct.
(pending_diagnostic::describe_region_creation_event): Added virtual
function to overwrite description of a region creation.
* region-model.cc (class dubious_allocation_size): New class.
(capacity_compatible_with_type): New helper function.
(class size_visitor): New class.
(struct_or_union_with_inheritance_p): New helper function.
(is_any_cast_p): New helper function.
(region_model::check_region_size): New function.
(region_model::set_value): Added call to
region_model::check_region_size.
* region-model.h (class region_model): New function check_region_size.
* svalue.cc (region_svalue::accept): Changed to post-order traversal.
(initial_svalue::accept): Likewise.
(unaryop_svalue::accept): Likewise.
(binop_svalue::accept): Likewise.
(sub_svalue::accept): Likewise.
(repeated_svalue::accept): Likewise.
(bits_within_svalue::accept): Likewise.
(widening_svalue::accept): Likewise.
(unmergeable_svalue::accept): Likewise.
(compound_svalue::accept): Likewise.
(conjured_svalue::accept): Likewise.
(asm_output_svalue::accept): Likewise.
(const_fn_result_svalue::accept): Likewise.
gcc/ChangeLog:
PR analyzer/105900
* doc/invoke.texi: Added Wanalyzer-allocation-size.
gcc/testsuite/ChangeLog:
PR analyzer/105900
* gcc.dg/analyzer/pr96639.c: Changed buffer size to omit warning.
* gcc.dg/analyzer/allocation-size-1.c: New test.
* gcc.dg/analyzer/allocation-size-2.c: New test.
* gcc.dg/analyzer/allocation-size-3.c: New test.
* gcc.dg/analyzer/allocation-size-4.c: New test.
* gcc.dg/analyzer/allocation-size-5.c: New test.
Signed-off-by: Tim Lange <mail@tim-lange.me>
Diff:
---
gcc/analyzer/analyzer.opt | 4 +
gcc/analyzer/checker-path.cc | 11 +-
gcc/analyzer/checker-path.h | 2 +-
gcc/analyzer/diagnostic-manager.cc | 61 ++++
gcc/analyzer/diagnostic-manager.h | 4 +
gcc/analyzer/pending-diagnostic.h | 20 ++
gcc/analyzer/region-model.cc | 370 ++++++++++++++++++++++
gcc/analyzer/region-model.h | 2 +
gcc/analyzer/svalue.cc | 26 +-
gcc/doc/invoke.texi | 14 +
gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c | 116 +++++++
gcc/testsuite/gcc.dg/analyzer/allocation-size-2.c | 155 +++++++++
gcc/testsuite/gcc.dg/analyzer/allocation-size-3.c | 45 +++
gcc/testsuite/gcc.dg/analyzer/allocation-size-4.c | 60 ++++
gcc/testsuite/gcc.dg/analyzer/allocation-size-5.c | 36 +++
gcc/testsuite/gcc.dg/analyzer/pr96639.c | 2 +-
16 files changed, 912 insertions(+), 16 deletions(-)
diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt
index 8ef6a6fa13c..5021376b6fb 100644
--- a/gcc/analyzer/analyzer.opt
+++ b/gcc/analyzer/analyzer.opt
@@ -54,6 +54,10 @@ The minimum number of supernodes within a function for the analyzer to consider
Common Joined UInteger Var(param_analyzer_max_enodes_for_full_dump) Init(200) Param
The maximum depth of exploded nodes that should appear in a dot dump before switching to a less verbose format.
+Wanalyzer-allocation-size
+Common Var(warn_analyzer_allocation_size) Init(1) Warning
+Warn about code paths in which a pointer to a buffer is assigned to an incompatible type.
+
Wanalyzer-double-fclose
Common Var(warn_analyzer_double_fclose) Init(1) Warning
Warn about code paths in which a stdio FILE can be closed more than once.
diff --git a/gcc/analyzer/checker-path.cc b/gcc/analyzer/checker-path.cc
index 0133dc94137..953e192cd55 100644
--- a/gcc/analyzer/checker-path.cc
+++ b/gcc/analyzer/checker-path.cc
@@ -302,8 +302,17 @@ region_creation_event::region_creation_event (const region *reg,
region_creation_event. */
label_text
-region_creation_event::get_desc (bool) const
+region_creation_event::get_desc (bool can_colorize) const
{
+ if (m_pending_diagnostic)
+ {
+ label_text custom_desc
+ = m_pending_diagnostic->describe_region_creation_event
+ (evdesc::region_creation (can_colorize, m_reg));
+ if (custom_desc.m_buffer)
+ return custom_desc;
+ }
+
switch (m_reg->get_memory_space ())
{
default:
diff --git a/gcc/analyzer/checker-path.h b/gcc/analyzer/checker-path.h
index 24decf5ce3d..8e48d8a07ab 100644
--- a/gcc/analyzer/checker-path.h
+++ b/gcc/analyzer/checker-path.h
@@ -219,7 +219,7 @@ public:
region_creation_event (const region *reg,
location_t loc, tree fndecl, int depth);
- label_text get_desc (bool) const final override;
+ label_text get_desc (bool can_colorize) const final override;
private:
const region *m_reg;
diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index 8ea1f61776e..4adfda1af65 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -1476,6 +1476,67 @@ 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);
+}
+
+/* Emit a region_creation_event when requested on the last statement in
+ the path.
+
+ If a region_creation_event should be emitted on the last statement of the
+ path, we need to peek to the successors to get whether the final enode
+ created a region.
+*/
+
+void
+diagnostic_manager::add_event_on_final_node (const exploded_node *final_enode,
+ checker_path *emission_path,
+ interesting_t *interest) const
+{
+ const program_point &src_point = final_enode->get_point ();
+ const int src_stack_depth = src_point.get_stack_depth ();
+ const program_state &src_state = final_enode->get_state ();
+ const region_model *src_model = src_state.m_region_model;
+
+ unsigned j;
+ exploded_edge *e;
+ FOR_EACH_VEC_ELT (final_enode->m_succs, j, e)
+ {
+ exploded_node *dst = e->m_dest;
+ const program_state &dst_state = dst->get_state ();
+ const region_model *dst_model = dst_state.m_region_model;
+ if (src_model->get_dynamic_extents ()
+ != dst_model->get_dynamic_extents ())
+ {
+ unsigned i;
+ const region *reg;
+ bool emitted = false;
+ FOR_EACH_VEC_ELT (interest->m_region_creation, i, reg)
+ {
+ const region *base_reg = reg->get_base_region ();
+ const svalue *old_extents
+ = src_model->get_dynamic_extents (base_reg);
+ const svalue *new_extents
+ = dst_model->get_dynamic_extents (base_reg);
+ if (old_extents == NULL && new_extents != NULL)
+ switch (base_reg->get_kind ())
+ {
+ default:
+ break;
+ case RK_HEAP_ALLOCATED:
+ case RK_ALLOCA:
+ emission_path->add_region_creation_event
+ (reg,
+ src_point.get_location (),
+ src_point.get_fndecl (),
+ src_stack_depth);
+ emitted = true;
+ break;
+ }
+ }
+ if (emitted)
+ break;
+ }
+ }
}
/* Subclass of state_change_visitor that creates state_change_event
diff --git a/gcc/analyzer/diagnostic-manager.h b/gcc/analyzer/diagnostic-manager.h
index b9bb7c8c254..266eed8f9cb 100644
--- a/gcc/analyzer/diagnostic-manager.h
+++ b/gcc/analyzer/diagnostic-manager.h
@@ -149,6 +149,10 @@ private:
const exploded_path &epath,
checker_path *emission_path) const;
+ void add_event_on_final_node (const exploded_node *final_enode,
+ checker_path *emission_path,
+ interesting_t *interest) const;
+
void add_events_for_eedge (const path_builder &pb,
const exploded_edge &eedge,
checker_path *emission_path,
diff --git a/gcc/analyzer/pending-diagnostic.h b/gcc/analyzer/pending-diagnostic.h
index 9e1c656bf0a..4ea469e1879 100644
--- a/gcc/analyzer/pending-diagnostic.h
+++ b/gcc/analyzer/pending-diagnostic.h
@@ -58,6 +58,17 @@ 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
@@ -215,6 +226,15 @@ class pending_diagnostic
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.
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 5bd5dc720e9..5d939327e01 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -73,6 +73,7 @@ along with GCC; see the file COPYING3. If not see
#include "tree-ssa-operands.h"
#include "ssa-iterators.h"
#include "calls.h"
+#include "is-a.h"
#if ENABLE_ANALYZER
@@ -2799,6 +2800,373 @@ region_model::check_region_for_read (const region *src_reg,
check_region_access (src_reg, DIR_READ, ctxt);
}
+/* Concrete subclass for casts of pointers that lead to trailing bytes. */
+
+class dubious_allocation_size
+: public pending_diagnostic_subclass<dubious_allocation_size>
+{
+public:
+ dubious_allocation_size (const region *lhs, const region *rhs)
+ : m_lhs (lhs), m_rhs (rhs), m_expr (NULL_TREE)
+ {}
+
+ dubious_allocation_size (const region *lhs, const region *rhs,
+ tree expr)
+ : m_lhs (lhs), m_rhs (rhs), m_expr (expr)
+ {}
+
+ const char *get_kind () const final override
+ {
+ return "dubious_allocation_size";
+ }
+
+ bool operator== (const dubious_allocation_size &other) const
+ {
+ return m_lhs == other.m_lhs && m_rhs == other.m_rhs
+ && pending_diagnostic::same_tree_p (m_expr, other.m_expr);
+ }
+
+ int get_controlling_option () const final override
+ {
+ return OPT_Wanalyzer_allocation_size;
+ }
+
+ bool emit (rich_location *rich_loc) final override
+ {
+ diagnostic_metadata m;
+ m.add_cwe (131);
+
+ return warning_meta (rich_loc, m, get_controlling_option (),
+ "allocated buffer size is not a multiple 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. */
+ return ev.formatted_print ("assigned to %qT here;"
+ " %<sizeof (%T)%> is %qE",
+ m_lhs->get_type (), pointee_type,
+ size_in_bytes (pointee_type));
+
+ if (m_expr)
+ {
+ if (TREE_CODE (m_expr) == INTEGER_CST)
+ return ev.formatted_print ("allocated %E bytes and assigned to"
+ " %qT here; %<sizeof (%T)%> is %qE",
+ m_expr, m_lhs->get_type (), pointee_type,
+ size_in_bytes (pointee_type));
+ else
+ return ev.formatted_print ("allocated %qE bytes and assigned to"
+ " %qT here; %<sizeof (%T)%> is %qE",
+ m_expr, m_lhs->get_type (), pointee_type,
+ size_in_bytes (pointee_type));
+ }
+
+ return ev.formatted_print ("allocated and assigned to %qT here;"
+ " %<sizeof (%T)%> is %qE",
+ m_lhs->get_type (), pointee_type,
+ size_in_bytes (pointee_type));
+ }
+
+ void mark_interesting_stuff (interesting_t *interest) final override
+ {
+ interest->add_region_creation (m_rhs);
+ }
+
+private:
+ const region *m_lhs;
+ const region *m_rhs;
+ const tree m_expr;
+ const evdesc::region_creation *m_allocation_event;
+};
+
+/* Return true on dubious allocation sizes for constant sizes. */
+
+static bool
+capacity_compatible_with_type (tree cst, tree pointee_size_tree,
+ bool is_struct)
+{
+ gcc_assert (TREE_CODE (cst) == INTEGER_CST);
+ gcc_assert (TREE_CODE (pointee_size_tree) == INTEGER_CST);
+
+ unsigned HOST_WIDE_INT pointee_size = TREE_INT_CST_LOW (pointee_size_tree);
+ unsigned HOST_WIDE_INT alloc_size = TREE_INT_CST_LOW (cst);
+
+ if (is_struct)
+ return alloc_size >= pointee_size;
+ return alloc_size % pointee_size == 0;
+}
+
+static bool
+capacity_compatible_with_type (tree cst, tree pointee_size_tree)
+{
+ return capacity_compatible_with_type (cst, pointee_size_tree, false);
+}
+
+/* Checks whether SVAL could be a multiple of SIZE_CST.
+
+ It works by visiting all svalues inside SVAL until it reaches
+ atomic nodes. From those, it goes back up again and adds each
+ node that might be a multiple of SIZE_CST to the RESULT_SET. */
+
+class size_visitor : public visitor
+{
+public:
+ size_visitor (tree size_cst, const svalue *sval, constraint_manager *cm)
+ : m_size_cst (size_cst), m_sval (sval), m_cm (cm)
+ {
+ sval->accept (this);
+ }
+
+ bool get_result ()
+ {
+ return result_set.contains (m_sval);
+ }
+
+ void visit_constant_svalue (const constant_svalue *sval) final override
+ {
+ if (capacity_compatible_with_type (sval->get_constant (), m_size_cst))
+ result_set.add (sval);
+ }
+
+ void visit_unknown_svalue (const unknown_svalue *sval ATTRIBUTE_UNUSED)
+ final override
+ {
+ result_set.add (sval);
+ }
+
+ void visit_poisoned_svalue (const poisoned_svalue *sval ATTRIBUTE_UNUSED)
+ final override
+ {
+ result_set.add (sval);
+ }
+
+ void visit_unaryop_svalue (const unaryop_svalue *sval)
+ {
+ const svalue *arg = sval->get_arg ();
+ if (result_set.contains (arg))
+ result_set.add (sval);
+ }
+
+ void visit_binop_svalue (const binop_svalue *sval) final override
+ {
+ const svalue *arg0 = sval->get_arg0 ();
+ const svalue *arg1 = sval->get_arg1 ();
+
+ if (sval->get_op () == MULT_EXPR)
+ {
+ if (result_set.contains (arg0) || result_set.contains (arg1))
+ result_set.add (sval);
+ }
+ else
+ {
+ if (result_set.contains (arg0) && result_set.contains (arg1))
+ result_set.add (sval);
+ }
+ }
+
+ void visit_repeated_svalue (const repeated_svalue *sval)
+ {
+ sval->get_inner_svalue ()->accept (this);
+ if (result_set.contains (sval->get_inner_svalue ()))
+ result_set.add (sval);
+ }
+
+ void visit_unmergeable_svalue (const unmergeable_svalue *sval) final override
+ {
+ sval->get_arg ()->accept (this);
+ if (result_set.contains (sval->get_arg ()))
+ result_set.add (sval);
+ }
+
+ void visit_widening_svalue (const widening_svalue *sval) final override
+ {
+ const svalue *base = sval->get_base_svalue ();
+ const svalue *iter = sval->get_iter_svalue ();
+
+ if (result_set.contains (base) && result_set.contains (iter))
+ result_set.add (sval);
+ }
+
+ void visit_conjured_svalue (const conjured_svalue *sval ATTRIBUTE_UNUSED)
+ final override
+ {
+ equiv_class_id id (-1);
+ if (m_cm->get_equiv_class_by_svalue (sval, &id))
+ {
+ if (tree cst_val = id.get_obj (*m_cm).get_any_constant ())
+ {
+ if (capacity_compatible_with_type (cst_val, m_size_cst))
+ result_set.add (sval);
+ }
+ else
+ {
+ result_set.add (sval);
+ }
+ }
+ }
+
+ void visit_asm_output_svalue (const asm_output_svalue *sval ATTRIBUTE_UNUSED)
+ final override
+ {
+ result_set.add (sval);
+ }
+
+ void visit_const_fn_result_svalue (const const_fn_result_svalue
+ *sval ATTRIBUTE_UNUSED) final override
+ {
+ result_set.add (sval);
+ }
+
+private:
+ tree m_size_cst;
+ const svalue *m_sval;
+ constraint_manager *m_cm;
+ svalue_set result_set; /* Used as a mapping of svalue*->bool. */
+};
+
+/* Return true if a struct or union either uses the inheritance pattern,
+ where the first field is a base struct, or the flexible array member
+ pattern, where the last field is an array without a specified size. */
+
+static bool
+struct_or_union_with_inheritance_p (tree struc)
+{
+ tree iter = TYPE_FIELDS (struc);
+ if (iter == NULL_TREE)
+ return false;
+ if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (iter)))
+ return true;
+
+ tree last_field;
+ while (iter != NULL_TREE)
+ {
+ last_field = iter;
+ iter = DECL_CHAIN (iter);
+ }
+
+ if (last_field != NULL_TREE
+ && TREE_CODE (TREE_TYPE (last_field)) == ARRAY_TYPE)
+ return true;
+
+ return false;
+}
+
+/* Return true if the lhs and rhs of an assignment have different types. */
+
+static bool
+is_any_cast_p (const gimple *stmt)
+{
+ if (const gassign *assign = dyn_cast<const gassign *>(stmt))
+ return gimple_assign_cast_p (assign)
+ || !pending_diagnostic::same_tree_p (
+ TREE_TYPE (gimple_assign_lhs (assign)),
+ TREE_TYPE (gimple_assign_rhs1 (assign)));
+ else if (const gcall *call = dyn_cast<const gcall *>(stmt))
+ {
+ tree lhs = gimple_call_lhs (call);
+ return lhs != NULL_TREE && !pending_diagnostic::same_tree_p (
+ TREE_TYPE (gimple_call_lhs (call)),
+ gimple_call_return_type (call));
+ }
+
+ return false;
+}
+
+/* On pointer assignments, check whether the buffer size of
+ RHS_SVAL is compatible with the type of the LHS_REG.
+ Use a non-null CTXT to report allocation size warnings. */
+
+void
+region_model::check_region_size (const region *lhs_reg, const svalue *rhs_sval,
+ region_model_context *ctxt) const
+{
+ if (!ctxt || ctxt->get_stmt () == NULL)
+ return;
+ /* Only report warnings on assignments that actually change the type. */
+ if (!is_any_cast_p (ctxt->get_stmt ()))
+ return;
+
+ const region_svalue *reg_sval = dyn_cast <const region_svalue *> (rhs_sval);
+ if (!reg_sval)
+ return;
+
+ tree pointer_type = lhs_reg->get_type ();
+ if (pointer_type == NULL_TREE || !POINTER_TYPE_P (pointer_type))
+ return;
+
+ tree pointee_type = TREE_TYPE (pointer_type);
+ /* Make sure that the type on the left-hand size actually has a size. */
+ if (pointee_type == NULL_TREE || VOID_TYPE_P (pointee_type)
+ || TYPE_SIZE_UNIT (pointee_type) == NULL_TREE)
+ return;
+
+ /* Bail out early on pointers to structs where we can
+ not deduce whether the buffer size is compatible. */
+ bool is_struct = RECORD_OR_UNION_TYPE_P (pointee_type);
+ if (is_struct && struct_or_union_with_inheritance_p (pointee_type))
+ return;
+
+ tree pointee_size_tree = size_in_bytes (pointee_type);
+ /* We give up if the type size is not known at compile-time or the
+ type size is always compatible regardless of the buffer size. */
+ if (TREE_CODE (pointee_size_tree) != INTEGER_CST
+ || integer_zerop (pointee_size_tree)
+ || integer_onep (pointee_size_tree))
+ return;
+
+ const region *rhs_reg = reg_sval->get_pointee ();
+ const svalue *capacity = get_capacity (rhs_reg);
+ switch (capacity->get_kind ())
+ {
+ case svalue_kind::SK_CONSTANT:
+ {
+ const constant_svalue *cst_cap_sval
+ = as_a <const constant_svalue *> (capacity);
+ tree cst_cap = cst_cap_sval->get_constant ();
+ if (!capacity_compatible_with_type (cst_cap, pointee_size_tree,
+ is_struct))
+ ctxt->warn (new dubious_allocation_size (lhs_reg, rhs_reg,
+ cst_cap));
+ }
+ break;
+ default:
+ {
+ if (!is_struct)
+ {
+ size_visitor v (pointee_size_tree, capacity, m_constraints);
+ if (!v.get_result ())
+ {
+ tree expr = get_representative_tree (capacity);
+ ctxt->warn (new dubious_allocation_size (lhs_reg, rhs_reg,
+ expr));
+ }
+ }
+ break;
+ }
+ }
+}
+
/* Set the value of the region given by LHS_REG to the value given
by RHS_SVAL.
Use CTXT to report any warnings associated with writing to LHS_REG. */
@@ -2810,6 +3178,8 @@ region_model::set_value (const region *lhs_reg, const svalue *rhs_sval,
gcc_assert (lhs_reg);
gcc_assert (rhs_sval);
+ check_region_size (lhs_reg, rhs_sval, ctxt);
+
check_region_for_write (lhs_reg, ctxt);
m_store.set_value (m_mgr->get_store_manager(), lhs_reg, rhs_sval,
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index 129aad2945f..6dda43f5658 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -865,6 +865,8 @@ class region_model
region_model_context *ctxt) const;
void check_region_for_read (const region *src_reg,
region_model_context *ctxt) const;
+ void check_region_size (const region *lhs_reg, const svalue *rhs_sval,
+ region_model_context *ctxt) const;
void check_call_args (const call_details &cd) const;
void check_external_function_for_access_attr (const gcall *call,
diff --git a/gcc/analyzer/svalue.cc b/gcc/analyzer/svalue.cc
index 2f9149412b9..7bad3cea31b 100644
--- a/gcc/analyzer/svalue.cc
+++ b/gcc/analyzer/svalue.cc
@@ -732,8 +732,8 @@ region_svalue::dump_to_pp (pretty_printer *pp, bool simple) const
void
region_svalue::accept (visitor *v) const
{
- v->visit_region_svalue (this);
m_reg->accept (v);
+ v->visit_region_svalue (this);
}
/* Implementation of svalue::implicitly_live_p vfunc for region_svalue. */
@@ -1031,8 +1031,8 @@ initial_svalue::dump_to_pp (pretty_printer *pp, bool simple) const
void
initial_svalue::accept (visitor *v) const
{
- v->visit_initial_svalue (this);
m_reg->accept (v);
+ v->visit_initial_svalue (this);
}
/* Implementation of svalue::implicitly_live_p vfunc for initial_svalue. */
@@ -1123,8 +1123,8 @@ unaryop_svalue::dump_to_pp (pretty_printer *pp, bool simple) const
void
unaryop_svalue::accept (visitor *v) const
{
- v->visit_unaryop_svalue (this);
m_arg->accept (v);
+ v->visit_unaryop_svalue (this);
}
/* Implementation of svalue::implicitly_live_p vfunc for unaryop_svalue. */
@@ -1225,9 +1225,9 @@ binop_svalue::dump_to_pp (pretty_printer *pp, bool simple) const
void
binop_svalue::accept (visitor *v) const
{
- v->visit_binop_svalue (this);
m_arg0->accept (v);
m_arg1->accept (v);
+ v->visit_binop_svalue (this);
}
/* Implementation of svalue::implicitly_live_p vfunc for binop_svalue. */
@@ -1283,9 +1283,9 @@ sub_svalue::dump_to_pp (pretty_printer *pp, bool simple) const
void
sub_svalue::accept (visitor *v) const
{
- v->visit_sub_svalue (this);
m_parent_svalue->accept (v);
m_subregion->accept (v);
+ v->visit_sub_svalue (this);
}
/* Implementation of svalue::implicitly_live_p vfunc for sub_svalue. */
@@ -1352,8 +1352,8 @@ repeated_svalue::dump_to_pp (pretty_printer *pp, bool simple) const
void
repeated_svalue::accept (visitor *v) const
{
- v->visit_repeated_svalue (this);
m_inner_svalue->accept (v);
+ v->visit_repeated_svalue (this);
}
/* Implementation of svalue::all_zeroes_p for repeated_svalue. */
@@ -1494,8 +1494,8 @@ bits_within_svalue::maybe_fold_bits_within (tree type,
void
bits_within_svalue::accept (visitor *v) const
{
- v->visit_bits_within_svalue (this);
m_inner_svalue->accept (v);
+ v->visit_bits_within_svalue (this);
}
/* Implementation of svalue::implicitly_live_p vfunc for bits_within_svalue. */
@@ -1544,9 +1544,9 @@ widening_svalue::dump_to_pp (pretty_printer *pp, bool simple) const
void
widening_svalue::accept (visitor *v) const
{
- v->visit_widening_svalue (this);
m_base_sval->accept (v);
m_iter_sval->accept (v);
+ v->visit_widening_svalue (this);
}
/* Attempt to determine in which direction this value is changing
@@ -1711,8 +1711,8 @@ unmergeable_svalue::dump_to_pp (pretty_printer *pp, bool simple) const
void
unmergeable_svalue::accept (visitor *v) const
{
- v->visit_unmergeable_svalue (this);
m_arg->accept (v);
+ v->visit_unmergeable_svalue (this);
}
/* Implementation of svalue::implicitly_live_p vfunc for unmergeable_svalue. */
@@ -1776,13 +1776,13 @@ compound_svalue::dump_to_pp (pretty_printer *pp, bool simple) const
void
compound_svalue::accept (visitor *v) const
{
- v->visit_compound_svalue (this);
for (binding_map::iterator_t iter = m_map.begin ();
iter != m_map.end (); ++iter)
{
//(*iter).first.accept (v);
(*iter).second->accept (v);
}
+ v->visit_compound_svalue (this);
}
/* Calculate what the complexity of a compound_svalue instance for MAP
@@ -1903,8 +1903,8 @@ conjured_svalue::dump_to_pp (pretty_printer *pp, bool simple) const
void
conjured_svalue::accept (visitor *v) const
{
- v->visit_conjured_svalue (this);
m_id_reg->accept (v);
+ v->visit_conjured_svalue (this);
}
/* class asm_output_svalue : public svalue. */
@@ -1968,9 +1968,9 @@ asm_output_svalue::input_idx_to_asm_idx (unsigned input_idx) const
void
asm_output_svalue::accept (visitor *v) const
{
- v->visit_asm_output_svalue (this);
for (unsigned i = 0; i < m_num_inputs; i++)
m_input_arr[i]->accept (v);
+ v->visit_asm_output_svalue (this);
}
/* class const_fn_result_svalue : public svalue. */
@@ -2021,9 +2021,9 @@ const_fn_result_svalue::dump_input (pretty_printer *pp,
void
const_fn_result_svalue::accept (visitor *v) const
{
- v->visit_const_fn_result_svalue (this);
for (unsigned i = 0; i < m_num_inputs; i++)
m_input_arr[i]->accept (v);
+ v->visit_const_fn_result_svalue (this);
}
} // namespace ana
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index d86e45ac982..89c8dce3038 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -9739,6 +9739,7 @@ This analysis is much more expensive than other GCC warnings.
Enabling this option effectively enables the following warnings:
@gccoptlist{ @gol
+-Wanalyzer-allocation-size @gol
-Wanalyzer-double-fclose @gol
-Wanalyzer-double-free @gol
-Wanalyzer-exposure-through-output-file @gol
@@ -9791,6 +9792,19 @@ By default, the analysis silently stops if the code is too
complicated for the analyzer to fully explore and it reaches an internal
limit. The @option{-Wanalyzer-too-complex} option warns if this occurs.
+@item -Wno-analyzer-allocation-size
+@opindex Wanalyzer-allocation-size
+@opindex Wno-analyzer-allocation-size
+This warning requires @option{-fanalyzer}, which enables it; use
+@option{-Wno-analyzer-allocation-size}
+to disable it.
+
+This diagnostic warns for paths through the code in which a pointer to
+a buffer is assigned to point at a buffer with a size that is not a
+multiple of @code{sizeof (*pointer)}.
+
+See @url{https://cwe.mitre.org/data/definitions/131.html, CWE-131: Incorrect Calculation of Buffer Size}.
+
@item -Wno-analyzer-double-fclose
@opindex Wanalyzer-double-fclose
@opindex Wno-analyzer-double-fclose
diff --git a/gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c b/gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c
new file mode 100644
index 00000000000..4fc2bf75d6c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c
@@ -0,0 +1,116 @@
+#include <stdlib.h>
+#include <stdio.h>
+
+/* Tests with constant buffer sizes. */
+
+void test_1 (void)
+{
+ short *ptr = malloc (21 * sizeof (short));
+ free (ptr);
+}
+
+void test_2 (void)
+{
+ int *ptr = malloc (21 * sizeof (short)); /* { dg-line malloc2 } */
+ free (ptr);
+
+ /* { dg-warning "allocated buffer size is not a multiple of the pointee's size \\\[CWE-131\\\]" "warning" { target *-*-* } malloc2 } */
+ /* { dg-message "\\d+ bytes" "note" { target *-*-* } malloc2 } */
+ /* { dg-message "'int \\*' here; 'sizeof \\(int\\)' is '\\d+'" "note" { target *-*-* } malloc2 } */
+}
+
+void test_3 (void)
+{
+ void *ptr = malloc (21 * sizeof (short));
+ short *sptr = (short *)ptr;
+ free (sptr);
+}
+
+void test_4 (void)
+{
+ void *ptr = malloc (21 * sizeof (short)); /* { dg-message "\\d+ bytes" } */
+ int *iptr = (int *)ptr; /* { dg-line assign4 } */
+ free (iptr);
+
+ /* { dg-warning "allocated buffer size is not a multiple of the pointee's size \\\[CWE-131\\\]" "warning" { target *-*-* } assign4 } */
+ /* { dg-message "'int \\*' here; 'sizeof \\(int\\)' is '\\d+'" "note" { target *-*-* } assign4 } */
+}
+
+void test_5 (void)
+{
+ int user_input;
+ scanf("%i", &user_input);
+ int n;
+ if (user_input == 0)
+ n = 21 * sizeof (short);
+ else
+ n = 42 * sizeof (short);
+ void *ptr = malloc (n);
+ short *sptr = (short *)ptr;
+ free (sptr);
+}
+
+void test_6 (void)
+{
+ int user_input;
+ scanf("%i", &user_input);
+ int n;
+ if (user_input == 0)
+ n = 21 * sizeof (short);
+ else
+ n = 42 * sizeof (short);
+ void *ptr = malloc (n); /* { dg-message "" "note" } */
+ /* ^^^ on widening_svalues no expr is returned
+ by get_representative_tree at the moment. */
+ int *iptr = (int *)ptr; /* { dg-line assign6 } */
+ free (iptr);
+
+ /* { dg-warning "allocated buffer size is not a multiple of the pointee's size \\\[CWE-131\\\]" "warning" { target *-*-* } assign6 } */
+ /* { dg-message "'int \\*' here; 'sizeof \\(int\\)' is '\\d+'" "note" { target *-*-* } assign6 } */
+}
+
+void test_7 (void)
+{
+ int user_input;
+ scanf("%i", &user_input);
+ int n;
+ if (user_input == 0)
+ n = 1;
+ else if (user_input == 2)
+ n = 5;
+ else
+ n = 7;
+ /* n is an unknown_svalue at this point. */
+ void *ptr = malloc (n);
+ int *iptr = (int *)ptr;
+ free (iptr);
+}
+
+void *create_buffer (int n)
+{
+ return malloc(n);
+}
+
+void test_8 (void)
+{
+ int *buf = create_buffer(4 * sizeof (int));
+ free (buf);
+}
+
+void test_9 (void)
+{
+ /* FIXME: At the moment, region_model::set_value (lhs, <return_value>)
+ is called at the src_node of the return edge. This edge has no stmts
+ associated with it, leading to a rejection of the warning inside
+ impl_region_model_context::warn. To ensure that the indentation
+ in the diagnostic is right, the warning has to be emitted on an EN
+ that is after the return edge. */
+ int *buf = create_buffer(42); /* { dg-warning "" "" { xfail *-*-* } } */
+ free (buf);
+}
+
+void test_10 (int n)
+{
+ char *ptr = malloc (7 * n);
+ free (ptr);
+}
\ No newline at end of file
diff --git a/gcc/testsuite/gcc.dg/analyzer/allocation-size-2.c b/gcc/testsuite/gcc.dg/analyzer/allocation-size-2.c
new file mode 100644
index 00000000000..37bbbac87c5
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/allocation-size-2.c
@@ -0,0 +1,155 @@
+#include <stdlib.h>
+#include <stdio.h>
+
+/* Tests with symbolic buffer sizes. */
+
+void test_1 (int n)
+{
+ short *ptr = malloc (n * sizeof (short));
+ free (ptr);
+}
+
+void test_2 (int n)
+{
+ int *ptr = malloc (n * sizeof (short)); /* { dg-line malloc2 } */
+ free (ptr);
+
+ /* { dg-warning "allocated buffer size is not a multiple of the pointee's size \\\[CWE-131\\\]" "warning" { target *-*-* } malloc2 } */
+ /* { dg-message "'\[a-z0-9\\*\\(\\)\\s\]*' bytes" "note" { target *-*-* } malloc2 } */
+ /* { dg-message "'int \\*' here; 'sizeof \\(int\\)' is '\\d+'" "note" { target *-*-* } malloc2 } */
+}
+
+void test_3 (int n)
+{
+ void *ptr = malloc (n * sizeof (short));
+ short *sptr = (short *)ptr;
+ free (sptr);
+}
+
+void test_4 (int n)
+{
+ void *ptr = malloc (n * sizeof (short)); /* { dg-message "'\[a-z0-9\\*\\(\\)\\s\]*'" "note" } */
+ int *iptr = (int *)ptr; /* { dg-line assign4 } */
+ free (iptr);
+
+ /* { dg-warning "allocated buffer size is not a multiple of the pointee's size \\\[CWE-131\\\]" "warning" { target *-*-* } assign4 } */
+ /* { dg-message "'int \\*' here; 'sizeof \\(int\\)' is '\\d+'" "note" { target *-*-* } assign4 } */
+}
+
+void test_5 (void)
+{
+ int user_input;
+ scanf("%i", &user_input);
+ int n;
+ if (user_input == 0)
+ n = 3 * user_input * sizeof (short);
+ else
+ n = 5 * user_input * sizeof (short);
+ void *ptr = malloc (n);
+ short *sptr = (short *)ptr;
+ free (sptr);
+}
+
+void test_6 (void)
+{
+ int user_input;
+ scanf("%i", &user_input);
+ int n;
+ if (user_input == 0)
+ n = user_input;
+ else if (user_input == 2)
+ n = user_input * 3;
+ else
+ n = user_input * 5;
+ /* n is an unknown_svalue at this point. */
+ void *ptr = malloc (n);
+ int *iptr = (int *)ptr;
+ free (iptr);
+}
+
+void *create_buffer(int n)
+{
+ return malloc(n);
+}
+
+void test_7(int n)
+{
+ int *buf = create_buffer(n * sizeof (int));
+ free (buf);
+}
+
+void test_8(int n)
+{
+ /* FIXME: At the moment, region_model::set_value (lhs, <return_value>)
+ is called at the src_node of the return edge. This edge has no stmts
+ associated with it, leading to a rejection of the warning inside
+ impl_region_model_context::warn. To ensure that the indentation
+ in the diagnostic is right, the warning has to be emitted on an EN
+ that is after the return edge. */
+ int *buf = create_buffer(n * sizeof(short)); /* { dg-warning "" "" { xfail *-*-* } } */
+ free (buf);
+}
+
+void test_9 (void)
+{
+ int n;
+ scanf("%i", &n);
+ /* n is a conjured_svalue. */
+ void *ptr = malloc (n); /* { dg-message "'n' bytes" "note" } */
+ int *iptr = (int *)ptr; /* { dg-line assign9 } */
+ free (iptr);
+
+ /* { dg-warning "allocated buffer size is not a multiple of the pointee's size \\\[CWE-131\\\]" "warning" { target *-*-* } assign9 } */
+ /* { dg-message "'int \\*' here; 'sizeof \\(int\\)' is '\\d+'" "note" { target *-*-* } assign9 } */
+}
+
+void test_11 (void)
+{
+ int n;
+ scanf("%i", &n);
+ void *ptr = malloc (n);
+ if (n == sizeof (int))
+ {
+ /* n is a conjured_svalue but guarded such that we
+ know the value is a multiple of sizeof (*iptr). */
+ int *iptr = (int *)ptr;
+ free (iptr);
+ }
+ else
+ free (ptr);
+}
+
+void test_12 (void)
+{
+ int n;
+ scanf("%i", &n);
+ void *ptr = malloc (n); /* { dg-message "'n' bytes" } */
+ if (n == 5)
+ {
+ /* n is a conjured_svalue but guarded such that we
+ know the value isn't a multiple of sizeof (*iptr). */
+ int *iptr = (int *)ptr; /* { dg-line assign12 } */
+ free (iptr);
+ }
+ else
+ free (ptr);
+ /* { dg-warning "allocated buffer size is not a multiple of the pointee's size \\\[CWE-131\\\]" "warning" { target *-*-* } assign12 } */
+ /* { dg-message "'int \\*' here; 'sizeof \\(int\\)' is '\\d+'" "note" { target *-*-* } assign12 } */
+}
+
+void test_13 (void)
+{
+ int n;
+ scanf("%i", &n);
+ void *ptr = malloc (n);
+ if (n == n * n)
+ {
+ /* n is a conjured_svalue but guarded such that we don't have an
+ equivalence class for it. In such cases, we assume that the
+ condition ensures that the value is okay. */
+ int *iptr = (int *)ptr;
+ free (iptr);
+ }
+ else
+ free (ptr);
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/allocation-size-3.c b/gcc/testsuite/gcc.dg/analyzer/allocation-size-3.c
new file mode 100644
index 00000000000..fdc1c56b7ee
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/allocation-size-3.c
@@ -0,0 +1,45 @@
+#include <stdlib.h>
+#include <stdio.h>
+
+/* CWE-131 example 5 */
+void test_1 (void)
+{
+ int *id_sequence = (int *) malloc (3); /* { dg-line malloc1 } */
+ if (id_sequence == NULL) exit (1);
+
+ id_sequence[0] = 13579;
+ id_sequence[1] = 24680;
+ id_sequence[2] = 97531;
+
+ free (id_sequence);
+
+ /* { dg-warning "allocated buffer size is not a multiple of the pointee's size \\\[CWE-131\\\]" "warning" { target *-*-* } malloc1 } */
+ /* { dg-message "\\d+ bytes" "note" { target *-*-* } malloc1 } */
+ /* { dg-message "'int \\*' here; 'sizeof \\(int\\)' is '\\d+'" "note" { target *-*-* } malloc1 } */
+}
+
+void test_2 (void)
+{
+ int *ptr = malloc (10 + sizeof(int)); /* { dg-line malloc2 } */
+ free (ptr);
+
+ /* { dg-warning "allocated buffer size is not a multiple of the pointee's size \\\[CWE-131\\\]" "warning" { target *-*-* } malloc2 } */
+ /* { dg-message "\\d+ bytes" "note" { target *-*-* } malloc2 } */
+ /* { dg-message "'int \\*' here; 'sizeof \\(int\\)' is '\\d+'" "note" { target *-*-* } malloc2 } */
+}
+
+void test_3 (int n)
+{
+ int *ptr = malloc (n + sizeof (int)); /* { dg-line malloc3 } */
+ free (ptr);
+
+ /* { dg-warning "allocated buffer size is not a multiple of the pointee's size \\\[CWE-131\\\]" "warning" { target *-*-* } malloc3 } */
+ /* { dg-message "'\[a-z0-9\\+\\(\\)\\s\]*' bytes" "note" { target *-*-* } malloc3 } */
+ /* { dg-message "'int \\*' here; 'sizeof \\(int\\)' is '\\d+'" "note" { target *-*-* } malloc3 } */
+}
+
+void test_4 (int n, int m)
+{
+ int *ptr = malloc ((n + m) * sizeof (int));
+ free (ptr);
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/allocation-size-4.c b/gcc/testsuite/gcc.dg/analyzer/allocation-size-4.c
new file mode 100644
index 00000000000..e475c1586a3
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/allocation-size-4.c
@@ -0,0 +1,60 @@
+#include <stdlib.h>
+
+/* Tests related to structs. */
+
+struct base {
+ int i;
+};
+
+struct sub {
+ struct base b;
+ int j;
+};
+
+struct var_len {
+ int i;
+ char arr[];
+};
+
+
+void test_1 (void)
+{
+ struct base *ptr = malloc (5 * sizeof (struct base));
+ free (ptr);
+}
+
+void test_2 (void)
+{
+ long *ptr = malloc (5 * sizeof (struct base)); /* { dg-line malloc2 } */
+ free (ptr);
+
+ /* { dg-warning "allocated buffer size is not a multiple of the pointee's size \\\[CWE-131\\\]" "warning" { target *-*-* } malloc2 } */
+ /* { dg-message "\\d+ bytes" "note" { target *-*-* } malloc2 } */
+ /* { dg-message "'long (int)? \\*' here; 'sizeof \\(long (int)?\\)' is '\\d+'" "note" { target *-*-* } malloc2 } */
+}
+
+void test_3 (void)
+{
+ /* Even though 10 bytes is not a multiple of 4, we do not warn to prevent
+ a false positive in case s is the base struct of a struct inheritance. */
+ struct base *ptr = malloc (10);
+ free (ptr);
+}
+
+void test_4 (void)
+{
+ struct var_len *ptr = malloc (10);
+ free (ptr);
+}
+
+void test_5 (void)
+{
+ /* For constant sizes, we warn if the buffer
+ is too small to hold a single struct. */
+ struct base *ptr = malloc (2); /* { dg-line malloc5 } */
+ free (ptr);
+
+ /* { dg-warning "allocated buffer size is not a multiple of the pointee's size \\\[CWE-131\\\]" "warning" { target *-*-* } malloc5 } */
+ /* { dg-message "\\d+ bytes" "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-5.c b/gcc/testsuite/gcc.dg/analyzer/allocation-size-5.c
new file mode 100644
index 00000000000..ae7e1074ebb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/allocation-size-5.c
@@ -0,0 +1,36 @@
+#include <stdlib.h>
+#include <stdio.h>
+
+/* Tests related to statically allocated buffers. */
+
+typedef struct a {
+ short s;
+} a;
+
+int *test_1 (void)
+{
+ a A; /* { dg-message "\\d+ bytes" "note" } */
+ A.s = 1;
+ int *ptr = (int *) &A; /* { dg-line assign1 } */
+ return ptr;
+
+ /* { dg-warning "allocated buffer size is not a multiple of the pointee's size \\\[CWE-131\\\]" "warning" { target *-*-* } assign1 } */
+ /* { dg-message "assigned to 'int \\*' here; 'sizeof \\(int\\)' is '\\d+'" "note" { target *-*-* } assign1 } */
+}
+
+int *test2 (void)
+{
+ char arr[sizeof (int)];
+ int *ptr = (int *)arr;
+ return ptr;
+}
+
+int *test3 (void)
+{
+ char arr[sizeof (short)]; /* { dg-message "\\d+ bytes" "note" } */
+ int *ptr = (int *)arr; /* { dg-line assign3 } */
+ return ptr;
+
+ /* { dg-warning "allocated buffer size is not a multiple of the pointee's size \\\[CWE-131\\\]" "warning" { target *-*-* } assign3 } */
+ /* { dg-message "assigned to 'int \\*' here; 'sizeof \\(int\\)' is '\\d+'" "note" { target *-*-* } assign3 } */
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr96639.c b/gcc/testsuite/gcc.dg/analyzer/pr96639.c
index 02ca3f084a2..aedf0464dc9 100644
--- a/gcc/testsuite/gcc.dg/analyzer/pr96639.c
+++ b/gcc/testsuite/gcc.dg/analyzer/pr96639.c
@@ -3,7 +3,7 @@ void *calloc (__SIZE_TYPE__, __SIZE_TYPE__);
int
x7 (void)
{
- int **md = calloc (1, 1);
+ int **md = calloc (1, sizeof (void *));
return md[0][0]; /* { dg-warning "possibly-NULL" "unchecked deref" } */
/* { dg-warning "leak of 'md'" "leak" { target *-*-* } .-1 } */
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2022-07-02 17:08 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-02 17:08 [gcc r13-1405] analyzer: add allocation size checker [PR105900] Tim Lange
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).