Re: Fix even more merging PIC and PIE options
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
Re: Fix even more merging PIC and PIE options
> 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?
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";
> > +
Re: Fix even more merging PIC and PIE options
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?
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 rem
Re: Fix even more merging PIC and PIE options
On 08/10/2018 04:33 PM, 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
Thank you Honza for it. Would it be then subject for backporting into GCC-8
branch?
Martin
>
> 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)
> + && !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
Fix even more merging PIC and PIE options
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)
+ && !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 c
