On Wed, Jun 17, 2020 at 5:00 AM Richard Biener <rguent...@suse.de> wrote:
>
> On Wed, 17 Jun 2020, H.J. Lu wrote:
>
> > On Wed, Jun 17, 2020 at 1:59 AM Richard Biener
> > <richard.guent...@gmail.com> wrote:
> > >
> > > On Mon, Jun 15, 2020 at 5:30 PM Matthias Klose <d...@ubuntu.com> wrote:
> > > >
> > > > PR lto/95604 was seen when checking for binaries without having CET
> > > > support in a
> > > > distro archive, for binaries built with LTO optimization. The
> > > > hardening flag
> > > > -fcf-protection=full is passed in CFLAGS, and maybe should be passed in
> > > > LDFLAGS
> > > > as well. However to make it work when not passed to the link step, it
> > > > should be
> > > > extracted from the options found in the lto opts section.
> > > >
> > > > Richard suggested two solutions offline. I checked that both fix the
> > > > test case.
> > > > Which one to install? Also ok for the 9 and 10 branches?
> > >
> > > I guess even though variant two is simpler it doesn't make much sense to
> > > have differing settings of -fcf-protection between different functions?
> > > HJ?
> >
> > -fcf-protection is applied to a file, not a function since CET marker
> > is per file.
> >
> > > So looking at variant one,
> > >
> > > @@ -287,6 +287,18 @@
> > > foption->orig_option_with_args_text);
> > > break;
> > >
> > > + case OPT_fcf_protection_:
> > > + /* Append or check identical. */
> > > + for (j = 0; j < *decoded_options_count; ++j)
> > > + if ((*decoded_options)[j].opt_index == foption->opt_index)
> > > + break;
> > > + if (j == *decoded_options_count)
> > > + append_option (decoded_options, decoded_options_count,
> > > foption);
> > > + else if (strcmp ((*decoded_options)[j].arg, foption->arg))
> > > + warning (input_location, "option %s with different values",
> > > + foption->orig_option_with_args_text);
> > > + break;
> > >
> > > you are always streaming a -fcf-protection option so the if (j ==
> > > *decoded_options_count)
> > > case shouldn't ever happen but I guess it's safe to leave the code
> > > as-is. Can you
> > > amend the warning with the option that prevails? Maybe
> > >
> > > + else if (strcmp ((*decoded_options)[j].arg, foption->arg))
> > > {
> > > static bool noted;
> > > + warning (input_location, "option %s with different values",
> > > + foption->orig_option_with_args_text);
> > > if (!noted)
> > > {
> > > inform ("%s will be used instead",
> > > (*decoded_options)[j].orig_option_with_args_text);
> > > noted = true;
> > > }
> > >
> > > I guess input_location is simply zero so the diagnostic doesn't
> > > contain the actual file we're
> > > looking at. Something to improve I guess (also applyign to other
> > > diagnostics we emit).
> > >
> > > Otherwise looks OK.
> > >
> > > Please wait for HJ in case he'd like to go with option two.
> > >
> >
> > I prefer option one. But what happens if input files are compiled
> > with different -fcf-protection settings?
>
> You get a warning and the first option setting wins (and I requested
> to inform the user which that is).
>
I think it should be an error and the user should explicitly specify the
option with -lfto in the final link command.
--
H.J.
