analyzer: add region_model::check_region_access
authorDavid Malcolm <dmalcolm@redhat.com>
Fri, 16 Jul 2021 19:49:17 +0000 (15:49 -0400)
committerDavid Malcolm <dmalcolm@redhat.com>
Fri, 16 Jul 2021 19:49:17 +0000 (15:49 -0400)
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.

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
gcc/analyzer/engine.cc
gcc/analyzer/program-state.cc
gcc/analyzer/region-model-impl-calls.cc
gcc/analyzer/region-model-reachability.cc
gcc/analyzer/region-model.cc
gcc/analyzer/region-model.h
gcc/analyzer/region.cc
gcc/analyzer/svalue.cc

index d42bee7..90143d9 100644 (file)
@@ -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,
index f9fc581..ee625fb 100644 (file)
@@ -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)
index 3081217..ccfe7b0 100644 (file)
@@ -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))
                {
index 545634b..eff8caa 100644 (file)
@@ -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 ());
index 1f65307..b5ae787 100644 (file)
@@ -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
index 190c852..4fab1ef 100644 (file)
@@ -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);
index f07a287..734ec60 100644 (file)
@@ -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;
index 6cccb0f..fa187fd 100644 (file)
@@ -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);
 }
 
index fa9a862..323df80 100644 (file)
@@ -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;
     }