]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
analyzer: add allocation size checker [PR105900]
authorTim Lange <mail@tim-lange.me>
Thu, 30 Jun 2022 22:02:17 +0000 (00:02 +0200)
committerTim Lange <mail@tim-lange.me>
Sat, 2 Jul 2022 17:07:49 +0000 (19:07 +0200)
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>
16 files changed:
gcc/analyzer/analyzer.opt
gcc/analyzer/checker-path.cc
gcc/analyzer/checker-path.h
gcc/analyzer/diagnostic-manager.cc
gcc/analyzer/diagnostic-manager.h
gcc/analyzer/pending-diagnostic.h
gcc/analyzer/region-model.cc
gcc/analyzer/region-model.h
gcc/analyzer/svalue.cc
gcc/doc/invoke.texi
gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/analyzer/allocation-size-2.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/analyzer/allocation-size-3.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/analyzer/allocation-size-4.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/analyzer/allocation-size-5.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/analyzer/pr96639.c

index 8ef6a6fa13cdfc13089087d688c8fea4dfc47a94..5021376b6fb6c148b6632939b6cf4b3d9a2770e9 100644 (file)
@@ -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.
index 0133dc94137e913d31dcf7152ad849ed8e4772d1..953e192cd55c6728922d2932025a23a466f58ce2 100644 (file)
@@ -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:
index 24decf5ce3d6e50f814f454cd98b6db7ee702cb8..8e48d8a07ab0d07b79fb5207fed038e227f26295 100644 (file)
@@ -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;
index 8ea1f61776e952fccb270b4db77e8502fea3e6f2..4adfda1af65721db6ac5931b3c0178d3bfa352cf 100644 (file)
@@ -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
index b9bb7c8c254640d8beeff88e5a05912ca5ac449d..266eed8f9cb554620986ae3a25efb365f00cd5e7 100644 (file)
@@ -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,
index 9e1c656bf0a12af8ec12a28bf3eeb3d773f6414f..4ea469e18791d20c05ec2fce9b859f16e206a7e4 100644 (file)
@@ -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.
 
index 5bd5dc720e928685c3fa13901fa8f19b6f43b8f7..5d939327e013c2b557abf4cfdde54b4d4950a344 100644 (file)
@@ -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,
index 129aad2945fede1c35f3c486ac747cec39ce9ad8..6dda43f5658b327d0932b7c0a4782f5fdd1e147d 100644 (file)
@@ -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,
index 2f9149412b9d61d7a78faaf617edb04f17d63623..7bad3cea31b4c07ecc3716b0c57941f8416df036 100644 (file)
@@ -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
index d86e45ac98221648a636a95ce09347fa452fac50..89c8dce3038263a67ed52f6a5b5898b63b54d11f 100644 (file)
@@ -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 (file)
index 0000000..4fc2bf7
--- /dev/null
@@ -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 (file)
index 0000000..37bbbac
--- /dev/null
@@ -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 (file)
index 0000000..fdc1c56
--- /dev/null
@@ -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 (file)
index 0000000..e475c15
--- /dev/null
@@ -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 (file)
index 0000000..ae7e107
--- /dev/null
@@ -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 } */
+}
index 02ca3f084a2f9d5b21eaf0db10bf653c4881a582..aedf0464dc937d6b90f6aba04b865fc6b0fae0c9 100644 (file)
@@ -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 } */