[gcc(refs/users/guojiufu/heads/guojiufu-branch)] analyzer: fix ICEs in region_model::get_lvalue_1 [PR 93388]
Jiu Fu Guo
guojiufu@gcc.gnu.org
Wed Mar 11 02:02:06 GMT 2020
https://gcc.gnu.org/g:f76a88ebf089871dcce215aa0cb1956ccc060895
commit f76a88ebf089871dcce215aa0cb1956ccc060895
Author: David Malcolm <dmalcolm@redhat.com>
Date: Thu Feb 13 21:17:11 2020 -0500
analyzer: fix ICEs in region_model::get_lvalue_1 [PR 93388]
There have been various ICEs with -fanalyzer involving unhandled tree
codes in region_model::get_lvalue_1; PR analyzer/93388 reports various
others e.g. for IMAGPART_EXPR, REALPART_EXPR, and VIEW_CONVERT_EXPR seen
when running the testsuite with -fanalyzer forcibly enabled.
Whilst we could implement lvalue-handling in the region model for every
tree code, for some of these we're straying far from my primary goal for
GCC 10 of implementing a double-free checker for C.
This patch implements a fallback for unimplemented tree codes: create a
dummy region, but mark the new state as being invalid, and stop
exploring state along this path. It also implements VIEW_CONVERT_EXPR.
Doing so fixes the ICEs, whilst effectively turning off the analyzer
along code paths that use such tree codes. Hopefully this compromise
is sensible for GCC 10.
gcc/analyzer/ChangeLog:
PR analyzer/93388
* engine.cc (impl_region_model_context::on_unknown_tree_code):
New.
(exploded_graph::get_or_create_node): Reject invalid states.
* exploded-graph.h
(impl_region_model_context::on_unknown_tree_code): New decl.
(point_and_state::point_and_state): Assert that the state is
valid.
* program-state.cc (program_state::program_state): Initialize
m_valid to true.
(program_state::operator=): Copy m_valid.
(program_state::program_state): Likewise for move constructor.
(program_state::print): Print m_valid.
(program_state::dump_to_pp): Likewise.
* program-state.h (program_state::m_valid): New field.
* region-model.cc (region_model::get_lvalue_1): Implement the
default case by returning a new symbolic region and calling
the context's on_unknown_tree_code, rather than issuing an
internal_error. Implement VIEW_CONVERT_EXPR.
* region-model.h (region_model_context::on_unknown_tree_code): New
vfunc.
(test_region_model_context::on_unknown_tree_code): New.
gcc/testsuite/ChangeLog:
PR analyzer/93388
* gcc.dg/analyzer/torture/20060625-1.c: New test.
* gcc.dg/analyzer/torture/pr51628-30.c: New test.
* gcc.dg/analyzer/torture/pr59037.c: New test.
Diff:
---
gcc/analyzer/ChangeLog | 25 +++++++++++++++++++
gcc/analyzer/engine.cc | 28 ++++++++++++++++++++++
gcc/analyzer/exploded-graph.h | 6 +++++
gcc/analyzer/program-state.cc | 21 ++++++++++++++--
gcc/analyzer/program-state.h | 5 ++++
gcc/analyzer/region-model.cc | 23 +++++++++++++++---
gcc/analyzer/region-model.h | 12 ++++++++++
gcc/testsuite/ChangeLog | 7 ++++++
gcc/testsuite/gcc.dg/analyzer/torture/20060625-1.c | 1 +
gcc/testsuite/gcc.dg/analyzer/torture/pr51628-30.c | 3 +++
gcc/testsuite/gcc.dg/analyzer/torture/pr59037.c | 1 +
11 files changed, 127 insertions(+), 5 deletions(-)
diff --git a/gcc/analyzer/ChangeLog b/gcc/analyzer/ChangeLog
index 5945abc04ee..d669c989ac1 100644
--- a/gcc/analyzer/ChangeLog
+++ b/gcc/analyzer/ChangeLog
@@ -1,3 +1,28 @@
+2020-02-17 David Malcolm <dmalcolm@redhat.com>
+
+ PR analyzer/93388
+ * engine.cc (impl_region_model_context::on_unknown_tree_code):
+ New.
+ (exploded_graph::get_or_create_node): Reject invalid states.
+ * exploded-graph.h
+ (impl_region_model_context::on_unknown_tree_code): New decl.
+ (point_and_state::point_and_state): Assert that the state is
+ valid.
+ * program-state.cc (program_state::program_state): Initialize
+ m_valid to true.
+ (program_state::operator=): Copy m_valid.
+ (program_state::program_state): Likewise for move constructor.
+ (program_state::print): Print m_valid.
+ (program_state::dump_to_pp): Likewise.
+ * program-state.h (program_state::m_valid): New field.
+ * region-model.cc (region_model::get_lvalue_1): Implement the
+ default case by returning a new symbolic region and calling
+ the context's on_unknown_tree_code, rather than issuing an
+ internal_error. Implement VIEW_CONVERT_EXPR.
+ * region-model.h (region_model_context::on_unknown_tree_code): New
+ vfunc.
+ (test_region_model_context::on_unknown_tree_code): New.
+
2020-02-17 David Malcolm <dmalcolm@redhat.com>
* sm-malloc.cc (malloc_diagnostic::describe_state_change): For
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 17507c7c08e..cd4ffe55dc5 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -684,6 +684,25 @@ impl_region_model_context::on_phi (const gphi *phi, tree rhs)
}
}
+/* Implementation of region_model_context::on_unknown_tree_code vfunc.
+ Mark the new state as being invalid for further exploration.
+ TODO(stage1): introduce a warning for when this occurs. */
+
+void
+impl_region_model_context::on_unknown_tree_code (path_var pv,
+ const dump_location_t &loc)
+{
+ logger * const logger = get_logger ();
+ if (logger)
+ logger->log ("unhandled tree code: %qs in %qs at %s:%i",
+ get_tree_code_name (TREE_CODE (pv.m_tree)),
+ loc.get_impl_location ().m_function,
+ loc.get_impl_location ().m_file,
+ loc.get_impl_location ().m_line);
+ if (m_new_state)
+ m_new_state->m_valid = false;
+}
+
/* struct point_and_state. */
/* Assert that this object is sane. */
@@ -1845,6 +1864,15 @@ exploded_graph::get_or_create_node (const program_point &point,
logger->end_log_line ();
}
+ /* Stop exploring paths for which we don't know how to effectively
+ model the state. */
+ if (!state.m_valid)
+ {
+ if (logger)
+ logger->log ("invalid state; not creating node");
+ return NULL;
+ }
+
auto_cfun sentinel (point.get_function ());
state.validate (get_ext_state ());
diff --git a/gcc/analyzer/exploded-graph.h b/gcc/analyzer/exploded-graph.h
index bb1f3ccff1a..614c37ce9af 100644
--- a/gcc/analyzer/exploded-graph.h
+++ b/gcc/analyzer/exploded-graph.h
@@ -76,6 +76,9 @@ class impl_region_model_context : public region_model_context
void on_phi (const gphi *phi, tree rhs) FINAL OVERRIDE;
+ void on_unknown_tree_code (path_var pv,
+ const dump_location_t &loc) FINAL OVERRIDE;
+
exploded_graph *m_eg;
log_user m_logger;
const exploded_node *m_enode_for_diag;
@@ -100,6 +103,9 @@ public:
m_state (state),
m_hash (m_point.hash () ^ m_state.hash ())
{
+ /* We shouldn't be building point_and_states and thus exploded_nodes
+ for states that aren't valid. */
+ gcc_assert (state.m_valid);
}
hashval_t hash () const
diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
index 82b921eb969..fb96e3c976b 100644
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@@ -573,7 +573,8 @@ sm_state_map::validate (const state_machine &sm,
program_state::program_state (const extrinsic_state &ext_state)
: m_region_model (new region_model ()),
- m_checker_states (ext_state.get_num_checkers ())
+ m_checker_states (ext_state.get_num_checkers ()),
+ m_valid (true)
{
int num_states = ext_state.get_num_checkers ();
for (int i = 0; i < num_states; i++)
@@ -584,7 +585,8 @@ program_state::program_state (const extrinsic_state &ext_state)
program_state::program_state (const program_state &other)
: m_region_model (new region_model (*other.m_region_model)),
- m_checker_states (other.m_checker_states.length ())
+ m_checker_states (other.m_checker_states.length ()),
+ m_valid (true)
{
int i;
sm_state_map *smap;
@@ -610,6 +612,8 @@ program_state::operator= (const program_state &other)
FOR_EACH_VEC_ELT (other.m_checker_states, i, smap)
m_checker_states.quick_push (smap->clone ());
+ m_valid = other.m_valid;
+
return *this;
}
@@ -626,6 +630,8 @@ program_state::program_state (program_state &&other)
FOR_EACH_VEC_ELT (other.m_checker_states, i, smap)
m_checker_states.quick_push (smap);
other.m_checker_states.truncate (0);
+
+ m_valid = other.m_valid;
}
#endif
@@ -693,6 +699,11 @@ program_state::print (const extrinsic_state &ext_state,
pp_newline (pp);
}
}
+ if (!m_valid)
+ {
+ pp_printf (pp, "invalid state");
+ pp_newline (pp);
+ }
}
/* Dump a multiline representation of this state to PP. */
@@ -716,6 +727,12 @@ program_state::dump_to_pp (const extrinsic_state &ext_state,
pp_newline (pp);
}
}
+
+ if (!m_valid)
+ {
+ pp_printf (pp, "invalid state");
+ pp_newline (pp);
+ }
}
/* Dump a multiline representation of this state to OUTF. */
diff --git a/gcc/analyzer/program-state.h b/gcc/analyzer/program-state.h
index a4608c7498d..80df649f565 100644
--- a/gcc/analyzer/program-state.h
+++ b/gcc/analyzer/program-state.h
@@ -286,6 +286,11 @@ public:
/* TODO: lose the pointer here (const-correctness issues?). */
region_model *m_region_model;
auto_delete_vec<sm_state_map> m_checker_states;
+
+ /* If false, then don't attempt to explore further states along this path.
+ For use in "handling" lvalues for tree codes we haven't yet
+ implemented. */
+ bool m_valid;
};
/* An abstract base class for use with for_each_state_change. */
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index ae810f5eb4b..b67660cf864 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -4641,9 +4641,19 @@ region_model::get_lvalue_1 (path_var pv, region_model_context *ctxt)
switch (TREE_CODE (expr))
{
default:
- internal_error ("unhandled tree code in region_model::get_lvalue_1: %qs",
- get_tree_code_name (TREE_CODE (expr)));
- gcc_unreachable ();
+ {
+ /* If we see a tree code we we don't know how to handle, rather than
+ ICE or generate bogus results, create a dummy region, and notify
+ CTXT so that it can mark the new state as being not properly
+ modelled. The exploded graph can then stop exploring that path,
+ since any diagnostics we might issue will have questionable
+ validity. */
+ region_id new_rid
+ = add_region (new symbolic_region (m_root_rid, NULL_TREE, false));
+ ctxt->on_unknown_tree_code (pv, dump_location_t ());
+ return new_rid;
+ }
+ break;
case ARRAY_REF:
{
@@ -4750,6 +4760,13 @@ region_model::get_lvalue_1 (path_var pv, region_model_context *ctxt)
return cst_rid;
}
break;
+
+ case VIEW_CONVERT_EXPR:
+ {
+ tree obj = TREE_OPERAND (expr, 0);
+ return get_or_create_view (get_lvalue (obj, ctxt), TREE_TYPE (expr));
+ };
+ break;
}
}
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index 9c9a936fae2..dc56d64a43b 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -1937,6 +1937,11 @@ class region_model_context
/* Hooks for clients to be notified when a phi node is handled,
where RHS is the pertinent argument. */
virtual void on_phi (const gphi *phi, tree rhs) = 0;
+
+ /* Hooks for clients to be notified when the region model doesn't
+ know how to handle the tree code of PV at LOC. */
+ virtual void on_unknown_tree_code (path_var pv,
+ const dump_location_t &loc) = 0;
};
/* A bundle of data for use when attempting to merge two region_model
@@ -2118,6 +2123,13 @@ public:
{
}
+ void on_unknown_tree_code (path_var pv, const dump_location_t &)
+ FINAL OVERRIDE
+ {
+ internal_error ("unhandled tree code: %qs",
+ get_tree_code_name (TREE_CODE (pv.m_tree)));
+ }
+
private:
/* Implicitly delete any diagnostics in the dtor. */
auto_delete_vec<pending_diagnostic> m_diagnostics;
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index a08ad2e7175..6c34e0bbe36 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2020-02-17 David Malcolm <dmalcolm@redhat.com>
+
+ PR analyzer/93388
+ * gcc.dg/analyzer/torture/20060625-1.c: New test.
+ * gcc.dg/analyzer/torture/pr51628-30.c: New test.
+ * gcc.dg/analyzer/torture/pr59037.c: New test.
+
2020-02-17 David Malcolm <dmalcolm@redhat.com>
* gcc.dg/analyzer/malloc-1.c (test_48): New.
diff --git a/gcc/testsuite/gcc.dg/analyzer/torture/20060625-1.c b/gcc/testsuite/gcc.dg/analyzer/torture/20060625-1.c
new file mode 100644
index 00000000000..2657a925a7a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/torture/20060625-1.c
@@ -0,0 +1 @@
+#include "../../../gcc.c-torture/compile/20060625-1.c"
diff --git a/gcc/testsuite/gcc.dg/analyzer/torture/pr51628-30.c b/gcc/testsuite/gcc.dg/analyzer/torture/pr51628-30.c
new file mode 100644
index 00000000000..4513e0f890c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/torture/pr51628-30.c
@@ -0,0 +1,3 @@
+/* { dg-additional-options "-Wno-address-of-packed-member" } */
+
+#include "../../../c-c++-common/pr51628-30.c"
diff --git a/gcc/testsuite/gcc.dg/analyzer/torture/pr59037.c b/gcc/testsuite/gcc.dg/analyzer/torture/pr59037.c
new file mode 100644
index 00000000000..d01aaec8eea
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/torture/pr59037.c
@@ -0,0 +1 @@
+#include "../../../c-c++-common/pr59037.c"
More information about the Gcc-cvs
mailing list