From c95c552804da90c830941111706e623106a7728a Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Mon, 19 Nov 2018 16:42:03 +0000 Subject: [PATCH] Fix -fsave-optimization-record ICE (PR tree-optimization/87025) 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. 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. From-SVN: r266280 --- gcc/ChangeLog | 12 ++++++++++++ gcc/dumpfile.c | 16 ++++++++++++++++ gcc/optinfo-emit-json.cc | 3 +++ gcc/optinfo.cc | 3 +++ gcc/testsuite/ChangeLog | 5 +++++ gcc/testsuite/gcc.dg/pr87025.c | 22 ++++++++++++++++++++++ 6 files changed, 61 insertions(+) create mode 100644 gcc/testsuite/gcc.dg/pr87025.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 28fb034ab56..126f168a15d 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,15 @@ +2018-11-19 David Malcolm + + 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. + 2018-11-19 David Malcolm PR tree-optimization/87025 diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c index 014acf102fc..e125650eed1 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 4fa6708600a..841a13b4f3b 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 f8e08dee72f..f76da45fb98 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/ChangeLog b/gcc/testsuite/ChangeLog index 2b9f0c483ad..cea66a545f1 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2018-11-19 David Malcolm + + PR tree-optimization/87025 + * gcc.dg/pr87025.c: New test. + 2018-11-19 Jakub Jelinek PR tree-optimization/88071 diff --git a/gcc/testsuite/gcc.dg/pr87025.c b/gcc/testsuite/gcc.dg/pr87025.c new file mode 100644 index 00000000000..059313ccdaa --- /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; + } +} -- 2.30.2