From: David Malcolm Date: Thu, 23 May 2019 00:42:03 +0000 (+0000) Subject: Bulletproof -fdiagnostics-format=json against bad locations (PR c++/90462) X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=30d3ba5142311df568e8f62a374d83d0bdab42bf;p=gcc.git Bulletproof -fdiagnostics-format=json against bad locations (PR c++/90462) PR c++/90462 reports an ICE with -fdiagnostics-format=json when attempting to serialize a malformed location to JSON. The compound location_t in question has meaningful "caret" and "start" locations, but has UNKNOWN_LOCATION for its "finish" location, leading to a NULL pointer dereference when attempting to build a JSON string for the filename. This patch bulletproofs the JSON output so that attempts to write a JSON object for a location with a NULL file will lead to an object with no "file" key, and attempts to write a compound location with UNKNOWN_LOCATION for its start or finish will lead to the corresponding JSON child object being omitted. This patch also adds a json::object::get member function, for self-testing the above. gcc/ChangeLog: PR c++/90462 * diagnostic-format-json.cc: Include "selftest.h". (json_from_expanded_location): Only add "file" key for non-NULL file strings. (json_from_location_range): Don't add "start" and "finish" children if they are UNKNOWN_LOCATION. (selftest::test_unknown_location): New selftest. (selftest::test_bad_endpoints): New selftest. (selftest::diagnostic_format_json_cc_tests): New function. * json.cc (json::object::get): New function. (selftest::test_object_get): New selftest. (selftest::json_cc_tests): Call it. * json.h (json::object::get): New decl. * selftest-run-tests.c (selftest::run_tests): Call selftest::diagnostic_format_json_cc_tests. * selftest.h (selftest::diagnostic_format_json_cc_tests): New decl. gcc/testsuite/ChangeLog: PR c++/90462 * g++.dg/pr90462.C: New test. From-SVN: r271535 --- diff --git a/gcc/ChangeLog b/gcc/ChangeLog index fd69b148632..9f0f8893022 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,23 @@ +2019-05-22 David Malcolm + + PR c++/90462 + * diagnostic-format-json.cc: Include "selftest.h". + (json_from_expanded_location): Only add "file" key for non-NULL + file strings. + (json_from_location_range): Don't add "start" and "finish" + children if they are UNKNOWN_LOCATION. + (selftest::test_unknown_location): New selftest. + (selftest::test_bad_endpoints): New selftest. + (selftest::diagnostic_format_json_cc_tests): New function. + * json.cc (json::object::get): New function. + (selftest::test_object_get): New selftest. + (selftest::json_cc_tests): Call it. + * json.h (json::object::get): New decl. + * selftest-run-tests.c (selftest::run_tests): Call + selftest::diagnostic_format_json_cc_tests. + * selftest.h (selftest::diagnostic_format_json_cc_tests): New + decl. + 2019-05-22 Kwok Cheung Yeung Andrew Stubbs diff --git a/gcc/diagnostic-format-json.cc b/gcc/diagnostic-format-json.cc index 03552104766..53c3b630b1c 100644 --- a/gcc/diagnostic-format-json.cc +++ b/gcc/diagnostic-format-json.cc @@ -24,6 +24,7 @@ along with GCC; see the file COPYING3. If not see #include "coretypes.h" #include "diagnostic.h" #include "json.h" +#include "selftest.h" /* The top-level JSON array of pending diagnostics. */ @@ -45,7 +46,8 @@ json_from_expanded_location (location_t loc) { expanded_location exploc = expand_location (loc); json::object *result = new json::object (); - result->set ("file", new json::string (exploc.file)); + if (exploc.file) + result->set ("file", new json::string (exploc.file)); result->set ("line", new json::number (exploc.line)); result->set ("column", new json::number (exploc.column)); return result; @@ -66,9 +68,11 @@ json_from_location_range (const location_range *loc_range, unsigned range_idx) json::object *result = new json::object (); result->set ("caret", json_from_expanded_location (caret_loc)); - if (start_loc != caret_loc) + if (start_loc != caret_loc + && start_loc != UNKNOWN_LOCATION) result->set ("start", json_from_expanded_location (start_loc)); - if (finish_loc != caret_loc) + if (finish_loc != caret_loc + && finish_loc != UNKNOWN_LOCATION) result->set ("finish", json_from_expanded_location (finish_loc)); if (loc_range->m_label) @@ -262,3 +266,53 @@ diagnostic_output_format_init (diagnostic_context *context, break; } } + +#if CHECKING_P + +namespace selftest { + +/* We shouldn't call json_from_expanded_location on UNKNOWN_LOCATION, + but verify that we handle this gracefully. */ + +static void +test_unknown_location () +{ + delete json_from_expanded_location (UNKNOWN_LOCATION); +} + +/* Verify that we gracefully handle attempts to serialize bad + compound locations. */ + +static void +test_bad_endpoints () +{ + location_t bad_endpoints + = make_location (BUILTINS_LOCATION, + UNKNOWN_LOCATION, UNKNOWN_LOCATION); + + location_range loc_range; + loc_range.m_loc = bad_endpoints; + loc_range.m_range_display_kind = SHOW_RANGE_WITH_CARET; + loc_range.m_label = NULL; + + json::object *obj = json_from_location_range (&loc_range, 0); + /* We should have a "caret" value, but no "start" or "finish" values. */ + ASSERT_TRUE (obj != NULL); + ASSERT_TRUE (obj->get ("caret") != NULL); + ASSERT_TRUE (obj->get ("start") == NULL); + ASSERT_TRUE (obj->get ("finish") == NULL); + delete obj; +} + +/* Run all of the selftests within this file. */ + +void +diagnostic_format_json_cc_tests () +{ + test_unknown_location (); + test_bad_endpoints (); +} + +} // namespace selftest + +#endif /* #if CHECKING_P */ diff --git a/gcc/json.cc b/gcc/json.cc index 2e8e21b6791..512e2e513b9 100644 --- a/gcc/json.cc +++ b/gcc/json.cc @@ -99,6 +99,22 @@ object::set (const char *key, value *v) m_map.put (xstrdup (key), v); } +/* Get the json::value * for KEY. + + The object retains ownership of the value. */ + +value * +object::get (const char *key) const +{ + gcc_assert (key); + + value **ptr = const_cast (m_map).get (key); + if (ptr) + return *ptr; + else + return NULL; +} + /* class json::array, a subclass of json::value, representing an ordered collection of values. */ @@ -240,6 +256,18 @@ assert_print_eq (const json::value &jv, const char *expected_json) ASSERT_STREQ (expected_json, pp_formatted_text (&pp)); } +/* Verify that object::get works as expected. */ + +static void +test_object_get () +{ + object obj; + value *val = new json::string ("value"); + obj.set ("foo", val); + ASSERT_EQ (obj.get ("foo"), val); + ASSERT_EQ (obj.get ("not-present"), NULL); +} + /* Verify that JSON objects are written correctly. We can't test more than one key/value pair, as we don't impose a guaranteed ordering. */ @@ -306,6 +334,7 @@ test_writing_literals () void json_cc_tests () { + test_object_get (); test_writing_objects (); test_writing_arrays (); test_writing_numbers (); diff --git a/gcc/json.h b/gcc/json.h index 0527a2fc01f..d8a690ede5c 100644 --- a/gcc/json.h +++ b/gcc/json.h @@ -90,6 +90,7 @@ class object : public value void print (pretty_printer *pp) const FINAL OVERRIDE; void set (const char *key, value *v); + value *get (const char *key) const; private: typedef hash_map + + PR c++/90462 + * g++.dg/pr90462.C: New test. + 2019-05-22 Marek Polacek * g++.dg/cpp1y/udlit-char-template-neg.C: Expect the error on a diff --git a/gcc/testsuite/g++.dg/pr90462.C b/gcc/testsuite/g++.dg/pr90462.C new file mode 100644 index 00000000000..2585ba0dcdb --- /dev/null +++ b/gcc/testsuite/g++.dg/pr90462.C @@ -0,0 +1,49 @@ +/* { dg-options "-Wdeprecated-copy -fdiagnostics-format=json" } */ + +template class b; +struct B { + typedef b *c; +}; +class d { +public: + B::c operator->(); +}; +template struct e; +class f { + typedef int g; +}; +template class h; +template class b { +public: + i j; + i k; + int l; + void assign() { + int m; + h n(&m); + n.o(&j, &k, l); + } +}; +template class s : f { s &p(const i *, const i *, g); }; +template s &s::p(const i *, const i *, g) { + d q; + q->assign(); +} +struct G { + G(); + G(int); + G(G &); +}; +template class h { +public: + h(int *); + void o(const i *, const i *, unsigned); + i r(); +}; +template void h::o(const i *, const i *, unsigned) { + G a; + a = r(); +} +template s> &s>::p(const char *, const char *, g); + +/* { dg-regexp ".*" } */