* symtab PATCH for c++/61623 (comdat issue in verify_symtab)
@ 2014-07-14 21:21 Jason Merrill
2014-07-15 9:18 ` Jan Hubicka
0 siblings, 1 reply; 4+ messages in thread
From: Jason Merrill @ 2014-07-14 21:21 UTC (permalink / raw)
To: Jan Hubicka, gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 528 bytes --]
The problem in this testcase is that we inlined the decloned constructor
into the calling thunks, so it was removed by
symtab_remove_unreachable_nodes. verify_symtab sees that it is no
longer linked to the calling thunks with same_comdat_group and complains.
Here I've changed verify_symtab to only look at TREE_PUBLIC decls, since
comdat-local symbols can be removed if they aren't needed. I also
adjusted the diagnostic so it would print the two symbols in question
rather than the same one twice. :)
OK for trunk?
[-- Attachment #2: 61623.patch --]
[-- Type: text/x-patch, Size: 1254 bytes --]
commit e29dc7a675fdbc1adce40908fda4d5408f0103a0
Author: Jason Merrill <jason@redhat.com>
Date: Mon Jul 14 16:58:57 2014 -0400
PR c++/61623
* symtab.c (verify_symtab): Don't worry about comdat-internal
symbols.
diff --git a/gcc/symtab.c b/gcc/symtab.c
index 3a59935..dc4d84d 100644
--- a/gcc/symtab.c
+++ b/gcc/symtab.c
@@ -1202,7 +1202,8 @@ verify_symtab (void)
FOR_EACH_SYMBOL (node)
{
verify_symtab_node (node);
- if (node->get_comdat_group ())
+ if (node->get_comdat_group ()
+ && TREE_PUBLIC (node->decl))
{
symtab_node **entry, *s;
bool existed;
@@ -1217,7 +1218,7 @@ verify_symtab (void)
{
error ("Two symbols with same comdat_group are not linked by the same_comdat_group list.");
dump_symtab_node (stderr, *entry);
- dump_symtab_node (stderr, s);
+ dump_symtab_node (stderr, node);
internal_error ("verify_symtab failed");
}
}
diff --git a/gcc/testsuite/g++.dg/opt/declone2.C b/gcc/testsuite/g++.dg/opt/declone2.C
new file mode 100644
index 0000000..e725d8e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/opt/declone2.C
@@ -0,0 +1,10 @@
+// PR c++/61623
+// { dg-options "-Os" }
+
+struct C {};
+struct B : virtual C {};
+struct A : B {
+ A (int) {}
+};
+
+A a (0);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: symtab PATCH for c++/61623 (comdat issue in verify_symtab)
2014-07-14 21:21 symtab PATCH for c++/61623 (comdat issue in verify_symtab) Jason Merrill
@ 2014-07-15 9:18 ` Jan Hubicka
2014-07-15 19:14 ` Jason Merrill
0 siblings, 1 reply; 4+ messages in thread
From: Jan Hubicka @ 2014-07-15 9:18 UTC (permalink / raw)
To: Jason Merrill; +Cc: Jan Hubicka, gcc-patches List
> The problem in this testcase is that we inlined the decloned
> constructor into the calling thunks, so it was removed by
> symtab_remove_unreachable_nodes. verify_symtab sees that it is no
> longer linked to the calling thunks with same_comdat_group and
> complains.
>
> Here I've changed verify_symtab to only look at TREE_PUBLIC decls,
> since comdat-local symbols can be removed if they aren't needed. I
> also adjusted the diagnostic so it would print the two symbols in
> question rather than the same one twice. :)
>
> OK for trunk?
I think we still want to check that the local comdats are linked into
the corresponding comdat group, so we probably want
to test node->definition instead of node->public, or perhaps just clear
COMDAT_GROUP info when removing symbol?
Honza
> commit e29dc7a675fdbc1adce40908fda4d5408f0103a0
> Author: Jason Merrill <jason@redhat.com>
> Date: Mon Jul 14 16:58:57 2014 -0400
>
> PR c++/61623
> * symtab.c (verify_symtab): Don't worry about comdat-internal
> symbols.
>
> diff --git a/gcc/symtab.c b/gcc/symtab.c
> index 3a59935..dc4d84d 100644
> --- a/gcc/symtab.c
> +++ b/gcc/symtab.c
> @@ -1202,7 +1202,8 @@ verify_symtab (void)
> FOR_EACH_SYMBOL (node)
> {
> verify_symtab_node (node);
> - if (node->get_comdat_group ())
> + if (node->get_comdat_group ()
> + && TREE_PUBLIC (node->decl))
> {
> symtab_node **entry, *s;
> bool existed;
> @@ -1217,7 +1218,7 @@ verify_symtab (void)
> {
> error ("Two symbols with same comdat_group are not linked by the same_comdat_group list.");
> dump_symtab_node (stderr, *entry);
> - dump_symtab_node (stderr, s);
> + dump_symtab_node (stderr, node);
> internal_error ("verify_symtab failed");
> }
> }
> diff --git a/gcc/testsuite/g++.dg/opt/declone2.C b/gcc/testsuite/g++.dg/opt/declone2.C
> new file mode 100644
> index 0000000..e725d8e
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/opt/declone2.C
> @@ -0,0 +1,10 @@
> +// PR c++/61623
> +// { dg-options "-Os" }
> +
> +struct C {};
> +struct B : virtual C {};
> +struct A : B {
> + A (int) {}
> +};
> +
> +A a (0);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: symtab PATCH for c++/61623 (comdat issue in verify_symtab)
2014-07-15 9:18 ` Jan Hubicka
@ 2014-07-15 19:14 ` Jason Merrill
2014-07-16 11:05 ` Jan Hubicka
0 siblings, 1 reply; 4+ messages in thread
From: Jason Merrill @ 2014-07-15 19:14 UTC (permalink / raw)
To: Jan Hubicka; +Cc: gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 299 bytes --]
On 07/15/2014 04:46 AM, Jan Hubicka wrote:
> I think we still want to check that the local comdats are linked into
> the corresponding comdat group, so we probably want
> to test node->definition instead of node->public, or perhaps just clear
> COMDAT_GROUP info when removing symbol?
Like this?
[-- Attachment #2: 61623-2.patch --]
[-- Type: text/x-patch, Size: 1275 bytes --]
commit deb1a9e023d457c88edf52231d32af5ef03179f4
Author: Jason Merrill <jason@redhat.com>
Date: Mon Jul 14 16:58:57 2014 -0400
PR c++/61623
* symtab.c (symtab_remove_from_same_comdat_group): Also
set_comdat_group to NULL_TREE.
(verify_symtab): Fix diagnostic.
diff --git a/gcc/symtab.c b/gcc/symtab.c
index 3a59935..0050573 100644
--- a/gcc/symtab.c
+++ b/gcc/symtab.c
@@ -314,6 +314,7 @@ symtab_remove_from_same_comdat_group (symtab_node *node)
else
prev->same_comdat_group = node->same_comdat_group;
node->same_comdat_group = NULL;
+ node->set_comdat_group (NULL_TREE);
}
}
@@ -1217,7 +1218,7 @@ verify_symtab (void)
{
error ("Two symbols with same comdat_group are not linked by the same_comdat_group list.");
dump_symtab_node (stderr, *entry);
- dump_symtab_node (stderr, s);
+ dump_symtab_node (stderr, node);
internal_error ("verify_symtab failed");
}
}
diff --git a/gcc/testsuite/g++.dg/opt/declone2.C b/gcc/testsuite/g++.dg/opt/declone2.C
new file mode 100644
index 0000000..e725d8e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/opt/declone2.C
@@ -0,0 +1,10 @@
+// PR c++/61623
+// { dg-options "-Os" }
+
+struct C {};
+struct B : virtual C {};
+struct A : B {
+ A (int) {}
+};
+
+A a (0);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: symtab PATCH for c++/61623 (comdat issue in verify_symtab)
2014-07-15 19:14 ` Jason Merrill
@ 2014-07-16 11:05 ` Jan Hubicka
0 siblings, 0 replies; 4+ messages in thread
From: Jan Hubicka @ 2014-07-16 11:05 UTC (permalink / raw)
To: Jason Merrill; +Cc: Jan Hubicka, gcc-patches List
> On 07/15/2014 04:46 AM, Jan Hubicka wrote:
> >I think we still want to check that the local comdats are linked into
> >the corresponding comdat group, so we probably want
> >to test node->definition instead of node->public, or perhaps just clear
> >COMDAT_GROUP info when removing symbol?
>
> Like this?
>
>
> commit deb1a9e023d457c88edf52231d32af5ef03179f4
> Author: Jason Merrill <jason@redhat.com>
> Date: Mon Jul 14 16:58:57 2014 -0400
>
> PR c++/61623
> * symtab.c (symtab_remove_from_same_comdat_group): Also
> set_comdat_group to NULL_TREE.
> (verify_symtab): Fix diagnostic.
Yes, this looks perfect. Thanks a lot for looking into this!
Honza
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-07-16 11:02 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-14 21:21 symtab PATCH for c++/61623 (comdat issue in verify_symtab) Jason Merrill
2014-07-15 9:18 ` Jan Hubicka
2014-07-15 19:14 ` Jason Merrill
2014-07-16 11:05 ` Jan Hubicka
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).