[committed] analyzer: add region_model::check_region_access

Message ID 20210716195656.75381-1-dmalcolm@redhat.com
State New
Headers show
Series
  • [committed] analyzer: add region_model::check_region_access
Related show

Commit Message

Andrew Pinski via Gcc-patches July 16, 2021, 7:56 p.m.
I've been experimenting with various new diagnostics that
require a common place for the analyzer to check the validity
of reads or writes to memory (e.g. buffer overflow).

As preliminary work, this patch adds new
  region_model::check_region_for_{read|write} functions
which are called anywhere that the analyzer "sees" memory being
read from or written to (via region_model::get_store_value and
region_model::set_value).

This takes over the hardcoded calls to check_for_writable_region
(allowing for other kinds of checks on writes); checking reads is
currently a no-op.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as 9faf8348621ae6ab583af593d67ac424300a2bad.

gcc/analyzer/ChangeLog:
	* analyzer.h (enum access_direction): New.
	* engine.cc (exploded_node::on_longjmp): Update for new param of
	get_store_value.
	* program-state.cc (program_state::prune_for_point): Likewise.
	* region-model-impl-calls.cc (region_model::impl_call_memcpy):
	Replace call to check_for_writable_region with call to
	check_region_for_write.
	(region_model::impl_call_memset): Likewise.
	(region_model::impl_call_strcpy): Likewise.
	* region-model-reachability.cc (reachable_regions::add): Update
	for new param of get_store_value.
	* region-model.cc (region_model::get_rvalue_1): Likewise, also for
	get_rvalue_for_bits.
	(region_model::get_store_value): Add ctxt param and use it to call
	check_region_for_read.
	(region_model::get_rvalue_for_bits): Add ctxt param and use it to
	call get_store_value.
	(region_model::check_region_access): New.
	(region_model::check_region_for_write): New.
	(region_model::check_region_for_read): New.
	(region_model::set_value): Update comment.  Replace call to
	check_for_writable_region with call to check_region_for_write.
	* region-model.h (region_model::get_rvalue_for_bits): Add ctxt
	param.
	(region_model::get_store_value): Add ctxt param.
	(region_model::check_region_access): New decl.
	(region_model::check_region_for_write): New decl.
	(region_model::check_region_for_read): New decl.
	* region.cc (region_model::copy_region): Update call to
	get_store_value.
	* svalue.cc (initial_svalue::implicitly_live_p): Likewise.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>

---
 gcc/analyzer/analyzer.h                   |  8 +++
 gcc/analyzer/engine.cc                    |  3 +-
 gcc/analyzer/program-state.cc             |  2 +-
 gcc/analyzer/region-model-impl-calls.cc   |  6 +-
 gcc/analyzer/region-model-reachability.cc |  2 +-
 gcc/analyzer/region-model.cc              | 70 +++++++++++++++++++----
 gcc/analyzer/region-model.h               | 13 ++++-
 gcc/analyzer/region.cc                    |  2 +-
 gcc/analyzer/svalue.cc                    |  2 +-
 9 files changed, 88 insertions(+), 20 deletions(-)

-- 
2.26.3

Patch

diff --git a/gcc/analyzer/analyzer.h b/gcc/analyzer/analyzer.h
index d42bee7eb0d..90143d9aba2 100644
--- a/gcc/analyzer/analyzer.h
+++ b/gcc/analyzer/analyzer.h
@@ -208,6 +208,14 @@  public:
   virtual logger *get_logger () const = 0;
 };
 
+/* An enum for describing the direction of an access to memory.  */
+
+enum access_direction
+{
+  DIR_READ,
+  DIR_WRITE
+};
+
 } // namespace ana
 
 extern bool is_special_named_call_p (const gcall *call, const char *funcname,
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index f9fc58180b7..ee625fbdcdf 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -1468,7 +1468,8 @@  exploded_node::on_longjmp (exploded_graph &eg,
   const region *buf = new_region_model->deref_rvalue (buf_ptr_sval, buf_ptr,
 						       ctxt);
 
-  const svalue *buf_content_sval = new_region_model->get_store_value (buf);
+  const svalue *buf_content_sval
+    = new_region_model->get_store_value (buf, ctxt);
   const setjmp_svalue *setjmp_sval
     = buf_content_sval->dyn_cast_setjmp_svalue ();
   if (!setjmp_sval)
diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
index 30812176bd8..ccfe7b019b0 100644
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@@ -1082,7 +1082,7 @@  program_state::prune_for_point (exploded_graph &eg,
 		 temporaries keep the value reachable until the frame is
 		 popped.  */
 	      const svalue *sval
-		= new_state.m_region_model->get_store_value (reg);
+		= new_state.m_region_model->get_store_value (reg, NULL);
 	      if (!new_state.can_purge_p (eg.get_ext_state (), sval)
 		  && SSA_NAME_VAR (ssa_name))
 		{
diff --git a/gcc/analyzer/region-model-impl-calls.cc b/gcc/analyzer/region-model-impl-calls.cc
index 545634b9dc7..eff8caa8c0a 100644
--- a/gcc/analyzer/region-model-impl-calls.cc
+++ b/gcc/analyzer/region-model-impl-calls.cc
@@ -431,7 +431,7 @@  region_model::impl_call_memcpy (const call_details &cd)
 	return;
     }
 
-  check_for_writable_region (dest_reg, cd.get_ctxt ());
+  check_region_for_write (dest_reg, cd.get_ctxt ());
 
   /* Otherwise, mark region's contents as unknown.  */
   mark_region_as_unknown (dest_reg, cd.get_uncertainty ());
@@ -455,7 +455,7 @@  region_model::impl_call_memset (const call_details &cd)
   const region *sized_dest_reg = m_mgr->get_sized_region (dest_reg,
 							  NULL_TREE,
 							  num_bytes_sval);
-  check_for_writable_region (sized_dest_reg, cd.get_ctxt ());
+  check_region_for_write (sized_dest_reg, cd.get_ctxt ());
   fill_region (sized_dest_reg, fill_value_u8);
   return true;
 }
@@ -515,7 +515,7 @@  region_model::impl_call_strcpy (const call_details &cd)
 
   cd.maybe_set_lhs (dest_sval);
 
-  check_for_writable_region (dest_reg, cd.get_ctxt ());
+  check_region_for_write (dest_reg, cd.get_ctxt ());
 
   /* For now, just mark region's contents as unknown.  */
   mark_region_as_unknown (dest_reg, cd.get_uncertainty ());
diff --git a/gcc/analyzer/region-model-reachability.cc b/gcc/analyzer/region-model-reachability.cc
index 1f65307e394..b5ae787cac9 100644
--- a/gcc/analyzer/region-model-reachability.cc
+++ b/gcc/analyzer/region-model-reachability.cc
@@ -154,7 +154,7 @@  reachable_regions::add (const region *reg, bool is_mutable)
   if (binding_cluster *bind_cluster = m_store->get_cluster (base_reg))
     bind_cluster->for_each_value (handle_sval_cb, this);
   else
-    handle_sval (m_model->get_store_value (reg));
+    handle_sval (m_model->get_store_value (reg, NULL));
 }
 
 void
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 190c8524f90..4fab1ef8427 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -1743,7 +1743,7 @@  region_model::get_rvalue_1 (path_var pv, region_model_context *ctxt) const
 	gcc_assert (TREE_CODE (first_bit_offset) == INTEGER_CST);
 	bit_range bits (TREE_INT_CST_LOW (first_bit_offset),
 			TREE_INT_CST_LOW (num_bits));
-	return get_rvalue_for_bits (TREE_TYPE (expr), reg, bits);
+	return get_rvalue_for_bits (TREE_TYPE (expr), reg, bits, ctxt);
       }
 
     case SSA_NAME:
@@ -1753,7 +1753,7 @@  region_model::get_rvalue_1 (path_var pv, region_model_context *ctxt) const
     case ARRAY_REF:
       {
 	const region *reg = get_lvalue (pv, ctxt);
-	return get_store_value (reg);
+	return get_store_value (reg, ctxt);
       }
 
     case REALPART_EXPR:
@@ -1808,7 +1808,7 @@  region_model::get_rvalue_1 (path_var pv, region_model_context *ctxt) const
     case MEM_REF:
       {
 	const region *ref_reg = get_lvalue (pv, ctxt);
-	return get_store_value (ref_reg);
+	return get_store_value (ref_reg, ctxt);
       }
     }
 }
@@ -1913,11 +1913,15 @@  region_model::get_initial_value_for_global (const region *reg) const
 }
 
 /* Get a value for REG, looking it up in the store, or otherwise falling
-   back to "initial" or "unknown" values.  */
+   back to "initial" or "unknown" values.
+   Use CTXT to report any warnings associated with reading from REG. */
 
 const svalue *
-region_model::get_store_value (const region *reg) const
+region_model::get_store_value (const region *reg,
+			       region_model_context *ctxt) const
 {
+  check_region_for_read (reg, ctxt);
+
   /* Special-case: handle var_decls in the constant pool.  */
   if (const decl_region *decl_reg = reg->dyn_cast_decl_region ())
     if (const svalue *sval = decl_reg->maybe_get_constant_value (m_mgr))
@@ -2077,14 +2081,16 @@  region_model::deref_rvalue (const svalue *ptr_sval, tree ptr_tree,
 /* Attempt to get BITS within any value of REG, as TYPE.
    In particular, extract values from compound_svalues for the case
    where there's a concrete binding at BITS.
-   Return an unknown svalue if we can't handle the given case.  */
+   Return an unknown svalue if we can't handle the given case.
+   Use CTXT to report any warnings associated with reading from REG.  */
 
 const svalue *
 region_model::get_rvalue_for_bits (tree type,
 				   const region *reg,
-				   const bit_range &bits) const
+				   const bit_range &bits,
+				   region_model_context *ctxt) const
 {
-  const svalue *sval = get_store_value (reg);
+  const svalue *sval = get_store_value (reg, ctxt);
   return m_mgr->get_or_create_bits_within (type, bits, sval);
 }
 
@@ -2240,8 +2246,52 @@  region_model::get_capacity (const region *reg) const
   return m_mgr->get_or_create_unknown_svalue (sizetype);
 }
 
