On Wed, Jun 17, 2020 at 5:33 AM Richard Biener <rguent...@suse.de> wrote: > > On Wed, 17 Jun 2020, H.J. Lu wrote: > > > 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. > > But AFAIK ld will not reject linking objects with mismatched settings?
Linker will silently change CET run-time behavior. > Does -fcf-protection have effects "early" (up to IPA)? That is, is it > safe to turn it on only post-IPA? I think so. We don't do anything with SHSTK. For IBT, we just insert an ENDBR when we output the indirect branch target. > So yeah, if the user provided an explicit -fcf-protection option at link > we honor that (but with the current patch we'd still warn about conflicts I don't think we should warn about the explicit -fcf-protection option at link time. > between the original TU setting - but not conflicts with the setting > at link time). If we want to error on mismatches but allow if at link > time a setting is specified that needs extra code (it would be the > first time we have this behavior). > I think we should give an error if there are any mismatches in inputs and let the explicit -fcf-protection option at link time override -fcf-protection options in inputs . -- H.J.