| Message ID | 20180417184224.GA22831@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | x86: Allow -fcf-protection with multi-byte NOPs | expand |
On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: > -fcf-protection -mcet can't be used with IFUNC features, like symbol > multiversioning or target clone, since IBT/SHSTK are applied to the whole > program and they may be disabled in some functions. But -fcf-protection > is implemented with multi-byte NOPs on all 64-bit processors as well as > 32-bit processors starting with Pentium Pro. If -fcf-protection requires > -mcet, IFUNC features can't be used on Linux when -fcf-protection is > enabled by default. > > This patch changes -fcf-protection to to enable the NOP portion of CET > ISAs unless IBT and/or SHSTK are disabled explicitly. The rest of CET > ISAs, including intrinsics, still requires -mcet, -mibt or -mshstk. > > OK for trunk? As said in the PR, NOP sequences have non-zero cost in the executable (they enlarge the executable), so I don't think this feature should be enabled by default. There is always a configure option if someone wants their compiler to always emit relevant multi-byte nops. Uros.
On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: >> -fcf-protection -mcet can't be used with IFUNC features, like symbol >> multiversioning or target clone, since IBT/SHSTK are applied to the whole >> program and they may be disabled in some functions. But -fcf-protection >> is implemented with multi-byte NOPs on all 64-bit processors as well as >> 32-bit processors starting with Pentium Pro. If -fcf-protection requires >> -mcet, IFUNC features can't be used on Linux when -fcf-protection is >> enabled by default. >> >> This patch changes -fcf-protection to to enable the NOP portion of CET >> ISAs unless IBT and/or SHSTK are disabled explicitly. The rest of CET >> ISAs, including intrinsics, still requires -mcet, -mibt or -mshstk. >> >> OK for trunk? > > As said in the PR, NOP sequences have non-zero cost in the executable > (they enlarge the executable), so I don't think this feature should be > enabled by default. > > There is always a configure option if someone wants their compiler to > always emit relevant multi-byte nops. What we need is an option to enable -fcf-function with multi-byte NOPs without -mcet which enables the full CET ISAs. A configure option without the corresponding the command-line option makes test and debug difficult. I can add --enable-cf-function-nop or --with-cf-function-nop with -fct-function-nop
On Tue, Apr 17, 2018 at 12:03 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >> On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: >>> -fcf-protection -mcet can't be used with IFUNC features, like symbol >>> multiversioning or target clone, since IBT/SHSTK are applied to the whole >>> program and they may be disabled in some functions. But -fcf-protection >>> is implemented with multi-byte NOPs on all 64-bit processors as well as >>> 32-bit processors starting with Pentium Pro. If -fcf-protection requires >>> -mcet, IFUNC features can't be used on Linux when -fcf-protection is >>> enabled by default. >>> >>> This patch changes -fcf-protection to to enable the NOP portion of CET >>> ISAs unless IBT and/or SHSTK are disabled explicitly. The rest of CET >>> ISAs, including intrinsics, still requires -mcet, -mibt or -mshstk. >>> >>> OK for trunk? >> >> As said in the PR, NOP sequences have non-zero cost in the executable >> (they enlarge the executable), so I don't think this feature should be >> enabled by default. >> >> There is always a configure option if someone wants their compiler to >> always emit relevant multi-byte nops. > > What we need is an option to enable -fcf-function with multi-byte NOPs > without -mcet which enables the full CET ISAs. A configure option > without the corresponding the command-line option makes test and > debug difficult. I can add > > --enable-cf-function-nop or --with-cf-function-nop > > with > > -fct-function-nop > How about adding -mno-cet, which enables the NOP portion of CET ISAs?
On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Tue, Apr 17, 2018 at 12:03 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >>> On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: >>>> -fcf-protection -mcet can't be used with IFUNC features, like symbol >>>> multiversioning or target clone, since IBT/SHSTK are applied to the whole >>>> program and they may be disabled in some functions. But -fcf-protection >>>> is implemented with multi-byte NOPs on all 64-bit processors as well as >>>> 32-bit processors starting with Pentium Pro. If -fcf-protection requires >>>> -mcet, IFUNC features can't be used on Linux when -fcf-protection is >>>> enabled by default. >>>> >>>> This patch changes -fcf-protection to to enable the NOP portion of CET >>>> ISAs unless IBT and/or SHSTK are disabled explicitly. The rest of CET >>>> ISAs, including intrinsics, still requires -mcet, -mibt or -mshstk. >>>> >>>> OK for trunk? >>> >>> As said in the PR, NOP sequences have non-zero cost in the executable >>> (they enlarge the executable), so I don't think this feature should be >>> enabled by default. >>> >>> There is always a configure option if someone wants their compiler to >>> always emit relevant multi-byte nops. >> >> What we need is an option to enable -fcf-function with multi-byte NOPs >> without -mcet which enables the full CET ISAs. A configure option >> without the corresponding the command-line option makes test and >> debug difficult. I can add >> >> --enable-cf-function-nop or --with-cf-function-nop >> >> with >> >> -fct-function-nop >> > > How about adding -mno-cet, which enables the NOP portion of CET I meant -mnop-cet, not -mno-cet. > ISAs? > > -- > H.J.
On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Tue, Apr 17, 2018 at 12:03 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>> On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >>>> On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: >>>>> -fcf-protection -mcet can't be used with IFUNC features, like symbol >>>>> multiversioning or target clone, since IBT/SHSTK are applied to the whole >>>>> program and they may be disabled in some functions. But -fcf-protection >>>>> is implemented with multi-byte NOPs on all 64-bit processors as well as >>>>> 32-bit processors starting with Pentium Pro. If -fcf-protection requires >>>>> -mcet, IFUNC features can't be used on Linux when -fcf-protection is >>>>> enabled by default. >>>>> >>>>> This patch changes -fcf-protection to to enable the NOP portion of CET >>>>> ISAs unless IBT and/or SHSTK are disabled explicitly. The rest of CET >>>>> ISAs, including intrinsics, still requires -mcet, -mibt or -mshstk. >>>>> >>>>> OK for trunk? >>>> >>>> As said in the PR, NOP sequences have non-zero cost in the executable >>>> (they enlarge the executable), so I don't think this feature should be >>>> enabled by default. >>>> >>>> There is always a configure option if someone wants their compiler to >>>> always emit relevant multi-byte nops. >>> >>> What we need is an option to enable -fcf-function with multi-byte NOPs >>> without -mcet which enables the full CET ISAs. A configure option >>> without the corresponding the command-line option makes test and >>> debug difficult. I can add >>> >>> --enable-cf-function-nop or --with-cf-function-nop >>> >>> with >>> >>> -fct-function-nop >>> >> >> How about adding -mno-cet, which enables the NOP portion of CET > > I meant -mnop-cet, not -mno-cet. > Here is a patch to add -mnop and use it with -fcf-protection.
On Wed, Apr 18, 2018 at 1:24 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>> On Tue, Apr 17, 2018 at 12:03 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>>> On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >>>>> On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: >>>>>> -fcf-protection -mcet can't be used with IFUNC features, like symbol >>>>>> multiversioning or target clone, since IBT/SHSTK are applied to the whole >>>>>> program and they may be disabled in some functions. But -fcf-protection >>>>>> is implemented with multi-byte NOPs on all 64-bit processors as well as >>>>>> 32-bit processors starting with Pentium Pro. If -fcf-protection requires >>>>>> -mcet, IFUNC features can't be used on Linux when -fcf-protection is >>>>>> enabled by default. >>>>>> >>>>>> This patch changes -fcf-protection to to enable the NOP portion of CET >>>>>> ISAs unless IBT and/or SHSTK are disabled explicitly. The rest of CET >>>>>> ISAs, including intrinsics, still requires -mcet, -mibt or -mshstk. >>>>>> >>>>>> OK for trunk? >>>>> >>>>> As said in the PR, NOP sequences have non-zero cost in the executable >>>>> (they enlarge the executable), so I don't think this feature should be >>>>> enabled by default. >>>>> >>>>> There is always a configure option if someone wants their compiler to >>>>> always emit relevant multi-byte nops. >>>> >>>> What we need is an option to enable -fcf-function with multi-byte NOPs >>>> without -mcet which enables the full CET ISAs. A configure option >>>> without the corresponding the command-line option makes test and >>>> debug difficult. I can add >>>> >>>> --enable-cf-function-nop or --with-cf-function-nop >>>> >>>> with >>>> >>>> -fct-function-nop >>>> >>> >>> How about adding -mno-cet, which enables the NOP portion of CET >> >> I meant -mnop-cet, not -mno-cet. >> > > Here is a patch to add -mnop and use it with -fcf-protection. +mnop +Target Report Var(flag_nop) Init(0) +Support multi-byte NOP code generation. the option name is incredibly bad and the documentation doesn't make it better either. The invoke.texi docs refer to duplicate {-mcet}. Isn't there a -fcf-protection sub-set that can be used to automatically enable this? Or simply do this mode by default when -fcf-protection is used but neither -mcet nor -mibt is enabled? > > -- > H.J.
On Wed, Apr 18, 2018 at 4:35 AM, Richard Biener <richard.guenther@gmail.com> wrote: > On Wed, Apr 18, 2018 at 1:24 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>> On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>>> On Tue, Apr 17, 2018 at 12:03 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>>>> On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >>>>>> On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: >>>>>>> -fcf-protection -mcet can't be used with IFUNC features, like symbol >>>>>>> multiversioning or target clone, since IBT/SHSTK are applied to the whole >>>>>>> program and they may be disabled in some functions. But -fcf-protection >>>>>>> is implemented with multi-byte NOPs on all 64-bit processors as well as >>>>>>> 32-bit processors starting with Pentium Pro. If -fcf-protection requires >>>>>>> -mcet, IFUNC features can't be used on Linux when -fcf-protection is >>>>>>> enabled by default. >>>>>>> >>>>>>> This patch changes -fcf-protection to to enable the NOP portion of CET >>>>>>> ISAs unless IBT and/or SHSTK are disabled explicitly. The rest of CET >>>>>>> ISAs, including intrinsics, still requires -mcet, -mibt or -mshstk. >>>>>>> >>>>>>> OK for trunk? >>>>>> >>>>>> As said in the PR, NOP sequences have non-zero cost in the executable >>>>>> (they enlarge the executable), so I don't think this feature should be >>>>>> enabled by default. >>>>>> >>>>>> There is always a configure option if someone wants their compiler to >>>>>> always emit relevant multi-byte nops. >>>>> >>>>> What we need is an option to enable -fcf-function with multi-byte NOPs >>>>> without -mcet which enables the full CET ISAs. A configure option >>>>> without the corresponding the command-line option makes test and >>>>> debug difficult. I can add >>>>> >>>>> --enable-cf-function-nop or --with-cf-function-nop >>>>> >>>>> with >>>>> >>>>> -fct-function-nop >>>>> >>>> >>>> How about adding -mno-cet, which enables the NOP portion of CET >>> >>> I meant -mnop-cet, not -mno-cet. >>> >> >> Here is a patch to add -mnop and use it with -fcf-protection. > > +mnop > +Target Report Var(flag_nop) Init(0) > +Support multi-byte NOP code generation. > > the option name is incredibly bad and the documentation doesn't make it > better either. The invoke.texi docs refer to duplicate {-mcet}. > > Isn't there a -fcf-protection sub-set that can be used to automatically > enable this? Or simply do this mode by default when > -fcf-protection is used but neither -mcet nor -mibt is enabled? Make -fcf-protection default to multi-byte NOPs works. Uros, should I prepare a patch?
On Wed, Apr 18, 2018 at 1:39 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>> Here is a patch to add -mnop and use it with -fcf-protection. >> >> +mnop >> +Target Report Var(flag_nop) Init(0) >> +Support multi-byte NOP code generation. >> >> the option name is incredibly bad and the documentation doesn't make it >> better either. The invoke.texi docs refer to duplicate {-mcet}. >> >> Isn't there a -fcf-protection sub-set that can be used to automatically >> enable this? Or simply do this mode by default when >> -fcf-protection is used but neither -mcet nor -mibt is enabled? > > Make -fcf-protection default to multi-byte NOPs works. Uros, > should I prepare a patch? Please make it an opt-in feature, so the compiler won't litter the executable with unnecessary nops without user consent. Uros.
On Wed, Apr 18, 2018 at 4:55 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Wed, Apr 18, 2018 at 1:39 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > >>>> Here is a patch to add -mnop and use it with -fcf-protection. >>> >>> +mnop >>> +Target Report Var(flag_nop) Init(0) >>> +Support multi-byte NOP code generation. >>> >>> the option name is incredibly bad and the documentation doesn't make it >>> better either. The invoke.texi docs refer to duplicate {-mcet}. >>> >>> Isn't there a -fcf-protection sub-set that can be used to automatically >>> enable this? Or simply do this mode by default when >>> -fcf-protection is used but neither -mcet nor -mibt is enabled? >> >> Make -fcf-protection default to multi-byte NOPs works. Uros, >> should I prepare a patch? > > Please make it an opt-in feature, so the compiler won't litter the > executable with unnecessary nops without user consent. > -fcf-protection is off by default. Users need to pass -fcf-protection to enable it. I will work on such a patch. Thanks.
On Wed, Apr 18, 2018 at 04:57:41AM -0700, H.J. Lu wrote: > On Wed, Apr 18, 2018 at 4:55 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > > On Wed, Apr 18, 2018 at 1:39 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > > > >>>> Here is a patch to add -mnop and use it with -fcf-protection. > >>> > >>> +mnop > >>> +Target Report Var(flag_nop) Init(0) > >>> +Support multi-byte NOP code generation. > >>> > >>> the option name is incredibly bad and the documentation doesn't make it > >>> better either. The invoke.texi docs refer to duplicate {-mcet}. > >>> > >>> Isn't there a -fcf-protection sub-set that can be used to automatically > >>> enable this? Or simply do this mode by default when > >>> -fcf-protection is used but neither -mcet nor -mibt is enabled? > >> > >> Make -fcf-protection default to multi-byte NOPs works. Uros, > >> should I prepare a patch? > > > > Please make it an opt-in feature, so the compiler won't litter the > > executable with unnecessary nops without user consent. > > > > -fcf-protection is off by default. Users need to pass -fcf-protection > to enable it. I will work on such a patch. That is not true. When building gcc itself, config/cet.m4 makes -fcf-protection -mcet the default if assembler supports it. The request was to change --enable-cet configure option from having yes,no,default arguments with default autodetection and being a default if --enable-cet*/--disable-cet is not specified to say yes,no,auto arguments where no would be the default and auto would be the current default - enable it if as supports it, disable otherwise. Jakub
On Wed, Apr 18, 2018 at 1:57 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Wed, Apr 18, 2018 at 4:55 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >> On Wed, Apr 18, 2018 at 1:39 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> >>>>> Here is a patch to add -mnop and use it with -fcf-protection. >>>> >>>> +mnop >>>> +Target Report Var(flag_nop) Init(0) >>>> +Support multi-byte NOP code generation. >>>> >>>> the option name is incredibly bad and the documentation doesn't make it >>>> better either. The invoke.texi docs refer to duplicate {-mcet}. >>>> >>>> Isn't there a -fcf-protection sub-set that can be used to automatically >>>> enable this? Or simply do this mode by default when >>>> -fcf-protection is used but neither -mcet nor -mibt is enabled? >>> >>> Make -fcf-protection default to multi-byte NOPs works. Uros, >>> should I prepare a patch? >> >> Please make it an opt-in feature, so the compiler won't litter the >> executable with unnecessary nops without user consent. >> > > -fcf-protection is off by default. Users need to pass -fcf-protection > to enable it. I will work on such a patch. Please note that currently all libraries are compiled with "-fcf-protection -mcet" by default, even without using --enable-cet during configure. The CET instrumentation of libraries should be put under strict user control, so please remove the "default" from config/cet.m4. Uros.
On Wed, Apr 18, 2018 at 5:08 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Wed, Apr 18, 2018 at 1:57 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Wed, Apr 18, 2018 at 4:55 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >>> On Wed, Apr 18, 2018 at 1:39 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>> >>>>>> Here is a patch to add -mnop and use it with -fcf-protection. >>>>> >>>>> +mnop >>>>> +Target Report Var(flag_nop) Init(0) >>>>> +Support multi-byte NOP code generation. >>>>> >>>>> the option name is incredibly bad and the documentation doesn't make it >>>>> better either. The invoke.texi docs refer to duplicate {-mcet}. >>>>> >>>>> Isn't there a -fcf-protection sub-set that can be used to automatically >>>>> enable this? Or simply do this mode by default when >>>>> -fcf-protection is used but neither -mcet nor -mibt is enabled? >>>> >>>> Make -fcf-protection default to multi-byte NOPs works. Uros, >>>> should I prepare a patch? >>> >>> Please make it an opt-in feature, so the compiler won't litter the >>> executable with unnecessary nops without user consent. >>> >> >> -fcf-protection is off by default. Users need to pass -fcf-protection >> to enable it. I will work on such a patch. > > Please note that currently all libraries are compiled with > "-fcf-protection -mcet" by default, even without using --enable-cet > during configure. The CET instrumentation of libraries should be put > under strict user control, so please remove the "default" from > config/cet.m4. > Igor, please prepare such a patch to config/cet.m4.
On Wed, Apr 18, 2018 at 02:04:50PM +0200, Jakub Jelinek wrote: > On Wed, Apr 18, 2018 at 04:57:41AM -0700, H.J. Lu wrote: > > On Wed, Apr 18, 2018 at 4:55 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > > > On Wed, Apr 18, 2018 at 1:39 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > >>>> Here is a patch to add -mnop and use it with -fcf-protection. > > >>> > > >>> +mnop > > >>> +Target Report Var(flag_nop) Init(0) > > >>> +Support multi-byte NOP code generation. > > >>> > > >>> the option name is incredibly bad and the documentation doesn't make it > > >>> better either. The invoke.texi docs refer to duplicate {-mcet}. > > >>> > > >>> Isn't there a -fcf-protection sub-set that can be used to automatically > > >>> enable this? Or simply do this mode by default when > > >>> -fcf-protection is used but neither -mcet nor -mibt is enabled? > > >> > > >> Make -fcf-protection default to multi-byte NOPs works. Uros, > > >> should I prepare a patch? > > > > > > Please make it an opt-in feature, so the compiler won't litter the > > > executable with unnecessary nops without user consent. > > > > > > > -fcf-protection is off by default. Users need to pass -fcf-protection > > to enable it. I will work on such a patch. > > That is not true. When building gcc itself, config/cet.m4 makes > -fcf-protection -mcet the default if assembler supports it. > The request was to change --enable-cet configure option from having > yes,no,default arguments with default autodetection and being a default > if --enable-cet*/--disable-cet is not specified to say > yes,no,auto arguments where no would be the default and auto would be the > current default - enable it if as supports it, disable otherwise. So untested patch would be something like: 2018-04-18 Jakub Jelinek <jakub@redhat.com> * config/cet.m4 (GCC_CET_FLAGS): Default to --disable-cet, replace --enable-cet=default with --enable-cet=auto. * doc/install.texi: Document --disable-cet being the default and --enable-cet=auto. --- gcc/config/cet.m4.jj 2018-02-19 19:57:05.221280084 +0100 +++ gcc/config/cet.m4 2018-04-18 14:05:31.514859185 +0200 @@ -3,14 +3,14 @@ dnl GCC_CET_FLAGS dnl (SHELL-CODE_HANDLER) dnl AC_DEFUN([GCC_CET_FLAGS],[dnl -GCC_ENABLE(cet, default, ,[enable Intel CET in target libraries], - permit yes|no|default) +GCC_ENABLE(cet, no, ,[enable Intel CET in target libraries], + permit yes|no|auto) AC_MSG_CHECKING([for CET support]) case "$host" in i[[34567]]86-*-linux* | x86_64-*-linux*) case "$enable_cet" in - default) + auto) # Check if target supports multi-byte NOPs # and if assembler supports CET insn. AC_COMPILE_IFELSE( --- gcc/doc/install.texi.jj 2018-02-08 12:21:20.791749480 +0100 +++ gcc/doc/install.texi 2018-04-18 14:07:19.637901528 +0200 @@ -2103,10 +2103,11 @@ instrumentation, see @option{-fcf-protec to add @option{-fcf-protection} and, if needed, other target specific options to a set of building options. -The option is enabled by default on Linux/x86 if target binutils -supports @code{Intel CET} instructions. In this case the target -libraries are configured to get additional @option{-fcf-protection} -and @option{-mcet} options. +The option is disabled by default on Linux/x86. When +@code{--enable-cet=auto} is used, it is enabled if target binutils +supports @code{Intel CET} instructions and disabled otherwise. +In this case the target libraries are configured to get additional +@option{-fcf-protection} and @option{-mcet} options. @end table @subheading Cross-Compiler-Specific Options Jakub
> -----Original Message----- > From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches- > owner@gcc.gnu.org] On Behalf Of H.J. Lu > Sent: Wednesday, April 18, 2018 1:39 PM > To: Richard Biener <richard.guenther@gmail.com> > Cc: Uros Bizjak <ubizjak@gmail.com>; gcc-patches@gcc.gnu.org; Tsimbalist, > Igor V <igor.v.tsimbalist@intel.com> > Subject: Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs > > On Wed, Apr 18, 2018 at 4:35 AM, Richard Biener > <richard.guenther@gmail.com> wrote: > > On Wed, Apr 18, 2018 at 1:24 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > >> On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > >>> On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu <hjl.tools@gmail.com> > wrote: > >>>> On Tue, Apr 17, 2018 at 12:03 PM, H.J. Lu <hjl.tools@gmail.com> > wrote: > >>>>> On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak > <ubizjak@gmail.com> wrote: > >>>>>> On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu <hongjiu.lu@intel.com> > wrote: > >>>>>>> -fcf-protection -mcet can't be used with IFUNC features, like > symbol > >>>>>>> multiversioning or target clone, since IBT/SHSTK are applied to > the whole > >>>>>>> program and they may be disabled in some functions. But -fcf- > protection > >>>>>>> is implemented with multi-byte NOPs on all 64-bit processors as > well as > >>>>>>> 32-bit processors starting with Pentium Pro. If -fcf-protection > requires > >>>>>>> -mcet, IFUNC features can't be used on Linux when -fcf- > protection is > >>>>>>> enabled by default. > >>>>>>> > >>>>>>> This patch changes -fcf-protection to to enable the NOP portion > of CET > >>>>>>> ISAs unless IBT and/or SHSTK are disabled explicitly. The rest of > CET > >>>>>>> ISAs, including intrinsics, still requires -mcet, -mibt or -mshstk. > >>>>>>> > >>>>>>> OK for trunk? > >>>>>> > >>>>>> As said in the PR, NOP sequences have non-zero cost in the > executable > >>>>>> (they enlarge the executable), so I don't think this feature should > be > >>>>>> enabled by default. > >>>>>> > >>>>>> There is always a configure option if someone wants their compiler > to > >>>>>> always emit relevant multi-byte nops. > >>>>> > >>>>> What we need is an option to enable -fcf-function with multi-byte > NOPs > >>>>> without -mcet which enables the full CET ISAs. A configure option > >>>>> without the corresponding the command-line option makes test and > >>>>> debug difficult. I can add > >>>>> > >>>>> --enable-cf-function-nop or --with-cf-function-nop > >>>>> > >>>>> with > >>>>> > >>>>> -fct-function-nop > >>>>> > >>>> > >>>> How about adding -mno-cet, which enables the NOP portion of CET > >>> > >>> I meant -mnop-cet, not -mno-cet. > >>> > >> > >> Here is a patch to add -mnop and use it with -fcf-protection. > > > > +mnop > > +Target Report Var(flag_nop) Init(0) > > +Support multi-byte NOP code generation. > > > > the option name is incredibly bad and the documentation doesn't make it > > better either. The invoke.texi docs refer to duplicate {-mcet}. > > > > Isn't there a -fcf-protection sub-set that can be used to automatically > > enable this? Or simply do this mode by default when > > -fcf-protection is used but neither -mcet nor -mibt is enabled? > > Make -fcf-protection default to multi-byte NOPs works. Uros, > should I prepare a patch? This is going to change the designed approach and has to be communicated to/agreed with other compilers. And I assume there will be no extra option introduced, like -mnop. Igor > -- > H.J.
> -----Original Message----- > From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches- > owner@gcc.gnu.org] On Behalf Of Jakub Jelinek > Sent: Wednesday, April 18, 2018 2:10 PM > To: H.J. Lu <hjl.tools@gmail.com> > Cc: Uros Bizjak <ubizjak@gmail.com>; Richard Biener > <richard.guenther@gmail.com>; gcc-patches@gcc.gnu.org; Tsimbalist, Igor > V <igor.v.tsimbalist@intel.com> > Subject: Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs > > On Wed, Apr 18, 2018 at 02:04:50PM +0200, Jakub Jelinek wrote: > > On Wed, Apr 18, 2018 at 04:57:41AM -0700, H.J. Lu wrote: > > > On Wed, Apr 18, 2018 at 4:55 AM, Uros Bizjak <ubizjak@gmail.com> > wrote: > > > > On Wed, Apr 18, 2018 at 1:39 PM, H.J. Lu <hjl.tools@gmail.com> > wrote: > > > > > > > >>>> Here is a patch to add -mnop and use it with -fcf-protection. > > > >>> > > > >>> +mnop > > > >>> +Target Report Var(flag_nop) Init(0) > > > >>> +Support multi-byte NOP code generation. > > > >>> > > > >>> the option name is incredibly bad and the documentation doesn't > make it > > > >>> better either. The invoke.texi docs refer to duplicate {-mcet}. > > > >>> > > > >>> Isn't there a -fcf-protection sub-set that can be used to > automatically > > > >>> enable this? Or simply do this mode by default when > > > >>> -fcf-protection is used but neither -mcet nor -mibt is enabled? > > > >> > > > >> Make -fcf-protection default to multi-byte NOPs works. Uros, > > > >> should I prepare a patch? > > > > > > > > Please make it an opt-in feature, so the compiler won't litter the > > > > executable with unnecessary nops without user consent. > > > > > > > > > > -fcf-protection is off by default. Users need to pass -fcf-protection > > > to enable it. I will work on such a patch. > > > > That is not true. When building gcc itself, config/cet.m4 makes > > -fcf-protection -mcet the default if assembler supports it. > > The request was to change --enable-cet configure option from having > > yes,no,default arguments with default autodetection and being a default > > if --enable-cet*/--disable-cet is not specified to say > > yes,no,auto arguments where no would be the default and auto would be > the > > current default - enable it if as supports it, disable otherwise. > > So untested patch would be something like: > > 2018-04-18 Jakub Jelinek <jakub@redhat.com> > > * config/cet.m4 (GCC_CET_FLAGS): Default to --disable-cet, replace > --enable-cet=default with --enable-cet=auto. > > * doc/install.texi: Document --disable-cet being the default and > --enable-cet=auto. > > --- gcc/config/cet.m4.jj 2018-02-19 19:57:05.221280084 +0100 > +++ gcc/config/cet.m4 2018-04-18 14:05:31.514859185 +0200 > @@ -3,14 +3,14 @@ dnl GCC_CET_FLAGS > dnl (SHELL-CODE_HANDLER) > dnl > AC_DEFUN([GCC_CET_FLAGS],[dnl > -GCC_ENABLE(cet, default, ,[enable Intel CET in target libraries], > - permit yes|no|default) > +GCC_ENABLE(cet, no, ,[enable Intel CET in target libraries], > + permit yes|no|auto) > AC_MSG_CHECKING([for CET support]) > > case "$host" in > i[[34567]]86-*-linux* | x86_64-*-linux*) > case "$enable_cet" in > - default) > + auto) > # Check if target supports multi-byte NOPs > # and if assembler supports CET insn. > AC_COMPILE_IFELSE( > --- gcc/doc/install.texi.jj 2018-02-08 12:21:20.791749480 +0100 > +++ gcc/doc/install.texi 2018-04-18 14:07:19.637901528 +0200 > @@ -2103,10 +2103,11 @@ instrumentation, see @option{-fcf-protec > to add @option{-fcf-protection} and, if needed, other target > specific options to a set of building options. > > -The option is enabled by default on Linux/x86 if target binutils > -supports @code{Intel CET} instructions. In this case the target > -libraries are configured to get additional @option{-fcf-protection} > -and @option{-mcet} options. > +The option is disabled by default on Linux/x86. When > +@code{--enable-cet=auto} is used, it is enabled if target binutils > +supports @code{Intel CET} instructions and disabled otherwise. > +In this case the target libraries are configured to get additional > +@option{-fcf-protection} and @option{-mcet} options. > @end table > > @subheading Cross-Compiler-Specific Options > Thanks! I will work on this. > Jakub
On Wed, Apr 18, 2018 at 2:09 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Wed, Apr 18, 2018 at 02:04:50PM +0200, Jakub Jelinek wrote: >> On Wed, Apr 18, 2018 at 04:57:41AM -0700, H.J. Lu wrote: >> > On Wed, Apr 18, 2018 at 4:55 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >> > > On Wed, Apr 18, 2018 at 1:39 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> > > >> > >>>> Here is a patch to add -mnop and use it with -fcf-protection. >> > >>> >> > >>> +mnop >> > >>> +Target Report Var(flag_nop) Init(0) >> > >>> +Support multi-byte NOP code generation. >> > >>> >> > >>> the option name is incredibly bad and the documentation doesn't make it >> > >>> better either. The invoke.texi docs refer to duplicate {-mcet}. >> > >>> >> > >>> Isn't there a -fcf-protection sub-set that can be used to automatically >> > >>> enable this? Or simply do this mode by default when >> > >>> -fcf-protection is used but neither -mcet nor -mibt is enabled? >> > >> >> > >> Make -fcf-protection default to multi-byte NOPs works. Uros, >> > >> should I prepare a patch? >> > > >> > > Please make it an opt-in feature, so the compiler won't litter the >> > > executable with unnecessary nops without user consent. >> > > >> > >> > -fcf-protection is off by default. Users need to pass -fcf-protection >> > to enable it. I will work on such a patch. >> >> That is not true. When building gcc itself, config/cet.m4 makes >> -fcf-protection -mcet the default if assembler supports it. >> The request was to change --enable-cet configure option from having >> yes,no,default arguments with default autodetection and being a default >> if --enable-cet*/--disable-cet is not specified to say >> yes,no,auto arguments where no would be the default and auto would be the >> current default - enable it if as supports it, disable otherwise. > > So untested patch would be something like: Yes, this is what I think should be the most appropriate approach. Uros. > 2018-04-18 Jakub Jelinek <jakub@redhat.com> > > * config/cet.m4 (GCC_CET_FLAGS): Default to --disable-cet, replace > --enable-cet=default with --enable-cet=auto. > > * doc/install.texi: Document --disable-cet being the default and > --enable-cet=auto. > > --- gcc/config/cet.m4.jj 2018-02-19 19:57:05.221280084 +0100 > +++ gcc/config/cet.m4 2018-04-18 14:05:31.514859185 +0200 > @@ -3,14 +3,14 @@ dnl GCC_CET_FLAGS > dnl (SHELL-CODE_HANDLER) > dnl > AC_DEFUN([GCC_CET_FLAGS],[dnl > -GCC_ENABLE(cet, default, ,[enable Intel CET in target libraries], > - permit yes|no|default) > +GCC_ENABLE(cet, no, ,[enable Intel CET in target libraries], > + permit yes|no|auto) > AC_MSG_CHECKING([for CET support]) > > case "$host" in > i[[34567]]86-*-linux* | x86_64-*-linux*) > case "$enable_cet" in > - default) > + auto) > # Check if target supports multi-byte NOPs > # and if assembler supports CET insn. > AC_COMPILE_IFELSE( > --- gcc/doc/install.texi.jj 2018-02-08 12:21:20.791749480 +0100 > +++ gcc/doc/install.texi 2018-04-18 14:07:19.637901528 +0200 > @@ -2103,10 +2103,11 @@ instrumentation, see @option{-fcf-protec > to add @option{-fcf-protection} and, if needed, other target > specific options to a set of building options. > > -The option is enabled by default on Linux/x86 if target binutils > -supports @code{Intel CET} instructions. In this case the target > -libraries are configured to get additional @option{-fcf-protection} > -and @option{-mcet} options. > +The option is disabled by default on Linux/x86. When > +@code{--enable-cet=auto} is used, it is enabled if target binutils > +supports @code{Intel CET} instructions and disabled otherwise. > +In this case the target libraries are configured to get additional > +@option{-fcf-protection} and @option{-mcet} options. > @end table > > @subheading Cross-Compiler-Specific Options > > > Jakub
On Wed, Apr 18, 2018 at 5:32 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Wed, Apr 18, 2018 at 2:09 PM, Jakub Jelinek <jakub@redhat.com> wrote: >> On Wed, Apr 18, 2018 at 02:04:50PM +0200, Jakub Jelinek wrote: >>> On Wed, Apr 18, 2018 at 04:57:41AM -0700, H.J. Lu wrote: >>> > On Wed, Apr 18, 2018 at 4:55 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >>> > > On Wed, Apr 18, 2018 at 1:39 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>> > > >>> > >>>> Here is a patch to add -mnop and use it with -fcf-protection. >>> > >>> >>> > >>> +mnop >>> > >>> +Target Report Var(flag_nop) Init(0) >>> > >>> +Support multi-byte NOP code generation. >>> > >>> >>> > >>> the option name is incredibly bad and the documentation doesn't make it >>> > >>> better either. The invoke.texi docs refer to duplicate {-mcet}. >>> > >>> >>> > >>> Isn't there a -fcf-protection sub-set that can be used to automatically >>> > >>> enable this? Or simply do this mode by default when >>> > >>> -fcf-protection is used but neither -mcet nor -mibt is enabled? >>> > >> >>> > >> Make -fcf-protection default to multi-byte NOPs works. Uros, >>> > >> should I prepare a patch? >>> > > >>> > > Please make it an opt-in feature, so the compiler won't litter the >>> > > executable with unnecessary nops without user consent. >>> > > >>> > >>> > -fcf-protection is off by default. Users need to pass -fcf-protection >>> > to enable it. I will work on such a patch. >>> >>> That is not true. When building gcc itself, config/cet.m4 makes >>> -fcf-protection -mcet the default if assembler supports it. >>> The request was to change --enable-cet configure option from having >>> yes,no,default arguments with default autodetection and being a default >>> if --enable-cet*/--disable-cet is not specified to say >>> yes,no,auto arguments where no would be the default and auto would be the >>> current default - enable it if as supports it, disable otherwise. >> >> So untested patch would be something like: > > Yes, this is what I think should be the most appropriate approach. > > Uros. > >> 2018-04-18 Jakub Jelinek <jakub@redhat.com> >> >> * config/cet.m4 (GCC_CET_FLAGS): Default to --disable-cet, replace >> --enable-cet=default with --enable-cet=auto. >> >> * doc/install.texi: Document --disable-cet being the default and >> --enable-cet=auto. >> >> --- gcc/config/cet.m4.jj 2018-02-19 19:57:05.221280084 +0100 >> +++ gcc/config/cet.m4 2018-04-18 14:05:31.514859185 +0200 >> @@ -3,14 +3,14 @@ dnl GCC_CET_FLAGS >> dnl (SHELL-CODE_HANDLER) >> dnl >> AC_DEFUN([GCC_CET_FLAGS],[dnl >> -GCC_ENABLE(cet, default, ,[enable Intel CET in target libraries], >> - permit yes|no|default) >> +GCC_ENABLE(cet, no, ,[enable Intel CET in target libraries], >> + permit yes|no|auto) >> AC_MSG_CHECKING([for CET support]) >> >> case "$host" in >> i[[34567]]86-*-linux* | x86_64-*-linux*) >> case "$enable_cet" in >> - default) >> + auto) >> # Check if target supports multi-byte NOPs >> # and if assembler supports CET insn. >> AC_COMPILE_IFELSE( >> --- gcc/doc/install.texi.jj 2018-02-08 12:21:20.791749480 +0100 >> +++ gcc/doc/install.texi 2018-04-18 14:07:19.637901528 +0200 >> @@ -2103,10 +2103,11 @@ instrumentation, see @option{-fcf-protec >> to add @option{-fcf-protection} and, if needed, other target >> specific options to a set of building options. >> >> -The option is enabled by default on Linux/x86 if target binutils >> -supports @code{Intel CET} instructions. In this case the target >> -libraries are configured to get additional @option{-fcf-protection} >> -and @option{-mcet} options. >> +The option is disabled by default on Linux/x86. When >> +@code{--enable-cet=auto} is used, it is enabled if target binutils >> +supports @code{Intel CET} instructions and disabled otherwise. >> +In this case the target libraries are configured to get additional >> +@option{-fcf-protection} and @option{-mcet} options. >> @end table >> >> @subheading Cross-Compiler-Specific Options >> >> >> Jakub Looks good to me. Thanks.
On Wed, Apr 18, 2018 at 01:35:33PM +0200, Richard Biener wrote: > On Wed, Apr 18, 2018 at 1:24 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > > On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > >> On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > >>> On Tue, Apr 17, 2018 at 12:03 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > >>>> On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > >>>>> On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: > >>>>>> -fcf-protection -mcet can't be used with IFUNC features, like symbol > >>>>>> multiversioning or target clone, since IBT/SHSTK are applied to the whole > >>>>>> program and they may be disabled in some functions. But -fcf-protection > >>>>>> is implemented with multi-byte NOPs on all 64-bit processors as well as > >>>>>> 32-bit processors starting with Pentium Pro. If -fcf-protection requires > >>>>>> -mcet, IFUNC features can't be used on Linux when -fcf-protection is > >>>>>> enabled by default. > >>>>>> > >>>>>> This patch changes -fcf-protection to to enable the NOP portion of CET > >>>>>> ISAs unless IBT and/or SHSTK are disabled explicitly. The rest of CET > >>>>>> ISAs, including intrinsics, still requires -mcet, -mibt or -mshstk. > >>>>>> > >>>>>> OK for trunk? > >>>>> > >>>>> As said in the PR, NOP sequences have non-zero cost in the executable > >>>>> (they enlarge the executable), so I don't think this feature should be > >>>>> enabled by default. > >>>>> > >>>>> There is always a configure option if someone wants their compiler to > >>>>> always emit relevant multi-byte nops. > >>>> > >>>> What we need is an option to enable -fcf-function with multi-byte NOPs > >>>> without -mcet which enables the full CET ISAs. A configure option > >>>> without the corresponding the command-line option makes test and > >>>> debug difficult. I can add > >>>> > >>>> --enable-cf-function-nop or --with-cf-function-nop > >>>> > >>>> with > >>>> > >>>> -fct-function-nop > >>>> > >>> > >>> How about adding -mno-cet, which enables the NOP portion of CET > >> > >> I meant -mnop-cet, not -mno-cet. > >> > > > > Here is a patch to add -mnop and use it with -fcf-protection. > > +mnop > +Target Report Var(flag_nop) Init(0) > +Support multi-byte NOP code generation. > > the option name is incredibly bad and the documentation doesn't make it > better either. The invoke.texi docs refer to duplicate {-mcet}. > > Isn't there a -fcf-protection sub-set that can be used to automatically > enable this? Or simply do this mode by default when > -fcf-protection is used but neither -mcet nor -mibt is enabled? > Since multi-byte NOPs are used to implement -fcf-protection on x86, we propose a new design for -fcf-protection: 1. -fcf-protection option will report the unsupported error on non-x86 platform. On x86 platform it's supported and inserts endbr-nop instructions and properties, depending on its value (full/branch/return) 2. -mcet/-mibt/-mshstk options control intrinsics only. 3. These options are independent and do not influence each other so no need for cross checking between them. OK for trunk? H.J. ---- ---- -fcf-protection -mcet can't be used with IFUNC features, like symbol multiversioning or target clone, since IBT/SHSTK are applied to the whole program and they may be disabled in some functions. But -fcf-protection is implemented with multi-byte NOPs on all 64-bit processors as well as 32-bit processors starting with Pentium Pro. If -fcf-protection requires -mcet, IFUNC features can't be used on Linux when -fcf-protection is enabled by default. This patch changes -fcf-protection to implement indirect branch and return address tracking with multi-byte NOPs. -mibt and -mshstk are changed to only CET built-in functions. CET tests are updated to allow -fcf-protection without -mibt, -mshstk and -mcet on x86. -fcf-protection=none are also added to tests which fail with -fcf-protection so that -fcf-protection can be added to RUNTESTFLAGS to verify -fcf-protection implementation. gcc/ PR target/85417 * config/i386/cet.c (file_end_indicate_exec_stack_and_cet): Check flag_cf_protection instead of TARGET_IBT and TARGET_SHSTK. * config/i386/i386-c.c (ix86_target_macros_internal): Also define __IBT__ and __SHSTK__ for -fcf-protection. * config/i386/i386.c (pass_insert_endbranch::gate): Don't check TARGET_IBT. (ix86_trampoline_init): Likewise. (x86_output_mi_thunk): Likewise. (ix86_notrack_prefixed_insn_p): Likewise. (ix86_option_override_internal): Don't disallow -fcf-protection. * config/i386/i386.md (rdssp<mode>): Also enable for -fcf-protection. (incssp<mode>): Likewise. (nop_endbr): Likewise. * config/i386/i386.opt (mcet): Change help message to built-in functions only. (mibt): Likewise. (mshstk): Likewise. * doc/invoke.texi: Remove -mcet, -mibt and -mshstk condition on -fcf-protection. Change -mcet, -mibt and -mshstk to only enable CET built-in functions. gcc/testsuite/ PR target/85417 * c-c++-common/attr-nocf-check-1.c: Compile with -fcf-protection=none. * c-c++-common/attr-nocf-check-3.c: Likewise. * gcc.dg/march-generic.c: Likewise. * gcc.target/i386/align-limit.c: Likewise. * gcc.target/i386/cet-notrack-icf-1.c: Likewise. * gcc.target/i386/cet-notrack-icf-3.c: Likewise. * gcc.target/i386/cet-property-2.c: Likewise. * gcc.target/i386/ret-thunk-26.c: Likewise. * c-c++-common/fcf-protection-1.c: Remove dg-error for x86 targets. * c-c++-common/fcf-protection-2.c: Likewise. * c-c++-common/fcf-protection-3.c: Likewise. * c-c++-common/fcf-protection-5.c: Likewise. * c-c++-common/fcf-protection-6.c: Likewise. * c-c++-common/fcf-protection-7.c: Likewise. * gcc.target/i386/cet-label-3.c: New test. * gcc.target/i386/cet-property-3.c: Likewise. * gcc.target/i386/cet-sjlj-7.c: Likewise. * gcc.target/i386/pr85417-1.c: Likewise. * gcc.target/i386/indirect-thunk-attr-7.c: Also expect __x86_indirect_thunk_nt_(r|e)ax * gcc.target/i386/indirect-thunk-extern-7.c: Likewise. * gcc.target/i386/pr85403.c: Remove dg-error, --- gcc/config/i386/cet.c | 4 +- gcc/config/i386/i386-c.c | 6 ++- gcc/config/i386/i386.c | 54 +++------------------- gcc/config/i386/i386.md | 6 +-- gcc/config/i386/i386.opt | 9 ++-- gcc/doc/invoke.texi | 28 ++++------- gcc/testsuite/c-c++-common/attr-nocf-check-1.c | 1 + gcc/testsuite/c-c++-common/attr-nocf-check-3.c | 1 + gcc/testsuite/c-c++-common/fcf-protection-1.c | 1 - gcc/testsuite/c-c++-common/fcf-protection-2.c | 1 - gcc/testsuite/c-c++-common/fcf-protection-3.c | 1 - gcc/testsuite/c-c++-common/fcf-protection-5.c | 1 - gcc/testsuite/c-c++-common/fcf-protection-6.c | 2 - gcc/testsuite/c-c++-common/fcf-protection-7.c | 2 - gcc/testsuite/gcc.dg/march-generic.c | 2 +- gcc/testsuite/gcc.target/i386/align-limit.c | 2 +- gcc/testsuite/gcc.target/i386/cet-label-3.c | 16 +++++++ gcc/testsuite/gcc.target/i386/cet-notrack-icf-1.c | 2 +- gcc/testsuite/gcc.target/i386/cet-notrack-icf-3.c | 2 +- gcc/testsuite/gcc.target/i386/cet-property-2.c | 2 +- gcc/testsuite/gcc.target/i386/cet-property-3.c | 11 +++++ gcc/testsuite/gcc.target/i386/cet-sjlj-7.c | 48 +++++++++++++++++++ .../gcc.target/i386/indirect-thunk-attr-7.c | 2 +- .../gcc.target/i386/indirect-thunk-extern-7.c | 2 +- gcc/testsuite/gcc.target/i386/pr85403.c | 2 +- gcc/testsuite/gcc.target/i386/pr85417-1.c | 17 +++++++ gcc/testsuite/gcc.target/i386/ret-thunk-26.c | 2 +- 27 files changed, 133 insertions(+), 94 deletions(-) create mode 100644 gcc/testsuite/gcc.target/i386/cet-label-3.c create mode 100644 gcc/testsuite/gcc.target/i386/cet-property-3.c create mode 100644 gcc/testsuite/gcc.target/i386/cet-sjlj-7.c create mode 100644 gcc/testsuite/gcc.target/i386/pr85417-1.c diff --git a/gcc/config/i386/cet.c b/gcc/config/i386/cet.c index 4a1e013fdde..eb3be171471 100644 --- a/gcc/config/i386/cet.c +++ b/gcc/config/i386/cet.c @@ -34,11 +34,11 @@ file_end_indicate_exec_stack_and_cet (void) unsigned int feature_1 = 0; - if (TARGET_IBT) + if (flag_cf_protection & CF_BRANCH) /* GNU_PROPERTY_X86_FEATURE_1_IBT. */ feature_1 |= 0x1; - if (TARGET_SHSTK) + if (flag_cf_protection & CF_RETURN) /* GNU_PROPERTY_X86_FEATURE_1_SHSTK. */ feature_1 |= 0x2; diff --git a/gcc/config/i386/i386-c.c b/gcc/config/i386/i386-c.c index 2e0e9f66c9e..9961d418ee9 100644 --- a/gcc/config/i386/i386-c.c +++ b/gcc/config/i386/i386-c.c @@ -499,13 +499,15 @@ 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) + 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) + if ((isa_flag & OPTION_MASK_ISA_SHSTK) + || (flag_cf_protection & CF_RETURN)) { def_or_undef (parse_in, "__SHSTK__"); if (flag_cf_protection != CF_NONE) diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index d24c81b0dfe..2a628734068 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -2701,7 +2701,7 @@ public: /* opt_pass methods: */ virtual bool gate (function *) { - return ((flag_cf_protection & CF_BRANCH) && TARGET_IBT); + return ((flag_cf_protection & CF_BRANCH)); } virtual unsigned int execute (function *) @@ -4931,49 +4931,9 @@ ix86_option_override_internal (bool main_args_p, target_option_default_node = target_option_current_node = build_target_option_node (opts); - /* Do not support control flow instrumentation if CET is not enabled. */ - cf_protection_level cf_protection - = (cf_protection_level) (opts->x_flag_cf_protection & ~CF_SET); - if (cf_protection != CF_NONE) - { - switch (cf_protection) - { - case CF_BRANCH: - if (! TARGET_IBT_P (opts->x_ix86_isa_flags2)) - { - error ("%<-fcf-protection=branch%> requires Intel CET " - "support. Use -mcet or -mibt option to enable CET"); - flag_cf_protection = CF_NONE; - return false; - } - break; - case CF_RETURN: - if (! TARGET_SHSTK_P (opts->x_ix86_isa_flags)) - { - error ("%<-fcf-protection=return%> requires Intel CET " - "support. Use -mcet or -mshstk option to enable CET"); - flag_cf_protection = CF_NONE; - return false; - } - break; - case CF_FULL: - if ( ! TARGET_IBT_P (opts->x_ix86_isa_flags2) - || ! TARGET_SHSTK_P (opts->x_ix86_isa_flags)) - { - error ("%<-fcf-protection=full%> requires Intel CET " - "support. Use -mcet or both of -mibt and " - "-mshstk options to enable CET"); - flag_cf_protection = CF_NONE; - return false; - } - break; - default: - gcc_unreachable (); - } - - opts->x_flag_cf_protection = - (cf_protection_level) (cf_protection | CF_SET); - } + if (opts->x_flag_cf_protection != CF_NONE) + opts->x_flag_cf_protection = + (cf_protection_level) (opts->x_flag_cf_protection | CF_SET); if (ix86_tune_features [X86_TUNE_AVOID_128FMA_CHAINS]) maybe_set_param_value (PARAM_AVOID_FMA_MAX_BITS, 128, @@ -30408,7 +30368,7 @@ ix86_trampoline_init (rtx m_tramp, tree fndecl, rtx chain_value) rtx mem, fnaddr; int opcode; int offset = 0; - bool need_endbr = (flag_cf_protection & CF_BRANCH) && TARGET_IBT; + bool need_endbr = (flag_cf_protection & CF_BRANCH); fnaddr = XEXP (DECL_RTL (fndecl), 0); @@ -41766,7 +41726,7 @@ x86_output_mi_thunk (FILE *file, tree, HOST_WIDE_INT delta, emit_note (NOTE_INSN_PROLOGUE_END); /* CET is enabled, insert EB instruction. */ - if ((flag_cf_protection & CF_BRANCH) && TARGET_IBT) + if ((flag_cf_protection & CF_BRANCH)) emit_insn (gen_nop_endbr ()); /* If VCALL_OFFSET, we'll need THIS in a register. Might as well @@ -49766,7 +49726,7 @@ ix86_bnd_prefixed_insn_p (rtx insn) static bool ix86_notrack_prefixed_insn_p (rtx insn) { - if (!insn || !((flag_cf_protection & CF_BRANCH) && TARGET_IBT)) + if (!insn || !((flag_cf_protection & CF_BRANCH))) return false; if (CALL_P (insn)) diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index 95ca2cf9e3d..48daff1c9eb 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -20278,7 +20278,7 @@ (define_insn "rdssp<mode>" [(set (match_operand:SWI48x 0 "register_operand" "=r") (unspec_volatile:SWI48x [(const_int 0)] UNSPECV_NOP_RDSSP))] - "TARGET_SHSTK" + "TARGET_SHSTK || (flag_cf_protection & CF_RETURN)" "xor{l}\t%k0, %k0\n\trdssp<mskmodesuffix>\t%0" [(set_attr "length" "6") (set_attr "type" "other")]) @@ -20286,7 +20286,7 @@ (define_insn "incssp<mode>" [(unspec_volatile [(match_operand:SWI48x 0 "register_operand" "r")] UNSPECV_INCSSP)] - "TARGET_SHSTK" + "TARGET_SHSTK || (flag_cf_protection & CF_RETURN)" "incssp<mskmodesuffix>\t%0" [(set_attr "length" "4") (set_attr "type" "other")]) @@ -20341,7 +20341,7 @@ (define_insn "nop_endbr" [(unspec_volatile [(const_int 0)] UNSPECV_NOP_ENDBR)] - "TARGET_IBT" + "TARGET_IBT || (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 c063ae8b1ae..e55acf83d97 100644 --- a/gcc/config/i386/i386.opt +++ b/gcc/config/i386/i386.opt @@ -1008,17 +1008,16 @@ 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 -and code generation. +Support Control-flow Enforcement Technology (CET) built-in functions. mibt Target Report Mask(ISA_IBT) Var(ix86_isa_flags2) Save -Specifically enables an indirect branch tracking feature from Control-flow -Enforcement Technology (CET). +Specifically enable indirect branch tracking built-in functions from +Control-flow Enforcement Technology (CET). mshstk Target Report Mask(ISA_SHSTK) Var(ix86_isa_flags) Save -Specifically enables an shadow stack support feature from Control-flow +Specifically enable shadow stack built-in functions from Control-flow Enforcement Technology (CET). mcet-switch diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 8c0d275626f..1d8a0bd0ca0 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -11833,9 +11833,7 @@ which functions and calls should be skipped from instrumentation (@pxref{Function Attributes}). Currently the x86 GNU/Linux target provides an implementation based -on Intel Control-flow Enforcement Technology (CET). Instrumentation -for x86 is controlled by target-specific options @option{-mcet}, -@option{-mibt} and @option{-mshstk} (@pxref{x86 Options}). +on Intel Control-flow Enforcement Technology (CET). @item -fstack-protector @opindex fstack-protector @@ -27345,11 +27343,9 @@ 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 @option{-mibt} option enables indirect branch tracking support -and the @option{-mshstk} option enables shadow stack support from -Intel Control-flow Enforcement Technology (CET). The compiler also 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. +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 @@ -27445,19 +27441,15 @@ This option enables use of the @code{movbe} instruction to implement @item -mibt @opindex mibt -This option tells the compiler to use indirect branch tracking support -(for indirect calls and jumps) from x86 Control-flow Enforcement -Technology (CET). The option has effect only if the -@option{-fcf-protection=full} or @option{-fcf-protection=branch} option -is specified. The option @option{-mibt} is on by default when the -@code{-mcet} option is specified. +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 @opindex mshstk -This option tells the compiler to use shadow stack support (return -address tracking) from x86 Control-flow Enforcement Technology (CET). -The option has effect only if the @option{-fcf-protection=full} or -@option{-fcf-protection=return} option is specified. The option +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. diff --git a/gcc/testsuite/c-c++-common/attr-nocf-check-1.c b/gcc/testsuite/c-c++-common/attr-nocf-check-1.c index 15f69731b91..c5ac7cb9f86 100644 --- a/gcc/testsuite/c-c++-common/attr-nocf-check-1.c +++ b/gcc/testsuite/c-c++-common/attr-nocf-check-1.c @@ -1,4 +1,5 @@ /* { dg-do compile } */ +/* { dg-additional-options "-fcf-protection=none" } */ int func (int) __attribute__ ((nocf_check)); /* { dg-warning "'nocf_check' attribute ignored" } */ int (*fptr) (int) __attribute__ ((nocf_check)); /* { dg-warning "'nocf_check' attribute ignored" } */ diff --git a/gcc/testsuite/c-c++-common/attr-nocf-check-3.c b/gcc/testsuite/c-c++-common/attr-nocf-check-3.c index ad1ca7eec9b..02b56cb155e 100644 --- a/gcc/testsuite/c-c++-common/attr-nocf-check-3.c +++ b/gcc/testsuite/c-c++-common/attr-nocf-check-3.c @@ -1,4 +1,5 @@ /* { dg-do compile } */ +/* { dg-additional-options "-fcf-protection=none" } */ int foo (void) __attribute__ ((nocf_check)); /* { dg-warning "'nocf_check' attribute ignored" } */ void (*foo1) (void) __attribute__((nocf_check)); /* { dg-warning "'nocf_check' attribute ignored" } */ diff --git a/gcc/testsuite/c-c++-common/fcf-protection-1.c b/gcc/testsuite/c-c++-common/fcf-protection-1.c index 8e71f47dde0..f59a8fbdfdc 100644 --- a/gcc/testsuite/c-c++-common/fcf-protection-1.c +++ b/gcc/testsuite/c-c++-common/fcf-protection-1.c @@ -1,4 +1,3 @@ /* { dg-do compile } */ /* { dg-options "-fcf-protection=full" } */ -/* { dg-error "'-fcf-protection=full' requires Intel CET.*-mcet.*-mibt and -mshstk option" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */ /* { dg-error "'-fcf-protection=full' is not supported for this target" "" { target { ! "i?86-*-* x86_64-*-*" } } 0 } */ diff --git a/gcc/testsuite/c-c++-common/fcf-protection-2.c b/gcc/testsuite/c-c++-common/fcf-protection-2.c index d7d6db0e95d..61059725af6 100644 --- a/gcc/testsuite/c-c++-common/fcf-protection-2.c +++ b/gcc/testsuite/c-c++-common/fcf-protection-2.c @@ -1,4 +1,3 @@ /* { dg-do compile } */ /* { dg-options "-fcf-protection=branch" } */ -/* { dg-error "'-fcf-protection=branch' requires Intel CET.*-mcet or -mibt option" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */ /* { dg-error "'-fcf-protection=branch' is not supported for this target" "" { target { ! "i?86-*-* x86_64-*-*" } } 0 } */ diff --git a/gcc/testsuite/c-c++-common/fcf-protection-3.c b/gcc/testsuite/c-c++-common/fcf-protection-3.c index 5b903c5fa51..257e944c4a6 100644 --- a/gcc/testsuite/c-c++-common/fcf-protection-3.c +++ b/gcc/testsuite/c-c++-common/fcf-protection-3.c @@ -1,4 +1,3 @@ /* { dg-do compile } */ /* { dg-options "-fcf-protection=return" } */ -/* { dg-error "'-fcf-protection=return' requires Intel CET.*-mcet or -mshstk option" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */ /* { dg-error "'-fcf-protection=return' is not supported for this target" "" { target { ! "i?86-*-* x86_64-*-*" } } 0 } */ diff --git a/gcc/testsuite/c-c++-common/fcf-protection-5.c b/gcc/testsuite/c-c++-common/fcf-protection-5.c index d7a67801e2e..dc317f84b07 100644 --- a/gcc/testsuite/c-c++-common/fcf-protection-5.c +++ b/gcc/testsuite/c-c++-common/fcf-protection-5.c @@ -1,4 +1,3 @@ /* { dg-do compile } */ /* { dg-options "-fcf-protection" } */ -/* { dg-error "'-fcf-protection=full' requires Intel CET.*-mcet.*-mibt and -mshstk option" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */ /* { dg-error "'-fcf-protection=full' is not supported for this target" "" { target { ! "i?86-*-* x86_64-*-*" } } 0 } */ diff --git a/gcc/testsuite/c-c++-common/fcf-protection-6.c b/gcc/testsuite/c-c++-common/fcf-protection-6.c index 532e76e6915..61059725af6 100644 --- a/gcc/testsuite/c-c++-common/fcf-protection-6.c +++ b/gcc/testsuite/c-c++-common/fcf-protection-6.c @@ -1,5 +1,3 @@ /* { dg-do compile } */ /* { dg-options "-fcf-protection=branch" } */ -/* { dg-additional-options "-mshstk" { target { i?86-*-* x86_64-*-* } } } */ -/* { dg-error "'-fcf-protection=branch' requires Intel CET.*-mcet or -mibt option" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */ /* { dg-error "'-fcf-protection=branch' is not supported for this target" "" { target { ! "i?86-*-* x86_64-*-*" } } 0 } */ diff --git a/gcc/testsuite/c-c++-common/fcf-protection-7.c b/gcc/testsuite/c-c++-common/fcf-protection-7.c index 4c879692708..257e944c4a6 100644 --- a/gcc/testsuite/c-c++-common/fcf-protection-7.c +++ b/gcc/testsuite/c-c++-common/fcf-protection-7.c @@ -1,5 +1,3 @@ /* { dg-do compile } */ /* { dg-options "-fcf-protection=return" } */ -/* { dg-additional-options "-mibt" { target { i?86-*-* x86_64-*-* } } } */ -/* { dg-error "'-fcf-protection=return' requires Intel CET.*-mcet or -mshstk option" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */ /* { dg-error "'-fcf-protection=return' is not supported for this target" "" { target { ! "i?86-*-* x86_64-*-*" } } 0 } */ diff --git a/gcc/testsuite/gcc.dg/march-generic.c b/gcc/testsuite/gcc.dg/march-generic.c index fb5b83c7d74..f9c00e4a1c1 100644 --- a/gcc/testsuite/gcc.dg/march-generic.c +++ b/gcc/testsuite/gcc.dg/march-generic.c @@ -1,6 +1,6 @@ /* { dg-do compile { target i?86-*-* x86_64-*-* } } */ /* { dg-skip-if "" { *-*-* } { "-march=*" } { "" } } */ -/* { dg-options "-march=generic" } */ +/* { dg-options "-march=generic -fcf-protection=none" } */ /* { dg-error "'generic' CPU can be used only for '-mtune=' switch" "" { target *-*-* } 0 } */ /* { dg-bogus "march" "" { target *-*-* } 0 } */ int i; diff --git a/gcc/testsuite/gcc.target/i386/align-limit.c b/gcc/testsuite/gcc.target/i386/align-limit.c index d3d8dc5656e..849d741189c 100644 --- a/gcc/testsuite/gcc.target/i386/align-limit.c +++ b/gcc/testsuite/gcc.target/i386/align-limit.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -falign-functions=64 -flimit-function-alignment -march=amdfam10" } */ +/* { dg-options "-O2 -falign-functions=64 -flimit-function-alignment -march=amdfam10 -fcf-protection=none" } */ /* { dg-final { scan-assembler ".p2align 6,,1" } } */ /* { dg-final { scan-assembler-not ".p2align 6,,63" } } */ diff --git a/gcc/testsuite/gcc.target/i386/cet-label-3.c b/gcc/testsuite/gcc.target/i386/cet-label-3.c new file mode 100644 index 00000000000..5e0892e5b4d --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/cet-label-3.c @@ -0,0 +1,16 @@ +/* Verify that -fcf-protection works without -mcet. */ +/* { dg-do compile } */ +/* { dg-options "-O -fcf-protection" } */ +/* { dg-final { scan-assembler-times "endbr32" 3 { target ia32 } } } */ +/* { dg-final { scan-assembler-times "endbr64" 3 { target { ! ia32 } } } } */ + +int func (int arg) +{ + static void *array[] = { &&foo, &&bar }; + + goto *array[arg]; +foo: + return arg*111; +bar: + return arg*777; +} diff --git a/gcc/testsuite/gcc.target/i386/cet-notrack-icf-1.c b/gcc/testsuite/gcc.target/i386/cet-notrack-icf-1.c index 7987d53d305..0bddf54862a 100644 --- a/gcc/testsuite/gcc.target/i386/cet-notrack-icf-1.c +++ b/gcc/testsuite/gcc.target/i386/cet-notrack-icf-1.c @@ -1,6 +1,6 @@ /* Verify nocf_check functions are not ICF optimized. */ /* { dg-do compile } */ -/* { dg-options "-O2" } */ +/* { dg-options "-O2 -fcf-protection=none" } */ /* { dg-final { scan-assembler-not "endbr" } } */ /* { dg-final { scan-assembler-not "fn3:" } } */ /* { dg-final { scan-assembler "set\[ \t]+fn2,fn1" } } */ diff --git a/gcc/testsuite/gcc.target/i386/cet-notrack-icf-3.c b/gcc/testsuite/gcc.target/i386/cet-notrack-icf-3.c index 07c4a6b61ef..ed2d53ac5ef 100644 --- a/gcc/testsuite/gcc.target/i386/cet-notrack-icf-3.c +++ b/gcc/testsuite/gcc.target/i386/cet-notrack-icf-3.c @@ -1,6 +1,6 @@ /* Verify nocf_check function calls are not ICF optimized. */ /* { dg-do compile } */ -/* { dg-options "-O2" } */ +/* { dg-options "-O2 -fcf-protection=none" } */ /* { dg-final { scan-assembler-not "endbr" } } */ /* { dg-final { scan-assembler-not "fn2:" } } */ /* { dg-final { scan-assembler "set\[ \t]+fn2,fn1" } } */ diff --git a/gcc/testsuite/gcc.target/i386/cet-property-2.c b/gcc/testsuite/gcc.target/i386/cet-property-2.c index 5a87dab92f1..bca6f6cdeb7 100644 --- a/gcc/testsuite/gcc.target/i386/cet-property-2.c +++ b/gcc/testsuite/gcc.target/i386/cet-property-2.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-mcet" } */ +/* { dg-options "-mcet -fcf-protection=none" } */ /* { dg-final { scan-assembler-not ".note.gnu.property" } } */ extern void foo (void); diff --git a/gcc/testsuite/gcc.target/i386/cet-property-3.c b/gcc/testsuite/gcc.target/i386/cet-property-3.c new file mode 100644 index 00000000000..3e211c970aa --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/cet-property-3.c @@ -0,0 +1,11 @@ +/* { dg-do compile { target *-*-linux* } } */ +/* { dg-options "-fcf-protection" } */ +/* { dg-final { scan-assembler ".note.gnu.property" } } */ + +extern void foo (void); + +void +bar (void) +{ + foo (); +} diff --git a/gcc/testsuite/gcc.target/i386/cet-sjlj-7.c b/gcc/testsuite/gcc.target/i386/cet-sjlj-7.c new file mode 100644 index 00000000000..1b624327d0f --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/cet-sjlj-7.c @@ -0,0 +1,48 @@ +/* { dg-do compile } */ +/* { dg-options "-O -fcf-protection" } */ +/* { dg-final { scan-assembler-times "endbr32" 2 { target ia32 } } } */ +/* { dg-final { scan-assembler-times "endbr64" 2 { target { ! ia32 } } } } */ +/* { dg-final { scan-assembler-times "call _?setjmp" 1 } } */ +/* { dg-final { scan-assembler-times "call longjmp" 1 } } */ + +#include <stdio.h> +#include <setjmp.h> + +jmp_buf buf; +static int bar (int); + +__attribute__ ((noinline, noclone)) +static int +foo (int i) +{ + int j = i * 11; + + if (!setjmp (buf)) + { + j += 33; + printf ("After setjmp: j = %d\n", j); + bar (j); + } + + return j + i; +} + +__attribute__ ((noinline, noclone)) +static int +bar (int i) +{ + int j = i; + + j -= 111; + printf ("In longjmp: j = %d\n", j); + longjmp (buf, 1); + + return j; +} + +int +main () +{ + foo (10); + return 0; +} diff --git a/gcc/testsuite/gcc.target/i386/indirect-thunk-attr-7.c b/gcc/testsuite/gcc.target/i386/indirect-thunk-attr-7.c index d53fc887dcc..ffe7350fce4 100644 --- a/gcc/testsuite/gcc.target/i386/indirect-thunk-attr-7.c +++ b/gcc/testsuite/gcc.target/i386/indirect-thunk-attr-7.c @@ -37,7 +37,7 @@ bar (int i) } /* { dg-final { scan-assembler "mov(?:l|q)\[ \t\]*\.L\[0-9\]+\\(,%" { target *-*-linux* } } } */ -/* { dg-final { scan-assembler "jmp\[ \t\]*__x86_indirect_thunk_(r|e)ax" } } */ +/* { dg-final { scan-assembler "jmp\[ \t\]*__x86_indirect_thunk(_nt|)_(r|e)ax" } } */ /* { dg-final { scan-assembler-not {\t(lfence|pause)} } } */ /* { dg-final { scan-assembler-not "jmp\[ \t\]*\.LIND" } } */ /* { dg-final { scan-assembler-not "call\[ \t\]*\.LIND" } } */ diff --git a/gcc/testsuite/gcc.target/i386/indirect-thunk-extern-7.c b/gcc/testsuite/gcc.target/i386/indirect-thunk-extern-7.c index 2b9a33e93dc..b7339745116 100644 --- a/gcc/testsuite/gcc.target/i386/indirect-thunk-extern-7.c +++ b/gcc/testsuite/gcc.target/i386/indirect-thunk-extern-7.c @@ -36,7 +36,7 @@ bar (int i) } /* { dg-final { scan-assembler "mov(?:l|q)\[ \t\]*\.L\[0-9\]+\\(,%" { target *-*-linux* } } } */ -/* { dg-final { scan-assembler "jmp\[ \t\]*__x86_indirect_thunk_(r|e)ax" } } */ +/* { dg-final { scan-assembler "jmp\[ \t\]*__x86_indirect_thunk(_nt|)_(r|e)ax" } } */ /* { dg-final { scan-assembler-not {\t(lfence|pause)} } } */ /* { dg-final { scan-assembler-not "jmp\[ \t\]*\.LIND" } } */ /* { dg-final { scan-assembler-not "call\[ \t\]*\.LIND" } } */ diff --git a/gcc/testsuite/gcc.target/i386/pr85403.c b/gcc/testsuite/gcc.target/i386/pr85403.c index f4fb12dd4e2..0bbd7ca5610 100644 --- a/gcc/testsuite/gcc.target/i386/pr85403.c +++ b/gcc/testsuite/gcc.target/i386/pr85403.c @@ -7,4 +7,4 @@ int foo () { return -2; -} /* { dg-error "requires Intel CET support" } */ +} diff --git a/gcc/testsuite/gcc.target/i386/pr85417-1.c b/gcc/testsuite/gcc.target/i386/pr85417-1.c new file mode 100644 index 00000000000..17d52403744 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr85417-1.c @@ -0,0 +1,17 @@ +/* { dg-do compile } */ +/* { dg-require-ifunc "" } */ +/* { dg-options "-O3 -fcf-protection" } */ +/* { dg-final { scan-assembler "vpshufb" } } */ +/* { dg-final { scan-assembler "punpcklbw" } } */ + +__attribute__((target_clones("arch=core-avx2","arch=slm","default"))) +void +foo(char *in, char *out, int size) +{ + int i; + for(i = 0; i < size; i++) + { + out[2 * i] = in[i]; + out[2 * i + 1] = in[i]; + } +} diff --git a/gcc/testsuite/gcc.target/i386/ret-thunk-26.c b/gcc/testsuite/gcc.target/i386/ret-thunk-26.c index 9144e988735..dc722c2f5f9 100644 --- a/gcc/testsuite/gcc.target/i386/ret-thunk-26.c +++ b/gcc/testsuite/gcc.target/i386/ret-thunk-26.c @@ -1,6 +1,6 @@ /* PR target/r84530 */ /* { dg-do run } */ -/* { dg-options "-Os -mfunction-return=thunk" } */ +/* { dg-options "-Os -mfunction-return=thunk -fcf-protection=none" } */ struct S { int i; }; __attribute__((const, noinline, noclone))
On Thu, Apr 19, 2018 at 3:30 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Wed, Apr 18, 2018 at 01:35:33PM +0200, Richard Biener wrote: >> On Wed, Apr 18, 2018 at 1:24 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> > On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> >> On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> >>> On Tue, Apr 17, 2018 at 12:03 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> >>>> On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >> >>>>> On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: >> >>>>>> -fcf-protection -mcet can't be used with IFUNC features, like symbol >> >>>>>> multiversioning or target clone, since IBT/SHSTK are applied to the whole >> >>>>>> program and they may be disabled in some functions. But -fcf-protection >> >>>>>> is implemented with multi-byte NOPs on all 64-bit processors as well as >> >>>>>> 32-bit processors starting with Pentium Pro. If -fcf-protection requires >> >>>>>> -mcet, IFUNC features can't be used on Linux when -fcf-protection is >> >>>>>> enabled by default. >> >>>>>> >> >>>>>> This patch changes -fcf-protection to to enable the NOP portion of CET >> >>>>>> ISAs unless IBT and/or SHSTK are disabled explicitly. The rest of CET >> >>>>>> ISAs, including intrinsics, still requires -mcet, -mibt or -mshstk. >> >>>>>> >> >>>>>> OK for trunk? >> >>>>> >> >>>>> As said in the PR, NOP sequences have non-zero cost in the executable >> >>>>> (they enlarge the executable), so I don't think this feature should be >> >>>>> enabled by default. >> >>>>> >> >>>>> There is always a configure option if someone wants their compiler to >> >>>>> always emit relevant multi-byte nops. >> >>>> >> >>>> What we need is an option to enable -fcf-function with multi-byte NOPs >> >>>> without -mcet which enables the full CET ISAs. A configure option >> >>>> without the corresponding the command-line option makes test and >> >>>> debug difficult. I can add >> >>>> >> >>>> --enable-cf-function-nop or --with-cf-function-nop >> >>>> >> >>>> with >> >>>> >> >>>> -fct-function-nop >> >>>> >> >>> >> >>> How about adding -mno-cet, which enables the NOP portion of CET >> >> >> >> I meant -mnop-cet, not -mno-cet. >> >> >> > >> > Here is a patch to add -mnop and use it with -fcf-protection. >> >> +mnop >> +Target Report Var(flag_nop) Init(0) >> +Support multi-byte NOP code generation. >> >> the option name is incredibly bad and the documentation doesn't make it >> better either. The invoke.texi docs refer to duplicate {-mcet}. >> >> Isn't there a -fcf-protection sub-set that can be used to automatically >> enable this? Or simply do this mode by default when >> -fcf-protection is used but neither -mcet nor -mibt is enabled? >> > > Since multi-byte NOPs are used to implement -fcf-protection on x86, we > propose a new design for -fcf-protection: > > 1. -fcf-protection option will report the unsupported error on non-x86 > platform. On x86 platform it's supported and inserts endbr-nop > instructions and properties, depending on its value (full/branch/return) > 2. -mcet/-mibt/-mshstk options control intrinsics only. > 3. These options are independent and do not influence each other so no > need for cross checking between them. > > OK for trunk? This patch touches only CET related code, so Igor's OK should be enough. Uros.
On Thu, Apr 19, 2018 at 3:30 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Wed, Apr 18, 2018 at 01:35:33PM +0200, Richard Biener wrote: >> On Wed, Apr 18, 2018 at 1:24 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> > On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> >> On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> >>> On Tue, Apr 17, 2018 at 12:03 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> >>>> On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >> >>>>> On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: >> >>>>>> -fcf-protection -mcet can't be used with IFUNC features, like symbol >> >>>>>> multiversioning or target clone, since IBT/SHSTK are applied to the whole >> >>>>>> program and they may be disabled in some functions. But -fcf-protection >> >>>>>> is implemented with multi-byte NOPs on all 64-bit processors as well as >> >>>>>> 32-bit processors starting with Pentium Pro. If -fcf-protection requires >> >>>>>> -mcet, IFUNC features can't be used on Linux when -fcf-protection is >> >>>>>> enabled by default. >> >>>>>> >> >>>>>> This patch changes -fcf-protection to to enable the NOP portion of CET >> >>>>>> ISAs unless IBT and/or SHSTK are disabled explicitly. The rest of CET >> >>>>>> ISAs, including intrinsics, still requires -mcet, -mibt or -mshstk. >> >>>>>> >> >>>>>> OK for trunk? >> >>>>> >> >>>>> As said in the PR, NOP sequences have non-zero cost in the executable >> >>>>> (they enlarge the executable), so I don't think this feature should be >> >>>>> enabled by default. >> >>>>> >> >>>>> There is always a configure option if someone wants their compiler to >> >>>>> always emit relevant multi-byte nops. >> >>>> >> >>>> What we need is an option to enable -fcf-function with multi-byte NOPs >> >>>> without -mcet which enables the full CET ISAs. A configure option >> >>>> without the corresponding the command-line option makes test and >> >>>> debug difficult. I can add >> >>>> >> >>>> --enable-cf-function-nop or --with-cf-function-nop >> >>>> >> >>>> with >> >>>> >> >>>> -fct-function-nop >> >>>> >> >>> >> >>> How about adding -mno-cet, which enables the NOP portion of CET >> >> >> >> I meant -mnop-cet, not -mno-cet. >> >> >> > >> > Here is a patch to add -mnop and use it with -fcf-protection. >> >> +mnop >> +Target Report Var(flag_nop) Init(0) >> +Support multi-byte NOP code generation. >> >> the option name is incredibly bad and the documentation doesn't make it >> better either. The invoke.texi docs refer to duplicate {-mcet}. >> >> Isn't there a -fcf-protection sub-set that can be used to automatically >> enable this? Or simply do this mode by default when >> -fcf-protection is used but neither -mcet nor -mibt is enabled? >> > > Since multi-byte NOPs are used to implement -fcf-protection on x86, we > propose a new design for -fcf-protection: > > 1. -fcf-protection option will report the unsupported error on non-x86 > platform. On x86 platform it's supported and inserts endbr-nop > instructions and properties, depending on its value (full/branch/return) > 2. -mcet/-mibt/-mshstk options control intrinsics only. > 3. These options are independent and do not influence each other so no > need for cross checking between them. > > OK for trunk? I think it makes more sense this way, thanks for doing the change (this isn't an approval). Richard. > > H.J. > ---- > ---- > -fcf-protection -mcet can't be used with IFUNC features, like symbol > multiversioning or target clone, since IBT/SHSTK are applied to the whole > program and they may be disabled in some functions. But -fcf-protection > is implemented with multi-byte NOPs on all 64-bit processors as well as > 32-bit processors starting with Pentium Pro. If -fcf-protection requires > -mcet, IFUNC features can't be used on Linux when -fcf-protection is > enabled by default. > > This patch changes -fcf-protection to implement indirect branch and > return address tracking with multi-byte NOPs. -mibt and -mshstk are > changed to only CET built-in functions. CET tests are updated to > allow -fcf-protection without -mibt, -mshstk and -mcet on x86. > -fcf-protection=none are also added to tests which fail with > -fcf-protection so that -fcf-protection can be added to RUNTESTFLAGS > to verify -fcf-protection implementation. > > gcc/ > > PR target/85417 > * config/i386/cet.c (file_end_indicate_exec_stack_and_cet): > Check flag_cf_protection instead of TARGET_IBT and TARGET_SHSTK. > * config/i386/i386-c.c (ix86_target_macros_internal): Also > define __IBT__ and __SHSTK__ for -fcf-protection. > * config/i386/i386.c (pass_insert_endbranch::gate): Don't check > TARGET_IBT. > (ix86_trampoline_init): Likewise. > (x86_output_mi_thunk): Likewise. > (ix86_notrack_prefixed_insn_p): Likewise. > (ix86_option_override_internal): Don't disallow -fcf-protection. > * config/i386/i386.md (rdssp<mode>): Also enable for > -fcf-protection. > (incssp<mode>): Likewise. > (nop_endbr): Likewise. > * config/i386/i386.opt (mcet): Change help message to built-in > functions only. > (mibt): Likewise. > (mshstk): Likewise. > * doc/invoke.texi: Remove -mcet, -mibt and -mshstk condition > on -fcf-protection. Change -mcet, -mibt and -mshstk to only > enable CET built-in functions. > > gcc/testsuite/ > > PR target/85417 > * c-c++-common/attr-nocf-check-1.c: Compile with > -fcf-protection=none. > * c-c++-common/attr-nocf-check-3.c: Likewise. > * gcc.dg/march-generic.c: Likewise. > * gcc.target/i386/align-limit.c: Likewise. > * gcc.target/i386/cet-notrack-icf-1.c: Likewise. > * gcc.target/i386/cet-notrack-icf-3.c: Likewise. > * gcc.target/i386/cet-property-2.c: Likewise. > * gcc.target/i386/ret-thunk-26.c: Likewise. > * c-c++-common/fcf-protection-1.c: Remove dg-error for x86 > targets. > * c-c++-common/fcf-protection-2.c: Likewise. > * c-c++-common/fcf-protection-3.c: Likewise. > * c-c++-common/fcf-protection-5.c: Likewise. > * c-c++-common/fcf-protection-6.c: Likewise. > * c-c++-common/fcf-protection-7.c: Likewise. > * gcc.target/i386/cet-label-3.c: New test. > * gcc.target/i386/cet-property-3.c: Likewise. > * gcc.target/i386/cet-sjlj-7.c: Likewise. > * gcc.target/i386/pr85417-1.c: Likewise. > * gcc.target/i386/indirect-thunk-attr-7.c: Also expect > __x86_indirect_thunk_nt_(r|e)ax > * gcc.target/i386/indirect-thunk-extern-7.c: Likewise. > * gcc.target/i386/pr85403.c: Remove dg-error, > --- > gcc/config/i386/cet.c | 4 +- > gcc/config/i386/i386-c.c | 6 ++- > gcc/config/i386/i386.c | 54 +++------------------- > gcc/config/i386/i386.md | 6 +-- > gcc/config/i386/i386.opt | 9 ++-- > gcc/doc/invoke.texi | 28 ++++------- > gcc/testsuite/c-c++-common/attr-nocf-check-1.c | 1 + > gcc/testsuite/c-c++-common/attr-nocf-check-3.c | 1 + > gcc/testsuite/c-c++-common/fcf-protection-1.c | 1 - > gcc/testsuite/c-c++-common/fcf-protection-2.c | 1 - > gcc/testsuite/c-c++-common/fcf-protection-3.c | 1 - > gcc/testsuite/c-c++-common/fcf-protection-5.c | 1 - > gcc/testsuite/c-c++-common/fcf-protection-6.c | 2 - > gcc/testsuite/c-c++-common/fcf-protection-7.c | 2 - > gcc/testsuite/gcc.dg/march-generic.c | 2 +- > gcc/testsuite/gcc.target/i386/align-limit.c | 2 +- > gcc/testsuite/gcc.target/i386/cet-label-3.c | 16 +++++++ > gcc/testsuite/gcc.target/i386/cet-notrack-icf-1.c | 2 +- > gcc/testsuite/gcc.target/i386/cet-notrack-icf-3.c | 2 +- > gcc/testsuite/gcc.target/i386/cet-property-2.c | 2 +- > gcc/testsuite/gcc.target/i386/cet-property-3.c | 11 +++++ > gcc/testsuite/gcc.target/i386/cet-sjlj-7.c | 48 +++++++++++++++++++ > .../gcc.target/i386/indirect-thunk-attr-7.c | 2 +- > .../gcc.target/i386/indirect-thunk-extern-7.c | 2 +- > gcc/testsuite/gcc.target/i386/pr85403.c | 2 +- > gcc/testsuite/gcc.target/i386/pr85417-1.c | 17 +++++++ > gcc/testsuite/gcc.target/i386/ret-thunk-26.c | 2 +- > 27 files changed, 133 insertions(+), 94 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/i386/cet-label-3.c > create mode 100644 gcc/testsuite/gcc.target/i386/cet-property-3.c > create mode 100644 gcc/testsuite/gcc.target/i386/cet-sjlj-7.c > create mode 100644 gcc/testsuite/gcc.target/i386/pr85417-1.c > > diff --git a/gcc/config/i386/cet.c b/gcc/config/i386/cet.c > index 4a1e013fdde..eb3be171471 100644 > --- a/gcc/config/i386/cet.c > +++ b/gcc/config/i386/cet.c > @@ -34,11 +34,11 @@ file_end_indicate_exec_stack_and_cet (void) > > unsigned int feature_1 = 0; > > - if (TARGET_IBT) > + if (flag_cf_protection & CF_BRANCH) > /* GNU_PROPERTY_X86_FEATURE_1_IBT. */ > feature_1 |= 0x1; > > - if (TARGET_SHSTK) > + if (flag_cf_protection & CF_RETURN) > /* GNU_PROPERTY_X86_FEATURE_1_SHSTK. */ > feature_1 |= 0x2; > > diff --git a/gcc/config/i386/i386-c.c b/gcc/config/i386/i386-c.c > index 2e0e9f66c9e..9961d418ee9 100644 > --- a/gcc/config/i386/i386-c.c > +++ b/gcc/config/i386/i386-c.c > @@ -499,13 +499,15 @@ 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) > + 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) > + if ((isa_flag & OPTION_MASK_ISA_SHSTK) > + || (flag_cf_protection & CF_RETURN)) > { > def_or_undef (parse_in, "__SHSTK__"); > if (flag_cf_protection != CF_NONE) > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index d24c81b0dfe..2a628734068 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -2701,7 +2701,7 @@ public: > /* opt_pass methods: */ > virtual bool gate (function *) > { > - return ((flag_cf_protection & CF_BRANCH) && TARGET_IBT); > + return ((flag_cf_protection & CF_BRANCH)); > } > > virtual unsigned int execute (function *) > @@ -4931,49 +4931,9 @@ ix86_option_override_internal (bool main_args_p, > target_option_default_node = target_option_current_node > = build_target_option_node (opts); > > - /* Do not support control flow instrumentation if CET is not enabled. */ > - cf_protection_level cf_protection > - = (cf_protection_level) (opts->x_flag_cf_protection & ~CF_SET); > - if (cf_protection != CF_NONE) > - { > - switch (cf_protection) > - { > - case CF_BRANCH: > - if (! TARGET_IBT_P (opts->x_ix86_isa_flags2)) > - { > - error ("%<-fcf-protection=branch%> requires Intel CET " > - "support. Use -mcet or -mibt option to enable CET"); > - flag_cf_protection = CF_NONE; > - return false; > - } > - break; > - case CF_RETURN: > - if (! TARGET_SHSTK_P (opts->x_ix86_isa_flags)) > - { > - error ("%<-fcf-protection=return%> requires Intel CET " > - "support. Use -mcet or -mshstk option to enable CET"); > - flag_cf_protection = CF_NONE; > - return false; > - } > - break; > - case CF_FULL: > - if ( ! TARGET_IBT_P (opts->x_ix86_isa_flags2) > - || ! TARGET_SHSTK_P (opts->x_ix86_isa_flags)) > - { > - error ("%<-fcf-protection=full%> requires Intel CET " > - "support. Use -mcet or both of -mibt and " > - "-mshstk options to enable CET"); > - flag_cf_protection = CF_NONE; > - return false; > - } > - break; > - default: > - gcc_unreachable (); > - } > - > - opts->x_flag_cf_protection = > - (cf_protection_level) (cf_protection | CF_SET); > - } > + if (opts->x_flag_cf_protection != CF_NONE) > + opts->x_flag_cf_protection = > + (cf_protection_level) (opts->x_flag_cf_protection | CF_SET); > > if (ix86_tune_features [X86_TUNE_AVOID_128FMA_CHAINS]) > maybe_set_param_value (PARAM_AVOID_FMA_MAX_BITS, 128, > @@ -30408,7 +30368,7 @@ ix86_trampoline_init (rtx m_tramp, tree fndecl, rtx chain_value) > rtx mem, fnaddr; > int opcode; > int offset = 0; > - bool need_endbr = (flag_cf_protection & CF_BRANCH) && TARGET_IBT; > + bool need_endbr = (flag_cf_protection & CF_BRANCH); > > fnaddr = XEXP (DECL_RTL (fndecl), 0); > > @@ -41766,7 +41726,7 @@ x86_output_mi_thunk (FILE *file, tree, HOST_WIDE_INT delta, > emit_note (NOTE_INSN_PROLOGUE_END); > > /* CET is enabled, insert EB instruction. */ > - if ((flag_cf_protection & CF_BRANCH) && TARGET_IBT) > + if ((flag_cf_protection & CF_BRANCH)) > emit_insn (gen_nop_endbr ()); > > /* If VCALL_OFFSET, we'll need THIS in a register. Might as well > @@ -49766,7 +49726,7 @@ ix86_bnd_prefixed_insn_p (rtx insn) > static bool > ix86_notrack_prefixed_insn_p (rtx insn) > { > - if (!insn || !((flag_cf_protection & CF_BRANCH) && TARGET_IBT)) > + if (!insn || !((flag_cf_protection & CF_BRANCH))) > return false; > > if (CALL_P (insn)) > diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md > index 95ca2cf9e3d..48daff1c9eb 100644 > --- a/gcc/config/i386/i386.md > +++ b/gcc/config/i386/i386.md > @@ -20278,7 +20278,7 @@ > (define_insn "rdssp<mode>" > [(set (match_operand:SWI48x 0 "register_operand" "=r") > (unspec_volatile:SWI48x [(const_int 0)] UNSPECV_NOP_RDSSP))] > - "TARGET_SHSTK" > + "TARGET_SHSTK || (flag_cf_protection & CF_RETURN)" > "xor{l}\t%k0, %k0\n\trdssp<mskmodesuffix>\t%0" > [(set_attr "length" "6") > (set_attr "type" "other")]) > @@ -20286,7 +20286,7 @@ > (define_insn "incssp<mode>" > [(unspec_volatile [(match_operand:SWI48x 0 "register_operand" "r")] > UNSPECV_INCSSP)] > - "TARGET_SHSTK" > + "TARGET_SHSTK || (flag_cf_protection & CF_RETURN)" > "incssp<mskmodesuffix>\t%0" > [(set_attr "length" "4") > (set_attr "type" "other")]) > @@ -20341,7 +20341,7 @@ > > (define_insn "nop_endbr" > [(unspec_volatile [(const_int 0)] UNSPECV_NOP_ENDBR)] > - "TARGET_IBT" > + "TARGET_IBT || (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 c063ae8b1ae..e55acf83d97 100644 > --- a/gcc/config/i386/i386.opt > +++ b/gcc/config/i386/i386.opt > @@ -1008,17 +1008,16 @@ 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 > -and code generation. > +Support Control-flow Enforcement Technology (CET) built-in functions. > > mibt > Target Report Mask(ISA_IBT) Var(ix86_isa_flags2) Save > -Specifically enables an indirect branch tracking feature from Control-flow > -Enforcement Technology (CET). > +Specifically enable indirect branch tracking built-in functions from > +Control-flow Enforcement Technology (CET). > > mshstk > Target Report Mask(ISA_SHSTK) Var(ix86_isa_flags) Save > -Specifically enables an shadow stack support feature from Control-flow > +Specifically enable shadow stack built-in functions from Control-flow > Enforcement Technology (CET). > > mcet-switch > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index 8c0d275626f..1d8a0bd0ca0 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -11833,9 +11833,7 @@ which functions and calls should be skipped from instrumentation > (@pxref{Function Attributes}). > > Currently the x86 GNU/Linux target provides an implementation based > -on Intel Control-flow Enforcement Technology (CET). Instrumentation > -for x86 is controlled by target-specific options @option{-mcet}, > -@option{-mibt} and @option{-mshstk} (@pxref{x86 Options}). > +on Intel Control-flow Enforcement Technology (CET). > > @item -fstack-protector > @opindex fstack-protector > @@ -27345,11 +27343,9 @@ 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 @option{-mibt} option enables indirect branch tracking support > -and the @option{-mshstk} option enables shadow stack support from > -Intel Control-flow Enforcement Technology (CET). The compiler also 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. > +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 > @@ -27445,19 +27441,15 @@ This option enables use of the @code{movbe} instruction to implement > > @item -mibt > @opindex mibt > -This option tells the compiler to use indirect branch tracking support > -(for indirect calls and jumps) from x86 Control-flow Enforcement > -Technology (CET). The option has effect only if the > -@option{-fcf-protection=full} or @option{-fcf-protection=branch} option > -is specified. The option @option{-mibt} is on by default when the > -@code{-mcet} option is specified. > +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 > @opindex mshstk > -This option tells the compiler to use shadow stack support (return > -address tracking) from x86 Control-flow Enforcement Technology (CET). > -The option has effect only if the @option{-fcf-protection=full} or > -@option{-fcf-protection=return} option is specified. The option > +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. > > diff --git a/gcc/testsuite/c-c++-common/attr-nocf-check-1.c b/gcc/testsuite/c-c++-common/attr-nocf-check-1.c > index 15f69731b91..c5ac7cb9f86 100644 > --- a/gcc/testsuite/c-c++-common/attr-nocf-check-1.c > +++ b/gcc/testsuite/c-c++-common/attr-nocf-check-1.c > @@ -1,4 +1,5 @@ > /* { dg-do compile } */ > +/* { dg-additional-options "-fcf-protection=none" } */ > > int func (int) __attribute__ ((nocf_check)); /* { dg-warning "'nocf_check' attribute ignored" } */ > int (*fptr) (int) __attribute__ ((nocf_check)); /* { dg-warning "'nocf_check' attribute ignored" } */ > diff --git a/gcc/testsuite/c-c++-common/attr-nocf-check-3.c b/gcc/testsuite/c-c++-common/attr-nocf-check-3.c > index ad1ca7eec9b..02b56cb155e 100644 > --- a/gcc/testsuite/c-c++-common/attr-nocf-check-3.c > +++ b/gcc/testsuite/c-c++-common/attr-nocf-check-3.c > @@ -1,4 +1,5 @@ > /* { dg-do compile } */ > +/* { dg-additional-options "-fcf-protection=none" } */ > > int foo (void) __attribute__ ((nocf_check)); /* { dg-warning "'nocf_check' attribute ignored" } */ > void (*foo1) (void) __attribute__((nocf_check)); /* { dg-warning "'nocf_check' attribute ignored" } */ > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-1.c b/gcc/testsuite/c-c++-common/fcf-protection-1.c > index 8e71f47dde0..f59a8fbdfdc 100644 > --- a/gcc/testsuite/c-c++-common/fcf-protection-1.c > +++ b/gcc/testsuite/c-c++-common/fcf-protection-1.c > @@ -1,4 +1,3 @@ > /* { dg-do compile } */ > /* { dg-options "-fcf-protection=full" } */ > -/* { dg-error "'-fcf-protection=full' requires Intel CET.*-mcet.*-mibt and -mshstk option" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */ > /* { dg-error "'-fcf-protection=full' is not supported for this target" "" { target { ! "i?86-*-* x86_64-*-*" } } 0 } */ > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-2.c b/gcc/testsuite/c-c++-common/fcf-protection-2.c > index d7d6db0e95d..61059725af6 100644 > --- a/gcc/testsuite/c-c++-common/fcf-protection-2.c > +++ b/gcc/testsuite/c-c++-common/fcf-protection-2.c > @@ -1,4 +1,3 @@ > /* { dg-do compile } */ > /* { dg-options "-fcf-protection=branch" } */ > -/* { dg-error "'-fcf-protection=branch' requires Intel CET.*-mcet or -mibt option" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */ > /* { dg-error "'-fcf-protection=branch' is not supported for this target" "" { target { ! "i?86-*-* x86_64-*-*" } } 0 } */ > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-3.c b/gcc/testsuite/c-c++-common/fcf-protection-3.c > index 5b903c5fa51..257e944c4a6 100644 > --- a/gcc/testsuite/c-c++-common/fcf-protection-3.c > +++ b/gcc/testsuite/c-c++-common/fcf-protection-3.c > @@ -1,4 +1,3 @@ > /* { dg-do compile } */ > /* { dg-options "-fcf-protection=return" } */ > -/* { dg-error "'-fcf-protection=return' requires Intel CET.*-mcet or -mshstk option" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */ > /* { dg-error "'-fcf-protection=return' is not supported for this target" "" { target { ! "i?86-*-* x86_64-*-*" } } 0 } */ > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-5.c b/gcc/testsuite/c-c++-common/fcf-protection-5.c > index d7a67801e2e..dc317f84b07 100644 > --- a/gcc/testsuite/c-c++-common/fcf-protection-5.c > +++ b/gcc/testsuite/c-c++-common/fcf-protection-5.c > @@ -1,4 +1,3 @@ > /* { dg-do compile } */ > /* { dg-options "-fcf-protection" } */ > -/* { dg-error "'-fcf-protection=full' requires Intel CET.*-mcet.*-mibt and -mshstk option" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */ > /* { dg-error "'-fcf-protection=full' is not supported for this target" "" { target { ! "i?86-*-* x86_64-*-*" } } 0 } */ > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-6.c b/gcc/testsuite/c-c++-common/fcf-protection-6.c > index 532e76e6915..61059725af6 100644 > --- a/gcc/testsuite/c-c++-common/fcf-protection-6.c > +++ b/gcc/testsuite/c-c++-common/fcf-protection-6.c > @@ -1,5 +1,3 @@ > /* { dg-do compile } */ > /* { dg-options "-fcf-protection=branch" } */ > -/* { dg-additional-options "-mshstk" { target { i?86-*-* x86_64-*-* } } } */ > -/* { dg-error "'-fcf-protection=branch' requires Intel CET.*-mcet or -mibt option" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */ > /* { dg-error "'-fcf-protection=branch' is not supported for this target" "" { target { ! "i?86-*-* x86_64-*-*" } } 0 } */ > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-7.c b/gcc/testsuite/c-c++-common/fcf-protection-7.c > index 4c879692708..257e944c4a6 100644 > --- a/gcc/testsuite/c-c++-common/fcf-protection-7.c > +++ b/gcc/testsuite/c-c++-common/fcf-protection-7.c > @@ -1,5 +1,3 @@ > /* { dg-do compile } */ > /* { dg-options "-fcf-protection=return" } */ > -/* { dg-additional-options "-mibt" { target { i?86-*-* x86_64-*-* } } } */ > -/* { dg-error "'-fcf-protection=return' requires Intel CET.*-mcet or -mshstk option" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */ > /* { dg-error "'-fcf-protection=return' is not supported for this target" "" { target { ! "i?86-*-* x86_64-*-*" } } 0 } */ > diff --git a/gcc/testsuite/gcc.dg/march-generic.c b/gcc/testsuite/gcc.dg/march-generic.c > index fb5b83c7d74..f9c00e4a1c1 100644 > --- a/gcc/testsuite/gcc.dg/march-generic.c > +++ b/gcc/testsuite/gcc.dg/march-generic.c > @@ -1,6 +1,6 @@ > /* { dg-do compile { target i?86-*-* x86_64-*-* } } */ > /* { dg-skip-if "" { *-*-* } { "-march=*" } { "" } } */ > -/* { dg-options "-march=generic" } */ > +/* { dg-options "-march=generic -fcf-protection=none" } */ > /* { dg-error "'generic' CPU can be used only for '-mtune=' switch" "" { target *-*-* } 0 } */ > /* { dg-bogus "march" "" { target *-*-* } 0 } */ > int i; > diff --git a/gcc/testsuite/gcc.target/i386/align-limit.c b/gcc/testsuite/gcc.target/i386/align-limit.c > index d3d8dc5656e..849d741189c 100644 > --- a/gcc/testsuite/gcc.target/i386/align-limit.c > +++ b/gcc/testsuite/gcc.target/i386/align-limit.c > @@ -1,5 +1,5 @@ > /* { dg-do compile } */ > -/* { dg-options "-O2 -falign-functions=64 -flimit-function-alignment -march=amdfam10" } */ > +/* { dg-options "-O2 -falign-functions=64 -flimit-function-alignment -march=amdfam10 -fcf-protection=none" } */ > /* { dg-final { scan-assembler ".p2align 6,,1" } } */ > /* { dg-final { scan-assembler-not ".p2align 6,,63" } } */ > > diff --git a/gcc/testsuite/gcc.target/i386/cet-label-3.c b/gcc/testsuite/gcc.target/i386/cet-label-3.c > new file mode 100644 > index 00000000000..5e0892e5b4d > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/cet-label-3.c > @@ -0,0 +1,16 @@ > +/* Verify that -fcf-protection works without -mcet. */ > +/* { dg-do compile } */ > +/* { dg-options "-O -fcf-protection" } */ > +/* { dg-final { scan-assembler-times "endbr32" 3 { target ia32 } } } */ > +/* { dg-final { scan-assembler-times "endbr64" 3 { target { ! ia32 } } } } */ > + > +int func (int arg) > +{ > + static void *array[] = { &&foo, &&bar }; > + > + goto *array[arg]; > +foo: > + return arg*111; > +bar: > + return arg*777; > +} > diff --git a/gcc/testsuite/gcc.target/i386/cet-notrack-icf-1.c b/gcc/testsuite/gcc.target/i386/cet-notrack-icf-1.c > index 7987d53d305..0bddf54862a 100644 > --- a/gcc/testsuite/gcc.target/i386/cet-notrack-icf-1.c > +++ b/gcc/testsuite/gcc.target/i386/cet-notrack-icf-1.c > @@ -1,6 +1,6 @@ > /* Verify nocf_check functions are not ICF optimized. */ > /* { dg-do compile } */ > -/* { dg-options "-O2" } */ > +/* { dg-options "-O2 -fcf-protection=none" } */ > /* { dg-final { scan-assembler-not "endbr" } } */ > /* { dg-final { scan-assembler-not "fn3:" } } */ > /* { dg-final { scan-assembler "set\[ \t]+fn2,fn1" } } */ > diff --git a/gcc/testsuite/gcc.target/i386/cet-notrack-icf-3.c b/gcc/testsuite/gcc.target/i386/cet-notrack-icf-3.c > index 07c4a6b61ef..ed2d53ac5ef 100644 > --- a/gcc/testsuite/gcc.target/i386/cet-notrack-icf-3.c > +++ b/gcc/testsuite/gcc.target/i386/cet-notrack-icf-3.c > @@ -1,6 +1,6 @@ > /* Verify nocf_check function calls are not ICF optimized. */ > /* { dg-do compile } */ > -/* { dg-options "-O2" } */ > +/* { dg-options "-O2 -fcf-protection=none" } */ > /* { dg-final { scan-assembler-not "endbr" } } */ > /* { dg-final { scan-assembler-not "fn2:" } } */ > /* { dg-final { scan-assembler "set\[ \t]+fn2,fn1" } } */ > diff --git a/gcc/testsuite/gcc.target/i386/cet-property-2.c b/gcc/testsuite/gcc.target/i386/cet-property-2.c > index 5a87dab92f1..bca6f6cdeb7 100644 > --- a/gcc/testsuite/gcc.target/i386/cet-property-2.c > +++ b/gcc/testsuite/gcc.target/i386/cet-property-2.c > @@ -1,5 +1,5 @@ > /* { dg-do compile } */ > -/* { dg-options "-mcet" } */ > +/* { dg-options "-mcet -fcf-protection=none" } */ > /* { dg-final { scan-assembler-not ".note.gnu.property" } } */ > > extern void foo (void); > diff --git a/gcc/testsuite/gcc.target/i386/cet-property-3.c b/gcc/testsuite/gcc.target/i386/cet-property-3.c > new file mode 100644 > index 00000000000..3e211c970aa > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/cet-property-3.c > @@ -0,0 +1,11 @@ > +/* { dg-do compile { target *-*-linux* } } */ > +/* { dg-options "-fcf-protection" } */ > +/* { dg-final { scan-assembler ".note.gnu.property" } } */ > + > +extern void foo (void); > + > +void > +bar (void) > +{ > + foo (); > +} > diff --git a/gcc/testsuite/gcc.target/i386/cet-sjlj-7.c b/gcc/testsuite/gcc.target/i386/cet-sjlj-7.c > new file mode 100644 > index 00000000000..1b624327d0f > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/cet-sjlj-7.c > @@ -0,0 +1,48 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O -fcf-protection" } */ > +/* { dg-final { scan-assembler-times "endbr32" 2 { target ia32 } } } */ > +/* { dg-final { scan-assembler-times "endbr64" 2 { target { ! ia32 } } } } */ > +/* { dg-final { scan-assembler-times "call _?setjmp" 1 } } */ > +/* { dg-final { scan-assembler-times "call longjmp" 1 } } */ > + > +#include <stdio.h> > +#include <setjmp.h> > + > +jmp_buf buf; > +static int bar (int); > + > +__attribute__ ((noinline, noclone)) > +static int > +foo (int i) > +{ > + int j = i * 11; > + > + if (!setjmp (buf)) > + { > + j += 33; > + printf ("After setjmp: j = %d\n", j); > + bar (j); > + } > + > + return j + i; > +} > + > +__attribute__ ((noinline, noclone)) > +static int > +bar (int i) > +{ > + int j = i; > + > + j -= 111; > + printf ("In longjmp: j = %d\n", j); > + longjmp (buf, 1); > + > + return j; > +} > + > +int > +main () > +{ > + foo (10); > + return 0; > +} > diff --git a/gcc/testsuite/gcc.target/i386/indirect-thunk-attr-7.c b/gcc/testsuite/gcc.target/i386/indirect-thunk-attr-7.c > index d53fc887dcc..ffe7350fce4 100644 > --- a/gcc/testsuite/gcc.target/i386/indirect-thunk-attr-7.c > +++ b/gcc/testsuite/gcc.target/i386/indirect-thunk-attr-7.c > @@ -37,7 +37,7 @@ bar (int i) > } > > /* { dg-final { scan-assembler "mov(?:l|q)\[ \t\]*\.L\[0-9\]+\\(,%" { target *-*-linux* } } } */ > -/* { dg-final { scan-assembler "jmp\[ \t\]*__x86_indirect_thunk_(r|e)ax" } } */ > +/* { dg-final { scan-assembler "jmp\[ \t\]*__x86_indirect_thunk(_nt|)_(r|e)ax" } } */ > /* { dg-final { scan-assembler-not {\t(lfence|pause)} } } */ > /* { dg-final { scan-assembler-not "jmp\[ \t\]*\.LIND" } } */ > /* { dg-final { scan-assembler-not "call\[ \t\]*\.LIND" } } */ > diff --git a/gcc/testsuite/gcc.target/i386/indirect-thunk-extern-7.c b/gcc/testsuite/gcc.target/i386/indirect-thunk-extern-7.c > index 2b9a33e93dc..b7339745116 100644 > --- a/gcc/testsuite/gcc.target/i386/indirect-thunk-extern-7.c > +++ b/gcc/testsuite/gcc.target/i386/indirect-thunk-extern-7.c > @@ -36,7 +36,7 @@ bar (int i) > } > > /* { dg-final { scan-assembler "mov(?:l|q)\[ \t\]*\.L\[0-9\]+\\(,%" { target *-*-linux* } } } */ > -/* { dg-final { scan-assembler "jmp\[ \t\]*__x86_indirect_thunk_(r|e)ax" } } */ > +/* { dg-final { scan-assembler "jmp\[ \t\]*__x86_indirect_thunk(_nt|)_(r|e)ax" } } */ > /* { dg-final { scan-assembler-not {\t(lfence|pause)} } } */ > /* { dg-final { scan-assembler-not "jmp\[ \t\]*\.LIND" } } */ > /* { dg-final { scan-assembler-not "call\[ \t\]*\.LIND" } } */ > diff --git a/gcc/testsuite/gcc.target/i386/pr85403.c b/gcc/testsuite/gcc.target/i386/pr85403.c > index f4fb12dd4e2..0bbd7ca5610 100644 > --- a/gcc/testsuite/gcc.target/i386/pr85403.c > +++ b/gcc/testsuite/gcc.target/i386/pr85403.c > @@ -7,4 +7,4 @@ int > foo () > { > return -2; > -} /* { dg-error "requires Intel CET support" } */ > +} > diff --git a/gcc/testsuite/gcc.target/i386/pr85417-1.c b/gcc/testsuite/gcc.target/i386/pr85417-1.c > new file mode 100644 > index 00000000000..17d52403744 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr85417-1.c > @@ -0,0 +1,17 @@ > +/* { dg-do compile } */ > +/* { dg-require-ifunc "" } */ > +/* { dg-options "-O3 -fcf-protection" } */ > +/* { dg-final { scan-assembler "vpshufb" } } */ > +/* { dg-final { scan-assembler "punpcklbw" } } */ > + > +__attribute__((target_clones("arch=core-avx2","arch=slm","default"))) > +void > +foo(char *in, char *out, int size) > +{ > + int i; > + for(i = 0; i < size; i++) > + { > + out[2 * i] = in[i]; > + out[2 * i + 1] = in[i]; > + } > +} > diff --git a/gcc/testsuite/gcc.target/i386/ret-thunk-26.c b/gcc/testsuite/gcc.target/i386/ret-thunk-26.c > index 9144e988735..dc722c2f5f9 100644 > --- a/gcc/testsuite/gcc.target/i386/ret-thunk-26.c > +++ b/gcc/testsuite/gcc.target/i386/ret-thunk-26.c > @@ -1,6 +1,6 @@ > /* PR target/r84530 */ > /* { dg-do run } */ > -/* { dg-options "-Os -mfunction-return=thunk" } */ > +/* { dg-options "-Os -mfunction-return=thunk -fcf-protection=none" } */ > > struct S { int i; }; > __attribute__((const, noinline, noclone)) > -- > 2.14.3 >
> -----Original Message----- > From: Uros Bizjak [mailto:ubizjak@gmail.com] > Sent: Thursday, April 19, 2018 3:36 PM > To: H.J. Lu <hjl.tools@gmail.com> > Cc: Richard Biener <richard.guenther@gmail.com>; gcc- > patches@gcc.gnu.org; Tsimbalist, Igor V <igor.v.tsimbalist@intel.com> > Subject: Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs > > On Thu, Apr 19, 2018 at 3:30 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > > On Wed, Apr 18, 2018 at 01:35:33PM +0200, Richard Biener wrote: > >> On Wed, Apr 18, 2018 at 1:24 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > >> > On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu <hjl.tools@gmail.com> > wrote: > >> >> On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu <hjl.tools@gmail.com> > wrote: > >> >>> On Tue, Apr 17, 2018 at 12:03 PM, H.J. Lu <hjl.tools@gmail.com> > wrote: > >> >>>> On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak > <ubizjak@gmail.com> wrote: > >> >>>>> On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu > <hongjiu.lu@intel.com> wrote: > >> >>>>>> -fcf-protection -mcet can't be used with IFUNC features, like > symbol > >> >>>>>> multiversioning or target clone, since IBT/SHSTK are applied to > the whole > >> >>>>>> program and they may be disabled in some functions. But - > fcf-protection > >> >>>>>> is implemented with multi-byte NOPs on all 64-bit processors > as well as > >> >>>>>> 32-bit processors starting with Pentium Pro. If -fcf-protection > requires > >> >>>>>> -mcet, IFUNC features can't be used on Linux when -fcf- > protection is > >> >>>>>> enabled by default. > >> >>>>>> > >> >>>>>> This patch changes -fcf-protection to to enable the NOP > portion of CET > >> >>>>>> ISAs unless IBT and/or SHSTK are disabled explicitly. The rest of > CET > >> >>>>>> ISAs, including intrinsics, still requires -mcet, -mibt or -mshstk. > >> >>>>>> > >> >>>>>> OK for trunk? > >> >>>>> > >> >>>>> As said in the PR, NOP sequences have non-zero cost in the > executable > >> >>>>> (they enlarge the executable), so I don't think this feature should > be > >> >>>>> enabled by default. > >> >>>>> > >> >>>>> There is always a configure option if someone wants their > compiler to > >> >>>>> always emit relevant multi-byte nops. > >> >>>> > >> >>>> What we need is an option to enable -fcf-function with multi-byte > NOPs > >> >>>> without -mcet which enables the full CET ISAs. A configure option > >> >>>> without the corresponding the command-line option makes test > and > >> >>>> debug difficult. I can add > >> >>>> > >> >>>> --enable-cf-function-nop or --with-cf-function-nop > >> >>>> > >> >>>> with > >> >>>> > >> >>>> -fct-function-nop > >> >>>> > >> >>> > >> >>> How about adding -mno-cet, which enables the NOP portion of > CET > >> >> > >> >> I meant -mnop-cet, not -mno-cet. > >> >> > >> > > >> > Here is a patch to add -mnop and use it with -fcf-protection. > >> > >> +mnop > >> +Target Report Var(flag_nop) Init(0) > >> +Support multi-byte NOP code generation. > >> > >> the option name is incredibly bad and the documentation doesn't make it > >> better either. The invoke.texi docs refer to duplicate {-mcet}. > >> > >> Isn't there a -fcf-protection sub-set that can be used to automatically > >> enable this? Or simply do this mode by default when > >> -fcf-protection is used but neither -mcet nor -mibt is enabled? > >> > > > > Since multi-byte NOPs are used to implement -fcf-protection on x86, we > > propose a new design for -fcf-protection: > > > > 1. -fcf-protection option will report the unsupported error on non-x86 > > platform. On x86 platform it's supported and inserts endbr-nop > > instructions and properties, depending on its value (full/branch/return) > > 2. -mcet/-mibt/-mshstk options control intrinsics only. > > 3. These options are independent and do not influence each other so no > > need for cross checking between them. > > > > OK for trunk? > > This patch touches only CET related code, so Igor's OK should be enough. I have reviewed the patch and I'm ok with it. Igor > Uros.
On Thu, Apr 19, 2018 at 06:30:37AM -0700, H.J. Lu wrote: > * config/i386/i386-c.c (ix86_target_macros_internal): Also > define __IBT__ and __SHSTK__ for -fcf-protection. > --- a/gcc/config/i386/i386-c.c > +++ b/gcc/config/i386/i386-c.c > @@ -499,13 +499,15 @@ 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) > + 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) > + 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__"); > } This looks completely wrong to me. 1) there is no way to find out through preprocessor macros if -mibt or -mshstk was actually used or not, so e.g. if you #include <cetintrin.h> and compile with -fcf-protection -mno-cet, then #ifndef __SHSTK__ #pragma GCC push_options #pragma GCC target ("shstk") #define __DISABLE_SHSTK__ #endif /* __SHSTK__ */ will not be done and thus the intrinsics will appear to be in in the default target (-mno-cet) 2) preexisting - __CET__ is predefined twice, it should be done only once using a condition that covers all cases when the macro should be defined Don't you want to just predefine __CET__ and not __IBT__/__SHSTK__ if -fcf-protection -mno-cet, to make it clear? Jakub
On Thu, Apr 19, 2018 at 12:25 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Thu, Apr 19, 2018 at 06:30:37AM -0700, H.J. Lu wrote: >> * config/i386/i386-c.c (ix86_target_macros_internal): Also >> define __IBT__ and __SHSTK__ for -fcf-protection. > >> --- a/gcc/config/i386/i386-c.c >> +++ b/gcc/config/i386/i386-c.c >> @@ -499,13 +499,15 @@ 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) >> + 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) >> + 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__"); >> } > > This looks completely wrong to me. > 1) there is no way to find out through preprocessor macros if > -mibt or -mshstk was actually used or not, so e.g. if you > #include <cetintrin.h> > and compile with -fcf-protection -mno-cet, then > #ifndef __SHSTK__ > #pragma GCC push_options > #pragma GCC target ("shstk") > #define __DISABLE_SHSTK__ > #endif /* __SHSTK__ */ > will not be done and thus the intrinsics will appear to be in > in the default target (-mno-cet) > 2) preexisting - __CET__ is predefined twice, it should be done only > once using a condition that covers all cases when the macro should be > defined > > Don't you want to just predefine __CET__ and not __IBT__/__SHSTK__ > if -fcf-protection -mno-cet, to make it clear? > We are removing -mibt: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85469 How about this? diff --git a/gcc/config/i386/i386-c.c b/gcc/config/i386/i386-c.c index fa8b3682b0c..26c7641075d 100644 --- a/gcc/config/i386/i386-c.c +++ b/gcc/config/i386/i386-c.c @@ -499,20 +499,14 @@ 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 (flag_cf_protection != CF_NONE) + def_or_undef (parse_in, "__CET__"); + if ((flag_cf_protection & CF_BRANCH)) + def_or_undef (parse_in, "__CET_IBT__"); + if ((flag_cf_protection & CF_RETURN)) + def_or_undef (parse_in, "__CET_SHSTK__"); if (isa_flag2 & OPTION_MASK_ISA_VAES) def_or_undef (parse_in, "__VAES__"); if (isa_flag & OPTION_MASK_ISA_VPCLMULQDQ) This adds __CET_IBT__ and __CET_SHSTK__.
> -----Original Message----- > From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches- > owner@gcc.gnu.org] On Behalf Of H.J. Lu > Sent: Thursday, April 19, 2018 10:02 PM > To: Jakub Jelinek <jakub@redhat.com> > Cc: Richard Biener <richard.guenther@gmail.com>; Uros Bizjak > <ubizjak@gmail.com>; gcc-patches@gcc.gnu.org; Tsimbalist, Igor V > <igor.v.tsimbalist@intel.com> > Subject: Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs > > On Thu, Apr 19, 2018 at 12:25 PM, Jakub Jelinek <jakub@redhat.com> > wrote: > > On Thu, Apr 19, 2018 at 06:30:37AM -0700, H.J. Lu wrote: > >> * config/i386/i386-c.c (ix86_target_macros_internal): Also > >> define __IBT__ and __SHSTK__ for -fcf-protection. > > > >> --- a/gcc/config/i386/i386-c.c > >> +++ b/gcc/config/i386/i386-c.c > >> @@ -499,13 +499,15 @@ 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) > >> + 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) > >> + 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__"); > >> } > > > > This looks completely wrong to me. > > 1) there is no way to find out through preprocessor macros if > > -mibt or -mshstk was actually used or not, so e.g. if you > > #include <cetintrin.h> > > and compile with -fcf-protection -mno-cet, then > > #ifndef __SHSTK__ > > #pragma GCC push_options > > #pragma GCC target ("shstk") > > #define __DISABLE_SHSTK__ > > #endif /* __SHSTK__ */ > > will not be done and thus the intrinsics will appear to be in > > in the default target (-mno-cet) > > 2) preexisting - __CET__ is predefined twice, it should be done only > > once using a condition that covers all cases when the macro should be > > defined > > > > Don't you want to just predefine __CET__ and not __IBT__/__SHSTK__ > > if -fcf-protection -mno-cet, to make it clear? > > > > We are removing -mibt: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85469 > > How about this? > > > diff --git a/gcc/config/i386/i386-c.c b/gcc/config/i386/i386-c.c > index fa8b3682b0c..26c7641075d 100644 > --- a/gcc/config/i386/i386-c.c > +++ b/gcc/config/i386/i386-c.c > @@ -499,20 +499,14 @@ 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 (flag_cf_protection != CF_NONE) > + def_or_undef (parse_in, "__CET__"); > + if ((flag_cf_protection & CF_BRANCH)) > + def_or_undef (parse_in, "__CET_IBT__"); > + if ((flag_cf_protection & CF_RETURN)) > + def_or_undef (parse_in, "__CET_SHSTK__"); > if (isa_flag2 & OPTION_MASK_ISA_VAES) > def_or_undef (parse_in, "__VAES__"); > if (isa_flag & OPTION_MASK_ISA_VPCLMULQDQ) > > This adds __CET_IBT__ and __CET_SHSTK__. As -fcf-protection and -mcet/-mibt/-mshstk are are disjoint and control different parts I agree with + if ((isa_flag & OPTION_MASK_ISA_SHSTK)) + def_or_undef (parse_in, "__SHSTK__"); + if (flag_cf_protection != CF_NONE) + def_or_undef (parse_in, "__CET__"); Why __CET_IBT__ and __CET_SHSTK__ are needed? Moreover the naming is confusing as 'IBT' and 'SHSTK' are related to HW features which are controlled by -m options. __CET__ seems to be enough. Igor > > -- > H.J.
On Thu, Apr 19, 2018 at 2:18 PM, Tsimbalist, Igor V <igor.v.tsimbalist@intel.com> wrote: >> -----Original Message----- >> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches- >> owner@gcc.gnu.org] On Behalf Of H.J. Lu >> Sent: Thursday, April 19, 2018 10:02 PM >> To: Jakub Jelinek <jakub@redhat.com> >> Cc: Richard Biener <richard.guenther@gmail.com>; Uros Bizjak >> <ubizjak@gmail.com>; gcc-patches@gcc.gnu.org; Tsimbalist, Igor V >> <igor.v.tsimbalist@intel.com> >> Subject: Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs >> >> On Thu, Apr 19, 2018 at 12:25 PM, Jakub Jelinek <jakub@redhat.com> >> wrote: >> > On Thu, Apr 19, 2018 at 06:30:37AM -0700, H.J. Lu wrote: >> >> * config/i386/i386-c.c (ix86_target_macros_internal): Also >> >> define __IBT__ and __SHSTK__ for -fcf-protection. >> > >> >> --- a/gcc/config/i386/i386-c.c >> >> +++ b/gcc/config/i386/i386-c.c >> >> @@ -499,13 +499,15 @@ 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) >> >> + 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) >> >> + 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__"); >> >> } >> > >> > This looks completely wrong to me. >> > 1) there is no way to find out through preprocessor macros if >> > -mibt or -mshstk was actually used or not, so e.g. if you >> > #include <cetintrin.h> >> > and compile with -fcf-protection -mno-cet, then >> > #ifndef __SHSTK__ >> > #pragma GCC push_options >> > #pragma GCC target ("shstk") >> > #define __DISABLE_SHSTK__ >> > #endif /* __SHSTK__ */ >> > will not be done and thus the intrinsics will appear to be in >> > in the default target (-mno-cet) >> > 2) preexisting - __CET__ is predefined twice, it should be done only >> > once using a condition that covers all cases when the macro should be >> > defined >> > >> > Don't you want to just predefine __CET__ and not __IBT__/__SHSTK__ >> > if -fcf-protection -mno-cet, to make it clear? >> > >> >> We are removing -mibt: >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85469 >> >> How about this? >> >> >> diff --git a/gcc/config/i386/i386-c.c b/gcc/config/i386/i386-c.c >> index fa8b3682b0c..26c7641075d 100644 >> --- a/gcc/config/i386/i386-c.c >> +++ b/gcc/config/i386/i386-c.c >> @@ -499,20 +499,14 @@ 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 (flag_cf_protection != CF_NONE) >> + def_or_undef (parse_in, "__CET__"); >> + if ((flag_cf_protection & CF_BRANCH)) >> + def_or_undef (parse_in, "__CET_IBT__"); >> + if ((flag_cf_protection & CF_RETURN)) >> + def_or_undef (parse_in, "__CET_SHSTK__"); >> if (isa_flag2 & OPTION_MASK_ISA_VAES) >> def_or_undef (parse_in, "__VAES__"); >> if (isa_flag & OPTION_MASK_ISA_VPCLMULQDQ) >> >> This adds __CET_IBT__ and __CET_SHSTK__. > > As -fcf-protection and -mcet/-mibt/-mshstk are are disjoint and > control different parts I agree with > > + if ((isa_flag & OPTION_MASK_ISA_SHSTK)) > + def_or_undef (parse_in, "__SHSTK__"); > + if (flag_cf_protection != CF_NONE) > + def_or_undef (parse_in, "__CET__"); > > Why __CET_IBT__ and __CET_SHSTK__ are needed? Moreover the naming is > confusing as 'IBT' and 'SHSTK' are related to HW features which are controlled > by -m options. __CET__ seems to be enough. > One needs to know if IBT and SHSTK are enabled by -fcf-protection. They will be checked by <cet.h> and glibc.
On Thu, Apr 19, 2018 at 03:08:06PM -0700, H.J. Lu wrote: > > As -fcf-protection and -mcet/-mibt/-mshstk are are disjoint and > > control different parts I agree with > > > > + if ((isa_flag & OPTION_MASK_ISA_SHSTK)) > > + def_or_undef (parse_in, "__SHSTK__"); > > + if (flag_cf_protection != CF_NONE) > > + def_or_undef (parse_in, "__CET__"); > > > > Why __CET_IBT__ and __CET_SHSTK__ are needed? Moreover the naming is > > confusing as 'IBT' and 'SHSTK' are related to HW features which are controlled > > by -m options. __CET__ seems to be enough. > > > > One needs to know if IBT and SHSTK are enabled by -fcf-protection. They will > be checked by <cet.h> and glibc. So can't you define __CET__ to 3 if CF_FULL, to 1 if CF_BRANCH and 2 if CF_RETURN? Then if code doesn't care which one it is, it can just #ifdef __CET__, otherwise it can test which of those is enabled. Implementation-wise it would probably need to be: if (flag_cf_protection != CF_NONE) { if (def_or_undef == cpp_undef) def_or_undef (parse_in, "__CET__"); else if ((flag_cf_protection & CF_FULL) == CF_FULL) def_or_undef (parse_in, "__CET__=3"); else if (flag_cf_protection & CF_BRANCH) def_or_undef (parse_in, "__CET__=1"); else if (flag_cf_protection & CF_RETURN) def_or_undef (parse_in, "__CET__=2"); } or so. Actually, because it doesn't depend on something that can change depending on target attributes, it probably doesn't even belong in this function, but to ix86_target_macros and there you can just cpp_define it, don't deal with cpp_undef at all. Jakub
> -----Original Message----- > From: H.J. Lu [mailto:hjl.tools@gmail.com] > Sent: Friday, April 20, 2018 12:08 AM > To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com> > Cc: Jakub Jelinek <jakub@redhat.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 Thu, Apr 19, 2018 at 2:18 PM, Tsimbalist, Igor V > <igor.v.tsimbalist@intel.com> wrote: > >> -----Original Message----- > >> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches- > >> owner@gcc.gnu.org] On Behalf Of H.J. Lu > >> Sent: Thursday, April 19, 2018 10:02 PM > >> To: Jakub Jelinek <jakub@redhat.com> > >> Cc: Richard Biener <richard.guenther@gmail.com>; Uros Bizjak > >> <ubizjak@gmail.com>; gcc-patches@gcc.gnu.org; Tsimbalist, Igor V > >> <igor.v.tsimbalist@intel.com> > >> Subject: Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs > >> > >> On Thu, Apr 19, 2018 at 12:25 PM, Jakub Jelinek <jakub@redhat.com> > >> wrote: > >> > On Thu, Apr 19, 2018 at 06:30:37AM -0700, H.J. Lu wrote: > >> >> * config/i386/i386-c.c (ix86_target_macros_internal): Also > >> >> define __IBT__ and __SHSTK__ for -fcf-protection. > >> > > >> >> --- a/gcc/config/i386/i386-c.c > >> >> +++ b/gcc/config/i386/i386-c.c > >> >> @@ -499,13 +499,15 @@ 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) > >> >> + 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) > >> >> + 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__"); > >> >> } > >> > > >> > This looks completely wrong to me. > >> > 1) there is no way to find out through preprocessor macros if > >> > -mibt or -mshstk was actually used or not, so e.g. if you > >> > #include <cetintrin.h> > >> > and compile with -fcf-protection -mno-cet, then > >> > #ifndef __SHSTK__ > >> > #pragma GCC push_options > >> > #pragma GCC target ("shstk") > >> > #define __DISABLE_SHSTK__ > >> > #endif /* __SHSTK__ */ > >> > will not be done and thus the intrinsics will appear to be in > >> > in the default target (-mno-cet) > >> > 2) preexisting - __CET__ is predefined twice, it should be done only > >> > once using a condition that covers all cases when the macro should be > >> > defined > >> > > >> > Don't you want to just predefine __CET__ and not __IBT__/__SHSTK__ > >> > if -fcf-protection -mno-cet, to make it clear? > >> > > >> > >> We are removing -mibt: > >> > >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85469 > >> > >> How about this? > >> > >> > >> diff --git a/gcc/config/i386/i386-c.c b/gcc/config/i386/i386-c.c > >> index fa8b3682b0c..26c7641075d 100644 > >> --- a/gcc/config/i386/i386-c.c > >> +++ b/gcc/config/i386/i386-c.c > >> @@ -499,20 +499,14 @@ 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 (flag_cf_protection != CF_NONE) > >> + def_or_undef (parse_in, "__CET__"); > >> + if ((flag_cf_protection & CF_BRANCH)) > >> + def_or_undef (parse_in, "__CET_IBT__"); > >> + if ((flag_cf_protection & CF_RETURN)) > >> + def_or_undef (parse_in, "__CET_SHSTK__"); > >> if (isa_flag2 & OPTION_MASK_ISA_VAES) > >> def_or_undef (parse_in, "__VAES__"); > >> if (isa_flag & OPTION_MASK_ISA_VPCLMULQDQ) > >> > >> This adds __CET_IBT__ and __CET_SHSTK__. > > > > As -fcf-protection and -mcet/-mibt/-mshstk are are disjoint and > > control different parts I agree with > > > > + if ((isa_flag & OPTION_MASK_ISA_SHSTK)) > > + def_or_undef (parse_in, "__SHSTK__"); > > + if (flag_cf_protection != CF_NONE) > > + def_or_undef (parse_in, "__CET__"); > > > > Why __CET_IBT__ and __CET_SHSTK__ are needed? Moreover the naming is > > confusing as 'IBT' and 'SHSTK' are related to HW features which are > controlled > > by -m options. __CET__ seems to be enough. > > > > One needs to know if IBT and SHSTK are enabled by -fcf-protection. They will > be checked by <cet.h> and glibc. Ok. But then __CET__ could be removed as it's easily calculated given these two _CET_IBT_ and _CET_SHSTK_ macros. Also individual macro is used mostly. Igor > -- > H.J.
On Thu, Apr 19, 2018 at 3:37 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Thu, Apr 19, 2018 at 03:08:06PM -0700, H.J. Lu wrote: >> > As -fcf-protection and -mcet/-mibt/-mshstk are are disjoint and >> > control different parts I agree with >> > >> > + if ((isa_flag & OPTION_MASK_ISA_SHSTK)) >> > + def_or_undef (parse_in, "__SHSTK__"); >> > + if (flag_cf_protection != CF_NONE) >> > + def_or_undef (parse_in, "__CET__"); >> > >> > Why __CET_IBT__ and __CET_SHSTK__ are needed? Moreover the naming is >> > confusing as 'IBT' and 'SHSTK' are related to HW features which are controlled >> > by -m options. __CET__ seems to be enough. >> > >> >> One needs to know if IBT and SHSTK are enabled by -fcf-protection. They will >> be checked by <cet.h> and glibc. > > So can't you define __CET__ to 3 if CF_FULL, to 1 if CF_BRANCH and 2 if > CF_RETURN? Then if code doesn't care which one it is, it can just #ifdef > __CET__, otherwise it can test which of those is enabled. > Implementation-wise it would probably need to be: > if (flag_cf_protection != CF_NONE) > { > if (def_or_undef == cpp_undef) > def_or_undef (parse_in, "__CET__"); > else if ((flag_cf_protection & CF_FULL) == CF_FULL) > def_or_undef (parse_in, "__CET__=3"); > else if (flag_cf_protection & CF_BRANCH) > def_or_undef (parse_in, "__CET__=1"); > else if (flag_cf_protection & CF_RETURN) > def_or_undef (parse_in, "__CET__=2"); > } > or so. Actually, because it doesn't depend on something that can change > depending on target attributes, it probably doesn't even belong in this > function, but to ix86_target_macros and there you can just cpp_define > it, don't deal with cpp_undef at all. Something like this?
> -----Original Message----- > From: H.J. Lu [mailto:hjl.tools@gmail.com] > Sent: Friday, April 20, 2018 3:17 AM > 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 Thu, Apr 19, 2018 at 3:37 PM, Jakub Jelinek <jakub@redhat.com> wrote: > > On Thu, Apr 19, 2018 at 03:08:06PM -0700, H.J. Lu wrote: > >> > As -fcf-protection and -mcet/-mibt/-mshstk are are disjoint and > >> > control different parts I agree with > >> > > >> > + if ((isa_flag & OPTION_MASK_ISA_SHSTK)) > >> > + def_or_undef (parse_in, "__SHSTK__"); > >> > + if (flag_cf_protection != CF_NONE) > >> > + def_or_undef (parse_in, "__CET__"); > >> > > >> > Why __CET_IBT__ and __CET_SHSTK__ are needed? Moreover the > naming is > >> > confusing as 'IBT' and 'SHSTK' are related to HW features which are > controlled > >> > by -m options. __CET__ seems to be enough. > >> > > >> > >> One needs to know if IBT and SHSTK are enabled by -fcf-protection. > They will > >> be checked by <cet.h> and glibc. > > > > So can't you define __CET__ to 3 if CF_FULL, to 1 if CF_BRANCH and 2 if > > CF_RETURN? Then if code doesn't care which one it is, it can just #ifdef > > __CET__, otherwise it can test which of those is enabled. > > Implementation-wise it would probably need to be: > > if (flag_cf_protection != CF_NONE) > > { > > if (def_or_undef == cpp_undef) > > def_or_undef (parse_in, "__CET__"); > > else if ((flag_cf_protection & CF_FULL) == CF_FULL) > > def_or_undef (parse_in, "__CET__=3"); > > else if (flag_cf_protection & CF_BRANCH) > > def_or_undef (parse_in, "__CET__=1"); > > else if (flag_cf_protection & CF_RETURN) > > def_or_undef (parse_in, "__CET__=2"); > > } > > or so. Actually, because it doesn't depend on something that can change > > depending on target attributes, it probably doesn't even belong in this > > function, but to ix86_target_macros and there you can just cpp_define > > it, don't deal with cpp_undef at all. > > Something like this? Shouldn't this -# ifdef __IBT__ +# if (__CET__ & 1) != 0 Be as -# ifdef __IBT__ +#ifdef __CET__ +# if (__CET__ & 1) != 0 OK otherwise. Igor > > -- > H.J.
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
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. 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"
> -----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
diff --git a/gcc/config/i386/cet.c b/gcc/config/i386/cet.c index 4a1e013fdde..eb3be171471 100644 --- a/gcc/config/i386/cet.c +++ b/gcc/config/i386/cet.c @@ -34,11 +34,11 @@ file_end_indicate_exec_stack_and_cet (void) unsigned int feature_1 = 0; - if (TARGET_IBT) + if (flag_cf_protection & CF_BRANCH) /* GNU_PROPERTY_X86_FEATURE_1_IBT. */ feature_1 |= 0x1; - if (TARGET_SHSTK) + if (flag_cf_protection & CF_RETURN) /* GNU_PROPERTY_X86_FEATURE_1_SHSTK. */ feature_1 |= 0x2; diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 9074526b8a1..5e96e19a0fb 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -2701,7 +2701,7 @@ public: /* opt_pass methods: */ virtual bool gate (function *) { - return ((flag_cf_protection & CF_BRANCH) && TARGET_IBT); + return ((flag_cf_protection & CF_BRANCH)); } virtual unsigned int execute (function *) @@ -4936,10 +4936,15 @@ ix86_option_override_internal (bool main_args_p, = (cf_protection_level) (opts->x_flag_cf_protection & ~CF_SET); if (cf_protection != CF_NONE) { + /* Since -fcf-protection is implemented with multi-byte NOPs on + all 64-bit processors as well as 32-bit processors starting + with Pentium Pro, allow -fcf-protection to enable the NOP + portion of CET unless CET is disabled explicitly. */ switch (cf_protection) { case CF_BRANCH: - if (! TARGET_IBT_P (opts->x_ix86_isa_flags2)) + if (!TARGET_IBT_P (opts->x_ix86_isa_flags2) + && opts->x_flag_cet >= 0) { error ("%<-fcf-protection=branch%> requires Intel CET " "support. Use -mcet or -mibt option to enable CET"); @@ -4948,7 +4953,8 @@ ix86_option_override_internal (bool main_args_p, } break; case CF_RETURN: - if (! TARGET_SHSTK_P (opts->x_ix86_isa_flags)) + if (!TARGET_SHSTK_P (opts->x_ix86_isa_flags) + && opts->x_flag_cet >= 0) { error ("%<-fcf-protection=return%> requires Intel CET " "support. Use -mcet or -mshstk option to enable CET"); @@ -4957,8 +4963,9 @@ ix86_option_override_internal (bool main_args_p, } break; case CF_FULL: - if ( ! TARGET_IBT_P (opts->x_ix86_isa_flags2) - || ! TARGET_SHSTK_P (opts->x_ix86_isa_flags)) + if ((!TARGET_IBT_P (opts->x_ix86_isa_flags2) + || !TARGET_SHSTK_P (opts->x_ix86_isa_flags)) + && opts->x_flag_cet >= 0) { error ("%<-fcf-protection=full%> requires Intel CET " "support. Use -mcet or both of -mibt and " @@ -30399,7 +30406,7 @@ ix86_trampoline_init (rtx m_tramp, tree fndecl, rtx chain_value) rtx mem, fnaddr; int opcode; int offset = 0; - bool need_endbr = (flag_cf_protection & CF_BRANCH) && TARGET_IBT; + bool need_endbr = (flag_cf_protection & CF_BRANCH); fnaddr = XEXP (DECL_RTL (fndecl), 0); @@ -41757,7 +41764,7 @@ x86_output_mi_thunk (FILE *file, tree, HOST_WIDE_INT delta, emit_note (NOTE_INSN_PROLOGUE_END); /* CET is enabled, insert EB instruction. */ - if ((flag_cf_protection & CF_BRANCH) && TARGET_IBT) + if ((flag_cf_protection & CF_BRANCH)) emit_insn (gen_nop_endbr ()); /* If VCALL_OFFSET, we'll need THIS in a register. Might as well @@ -49757,7 +49764,7 @@ ix86_bnd_prefixed_insn_p (rtx insn) static bool ix86_notrack_prefixed_insn_p (rtx insn) { - if (!insn || !((flag_cf_protection & CF_BRANCH) && TARGET_IBT)) + if (!insn || !((flag_cf_protection & CF_BRANCH))) return false; if (CALL_P (insn)) diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index 352212094ec..b7bd71dc3ec 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -20278,7 +20278,7 @@ (define_insn "rdssp<mode>" [(set (match_operand:SWI48x 0 "register_operand" "=r") (unspec_volatile:SWI48x [(const_int 0)] UNSPECV_NOP_RDSSP))] - "TARGET_SHSTK" + "TARGET_SHSTK || flag_cet" "xor{l}\t%k0, %k0\n\trdssp<mskmodesuffix>\t%0" [(set_attr "length" "6") (set_attr "type" "other")]) @@ -20286,7 +20286,7 @@ (define_insn "incssp<mode>" [(unspec_volatile [(match_operand:SWI48x 0 "register_operand" "r")] UNSPECV_INCSSP)] - "TARGET_SHSTK" + "TARGET_SHSTK || flag_cet" "incssp<mskmodesuffix>\t%0" [(set_attr "length" "4") (set_attr "type" "other")]) @@ -20341,7 +20341,7 @@ (define_insn "nop_endbr" [(unspec_volatile [(const_int 0)] UNSPECV_NOP_ENDBR)] - "TARGET_IBT" + "TARGET_IBT || flag_cet" "* { return (TARGET_64BIT)? \"endbr64\" : \"endbr32\"; }" [(set_attr "length" "4") diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt index c063ae8b1ae..dea8551bf7f 100644 --- a/gcc/config/i386/i386.opt +++ b/gcc/config/i386/i386.opt @@ -1007,7 +1007,7 @@ Target Report RejectNegative Mask(GENERAL_REGS_ONLY) Var(ix86_target_flags) Save Generate code which uses only the general registers. mcet -Target Report Var(flag_cet) Init(0) +Target Report Var(flag_cet) Init(-1) Support Control-flow Enforcement Technology (CET) built-in functions and code generation. diff --git a/gcc/testsuite/c-c++-common/attr-nocf-check-1.c b/gcc/testsuite/c-c++-common/attr-nocf-check-1.c index 15f69731b91..c5ac7cb9f86 100644 --- a/gcc/testsuite/c-c++-common/attr-nocf-check-1.c +++ b/gcc/testsuite/c-c++-common/attr-nocf-check-1.c @@ -1,4 +1,5 @@ /* { dg-do compile } */ +/* { dg-additional-options "-fcf-protection=none" } */ int func (int) __attribute__ ((nocf_check)); /* { dg-warning "'nocf_check' attribute ignored" } */ int (*fptr) (int) __attribute__ ((nocf_check)); /* { dg-warning "'nocf_check' attribute ignored" } */ diff --git a/gcc/testsuite/c-c++-common/attr-nocf-check-3.c b/gcc/testsuite/c-c++-common/attr-nocf-check-3.c index ad1ca7eec9b..02b56cb155e 100644 --- a/gcc/testsuite/c-c++-common/attr-nocf-check-3.c +++ b/gcc/testsuite/c-c++-common/attr-nocf-check-3.c @@ -1,4 +1,5 @@ /* { dg-do compile } */ +/* { dg-additional-options "-fcf-protection=none" } */ int foo (void) __attribute__ ((nocf_check)); /* { dg-warning "'nocf_check' attribute ignored" } */ void (*foo1) (void) __attribute__((nocf_check)); /* { dg-warning "'nocf_check' attribute ignored" } */ diff --git a/gcc/testsuite/c-c++-common/fcf-protection-1.c b/gcc/testsuite/c-c++-common/fcf-protection-1.c index 8e71f47dde0..f59a8fbdfdc 100644 --- a/gcc/testsuite/c-c++-common/fcf-protection-1.c +++ b/gcc/testsuite/c-c++-common/fcf-protection-1.c @@ -1,4 +1,3 @@ /* { dg-do compile } */ /* { dg-options "-fcf-protection=full" } */ -/* { dg-error "'-fcf-protection=full' requires Intel CET.*-mcet.*-mibt and -mshstk option" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */ /* { dg-error "'-fcf-protection=full' is not supported for this target" "" { target { ! "i?86-*-* x86_64-*-*" } } 0 } */ diff --git a/gcc/testsuite/c-c++-common/fcf-protection-2.c b/gcc/testsuite/c-c++-common/fcf-protection-2.c index d7d6db0e95d..61059725af6 100644 --- a/gcc/testsuite/c-c++-common/fcf-protection-2.c +++ b/gcc/testsuite/c-c++-common/fcf-protection-2.c @@ -1,4 +1,3 @@ /* { dg-do compile } */ /* { dg-options "-fcf-protection=branch" } */ -/* { dg-error "'-fcf-protection=branch' requires Intel CET.*-mcet or -mibt option" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */ /* { dg-error "'-fcf-protection=branch' is not supported for this target" "" { target { ! "i?86-*-* x86_64-*-*" } } 0 } */ diff --git a/gcc/testsuite/c-c++-common/fcf-protection-3.c b/gcc/testsuite/c-c++-common/fcf-protection-3.c index 5b903c5fa51..257e944c4a6 100644 --- a/gcc/testsuite/c-c++-common/fcf-protection-3.c +++ b/gcc/testsuite/c-c++-common/fcf-protection-3.c @@ -1,4 +1,3 @@ /* { dg-do compile } */ /* { dg-options "-fcf-protection=return" } */ -/* { dg-error "'-fcf-protection=return' requires Intel CET.*-mcet or -mshstk option" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */ /* { dg-error "'-fcf-protection=return' is not supported for this target" "" { target { ! "i?86-*-* x86_64-*-*" } } 0 } */ diff --git a/gcc/testsuite/c-c++-common/fcf-protection-5.c b/gcc/testsuite/c-c++-common/fcf-protection-5.c index d7a67801e2e..dc317f84b07 100644 --- a/gcc/testsuite/c-c++-common/fcf-protection-5.c +++ b/gcc/testsuite/c-c++-common/fcf-protection-5.c @@ -1,4 +1,3 @@ /* { dg-do compile } */ /* { dg-options "-fcf-protection" } */ -/* { dg-error "'-fcf-protection=full' requires Intel CET.*-mcet.*-mibt and -mshstk option" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */ /* { dg-error "'-fcf-protection=full' is not supported for this target" "" { target { ! "i?86-*-* x86_64-*-*" } } 0 } */ diff --git a/gcc/testsuite/c-c++-common/fcf-protection-6.c b/gcc/testsuite/c-c++-common/fcf-protection-6.c index 532e76e6915..a1e919a2f63 100644 --- a/gcc/testsuite/c-c++-common/fcf-protection-6.c +++ b/gcc/testsuite/c-c++-common/fcf-protection-6.c @@ -1,5 +1,4 @@ /* { dg-do compile } */ /* { dg-options "-fcf-protection=branch" } */ -/* { dg-additional-options "-mshstk" { target { i?86-*-* x86_64-*-* } } } */ -/* { dg-error "'-fcf-protection=branch' requires Intel CET.*-mcet or -mibt option" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */ +/* { dg-additional-options "-mshstk -mno-ibt" { target { i?86-*-* x86_64-*-* } } } */ /* { dg-error "'-fcf-protection=branch' is not supported for this target" "" { target { ! "i?86-*-* x86_64-*-*" } } 0 } */ diff --git a/gcc/testsuite/c-c++-common/fcf-protection-7.c b/gcc/testsuite/c-c++-common/fcf-protection-7.c index 4c879692708..9e89becdaad 100644 --- a/gcc/testsuite/c-c++-common/fcf-protection-7.c +++ b/gcc/testsuite/c-c++-common/fcf-protection-7.c @@ -1,5 +1,4 @@ /* { dg-do compile } */ /* { dg-options "-fcf-protection=return" } */ -/* { dg-additional-options "-mibt" { target { i?86-*-* x86_64-*-* } } } */ -/* { dg-error "'-fcf-protection=return' requires Intel CET.*-mcet or -mshstk option" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */ +/* { dg-additional-options "-mibt -mno-shstk" { target { i?86-*-* x86_64-*-* } } } */ /* { dg-error "'-fcf-protection=return' is not supported for this target" "" { target { ! "i?86-*-* x86_64-*-*" } } 0 } */ diff --git a/gcc/testsuite/gcc.dg/march-generic.c b/gcc/testsuite/gcc.dg/march-generic.c index fb5b83c7d74..f9c00e4a1c1 100644 --- a/gcc/testsuite/gcc.dg/march-generic.c +++ b/gcc/testsuite/gcc.dg/march-generic.c @@ -1,6 +1,6 @@ /* { dg-do compile { target i?86-*-* x86_64-*-* } } */ /* { dg-skip-if "" { *-*-* } { "-march=*" } { "" } } */ -/* { dg-options "-march=generic" } */ +/* { dg-options "-march=generic -fcf-protection=none" } */ /* { dg-error "'generic' CPU can be used only for '-mtune=' switch" "" { target *-*-* } 0 } */ /* { dg-bogus "march" "" { target *-*-* } 0 } */ int i; diff --git a/gcc/testsuite/gcc.target/i386/align-limit.c b/gcc/testsuite/gcc.target/i386/align-limit.c index d3d8dc5656e..849d741189c 100644 --- a/gcc/testsuite/gcc.target/i386/align-limit.c +++ b/gcc/testsuite/gcc.target/i386/align-limit.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -falign-functions=64 -flimit-function-alignment -march=amdfam10" } */ +/* { dg-options "-O2 -falign-functions=64 -flimit-function-alignment -march=amdfam10 -fcf-protection=none" } */ /* { dg-final { scan-assembler ".p2align 6,,1" } } */ /* { dg-final { scan-assembler-not ".p2align 6,,63" } } */ diff --git a/gcc/testsuite/gcc.target/i386/cet-label-3.c b/gcc/testsuite/gcc.target/i386/cet-label-3.c new file mode 100644 index 00000000000..ae3ea632a27 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/cet-label-3.c @@ -0,0 +1,16 @@ +/* Verify that CET works. */ +/* { dg-do compile } */ +/* { dg-options "-O -fcf-protection" } */ +/* { dg-final { scan-assembler-times "endbr32" 3 { target ia32 } } } */ +/* { dg-final { scan-assembler-times "endbr64" 3 { target { ! ia32 } } } } */ + +int func (int arg) +{ + static void *array[] = { &&foo, &&bar }; + + goto *array[arg]; +foo: + return arg*111; +bar: + return arg*777; +} diff --git a/gcc/testsuite/gcc.target/i386/cet-notrack-icf-1.c b/gcc/testsuite/gcc.target/i386/cet-notrack-icf-1.c index 7987d53d305..00a3f3e5d5f 100644 --- a/gcc/testsuite/gcc.target/i386/cet-notrack-icf-1.c +++ b/gcc/testsuite/gcc.target/i386/cet-notrack-icf-1.c @@ -1,6 +1,6 @@ /* Verify nocf_check functions are not ICF optimized. */ /* { dg-do compile } */ -/* { dg-options "-O2" } */ +/* { dg-options "-O2 -fcf-protection=none -mno-cet" } */ /* { dg-final { scan-assembler-not "endbr" } } */ /* { dg-final { scan-assembler-not "fn3:" } } */ /* { dg-final { scan-assembler "set\[ \t]+fn2,fn1" } } */ diff --git a/gcc/testsuite/gcc.target/i386/cet-notrack-icf-3.c b/gcc/testsuite/gcc.target/i386/cet-notrack-icf-3.c index 07c4a6b61ef..c8b26f947d3 100644 --- a/gcc/testsuite/gcc.target/i386/cet-notrack-icf-3.c +++ b/gcc/testsuite/gcc.target/i386/cet-notrack-icf-3.c @@ -1,6 +1,6 @@ /* Verify nocf_check function calls are not ICF optimized. */ /* { dg-do compile } */ -/* { dg-options "-O2" } */ +/* { dg-options "-O2 -fcf-protection=none -mno-cet" } */ /* { dg-final { scan-assembler-not "endbr" } } */ /* { dg-final { scan-assembler-not "fn2:" } } */ /* { dg-final { scan-assembler "set\[ \t]+fn2,fn1" } } */ diff --git a/gcc/testsuite/gcc.target/i386/cet-property-2.c b/gcc/testsuite/gcc.target/i386/cet-property-2.c index 5a87dab92f1..bca6f6cdeb7 100644 --- a/gcc/testsuite/gcc.target/i386/cet-property-2.c +++ b/gcc/testsuite/gcc.target/i386/cet-property-2.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-mcet" } */ +/* { dg-options "-mcet -fcf-protection=none" } */ /* { dg-final { scan-assembler-not ".note.gnu.property" } } */ extern void foo (void); diff --git a/gcc/testsuite/gcc.target/i386/cet-property-3.c b/gcc/testsuite/gcc.target/i386/cet-property-3.c new file mode 100644 index 00000000000..3e211c970aa --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/cet-property-3.c @@ -0,0 +1,11 @@ +/* { dg-do compile { target *-*-linux* } } */ +/* { dg-options "-fcf-protection" } */ +/* { dg-final { scan-assembler ".note.gnu.property" } } */ + +extern void foo (void); + +void +bar (void) +{ + foo (); +} diff --git a/gcc/testsuite/gcc.target/i386/cet-sjlj-7.c b/gcc/testsuite/gcc.target/i386/cet-sjlj-7.c new file mode 100644 index 00000000000..1b624327d0f --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/cet-sjlj-7.c @@ -0,0 +1,48 @@ +/* { dg-do compile } */ +/* { dg-options "-O -fcf-protection" } */ +/* { dg-final { scan-assembler-times "endbr32" 2 { target ia32 } } } */ +/* { dg-final { scan-assembler-times "endbr64" 2 { target { ! ia32 } } } } */ +/* { dg-final { scan-assembler-times "call _?setjmp" 1 } } */ +/* { dg-final { scan-assembler-times "call longjmp" 1 } } */ + +#include <stdio.h> +#include <setjmp.h> + +jmp_buf buf; +static int bar (int); + +__attribute__ ((noinline, noclone)) +static int +foo (int i) +{ + int j = i * 11; + + if (!setjmp (buf)) + { + j += 33; + printf ("After setjmp: j = %d\n", j); + bar (j); + } + + return j + i; +} + +__attribute__ ((noinline, noclone)) +static int +bar (int i) +{ + int j = i; + + j -= 111; + printf ("In longjmp: j = %d\n", j); + longjmp (buf, 1); + + return j; +} + +int +main () +{ + foo (10); + return 0; +} diff --git a/gcc/testsuite/gcc.target/i386/indirect-thunk-attr-7.c b/gcc/testsuite/gcc.target/i386/indirect-thunk-attr-7.c index d53fc887dcc..5c120519ad9 100644 --- a/gcc/testsuite/gcc.target/i386/indirect-thunk-attr-7.c +++ b/gcc/testsuite/gcc.target/i386/indirect-thunk-attr-7.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -mno-indirect-branch-register -mfunction-return=keep -fno-pic" } */ +/* { dg-options "-O2 -mno-indirect-branch-register -mfunction-return=keep -fno-pic -fcf-protection=none" } */ void func0 (void); void func1 (void); diff --git a/gcc/testsuite/gcc.target/i386/indirect-thunk-extern-7.c b/gcc/testsuite/gcc.target/i386/indirect-thunk-extern-7.c index 2b9a33e93dc..74af6198bf4 100644 --- a/gcc/testsuite/gcc.target/i386/indirect-thunk-extern-7.c +++ b/gcc/testsuite/gcc.target/i386/indirect-thunk-extern-7.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -mno-indirect-branch-register -mfunction-return=keep -mindirect-branch=thunk-extern -fno-pic" } */ +/* { dg-options "-O2 -mno-indirect-branch-register -mfunction-return=keep -mindirect-branch=thunk-extern -fno-pic -fcf-protection=none" } */ void func0 (void); void func1 (void); diff --git a/gcc/testsuite/gcc.target/i386/pr85417-1.c b/gcc/testsuite/gcc.target/i386/pr85417-1.c new file mode 100644 index 00000000000..ee345961b73 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr85417-1.c @@ -0,0 +1,4 @@ +/* { dg-do compile } */ +/* { dg-options "-march=native -fcf-protection" } */ + +int foo; diff --git a/gcc/testsuite/gcc.target/i386/pr85417-2.c b/gcc/testsuite/gcc.target/i386/pr85417-2.c new file mode 100644 index 00000000000..17d52403744 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr85417-2.c @@ -0,0 +1,17 @@ +/* { dg-do compile } */ +/* { dg-require-ifunc "" } */ +/* { dg-options "-O3 -fcf-protection" } */ +/* { dg-final { scan-assembler "vpshufb" } } */ +/* { dg-final { scan-assembler "punpcklbw" } } */ + +__attribute__((target_clones("arch=core-avx2","arch=slm","default"))) +void +foo(char *in, char *out, int size) +{ + int i; + for(i = 0; i < size; i++) + { + out[2 * i] = in[i]; + out[2 * i + 1] = in[i]; + } +} diff --git a/gcc/testsuite/gcc.target/i386/ret-thunk-26.c b/gcc/testsuite/gcc.target/i386/ret-thunk-26.c index 9144e988735..dc722c2f5f9 100644 --- a/gcc/testsuite/gcc.target/i386/ret-thunk-26.c +++ b/gcc/testsuite/gcc.target/i386/ret-thunk-26.c @@ -1,6 +1,6 @@ /* PR target/r84530 */ /* { dg-do run } */ -/* { dg-options "-Os -mfunction-return=thunk" } */ +/* { dg-options "-Os -mfunction-return=thunk -fcf-protection=none" } */ struct S { int i; }; __attribute__((const, noinline, noclone))