[PATCH] x86: Allow -fcf-protection with multi-byte NOPs

Tsimbalist, Igor V igor.v.tsimbalist@intel.com
Fri Apr 20 13:22:00 GMT 2018


> -----Original Message-----
> From: H.J. Lu [mailto:hjl.tools@gmail.com]
> Sent: Friday, April 20, 2018 1:15 PM
> To: Jakub Jelinek <jakub@redhat.com>
> Cc: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>; Richard Biener
> <richard.guenther@gmail.com>; Uros Bizjak <ubizjak@gmail.com>; gcc-
> patches@gcc.gnu.org
> Subject: Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
> 
> On Fri, Apr 20, 2018 at 09:39:58AM +0200, Jakub Jelinek wrote:
> > On Fri, Apr 20, 2018 at 06:25:10AM +0000, Tsimbalist, Igor V wrote:
> > > > Something like this?
> > >
> > > Shouldn't this
> > >
> > > -# ifdef __IBT__
> > > +# if (__CET__ & 1) != 0
> > >
> > > Be as
> > >
> > > -# ifdef __IBT__
> > > +#ifdef __CET__
> > > +# if (__CET__ & 1) != 0
> > >
> > > OK otherwise.
> >
> > Only if you use -Wundef warning (not part of -Wall or -W) and, if this
> > is a system header, only with -Wundef -Wsystem-headers.
> > But perhaps it doesn't hurt to wrap it.
> >
> > 	Jakub
> 
> Here is the patch.  OK for trunk?
> 
> Thanks.

OK.

Igor

