[PATCH] Fix segfault in -fsave-optimization-record (PR tree-optimization/86636)
Andre Vieira (lists)
Andre.SimoesDiasVieira@arm.com
Thu Jul 26 12:22:00 GMT 2018
On 24/07/18 15:12, Richard Biener wrote:
> On Tue, Jul 24, 2018 at 1:44 AM David Malcolm <dmalcolm@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
>>
Hi David,
I believe the test in this patch needs a "{ dg-require-effective-target
pthread }" as -ftree-parallelize-loops seems to be turning on -pthread.
Cheers,
Andre
More information about the Gcc-patches
mailing list