Patchwork Fix -fPIC issue with GNU LD and LTO

login
register
mail settings
Submitter Jan Hubicka
Date Jan. 5, 2011, 1:15 p.m.
Message ID <20110105131525.GC24016@kam.mff.cuni.cz>
Download mbox | patch
Permalink /patch/77592/
State New
Headers show

Comments

Jan Hubicka - Jan. 5, 2011, 1:15 p.m.
Hi,
this patch enables us to get past failure with LTO mozilla build using GNU LD.
Mozilla invoke link as
/abuild/jh/trunk-install/bin/gcc  -O3 -flto -flto-partition=none
-fuse-linker-plugin -shared -Wl,-soname -Wl,libplds4.so  -o libplds4.so
./plarena.o ./plhash.o ./plvrsion.o    -L/abuild/jh/build-mozilla-new7/dist/lib
-lnspr4

that leads us to build PC32 relocations to external symbols.  This is because flag_shlib
is not set.  Normally -fPIC imply flag_shlib via finalize_options, but since -fPIC
is issued only later due to lto_reissue_options, we never get around setting it.

The following patch fixes it by adding logic to lto_reissue_options. Hacky, but the
whole thing is a hack and I can't think of anything better.

OK?

Bootstrapped/regtested x86_64-linux.

	PR lto/45375
	* lto-opt.c (lto_reissue_options): Set flag_shlib.
Jakub Jelinek - Jan. 5, 2011, 1:20 p.m.
On Wed, Jan 05, 2011 at 02:15:25PM +0100, Jan Hubicka wrote:
> +  /* Flag_shlib is usually set by finish_options, but we are issing flag_pic

issing?

> +     too late.  */
> +  if (flag_pic && !flag_pie)
> +    flag_shlib = 1;
>    VEC_free (opt_t, heap, opts);
>  }

	Jakub
Jan Hubicka - Jan. 5, 2011, 1:21 p.m.
> On Wed, Jan 05, 2011 at 02:15:25PM +0100, Jan Hubicka wrote:
> > +  /* Flag_shlib is usually set by finish_options, but we are issing flag_pic
> 
> issing?
issuing. Fixed in my local copy.

Honza
> 
> > +     too late.  */
> > +  if (flag_pic && !flag_pie)
> > +    flag_shlib = 1;
> >    VEC_free (opt_t, heap, opts);
> >  }
> 
> 	Jakub
Richard Guenther - Jan. 7, 2011, 5:35 p.m.
On Wed, Jan 5, 2011 at 2:21 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> On Wed, Jan 05, 2011 at 02:15:25PM +0100, Jan Hubicka wrote:
>> > +  /* Flag_shlib is usually set by finish_options, but we are issing flag_pic
>>
>> issing?
> issuing. Fixed in my local copy.

Hm, I suppose we might want to call finish_options again after
re-issuing options instead.

Richard.

> Honza
>>
>> > +     too late.  */
>> > +  if (flag_pic && !flag_pie)
>> > +    flag_shlib = 1;
>> >    VEC_free (opt_t, heap, opts);
>> >  }
>>
>>       Jakub
>
Jan Hubicka - Jan. 7, 2011, 5:47 p.m.
> On Wed, Jan 5, 2011 at 2:21 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> >> On Wed, Jan 05, 2011 at 02:15:25PM +0100, Jan Hubicka wrote:
> >> > +  /* Flag_shlib is usually set by finish_options, but we are issing flag_pic
> >>
> >> issing?
> > issuing. Fixed in my local copy.
> 
> Hm, I suppose we might want to call finish_options again after
> re-issuing options instead.

Hmm, I didn't do that since finish_options seems to contain guard against
repeated execution
  if (!opts->x_flag_opts_finished)
    {
      if (opts->x_flag_pie)
        opts->x_flag_pic = opts->x_flag_pie;
      if (opts->x_flag_pic && !opts->x_flag_pie)
        opts->x_flag_shlib = 1;
      opts->x_flag_opts_finished = false;
    }
only now I noticed that opts_finished is set to false that it is already. This looks like typo.
What was original motivation for that thing?

I am not sure if we want to finish options again since it has all that tristate handling in it
that would need to be saved into opts section.
But basically I have no idea, option handling seemed like magic to me all the time and lto
opts like a mess...  We just need to fix the shlib issue somehow or most of PIC builds with
LTO will be broken.

Honza
> 
> Richard.
> 
> > Honza
> >>
> >> > +     too late.  */
> >> > +  if (flag_pic && !flag_pie)
> >> > +    flag_shlib = 1;
> >> >    VEC_free (opt_t, heap, opts);
> >> >  }
> >>
> >>       Jakub
> >
Richard Guenther - Jan. 7, 2011, 6:15 p.m.
2011/1/7 Jan Hubicka <hubicka@ucw.cz>:
>> On Wed, Jan 5, 2011 at 2:21 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> >> On Wed, Jan 05, 2011 at 02:15:25PM +0100, Jan Hubicka wrote:
>> >> > +  /* Flag_shlib is usually set by finish_options, but we are issing flag_pic
>> >>
>> >> issing?
>> > issuing. Fixed in my local copy.
>>
>> Hm, I suppose we might want to call finish_options again after
>> re-issuing options instead.
>
> Hmm, I didn't do that since finish_options seems to contain guard against
> repeated execution
>  if (!opts->x_flag_opts_finished)
>    {
>      if (opts->x_flag_pie)
>        opts->x_flag_pic = opts->x_flag_pie;
>      if (opts->x_flag_pic && !opts->x_flag_pie)
>        opts->x_flag_shlib = 1;
>      opts->x_flag_opts_finished = false;
>    }
> only now I noticed that opts_finished is set to false that it is already. This looks like typo.
> What was original motivation for that thing?
>
> I am not sure if we want to finish options again since it has all that tristate handling in it
> that would need to be saved into opts section.
> But basically I have no idea, option handling seemed like magic to me all the time and lto
> opts like a mess...  We just need to fix the shlib issue somehow or most of PIC builds with
> LTO will be broken.

I guess we can sort out things when we fix LTO option handling for
real in 4.7.  Your original
patch is ok.

Thanks,
Richard.

> Honza
>>
>> Richard.
>>
>> > Honza
>> >>
>> >> > +     too late.  */
>> >> > +  if (flag_pic && !flag_pie)
>> >> > +    flag_shlib = 1;
>> >> >    VEC_free (opt_t, heap, opts);
>> >> >  }
>> >>
>> >>       Jakub
>> >
>
Joseph S. Myers - Jan. 7, 2011, 6:25 p.m.
On Fri, 7 Jan 2011, Jan Hubicka wrote:

