On Thu, 30 Aug 2018, Jan Hubicka wrote:
> > On Fri, 10 Aug 2018, Jan Hubicka wrote:
> >
> > > Hi,
> > > this patch should fix merging of PIC and PIE options so we always resort
> > > to the least common denominator of the object files compiled (i.e.
> > > linking together -fpic and -fPIE will result in -fpie binary).
> > > Note that we also use information about format of output passed by linker
> > > plugin so we will disable pic/pie when resulting binary is not
> > > relocatable.
> > > However for shared libraries and pie we want to pick right mode that makes
> > > sense.
> > >
> > > I wrote simple script that tries all possible options of combining two
> > > files.
> > > Mode picked is specified in the output file.
> > >
> > > To support targets that default to pic/pie well, I had to also hack
> > > lto_write_options to always stream what mode was used in the given file.
> > >
> > > lto-bootstrapped/regtested x86_64-linux, OK?
> > >
> > > Honza
> > >
> > > PR lto/86517
> > > * lto-opts.c (lto_write_options): Always stream PIC/PIE mode.
> > > * lto-wrapper.c (merge_and_complain): Fix merging of PIC/PIE
> > > Index: lto-opts.c
> > > ===================================================================
> > > --- lto-opts.c (revision 263356)
> > > +++ lto-opts.c (working copy)
> > > @@ -78,6 +78,22 @@ lto_write_options (void)
> > > && !global_options.x_flag_openacc)
> > > append_to_collect_gcc_options (&temporary_obstack, &first_p,
> > > "-fno-openacc");
> > > + /* Append PIC/PIE mode because its default depends on target and it is
> > > + subject of merging in lto-wrapper. */
> > > + if ((!global_options_set.x_flag_pic || global_options.x_flag_pic == 0)
> >
> > why's that checked only for flag_pic?
> > Shouldn't it be consistent with flag_pie?
> >
> > Isn't it so that setting either -f[no-]pic or -f[no-]pie fully specifies
> > the state? So the == 0 check is superfluous?
>
> Yep, it is superflows. I have changed it to test if at least one option is
> set,
> re-tested and going to commit.
>
> OK for release branch after some soaking?
OK after Cauldron if nothing bad happened.
Richard.
> Honza
>
> >
> > The rest of the patch looks OK to me.
> >
> > Thanks,
> > Richard.
> >
> > > + && !global_options_set.x_flag_pie)
> > > + {
> > > + append_to_collect_gcc_options (&temporary_obstack, &first_p,
> > > + global_options.x_flag_pic == 2
> > > + ? "-fPIC"
> > > + : global_options.x_flag_pic == 1
> > > + ? "-fpic"
> > > + : global_options.x_flag_pie == 2
> > > + ? "-fPIE"
> > > + : global_options.x_flag_pie == 1
> > > + ? "-fpie"
> > > + : "-fno-pie");
> > > + }
> > >
> > > /* Append options from target hook and store them to offload_lto
> > > section. */
> > > if (lto_stream_offload_p)
> > > Index: lto-wrapper.c
> > > ===================================================================
> > > --- lto-wrapper.c (revision 263356)
> > > +++ lto-wrapper.c (working copy)
> > > @@ -408,6 +408,11 @@ merge_and_complain (struct cl_decoded_op
> > > It is a common mistake to mix few -fPIC compiled objects into
> > > otherwise
> > > non-PIC code. We do not want to build everything with PIC then.
> > >
> > > + Similarly we merge PIE options, however in addition we keep
> > > + -fPIC + -fPIE = -fPIE
> > > + -fpic + -fPIE = -fpie
> > > + -fPIC/-fpic + -fpie = -fpie
> > > +
> > > It would be good to warn on mismatches, but it is bit hard to do as
> > > we do not know what nothing translates to. */
> > >
> > > @@ -415,11 +420,38 @@ merge_and_complain (struct cl_decoded_op
> > > if ((*decoded_options)[j].opt_index == OPT_fPIC
> > > || (*decoded_options)[j].opt_index == OPT_fpic)
> > > {
> > > - if (!pic_option
> > > - || (pic_option->value > 0) != ((*decoded_options)[j].value > 0))
> > > - remove_option (decoded_options, j, decoded_options_count);
> > > - else if (pic_option->opt_index == OPT_fPIC
> > > - && (*decoded_options)[j].opt_index == OPT_fpic)
> > > + /* -fno-pic in one unit implies -fno-pic everywhere. */
> > > + if ((*decoded_options)[j].value == 0)
> > > + j++;
> > > + /* If we have no pic option or merge in -fno-pic, we still may turn
> > > + existing pic/PIC mode into pie/PIE if -fpie/-fPIE is present. */
> > > + else if ((pic_option && pic_option->value == 0)
> > > + || !pic_option)
> > > + {
> > > + if (pie_option)
> > > + {
> > > + bool big = (*decoded_options)[j].opt_index == OPT_fPIC
> > > + && pie_option->opt_index == OPT_fPIE;
> > > + (*decoded_options)[j].opt_index = big ? OPT_fPIE : OPT_fpie;
> > > + if (pie_option->value)
> > > + (*decoded_options)[j].canonical_option[0] = big ? "-fPIE" :
> > > "-fpie";
> > > + else
> > > + (*decoded_options)[j].canonical_option[0] = big ? "-fno-pie"
> > > : "-fno-pie";
> > > + (*decoded_options)[j].value = pie_option->value;
> > > + j++;
> > > + }
> > > + else if (pic_option)
> > > + {
> > > + (*decoded_options)[j] = *pic_option;
> > > + j++;
> > > + }
> > > + /* We do not know if target defaults to pic or not, so just remove
> > > + option if it is missing in one unit but enabled in other. */
> > > + else
> > > + remove_option (decoded_options, j, decoded_options_count);
> > > + }
> > > + else if (pic_option->opt_index == OPT_fpic
> > > + && (*decoded_options)[j].opt_index == OPT_fPIC)
> > > {
> > > (*decoded_options)[j] = *pic_option;
> > > j++;
> > > @@ -430,11 +462,42 @@ merge_and_complain (struct cl_decoded_op
> > > else if ((*decoded_options)[j].opt_index == OPT_fPIE
> > > || (*decoded_options)[j].opt_index == OPT_fpie)
> > > {
> > > - if (!pie_option
> > > - || pie_option->value != (*decoded_options)[j].value)
> > > - remove_option (decoded_options, j, decoded_options_count);
> > > - else if (pie_option->opt_index == OPT_fPIE
> > > - && (*decoded_options)[j].opt_index == OPT_fpie)
> > > + /* -fno-pie in one unit implies -fno-pie everywhere. */
> > > + if ((*decoded_options)[j].value == 0)
> > > + j++;
> > > + /* If we have no pie option or merge in -fno-pie, we still preserve
> > > + PIE/pie if pic/PIC is present. */
> > > + else if ((pie_option && pie_option->value == 0)
> > > + || !pie_option)
> > > + {
> > > + /* If -fPIC/-fpic is given, merge it with -fPIE/-fpie. */
> > > + if (pic_option)
> > > + {
> > > + if (pic_option->opt_index == OPT_fpic
> > > + && (*decoded_options)[j].opt_index == OPT_fPIE)
> > > + {
> > > + (*decoded_options)[j].opt_index = OPT_fpie;
> > > + (*decoded_options)[j].canonical_option[0]
> > > + = pic_option->value ? "-fpie" : "-fno-pie";
> > > + }
> > > + else if (!pic_option->value)
> > > + (*decoded_options)[j].canonical_option[0] = "-fno-pie";
> > > + (*decoded_options)[j].value = pic_option->value;
> > > + j++;
> > > + }
> > > + else if (pie_option)
> > > + {
> > > + (*decoded_options)[j] = *pie_option;
> > > + j++;
> > > + }
> > > + /* Because we always append pic/PIE options this code path should
> > > + not happen unless the LTO object was built by old lto1 which
> > > + did not contain that logic yet. */
> > > + else
> > > + remove_option (decoded_options, j, decoded_options_count);
> > > + }
> > > + else if (pie_option->opt_index == OPT_fpie
> > > + && (*decoded_options)[j].opt_index == OPT_fPIE)
> > > {
> > > (*decoded_options)[j] = *pie_option;
> > > j++;
> > >
> >
> > --
> > Richard Biener <[email protected]>
> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
> > 21284 (AG Nuernberg)
>
>
--
Richard Biener <[email protected]>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
