On Sat, Nov 17, 2018 at 1:12 AM David Malcolm <dmalc...@redhat.com> wrote: > > 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?
OK. > 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 >