diff mbox

symtab PATCH for c++/61623 (comdat issue in verify_symtab)

Message ID 53C44915.9080903@redhat.com
State New
Headers show

Commit Message

Jason Merrill July 14, 2014, 9:18 p.m. UTC 
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?

Comments

Jan Hubicka July 15, 2014, 8:46 a.m. UTC | #1 
> 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);
diff mbox

Patch

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);