+/* 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.  */
+
+void
+region_model::check_region_access (const region *reg,
+				   enum access_direction dir,
+				   region_model_context *ctxt) const
+{
+  /* Fail gracefully if CTXT is NULL.  */
+  if (!ctxt)
+    return;
+
+  switch (dir)
+    {
+    default:
+      gcc_unreachable ();
+    case DIR_READ:
+      /* Currently a no-op.  */
+      break;
+    case DIR_WRITE:
+      check_for_writable_region (reg, ctxt);
+      break;
+    }
+}
+
+/* If CTXT is non-NULL, use it to warn about any problems writing to REG.  */
+
+void
+region_model::check_region_for_write (const region *dest_reg,
+				      region_model_context *ctxt) const
+{
+  check_region_access (dest_reg, DIR_WRITE, ctxt);
+}
+
+/* If CTXT is non-NULL, use it to warn about any problems reading from REG.  */
+
+void
+region_model::check_region_for_read (const region *src_reg,
+				     region_model_context *ctxt) const
+{
+  check_region_access (src_reg, DIR_READ, ctxt);
+}
+
 /* Set the value of the region given by LHS_REG to the value given
-   by RHS_SVAL.  */
+   by RHS_SVAL.
+   Use CTXT to report any warnings associated with writing to LHS_REG.  */
 
 void
 region_model::set_value (const region *lhs_reg, const svalue *rhs_sval,
@@ -2250,7 +2300,7 @@  region_model::set_value (const region *lhs_reg, const svalue *rhs_sval,
   gcc_assert (lhs_reg);
   gcc_assert (rhs_sval);
 
-  check_for_writable_region (lhs_reg, ctxt);
+  check_region_for_write (lhs_reg, ctxt);
 
   m_store.set_value (m_mgr->get_store_manager(), lhs_reg, rhs_sval,
 		     ctxt ? ctxt->get_uncertainty () : NULL);
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index f07a287f681..734ec601237 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -609,7 +609,8 @@  class region_model
 
   const svalue *get_rvalue_for_bits (tree type,
 				     const region *reg,
-				     const bit_range &bits) const;
+				     const bit_range &bits,
+				     region_model_context *ctxt) const;
 
   void set_value (const region *lhs_reg, const svalue *rhs_sval,
 		  region_model_context *ctxt);
@@ -687,7 +688,8 @@  class region_model
   static void append_ssa_names_cb (const region *base_reg,
 				   struct append_ssa_names_cb_data *data);
 
-  const svalue *get_store_value (const region *reg) const;
+  const svalue *get_store_value (const region *reg,
+				 region_model_context *ctxt) const;
 
   bool region_exists_p (const region *reg) const;
 
@@ -748,6 +750,13 @@  class region_model
 
   void check_for_writable_region (const region* dest_reg,
 				  region_model_context *ctxt) const;
+  void check_region_access (const region *reg,
+			    enum access_direction dir,
+			    region_model_context *ctxt) const;
+  void check_region_for_write (const region *dest_reg,
+			       region_model_context *ctxt) const;
+  void check_region_for_read (const region *src_reg,
+			      region_model_context *ctxt) const;
 
   /* Storing this here to avoid passing it around everywhere.  */
   region_model_manager *const m_mgr;
diff --git a/gcc/analyzer/region.cc b/gcc/analyzer/region.cc
index 6cccb0f48f2..fa187fde331 100644
--- a/gcc/analyzer/region.cc
+++ b/gcc/analyzer/region.cc
@@ -573,7 +573,7 @@  region_model::copy_region (const region *dst_reg, const region *src_reg,
   if (dst_reg == src_reg)
     return;
 
-  const svalue *sval = get_store_value (src_reg);
+  const svalue *sval = get_store_value (src_reg, ctxt);
   set_value (dst_reg, sval, ctxt);
 }
 
diff --git a/gcc/analyzer/svalue.cc b/gcc/analyzer/svalue.cc
index fa9a862bdb5..323df8015fd 100644
--- a/gcc/analyzer/svalue.cc
+++ b/gcc/analyzer/svalue.cc
@@ -936,7 +936,7 @@  initial_svalue::implicitly_live_p (const svalue_set *,
      a popped stack frame.  */
   if (model->region_exists_p (m_reg))
     {
-      const svalue *reg_sval = model->get_store_value (m_reg);
+      const svalue *reg_sval = model->get_store_value (m_reg, NULL);
       if (reg_sval == this)
 	return true;
     }