* Re: [PATCH 1/1] replace_typedefs: handle templates in namespaces
2020-05-28 22:13 ` Keith Seitz
@ 2020-05-30 15:34 ` Pedro Alves
0 siblings, 0 replies; 4+ messages in thread
From: Pedro Alves @ 2020-05-30 15:34 UTC (permalink / raw)
To: Keith Seitz, gdb-patches
On 5/28/20 11:13 PM, Keith Seitz via Gdb-patches wrote:
> On 5/28/20 6:37 AM, Pedro Alves via Gdb-patches wrote:
>> gdb/ChangeLog:
>> 2020-05-28 Pedro Alves <palves@redhat.com>
>>
>> * cp-support.c (replace_typedefs_template): New.
>> (replace_typedefs_qualified_name): Handle
>> DEMANGLE_COMPONENT_TEMPLATE.
>>
>> gdb/testsuite/ChangeLog:
>> 2020-05-28 Pedro Alves <palves@redhat.com>
>>
>> * gdb.linespec/cp-replace-typedefs-ns-template.cc: New.
>> * gdb.linespec/cp-replace-typedefs-ns-template.exp: New.
>
> This is very close to where I was headed, so AFAICT, it LGTM.
Great, thanks. I've pushed it, with some clarifications to
the commit log, and some more tweaking to the testcase, to
reflect further investigation of the issues I had left open
in the prior version. Let me clarify.
About:
On 5/28/20 2:37 PM, Pedro Alves via Gdb-patches wrote:
> The handling for template aliases was a nice surprise. I coded this
> while testing against GCC, and wrote it in the way that I thought I
> should work. But, it didn't really work, because GDB doesn't find a
> typedef for "NS1::NS2::AliasTempl<int>". So I KFAIL it. Then I
> tested against Clang, and that test KPASSed. Eh, cool, nice
> surprised. I haven't dug into the debug info yet to figure out
> exactly what is different. I did test both GCC 7.3 and GCC 10, and I
> get the same results. the Clang I used was version 5.0.2, the version
> shipping with Fedora 27.
I looked into the debug info, and the difference is that GCC emits
a typedef of the template alias without the template parameter info, while
Clang includes the template parameter info in the typedef name.
So we get a "AliasTempl" typedef with GCC, and a "AliasTempl<int>"
with Clang. I filed a GCC bug about it, and updated the xfail reference in
the new test to refer to it. See:
[DW_TAG_typedef for template alias missing template type parameters]
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95437
As for:
> There are some differences in the resolved canonical names in GCC vs
> Clang (this is visible in "info breakpoints" output, which is what
> check_bp_locations_match_list uses internally). In some cases w/ GCC
> we seem to keep the typedef, while w/ Clang we always seem to end up
> with the resolved type. I'm not sure what to make of that.
I figured out when does GDB print the typedef vs the resolved type.
GDB prints the resolved type ("object*") in the canonical name when the
symbol has external linkage, and with the typedef ("object_p") if the
symbol has internal linkage. And the reason for the extern vs static
difference is that GCC emits a DW_AT_linkage_name for the extern symbol, but
not for the static symbol. When there's no linkage_name,
gdb/dwarf/read.c:dwarf2_physname (via new_symbol) calls
dwarf2_compute_name:
if (canon == NULL || check_physname)
{
const char *physname = dwarf2_compute_name (name, die, cu, 1);
while if there's a linkage name available, GDB prefers to use it
(in demangled form) (canon is not NULL). The linkage name has no
typedefs in it, of course. The computed name has typedefs in it.
And then the difference between GCC and Clang is that Clang always
emits the DW_AT_linkage_name, even for static symbols. E.g., with:
typedef int *object_p;
static void static_function (object_p) {}
void extern_function (object_p) {}
int
main ()
{
static_function (0);
extern_function (0);
return 0;
}
We get:
$ readelf --debug /tmp/cp-replace-typedefs-ns-template.o.gcc | grep DW_ | grep _function
<67> DW_AT_name : (indirect string, offset: 0x1a00): extern_function
<6d> DW_AT_linkage_name: (indirect string, offset: 0xadd): _Z15extern_functionPi
<91> DW_AT_name : (indirect string, offset: 0x2003): static_function
$ readelf --debug /tmp/cp-replace-typedefs-ns-template.o.clang | grep DW_ | grep _function
<39> DW_AT_linkage_name: (indirect string, offset: 0xa0): _Z15extern_functionPi
<3d> DW_AT_name : (indirect string, offset: 0xb6): extern_function
<76> DW_AT_linkage_name: (indirect string, offset: 0xcf): _ZL15static_functionPi
<7a> DW_AT_name : (indirect string, offset: 0xe6): static_function
Since this isn't the point of the testcase, I made all functions extern,
which eliminates the GCC vs Clang difference, simplifying the testcase.
Here's what I pushed.
From f68f85b52b2897ba54e0b119322be1abb2d53afe Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
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<typename T> struct Templ1
{
T x;
Templ1 (object_p) {}
}} // namespace NS1::NS2
using Templ1 = NS1::NS2::Templ1<unsigned>;
Setting a breakpoint like so:
(gdb) break NS1::NS2::Templ1<int>::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<int>::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<unsigned>" 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<int>*)
d_dump tree for NS1::NS2::grab_it(NS1::NS2::Templ1<int>*):
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<int>". 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 <palves@redhat.com>
* cp-support.c (replace_typedefs_template): New.
(replace_typedefs_qualified_name): Handle
DEMANGLE_COMPONENT_TEMPLATE.
gdb/testsuite/ChangeLog:
2020-05-30 Pedro Alves <palves@redhat.com>
* gdb.linespec/cp-replace-typedefs-ns-template.cc: New.
* gdb.linespec/cp-replace-typedefs-ns-template.exp: New.
---
gdb/ChangeLog | 6 +
gdb/testsuite/ChangeLog | 5 +
gdb/cp-support.c | 76 ++++++++++++-
.../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 <palves@redhat.com>
+
+ * cp-support.c (replace_typedefs_template): New.
+ (replace_typedefs_qualified_name): Handle
+ DEMANGLE_COMPONENT_TEMPLATE.
+
2020-05-29 Simon Marchi <simon.marchi@efficios.com>
* dwarf2/comp-unit.c, dwarf2/comp-unit.h, dwarf2/index-cache.c,
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 <palves@redhat.com>
+
+ * gdb.linespec/cp-replace-typedefs-ns-template.cc: New.
+ * gdb.linespec/cp-replace-typedefs-ns-template.exp: New.
+
2020-05-29 Gary Benson <gbenson@redhat.com>
* gdb.compile/compile-cplus.exp (additional_flags): Also
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<char> 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<typename T> using alias = base<int, t>;
+ in which case we want inspect_type to do a replacement like:
+ alias<int> -> base<int, int>
+ */
+ }
+
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/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 <http://www.gnu.org/licenses/>. */
+
+namespace NS1 {
+
+namespace NS2 {
+
+struct object
+{
+ object ()
+ {
+ }
+};
+
+typedef object *object_p;
+
+template<typename T>
+struct Templ1
+{
+ explicit Templ1 (object_p)
+ {
+ }
+
+ template<typename I>
+ static void
+ static_method (object_p)
+ {
+ }
+};
+
+template<typename T, typename U>
+struct Templ2
+{
+ explicit Templ2 (object_p)
+ {
+ }
+
+ template<typename I>
+ static void
+ static_method (object_p)
+ {
+ }
+};
+
+template<typename T> using AliasTempl = Templ2<int, T>;
+
+typedef Templ1<int> 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<unsigned>;
+using Templ2 = NS1::NS2::Templ2<long, long>;
+using AliasTempl = NS1::NS2::AliasTempl<int>;
+
+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<int>::static_method<int> (0);
+ NS1::NS2::AliasTempl<int>::static_method<int> (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 <http://www.gnu.org/licenses/>.
+
+# 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<unsigned int> .*"
+gdb_test "ptype AliasTempl" "type = struct NS1::NS2::Templ2<int, int> .*"
+
+# 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<int>::Tem" \
+ "break NS1::NS2::Templ1<int>::Templ1(NS1::NS2::object*)"
+check_bp "NS1::NS2::Templ1<int>::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<int>::static_method<int>(NS1::NS2::object*)"
+ "NS1::NS2::Templ1<int>::static_method<NS2>(NS1::NS2::object*)"
+ "NS1::NS2::Templ1<NS2>::static_method<int>(NS1::NS2::object*)"
+ "NS1::NS2::Templ1<NS2>::static_method<NS2>(NS1::NS2::object*)"
+} {
+ check_bp $loc "NS1::NS2::Templ1<int>::static_method<int>(NS1::NS2::object*)"
+}
+
+foreach loc {
+ "NS1::NS2::Templ2<int, int>::static_method<int>(NS1::NS2::object*)"
+ "NS1::NS2::Templ2<int, int>::static_method<int>(NS1::NS2::object_p)"
+} {
+ check_bp $loc "NS1::NS2::Templ2<int, int>::static_method<int>(NS1::NS2::object*)"
+}
+
+# Check that GDB expands the "NS1::NS2::AliasTempl<int>" as
+# "NS1::NS2::Templ2<int, int>".
+foreach loc {
+ "NS1::NS2::AliasTempl<int>::static_method<int>(NS1::NS2::object*)"
+ "NS1::NS2::AliasTempl<int>::static_method<int>(NS1::NS2::object_p)"
+} {
+ if [test_compiler_info gcc*] {
+ # While Clang emits "AliasTempl<int>" (etc.) typedefs, GCC
+ # emits "AliasTempl" typedefs with no template parameter info.
+ setup_xfail gcc/95437 *-*-*
+ }
+ check_bp $loc "NS1::NS2::Templ2<int, int>::static_method<int>(NS1::NS2::object*)"
+
+ # Check that setting the breakpoint with GCC really failed,
+ # instead of succeeding with e.g., "AliasTempl<int>" 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<int>*)"
+} {
+ check_bp $loc "NS1::NS2::int_Templ1_t_func(NS1::NS2::Templ1<int>*)"
+}
base-commit: bb6e246742f8795aacfbea2401e505e97c079ffa
--
2.14.5
^ permalink raw reply [flat|nested] 4+ messages in thread