[PATCH][gdb/c++] Print destructor the same for gcc and clang
Bruno Larsen
blarsen@redhat.com
Tue Sep 27 14:16:24 GMT 2022
On 27/09/2022 12:35, Tom de Vries wrote:
> 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
Hi Tom,
I am ok with these changes, they make the code easier to read and easier
to change, based on the output of the A()/A(void) discussion.
Cheers,
Bruno
> ---
> 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"
> }
> }
>
More information about the Gdb-patches
mailing list