> Hmm, I didn't do that since finish_options seems to contain guard against
> repeated execution
>   if (!opts->x_flag_opts_finished)
>     {
>       if (opts->x_flag_pie)
>         opts->x_flag_pic = opts->x_flag_pie;
>       if (opts->x_flag_pic && !opts->x_flag_pie)
>         opts->x_flag_shlib = 1;
>       opts->x_flag_opts_finished = false;
>     }

> only now I noticed that opts_finished is set to false that it is 
> already. This looks like typo.

That's definitely a mistake.

> What was original motivation for that thing?

x_flag_opts_finished is meant to be a replacement (avoiding global state) 
for code that ran only the first time options were processed (i.e., from 
the command line but not from attributes).  It's a hack in that ideally 
we'd save the options structures before passing them through 
finish_options, options from attributes would apply to the saved 
structures and then all the finishing (including bits that aren't 
currently in finish_options) would be rerun - hopefully fixing some of the 
bugs relating to target and optimize attributes.
Jan Hubicka - Jan. 7, 2011, 6:42 p.m.
> On Fri, 7 Jan 2011, Jan Hubicka wrote:
> 
> > Hmm, I didn't do that since finish_options seems to contain guard against
> > repeated execution
> >   if (!opts->x_flag_opts_finished)
> >     {
> >       if (opts->x_flag_pie)
> >         opts->x_flag_pic = opts->x_flag_pie;
> >       if (opts->x_flag_pic && !opts->x_flag_pie)
> >         opts->x_flag_shlib = 1;
> >       opts->x_flag_opts_finished = false;
> >     }
> 
> > only now I noticed that opts_finished is set to false that it is 
> > already. This looks like typo.
> 
> That's definitely a mistake.
> 
> > What was original motivation for that thing?
> 
> x_flag_opts_finished is meant to be a replacement (avoiding global state) 
> for code that ran only the first time options were processed (i.e., from 
> the command line but not from attributes).  It's a hack in that ideally 
> we'd save the options structures before passing them through 
> finish_options, options from attributes would apply to the saved 
> structures and then all the finishing (including bits that aren't 
> currently in finish_options) would be rerun - hopefully fixing some of the 
> bugs relating to target and optimize attributes.

OK, thanks for explanation.  Could you please care fixing this, or shall I drop
it into TODO?

Honza
Joseph S. Myers - Jan. 7, 2011, 9:16 p.m.
On Fri, 7 Jan 2011, Jan Hubicka wrote:

> > x_flag_opts_finished is meant to be a replacement (avoiding global state) 
> > for code that ran only the first time options were processed (i.e., from 
> > the command line but not from attributes).  It's a hack in that ideally 
> > we'd save the options structures before passing them through 
> > finish_options, options from attributes would apply to the saved 
> > structures and then all the finishing (including bits that aren't 
> > currently in finish_options) would be rerun - hopefully fixing some of the 
> > bugs relating to target and optimize attributes.
> 
> OK, thanks for explanation.  Could you please care fixing this, or shall I drop
> it into TODO?

I'll deal with fixing the incorrect "false" setting.

Patch

Index: lto-opts.c
===================================================================
--- lto-opts.c	(revision 168506)
+++ lto-opts.c	(working copy)
@@ -420,5 +420,9 @@ 
 	gcc_unreachable ();
     }
 
+  /* Flag_shlib is usually set by finish_options, but we are issing flag_pic
+     too late.  */
+  if (flag_pic && !flag_pie)
+    flag_shlib = 1;
   VEC_free (opt_t, heap, opts);
 }