On Tue, Jul 24, 2018 at 1:44 AM David Malcolm <dmalc...@redhat.com> wrote: > > There are various ways that it's possible for a gimple statement to > have an UNKNOWN_LOCATION, and for that UNKNOWN_LOCATION to be wrapped > in an ad-hoc location to capture inlining information. > > For such a location, LOCATION_FILE (loc) is NULL. > > Various places in -fsave-optimization-record were checking for > loc != UNKNOWN_LOCATION > and were passing LOCATION_FILE (loc) to code that assumed a non-NULL > filename, thus leading to segfaults for the above cases. > > This patch updates the tests to use > LOCATION_LOCUS (loc) != UNKNOWN_LOCATION > instead, to look through ad-hoc location wrappers, fixing the segfaults. > > It also adds various assertions to the affected code. > > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu; adds > 8 PASS results to gcc.sum. > > OK for trunk?
OK. Richard. > > gcc/ChangeLog: > PR tree-optimization/86636 > * json.cc (json::object::set): Fix comment. Add assertions. > (json::array::append): Move here from json.h. Add comment and an > assertion. > (json::string::string): Likewise. > * json.h (json::array::append): Move to json.cc. > (json::string::string): Likewise. > * optinfo-emit-json.cc > (optrecord_json_writer::impl_location_to_json): Assert that we > aren't attempting to write out UNKNOWN_LOCATION, or an ad-hoc > wrapper around it. Expand the location once, rather than three > times. > (optrecord_json_writer::inlining_chain_to_json): Fix the check for > UNKNOWN_LOCATION, to use LOCATION_LOCUS to look through ad-hoc > wrappers. > (optrecord_json_writer::optinfo_to_json): Likewise, in four > places. Fix some overlong lines. > > gcc/testsuite/ChangeLog: > PR tree-optimization/86636 > * gcc.c-torture/compile/pr86636.c: New test. > --- > gcc/json.cc | 24 +++++++++++++++++++++++- > gcc/json.h | 4 ++-- > gcc/optinfo-emit-json.cc | 25 +++++++++++++++---------- > gcc/testsuite/gcc.c-torture/compile/pr86636.c | 8 ++++++++ > 4 files changed, 48 insertions(+), 13 deletions(-) > create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr86636.c > > diff --git a/gcc/json.cc b/gcc/json.cc > index 3c2aa77..3ead980 100644 > --- a/gcc/json.cc > +++ b/gcc/json.cc > @@ -76,12 +76,15 @@ object::print (pretty_printer *pp) const > pp_character (pp, '}'); > } > > -/* Set the json::value * for KEY, taking ownership of VALUE > +/* Set the json::value * for KEY, taking ownership of V > (and taking a copy of KEY if necessary). */ > > void > object::set (const char *key, value *v) > { > + gcc_assert (key); > + gcc_assert (v); > + > value **ptr = m_map.get (key); > if (ptr) > { > @@ -126,6 +129,15 @@ array::print (pretty_printer *pp) const > pp_character (pp, ']'); > } > > +/* Append non-NULL value V to a json::array, taking ownership of V. */ > + > +void > +array::append (value *v) > +{ > + gcc_assert (v); > + m_elements.safe_push (v); > +} > + > /* class json::number, a subclass of json::value, wrapping a double. */ > > /* Implementation of json::value::print for json::number. */ > @@ -140,6 +152,16 @@ number::print (pretty_printer *pp) const > > /* class json::string, a subclass of json::value. */ > > +/* json::string's ctor. */ > + > +string::string (const char *utf8) > +{ > + gcc_assert (utf8); > + m_utf8 = xstrdup (utf8); > +} > + > +/* Implementation of json::value::print for json::string. */ > + > void > string::print (pretty_printer *pp) const > { > diff --git a/gcc/json.h b/gcc/json.h > index 5c3274c..154d9e1 100644 > --- a/gcc/json.h > +++ b/gcc/json.h > @@ -107,7 +107,7 @@ class array : public value > enum kind get_kind () const FINAL OVERRIDE { return JSON_ARRAY; } > void print (pretty_printer *pp) const FINAL OVERRIDE; > > - void append (value *v) { m_elements.safe_push (v); } > + void append (value *v); > > private: > auto_vec<value *> m_elements; > @@ -134,7 +134,7 @@ class number : public value > class string : public value > { > public: > - string (const char *utf8) : m_utf8 (xstrdup (utf8)) {} > + string (const char *utf8); > ~string () { free (m_utf8); } > > enum kind get_kind () const FINAL OVERRIDE { return JSON_STRING; } > diff --git a/gcc/optinfo-emit-json.cc b/gcc/optinfo-emit-json.cc > index bf1172a..6460a81 100644 > --- a/gcc/optinfo-emit-json.cc > +++ b/gcc/optinfo-emit-json.cc > @@ -202,10 +202,12 @@ optrecord_json_writer::impl_location_to_json > (dump_impl_location_t loc) > json::object * > optrecord_json_writer::location_to_json (location_t loc) > { > + gcc_assert (LOCATION_LOCUS (loc) != UNKNOWN_LOCATION); > + expanded_location exploc = expand_location (loc); > json::object *obj = new json::object (); > - obj->set ("file", new json::string (LOCATION_FILE (loc))); > - obj->set ("line", new json::number (LOCATION_LINE (loc))); > - obj->set ("column", new json::number (LOCATION_COLUMN (loc))); > + obj->set ("file", new json::string (exploc.file)); > + obj->set ("line", new json::number (exploc.line)); > + obj->set ("column", new json::number (exploc.column)); > return obj; > } > > @@ -330,7 +332,7 @@ optrecord_json_writer::inlining_chain_to_json (location_t > loc) > const char *printable_name > = lang_hooks.decl_printable_name (fndecl, 2); > obj->set ("fndecl", new json::string (printable_name)); > - if (*locus != UNKNOWN_LOCATION) > + if (LOCATION_LOCUS (*locus) != UNKNOWN_LOCATION) > obj->set ("site", location_to_json (*locus)); > array->append (obj); > } > @@ -371,8 +373,9 @@ optrecord_json_writer::optinfo_to_json (const optinfo > *optinfo) > json_item->set ("expr", new json::string (item->get_text ())); > > /* Capture any location for the node. */ > - if (item->get_location () != UNKNOWN_LOCATION) > - json_item->set ("location", location_to_json > (item->get_location ())); > + if (LOCATION_LOCUS (item->get_location ()) != UNKNOWN_LOCATION) > + json_item->set ("location", > + location_to_json (item->get_location ())); > > message->append (json_item); > } > @@ -383,8 +386,9 @@ optrecord_json_writer::optinfo_to_json (const optinfo > *optinfo) > json_item->set ("stmt", new json::string (item->get_text ())); > > /* Capture any location for the stmt. */ > - if (item->get_location () != UNKNOWN_LOCATION) > - json_item->set ("location", location_to_json > (item->get_location ())); > + if (LOCATION_LOCUS (item->get_location ()) != UNKNOWN_LOCATION) > + json_item->set ("location", > + location_to_json (item->get_location ())); > > message->append (json_item); > } > @@ -395,8 +399,9 @@ optrecord_json_writer::optinfo_to_json (const optinfo > *optinfo) > json_item->set ("symtab_node", new json::string (item->get_text > ())); > > /* Capture any location for the node. */ > - if (item->get_location () != UNKNOWN_LOCATION) > - json_item->set ("location", location_to_json > (item->get_location ())); > + if (LOCATION_LOCUS (item->get_location ()) != UNKNOWN_LOCATION) > + json_item->set ("location", > + location_to_json (item->get_location ())); > message->append (json_item); > } > break; > diff --git a/gcc/testsuite/gcc.c-torture/compile/pr86636.c > b/gcc/testsuite/gcc.c-torture/compile/pr86636.c > new file mode 100644 > index 0000000..2fe2f70 > --- /dev/null > +++ b/gcc/testsuite/gcc.c-torture/compile/pr86636.c > @@ -0,0 +1,8 @@ > +/* { dg-options "-fsave-optimization-record -ftree-loop-vectorize > -ftree-parallelize-loops=2" } */ > + > +void > +n2 (int ih) > +{ > + while (ih < 1) > + ++ih; > +} > -- > 1.8.5.3 >