From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2209) id B4F6639A8C11; Fri, 16 Jul 2021 19:50:05 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B4F6639A8C11 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: David Malcolm To: gcc-cvs@gcc.gnu.org Subject: [gcc r12-2376] analyzer: add region_model::check_region_access X-Act-Checkin: gcc X-Git-Author: David Malcolm X-Git-Refname: refs/heads/master X-Git-Oldrev: 9ea10c480565fa42b1804fb436f7e26ca77b71a3 X-Git-Newrev: 9faf8348621ae6ab583af593d67ac424300a2bad Message-Id: <20210716195005.B4F6639A8C11@sourceware.org> Date: Fri, 16 Jul 2021 19:50:05 +0000 (GMT) X-BeenThere: gcc-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 16 Jul 2021 19:50:05 -0000 https://gcc.gnu.org/g:9faf8348621ae6ab583af593d67ac424300a2bad commit r12-2376-g9faf8348621ae6ab583af593d67ac424300a2bad Author: David Malcolm Date: Fri Jul 16 15:49:17 2021 -0400 analyzer: add region_model::check_region_access 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 Diff: --- 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(-) 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; }