Re: [PATCH 2/2 v2] analyzer: strcpy semantics

2022-09-04 Thread David Malcolm via Gcc-patches
On Sun, 2022-09-04 at 21:17 +0200, Tim Lange wrote:
> Hi Dave,
> 
> sorry about the strncpy thing, I should've been more careful. Below
> is the
> patch with just the strcpy part.

Thanks - this patch looks OK for trunk.

Dave



[PATCH 2/2 v2] analyzer: strcpy semantics

2022-09-04 Thread Tim Lange
Hi Dave,

sorry about the strncpy thing, I should've been more careful. Below is the
patch with just the strcpy part.

- Tim

This patch adds modelling for the semantics of strcpy in the simple case
where the analyzer is able to infer a concrete string size.

Regrtested on Linux x86_64.

2022-09-04  Tim Lange  

gcc/analyzer/ChangeLog:

* region-model-impl-calls.cc (region_model::impl_call_strcpy):
Handle the constant string case.
* region-model.cc (region_model::get_string_size):
New function to get the string size from a region or svalue.
* region-model.h (class region_model): Add get_string_size.

gcc/testsuite/ChangeLog:

* gcc.dg/analyzer/out-of-bounds-4.c: New test.
* gcc.dg/analyzer/strcpy-3.c: New test.

---
 gcc/analyzer/region-model-impl-calls.cc   | 16 -
 gcc/analyzer/region-model.cc  | 29 +
 gcc/analyzer/region-model.h   |  3 +
 .../gcc.dg/analyzer/out-of-bounds-4.c | 65 +++
 gcc/testsuite/gcc.dg/analyzer/strcpy-3.c  | 23 +++
 5 files changed, 133 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/out-of-bounds-4.c
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/strcpy-3.c

diff --git a/gcc/analyzer/region-model-impl-calls.cc 
b/gcc/analyzer/region-model-impl-calls.cc
index 8eebd122d42..3790eaf2d79 100644
--- a/gcc/analyzer/region-model-impl-calls.cc
+++ b/gcc/analyzer/region-model-impl-calls.cc
@@ -1019,13 +1019,23 @@ region_model::impl_call_strcpy (const call_details )
   const svalue *dest_sval = cd.get_arg_svalue (0);
   const region *dest_reg = deref_rvalue (dest_sval, cd.get_arg_tree (0),
 cd.get_ctxt ());
+  const svalue *src_sval = cd.get_arg_svalue (1);
+  const region *src_reg = deref_rvalue (src_sval, cd.get_arg_tree (1),
+   cd.get_ctxt ());
+  const svalue *src_contents_sval = get_store_value (src_reg,
+cd.get_ctxt ());
 
   cd.maybe_set_lhs (dest_sval);
 
-  check_region_for_write (dest_reg, cd.get_ctxt ());
+  /* Try to get the string size if SRC_REG is a string_region.  */
+  const svalue *copied_bytes_sval = get_string_size (src_reg);
+  /* Otherwise, check if the contents of SRC_REG is a string.  */
+  if (copied_bytes_sval->get_kind () == SK_UNKNOWN)
+copied_bytes_sval = get_string_size (src_contents_sval);
 
-  /* For now, just mark region's contents as unknown.  */
-  mark_region_as_unknown (dest_reg, cd.get_uncertainty ());
+  const region *sized_dest_reg
+= m_mgr->get_sized_region (dest_reg, NULL_TREE, copied_bytes_sval);
+  set_value (sized_dest_reg, src_contents_sval, cd.get_ctxt ());
 }
 
 /* Handle the on_call_pre part of "strlen".  */
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index ec29be259b5..4ec18c86774 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -3218,6 +3218,35 @@ region_model::get_capacity (const region *reg) const
   return m_mgr->get_or_create_unknown_svalue (sizetype);
 }
 
+/* Return the string size, including the 0-terminator, if SVAL is a
+   constant_svalue holding a string.  Otherwise, return an unknown_svalue.  */
+
+const svalue *
+region_model::get_string_size (const svalue *sval) const
+{
+  tree cst = sval->maybe_get_constant ();
+  if (!cst || TREE_CODE (cst) != STRING_CST)
+return m_mgr->get_or_create_unknown_svalue (size_type_node);
+
+  tree out = build_int_cst (size_type_node, TREE_STRING_LENGTH (cst));
+  return m_mgr->get_or_create_constant_svalue (out);
+}
+
+/* Return the string size, including the 0-terminator, if REG is a
+   string_region.  Otherwise, return an unknown_svalue.  */
+
+const svalue *
+region_model::get_string_size (const region *reg) const
+{
+  const string_region *str_reg = dyn_cast  (reg);
+  if (!str_reg)
+return m_mgr->get_or_create_unknown_svalue (size_type_node);
+
+  tree cst = str_reg->get_string_cst ();
+  tree out = build_int_cst (size_type_node, TREE_STRING_LENGTH (cst));
+  return m_mgr->get_or_create_constant_svalue (out);
+}
+
 /* If CTXT is non-NULL, use it to warn about any problems accessing REG,
using DIR to determine if this access is a read or write.  */
 
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index 7ce832f6ce4..a1f2165e145 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -793,6 +793,9 @@ class region_model
 
   const svalue *get_capacity (const region *reg) const;
 
+  const svalue *get_string_size (const svalue *sval) const;
+  const svalue *get_string_size (const region *reg) const;
+
   /* Implemented in sm-malloc.cc  */
   void on_realloc_with_move (const call_details ,
 const svalue *old_ptr_sval,
diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-4.c 
b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-4.c
new file mode 100644