Fix segfault in -fsave-optimization-record (PR tree-optimization/86636)

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.

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.

From-SVN: r262950

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index f1fdb1f35ed9acf2c8c7c148466335a6de3bb8cd..02991c17bc5f2d4b540173ed85897af72ec9b4f2 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,23 @@
+2018-07-24  David Malcolm  <dmalcolm@redhat.com>
+
+       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.
+
 2018-07-24  Matthew Malcomson  <matthew.malcomson@arm.com>
 
        * config/aarch64/aarch64-simd.md

diff --git a/gcc/json.cc b/gcc/json.cc
index 3c2aa77fb8dccccf96f36670b60b6e3a13fcb960..3ead98073f6a31d7fdd285e2dc0ed8f94d7829a9 100644 (file)
--- 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 5c3274cab79be2fba1642118d09019f70eb8db50..154d9e1b57517a75323fb2dc58981935c87ee1e5 100644 (file)
--- 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 bf1172a2e09f0859c85cebd6973bdc923b4b5cd0..6460a812f9e8e09377587a3fb8963b734b2e0fa9 100644 (file)
--- 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/ChangeLog b/gcc/testsuite/ChangeLog
index 10488cac66da649c1470c25bc49b91ec633baa58..f8cf9eeb466176a2a444825f87914a83a04f222d 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2018-07-24  David Malcolm  <dmalcolm@redhat.com>
+
+       PR tree-optimization/86636
+       * gcc.c-torture/compile/pr86636.c: New test.
+
 2018-07-24  Matthew Malcomson  <matthew.malcomson@arm.com>
 
        * gcc.target/aarch64/vect-su-add-sub.c: New.

diff --git a/gcc/testsuite/gcc.c-torture/compile/pr86636.c b/gcc/testsuite/gcc.c-torture/compile/pr86636.c
new file mode 100644 (file)
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;
+}