> -----Original Message----- > From: H.J. Lu [mailto:hjl.to...@gmail.com] > Sent: Friday, April 20, 2018 3:17 AM > To: Jakub Jelinek <ja...@redhat.com> > Cc: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>; Richard Biener > <richard.guent...@gmail.com>; Uros Bizjak <ubiz...@gmail.com>; gcc- > patc...@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 <ja...@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.
