https://gcc.gnu.org/g:a2bd06e89526a69925157d0cbecb521fb2a360e0
commit r17-360-ga2bd06e89526a69925157d0cbecb521fb2a360e0 Author: David Malcolm <[email protected]> Date: Wed May 6 09:57:07 2026 -0400 analyzer: generating raw state for rewinding data flow The previous patch used the states of the exploded nodes at each end of an exploded edge for rewinding state, but this doesn't always work when state purging is active (the default) due to the state purging eliminating pertinent information. This can be see in gcc.dg/analyzer/divide-by-zero-{4,5,6}.c, which required -fno-analyzer-state-purge to rewind the data flow to locate where the zero values were coming from. This patch extends diagnostic_manager::annotate_exploded_path so that it performs an initial forward walk through the exploded path, building region_model instances along the path without any state purging or merging, and updates the rewind_context to use these states to rewind pertinent state for a diagnostic. Doing so allows us to drop -fno-analyzer-state-purge from the testcases which needed them before. gcc/analyzer/ChangeLog: * analyzer-logging.h (class text_art::canvas): New forward decl. (logger::log_canvas): New decl. * diagnostic-manager.cc (path_builder::get_supergraph): New. (logger::log_canvas): New. (dump_to_logger): New. (log_region_model): New. (epath_rewind_context::epath_rewind_context): Add params src_model and dst_model, using them to initialize m_src_model and m_dst_model. (epath_rewind_context::get_src_region_model): New. (epath_rewind_context::get_dst_region_model): New. (epath_rewind_context::m_src_model): New. (epath_rewind_context::m_dst_model): New. (make_raw_dst_region_model): New. (diagnostic_manager::annotate_exploded_path): Walk EPATH forwards, populating new vectors src_models and dst_models without state purging or merging, so that we can reliably rewind state. Pass these models to epath_rewind_context when rewinding state. * engine.cc (interprocedural_call::try_to_rewind_data_flow): Get src and dst region_model instances via vfuncs of ctxt, rather than accessing the eedge. * ops.cc (rewind_context::on_data_origin): Get src and dst region_model instances via vfuncs of ctxt, rather than accessing the eedge. (greturn_op::try_to_rewind_data_flow): Likewise. * ops.h (rewind_context::rewind_context): Drop param "eedge". (rewind_context::get_src_region_model): New vfunc. (rewind_context::get_dst_region_model): New vfunc. (rewind_context::m_eedge): Drop field. gcc/testsuite/ChangeLog: * gcc.dg/analyzer/divide-by-zero-4.c: Drop option. -fno-analyzer-state-merge. * gcc.dg/analyzer/divide-by-zero-5.c: Likewise. * gcc.dg/analyzer/divide-by-zero-6.c: Likewise. Signed-off-by: David Malcolm <[email protected]> Diff: --- gcc/analyzer/analyzer-logging.h | 4 + gcc/analyzer/diagnostic-manager.cc | 212 ++++++++++++++++++++++- gcc/analyzer/engine.cc | 17 +- gcc/analyzer/ops.cc | 20 +-- gcc/analyzer/ops.h | 13 +- gcc/testsuite/gcc.dg/analyzer/divide-by-zero-4.c | 3 - gcc/testsuite/gcc.dg/analyzer/divide-by-zero-5.c | 3 - gcc/testsuite/gcc.dg/analyzer/divide-by-zero-6.c | 3 - 8 files changed, 231 insertions(+), 44 deletions(-) diff --git a/gcc/analyzer/analyzer-logging.h b/gcc/analyzer/analyzer-logging.h index 00ee99a62182..a45b6acd3d0a 100644 --- a/gcc/analyzer/analyzer-logging.h +++ b/gcc/analyzer/analyzer-logging.h @@ -25,6 +25,8 @@ along with GCC; see the file COPYING3. If not see #include "diagnostic-core.h" +namespace text_art { class canvas; } + namespace ana { /* A logger encapsulates a logging stream: a way to send @@ -50,6 +52,8 @@ class logger ATTRIBUTE_GCC_DIAG(2, 0); void end_log_line (); + void log_canvas (const text_art::canvas &); + void enter_scope (const char *scope_name); void enter_scope (const char *scope_name, const char *fmt, va_list *ap) ATTRIBUTE_GCC_DIAG(3, 0); diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc index 36ce445cf8ce..cef1694b156c 100644 --- a/gcc/analyzer/diagnostic-manager.cc +++ b/gcc/analyzer/diagnostic-manager.cc @@ -1087,6 +1087,9 @@ public: const state_machine *get_sm () const { return m_sd.m_sm; } + const supergraph & + get_supergraph () const { return m_eg.get_supergraph (); } + private: typedef reachability<eg_traits> enode_reachability; @@ -1648,18 +1651,84 @@ diagnostic_manager::get_logical_location_manager () const return *mgr; } +/* Dump C to this logger, indenting each line by the current + indentation level. */ + +void +logger::log_canvas (const text_art::canvas &c) +{ + std::string per_line_prefix (m_indent_level, ' '); + c.print_to_pp (get_printer (), per_line_prefix.c_str ()); + pp_flush (get_printer ()); +} + +/* Dump OBJ to LOGGER, using OBJ's make_dump_widget member function. */ + +template <typename T> +void +dump_to_logger (const T &obj, + logger *logger, + const char *label) +{ + if (!logger) + return; + text_art::theme *theme = global_dc->get_diagram_theme (); + if (!theme) + return; + + logger->log ("%s:", label); + logger->inc_indent (); + + text_art::style_manager sm; + text_art::style tree_style (text_art::get_style_from_color_cap_name ("note")); + + text_art::style::id_t tree_style_id (sm.get_or_create_id (tree_style)); + + text_art::dump_widget_info dwi (sm, *theme, tree_style_id); + if (auto w = obj.make_dump_widget (dwi)) + { + text_art::canvas c (w->to_canvas (dwi.m_sm)); + logger->log_canvas (c); + } + + logger->dec_indent (); +} + +static void +log_region_model (logger *logger, + const char *label, + const region_model &model) +{ + dump_to_logger<region_model> (model, logger, label); +} + class epath_rewind_context : public rewind_context { public: - epath_rewind_context (const exploded_edge &eedge, - logger *logger, + epath_rewind_context (logger *logger, diagnostic_state input_state, state_transition *&last_state_transition, - exploded_path::element_t &epath_element) - : rewind_context (eedge, logger, input_state), + exploded_path::element_t &epath_element, + const region_model &src_model, + const region_model &dst_model) + : rewind_context (logger, input_state), m_last_state_transition (last_state_transition), - m_epath_element (epath_element) + m_epath_element (epath_element), + m_src_model (src_model), + m_dst_model (dst_model) + { + } + + const region_model & + get_src_region_model () const final override { + return m_src_model; + } + + const region_model & + get_dst_region_model () const final override + { + return m_dst_model; } bool @@ -1700,8 +1769,109 @@ public: private: state_transition *&m_last_state_transition; exploded_path::element_t &m_epath_element; + const region_model &m_src_model; + const region_model &m_dst_model; }; +/* Return a region_model for the state after any operation/custom_edge_info + on EEDGE, but without state purging or state merging. Use SRC_MODEL as + the source state. + + We do this to make it easier to rewind state transitions in + diagnostic_manager::annotate_exploded_path. */ + +static region_model +make_raw_dst_region_model (logger *logger, + const exploded_edge *eedge, + const region_model &src_model, + const supergraph &sg) +{ + LOG_SCOPE (logger); + + region_model_context *const ctxt = nullptr; + + if (logger) + { + log_region_model (logger, "src_model", src_model); + log_region_model (logger, "dst_model", + *eedge->m_dest->get_state ().m_region_model); + } + + if (eedge->m_custom_info) + { + if (logger) + { + logger->start_log_line (); + eedge->m_custom_info->print (logger->get_printer ()); + logger->end_log_line (); + } + region_model new_model (src_model); + eedge->m_custom_info->update_model (&new_model, eedge, ctxt); + if (logger) + log_region_model (logger, "new model after custom_info", new_model); + return new_model; + } + else + { + const superedge *sedge = eedge->m_sedge; + if (sedge) + { + if (logger) + { + label_text desc (sedge->get_description (false)); + logger->log (" sedge: SN:%i -> SN:%i %s", + sedge->m_src->m_id, + sedge->m_dest->m_id, + desc.get ()); + } + + if (auto op = sedge->get_op ()) + { + if (logger) + { + logger->start_log_line (); + op->print_as_edge_label (logger->get_printer (), false); + logger->end_log_line (); + } + feasibility_state fs (src_model, sg); + op->execute_for_feasibility (*eedge, + fs, + ctxt, + nullptr); + log_region_model (logger, "after operation, fs model", + fs.get_model ()); + return fs.get_model (); + } + else + { + if (logger) + logger->log ("null operation, using src_model"); + return src_model; + } + } + else + { + /* Special-case the initial eedge from the origin node to the + initial function by pushing a frame for it. */ + if (eedge->m_src->m_index == 0) + { + function *fun = eedge->m_dest->get_function (); + gcc_assert (fun); + region_model new_model (src_model); + new_model.push_frame (*fun, nullptr, nullptr, ctxt); + if (logger) + { + logger->log (" pushing frame for %qD", fun->decl); + log_region_model (logger, "new model", new_model); + } + return new_model; + } + } + } + + return src_model; +} + /* Populate the elements of EPATH with diagnostic_state and state_transition information pertinent to the pending diagnostic. */ @@ -1724,7 +1894,32 @@ diagnostic_manager::annotate_exploded_path (const path_builder &pb, if (interest.m_read_regions.size () > 0) curr_state = interest.m_read_regions[0]; - // Walk epath backwards, propagating annotation information + /* Walk EPATH forwards, generating region_model instances for the elements + of EPATH without state purging or merging, so that we can reliably + rewind state. */ + std::vector<region_model> src_models; + std::vector<region_model> dst_models; + for (int idx = 0; idx < epath.m_elements.size (); ++idx) + { + auto eedge = epath.m_elements[idx].m_eedge; + if (logger) + logger->log ("edge[%i]: considering EN %i -> EN %i", + idx, + eedge->m_src->m_index, + eedge->m_dest->m_index); + region_model src_model (pb.get_ext_state ().get_model_manager ()); + if (idx > 0) + src_model = dst_models[idx - 1]; + src_models.push_back (src_model); + dst_models.push_back + (make_raw_dst_region_model (logger, + eedge, + src_model, + pb.get_supergraph ())); + } + + /* Walk EPATH backwards, using the region_models we just built, + propagating annotation information backwards. */ for (int idx = epath.m_elements.size () - 1; idx >= 0; --idx) { exploded_path::element_t &iter_element = epath.m_elements[idx]; @@ -1743,8 +1938,9 @@ diagnostic_manager::annotate_exploded_path (const path_builder &pb, const exploded_edge *eedge = iter_element.m_eedge; gcc_assert (eedge); - epath_rewind_context ctxt (*eedge, logger, curr_state, - last_state_transition, iter_element); + epath_rewind_context ctxt (logger, curr_state, + last_state_transition, iter_element, + src_models[idx], dst_models[idx]); if (eedge->m_custom_info) { if (logger) diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc index f1a9ad4b7858..907a36af52be 100644 --- a/gcc/analyzer/engine.cc +++ b/gcc/analyzer/engine.cc @@ -1694,11 +1694,10 @@ interprocedural_call::try_to_rewind_data_flow (rewind_context &ctxt) const // Rewind from params to arguments if (ctxt.m_input.m_region_holding_value) { - const region_model *dst_enode_model - = ctxt.m_eedge.m_dest->get_state ().m_region_model; + const region_model &dst_enode_model = ctxt.get_dst_region_model (); tree dst_tree - = dst_enode_model->get_representative_tree - (ctxt.m_input.m_region_holding_value); + = dst_enode_model.get_representative_tree + (ctxt.m_input.m_region_holding_value); if (dst_tree) { callsite_expr expr; @@ -1708,10 +1707,9 @@ interprocedural_call::try_to_rewind_data_flow (rewind_context &ctxt) const &expr); if (src_tree) { - const region_model *src_enode_model - = ctxt.m_eedge.m_src->get_state ().m_region_model; + const region_model &src_enode_model = ctxt.get_src_region_model (); ctxt.m_output.m_region_holding_value - = src_enode_model->get_lvalue (src_tree, nullptr); + = src_enode_model.get_lvalue (src_tree, nullptr); ctxt.add_state_transition (std::make_unique<state_transition_at_call> (expr)); @@ -1796,9 +1794,8 @@ interprocedural_return::try_to_rewind_data_flow (rewind_context &ctxt) const if (!lhs) return true; - const region_model *src_enode_model - = ctxt.m_eedge.m_src->get_state ().m_region_model; - tree fndecl = src_enode_model->get_current_function ()->decl; + const region_model &src_enode_model = ctxt.get_src_region_model (); + tree fndecl = src_enode_model.get_current_function ()->decl; tree fn_result = DECL_RESULT (fndecl); ctxt.on_data_flow (DECL_RESULT (fndecl), lhs); diff --git a/gcc/analyzer/ops.cc b/gcc/analyzer/ops.cc index 549a2e00cced..ae6d44bc8b83 100644 --- a/gcc/analyzer/ops.cc +++ b/gcc/analyzer/ops.cc @@ -217,10 +217,9 @@ void rewind_context::on_data_origin (tree dst_tree) { gcc_assert (dst_tree); - const region_model *dst_enode_model - = m_eedge.m_dest->get_state ().m_region_model; + const region_model &dst_enode_model = get_dst_region_model (); const region *dst_reg_in_dst_enode - = dst_enode_model->get_lvalue (dst_tree, nullptr); + = dst_enode_model.get_lvalue (dst_tree, nullptr); if (m_input.m_region_holding_value == dst_reg_in_dst_enode) { if (m_logger) @@ -236,18 +235,16 @@ rewind_context::on_data_flow (tree src_tree, tree dst_tree) { gcc_assert (src_tree); gcc_assert (dst_tree); - const region_model *dst_enode_model - = m_eedge.m_dest->get_state ().m_region_model; + const region_model &dst_enode_model = get_dst_region_model (); const region *dst_reg_in_dst_enode - = dst_enode_model->get_lvalue (dst_tree, nullptr); + = dst_enode_model.get_lvalue (dst_tree, nullptr); if (m_input.m_region_holding_value == dst_reg_in_dst_enode) { if (m_logger) m_logger->log ("rewinding from %qE to %qE", dst_tree, src_tree); - const region_model *src_enode_model - = m_eedge.m_src->get_state ().m_region_model; + const region_model &src_enode_model = get_src_region_model (); const region *src_reg_in_src_enode - = src_enode_model->get_lvalue (src_tree, nullptr); + = src_enode_model.get_lvalue (src_tree, nullptr); m_output.m_region_holding_value = src_reg_in_src_enode; if (TREE_CODE (src_tree) == RESULT_DECL) @@ -864,9 +861,8 @@ greturn_op::try_to_rewind_data_flow (rewind_context &ctxt) const if (get_retval ()) { - const region_model *src_enode_model - = ctxt.m_eedge.m_src->get_state ().m_region_model; - tree fndecl = src_enode_model->get_current_function ()->decl; + const region_model &src_enode_model = ctxt.get_src_region_model (); + tree fndecl = src_enode_model.get_current_function ()->decl; ctxt.on_data_flow (get_retval (), DECL_RESULT (fndecl)); } diff --git a/gcc/analyzer/ops.h b/gcc/analyzer/ops.h index f8a09c0b8194..a653eb0a6b31 100644 --- a/gcc/analyzer/ops.h +++ b/gcc/analyzer/ops.h @@ -76,11 +76,9 @@ struct operation_context struct rewind_context { - rewind_context (const exploded_edge &eedge, - logger *logger, + rewind_context (logger *logger, diagnostic_state input_state) - : m_eedge (eedge), - m_logger (logger), + : m_logger (logger), m_input (input_state), m_output (input_state) { @@ -92,13 +90,18 @@ struct rewind_context void on_data_flow (tree src, tree dst); + virtual const region_model & + get_src_region_model () const = 0; + + virtual const region_model & + get_dst_region_model () const = 0; + virtual bool could_be_affected_by_write_p (tree lhs) = 0; virtual void add_state_transition (std::unique_ptr<state_transition>) = 0; - const exploded_edge &m_eedge; logger *m_logger; diagnostic_state m_input; diagnostic_state m_output; diff --git a/gcc/testsuite/gcc.dg/analyzer/divide-by-zero-4.c b/gcc/testsuite/gcc.dg/analyzer/divide-by-zero-4.c index b685ba1d91b1..2585dc7c34e2 100644 --- a/gcc/testsuite/gcc.dg/analyzer/divide-by-zero-4.c +++ b/gcc/testsuite/gcc.dg/analyzer/divide-by-zero-4.c @@ -1,8 +1,5 @@ /* { dg-additional-options "-fno-analyzer-state-merge" } */ -/* TODO: we shouldn't need this: */ -/* { dg-additional-options "-fno-analyzer-state-purge" } */ - int get_zero (void) { diff --git a/gcc/testsuite/gcc.dg/analyzer/divide-by-zero-5.c b/gcc/testsuite/gcc.dg/analyzer/divide-by-zero-5.c index 96c9d01700d8..49a91cbdeb7c 100644 --- a/gcc/testsuite/gcc.dg/analyzer/divide-by-zero-5.c +++ b/gcc/testsuite/gcc.dg/analyzer/divide-by-zero-5.c @@ -1,8 +1,5 @@ /* { dg-additional-options "-fno-analyzer-state-merge" } */ -/* TODO: we shouldn't need this: */ -/* { dg-additional-options "-fno-analyzer-state-purge" } */ - int maybe_get_zero (int flag) { diff --git a/gcc/testsuite/gcc.dg/analyzer/divide-by-zero-6.c b/gcc/testsuite/gcc.dg/analyzer/divide-by-zero-6.c index 5fd8539f109c..7b50a19fe38e 100644 --- a/gcc/testsuite/gcc.dg/analyzer/divide-by-zero-6.c +++ b/gcc/testsuite/gcc.dg/analyzer/divide-by-zero-6.c @@ -1,8 +1,5 @@ /* { dg-additional-options "-fno-analyzer-state-merge" } */ -/* TODO: we shouldn't need this: */ -/* { dg-additional-options "-fno-analyzer-state-purge" } */ - struct foo { int x; int y; }; void
