pr 64076 - tolerate different definitions of symbols in lto
diff mbox

Message ID 1421721072-6962-1-git-send-email-tbsaunde+gcc@tbsaunde.org
State New
Headers show

Commit Message

tbsaunde+gcc@tbsaunde.org Jan. 20, 2015, 2:31 a.m. UTC
From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>

Hi,

when doing an lto link we can have some symbols be ir only and others be
machine code, which trips the assert here.  Just adjust the assert to handle
that.

bootstrapped + regtested x86_64-linux-gnu, ok?

Trev

gcc/

	* ipa-visibility.c (update_visibility_by_resolution_info): Only
	assert when not in lto mode.
---
 gcc/ipa-visibility.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

Comments

Richard Biener Jan. 20, 2015, 9:18 a.m. UTC | #1
On Tue, Jan 20, 2015 at 3:31 AM,  <tbsaunde+gcc@tbsaunde.org> wrote:
> From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
>
> Hi,
>
> when doing an lto link we can have some symbols be ir only and others be
> machine code, which trips the assert here.  Just adjust the assert to handle
> that.
>
> bootstrapped + regtested x86_64-linux-gnu, ok?

Testcase?  It's hard to understand what "machine code" is otherwise
or why this assert would fail.

Thanks,
Richard.

> Trev
>
> gcc/
>
>         * ipa-visibility.c (update_visibility_by_resolution_info): Only
>         assert when not in lto mode.
> ---
>  gcc/ipa-visibility.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/ipa-visibility.c b/gcc/ipa-visibility.c
> index 71894af..0791a1c 100644
> --- a/gcc/ipa-visibility.c
> +++ b/gcc/ipa-visibility.c
> @@ -425,11 +425,19 @@ update_visibility_by_resolution_info (symtab_node * node)
>    if (node->same_comdat_group)
>      for (symtab_node *next = node->same_comdat_group;
>          next != node; next = next->same_comdat_group)
> -      gcc_assert (!next->externally_visible
> -                 || define == (next->resolution == LDPR_PREVAILING_DEF_IRONLY
> -                               || next->resolution == LDPR_PREVAILING_DEF
> -                               || next->resolution == LDPR_UNDEF
> -                               || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP));
> +      {
> +       if (!next->externally_visible)
> +         continue;
> +
> +       bool same_def
> +         = define == (next->resolution == LDPR_PREVAILING_DEF_IRONLY
> +                      || next->resolution == LDPR_PREVAILING_DEF
> +                      || next->resolution == LDPR_UNDEF
> +                      || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP);
> +       gcc_assert (in_lto_p || same_def);
> +       if (!same_def)
> +         return;
> +      }
>
>    if (node->same_comdat_group)
>      for (symtab_node *next = node->same_comdat_group;
> --
> 2.1.4
>
Trevor Saunders Jan. 20, 2015, 2:15 p.m. UTC | #2
On Tue, Jan 20, 2015 at 10:18:25AM +0100, Richard Biener wrote:
> On Tue, Jan 20, 2015 at 3:31 AM,  <tbsaunde+gcc@tbsaunde.org> wrote:
> > From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
> >
> > Hi,
> >
> > when doing an lto link we can have some symbols be ir only and others be
> > machine code, which trips the assert here.  Just adjust the assert to handle
> > that.
> >
> > bootstrapped + regtested x86_64-linux-gnu, ok?
> 
> Testcase?  It's hard to understand what "machine code" is otherwise
> or why this assert would fail.
> 
> Thanks,

the one in the pr, basically one file is compiled without -flto and then
linked with -flto that + an odr violation making the two object files
different sets of thunks.

does the lto test suite support not passing -flto to one compilation? it
wasn't clear to me how to put the test case in the testsuite.

Trev

> Richard.
> 
> > Trev
> >
> > gcc/
> >
> >         * ipa-visibility.c (update_visibility_by_resolution_info): Only
> >         assert when not in lto mode.
> > ---
> >  gcc/ipa-visibility.c | 18 +++++++++++++-----
> >  1 file changed, 13 insertions(+), 5 deletions(-)
> >
> > diff --git a/gcc/ipa-visibility.c b/gcc/ipa-visibility.c
> > index 71894af..0791a1c 100644
> > --- a/gcc/ipa-visibility.c
> > +++ b/gcc/ipa-visibility.c
> > @@ -425,11 +425,19 @@ update_visibility_by_resolution_info (symtab_node * node)
> >    if (node->same_comdat_group)
> >      for (symtab_node *next = node->same_comdat_group;
> >          next != node; next = next->same_comdat_group)
> > -      gcc_assert (!next->externally_visible
> > -                 || define == (next->resolution == LDPR_PREVAILING_DEF_IRONLY
> > -                               || next->resolution == LDPR_PREVAILING_DEF
> > -                               || next->resolution == LDPR_UNDEF
> > -                               || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP));
> > +      {
> > +       if (!next->externally_visible)
> > +         continue;
> > +
> > +       bool same_def
> > +         = define == (next->resolution == LDPR_PREVAILING_DEF_IRONLY
> > +                      || next->resolution == LDPR_PREVAILING_DEF
> > +                      || next->resolution == LDPR_UNDEF
> > +                      || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP);
> > +       gcc_assert (in_lto_p || same_def);
> > +       if (!same_def)
> > +         return;
> > +      }
> >
> >    if (node->same_comdat_group)
> >      for (symtab_node *next = node->same_comdat_group;
> > --
> > 2.1.4
> >
Richard Biener Jan. 20, 2015, 2:20 p.m. UTC | #3
On Tue, Jan 20, 2015 at 3:15 PM, Trevor Saunders <tbsaunde@tbsaunde.org> wrote:
> On Tue, Jan 20, 2015 at 10:18:25AM +0100, Richard Biener wrote:
>> On Tue, Jan 20, 2015 at 3:31 AM,  <tbsaunde+gcc@tbsaunde.org> wrote:
>> > From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
>> >
>> > Hi,
>> >
>> > when doing an lto link we can have some symbols be ir only and others be
>> > machine code, which trips the assert here.  Just adjust the assert to handle
>> > that.
>> >
>> > bootstrapped + regtested x86_64-linux-gnu, ok?
>>
>> Testcase?  It's hard to understand what "machine code" is otherwise
>> or why this assert would fail.
>>
>> Thanks,
>
> the one in the pr, basically one file is compiled without -flto and then
> linked with -flto that + an odr violation making the two object files
> different sets of thunks.
>
> does the lto test suite support not passing -flto to one compilation? it
> wasn't clear to me how to put the test case in the testsuite.

Yes, simply add  { dg-options -fno-lto } to one of the secondary files.

Richard.

> Trev
>
>> Richard.
>>
>> > Trev
>> >
>> > gcc/
>> >
>> >         * ipa-visibility.c (update_visibility_by_resolution_info): Only
>> >         assert when not in lto mode.
>> > ---
>> >  gcc/ipa-visibility.c | 18 +++++++++++++-----
>> >  1 file changed, 13 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/gcc/ipa-visibility.c b/gcc/ipa-visibility.c
>> > index 71894af..0791a1c 100644
>> > --- a/gcc/ipa-visibility.c
>> > +++ b/gcc/ipa-visibility.c
>> > @@ -425,11 +425,19 @@ update_visibility_by_resolution_info (symtab_node * node)
>> >    if (node->same_comdat_group)
>> >      for (symtab_node *next = node->same_comdat_group;
>> >          next != node; next = next->same_comdat_group)
>> > -      gcc_assert (!next->externally_visible
>> > -                 || define == (next->resolution == LDPR_PREVAILING_DEF_IRONLY
>> > -                               || next->resolution == LDPR_PREVAILING_DEF
>> > -                               || next->resolution == LDPR_UNDEF
>> > -                               || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP));
>> > +      {
>> > +       if (!next->externally_visible)
>> > +         continue;
>> > +
>> > +       bool same_def
>> > +         = define == (next->resolution == LDPR_PREVAILING_DEF_IRONLY
>> > +                      || next->resolution == LDPR_PREVAILING_DEF
>> > +                      || next->resolution == LDPR_UNDEF
>> > +                      || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP);
>> > +       gcc_assert (in_lto_p || same_def);
>> > +       if (!same_def)
>> > +         return;
>> > +      }
>> >
>> >    if (node->same_comdat_group)
>> >      for (symtab_node *next = node->same_comdat_group;
>> > --
>> > 2.1.4
>> >

Patch
diff mbox

diff --git a/gcc/ipa-visibility.c b/gcc/ipa-visibility.c
index 71894af..0791a1c 100644
--- a/gcc/ipa-visibility.c
+++ b/gcc/ipa-visibility.c
@@ -425,11 +425,19 @@  update_visibility_by_resolution_info (symtab_node * node)
   if (node->same_comdat_group)
     for (symtab_node *next = node->same_comdat_group;
 	 next != node; next = next->same_comdat_group)
-      gcc_assert (!next->externally_visible
-		  || define == (next->resolution == LDPR_PREVAILING_DEF_IRONLY
-			        || next->resolution == LDPR_PREVAILING_DEF
-			        || next->resolution == LDPR_UNDEF
-			        || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP));
+      {
+	if (!next->externally_visible)
+	  continue;
+
+	bool same_def
+	  = define == (next->resolution == LDPR_PREVAILING_DEF_IRONLY
+		       || next->resolution == LDPR_PREVAILING_DEF
+		       || next->resolution == LDPR_UNDEF
+		       || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP);
+	gcc_assert (in_lto_p || same_def);
+	if (!same_def)
+	  return;
+      }
 
   if (node->same_comdat_group)
     for (symtab_node *next = node->same_comdat_group;