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.
Successfully bootstrapped & regrtested on aarch64-unknown-linux-gnu
Pushed to trunk as r17-360-ga2bd06e89526a6.
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.
---
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.dg/analyzer/divide-by-zero-4.c | 3 -
.../gcc.dg/analyzer/divide-by-zero-5.c | 3 -
.../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 00ee99a6218..a45b6acd3d0 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 36ce445cf8c..cef1694b156 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 f1a9ad4b785..907a36af52b 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 549a2e00cce..ae6d44bc8b8 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 f8a09c0b819..a653eb0a6b3 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 b685ba1d91b..2585dc7c34e 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 96c9d01700d..49a91cbdeb7 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 5fd8539f109..7b50a19fe38 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
--
2.49.0