> 
> H.J.
> ---
> With revision 259496:
> 
> commit b1384095a7c1d06a44b70853372ebe037b2f7867
> Author: hjl <hjl@138bc75d-0d04-0410-961f-82ee72b054a4>
> Date:   Thu Apr 19 15:15:04 2018 +0000
> 
>     x86: Enable -fcf-protection with multi-byte NOPs
> 
> -mibt does nothing and can be removed.  Define __CET__ to indicate level
> protection with -fcf-protection:
> 
> (__CET__ & 1) != 0: -fcf-protection=branch or -fcf-protection=full
> (__CET__ & 2) != 0: -fcf-protection=return or -fcf-protection=full
> 
> gcc/
> 
> 	PR target/85469
> 	* common/config/i386/i386-common.c
> (OPTION_MASK_ISA_IBT_SET):
> 	Removed.
> 	(OPTION_MASK_ISA_IBT_UNSET): Likewise.
> 	(ix86_handle_option): Don't handle OPT_mibt.
> 	* config/i386/cet.h: Check __CET__ instead of __IBT__ and
> 	__SHSTK__.
> 	* config/i386/driver-i386.c (host_detect_local_cpu): Remove
> 	has_ibt and ibt.
> 	* config/i386/i386-c.c (ix86_target_macros_internal): Don't
> 	check OPTION_MASK_ISA_IBT nor flag_cf_protection.
> 	(ix86_target_macros): Define __CET__ with flag_cf_protection
> 	for -fcf-protection.
> 	* config/i386/i386.c (isa2_opts): Remove -mibt.
> 	* config/i386/i386.h (TARGET_IBT): Removed.
> 	(TARGET_IBT_P): Likewise.
> 	(ix86_valid_target_attribute_inner_p): Don't check OPT_mibt.
> 	* config/i386/i386.md (nop_endbr): Don't check TARGET_IBT.
> 	* config/i386/i386.opt (mcet): Update help message.
> 	(mshstk): Likewise.
> 	(mibt): Removed.
> 	* doc/invoke.texi: Remove -mibt.  Document __CET__.  Document
> 	-mcet as an alias for -mshstk.
> 
> gcc/testsuite/
> 
> 	PR target/85469
> 	* gcc.target/i386/pr85044.c (dg-options): Remove -mibt.
> 	* gcc.target/i386/sse-26.c (dg-options): Remove -mno-ibt.
> ---
>  gcc/common/config/i386/i386-common.c    | 17 -----------------
>  gcc/config/i386/cet.h                   |  6 +++---
>  gcc/config/i386/driver-i386.c           |  6 ++----
>  gcc/config/i386/i386-c.c                | 20 ++++++--------------
>  gcc/config/i386/i386.c                  |  2 --
>  gcc/config/i386/i386.h                  |  2 --
>  gcc/config/i386/i386.md                 |  2 +-
>  gcc/config/i386/i386.opt                | 12 ++++--------
>  gcc/doc/invoke.texi                     | 28 +++++++++++-----------------
>  gcc/testsuite/gcc.target/i386/pr85044.c |  2 +-
>  gcc/testsuite/gcc.target/i386/sse-26.c  |  2 +-
>  11 files changed, 29 insertions(+), 70 deletions(-)
> 
> diff --git a/gcc/common/config/i386/i386-common.c
> b/gcc/common/config/i386/i386-common.c
> index 0bb2783cfab..74a3490f7a3 100644
> --- a/gcc/common/config/i386/i386-common.c
> +++ b/gcc/common/config/i386/i386-common.c
> @@ -147,7 +147,6 @@ along with GCC; see the file COPYING3.  If not see
>  #define OPTION_MASK_ISA_PKU_SET OPTION_MASK_ISA_PKU
>  #define OPTION_MASK_ISA_RDPID_SET OPTION_MASK_ISA_RDPID
>  #define OPTION_MASK_ISA_GFNI_SET OPTION_MASK_ISA_GFNI
> -#define OPTION_MASK_ISA_IBT_SET OPTION_MASK_ISA_IBT
>  #define OPTION_MASK_ISA_SHSTK_SET OPTION_MASK_ISA_SHSTK
>  #define OPTION_MASK_ISA_VAES_SET OPTION_MASK_ISA_VAES
>  #define OPTION_MASK_ISA_VPCLMULQDQ_SET
> OPTION_MASK_ISA_VPCLMULQDQ
> @@ -224,7 +223,6 @@ along with GCC; see the file COPYING3.  If not see
>  #define OPTION_MASK_ISA_PKU_UNSET OPTION_MASK_ISA_PKU
>  #define OPTION_MASK_ISA_RDPID_UNSET OPTION_MASK_ISA_RDPID
>  #define OPTION_MASK_ISA_GFNI_UNSET OPTION_MASK_ISA_GFNI
> -#define OPTION_MASK_ISA_IBT_UNSET OPTION_MASK_ISA_IBT
>  #define OPTION_MASK_ISA_SHSTK_UNSET OPTION_MASK_ISA_SHSTK
>  #define OPTION_MASK_ISA_VAES_UNSET OPTION_MASK_ISA_VAES
>  #define OPTION_MASK_ISA_VPCLMULQDQ_UNSET
> OPTION_MASK_ISA_VPCLMULQDQ
> @@ -546,21 +544,6 @@ ix86_handle_option (struct gcc_options *opts,
>        return true;
> 
>      case OPT_mcet:
> -    case OPT_mibt:
> -      if (value)
> -	{
> -	  opts->x_ix86_isa_flags2 |= OPTION_MASK_ISA_IBT_SET;
> -	  opts->x_ix86_isa_flags2_explicit |= OPTION_MASK_ISA_IBT_SET;
> -	}
> -      else
> -	{
> -	  opts->x_ix86_isa_flags2 &= ~OPTION_MASK_ISA_IBT_UNSET;
> -	  opts->x_ix86_isa_flags2_explicit |= OPTION_MASK_ISA_IBT_UNSET;
> -	}
> -      if (code != OPT_mcet)
> -	return true;
> -      /* fall through.  */
> -
>      case OPT_mshstk:
>        if (value)
>  	{
> diff --git a/gcc/config/i386/cet.h b/gcc/config/i386/cet.h
> index 9dca41bad2d..309f6428735 100644
> --- a/gcc/config/i386/cet.h
> +++ b/gcc/config/i386/cet.h
> @@ -32,7 +32,7 @@
> 
>  #ifdef __ASSEMBLER__
> 
> -# ifdef __IBT__
> +# if defined __CET__ && (__CET__ & 1) != 0
>  #  ifdef __x86_64__
>  #   define _CET_ENDBR endbr64
>  #  else
> @@ -44,14 +44,14 @@
> 
>  # ifdef __ELF__
>  #  ifdef __CET__
> -#   ifdef __IBT__
> +#   if (__CET__ & 1) != 0
>  /* GNU_PROPERTY_X86_FEATURE_1_IBT.  */
>  #    define __PROPERTY_IBT 0x1
>  #   else
>  #    define __PROPERTY_IBT 0x0
>  #   endif
> 
> -#   ifdef __SHSTK__
> +#   if (__CET__ & 2) != 0
>  /* GNU_PROPERTY_X86_FEATURE_1_SHSTK.  */
>  #    define __PROPERTY_SHSTK 0x2
>  #   else
> diff --git a/gcc/config/i386/driver-i386.c b/gcc/config/i386/driver-i386.c
> index 19db252dfc0..704cadd8fcf 100644
> --- a/gcc/config/i386/driver-i386.c
> +++ b/gcc/config/i386/driver-i386.c
> @@ -420,7 +420,7 @@ const char *host_detect_local_cpu (int argc, const
> char **argv)
>    unsigned int has_avx5124fmaps = 0, has_avx5124vnniw = 0;
>    unsigned int has_gfni = 0, has_avx512vbmi2 = 0;
>    unsigned int has_avx512bitalg = 0;
> -  unsigned int has_ibt = 0, has_shstk = 0;
> +  unsigned int has_shstk = 0;
>    unsigned int has_avx512vnni = 0, has_vaes = 0;
>    unsigned int has_vpclmulqdq = 0;
>    unsigned int has_movdiri = 0, has_movdir64b = 0;
> @@ -526,7 +526,6 @@ const char *host_detect_local_cpu (int argc, const
> char **argv)
>        has_avx5124fmaps = edx & bit_AVX5124FMAPS;
> 
>        has_shstk = ecx & bit_SHSTK;
> -      has_ibt = edx & bit_IBT;
>        has_pconfig = edx & bit_PCONFIG;
>      }
> 
> @@ -1095,7 +1094,6 @@ const char *host_detect_local_cpu (int argc, const
> char **argv)
>        const char *pku = has_pku ? " -mpku" : " -mno-pku";
>        const char *rdpid = has_rdpid ? " -mrdpid" : " -mno-rdpid";
>        const char *gfni = has_gfni ? " -mgfni" : " -mno-gfni";
> -      const char *ibt = has_ibt ? " -mibt" : " -mno-ibt";
>        const char *shstk = has_shstk ? " -mshstk" : " -mno-shstk";
>        const char *vaes = has_vaes ? " -mvaes" : " -mno-vaes";
>        const char *vpclmulqdq = has_vpclmulqdq ? " -mvpclmulqdq" : " -mno-
> vpclmulqdq";
> @@ -1112,7 +1110,7 @@ const char *host_detect_local_cpu (int argc, const
> char **argv)
>  			avx512cd, avx512pf, prefetchwt1, clflushopt,
>  			xsavec, xsaves, avx512dq, avx512bw, avx512vl,
>  			avx512ifma, avx512vbmi, avx5124fmaps,
> avx5124vnniw,
> -			clwb, mwaitx, clzero, pku, rdpid, gfni, ibt, shstk,
> +			clwb, mwaitx, clzero, pku, rdpid, gfni, shstk,
>  			avx512vbmi2, avx512vnni, vaes, vpclmulqdq,
>  			avx512bitalg, movdiri, movdir64b, NULL);
>      }
> diff --git a/gcc/config/i386/i386-c.c b/gcc/config/i386/i386-c.c
> index fa8b3682b0c..ae7d678e77e 100644
> --- a/gcc/config/i386/i386-c.c
> +++ b/gcc/config/i386/i386-c.c
> @@ -499,20 +499,8 @@ ix86_target_macros_internal (HOST_WIDE_INT
> isa_flag,
>      def_or_undef (parse_in, "__RDPID__");
>    if (isa_flag & OPTION_MASK_ISA_GFNI)
>      def_or_undef (parse_in, "__GFNI__");
> -  if ((isa_flag2 & OPTION_MASK_ISA_IBT)
> -      || (flag_cf_protection & CF_BRANCH))
> -    {
> -      def_or_undef (parse_in, "__IBT__");
> -      if (flag_cf_protection != CF_NONE)
> -	def_or_undef (parse_in, "__CET__");
> -    }
> -  if ((isa_flag & OPTION_MASK_ISA_SHSTK)
> -      || (flag_cf_protection & CF_RETURN))
> -    {
> -      def_or_undef (parse_in, "__SHSTK__");
> -      if (flag_cf_protection != CF_NONE)
> -	def_or_undef (parse_in, "__CET__");
> -    }
> +  if ((isa_flag & OPTION_MASK_ISA_SHSTK))
> +    def_or_undef (parse_in, "__SHSTK__");
>    if (isa_flag2 & OPTION_MASK_ISA_VAES)
>      def_or_undef (parse_in, "__VAES__");
>    if (isa_flag & OPTION_MASK_ISA_VPCLMULQDQ)
> @@ -680,6 +668,10 @@ ix86_target_macros (void)
> 
>    cpp_define (parse_in, "__SEG_FS");
>    cpp_define (parse_in, "__SEG_GS");
> +
> +  if (flag_cf_protection != CF_NONE)
> +    cpp_define_formatted (parse_in, "__CET__=%d",
> +			  flag_cf_protection & ~CF_SET);
>  }
> 
>  

> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index a0435377872..dc80b34f302 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -2766,7 +2766,6 @@ ix86_target_string (HOST_WIDE_INT isa,
> HOST_WIDE_INT isa2,
>      { "-msgx",		OPTION_MASK_ISA_SGX },
>      { "-mavx5124vnniw", OPTION_MASK_ISA_AVX5124VNNIW },
>      { "-mavx5124fmaps", OPTION_MASK_ISA_AVX5124FMAPS },
> -    { "-mibt",		OPTION_MASK_ISA_IBT },
>      { "-mhle",		OPTION_MASK_ISA_HLE },
>      { "-mmovbe",	OPTION_MASK_ISA_MOVBE },
>      { "-mclzero",	OPTION_MASK_ISA_CLZERO },
> @@ -5377,7 +5376,6 @@ ix86_valid_target_attribute_inner_p (tree args,
> char *p_strings[],
>      IX86_ATTR_ISA ("clwb",	OPT_mclwb),
>      IX86_ATTR_ISA ("rdpid",	OPT_mrdpid),
>      IX86_ATTR_ISA ("gfni",	OPT_mgfni),
> -    IX86_ATTR_ISA ("ibt",	OPT_mibt),
>      IX86_ATTR_ISA ("shstk",	OPT_mshstk),
>      IX86_ATTR_ISA ("vaes",	OPT_mvaes),
>      IX86_ATTR_ISA ("vpclmulqdq", OPT_mvpclmulqdq),
> diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
> index 3734ba1bc12..795ad2a322b 100644
> --- a/gcc/config/i386/i386.h
> +++ b/gcc/config/i386/i386.h
> @@ -183,8 +183,6 @@ see the files COPYING3 and COPYING.RUNTIME
> respectively.  If not, see
>  #define TARGET_MWAITX_P(x)	TARGET_ISA_MWAITX_P(x)
>  #define TARGET_PKU	TARGET_ISA_PKU
>  #define TARGET_PKU_P(x)	TARGET_ISA_PKU_P(x)
> -#define TARGET_IBT	TARGET_ISA_IBT
> -#define TARGET_IBT_P(x)	TARGET_ISA_IBT_P(x)
>  #define TARGET_SHSTK	TARGET_ISA_SHSTK
>  #define TARGET_SHSTK_P(x)	TARGET_ISA_SHSTK_P(x)
>  #define TARGET_MOVDIRI	TARGET_ISA_MOVDIRI
> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
> index 33e8060fa56..a134ca88014 100644
> --- a/gcc/config/i386/i386.md
> +++ b/gcc/config/i386/i386.md
> @@ -20332,7 +20332,7 @@
> 
>  (define_insn "nop_endbr"
>    [(unspec_volatile [(const_int 0)] UNSPECV_NOP_ENDBR)]
> -  "TARGET_IBT || (flag_cf_protection & CF_BRANCH)"
> +  "(flag_cf_protection & CF_BRANCH)"
>    "*
>  { return (TARGET_64BIT)? \"endbr64\" : \"endbr32\"; }"
>    [(set_attr "length" "4")
> diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt
> index 646cfcbbd3b..815eceb713d 100644
> --- a/gcc/config/i386/i386.opt
> +++ b/gcc/config/i386/i386.opt
> @@ -1008,17 +1008,13 @@ Generate code which uses only the general
> registers.
> 
>  mcet
>  Target Report Var(flag_cet) Init(0)
> -Support Control-flow Enforcement Technology (CET) built-in functions.
> -
> -mibt
> -Target Report Mask(ISA_IBT) Var(ix86_isa_flags2) Save
> -Specifically enable indirect branch tracking built-in functions from
> -Control-flow Enforcement Technology (CET).
> +Enable shadow stack built-in functions from Control-flow Enforcement
> +Technology (CET).
> 
>  mshstk
>  Target Report Mask(ISA_SHSTK) Var(ix86_isa_flags) Save
> -Specifically enable shadow stack built-in functions from Control-flow
> -Enforcement Technology (CET).
> +Enable shadow stack built-in functions from Control-flow Enforcement
> +Technology (CET).
> 
>  mcet-switch
>  Target Report Undocumented Var(flag_cet_switch) Init(0)
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 0f2c83964f4..b6784b75fa2 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -1261,7 +1261,7 @@ See RS/6000 and PowerPC Options.
>  -msse4a  -m3dnow  -m3dnowa  -mpopcnt  -mabm  -mbmi  -mtbm  -
> mfma4  -mxop @gol
>  -mlzcnt  -mbmi2  -mfxsr  -mxsave  -mxsaveopt  -mrtm  -mlwp  -mmpx
> @gol
>  -mmwaitx  -mclzero  -mpku  -mthreads -mgfni  -mvaes  @gol
> --mcet -mibt -mshstk -mforce-indirect-call -mavx512vbmi2 @gol
> +-mcet -mshstk -mforce-indirect-call -mavx512vbmi2 @gol
>  -mvpclmulqdq -mavx512bitalg -mmovdiri -mmovdir64b -
> mavx512vpopcntdq @gol
>  -mms-bitfields  -mno-align-stringops  -minline-all-stringops @gol
>  -minline-stringops-dynamically  -mstringop-strategy=@var{alg} @gol
> @@ -11829,6 +11829,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 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
> +the @code{return}.
> +
>  You can also use the @code{nocf_check} attribute to identify
>  which functions and calls should be skipped from instrumentation
>  (@pxref{Function Attributes}).
> @@ -27349,11 +27354,6 @@ supported architecture, using the appropriate
> flags.  In particular,
>  the file containing the CPU detection code should be compiled without
>  these options.
> 
> -The @option{-mcet} option turns on the @option{-mibt} and @option{-
> mshstk}
> -options.  The compiler provides a number of built-in functions for
> -fine-grained control in a CET-based application.  See
> -@xref{x86 Built-in Functions}, for more information.
> -
>  @item -mdump-tune-features
>  @opindex mdump-tune-features
>  This option instructs GCC to dump the names of the x86 performance
> @@ -27446,19 +27446,13 @@ see @ref{Other Builtins} for details.
>  This option enables use of the @code{movbe} instruction to implement
>  @code{__builtin_bswap32} and @code{__builtin_bswap64}.
> 
> -@item -mibt
> -@opindex mibt
> -This option enables indirect branch tracking built-in functions from
> -x86 Control-flow Enforcement Technology (CET).  The option
> -@option{-mibt} is on by default when the @code{-mcet} option is
> -specified.
> -
>  @item -mshstk
> +@itemx -mcet
>  @opindex mshstk
> -This option enables shadow stack built-in functions from x86
> -Control-flow Enforcement Technology (CET).  The option
> -@option{-mshstk} is on by default when the @option{-mcet} option is
> -specified.
> +@opindex mcet
> +The @option{-mshstk} option enables shadow stack built-in functions
> +from x86 Control-flow Enforcement Technology (CET).  The @option{-mcet}
> +option is an alias for the @option{-mshstk} option.
> 
>  @item -mcrc32
>  @opindex mcrc32
> diff --git a/gcc/testsuite/gcc.target/i386/pr85044.c
> b/gcc/testsuite/gcc.target/i386/pr85044.c
> index 332f582d79b..a25cc7fe325 100644
> --- a/gcc/testsuite/gcc.target/i386/pr85044.c
> +++ b/gcc/testsuite/gcc.target/i386/pr85044.c
> @@ -1,5 +1,5 @@
>  /* { dg-do run { target cet } } */
> -/* { dg-options "-O2 -fcf-protection=branch -mibt" } */
> +/* { dg-options "-O2 -fcf-protection=branch" } */
> 
>  void callme (void (*callback) (void));
> 
> diff --git a/gcc/testsuite/gcc.target/i386/sse-26.c
> b/gcc/testsuite/gcc.target/i386/sse-26.c
> index f2607e64b59..04ffe10f42a 100644
> --- a/gcc/testsuite/gcc.target/i386/sse-26.c
> +++ b/gcc/testsuite/gcc.target/i386/sse-26.c
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -Werror-implicit-function-declaration -march=k8 -
> msse2 -mmmx -mno-sse3 -mno-3dnow -mno-fma -mno-fxsr -mno-xsave -
> mno-rtm -mno-prfchw -mno-rdseed -mno-adx -mno-prefetchwt1 -mno-
> clflushopt -mno-xsavec -mno-xsaves -mno-clwb -mno-mwaitx -mno-clzero -
> mno-pku -mno-rdpid -mno-gfni -mno-ibt -mno-shstk -mno-vaes -mno-
> vpclmulqdq" } */
> +/* { dg-options "-O2 -Werror-implicit-function-declaration -march=k8 -
> msse2 -mmmx -mno-sse3 -mno-3dnow -mno-fma -mno-fxsr -mno-xsave -
> mno-rtm -mno-prfchw -mno-rdseed -mno-adx -mno-prefetchwt1 -mno-
> clflushopt -mno-xsavec -mno-xsaves -mno-clwb -mno-mwaitx -mno-clzero -
> mno-pku -mno-rdpid -mno-gfni -mno-shstk -mno-vaes -mno-vpclmulqdq"
> } */
>  /* { dg-add-options bind_pic_locally } */
> 
>  #include "sse-13.c"
> --
> 2.14.3



More information about the Gcc-patches mailing list