From 0bcc0c1b2bd927a50d16acab5ff7ca7bcfcd55f9 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Wed, 29 Apr 2020 17:05:56 -0400 Subject: [PATCH 091/179] FIXME: WIP on state-change events --- gcc/analyzer/checker-path.h | 9 ++ gcc/analyzer/diagnostic-manager.cc | 231 +++++++++++++++++++++++++++-- gcc/analyzer/program-state.h | 25 ++++ 3 files changed, 256 insertions(+), 9 deletions(-) diff --git a/gcc/analyzer/checker-path.h b/gcc/analyzer/checker-path.h index 2eead25f058..8dcc5246dfa 100644 --- a/gcc/analyzer/checker-path.h +++ b/gcc/analyzer/checker-path.h @@ -21,6 +21,8 @@ along with GCC; see the file COPYING3. If not see #ifndef GCC_ANALYZER_CHECKER_PATH_H #define GCC_ANALYZER_CHECKER_PATH_H +#include "region-model2.h" // FIXME + namespace ana { /* An enum for discriminating between the concrete subclasses of @@ -203,9 +205,16 @@ public: region_id get_lvalue (tree expr, region_model_context *ctxt) const { + gcc_assert (m_dst_state.m_region_model); return m_dst_state.m_region_model->get_lvalue (expr, ctxt); } + region2* get_lvalue2 (tree expr, region_model2_context *ctxt) const + { + gcc_assert (m_dst_state.m_region_model2); + return m_dst_state.m_region_model2->get_lvalue (expr, ctxt); + } + const supernode *m_node; const gimple *m_stmt; const state_machine &m_sm; diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc index 7b3c435182e..4fe4b1a3680 100644 --- a/gcc/analyzer/diagnostic-manager.cc +++ b/gcc/analyzer/diagnostic-manager.cc @@ -813,6 +813,196 @@ for_each_state_change (const program_state &src_state, return false; } +/* Subclass of state_change_visitor that creates state_change_event + instances. */ + +class state_change_event_creator2 : public state_change_visitor2 +{ +public: + state_change_event_creator2 (const exploded_edge &eedge, + checker_path *emission_path) + : m_eedge (eedge), + m_emission_path (emission_path) + {} + + bool on_global_state_change (const state_machine &sm, + state_machine::state_t src_sm_val, + state_machine::state_t dst_sm_val) + FINAL OVERRIDE + { + const exploded_node *src_node = m_eedge.m_src; + const program_point &src_point = src_node->get_point (); + const int src_stack_depth = src_point.get_stack_depth (); + const exploded_node *dst_node = m_eedge.m_dest; + const gimple *stmt = src_point.get_stmt (); + const supernode *supernode = src_point.get_supernode (); + const program_state &dst_state = dst_node->get_state (); + + int stack_depth = src_stack_depth; + + m_emission_path->add_event (new state_change_event (supernode, + stmt, + stack_depth, + sm, + NULL_TREE, + src_sm_val, + dst_sm_val, + NULL_TREE, + dst_state)); + return false; + } + + bool on_state_change (const state_machine &sm, + state_machine::state_t src_sm_val, + state_machine::state_t dst_sm_val, + /* tree dst_rep, */ + svalue2 *dst_origin_sval) FINAL OVERRIDE + { + const exploded_node *src_node = m_eedge.m_src; + const program_point &src_point = src_node->get_point (); + const int src_stack_depth = src_point.get_stack_depth (); + const exploded_node *dst_node = m_eedge.m_dest; + const gimple *stmt = src_point.get_stmt (); + const supernode *supernode = src_point.get_supernode (); + const program_state &dst_state = dst_node->get_state (); + + int stack_depth = src_stack_depth; + + if (m_eedge.m_sedge + && m_eedge.m_sedge->m_kind == SUPEREDGE_CFG_EDGE) + { + supernode = src_point.get_supernode (); + stmt = supernode->get_last_stmt (); + stack_depth = src_stack_depth; + } + + /* Bulletproofing for state changes at calls/returns; + TODO: is there a better way? */ + if (!stmt) + return false; + + tree origin_rep +#if 0 + = dst_state.get_representative_tree (dst_origin_sid); +#else + = NULL_TREE; // FIXME +#endif +#if 0 + if (origin_rep == NULL_TREE) + origin_rep = get_any_origin (stmt, dst_rep, dst_state); +#endif + m_emission_path->add_event (new state_change_event (supernode, + stmt, + stack_depth, + sm, + NULL_TREE, /* dst_rep FIXME */ + src_sm_val, + dst_sm_val, + origin_rep, + dst_state)); + return false; + } + + const exploded_edge &m_eedge; + checker_path *m_emission_path; +}; + +/* Compare SRC_STATE and DST_STATE (which use EXT_STATE), and call + VISITOR's on_state_change for every sm-state change that occurs + to a tree, and on_global_state_change for every global state change + that occurs. + + This determines the state changes that ought to be reported to + the user: a combination of the effects of changes to sm_state_map + (which maps svalues to sm-states), and of region_model changes + (which map trees to svalues). + + Bail out early and return true if any call to on_global_state_change + or on_state_change returns true, otherwise return false. + + This is split out to make it easier to experiment with changes to + exploded_node granularity (so that we can observe what state changes + lead to state_change_events being emitted). */ + +bool +for_each_state_change2 (const program_state &src_state, + const program_state &dst_state, + const extrinsic_state &ext_state, + state_change_visitor2 *visitor) +{ + gcc_assert (src_state.m_checker_states2.length () + == ext_state.get_num_checkers ()); + gcc_assert (dst_state.m_checker_states2.length () + == ext_state.get_num_checkers ()); + for (unsigned i = 0; i < ext_state.get_num_checkers (); i++) + { + const state_machine &sm = ext_state.get_sm (i); + const sm_state_map2 &src_smap = *src_state.m_checker_states2[i]; + const sm_state_map2 &dst_smap = *dst_state.m_checker_states2[i]; + + /* Add events for any global state changes. */ + if (src_smap.get_global_state () != dst_smap.get_global_state ()) + if (visitor->on_global_state_change (sm, + src_smap.get_global_state (), + dst_smap.get_global_state ())) + return true; + + /* Add events for per-svalue state changes. */ + for (sm_state_map2::iterator_t iter = dst_smap.begin (); + iter != dst_smap.end (); + ++iter) + { +#if 1 + + // FIXME: we can compare them now + svalue2 *sval = (*iter).first; + state_machine::state_t dst_sm_val = (*iter).second.m_state; + state_machine::state_t src_sm_val = src_smap.get_state (sval); + if (dst_sm_val != src_sm_val) + { + svalue2 *origin_sval = (*iter).second.m_origin; + if (visitor->on_state_change (sm, src_sm_val, dst_sm_val, + /*dst_rep, */origin_sval)) + return true; + } +#else + /* Ideally we'd directly compare the SM state between src state + and dst state, but there's no guarantee that the IDs can + be meaningfully compared. */ + auto_vec dst_pvs; + dst_state.m_region_model2->get_path_vars_for_svalue (dst_sval, + &dst_pvs); + + unsigned j; + path_var *dst_pv; + FOR_EACH_VEC_ELT (dst_pvs, j, dst_pv) + { + tree dst_rep = dst_pv->m_tree; + gcc_assert (dst_rep); + if (dst_pv->m_stack_depth + >= src_state.m_region_model->get_stack_depth ()) + continue; + tentative_region_model_context ctxt; + svalue2 *src_sval + = src_state.m_region_model2->get_rvalue (*dst_pv, &ctxt); + if (src_val == NULL || ctxt.had_errors_p ()) + continue; + state_machine::state_t src_sm_val = src_smap.get_state (src_sval); + if (dst_sm_val != src_sm_val) + { + svalue2 *dst_origin_sval = (*iter).second.m_origin; + if (visitor->on_state_change (sm, src_sm_val, dst_sm_val, + dst_rep, dst_origin_sval)) + return true; + } + } +#endif + } + } + return false; +} + + /* Subroutine of diagnostic_manager::build_emission_path. Add any events for EEDGE to EMISSION_PATH. */ @@ -860,6 +1050,10 @@ diagnostic_manager::add_events_for_eedge (const path_builder &pb, for_each_state_change (src_state, dst_state, pb.get_ext_state (), &visitor); + state_change_event_creator2 visitor2 (eedge, emission_path); + for_each_state_change2 (src_state, dst_state, pb.get_ext_state (), + &visitor2); + /* Allow non-standard edges to add events, e.g. when rewinding from longjmp to a setjmp. */ if (eedge.m_custom_info) @@ -1204,14 +1398,33 @@ diagnostic_manager::prune_for_sm_diagnostic (checker_path *path, case EK_STATE_CHANGE: { state_change_event *state_change = (state_change_event *)base_event; - /* Use region IDs to compare var with the state_change's m_var, - bulletproofing against cases where they can't have lvalues by - using tentative_region_model_context. */ - tentative_region_model_context ctxt; - region_id state_var_rid - = state_change->get_lvalue (state_change->m_var, &ctxt); - region_id var_rid = state_change->get_lvalue (var, &ctxt); - if (state_var_rid == var_rid && !ctxt.had_errors_p ()) + bool same_region = false; + bool ctxt_had_errors = false; + if (state_change->m_dst_state.m_region_model2) + { + /* Use regions to compare var with the state_change's m_var, + bulletproofing against cases where they can't have lvalues by + using tentative_region_model_context. */ + tentative_region_model2_context ctxt; + region2 *state_var_reg + = state_change->get_lvalue2 (state_change->m_var, &ctxt); + region2 *var_reg = state_change->get_lvalue2 (var, &ctxt); + ctxt_had_errors = ctxt.had_errors_p (); + same_region = (state_var_reg == var_reg && !ctxt.had_errors_p ()); + } + else + { + /* Use region IDs to compare var with the state_change's m_var, + bulletproofing against cases where they can't have lvalues by + using tentative_region_model_context. */ + tentative_region_model_context ctxt; + region_id state_var_rid + = state_change->get_lvalue (state_change->m_var, &ctxt); + region_id var_rid = state_change->get_lvalue (var, &ctxt); + ctxt_had_errors = ctxt.had_errors_p (); + same_region = (state_var_rid == var_rid && !ctxt.had_errors_p ()); + } + if (same_region) { if (state_change->m_origin) { @@ -1234,7 +1447,7 @@ diagnostic_manager::prune_for_sm_diagnostic (checker_path *path, else log ("filtering event %i: state change to %qE", idx, state_change->m_var); - if (ctxt.had_errors_p ()) + if (ctxt_had_errors) log ("context had errors"); path->delete_event (idx); } diff --git a/gcc/analyzer/program-state.h b/gcc/analyzer/program-state.h index 2a1e9ee6914..cbe57e0f21d 100644 --- a/gcc/analyzer/program-state.h +++ b/gcc/analyzer/program-state.h @@ -443,6 +443,31 @@ extern bool for_each_state_change (const program_state &src_state, const extrinsic_state &ext_state, state_change_visitor *visitor); +/* An abstract base class for use with for_each_state_change2. */ + +class state_change_visitor2 +{ +public: + virtual ~state_change_visitor2 () {} + + /* Return true for early exit, false to keep iterating. */ + virtual bool on_global_state_change (const state_machine &sm, + state_machine::state_t src_sm_val, + state_machine::state_t dst_sm_val) = 0; + + /* Return true for early exit, false to keep iterating. */ + virtual bool on_state_change (const state_machine &sm, + state_machine::state_t src_sm_val, + state_machine::state_t dst_sm_val, + /*tree dst_rep, */ + svalue2 *dst_origin_sval) = 0; +}; + +extern bool for_each_state_change2 (const program_state &src_state, + const program_state &dst_state, + const extrinsic_state &ext_state, + state_change_visitor2 *visitor); + /* A class for recording "interesting" state changes. This is used for annotating edges in the GraphViz output of the exploded_graph, and for recording sm-state-changes, so that -- 2.21.0