From: Tom de Vries <tdevries@suse.de>
To: gdb-patches@sourceware.org
Cc: Pedro Alves <pedro@palves.net>, Keith Seitz <keiths@redhat.com>,
blarsen@redhat.com
Subject: [PATCH][gdb/c++] Print destructor the same for gcc and clang
Date: Tue, 27 Sep 2022 12:35:48 +0200 [thread overview]
Message-ID: <20220927103546.GA27362@delia.home> (raw)
Hi,
Consider the test-case contained in this patch.
With g++ (7.5.0) we have for "ptype A":
...
type = class A {
public:
int a;
A(void);
~A();
}
...
and with clang++ (13.0.1):
...
type = class A {
public:
int a;
A(void);
~A(void);
}
...
and we observe that the destructor is printed differently.
There's a difference in debug info between the two cases: in the clang case,
there's one artificial parameter, but in the g++ case, there are two, and
these similar cases are handled differently in cp_type_print_method_args.
This is due to this slightly convoluted bit of code:
...
i = staticp ? 0 : 1;
if (nargs > i)
{
while (i < nargs)
...
}
else if (varargs)
gdb_printf (stream, "...");
else if (language == language_cplus)
gdb_printf (stream, "void");
...
The purpose of "i = staticp ? 0 : 1" is to skip the printing of the implicit
this parameter.
In commit 5f4d1085085 ("c++/8218: Destructors w/arguments"), skipping of other
artificial parameters was added, but using a different method: rather than
adjusting the potential loop start, it skips the parameter in the loop.
The observed difference in printing is explained by whether we enter the loop:
- in the clang case, the loop is not entered and we print "void".
- in the gcc case, the loop is entered, and nothing is printed.
Fix this by rewriting the code to:
- always enter the loop
- handle whether arguments need printing in the loop
- keep track of how many arguments are printed, and
use that after the loop to print void etc.
such that we have the same for both gcc and clang:
...
A(void);
~A(void);
...
Note that I consider the discussion of whether we want to print:
- A(void) / ~A(void), or
- A() / ~A()
out-of-scope for this patch.
Tested on x86_64-linux.
Any comments?
Thanks,
- Tom
[gdb/c++] Print destructor the same for gcc and clang
---
gdb/c-typeprint.c | 55 +++++++++++++++++-------------
gdb/testsuite/gdb.base/ptype-offsets.exp | 2 +-
gdb/testsuite/gdb.cp/print-method-args.cc | 38 +++++++++++++++++++++
gdb/testsuite/gdb.cp/print-method-args.exp | 36 +++++++++++++++++++
gdb/testsuite/gdb.cp/templates.exp | 4 +--
5 files changed, 109 insertions(+), 26 deletions(-)
diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index 3a611cdac5d..23e8a5a4d6f 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -273,35 +273,44 @@ cp_type_print_method_args (struct type *mtype, const char *prefix,
language_cplus, DMGL_ANSI);
gdb_puts ("(", stream);
- /* Skip the class variable. We keep this here to accommodate older
- compilers and debug formats which may not support artificial
- parameters. */
- i = staticp ? 0 : 1;
- if (nargs > i)
+ int printed_args = 0;
+ for (i = 0; i < nargs; ++i)
{
- while (i < nargs)
+ if (i == 0 && !staticp)
{
- struct field arg = args[i++];
-
- /* Skip any artificial arguments. */
- if (FIELD_ARTIFICIAL (arg))
- continue;
+ /* Skip the class variable. We keep this here to accommodate older
+ compilers and debug formats which may not support artificial
+ parameters. */
+ continue;
+ }
- c_print_type (arg.type (), "", stream, 0, 0, language, flags);
+ struct field arg = args[i];
+ /* Skip any artificial arguments. */
+ if (FIELD_ARTIFICIAL (arg))
+ continue;
- if (i == nargs && varargs)
- gdb_printf (stream, ", ...");
- else if (i < nargs)
- {
- gdb_printf (stream, ", ");
- stream->wrap_here (8);
- }
+ if (printed_args > 0)
+ {
+ gdb_printf (stream, ", ");
+ stream->wrap_here (8);
}
+
+ c_print_type (arg.type (), "", stream, 0, 0, language, flags);
+ printed_args++;
+ }
+
+ if (varargs)
+ {
+ if (printed_args == 0)
+ gdb_printf (stream, "...");
+ else
+ gdb_printf (stream, ", ...");
+ }
+ else if (printed_args == 0)
+ {
+ if (language == language_cplus)
+ gdb_printf (stream, "void");
}
- else if (varargs)
- gdb_printf (stream, "...");
- else if (language == language_cplus)
- gdb_printf (stream, "void");
gdb_printf (stream, ")");
diff --git a/gdb/testsuite/gdb.base/ptype-offsets.exp b/gdb/testsuite/gdb.base/ptype-offsets.exp
index e80d876fa32..eb41bafb3e6 100644
--- a/gdb/testsuite/gdb.base/ptype-offsets.exp
+++ b/gdb/testsuite/gdb.base/ptype-offsets.exp
@@ -104,7 +104,7 @@ gdb_test "ptype /oTM struct abc" \
"/* 48 | 2 */ my_int_type field9;" \
"" \
" abc(void);" \
-" ~abc();" \
+" ~abc(void);" \
"" \
" typedef short my_int_type;" \
"/* XXX 6-byte padding */" \
diff --git a/gdb/testsuite/gdb.cp/print-method-args.cc b/gdb/testsuite/gdb.cp/print-method-args.cc
new file mode 100644
index 00000000000..98be8286aeb
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/print-method-args.cc
@@ -0,0 +1,38 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright (C) 2022 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ 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/>. */
+
+class A {
+public:
+ int a;
+
+ A() {
+ this->a = 3;
+ }
+ ~A() {
+ a = 0;
+ }
+};
+
+int
+main (void)
+{
+ A a;
+
+ return a.a;
+}
diff --git a/gdb/testsuite/gdb.cp/print-method-args.exp b/gdb/testsuite/gdb.cp/print-method-args.exp
new file mode 100644
index 00000000000..7df044711b0
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/print-method-args.exp
@@ -0,0 +1,36 @@
+# Copyright (C) 2022 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 is part of the gdb testsuite.
+
+if { [skip_cplus_tests] } { continue }
+
+standard_testfile .cc
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
+ return -1
+}
+
+set re \
+ [multi_line \
+ "type = class A {" \
+ " public:" \
+ " int a;" \
+ "" \
+ " A\\(void\\);" \
+ " ~A\\(void\\);" \
+ "}"]
+
+gdb_test "ptype A" $re
diff --git a/gdb/testsuite/gdb.cp/templates.exp b/gdb/testsuite/gdb.cp/templates.exp
index 27c9bbc1c7e..d3ede5a5fb6 100644
--- a/gdb/testsuite/gdb.cp/templates.exp
+++ b/gdb/testsuite/gdb.cp/templates.exp
@@ -68,7 +68,7 @@ proc test_ptype_of_templates {} {
# The destructor has an argument type.
kfail "gdb/8218" "ptype T5<int>"
}
- -re "type = class T5<int> \{${ws}public:${ws}static int X;${ws}int x;${ws}int val;${ws}T5\\(int\\);${ws}T5\\((T5<int> const|const T5<int>) ?&\\);${ws}~T5\\(\\);${ws}static void \\* operator new\\((size_t|unsigned( int| long|))\\);${ws}static void operator delete\\(void ?\\*\\);${ws}int value\\((void|)\\);${ws}\}\r\n$gdb_prompt $" {
+ -re "type = class T5<int> \{${ws}public:${ws}static int X;${ws}int x;${ws}int val;${ws}T5\\(int\\);${ws}T5\\((T5<int> const|const T5<int>) ?&\\);${ws}~T5\\(void\\);${ws}static void \\* operator new\\((size_t|unsigned( int| long|))\\);${ws}static void operator delete\\(void ?\\*\\);${ws}int value\\((void|)\\);${ws}\}\r\n$gdb_prompt $" {
pass "ptype T5<int>"
}
}
@@ -108,7 +108,7 @@ proc test_ptype_of_templates {} {
# The destructor has an argument type.
kfail "gdb/8218" "ptype t5i"
}
- -re "type = class T5<int> \{${ws}public:${ws}static int X;${ws}int x;${ws}int val;${ws}T5\\(int\\);${ws}T5\\((T5<int> const|const T5<int>) ?&\\);${ws}~T5\\(\\);${ws}static void \\* operator new\\((size_t|unsigned( int| long|))\\);${ws}static void operator delete\\(void ?\\*\\);${ws}int value\\((void|)\\);${ws}\}\r\n$gdb_prompt $" {
+ -re "type = class T5<int> \{${ws}public:${ws}static int X;${ws}int x;${ws}int val;${ws}T5\\(int\\);${ws}T5\\((T5<int> const|const T5<int>) ?&\\);${ws}~T5\\(void\\);${ws}static void \\* operator new\\((size_t|unsigned( int| long|))\\);${ws}static void operator delete\\(void ?\\*\\);${ws}int value\\((void|)\\);${ws}\}\r\n$gdb_prompt $" {
pass "ptype t5i"
}
}
next reply other threads:[~2022-09-27 10:35 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-27 10:35 Tom de Vries [this message]
2022-09-27 14:16 ` Bruno Larsen
2022-09-29 17:52 ` Keith Seitz
2022-09-29 20:03 ` Pedro Alves
2022-09-29 20:01 ` Pedro Alves
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220927103546.GA27362@delia.home \
--to=tdevries@suse.de \
--cc=blarsen@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=keiths@redhat.com \
--cc=pedro@palves.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).