From f68f85b52b2897ba54e0b119322be1abb2d53afe Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Sat, 30 May 2020 14:20:10 +0100 Subject: [PATCH] replace_typedefs: handle templates in namespaces GDB currently crashes with infinite recursion, if you set a breakpoint on a function inside a namespace that includes a template on its fully qualified name, and, the template's name is also used as typedef in the global scope that expands to a name that includes the template name in its qualified name. For example, from the testcase added by this commit: namespace NS1 { namespace NS2 { template struct Templ1 { T x; Templ1 (object_p) {} }} // namespace NS1::NS2 using Templ1 = NS1::NS2::Templ1; Setting a breakpoint like so: (gdb) break NS1::NS2::Templ1::Templ1(NS1::NS2::object*) Results in infinite recursion, with this cycle (started by cp_canonicalize_string_full) repeating until the stack is exhausted: ... #1709 0x000000000055533c in inspect_type (info=0x38ff720, ret_comp=0xd83be10, finder=0x0, data=0x0) at /home/pedro/gdb/mygit/src/gdb/cp-support.c:267 #1710 0x0000000000555a6f in replace_typedefs (info=0x38ff720, ret_comp=0xd83be10, finder=0x0, data=0x0) at /home/pedro/gdb/mygit/src/gdb/cp-support.c:475 #1711 0x0000000000555a36 in replace_typedefs (info=0x38ff720, ret_comp=0xd83be70, finder=0x0, data=0x0) at /home/pedro/gdb/mygit/src/gdb/cp-support.c:470 #1712 0x0000000000555800 in replace_typedefs_qualified_name (info=0x38ff720, ret_comp=0xd839470, finder=0x0, data=0x0) at /home/pedro/gdb/mygit/src/gdb/cp-support.c:389 #1713 0x0000000000555a8c in replace_typedefs (info=0x38ff720, ret_comp=0xd839470, finder=0x0, data=0x0) at /home/pedro/gdb/mygit/src/gdb/cp-support.c:479 ... The demangle component tree for that symbol name looks like this: d_dump tree for NS1::NS2::Templ1::Templ1(NS1::NS2::object*): typed name qualified name name 'NS1' qualified name name 'NS2' qualified name template <<<<<<<<<< name 'Templ1' template argument list builtin type int name 'Templ1' function type argument list pointer qualified name name 'NS1' qualified name name 'NS2' name 'object' The recursion starts at replace_typedefs_qualified_name, which doesn't handle the "template" node, and thus doesn't realize that the template name actually has the fully qualified name NS1::NS2::Templ1. replace_typedefs_qualified_name calls into replace_typedefs on the template node, and that ends up in inspect_type looking up for a symbol named "Templ1", which finds the global namespace typedef, which itself expands to NS1::NS2::Templ1. GDB then tries replacing typedefs in that newly expanded name, which ends up again in replace_typedefs_qualified_name, trying to expand a fully qualified name with "NS::NS2::Templ1" in its name, which results in recursion whenever the template node is reached. Fix this by teaching replace_typedefs_qualified_name how to handle template nodes. It needs handling in two places: the first spot handles the symbol above; the second spot handles a symbol like this, from the new test: (gdb) b NS1::NS2::grab_it(NS1::NS2::Templ1*) d_dump tree for NS1::NS2::grab_it(NS1::NS2::Templ1*): typed name qualified name name 'NS1' qualified name name 'NS2' name 'grab_it' function type argument list pointer qualified name name 'NS1' qualified name name 'NS2' template <<<<<<<< name 'Templ1' template argument list builtin type int What's different in this case is that the template node appears on the right child node of a qualified name, instead of on the left child. The testcase includes a test that checks whether template aliases are correctly replaced by GDB too. That fails with GCC due to GCC PR 95437, which makes GDB not know about a typedef for "NS1::NS2::AliasTempl". GCC emits a typedef named "NS1::NS2::AliasTempl" instead, with no template parameter info. The test passes with Clang (5.0.2 at least). See more details here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95437 gdb/ChangeLog: 2020-05-30 Pedro Alves * cp-support.c (replace_typedefs_template): New. (replace_typedefs_qualified_name): Handle DEMANGLE_COMPONENT_TEMPLATE. gdb/testsuite/ChangeLog: 2020-05-30 Pedro Alves * gdb.linespec/cp-replace-typedefs-ns-template.cc: New. * gdb.linespec/cp-replace-typedefs-ns-template.exp: New. --- gdb/ChangeLog | 6 + gdb/cp-support.c | 76 ++++++++++- gdb/testsuite/ChangeLog | 5 + .../cp-replace-typedefs-ns-template.cc | 101 +++++++++++++++ .../cp-replace-typedefs-ns-template.exp | 121 ++++++++++++++++++ 5 files changed, 305 insertions(+), 4 deletions(-) create mode 100644 gdb/testsuite/gdb.linespec/cp-replace-typedefs-ns-template.cc create mode 100644 gdb/testsuite/gdb.linespec/cp-replace-typedefs-ns-template.exp diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 86faab1a568..2ea60bb476d 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2020-05-30 Pedro Alves + + * cp-support.c (replace_typedefs_template): New. + (replace_typedefs_qualified_name): Handle + DEMANGLE_COMPONENT_TEMPLATE. + 2020-05-29 Simon Marchi * dwarf2/comp-unit.c, dwarf2/comp-unit.h, dwarf2/index-cache.c, diff --git a/gdb/cp-support.c b/gdb/cp-support.c index 1e54aaea3b1..11e54c272c5 100644 --- a/gdb/cp-support.c +++ b/gdb/cp-support.c @@ -294,6 +294,42 @@ inspect_type (struct demangle_parse_info *info, return 0; } +/* Helper for replace_typedefs_qualified_name to handle + DEMANGLE_COMPONENT_TEMPLATE. TMPL is the template node. BUF is + the buffer that holds the qualified name being built by + replace_typedefs_qualified_name. REPL is the node that will be + rewritten as a DEMANGLE_COMPONENT_NAME node holding the 'template + plus template arguments' name with typedefs replaced. */ + +static bool +replace_typedefs_template (struct demangle_parse_info *info, + string_file &buf, + struct demangle_component *tmpl, + struct demangle_component *repl, + canonicalization_ftype *finder, + void *data) +{ + demangle_component *tmpl_arglist = d_right (tmpl); + + /* Replace typedefs in the template argument list. */ + replace_typedefs (info, tmpl_arglist, finder, data); + + /* Convert 'template + replaced template argument list' to a string + and replace the REPL node. */ + gdb::unique_xmalloc_ptr tmpl_str = cp_comp_to_string (tmpl, 100); + if (tmpl_str == nullptr) + { + /* If something went astray, abort typedef substitutions. */ + return false; + } + buf.puts (tmpl_str.get ()); + + repl->type = DEMANGLE_COMPONENT_NAME; + repl->u.s_name.s = obstack_strdup (&info->obstack, buf.string ()); + repl->u.s_name.len = buf.size (); + return true; +} + /* Replace any typedefs appearing in the qualified name (DEMANGLE_COMPONENT_QUAL_NAME) represented in RET_COMP for the name parse given in INFO. */ @@ -314,6 +350,29 @@ replace_typedefs_qualified_name (struct demangle_parse_info *info, substituted name. */ while (comp->type == DEMANGLE_COMPONENT_QUAL_NAME) { + if (d_left (comp)->type == DEMANGLE_COMPONENT_TEMPLATE) + { + /* Convert 'template + replaced template argument list' to a + string and replace the top DEMANGLE_COMPONENT_QUAL_NAME + node. */ + if (!replace_typedefs_template (info, buf, + d_left (comp), d_left (ret_comp), + finder, data)) + return; + + buf.clear (); + d_right (ret_comp) = d_right (comp); + comp = ret_comp; + + /* Fallback to DEMANGLE_COMPONENT_NAME processing. We want + to call inspect_type for this template, in case we have a + template alias, like: + template using alias = base; + in which case we want inspect_type to do a replacement like: + alias -> base + */ + } + if (d_left (comp)->type == DEMANGLE_COMPONENT_NAME) { struct demangle_component newobj; @@ -370,11 +429,20 @@ replace_typedefs_qualified_name (struct demangle_parse_info *info, comp = d_right (comp); } - /* If the next component is DEMANGLE_COMPONENT_NAME, save the qualified - name assembled above and append the name given by COMP. Then use this - reassembled name to check for a typedef. */ + /* If the next component is DEMANGLE_COMPONENT_TEMPLATE or + DEMANGLE_COMPONENT_NAME, save the qualified name assembled above + and append the name given by COMP. Then use this reassembled + name to check for a typedef. */ - if (comp->type == DEMANGLE_COMPONENT_NAME) + if (comp->type == DEMANGLE_COMPONENT_TEMPLATE) + { + /* Replace the top (DEMANGLE_COMPONENT_QUAL_NAME) node with a + DEMANGLE_COMPONENT_NAME node containing the whole name. */ + if (!replace_typedefs_template (info, buf, comp, ret_comp, finder, data)) + return; + inspect_type (info, ret_comp, finder, data); + } + else if (comp->type == DEMANGLE_COMPONENT_NAME) { buf.write (comp->u.s_name.s, comp->u.s_name.len); diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index d4e7220b326..8c5553ee14d 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2020-05-30 Pedro Alves + + * gdb.linespec/cp-replace-typedefs-ns-template.cc: New. + * gdb.linespec/cp-replace-typedefs-ns-template.exp: New. + 2020-05-29 Gary Benson * gdb.compile/compile-cplus.exp (additional_flags): Also diff --git a/gdb/testsuite/gdb.linespec/cp-replace-typedefs-ns-template.cc b/gdb/testsuite/gdb.linespec/cp-replace-typedefs-ns-template.cc new file mode 100644 index 00000000000..fb30685f2a5 --- /dev/null +++ b/gdb/testsuite/gdb.linespec/cp-replace-typedefs-ns-template.cc @@ -0,0 +1,101 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2019-2020 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +namespace NS1 { + +namespace NS2 { + +struct object +{ + object () + { + } +}; + +typedef object *object_p; + +template +struct Templ1 +{ + explicit Templ1 (object_p) + { + } + + template + static void + static_method (object_p) + { + } +}; + +template +struct Templ2 +{ + explicit Templ2 (object_p) + { + } + + template + static void + static_method (object_p) + { + } +}; + +template using AliasTempl = Templ2; + +typedef Templ1 int_Templ1_t; + +void +object_p_func (object_p) +{ +} + +void +int_Templ1_t_func (int_Templ1_t *) +{ +} + +} // namespace NS2 + +} // namespace NS1 + +/* These typedefs have the same name as some of the components within + NS1 that they alias to, on purpose, to try to confuse GDB and cause + recursion. */ +using NS2 = int; +using object = NS1::NS2::object; +using Templ1 = NS1::NS2::Templ1; +using Templ2 = NS1::NS2::Templ2; +using AliasTempl = NS1::NS2::AliasTempl; + +NS2 ns2_int = 0; +object obj; +Templ1 templ1 (0); +NS1::NS2::int_Templ1_t int_templ1 (0); +AliasTempl alias (0); + +int +main () +{ + NS1::NS2::Templ1::static_method (0); + NS1::NS2::AliasTempl::static_method (0); + NS1::NS2::object_p_func (0); + NS1::NS2::int_Templ1_t_func (0); + + return 0; +} diff --git a/gdb/testsuite/gdb.linespec/cp-replace-typedefs-ns-template.exp b/gdb/testsuite/gdb.linespec/cp-replace-typedefs-ns-template.exp new file mode 100644 index 00000000000..590b06d34fc --- /dev/null +++ b/gdb/testsuite/gdb.linespec/cp-replace-typedefs-ns-template.exp @@ -0,0 +1,121 @@ +# Copyright 2020 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# This file tests GDB's ability to replace typedefs in C++ symbols +# when setting breakpoints, particularly around templates in +# namespaces. + +load_lib completion-support.exp + +standard_testfile .cc + +if {[prepare_for_testing "failed to prepare" $testfile $srcfile \ + {debug c++ additional_flags=-std=c++11}]} { + return -1 +} + +# Disable the completion limit for the whole testcase. +gdb_test_no_output "set max-completions unlimited" + +# Confirm that the important global namespace typedefs were indeed +# emited in the debug info. +gdb_test "ptype NS2" "type = int" +gdb_test "ptype object" "type = struct NS1::NS2::object {.*" +gdb_test "ptype Templ1" "type = struct NS1::NS2::Templ1 .*" +gdb_test "ptype AliasTempl" "type = struct NS1::NS2::Templ2 .*" + +# Wrapper around check_bp_locations_match_list that expect a single +# location in the set breakpoint, instead of a list of locations. If +# the set location isn't specified, then it is assumed to be the exact +# same as the input location. +proc check_bp {location_in {location_out ""}} { + if {$location_out == ""} { + set location_out $location_in + } + check_bp_locations_match_list "b $location_in" [list $location_out] +} + +# These used to crash GDB with infinite recursion because GDB would +# confuse the "Templ1" typedef in the global namespace with the "Templ1" +# template in within NS1::NS2. +test_gdb_complete_unique \ + "break NS1::NS2::Templ1::Tem" \ + "break NS1::NS2::Templ1::Templ1(NS1::NS2::object*)" +check_bp "NS1::NS2::Templ1::Templ1(NS1::NS2::object*)" + +# Similar test, but without a template. This would not crash. +test_gdb_complete_unique \ + "break NS1::NS2::object::obj" \ + "break NS1::NS2::object::object()" +check_bp "NS1::NS2::object::object()" + +# Test some non-template typedef replacing within a namespace. +test_gdb_complete_unique \ + "break NS1::NS2::object_p_f" \ + "break NS1::NS2::object_p_func(NS1::NS2::object*)" +check_bp \ + "NS1::NS2::object_p_func(NS1::NS2::object_p)" \ + "NS1::NS2::object_p_func(NS1::NS2::object*)" + +# Make sure the "NS2" in the template argument list is resolved as +# being a global typedef for int. +foreach loc { + "NS1::NS2::Templ1::static_method(NS1::NS2::object*)" + "NS1::NS2::Templ1::static_method(NS1::NS2::object*)" + "NS1::NS2::Templ1::static_method(NS1::NS2::object*)" + "NS1::NS2::Templ1::static_method(NS1::NS2::object*)" +} { + check_bp $loc "NS1::NS2::Templ1::static_method(NS1::NS2::object*)" +} + +foreach loc { + "NS1::NS2::Templ2::static_method(NS1::NS2::object*)" + "NS1::NS2::Templ2::static_method(NS1::NS2::object_p)" +} { + check_bp $loc "NS1::NS2::Templ2::static_method(NS1::NS2::object*)" +} + +# Check that GDB expands the "NS1::NS2::AliasTempl" as +# "NS1::NS2::Templ2". +foreach loc { + "NS1::NS2::AliasTempl::static_method(NS1::NS2::object*)" + "NS1::NS2::AliasTempl::static_method(NS1::NS2::object_p)" +} { + if [test_compiler_info gcc*] { + # While Clang emits "AliasTempl" (etc.) typedefs, GCC + # emits "AliasTempl" typedefs with no template parameter info. + setup_xfail gcc/95437 *-*-* + } + check_bp $loc "NS1::NS2::Templ2::static_method(NS1::NS2::object*)" + + # Check that setting the breakpoint with GCC really failed, + # instead of succeeding with e.g., "AliasTempl" preserved in + # the location text. If that ever happens, we'll need to update + # these tests. + if [test_compiler_info gcc*] { + check_setting_bp_fails "b $loc" + } +} + +# Check typedef substitution in a template in a qualified name in a +# function parameter list. These used to crash GDB with recursion +# around "Templ1", because there's a "Templ1" typedef in the global +# namespace. +foreach loc { + "NS1::NS2::int_Templ1_t_func(NS1::NS2::int_Templ1_t*)" + "NS1::NS2::int_Templ1_t_func(NS1::NS2::Templ1*)" +} { + check_bp $loc "NS1::NS2::int_Templ1_t_func(NS1::NS2::Templ1*)" +} -- 2.30.2