This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix segfault in -fsave-optimization-record (PR tree-optimization/86636)


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
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]