This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH 2/2] Fix -fsave-optimization-record ICE (PR tree-optimization/87025)
- From: David Malcolm <dmalcolm at redhat dot com>
- To: gcc-patches at gcc dot gnu dot org
- Cc: David Malcolm <dmalcolm at redhat dot com>
- Date: Fri, 16 Nov 2018 19:11:54 -0500
- Subject: [PATCH 2/2] Fix -fsave-optimization-record ICE (PR tree-optimization/87025)
- References: <1542413514-48487-1-git-send-email-dmalcolm@redhat.com>
PR tree-optimization/87025 reports an ICE within
-fsave-optimization-record's optrecord_json_writer.
The issue is that dump_context::begin_scope creates an optinfo
of kind OPTINFO_KIND_SCOPE, but fails to call
dump_context::end_any_optinfo, so the optinfo for the scope remains
pending.
The JSON writer would normally push a JSON array for the "scope" optinfo
when the latter is emitted. However, if a dump_* call happens that
doesn't flush the "scope" optinfo e.g. dump_printf (as opposed to
dump_printf_loc), that dump_ call is added to the pending optinfo, and
optinfo::handle_dump_file_kind changes the pending optinfo's m_kind
(e.g. to OPTINFO_KIND_NOTE). Hence when the pending optinfo is
eventually emitted, it isn't OPTINFO_KIND_SCOPE anymore, and hence
the JSON writer doesn't create and push a JSON array for it, leading
to dump_context's view of scopes getting out-of-sync with that of
the JSON writer's.
Later, dump_context::end_scope unconditionally tries to pop the JSON scope
array, but no JSON scope array was added, leading to an assertion
failure (or crash).
The fix is to call dump_context::end_any_optinfo immediately after
creating the scope optinfo, so that it is emitted immediately, ensuring
that the JSON writer stays in-sync with the dump_context.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu
(in conjuction with the previous patch)
OK for trunk?
gcc/ChangeLog:
PR tree-optimization/87025
* dumpfile.c (dump_context::begin_scope): Call end_any_optinfo
immediately after creating the scope optinfo.
(selftest::test_pr87025): New function.
(selftest::dumpfile_c_tests): Call it.
* optinfo-emit-json.cc (optrecord_json_writer::pop_scope): Assert
that we're not popping the top-level records array.
* optinfo.cc (optinfo::handle_dump_file_kind): Assert that we're
not changing the kind of a "scope" optinfo.
gcc/testsuite/ChangeLog:
PR tree-optimization/87025
* gcc.dg/pr87025.c: New test.
---
gcc/dumpfile.c | 16 ++++++++++++++++
gcc/optinfo-emit-json.cc | 3 +++
gcc/optinfo.cc | 3 +++
gcc/testsuite/gcc.dg/pr87025.c | 22 ++++++++++++++++++++++
4 files changed, 44 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/pr87025.c
diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
index 014acf1..e125650 100644
--- a/gcc/dumpfile.c
+++ b/gcc/dumpfile.c
@@ -1131,6 +1131,7 @@ dump_context::begin_scope (const char *name, const dump_location_t &loc)
optinfo &info = begin_next_optinfo (loc);
info.m_kind = OPTINFO_KIND_SCOPE;
info.add_item (item);
+ end_any_optinfo ();
}
else
delete item;
@@ -2575,6 +2576,20 @@ test_capture_of_dump_calls (const line_table_case &case_)
}
}
+static void
+test_pr87025 ()
+{
+ dump_user_location_t loc
+ = dump_user_location_t::from_location_t (UNKNOWN_LOCATION);
+
+ temp_dump_context tmp (true, true,
+ MSG_ALL_KINDS | MSG_PRIORITY_USER_FACING);
+ {
+ AUTO_DUMP_SCOPE ("outer scope", loc);
+ dump_printf (MSG_NOTE, "msg1\n");
+ }
+}
+
/* Run all of the selftests within this file. */
void
@@ -2582,6 +2597,7 @@ dumpfile_c_tests ()
{
test_impl_location ();
for_each_line_table_case (test_capture_of_dump_calls);
+ test_pr87025 ();
}
} // namespace selftest
diff --git a/gcc/optinfo-emit-json.cc b/gcc/optinfo-emit-json.cc
index 4fa6708..841a13b 100644
--- a/gcc/optinfo-emit-json.cc
+++ b/gcc/optinfo-emit-json.cc
@@ -169,6 +169,9 @@ void
optrecord_json_writer::pop_scope ()
{
m_scopes.pop ();
+
+ /* We should never pop the top-level records array. */
+ gcc_assert (m_scopes.length () > 0);
}
/* Create a JSON object representing LOC. */
diff --git a/gcc/optinfo.cc b/gcc/optinfo.cc
index f8e08de..f76da45 100644
--- a/gcc/optinfo.cc
+++ b/gcc/optinfo.cc
@@ -133,6 +133,9 @@ optinfo::emit_for_opt_problem () const
void
optinfo::handle_dump_file_kind (dump_flags_t dump_kind)
{
+ /* Any optinfo for a "scope" should have been emitted separately. */
+ gcc_assert (m_kind != OPTINFO_KIND_SCOPE);
+
if (dump_kind & MSG_OPTIMIZED_LOCATIONS)
m_kind = OPTINFO_KIND_SUCCESS;
else if (dump_kind & MSG_MISSED_OPTIMIZATION)
diff --git a/gcc/testsuite/gcc.dg/pr87025.c b/gcc/testsuite/gcc.dg/pr87025.c
new file mode 100644
index 0000000..059313c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr87025.c
@@ -0,0 +1,22 @@
+/* Ensure we don't ICE when tracking optimization record scopes within
+ the vectorizer. */
+/* { dg-do compile } */
+/* { dg-options "-O1 -fsave-optimization-record -ftree-vectorize -fno-tree-scev-cprop -fno-tree-sink" } */
+
+void
+fk (unsigned int sf)
+{
+ for (;;)
+ {
+ if (sf != 0)
+ {
+ while (sf != 0)
+ ++sf;
+
+ while (sf < 8)
+ ++sf;
+ }
+
+ ++sf;
+ }
+}
--
1.8.5.3