public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
From: Tim Lange <mail@tim-lange.me>
To: dmalcolm@redhat.com
Cc: gcc@gcc.gnu.org, Tim Lange <mail@tim-lange.me>
Subject: Re: [RFC] analyzer: allocation size warning
Date: Wed, 22 Jun 2022 16:57:55 +0200	[thread overview]
Message-ID: <20220622145755.38734-1-mail@tim-lange.me> (raw)
In-Reply-To: <5e65c86d518c6e26a970b5fb23c2ed507abd2085.camel@redhat.com>

The checker reaches region-model.cc#3083 in my patch with the
  impl_region_model_context
on the 'after' node of create_buffer() but then discards the warning inside
impl_region_model_context::warn because m_stmt is null. Even if m_stmt were
not be NULL at the 'after' node, my warning would be emitted before the
return edge was taken and thus be wrongly indented like shown below:
/path/to/.c:10:16: warning: Allocated buffer size is not a multiple of the pointee's size [CWE-131] [-Wanalyzer-allocation-size]
   10 |     int *buf = create_buffer(42);
      |                ^~~~~~~~~~~~~~~~~
  ‘main’: events 1-2
    |
    |    9 |   int main (int argc, char **argv) {
    |      |       ^~~~
    |      |       |
    |      |       (1) entry to ‘main’
    |   10 |     int *buf = create_buffer(42);
    |      |                ~~~~~~~~~~~~~~~~~
    |      |                |
    |      |                (2) calling ‘create_buffer’ from ‘main’
    |
    +--> ‘create_buffer’: events 3-4
           |
           |    4 |   void *create_buffer(int n)
           |      |         ^~~~~~~~~~~~~
           |      |         |
           |      |         (3) entry to ‘create_buffer’
           |    5 |   {
           |    6 |     return malloc(n);
           |      |            ~~~~~~~~~
           |      |            |
           |      |            (4) allocated 42 bytes here
           |
         ‘main’: event 5
           |
           |   10 |     int *buf = create_buffer(42);
           |      |                ^~~~~~~~~~~~~~~~~
           |      |                |
           |      |                (5) Assigned to ‘int *’ here
           |
           
The correct warning should be:
/path/to/.c:10:16: warning: Allocated buffer size is not a multiple of the pointee's size [CWE-131] [-Wanalyzer-allocation-size]
   10 |     int *buf = create_buffer(42);
      |                ^~~~~~~~~~~~~~~~~
  ‘main’: events 1-2
    |
    |    9 |   int main (int argc, char **argv) {
    |      |       ^~~~
    |      |       |
    |      |       (1) entry to ‘main’
    |   10 |     int *buf = create_buffer(42);
    |      |                ~~~~~~~~~~~~~~~~~
    |      |                |
    |      |                (2) calling ‘create_buffer’ from ‘main’
    |
    +------------> ‘create_buffer’: events 3-4
                   |
                   |    4 |   void *create_buffer(int n)
                   |      |         ^~~~~~~~~~~~~
                   |      |         |
                   |      |         (3) entry to ‘create_buffer’
                   |    5 |   {
                   |    6 |     return malloc(n);
                   |      |            ~~~~~~~~~
                   |      |            |
                   |      |            (4) allocated 42 bytes here
                   |
‘main’: event 5 <--+
   |
   |   10 |     int *buf = create_buffer(42);
   |      |                ^~~~~~~~~~~~~~~~~
   |      |                |
   |      |                (5) Assigned to ‘int *’ here
   |
For that, the return edge has to be visited to be part of the emission_path.
This is currently not the case as the assignment of the <return_value> to
the caller lhs is handled inside pop_frame, which is transitively called
from program_state::on_edge of the 'after' node of the callee.
I tried to defer the set_value(caller lhs, <return_value>) call to the
'before' node after the return edge but failed to do elegantly. My last try
is in the patch commented out with // FIXME.
My main problem is that I can not pop the frame and later get the
return value easily. Deferring the whole pop_frame to the before node
breaks the assumptions inside exploded_graph::get_or_create_node.

I don't know what's the best/elegant way of solving this. Is a solution to
attach the return svalue to the return edge and then use it later in the
PK_BEFORE_SUPERNODE?

Signed-off-by: Tim Lange <mail@tim-lange.me>
---
 gcc/analyzer/analyzer.opt                     |   4 +
 gcc/analyzer/checker-path.cc                  |  12 +-
 gcc/analyzer/checker-path.h                   |   2 +-
 gcc/analyzer/engine.cc                        |  12 +
 gcc/analyzer/pending-diagnostic.h             |  21 ++
 gcc/analyzer/region-model.cc                  | 322 ++++++++++++++++++
 gcc/analyzer/region-model.h                   |   4 +
 .../gcc.dg/analyzer/allocation-size-1.c       |  63 ++++
 .../gcc.dg/analyzer/allocation-size-2.c       |  44 +++
 .../gcc.dg/analyzer/allocation-size-3.c       |  48 +++
 .../gcc.dg/analyzer/allocation-size-4.c       |  92 +++++
 gcc/testsuite/gcc.dg/analyzer/attr-malloc-6.c |   2 +
 gcc/testsuite/gcc.dg/analyzer/malloc-4.c      |   2 +-
 gcc/testsuite/gcc.dg/analyzer/pr96639.c       |   2 +
 14 files changed, 627 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/allocation-size-2.c
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/allocation-size-3.c
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/allocation-size-4.c

diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt
index 4aea52d3a87..f213989e0bb 100644
--- a/gcc/analyzer/analyzer.opt
+++ b/gcc/analyzer/analyzer.opt
@@ -78,6 +78,10 @@ Wanalyzer-malloc-leak
 Common Var(warn_analyzer_malloc_leak) Init(1) Warning
 Warn about code paths in which a heap-allocated pointer leaks.
 
+Wanalyzer-allocation-size
+Common Var(warn_analyzer_allocation_size) Init(1) Warning
+Warn about code paths in which a buffer is assigned to a incompatible type.
+
 Wanalyzer-mismatching-deallocation
 Common Var(warn_analyzer_mismatching_deallocation) Init(1) Warning
 Warn about code paths in which the wrong deallocation function is called.
diff --git a/gcc/analyzer/checker-path.cc b/gcc/analyzer/checker-path.cc
index 0133dc94137..4ad75e636c1 100644
--- a/gcc/analyzer/checker-path.cc
+++ b/gcc/analyzer/checker-path.cc
@@ -302,8 +302,18 @@ 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/engine.cc b/gcc/analyzer/engine.cc
index 7237cc1a1ca..5bf8697d8f7 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -3740,6 +3740,18 @@ exploded_graph::process_node (exploded_node *node)
 	    program_state::detect_leaks (state, next_state, NULL,
 					 get_ext_state (), &ctxt);
 	  }
+  // FIX ME: Other way than calling return again here?
+  // else 
+  //   {
+  //     const supernode *snode = point.get_supernode ();
+  //     if (snode->m_returning_call)
+  //     {
+  //       impl_region_model_context ctxt (*this, node,
+  //           &state, &next_state,
+  //           &uncertainty, NULL, snode->m_returning_call);
+  //       next_state.m_region_model->set_return (snode->m_returning_call, &ctxt);
+  //     }
+  //   }
 
 	program_point next_point (point.get_next ());
 	exploded_node *next = get_or_create_node (next_point, next_state, node);
diff --git a/gcc/analyzer/pending-diagnostic.h b/gcc/analyzer/pending-diagnostic.h
index 9e1c656bf0a..3f79444ef40 100644
--- a/gcc/analyzer/pending-diagnostic.h
+++ b/gcc/analyzer/pending-diagnostic.h
@@ -1,3 +1,4 @@
+
 /* Classes for analyzer diagnostics.
    Copyright (C) 2019-2022 Free Software Foundation, Inc.
    Contributed by David Malcolm <dmalcolm@redhat.com>.
@@ -58,6 +59,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 +227,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 6b49719d521..acb8bd1bfca 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 "print-tree.h"
 
 #if ENABLE_ANALYZER
 
@@ -653,6 +654,66 @@ private:
   tree m_count_cst;
 };
 
+/* 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, 
+                           const svalue *capacity)
+  : m_lhs(lhs), m_rhs(rhs), m_capacity(capacity) {}
+
+  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;;
+  }
+
+  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
+  {
+    // TODO: better way to print the capacity
+    return ev.formatted_print ("allocated %s here", 
+                                          m_capacity->get_desc(true).m_buffer);
+  }
+
+  label_text describe_final_event (const evdesc::final_event &ev) final override
+  {
+    return ev.formatted_print ("Assigned to %qT here", m_lhs->get_type ());
+  }
+
+  void mark_interesting_stuff (interesting_t *interest) final override
+  {
+    if (m_lhs)
+      interest->add_region_creation (m_lhs);
+    if (m_rhs)
+      interest->add_region_creation (m_rhs);
+  }
+
+private:
+  const region *m_lhs;
+  const region *m_rhs;
+  const svalue *m_capacity;
+};
+
 /* If ASSIGN is a stmt that can be modelled via
      set_value (lhs_reg, SVALUE, CTXT)
    for some SVALUE, get the SVALUE.
@@ -2799,6 +2860,241 @@ region_model::check_region_for_read (const region *src_reg,
   check_region_access (src_reg, DIR_READ, ctxt);
 }
 
+/* Returns the trailing bytes on dubious allocation sizes.  */
+
+static unsigned HOST_WIDE_INT 
+capacity_compatible_with_type (tree cst, tree pointee_size_tree)
+{
+  unsigned HOST_WIDE_INT pointee_size = TREE_INT_CST_LOW (pointee_size_tree);
+  if (pointee_size == 0)
+    return 0;
+  unsigned HOST_WIDE_INT alloc_size = TREE_INT_CST_LOW (cst);
+
+  return alloc_size % pointee_size;
+}
+
+/* Visits svalues and checks whether the 
+   size_cst is a operand of the svalue.  */
+
+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()
+  {
+    /* The result_set gradually builts from atomtic nodes upwards. If a node is
+       in the result_set, itself or one/all of its children have an operand that
+       is a multiple of the size_cst. If the root is inside, the given sval 
+       is valid aka a multiple of the size_cst.*/
+    return result_set.contains(m_sval);
+  }
+
+  void 
+  visit_constant_svalue (const constant_svalue *sval) final override
+  {
+    unsigned HOST_WIDE_INT sval_int
+	  = TREE_INT_CST_LOW (sval->get_constant ());
+    unsigned HOST_WIDE_INT size_cst_int = TREE_INT_CST_LOW (m_size_cst);
+    if (size_cst_int == 0 || sval_int % size_cst_int == 0)
+      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 ();
+    arg->accept (this);
+    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 ();
+
+    arg0->accept (this);
+    arg1->accept (this);
+    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 ();
+
+    base->accept(this);
+    iter->accept(this);
+    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
+  {
+    if (m_cm->get_equiv_class_by_svalue (sval, NULL))
+      result_set.add (sval);
+  }
+
+  void visit_asm_output_svalue (const asm_output_svalue *sval ATTRIBUTE_UNUSED) 
+    final override
+  {
+    // TODO: Should we do something else than assume it could be correct
+    result_set.add (sval);
+  }
+
+  void visit_const_fn_result_svalue (const const_fn_result_svalue 
+				      *sval ATTRIBUTE_UNUSED) final override
+  {
+    // TODO: Should we do something else than assume it could be correct
+    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.  */
+};
+
+/* Returns true if there is a constant tree with 
+   the same constant value inside the sval.  */
+
+static bool
+const_operand_in_sval_p (tree type_size_cst, const svalue *sval,
+                         constraint_manager *cm)
+{
+  size_visitor v(type_size_cst, sval, cm);
+  // sval->accept(&v);
+  return v.get_result ();
+}
+
+/* Special handling for structs with "inheritance" or that hold an unbounded 
+     type. Those will be skipped to prevent false positives.  */
+
+static bool
+struct_or_union_with_inheritance_p (tree maybe_struct)
+{
+  if (RECORD_OR_UNION_TYPE_P (maybe_struct))
+    {
+      tree iter = TYPE_FIELDS (maybe_struct);
+      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 
+          && COMPLETE_OR_UNBOUND_ARRAY_TYPE_P (TREE_TYPE (last_field)))
+        return true;
+    }
+  return false;
+}
+
+void
+region_model::check_region_size (const region *lhs_reg, const svalue *rhs_sval,
+			                           region_model_context *ctxt) const
+{
+  if (!ctxt)
+    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);
+  /* void * is always compatible and make sure that the pointee_type actually
+     has a size, or else size_in_bytes might fail.  */
+  if (pointee_type == NULL_TREE || VOID_TYPE_P (pointee_type) 
+      || TYPE_SIZE_UNIT (pointee_type) == NULL_TREE)
+    return;
+  if (struct_or_union_with_inheritance_p (pointee_type))
+    return;
+
+  tree pointee_size_tree = size_in_bytes(pointee_type);
+  /* The size might be unknown e.g. being a array with n elements
+     or casting to char * never has any trailing bytes.  */
+  if (TREE_CODE (pointee_size_tree) != INTEGER_CST
+      || TREE_INT_CST_LOW (pointee_size_tree) == 1)
+    return;
+
+  const svalue *capacity = get_capacity (reg_sval->get_pointee ());
+  switch (capacity->get_kind ())
+    {
+    case svalue_kind::SK_CONSTANT:
+      {
+	const constant_svalue *cap_sval = capacity->dyn_cast_constant_svalue ();
+	tree cap = cap_sval->get_constant ();
+	unsigned HOST_WIDE_INT size_diff
+	  = capacity_compatible_with_type (cap, pointee_size_tree);
+	if (size_diff != 0)
+	  {
+	    ctxt->warn (new dubious_allocation_size (lhs_reg, reg_sval->get_pointee (), capacity));
+	  }
+      }
+      break;
+    default:
+      {
+	if (!const_operand_in_sval_p (pointee_size_tree, capacity, m_constraints))
+	  {
+	    ctxt->warn (new dubious_allocation_size (lhs_reg, reg_sval->get_pointee (), capacity));
+	  }
+      }
+      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 +3106,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,
@@ -3990,6 +4288,30 @@ region_model::pop_frame (tree result_lvalue,
   unbind_region_and_descendents (frame_reg,POISON_KIND_POPPED_STACK);
 }
 
+// FIXME: How to call the call_lhs <- <return_value> on the caller context
+//        with the call site set as a stmt?
+void
+region_model::set_return (const gcall *call, region_model_context *ctxt)
+{
+  if (0)
+    {
+      tree lhs = gimple_call_lhs (call);
+      tree fndecl = gimple_call_fndecl (call);
+      tree result = DECL_RESULT (fndecl);
+      const svalue *retval = NULL;
+      if (result && TREE_TYPE (result) != void_type_node)
+          retval = get_rvalue (result, ctxt);
+
+      if (lhs && retval)
+      {
+        /* Compute result_dst_reg using RESULT_LVALUE *after* popping
+      the frame, but before poisoning pointers into the old frame.  */
+        const region *result_dst_reg = get_lvalue (lhs, ctxt);
+        set_value (result_dst_reg, retval, ctxt);
+      }
+    }
+}
+
 /* Get the number of frames in this region_model's stack.  */
 
 int
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index 1bfa56a8cd2..5dbbf2cd546 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -668,6 +668,8 @@ class region_model
   void update_for_return_gcall (const gcall *call_stmt,
                                 region_model_context *ctxt);
 
+  void set_return (const gcall *call_stmt, region_model_context *ctxt);
+
   const region *push_frame (function *fun, const vec<const svalue *> *arg_sids,
 			    region_model_context *ctxt);
   const frame_region *get_current_frame () const { return m_current_frame; }
@@ -857,6 +859,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/testsuite/gcc.dg/analyzer/allocation-size-1.c b/gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c
new file mode 100644
index 00000000000..cb3df5516e7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c
@@ -0,0 +1,63 @@
+#include <stdlib.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 "" "" { target *-*-* } malloc2 } */
+  /* { dg-message "" "" { 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 } */
+  int *iptr = (int *)ptr; /* { dg-line assign } */
+  free (iptr);
+
+  /* { dg-warning "" "" { target *-*-* } assign } */
+  /* { dg-message "" "" { target *-*-* } assign } */
+}
+
+struct s {
+  int i;
+};
+
+void test_5 (void)
+{
+  struct s *ptr = malloc (5 * sizeof (struct s));
+  free (ptr);
+}
+
+void test_6 (void)
+{
+  long *ptr = malloc (5 * sizeof (struct s));  /* { dg-line malloc6 } */
+  free (ptr);
+
+  /* { dg-warning "" "" { target *-*-* } malloc6 } */
+  /* { dg-message "" "" { target *-*-* } malloc6 } */
+}
+
+void test_7 (void)
+{
+  char buf[2];
+  int *ptr = (int *)buf; /* { dg-line malloc7 } */
+
+  /* { dg-warning "" "" { target *-*-* } malloc7 } */
+  /* { dg-message "" "" { target *-*-* } malloc7 } */
+}
\ 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..a619a786a4e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/allocation-size-2.c
@@ -0,0 +1,44 @@
+#include <stdlib.h>
+#include <stdio.h>
+
+/* Tests with symbolic buffer sizes.  */
+
+void test_1 (void)
+{
+  int n;
+  scanf("%i", &n);
+  short *ptr = malloc (n * sizeof(short));
+  free (ptr);
+}
+
+void test_2 (void)
+{
+  int n;
+  scanf("%i", &n);
+  int *ptr = malloc (n * sizeof (short)); /* { dg-line malloc } */
+  free (ptr);
+
+  /* { dg-warning "" "" { target *-*-* } malloc } */
+  /* { dg-message "" "" { target *-*-* } malloc } */
+}
+
+void test_3 (void)
+{
+  int n;
+  scanf("%i", &n);
+  void *ptr = malloc (n * sizeof (short));
+  short *sptr = (short *)ptr;
+  free (sptr);
+}
+
+void test_4 (void)
+{
+  int n;
+  scanf("%i", &n);
+  void *ptr = malloc (n * sizeof (short)); /* { dg-message } */
+  int *iptr = (int *)ptr; /* { dg-line assign } */
+  free (iptr);
+
+  /* { dg-warning "" "" { target *-*-* } assign } */
+  /* { dg-message "" "" { target *-*-* } assign } */
+}
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..dafc0e73c63
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/allocation-size-3.c
@@ -0,0 +1,48 @@
+#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 "" "" { target *-*-* } malloc1 } */
+  /* { dg-message "" "" { target *-*-* } malloc1 } */
+}
+
+void test_2(void)
+{
+  int *ptr = malloc (10 + sizeof(int)); /* { dg-line malloc2 } */
+  free (ptr);
+
+  /* { dg-warning "" "" { target *-*-* } malloc2 } */
+  /* { dg-message "" "" { target *-*-* } malloc2 } */
+}
+
+void test_3(void)
+{
+  int n;
+  scanf("%i", &n);
+  int *ptr = malloc (n + sizeof (int)); /* { dg-line malloc3 } */
+  free (ptr);
+
+  /* { dg-warning "" "" { target *-*-* } malloc3 } */
+  /* { dg-message "" "" { target *-*-* } malloc3 } */
+}
+
+void test_4(void)
+{
+  int n;
+  scanf("%i", &n);
+  int m;
+  scanf("%i", &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..32e14bad6ec
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/allocation-size-4.c
@@ -0,0 +1,92 @@
+#include <stddef.h>
+#include <stdlib.h>
+#include <stdio.h>
+
+/* Flow warnings */
+
+void *create_buffer(int n)
+{
+  return malloc(n);
+}
+
+void test_1(void) 
+{
+  // FIXME
+  int *buf = create_buffer(42); /* { dg-warning "" "" { xfail *-*-* } } */
+  free (buf);
+}
+
+void test_2(void) 
+{
+  void *buf = create_buffer(42); /* { dg-message } */
+  int *ibuf = buf; /* { dg-line assign2 } */
+  free (ibuf);
+
+  /* { dg-warning "" "" { target *-*-* } assign2 } */
+  /* { dg-message "" "" { target *-*-* } assign2 } */
+}
+
+void test_3(void)
+{
+  void *buf = malloc(42); /* { dg-message } */
+  if (buf != NULL) /* { dg-message } */
+    {
+      int *ibuf = buf; /* { dg-line assign3 } */
+      free (ibuf);
+    }
+
+  /* { dg-warning "" "" { target *-*-* } assign3 } */
+  /* { dg-message "" "" { target *-*-* } assign3 } */
+}
+
+void test_4(void)
+{
+  int n;
+  scanf("%i", &n);
+
+  int size;
+  if (n == 0)
+    size = 1;
+  else if (n == 1)
+    size = 10;
+  else
+    size = 20;
+
+  int *buf = malloc(size); // Size should be 'unknown' at this point
+  free (buf);
+}
+
+void test_5(void)
+{
+  int n;
+  scanf("%i", &n);
+
+  int size;
+  if (n == 0)
+    size = 2;
+  else
+    size = 10;
+
+  short *buf = malloc(size); // Size should be widened to 2 and 10, both fit
+  free (buf);
+}
+
+
+void test_6(void)
+{
+  int n;
+  scanf("%i", &n);
+
+  int size;
+  if (n == 0)
+    size = 1;
+  else
+    size = 10;
+
+  short *buf = malloc(size); /* { dg-line malloc6 } */
+  free (buf);
+  
+
+  /* { dg-warning "" "" { target *-*-* } malloc6 } */
+  /* { dg-message "" "" { target *-*-* } malloc6 } */
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/attr-malloc-6.c b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-6.c
index bd28107d0d7..8fa6a6eb570 100644
--- a/gcc/testsuite/gcc.dg/analyzer/attr-malloc-6.c
+++ b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-6.c
@@ -1,7 +1,9 @@
+/* { dg-additional-options -Wno-analyzer-allocation-size } */
 /* Adapted from gcc.dg/Wmismatched-dealloc.c.  */
 
 #define A(...) __attribute__ ((malloc (__VA_ARGS__)))
 
+struct FILE;
 typedef struct FILE   FILE;
 typedef __SIZE_TYPE__ size_t;
 
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-4.c b/gcc/testsuite/gcc.dg/analyzer/malloc-4.c
index 908bb28ee50..f9a73c79403 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-4.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-4.c
@@ -1,4 +1,4 @@
-/* { dg-additional-options "-Wno-incompatible-pointer-types" } */
+/* { dg-additional-options "-Wno-incompatible-pointer-types -Wno-analyzer-allocation-size" } */
 
 #include <stdlib.h>
 
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr96639.c b/gcc/testsuite/gcc.dg/analyzer/pr96639.c
index 02ca3f084a2..6f365c3cb5d 100644
--- a/gcc/testsuite/gcc.dg/analyzer/pr96639.c
+++ b/gcc/testsuite/gcc.dg/analyzer/pr96639.c
@@ -1,3 +1,5 @@
+/* { dg-additional-options -Wno-analyzer-allocation-size } */
+
 void *calloc (__SIZE_TYPE__, __SIZE_TYPE__);
 
 int
-- 
2.36.1


  reply	other threads:[~2022-06-22 14:59 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-17 15:54 Tim Lange
2022-06-17 17:15 ` Prathamesh Kulkarni
2022-06-17 19:23   ` Tim Lange
2022-06-17 21:39     ` David Malcolm
2022-06-17 17:48 ` David Malcolm
2022-06-17 20:23   ` Tim Lange
2022-06-17 22:13     ` David Malcolm
2022-06-21 20:00       ` Tim Lange
2022-06-21 23:16         ` David Malcolm
2022-06-22 14:57           ` Tim Lange [this message]
2022-06-22 18:23             ` David Malcolm
2022-06-17 18:34 ` [RFC] analyzer: add " Tim Lange
2022-06-29 15:39 ` [PATCH v2] analyzer: add allocation size checker Tim Lange
2022-06-29 17:39   ` David Malcolm
2022-06-30 20:40     ` Tim Lange
2022-06-30 22:11 ` [PATCH v3] analyzer: add allocation size checker [PR105900] Tim Lange
2022-06-30 22:47   ` David Malcolm

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220622145755.38734-1-mail@tim-lange.me \
    --to=mail@tim-lange.me \
    --cc=dmalcolm@redhat.com \
    --cc=gcc@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).