On Wed, Nov 7, 2018 at 5:22 PM David Malcolm <dmalc...@redhat.com> wrote:
>
> This patch implements support for %C in dump_printf for dumping
> cgraph_node *.
> (I would have preferred to have a code for printing symtab_node *
> and both subclasses, but there doesn't seem to be a good way for
> -Wformat to handle inheritance, so, failing that, I went with
> this approach).
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu, in
> conjunction with the rest of the patch kit.
>
> OK for trunk?
OK
> gcc/c-family/ChangeLog:
> * c-format.c (local_cgraph_node_ptr_node): New variable.
> (gcc_dump_printf_char_table): Add entry for %C.
> (get_pointer_to_named_type): New function, taken from the handling
> code for "gimple *" from...
> (init_dynamic_diag_info): ...here. Add handling for
> "cgraph_node *".
> * c-format.h (T_CGRAPH_NODE): New.
>
> gcc/ChangeLog:
> * dump-context.h (ASSERT_IS_CGRAPH_NODE): New macro.
> * dumpfile.c (make_item_for_dump_cgraph_node): Move to before...
> (dump_pretty_printer::decode_format): Implement "%C" for
> cgraph_node *.
> (selftest::test_capture_of_dump_calls): Rename "where" to
> "stmt_loc". Convert test_decl to a function decl and set its
> location. Add a symbol_table_test RAII instance and a
> cgraph_node, using it to test "%C" and dump_symtab_node.
>
> gcc/testsuite/ChangeLog:
> * gcc.dg/format/gcc_diag-10.c (cgraph_node): New typedef.
> (test_dump): Add testing of %C.
> ---
> gcc/c-family/c-format.c | 56 ++++++++++-----
> gcc/c-family/c-format.h | 1 +
> gcc/dump-context.h | 8 +++
> gcc/dumpfile.c | 115
> ++++++++++++++++++++++--------
> gcc/testsuite/gcc.dg/format/gcc_diag-10.c | 5 +-
> 5 files changed, 135 insertions(+), 50 deletions(-)
>
> diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
> index a1133c7..385ee1a 100644
> --- a/gcc/c-family/c-format.c
> +++ b/gcc/c-family/c-format.c
> @@ -60,6 +60,7 @@ struct function_format_info
> /* Initialized in init_dynamic_diag_info. */
> static GTY(()) tree local_tree_type_node;
> static GTY(()) tree local_gimple_ptr_node;
> +static GTY(()) tree local_cgraph_node_ptr_node;
> static GTY(()) tree locus;
>
> static bool decode_format_attr (tree, function_format_info *, int);
> @@ -803,6 +804,9 @@ static const format_char_info
> gcc_dump_printf_char_table[] =
> /* E and G require a "gimple *" argument at runtime. */
> { "EG", 1, STD_C89, { T89_G, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "", "\"",
> NULL },
>
> + /* C requires a "cgraph_node *" argument at runtime. */
> + { "C", 1, STD_C89, { T_CGRAPH_NODE, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "", "\"",
> NULL },
> +
> /* T requires a "tree" at runtime. */
> { "T", 1, STD_C89, { T89_T, BADLEN, BADLEN, BADLEN, BADLEN,
> BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "", "\"",
> NULL },
>
> @@ -3879,6 +3883,33 @@ init_dynamic_gfc_info (void)
> }
> }
>
> +/* Lookup the type named NAME and return a pointer-to-NAME type if found.
> + Otherwise, return void_type_node if NAME has not been used yet, or
> NULL_TREE if
> + NAME is not a type (issuing an error). */
> +
> +static tree
> +get_pointer_to_named_type (const char *name)
> +{
> + tree result;
> + if ((result = maybe_get_identifier (name)))
> + {
> + result = identifier_global_value (result);
> + if (result)
> + {
> + if (TREE_CODE (result) != TYPE_DECL)
> + {
> + error ("%qs is not defined as a type", name);
> + result = NULL_TREE;
> + }
> + else
> + result = TREE_TYPE (result);
> + }
> + }
> + else
> + result = void_type_node;
> + return result;
> +}
> +
> /* Determine the types of "tree" and "location_t" in the code being
> compiled for use in GCC's diagnostic custom format attributes. You
> must have set dynamic_format_types before calling this function. */
> @@ -3932,25 +3963,12 @@ init_dynamic_diag_info (void)
> /* Similar to the above but for gimple*. */
> if (!local_gimple_ptr_node
> || local_gimple_ptr_node == void_type_node)
> - {
> - if ((local_gimple_ptr_node = maybe_get_identifier ("gimple")))
> - {
> - local_gimple_ptr_node
> - = identifier_global_value (local_gimple_ptr_node);
> - if (local_gimple_ptr_node)
> - {
> - if (TREE_CODE (local_gimple_ptr_node) != TYPE_DECL)
> - {
> - error ("%<gimple%> is not defined as a type");
> - local_gimple_ptr_node = 0;
> - }
> - else
> - local_gimple_ptr_node = TREE_TYPE (local_gimple_ptr_node);
> - }
> - }
> - else
> - local_gimple_ptr_node = void_type_node;
> - }
> + local_gimple_ptr_node = get_pointer_to_named_type ("gimple");
> +
> + /* Similar to the above but for cgraph_node*. */
> + if (!local_cgraph_node_ptr_node
> + || local_cgraph_node_ptr_node == void_type_node)
> + local_cgraph_node_ptr_node = get_pointer_to_named_type ("cgraph_node");
>
> static tree hwi;
>
> diff --git a/gcc/c-family/c-format.h b/gcc/c-family/c-format.h
> index d984d10..eabb4f0 100644
> --- a/gcc/c-family/c-format.h
> +++ b/gcc/c-family/c-format.h
> @@ -299,6 +299,7 @@ struct format_kind_info
> #define T99_UC { STD_C99, NULL, T_UC }
> #define T_V &void_type_node
> #define T89_G { STD_C89, NULL, &local_gimple_ptr_node }
> +#define T_CGRAPH_NODE { STD_C89, NULL, &local_cgraph_node_ptr_node }
> #define T89_T { STD_C89, NULL, &local_tree_type_node }
> #define T89_V { STD_C89, NULL, T_V }
> #define T_W &wchar_type_node
> diff --git a/gcc/dump-context.h b/gcc/dump-context.h
> index 3a45f23..ace139c 100644
> --- a/gcc/dump-context.h
> +++ b/gcc/dump-context.h
> @@ -251,6 +251,14 @@ verify_item (const location &loc,
> (EXPECTED_LOCATION), (EXPECTED_TEXT)); \
> SELFTEST_END_STMT
>
> +/* Verify that ITEM is a symtab node, with the expected values. */
> +
> +#define ASSERT_IS_SYMTAB_NODE(ITEM, EXPECTED_LOCATION, EXPECTED_TEXT) \
> + SELFTEST_BEGIN_STMT \
> + verify_item (SELFTEST_LOCATION, (ITEM), OPTINFO_ITEM_KIND_SYMTAB_NODE, \
> + (EXPECTED_LOCATION), (EXPECTED_TEXT)); \
> + SELFTEST_END_STMT
> +
> } // namespace selftest
>
> #endif /* CHECKING_P */
> diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
> index 0b140ff..09c2490 100644
> --- a/gcc/dumpfile.c
> +++ b/gcc/dumpfile.c
> @@ -748,6 +748,18 @@ dump_context::dump_generic_expr_loc (dump_flags_t
> dump_kind,
> dump_generic_expr (dump_kind, extra_dump_flags, t);
> }
>
> +/* Make an item for the given dump call. */
> +
> +static optinfo_item *
> +make_item_for_dump_symtab_node (symtab_node *node)
> +{
> + location_t loc = DECL_SOURCE_LOCATION (node->decl);
> + optinfo_item *item
> + = new optinfo_item (OPTINFO_ITEM_KIND_SYMTAB_NODE, loc,
> + xstrdup (node->dump_name ()));
> + return item;
> +}
> +
> /* dump_pretty_printer's ctor. */
>
> dump_pretty_printer::dump_pretty_printer (dump_context *context,
> @@ -881,6 +893,8 @@ dump_pretty_printer::format_decoder_cb (pretty_printer
> *pp, text_info *text,
> Supported format codes (in addition to the standard pretty_printer ones)
> are:
>
> + %C: cgraph_node *:
> + Equivalent to: dump_symtab_node (MSG_*, node)
> %E: gimple *:
> Equivalent to: dump_gimple_expr (MSG_*, TDF_SLIM, stmt, 0)
> %G: gimple *:
> @@ -888,7 +902,9 @@ dump_pretty_printer::format_decoder_cb (pretty_printer
> *pp, text_info *text,
> %T: tree:
> Equivalent to: dump_generic_expr (MSG_*, arg, TDF_SLIM).
>
> - FIXME: add symtab_node?
> + TODO: add a format code that can handle (symtab_node*) *and* both
> + subclasses (presumably means teaching -Wformat about non-virtual
> + subclasses).
>
> These format codes build optinfo_item instances, thus capturing metadata
> about the arguments being dumped, as well as the textual output. */
> @@ -901,6 +917,16 @@ dump_pretty_printer::decode_format (text_info *text,
> const char *spec,
> for later use (to capture metadata, rather than plain text). */
> switch (*spec)
> {
> + case 'C':
> + {
> + cgraph_node *node = va_arg (*text->args_ptr, cgraph_node *);
> +
> + /* Make an item for the node, and stash it. */
> + optinfo_item *item = make_item_for_dump_symtab_node (node);
> + stash_item (buffer_ptr, item);
> + return true;
> + }
> +
> case 'E':
> {
> gimple *stmt = va_arg (*text->args_ptr, gimple *);
> @@ -1023,18 +1049,6 @@ dump_context::dump_dec (dump_flags_t dump_kind, const
> poly_int<N, C> &value)
> delete item;
> }
>
> -/* Make an item for the given dump call. */
> -
> -static optinfo_item *
> -make_item_for_dump_symtab_node (symtab_node *node)
> -{
> - location_t loc = DECL_SOURCE_LOCATION (node->decl);
> - optinfo_item *item
> - = new optinfo_item (OPTINFO_ITEM_KIND_SYMTAB_NODE, loc,
> - xstrdup (node->dump_name ()));
> - return item;
> -}
> -
> /* Output the name of NODE on appropriate dump streams. */
>
> void
> @@ -2067,18 +2081,26 @@ test_capture_of_dump_calls (const line_table_case
> &case_)
> linemap_add (line_table, LC_ENTER, false, "test.txt", 0);
> linemap_line_start (line_table, 5, 100);
> linemap_add (line_table, LC_LEAVE, false, NULL, 0);
> - location_t where = linemap_position_for_column (line_table, 10);
> - if (where > LINE_MAP_MAX_LOCATION_WITH_COLS)
> + location_t decl_loc = linemap_position_for_column (line_table, 8);
> + location_t stmt_loc = linemap_position_for_column (line_table, 10);
> + if (stmt_loc > LINE_MAP_MAX_LOCATION_WITH_COLS)
> return;
>
> - dump_location_t loc = dump_location_t::from_location_t (where);
> + dump_location_t loc = dump_location_t::from_location_t (stmt_loc);
>
> gimple *stmt = gimple_build_return (NULL);
> - gimple_set_location (stmt, where);
> + gimple_set_location (stmt, stmt_loc);
>
> - tree test_decl = build_decl (UNKNOWN_LOCATION, VAR_DECL,
> + tree test_decl = build_decl (decl_loc, FUNCTION_DECL,
> get_identifier ("test_decl"),
> - integer_type_node);
> + build_function_type_list (void_type_node,
> + NULL_TREE));
> +
> + symbol_table_test tmp_symtab;
> +
> + cgraph_node *node = cgraph_node::get_create (test_decl);
> + gcc_assert (node);
> +
> /* Run all tests twice, with and then without optinfo enabled, to ensure
> that immediate destinations vs optinfo-based destinations both
> work, independently of each other, with no leaks. */
> @@ -2135,7 +2157,7 @@ test_capture_of_dump_calls (const line_table_case
> &case_)
> ASSERT_EQ (info->get_kind (), OPTINFO_KIND_NOTE);
> ASSERT_EQ (info->num_items (), 2);
> ASSERT_IS_TEXT (info->get_item (0), "gimple: ");
> - ASSERT_IS_GIMPLE (info->get_item (1), where, "return;");
> + ASSERT_IS_GIMPLE (info->get_item (1), stmt_loc, "return;");
> }
> }
>
> @@ -2153,7 +2175,25 @@ test_capture_of_dump_calls (const line_table_case
> &case_)
> ASSERT_EQ (info->get_kind (), OPTINFO_KIND_NOTE);
> ASSERT_EQ (info->num_items (), 2);
> ASSERT_IS_TEXT (info->get_item (0), "gimple: ");
> - ASSERT_IS_GIMPLE (info->get_item (1), where, "return;\n");
> + ASSERT_IS_GIMPLE (info->get_item (1), stmt_loc, "return;\n");
> + }
> + }
> +
> + /* Test of dump_printf with %C. */
> + {
> + temp_dump_context tmp (with_optinfo, true,
> + MSG_ALL_KINDS | MSG_PRIORITY_USER_FACING);
> + dump_printf (MSG_NOTE, "node: %C", node);
> +
> + ASSERT_DUMPED_TEXT_EQ (tmp, "node: test_decl/0");
> + if (with_optinfo)
> + {
> + optinfo *info = tmp.get_pending_optinfo ();
> + ASSERT_TRUE (info != NULL);
> + ASSERT_EQ (info->get_kind (), OPTINFO_KIND_NOTE);
> + ASSERT_EQ (info->num_items (), 2);
> + ASSERT_IS_TEXT (info->get_item (0), "node: ");
> + ASSERT_IS_SYMTAB_NODE (info->get_item (1), decl_loc,
> "test_decl/0");
> }
> }
>
> @@ -2184,8 +2224,8 @@ test_capture_of_dump_calls (const line_table_case
> &case_)
> ASSERT_IS_TEXT (info->get_item (2), " and ");
> ASSERT_IS_TREE (info->get_item (3), UNKNOWN_LOCATION,
> "test_decl");
> ASSERT_IS_TEXT (info->get_item (4), " 42 consecutive ");
> - ASSERT_IS_GIMPLE (info->get_item (5), where, "return;");
> - ASSERT_IS_GIMPLE (info->get_item (6), where, "return;");
> + ASSERT_IS_GIMPLE (info->get_item (5), stmt_loc, "return;");
> + ASSERT_IS_GIMPLE (info->get_item (6), stmt_loc, "return;");
> ASSERT_IS_TEXT (info->get_item (7), " after\n");
> }
> }
> @@ -2202,7 +2242,7 @@ test_capture_of_dump_calls (const line_table_case
> &case_)
> {
> optinfo *info = tmp.get_pending_optinfo ();
> ASSERT_TRUE (info != NULL);
> - ASSERT_EQ (info->get_location_t (), where);
> + ASSERT_EQ (info->get_location_t (), stmt_loc);
> ASSERT_EQ (info->get_kind (), OPTINFO_KIND_NOTE);
> ASSERT_EQ (info->num_items (), 2);
> ASSERT_IS_TEXT (info->get_item (0), "test of tree: ");
> @@ -2221,7 +2261,7 @@ test_capture_of_dump_calls (const line_table_case
> &case_)
> {
> optinfo *info = tmp.get_pending_optinfo ();
> ASSERT_TRUE (info != NULL);
> - ASSERT_EQ (info->get_location_t (), where);
> + ASSERT_EQ (info->get_location_t (), stmt_loc);
> ASSERT_EQ (info->get_kind (), OPTINFO_KIND_NOTE);
> ASSERT_EQ (info->num_items (), 1);
> ASSERT_IS_TREE (info->get_item (0), UNKNOWN_LOCATION, "1");
> @@ -2242,7 +2282,7 @@ test_capture_of_dump_calls (const line_table_case
> &case_)
> optinfo *info = tmp.get_pending_optinfo ();
> ASSERT_TRUE (info != NULL);
> ASSERT_EQ (info->num_items (), 1);
> - ASSERT_IS_GIMPLE (info->get_item (0), where, "return;\n");
> + ASSERT_IS_GIMPLE (info->get_item (0), stmt_loc, "return;\n");
> }
> }
>
> @@ -2258,7 +2298,7 @@ test_capture_of_dump_calls (const line_table_case
> &case_)
> optinfo *info = tmp.get_pending_optinfo ();
> ASSERT_TRUE (info != NULL);
> ASSERT_EQ (info->num_items (), 1);
> - ASSERT_IS_GIMPLE (info->get_item (0), where, "return;\n");
> + ASSERT_IS_GIMPLE (info->get_item (0), stmt_loc, "return;\n");
> }
> }
>
> @@ -2274,7 +2314,7 @@ test_capture_of_dump_calls (const line_table_case
> &case_)
> optinfo *info = tmp.get_pending_optinfo ();
> ASSERT_TRUE (info != NULL);
> ASSERT_EQ (info->num_items (), 1);
> - ASSERT_IS_GIMPLE (info->get_item (0), where, "return;");
> + ASSERT_IS_GIMPLE (info->get_item (0), stmt_loc, "return;");
> }
> }
>
> @@ -2290,11 +2330,28 @@ test_capture_of_dump_calls (const line_table_case
> &case_)
> optinfo *info = tmp.get_pending_optinfo ();
> ASSERT_TRUE (info != NULL);
> ASSERT_EQ (info->num_items (), 1);
> - ASSERT_IS_GIMPLE (info->get_item (0), where, "return;");
> + ASSERT_IS_GIMPLE (info->get_item (0), stmt_loc, "return;");
> }
> }
> }
>
> + /* symtab_node. */
> + {
> + temp_dump_context tmp (with_optinfo, true,
> + MSG_ALL_KINDS | MSG_PRIORITY_USER_FACING);
> + dump_symtab_node (MSG_NOTE, node);
> +
> + ASSERT_DUMPED_TEXT_EQ (tmp, "test_decl/0");
> + if (with_optinfo)
> + {
> + optinfo *info = tmp.get_pending_optinfo ();
> + ASSERT_TRUE (info != NULL);
> + ASSERT_EQ (info->get_kind (), OPTINFO_KIND_NOTE);
> + ASSERT_EQ (info->num_items (), 1);
> + ASSERT_IS_SYMTAB_NODE (info->get_item (0), decl_loc,
> "test_decl/0");
> + }
> + }
> +
> /* poly_int. */
> {
> temp_dump_context tmp (with_optinfo, true,
> diff --git a/gcc/testsuite/gcc.dg/format/gcc_diag-10.c
> b/gcc/testsuite/gcc.dg/format/gcc_diag-10.c
> index 2f6a002..97a1993 100644
> --- a/gcc/testsuite/gcc.dg/format/gcc_diag-10.c
> +++ b/gcc/testsuite/gcc.dg/format/gcc_diag-10.c
> @@ -20,7 +20,7 @@ typedef union tree_node *tree;
> typedef struct gimple gimple;
>
> /* Likewise for gimple. */
> -typedef struct gimple gimple;
> +typedef struct cgraph_node cgraph_node;
>
> #define FORMAT(kind) __attribute__ ((format (__gcc_## kind ##__, 1, 2)))
>
> @@ -162,7 +162,7 @@ void test_cxxdiag (tree t, gimple *gc)
> cxxdiag ("%<%X%>", t);
> }
>
> -void test_dump (tree t, gimple *stmt)
> +void test_dump (tree t, gimple *stmt, cgraph_node *node)
> {
> dump ("%<"); /* { dg-warning "unterminated quoting directive" } */
> dump ("%>"); /* { dg-warning "unmatched quoting directive " } */
> @@ -182,4 +182,5 @@ void test_dump (tree t, gimple *stmt)
> dump ("%E", stmt);
> dump ("%T", t);
> dump ("%G", stmt);
> + dump ("%C", node);
> }
> --
> 1.8.5.3
>
