* RFC: handle case arising from GCC PR 47510
@ 2011-02-02 20:21 Tom Tromey
2011-02-02 21:12 ` Jan Kratochvil
0 siblings, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2011-02-02 20:21 UTC (permalink / raw)
To: gdb-patches
I plan to check this in.
GCC PR 47510 concerns an odd case in C++, where a typedef can give a
name to an anonymous struct. This matters because the struct may have a
synthesized constructor and destructor; for gdb to handle this properly
we have to also give the name to the anonymous struct.
I think the pending GCC patch does this in way that is difficult for
debuggers. I think it would probably be better to just give up on
exactly representing this case and instead just emit a struct type with
a name.
However, others disagree, so in case the GCC patch goes in, I came up
with this patch to change gdb to deal with the resulting DWARF.
The valops.c part of the patch is, as far as I can tell, a latent bug
that ought to be committed regardless. Without it, it is possible to
end up with j==-1, causing us to read invalid memory.
Built and regtested on x86-64 (compile farm).
New test included, though of course it will only trigger with the
correct version of GCC.
Tom
2011-02-02 Tom Tromey <tromey@redhat.com>
* valops.c (value_struct_elt_for_reference): Refine artificial
type logic. Call error if j==-1.
* dwarf2read.c (maybe_smash_one_typedef): New function.
(process_die) <DW_TAG_typedef>: Use maybe_smash_one_typedef.
2011-02-02 Tom Tromey <tromey@redhat.com>
* gdb.cp/anon-struct.cc: New file.
* gdb.cp/anon-struct.exp: New file.
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 6a98d57..c894732a 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -4621,6 +4621,40 @@ process_full_comp_unit (struct dwarf2_per_cu_data *per_cu)
do_cleanups (back_to);
}
+/* Fix up a typedef to an anonymous structure, for C++.
+ Return 1 if the smashing was done, 0 otherwise.
+
+ In C++, a typedef can give a name to an anonymous structure. See
+ http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47510.
+
+ We detect this situation and smash a name into the anonymous
+ struct. */
+
+static int
+maybe_smash_one_typedef (struct type *typedef_type, const char *base_name)
+{
+ struct type *struct_type = TYPE_TARGET_TYPE (typedef_type);
+ int i;
+
+ for (i = 0; i < TYPE_NFN_FIELDS (struct_type); ++i)
+ {
+ if (TYPE_FN_FIELDLIST_NAME (struct_type, i)
+ && TYPE_FN_FIELDLIST_NAME (struct_type, i)[0] == '~'
+ && strcmp (TYPE_FN_FIELDLIST_NAME (struct_type, i) + 1,
+ base_name) == 0)
+ {
+ /* Found a destructor with the same name as our typedef, so
+ proceed with smashing. */
+ TYPE_TAG_NAME (struct_type) = TYPE_NAME (typedef_type);
+ TYPE_NAME (struct_type) = TYPE_NAME (typedef_type);
+
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
/* Process a die and its children. */
static void
@@ -4669,11 +4703,37 @@ process_die (struct die_info *die, struct dwarf2_cu *cu)
case DW_TAG_base_type:
case DW_TAG_subrange_type:
- case DW_TAG_typedef:
/* Add a typedef symbol for the type definition, if it has a
DW_AT_name. */
new_symbol (die, read_type_die (die, cu), cu);
break;
+
+ case DW_TAG_typedef:
+ {
+ struct type *this_type = read_type_die (die, cu);
+
+ if (cu->language == language_cplus
+ && TYPE_CODE (TYPE_TARGET_TYPE (this_type)) == TYPE_CODE_STRUCT
+ && TYPE_TAG_NAME (TYPE_TARGET_TYPE (this_type)) == NULL
+ && maybe_smash_one_typedef (this_type, dwarf2_name (die, cu)))
+ {
+ /* We turned this typedef into a real structure type, so
+ make a pretend DIE to get new_symbol to do the right
+ thing. */
+ struct die_info fake = *die;
+
+ fake.tag = DW_TAG_structure_type;
+ new_symbol (&fake, TYPE_TARGET_TYPE (this_type), cu);
+ }
+ else
+ {
+ /* Add a typedef symbol for the type definition, if it has
+ a DW_AT_name. */
+ new_symbol (die, this_type, cu);
+ }
+ }
+ break;
+
case DW_TAG_common_block:
read_common_block (die, cu);
break;
diff --git a/gdb/testsuite/gdb.cp/anon-struct.cc b/gdb/testsuite/gdb.cp/anon-struct.cc
new file mode 100644
index 0000000..a266d90
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/anon-struct.cc
@@ -0,0 +1,15 @@
+class C {
+public:
+ C() {}
+ ~C() {}
+};
+
+typedef struct {
+ C m;
+} t;
+typedef t S;
+S v;
+
+int main()
+{
+}
diff --git a/gdb/testsuite/gdb.cp/anon-struct.exp b/gdb/testsuite/gdb.cp/anon-struct.exp
new file mode 100644
index 0000000..a250d19
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/anon-struct.exp
@@ -0,0 +1,25 @@
+# Tests for anonymous union support.
+# Copyright 2011 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/>.
+
+set testfile anon-struct
+set srcfile ${testfile}.cc
+set binfile ${objdir}/${subdir}/${testfile}
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {debug c++}] } {
+ return -1
+}
+
+gdb_test "ptype t::t" "type = void \\(t \\* const\\)" \
+ "print type of t::t"
diff --git a/gdb/valops.c b/gdb/valops.c
index 24c2269..0287092 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -3238,25 +3238,32 @@ value_struct_elt_for_reference (struct type *domain, int offset,
int ii;
j = -1;
- for (ii = 0; ii < TYPE_FN_FIELDLIST_LENGTH (t, i);
- ++ii)
+ for (ii = 0; ii < len; ++ii)
{
/* Skip artificial methods. This is necessary if,
for example, the user wants to "print
subclass::subclass" with only one user-defined
- constructor. There is no ambiguity in this
- case. */
+ constructor. There is no ambiguity in this case.
+ We are careful here to allow artificial methods
+ if they are the unique result. */
if (TYPE_FN_FIELD_ARTIFICIAL (f, ii))
- continue;
+ {
+ if (j == -1)
+ j = ii;
+ continue;
+ }
/* Desired method is ambiguous if more than one
method is defined. */
- if (j != -1)
+ if (j != -1 && !TYPE_FN_FIELD_ARTIFICIAL (f, j))
error (_("non-unique member `%s' requires "
"type instantiation"), name);
j = ii;
}
+
+ if (j == -1)
+ error (_("no matching member function"));
}
if (TYPE_FN_FIELD_STATIC_P (f, j))
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: RFC: handle case arising from GCC PR 47510
2011-02-02 20:21 RFC: handle case arising from GCC PR 47510 Tom Tromey
@ 2011-02-02 21:12 ` Jan Kratochvil
2011-02-02 21:47 ` Michael Snyder
2011-02-02 22:04 ` RFC: handle case arising from GCC PR 47510 Tom Tromey
0 siblings, 2 replies; 8+ messages in thread
From: Jan Kratochvil @ 2011-02-02 21:12 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
Hi Tom,
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
[...]
> + if (cu->language == language_cplus
> + && TYPE_CODE (TYPE_TARGET_TYPE (this_type)) == TYPE_CODE_STRUCT
> + && TYPE_TAG_NAME (TYPE_TARGET_TYPE (this_type)) == NULL
> + && maybe_smash_one_typedef (this_type, dwarf2_name (die, cu)))
> + {
> + /* We turned this typedef into a real structure type, so
> + make a pretend DIE to get new_symbol to do the right
> + thing. */
> + struct die_info fake = *die;
fake.attrs are invalid now while being accessed by new_symbol.
> +
> + fake.tag = DW_TAG_structure_type;
> + new_symbol (&fake, TYPE_TARGET_TYPE (this_type), cu);
> + }
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/anon-struct.cc
> @@ -0,0 +1,15 @@
> +class C {
No copyright needed?
> +public:
> + C() {}
> + ~C() {}
If the destructor is not present maybe_smash_one_typedef() will not work. And
GDB crashes now due to it, that should be sanity-protected anyway.
> +};
> +
> +typedef struct {
> + C m;
> +} t;
> +typedef t S;
> +S v;
I would remove the intermediate type `S', it was there fore more illustrative
purposes.
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/anon-struct.exp
> @@ -0,0 +1,25 @@
> +# Tests for anonymous union support.
> +# Copyright 2011 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/>.
> +
> +set testfile anon-struct
> +set srcfile ${testfile}.cc
> +set binfile ${objdir}/${subdir}/${testfile}
> +if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {debug c++}] } {
> + return -1
> +}
> +
> +gdb_test "ptype t::t" "type = void \\(t \\* const\\)" \
> + "print type of t::t"
> diff --git a/gdb/valops.c b/gdb/valops.c
> index 24c2269..0287092 100644
> --- a/gdb/valops.c
> +++ b/gdb/valops.c
> @@ -3238,25 +3238,32 @@ value_struct_elt_for_reference (struct type *domain, int offset,
> j = -1;
> - for (ii = 0; ii < TYPE_FN_FIELDLIST_LENGTH (t, i);
> - ++ii)
> + for (ii = 0; ii < len; ++ii)
> {
> /* Skip artificial methods. This is necessary if,
> for example, the user wants to "print
> subclass::subclass" with only one user-defined
> - constructor. There is no ambiguity in this
> - case. */
> + constructor. There is no ambiguity in this case.
> + We are careful here to allow artificial methods
> + if they are the unique result. */
> if (TYPE_FN_FIELD_ARTIFICIAL (f, ii))
> - continue;
> + {
> + if (j == -1)
> + j = ii;
> + continue;
> + }
>
> /* Desired method is ambiguous if more than one
> method is defined. */
> - if (j != -1)
> + if (j != -1 && !TYPE_FN_FIELD_ARTIFICIAL (f, j))
> error (_("non-unique member `%s' requires "
> "type instantiation"), name);
>
> j = ii;
> }
> +
> + if (j == -1)
> + error (_("no matching member function"));
> }
I do not fully grok this change, it goes half way. Why two artificial methods
are not non-unique?
Thanks,
Jan
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: RFC: handle case arising from GCC PR 47510
2011-02-02 21:12 ` Jan Kratochvil
@ 2011-02-02 21:47 ` Michael Snyder
2011-02-02 21:50 ` copyright in testcase .cc [Re: RFC: handle case arising from GCC PR 47510] Jan Kratochvil
2011-02-02 22:04 ` RFC: handle case arising from GCC PR 47510 Tom Tromey
1 sibling, 1 reply; 8+ messages in thread
From: Michael Snyder @ 2011-02-02 21:47 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: Tom Tromey, gdb-patches
Jan Kratochvil wrote:
>> +++ b/gdb/testsuite/gdb.cp/anon-struct.cc
>> @@ -0,0 +1,15 @@
>> +class C {
>
> No copyright needed?
Although the majority of sample programs in the testsuite do have
copyright notices, some of the older ones don't.
^ permalink raw reply [flat|nested] 8+ messages in thread* copyright in testcase .cc [Re: RFC: handle case arising from GCC PR 47510]
2011-02-02 21:47 ` Michael Snyder
@ 2011-02-02 21:50 ` Jan Kratochvil
2011-02-02 21:58 ` Tom Tromey
0 siblings, 1 reply; 8+ messages in thread
From: Jan Kratochvil @ 2011-02-02 21:50 UTC (permalink / raw)
To: Michael Snyder; +Cc: Tom Tromey, gdb-patches
On Wed, 02 Feb 2011 22:46:58 +0100, Michael Snyder wrote:
> Jan Kratochvil wrote:
>
> >>+++ b/gdb/testsuite/gdb.cp/anon-struct.cc
> >>@@ -0,0 +1,15 @@
> >>+class C {
> >
> >No copyright needed?
>
> Although the majority of sample programs in the testsuite do have
> copyright notices, some of the older ones don't.
In fact I know that. This still does not answer my question.
(I have seen Tom's approval of similar other patches so it may be OK.)
Thanks,
Jan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: RFC: handle case arising from GCC PR 47510
2011-02-02 21:12 ` Jan Kratochvil
2011-02-02 21:47 ` Michael Snyder
@ 2011-02-02 22:04 ` Tom Tromey
2011-02-03 8:59 ` Jan Kratochvil
1 sibling, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2011-02-02 22:04 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
Tom> + struct die_info fake = *die;
Jan> fake.attrs are invalid now while being accessed by new_symbol.
Thanks, I forgot about that.
I fixed it by temporarily changing die->tag.
Tom> +++ b/gdb/testsuite/gdb.cp/anon-struct.cc
Jan> No copyright needed?
Fixed.
Tom> +public:
Tom> + C() {}
Tom> + ~C() {}
Jan> If the destructor is not present maybe_smash_one_typedef() will not
Jan> work. And GDB crashes now due to it, that should be
Jan> sanity-protected anyway.
The class C is not the problem in this test case. I think C just exists
to make sure that the "anonymous" struct is not a POD.
maybe_smash_one_typedef won't be called for C, because C has a name.
I don't understand about GDB crashing now due to C.
Jan> I would remove the intermediate type `S', it was there fore more
Jan> illustrative purposes.
Done.
Jan> I do not fully grok this change, it goes half way. Why two
Jan> artificial methods are not non-unique?
My understanding is that this loop is trying to filter out artificial
methods in a case like:
class K {
K(int) { ... }
};
Here, I think, the user can type "ptype K::K" and get "K::K(int)" --
which makes some kind of sense, ignoring the compiler-generated
K::K(void). At least, that is what I think it all means. I am not sure
this code is really correct, but this part of the patch is just avoiding
a crash.
I don't think it is possible for this loop to see two artificial
methods.
Tom
2011-02-02 Tom Tromey <tromey@redhat.com>
* valops.c (value_struct_elt_for_reference): Refine artificial
type logic. Call error if j==-1.
* dwarf2read.c (maybe_smash_one_typedef): New function.
(process_die) <DW_TAG_typedef>: Use maybe_smash_one_typedef.
2011-02-02 Tom Tromey <tromey@redhat.com>
* gdb.cp/anon-struct.cc: New file.
* gdb.cp/anon-struct.exp: New file.
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 6a98d57..61feb60 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -4621,6 +4621,40 @@ process_full_comp_unit (struct dwarf2_per_cu_data *per_cu)
do_cleanups (back_to);
}
+/* Fix up a typedef to an anonymous structure, for C++.
+ Return 1 if the smashing was done, 0 otherwise.
+
+ In C++, a typedef can give a name to an anonymous structure. See
+ http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47510.
+
+ We detect this situation and smash a name into the anonymous
+ struct. */
+
+static int
+maybe_smash_one_typedef (struct type *typedef_type, const char *base_name)
+{
+ struct type *struct_type = TYPE_TARGET_TYPE (typedef_type);
+ int i;
+
+ for (i = 0; i < TYPE_NFN_FIELDS (struct_type); ++i)
+ {
+ if (TYPE_FN_FIELDLIST_NAME (struct_type, i)
+ && TYPE_FN_FIELDLIST_NAME (struct_type, i)[0] == '~'
+ && strcmp (TYPE_FN_FIELDLIST_NAME (struct_type, i) + 1,
+ base_name) == 0)
+ {
+ /* Found a destructor with the same name as our typedef, so
+ proceed with smashing. */
+ TYPE_TAG_NAME (struct_type) = TYPE_NAME (typedef_type);
+ TYPE_NAME (struct_type) = TYPE_NAME (typedef_type);
+
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
/* Process a die and its children. */
static void
@@ -4669,11 +4703,34 @@ process_die (struct die_info *die, struct dwarf2_cu *cu)
case DW_TAG_base_type:
case DW_TAG_subrange_type:
- case DW_TAG_typedef:
/* Add a typedef symbol for the type definition, if it has a
DW_AT_name. */
new_symbol (die, read_type_die (die, cu), cu);
break;
+
+ case DW_TAG_typedef:
+ {
+ struct type *this_type = read_type_die (die, cu);
+
+ if (cu->language == language_cplus
+ && TYPE_CODE (TYPE_TARGET_TYPE (this_type)) == TYPE_CODE_STRUCT
+ && TYPE_TAG_NAME (TYPE_TARGET_TYPE (this_type)) == NULL
+ && maybe_smash_one_typedef (this_type, dwarf2_name (die, cu)))
+ {
+ /* Pretend this DIE is a structure, for new_symbol. */
+ die->tag = DW_TAG_structure_type;
+ new_symbol (die, TYPE_TARGET_TYPE (this_type), cu);
+ die->tag = DW_TAG_typedef;
+ }
+ else
+ {
+ /* Add a typedef symbol for the type definition, if it has
+ a DW_AT_name. */
+ new_symbol (die, this_type, cu);
+ }
+ }
+ break;
+
case DW_TAG_common_block:
read_common_block (die, cu);
break;
diff --git a/gdb/testsuite/gdb.cp/anon-struct.cc b/gdb/testsuite/gdb.cp/anon-struct.cc
new file mode 100644
index 0000000..d1085c9
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/anon-struct.cc
@@ -0,0 +1,33 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2011 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 C {
+public:
+ C() {}
+ ~C() {}
+};
+
+typedef struct {
+ C m;
+} t;
+
+t v;
+
+int main()
+{
+}
diff --git a/gdb/testsuite/gdb.cp/anon-struct.exp b/gdb/testsuite/gdb.cp/anon-struct.exp
new file mode 100644
index 0000000..a250d19
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/anon-struct.exp
@@ -0,0 +1,25 @@
+# Tests for anonymous union support.
+# Copyright 2011 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/>.
+
+set testfile anon-struct
+set srcfile ${testfile}.cc
+set binfile ${objdir}/${subdir}/${testfile}
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {debug c++}] } {
+ return -1
+}
+
+gdb_test "ptype t::t" "type = void \\(t \\* const\\)" \
+ "print type of t::t"
diff --git a/gdb/valops.c b/gdb/valops.c
index 24c2269..0287092 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -3238,25 +3238,32 @@ value_struct_elt_for_reference (struct type *domain, int offset,
int ii;
j = -1;
- for (ii = 0; ii < TYPE_FN_FIELDLIST_LENGTH (t, i);
- ++ii)
+ for (ii = 0; ii < len; ++ii)
{
/* Skip artificial methods. This is necessary if,
for example, the user wants to "print
subclass::subclass" with only one user-defined
- constructor. There is no ambiguity in this
- case. */
+ constructor. There is no ambiguity in this case.
+ We are careful here to allow artificial methods
+ if they are the unique result. */
if (TYPE_FN_FIELD_ARTIFICIAL (f, ii))
- continue;
+ {
+ if (j == -1)
+ j = ii;
+ continue;
+ }
/* Desired method is ambiguous if more than one
method is defined. */
- if (j != -1)
+ if (j != -1 && !TYPE_FN_FIELD_ARTIFICIAL (f, j))
error (_("non-unique member `%s' requires "
"type instantiation"), name);
j = ii;
}
+
+ if (j == -1)
+ error (_("no matching member function"));
}
if (TYPE_FN_FIELD_STATIC_P (f, j))
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: RFC: handle case arising from GCC PR 47510
2011-02-02 22:04 ` RFC: handle case arising from GCC PR 47510 Tom Tromey
@ 2011-02-03 8:59 ` Jan Kratochvil
2011-02-03 21:07 ` Tom Tromey
0 siblings, 1 reply; 8+ messages in thread
From: Jan Kratochvil @ 2011-02-03 8:59 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On Wed, 02 Feb 2011 23:03:55 +0100, Tom Tromey wrote:
> Tom> +public:
> Tom> + C() {}
> Tom> + ~C() {}
>
> Jan> If the destructor is not present maybe_smash_one_typedef() will not
> Jan> work. And GDB crashes now due to it, that should be
> Jan> sanity-protected anyway.
>
> The class C is not the problem in this test case. I think C just exists
> to make sure that the "anonymous" struct is not a POD.
>
> maybe_smash_one_typedef won't be called for C, because C has a name.
>
> I don't understand about GDB crashing now due to C.
C itself isn't a problem. But as C no longer has a destructor even `t' no
longer has an implicit destructor. Due to it maybe_smash_one_typedef gets
called for `t' but it does not do anything as it does not find the '~' field.
Then cp_lookup_nested_type crashes as TYPE_TAG_NAME (parent_type) == NULL
- which is correct for anonymous struct - but cp_lookup_nested_type does not
expect it.
> Jan> I do not fully grok this change, it goes half way. Why two
> Jan> artificial methods are not non-unique?
>
> My understanding is that this loop is trying to filter out artificial
> methods in a case like:
>
> class K {
> K(int) { ... }
> };
>
> Here, I think, the user can type "ptype K::K" and get "K::K(int)" --
> which makes some kind of sense, ignoring the compiler-generated
> K::K(void). At least, that is what I think it all means. I am not sure
> this code is really correct, but this part of the patch is just avoiding
> a crash.
>
> I don't think it is possible for this loop to see two artificial
> methods.
I thought about:
class C { public: C() {} };
class CC { C cf; } cc;
class D : CC {} d;
int main () {}
built with:
GNU C++ 4.4.6 20110124 (prerelease)
producing:
nm -C file
00000000004005ac W CC::CC()
0000000000400592 W CC::CC()
GDB HEAD:
(gdb) p CC::CC
Cannot reference virtual member function "CC"
GDB with your patch:
(gdb) p CC::CC
$1 = {void (CC * const)} 0x400592 <CC::CC()>
(that is the 0x4005ac function is not shown to the user)
But this problem is not related to this patch as it happens even with
non-artificial constructors. lookup_symbol_aux_symtabs just returns the first
matching symbol. But one cannot specify demangled name for specific kind of
ctor/dtor so lookup_symbol_aux_symtabs must not error on non-unique match.
g++-4.5+ no longer generates multiple kinds of constructors during my tests so
it should not be much a real world concern anymore.
value_struct_elt_for_reference in this case sees only a single CC::CC entry as
the abstract structure itself has only one DIE for CC::CC.
So not an applicable issue for this patch.
Thanks,
Jan
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: RFC: handle case arising from GCC PR 47510
2011-02-03 8:59 ` Jan Kratochvil
@ 2011-02-03 21:07 ` Tom Tromey
0 siblings, 0 replies; 8+ messages in thread
From: Tom Tromey @ 2011-02-03 21:07 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
Jan> Then cp_lookup_nested_type crashes as TYPE_TAG_NAME (parent_type)
Jan> == NULL - which is correct for anonymous struct - but
Jan> cp_lookup_nested_type does not expect it.
[...]
After coming up with some more examples, it turns out that the DWARF
reading part of this patch is very misguided. In particular it would
fail if the typedef appeared before the anonymous struct (and GCC did
emit this when a namespace was used). And, a more complicated solution
(deferring this smashing until after all DIEs were processed) had a
problem of its own (we needed multi-level deferrals due to local types).
I've asked for a new attribute from GCC, instead, to make the problem
more tractable on the gdb side. We'll see what happens.
Meanwhile I will probably put in the valops.c change.
Tom
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-02-03 21:07 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-02 20:21 RFC: handle case arising from GCC PR 47510 Tom Tromey
2011-02-02 21:12 ` Jan Kratochvil
2011-02-02 21:47 ` Michael Snyder
2011-02-02 21:50 ` copyright in testcase .cc [Re: RFC: handle case arising from GCC PR 47510] Jan Kratochvil
2011-02-02 21:58 ` Tom Tromey
2011-02-02 22:04 ` RFC: handle case arising from GCC PR 47510 Tom Tromey
2011-02-03 8:59 ` Jan Kratochvil
2011-02-03 21:07 ` Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox