Message ID | 20150327152309.GD63825@kam.mff.cuni.cz |
---|---|
State | New |
Headers | show |
2015-03-27 18:23 GMT+03:00 Jan Hubicka <hubicka@ucw.cz>: > Hi, > this patch fixes bug in symtab_node::verify_symtab_nodes pointed out by Ilya. > The loop checking that there all comdats are linked by same_comdat_group was > completely bogus. In addition it checked also external symbols that are > currently not kept in groups. This bug was in mainline for months but > apparently bounds checking is first code producing interesting comdat groups to > trigger it. > > Ilya, can you please mind comitting the testcase? I am not quite sure where > bounds checking should go offhand. > > Also concerning linking the comdat groups, currently all non-DECL_EXTERNAL symbols > are linked and all !DECL_EXTERNAL symbols are non-linked. Bounds checking should > follow this scheme. It may be cleaner to keep links for DECL_EXTERNAL, but lets > deffer that for next stage1. I'll send a corresponding fix in a checker with a testcase. Please let me know when you introduce comdat links for external symbols to keep the checker in sync. Thanks, Ilya > > Bootstrapped/regtested x86_64-linux. Committed. > > Honza >
2015-03-27 18:23 GMT+03:00 Jan Hubicka <hubicka@ucw.cz>: > Hi, > this patch fixes bug in symtab_node::verify_symtab_nodes pointed out by Ilya. > The loop checking that there all comdats are linked by same_comdat_group was > completely bogus. In addition it checked also external symbols that are > currently not kept in groups. This bug was in mainline for months but > apparently bounds checking is first code producing interesting comdat groups to > trigger it. > > Ilya, can you please mind comitting the testcase? I am not quite sure where > bounds checking should go offhand. > > Also concerning linking the comdat groups, currently all non-DECL_EXTERNAL symbols > are linked and all !DECL_EXTERNAL symbols are non-linked. Bounds checking should > follow this scheme. It may be cleaner to keep links for DECL_EXTERNAL, but lets > deffer that for next stage1. > > Bootstrapped/regtested x86_64-linux. Committed. > > Honza > > Index: ChangeLog > =================================================================== > --- ChangeLog (revision 221735) > +++ ChangeLog (working copy) > @@ -1,5 +1,11 @@ > 2015-03-27 Jan Hubicka <hubicka@ucw.cz> > > + PR target/65531 > + * symtab.c (symtab_node::verify_symtab_nodes): Fix verification of > + comdat groups. > + > +2015-03-27 Jan Hubicka <hubicka@ucw.cz> > + > PR ipa/65600 > * cgraph.c (cgraph_update_edges_for_call_stmt_node): Fix the case > of optimized out indirect call. > Index: symtab.c > =================================================================== > --- symtab.c (revision 221734) > +++ symtab.c (working copy) > @@ -1130,15 +1130,20 @@ symtab_node::verify_symtab_nodes (void) > &existed); > if (!existed) > *entry = node; > - else > - for (s = (*entry)->same_comdat_group; s != NULL && s != node; s = s->same_comdat_group) > + else if (!DECL_EXTERNAL (node->decl)) > + { > + for (s = (*entry)->same_comdat_group; s != NULL && s != node; > + s = s->same_comdat_group) > + ; With no if-statement in the loop body you need an additional exit condition for a case when you reach the entry. Thanks, Ilya > if (!s || s == *entry) > { > - error ("Two symbols with same comdat_group are not linked by the same_comdat_group list."); > + error ("Two symbols with same comdat_group are not linked by " > + "the same_comdat_group list."); > (*entry)->debug (); > node->debug (); > internal_error ("symtab_node::verify failed"); > } > + } > } > } > }
Index: ChangeLog =================================================================== --- ChangeLog (revision 221735) +++ ChangeLog (working copy) @@ -1,5 +1,11 @@ 2015-03-27 Jan Hubicka <hubicka@ucw.cz> + PR target/65531 + * symtab.c (symtab_node::verify_symtab_nodes): Fix verification of + comdat groups. + +2015-03-27 Jan Hubicka <hubicka@ucw.cz> + PR ipa/65600 * cgraph.c (cgraph_update_edges_for_call_stmt_node): Fix the case of optimized out indirect call. Index: symtab.c =================================================================== --- symtab.c (revision 221734) +++ symtab.c (working copy) @@ -1130,15 +1130,20 @@ symtab_node::verify_symtab_nodes (void) &existed); if (!existed) *entry = node; - else - for (s = (*entry)->same_comdat_group; s != NULL && s != node; s = s->same_comdat_group) + else if (!DECL_EXTERNAL (node->decl)) + { + for (s = (*entry)->same_comdat_group; s != NULL && s != node; + s = s->same_comdat_group) + ; if (!s || s == *entry) { - error ("Two symbols with same comdat_group are not linked by the same_comdat_group list."); + error ("Two symbols with same comdat_group are not linked by " + "the same_comdat_group list."); (*entry)->debug (); node->debug (); internal_error ("symtab_node::verify failed"); } + } } } }