* [RFA] fix disassemble foo::bar::~bar
@ 2013-02-14 0:14 Doug Evans
2013-02-21 21:14 ` Tom Tromey
0 siblings, 1 reply; 6+ messages in thread
From: Doug Evans @ 2013-02-14 0:14 UTC (permalink / raw)
To: gdb-patches
I happened to try this and noticed it failing with
"name of destructor must equal name of class".
This is because destructor_name_p doesn't handle foo::bar
for the type's name.
Regression tested on amd64-linux.
Ok to commit?
2013-02-13 Doug Evans <dje@google.com>
* valops.c (destructor_name_p): Fix handling of classes in namespaces.
testsuite/
* gdb.cp/cp-disasm.cc: New file.
* gdb.cp/cp-disasm.exp: New file.
Index: valops.c
===================================================================
RCS file: /cvs/src/src/gdb/valops.c,v
retrieving revision 1.312
diff -u -p -r1.312 valops.c
--- valops.c 11 Feb 2013 18:05:33 -0000 1.312
+++ valops.c 14 Feb 2013 00:13:16 -0000
@@ -3160,20 +3160,28 @@ destructor_name_p (const char *name, str
{
if (name[0] == '~')
{
- const char *dname = type_name_no_tag_or_error (type);
- const char *cp = strchr (dname, '<');
+ char *type_name = xstrdup (type_name_no_tag_or_error (type));
+ char *cp = strchr (type_name, '<');
+ char *colon;
+ struct cleanup *cleanups = make_cleanup (xfree, type_name);
unsigned int len;
/* Do not compare the template part for template classes. */
- if (cp == NULL)
- len = strlen (dname);
- else
- len = cp - dname;
- if (strlen (name + 1) != len || strncmp (dname, name + 1, len) != 0)
+ if (cp != NULL)
+ *cp = '\0';
+
+ /* If TYPE_TYPE is foo::bar, we only want bar. */
+ colon = strrchr (type_name, ':');
+ if (colon != NULL)
+ type_name = colon + 1;
+
+ if (strcmp (type_name, name + 1) != 0)
error (_("name of destructor must equal name of class"));
- else
- return 1;
+
+ do_cleanups (cleanups);
+ return 1;
}
+
return 0;
}
Index: testsuite/gdb.cp/cp-disasm.cc
===================================================================
RCS file: testsuite/gdb.cp/cp-disasm.cc
diff -N testsuite/gdb.cp/cp-disasm.cc
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/cp-disasm.cc 14 Feb 2013 00:13:16 -0000
@@ -0,0 +1,44 @@
+/* This test file is part of GDB, the GNU debugger.
+
+ Copyright 2013 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/>. */
+
+class bar
+{
+ public:
+ int x;
+ bar () {}
+ ~bar () {}
+};
+
+namespace foo
+{
+ class bar
+ {
+ public:
+ int x;
+ bar () {}
+ ~bar () {}
+ };
+}
+
+bar x;
+foo::bar y;
+
+int
+main ()
+{
+ return 0;
+}
Index: testsuite/gdb.cp/cp-disasm.exp
===================================================================
RCS file: testsuite/gdb.cp/cp-disasm.exp
diff -N testsuite/gdb.cp/cp-disasm.exp
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/cp-disasm.exp 14 Feb 2013 00:13:16 -0000
@@ -0,0 +1,52 @@
+# Copyright 2013 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/>.
+
+# Test disassembling ctors and dtors.
+
+standard_testfile .cc
+
+if { [skip_cplus_tests] } { continue }
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug c++}]} {
+ return -1
+}
+
+set name "disassemble ctor"
+gdb_test_multiple "disassemble bar::bar,+1" $name {
+ -re "Dump of assembler code.*$gdb_prompt $" {
+ pass "$name"
+ }
+}
+
+set name "disassemble dtor"
+gdb_test_multiple "disassemble bar::~bar,+1" "$name" {
+ -re "Dump of assembler code.*$gdb_prompt $" {
+ pass "$name"
+ }
+}
+
+set name "disassemble namespace ctor"
+gdb_test_multiple "disassemble foo::bar::bar,+1" "$name" {
+ -re "Dump of assembler code.*$gdb_prompt $" {
+ pass "$name"
+ }
+}
+
+set name "disassemble namespace dtor"
+gdb_test_multiple "disassemble foo::bar::~bar,+1" "$name" {
+ -re "Dump of assembler code.*$gdb_prompt $" {
+ pass "$name"
+ }
+}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] fix disassemble foo::bar::~bar
2013-02-14 0:14 [RFA] fix disassemble foo::bar::~bar Doug Evans
@ 2013-02-21 21:14 ` Tom Tromey
2013-02-21 23:58 ` Keith Seitz
0 siblings, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2013-02-21 21:14 UTC (permalink / raw)
To: Doug Evans; +Cc: gdb-patches
>>>>> "Doug" == Doug Evans <dje@google.com> writes:
Doug> I happened to try this and noticed it failing with
Doug> "name of destructor must equal name of class".
Doug> This is because destructor_name_p doesn't handle foo::bar
Doug> for the type's name.
Thanks for finding this.
Doug> + char *cp = strchr (type_name, '<');
I wonder whether this kind of parsing is sufficient.
It seems like there could be confounding cases.
I wonder what would happen if we just removed destructor_name_p,
or alternatively made it work by examining the type's function fields,
looking for a match.
If you've considered the various error cases in your analysis, then I
have no problem with your patch.
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] fix disassemble foo::bar::~bar
2013-02-21 21:14 ` Tom Tromey
@ 2013-02-21 23:58 ` Keith Seitz
2013-02-22 19:15 ` Tom Tromey
0 siblings, 1 reply; 6+ messages in thread
From: Keith Seitz @ 2013-02-21 23:58 UTC (permalink / raw)
To: Tom Tromey; +Cc: Doug Evans, gdb-patches
On 02/21/2013 01:14 PM, Tom Tromey wrote:
> I wonder what would happen if we just removed destructor_name_p,
> or alternatively made it work by examining the type's function fields,
> looking for a match.
You jogged my memory...
Another option is to use the c++ name parser:
diff --git a/gdb/valops.c b/gdb/valops.c
index 93c09d8..b006b49 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -3160,7 +3160,7 @@ destructor_name_p (const char *name, struct type
*type)
{
if (name[0] == '~')
{
- const char *dname = type_name_no_tag_or_error (type);
+ const char *dname = cp_func_name (type_name_no_tag_or_error (type));
const char *cp = strchr (dname, '<');
unsigned int len;
The above patch fixes Doug's test cases and also is regression-free on
native x86_64-unknown-linux (Fedora 15).
[I note that "print foo::bar::~bar" also didn't work for the same reason.]
Keith
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] fix disassemble foo::bar::~bar
2013-02-21 23:58 ` Keith Seitz
@ 2013-02-22 19:15 ` Tom Tromey
2013-02-22 19:43 ` Keith Seitz
0 siblings, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2013-02-22 19:15 UTC (permalink / raw)
To: Keith Seitz; +Cc: Doug Evans, gdb-patches
>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:
Keith> Another option is to use the c++ name parser:
Keith> diff --git a/gdb/valops.c b/gdb/valops.c
Keith> index 93c09d8..b006b49 100644
Keith> --- a/gdb/valops.c
Keith> +++ b/gdb/valops.c
Keith> @@ -3160,7 +3160,7 @@ destructor_name_p (const char *name, struct type
Keith> *type)
Keith> {
Keith> if (name[0] == '~')
Keith> {
Keith> - const char *dname = type_name_no_tag_or_error (type);
Keith> + const char *dname = cp_func_name (type_name_no_tag_or_error (type));
Keith> const char *cp = strchr (dname, '<');
Keith> unsigned int len;
Keith> The above patch fixes Doug's test cases and also is regression-free on
Keith> native x86_64-unknown-linux (Fedora 15).
I tend to think this is preferable. At least, as long as we have to
keep destructor_name_p.
This patch is ok with me, if it includes Doug's test case, though I'd
like to hear from Doug about it as well.
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] fix disassemble foo::bar::~bar
2013-02-22 19:15 ` Tom Tromey
@ 2013-02-22 19:43 ` Keith Seitz
2013-02-22 20:00 ` Tom Tromey
0 siblings, 1 reply; 6+ messages in thread
From: Keith Seitz @ 2013-02-22 19:43 UTC (permalink / raw)
To: Tom Tromey; +Cc: Doug Evans, gdb-patches
On 02/22/2013 11:15 AM, Tom Tromey wrote:
> Keith> The above patch fixes Doug's test cases and also is regression-free on
> Keith> native x86_64-unknown-linux (Fedora 15).
>
> I tend to think this is preferable. At least, as long as we have to
> keep destructor_name_p.
I was only attempting to point out a simpler alternative. As you rightly
point out, though, it is debatable whether destructor_name_p is needed
at all. As it is right now (with Doug's or my patch) we get,
(gdb) p foo::~bar
name of destructor must equal name of class
How much more useful is that than
(gdb) p foo::~bar
There is no field named ~bar
IMO, the second error is at least as useful, if not more so, than what
we have now. Since destructor_name_p is only used in this one place, I
would urge you/Doug to consider just nuking it and the test in c-exp.y
> This patch is ok with me, if it includes Doug's test case, though I'd
> like to hear from Doug about it as well.
I left the choice up to Doug. It's his ball after all.
Keith
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] fix disassemble foo::bar::~bar
2013-02-22 19:43 ` Keith Seitz
@ 2013-02-22 20:00 ` Tom Tromey
0 siblings, 0 replies; 6+ messages in thread
From: Tom Tromey @ 2013-02-22 20:00 UTC (permalink / raw)
To: Keith Seitz; +Cc: Doug Evans, gdb-patches
>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:
Keith> (gdb) p foo::~bar
Keith> name of destructor must equal name of class
Keith> How much more useful is that than
Keith> (gdb) p foo::~bar
Keith> There is no field named ~bar
I think the difference is that with the current code, it is a parse-time
error, but if we remove destructor_name_p, then it is an evaluation-time
error. This difference matters for breakpoint conditions.
Searching the type's function fields seems like the very best, to me.
I couldn't think of a drawback of this anyhow.
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-02-22 20:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-14 0:14 [RFA] fix disassemble foo::bar::~bar Doug Evans
2013-02-21 21:14 ` Tom Tromey
2013-02-21 23:58 ` Keith Seitz
2013-02-22 19:15 ` Tom Tromey
2013-02-22 19:43 ` Keith Seitz
2013-02-22 20:00 ` Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox