> -----Original Message----- > From: Uros Bizjak [mailto:ubiz...@gmail.com] > Sent: Thursday, April 19, 2018 3:36 PM > To: H.J. Lu <hjl.to...@gmail.com> > Cc: Richard Biener <richard.guent...@gmail.com>; gcc- > patc...@gcc.gnu.org; Tsimbalist, Igor V <igor.v.tsimbal...@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.to...@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.to...@gmail.com> wrote: > >> > On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu <hjl.to...@gmail.com> > wrote: > >> >> On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu <hjl.to...@gmail.com> > wrote: > >> >>> On Tue, Apr 17, 2018 at 12:03 PM, H.J. Lu <hjl.to...@gmail.com> > wrote: > >> >>>> On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak > <ubiz...@gmail.com> wrote: > >> >>>>> On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu > <hongjiu...@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.