Re: [PATCH] LTO: Add -fcf-protection=check

[Richard Biener <richard.guenther@gmail.com>]

On Fri, Jul 24, 2020 at 11:15 PM H.J. Lu via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Mixing -fcf-protection and -fcf-protection=none objects are allowed.
> Linker just merges -fcf-protection values from all input objects.
>
> Add -fcf-protection=check for the final link with LTO.  An error is
> issued if LTO object files are compiled with different -fcf-protection
> values.  Otherwise, -fcf-protection=check is ignored at the compile
> time.  Without explicit -fcf-protection at link time, -fcf-protection
> values from LTO object files are merged at the final link.

OK.

Thanks,
Richard.

> gcc/
>
>         PR bootstrap/96203
>         * common.opt: Add -fcf-protection=check.
>         * flag-types.h (cf_protection_level): Add CF_CHECK.
>         * lto-wrapper.c (merge_and_complain): Issue an error for
>         mismatching -fcf-protection values with -fcf-protection=check.
>         Otherwise, merge -fcf-protection values.
>         * doc/invoke.texi: Document -fcf-protection=check.
>
> gcc/testsuite/
>
>         PR bootstrap/96203
>         * gcc.target/i386/pr96203-1.c: New test.
>         * gcc.target/i386/pr96203-2.c: Likewise.
> ---
>  gcc/common.opt                            |  5 ++-
>  gcc/doc/invoke.texi                       |  9 ++++--
>  gcc/flag-types.h                          |  3 +-
>  gcc/lto-wrapper.c                         | 39 +++++++++++++++++++----
>  gcc/testsuite/gcc.target/i386/pr96203-1.c | 18 +++++++++++
>  gcc/testsuite/gcc.target/i386/pr96203-2.c | 11 +++++++
>  6 files changed, 74 insertions(+), 11 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr96203-1.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr96203-2.c
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index a3893a4725e..47f42615bfa 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -1776,7 +1776,7 @@ Common RejectNegative Alias(fcf-protection=,full)
>
>  fcf-protection=
>  Common Report Joined RejectNegative Enum(cf_protection_level) Var(flag_cf_protection) Init(CF_NONE)
> --fcf-protection=[full|branch|return|none]      Instrument functions with checks to verify jump/call/return control-flow transfer
> +-fcf-protection=[full|branch|return|none|check]        Instrument functions with checks to verify jump/call/return control-flow transfer
>  instructions have valid targets.
>
>  Enum
> @@ -1791,6 +1791,9 @@ Enum(cf_protection_level) String(branch) Value(CF_BRANCH)
>  EnumValue
>  Enum(cf_protection_level) String(return) Value(CF_RETURN)
>
> +EnumValue
> +Enum(cf_protection_level) String(check) Value(CF_CHECK)
> +
>  EnumValue
>  Enum(cf_protection_level) String(none) Value(CF_NONE)
>
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index ba18e05fb1a..df84bce3a56 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -559,7 +559,7 @@ Objective-C and Objective-C++ Dialects}.
>  -fsanitize=@var{style}  -fsanitize-recover  -fsanitize-recover=@var{style} @gol
>  -fasan-shadow-offset=@var{number}  -fsanitize-sections=@var{s1},@var{s2},... @gol
>  -fsanitize-undefined-trap-on-error  -fbounds-check @gol
> --fcf-protection=@r{[}full@r{|}branch@r{|}return@r{|}none@r{]} @gol
> +-fcf-protection=@r{[}full@r{|}branch@r{|}return@r{|}none@r{|}check@r{]} @gol
>  -fstack-protector  -fstack-protector-all  -fstack-protector-strong @gol
>  -fstack-protector-explicit  -fstack-check @gol
>  -fstack-limit-register=@var{reg}  -fstack-limit-symbol=@var{sym} @gol
> @@ -14210,7 +14210,7 @@ operand constant, @code{__sanitizer_cov_trace_cmpf} or
>  @code{__sanitizer_cov_trace_cmpd} for float or double comparisons and
>  @code{__sanitizer_cov_trace_switch} for switch statements.
>
> -@item -fcf-protection=@r{[}full@r{|}branch@r{|}return@r{|}none@r{]}
> +@item -fcf-protection=@r{[}full@r{|}branch@r{|}return@r{|}none@r{|}check@r{]}
>  @opindex fcf-protection
>  Enable code instrumentation of control-flow transfers to increase
>  program security by checking that target addresses of control-flow
> @@ -14228,6 +14228,11 @@ function.  The value @code{full} is an alias for specifying both
>  @code{branch} and @code{return}. The value @code{none} turns off
>  instrumentation.
>
> +The value @code{check} is used for the final link with link-time
> +optimization (LTO).  An error is issued if LTO object files are
> +compiled with different @option{-fcf-protection} values.  The
> +value @code{check} is ignored at the compile time.
> +
>  The macro @code{__CET__} is defined when @option{-fcf-protection} is
>  used.  The first bit of @code{__CET__} is set to 1 for the value
>  @code{branch} and the second bit of @code{__CET__} is set to 1 for
> diff --git a/gcc/flag-types.h b/gcc/flag-types.h
> index b092c563f3d..852ea76eaa2 100644
> --- a/gcc/flag-types.h
> +++ b/gcc/flag-types.h
> @@ -368,7 +368,8 @@ enum cf_protection_level
>    CF_BRANCH = 1 << 0,
>    CF_RETURN = 1 << 1,
>    CF_FULL = CF_BRANCH | CF_RETURN,
> -  CF_SET = 1 << 2
> +  CF_SET = 1 << 2,
> +  CF_CHECK = 1 << 3
>  };
>
>  /* Parloops schedule type.  */
> diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
> index 5578b6d3200..e3b5cb2407a 100644
> --- a/gcc/lto-wrapper.c
> +++ b/gcc/lto-wrapper.c
> @@ -310,20 +310,45 @@ merge_and_complain (struct cl_decoded_option **decoded_options,
>
>         case OPT_fcf_protection_:
>           /* Default to link-time option, else append or check identical.  */
> -         if (!cf_protection_option)
> +         if (!cf_protection_option
> +             || cf_protection_option->value == CF_CHECK)
>             {
>               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))
> -               fatal_error (input_location,
> -                            "option -fcf-protection with mismatching values"
> -                            " (%s, %s)",
> -                            (*decoded_options)[j].arg, foption->arg);
> +             else if ((*decoded_options)[j].value != foption->value)
> +               {
> +                 if (cf_protection_option
> +                     && cf_protection_option->value == CF_CHECK)
> +                   fatal_error (input_location,
> +                                "option -fcf-protection with mismatching values"
> +                                " (%s, %s)",
> +                                (*decoded_options)[j].arg, foption->arg);
> +                 else
> +                   {
> +                     /* Merge and update the -fcf-protection option.  */
> +                     (*decoded_options)[j].value &= (foption->value
> +                                                     & CF_FULL);
> +                     switch ((*decoded_options)[j].value)
> +                       {
> +                       case CF_NONE:
> +                         (*decoded_options)[j].arg = "none";
> +                         break;
> +                       case CF_BRANCH:
> +                         (*decoded_options)[j].arg = "branch";
> +                         break;
> +                       case CF_RETURN:
> +                         (*decoded_options)[j].arg = "return";
> +                         break;
> +                       default:
> +                         gcc_unreachable ();
> +                       }
> +                   }
> +               }
>             }
> -           break;
> +         break;
>
>         case OPT_O:
>         case OPT_Ofast:
> diff --git a/gcc/testsuite/gcc.target/i386/pr96203-1.c b/gcc/testsuite/gcc.target/i386/pr96203-1.c
> new file mode 100644
> index 00000000000..332911fc931
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr96203-1.c
> @@ -0,0 +1,18 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fcf-protection=check" } */
> +/* { dg-final { scan-assembler-not "endbr" } } */
> +
> +extern int x;
> +
> +static void
> +__attribute__ ((noinline, noclone))
> +test (int i)
> +{
> +  x = i;
> +}
> +
> +void
> +bar (int i)
> +{
> +  test (i);
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/pr96203-2.c b/gcc/testsuite/gcc.target/i386/pr96203-2.c
> new file mode 100644
> index 00000000000..1141cf2b51f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr96203-2.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fcf-protection=check -mmanual-endbr" } */
> +/* { dg-final { scan-assembler-not "endbr" } } */
> +
> +extern void bar (void) __attribute__((__cf_check__));
> +
> +void
> +foo (void)
> +{
> +  bar ();
> +}
> --
> 2.26.2
>