RE: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
> -Original Message- > From: H.J. Lu [mailto:hjl.to...@gmail.com] > Sent: Friday, April 20, 2018 1:15 PM > To: Jakub Jelinek > Cc: Tsimbalist, Igor V ; Richard Biener > ; Uros Bizjak ; gcc- > patc...@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 +, 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 > Date: Thu Apr 19 15:15:04 2018 + > > 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_IS
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 +, 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 Date: Thu Apr 19 15:15:04 2018 + 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
Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
On Fri, Apr 20, 2018 at 06:25:10AM +, 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
Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
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 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
RE: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
> -Original Message- > From: H.J. Lu [mailto:hjl.to...@gmail.com] > Sent: Friday, April 20, 2018 3:17 AM > To: Jakub Jelinek > Cc: Tsimbalist, Igor V ; Richard Biener > ; Uros Bizjak ; 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 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 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.
Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
On Thu, Apr 19, 2018 at 3:37 PM, Jakub Jelinek 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 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? -- H.J. From 435b905d0dd60c3e6b00f42d7614a47972d69551 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Thu, 19 Apr 2018 14:09:51 -0700 Subject: [PATCH] Define __CET__ with flag_cf_protection and remove -mibt --- gcc/common/config/i386/i386-common.c | 17 - gcc/config/i386/cet.h| 6 +++--- gcc/config/i386/i386-c.c | 20 ++-- gcc/config/i386/i386.c | 2 -- gcc/config/i386/i386.md | 2 +- gcc/config/i386/i386.opt | 12 gcc/doc/invoke.texi | 23 ++- 7 files changed, 20 insertions(+), 62 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..c38e594ad92 100644 --- a/gcc/config/i386/cet.h +++ b/gcc/config/i386/cet.h @@ -32,7 +32,7 @@ #ifdef __ASSEMBLER__ -# ifdef __IBT__ +# if (__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/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_fl
RE: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
> -Original Message- > From: H.J. Lu [mailto:hjl.to...@gmail.com] > Sent: Friday, April 20, 2018 12:08 AM > To: Tsimbalist, Igor V > Cc: Jakub Jelinek ; Richard Biener > ; Uros Bizjak ; gcc- > patc...@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 > wrote: > >> -Original Message- > >> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > >> ow...@gcc.gnu.org] On Behalf Of H.J. Lu > >> Sent: Thursday, April 19, 2018 10:02 PM > >> To: Jakub Jelinek > >> Cc: Richard Biener ; Uros Bizjak > >> ; gcc-patches@gcc.gnu.org; Tsimbalist, Igor V > >> > >> Subject: Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs > >> > >> On Thu, Apr 19, 2018 at 12:25 PM, Jakub Jelinek > >> 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 > >> > 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__"); > >&g
Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
On Thu, Apr 19, 2018 at 2:18 PM, Tsimbalist, Igor V wrote: >> -Original Message- >> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- >> ow...@gcc.gnu.org] On Behalf Of H.J. Lu >> Sent: Thursday, April 19, 2018 10:02 PM >> To: Jakub Jelinek >> Cc: Richard Biener ; Uros Bizjak >> ; gcc-patches@gcc.gnu.org; Tsimbalist, Igor V >> >> Subject: Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs >> >> On Thu, Apr 19, 2018 at 12:25 PM, Jakub Jelinek >> 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 >> > 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 and glibc. -- H.J.
RE: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
> -Original Message- > From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > ow...@gcc.gnu.org] On Behalf Of H.J. Lu > Sent: Thursday, April 19, 2018 10:02 PM > To: Jakub Jelinek > Cc: Richard Biener ; Uros Bizjak > ; gcc-patches@gcc.gnu.org; Tsimbalist, Igor V > > Subject: Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs > > On Thu, Apr 19, 2018 at 12:25 PM, Jakub Jelinek > 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 > > 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.
Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
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 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
Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
On Thu, Apr 19, 2018 at 12:25 PM, Jakub Jelinek 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 > 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__. -- H.J.
RE: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
> -Original Message- > From: Uros Bizjak [mailto:ubiz...@gmail.com] > Sent: Thursday, April 19, 2018 3:36 PM > To: H.J. Lu > Cc: Richard Biener ; gcc- > patc...@gcc.gnu.org; Tsimbalist, Igor V > Subject: Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs > > On Thu, Apr 19, 2018 at 3:30 PM, H.J. Lu 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 wrote: > >> > On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu > wrote: > >> >> On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu > wrote: > >> >>> On Tue, Apr 17, 2018 at 12:03 PM, H.J. Lu > wrote: > >> >>>> On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak > wrote: > >> >>>>> On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu > 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.
Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
On Thu, Apr 19, 2018 at 3:30 PM, H.J. Lu 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 wrote: >> > On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu wrote: >> >> On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu wrote: >> >>> On Tue, Apr 17, 2018 at 12:03 PM, H.J. Lu wrote: >> On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak wrote: >> > On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu 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): Also enable for > -fcf-protection. > (incssp): Likewise. >
Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
On Thu, Apr 19, 2018 at 3:30 PM, H.J. Lu 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 wrote: >> > On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu wrote: >> >> On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu wrote: >> >>> On Tue, Apr 17, 2018 at 12:03 PM, H.J. Lu wrote: >> On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak wrote: >> > On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu 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.
Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
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 wrote: > > On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu wrote: > >> On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu wrote: > >>> On Tue, Apr 17, 2018 at 12:03 PM, H.J. Lu wrote: > On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak wrote: > > On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu 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): Also enable for -fcf-protection. (incssp): 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
Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
On Wed, Apr 18, 2018 at 5:32 AM, Uros Bizjak wrote: > On Wed, Apr 18, 2018 at 2:09 PM, Jakub Jelinek 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 wrote: >>> > > On Wed, Apr 18, 2018 at 1:39 PM, H.J. Lu 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 >> >> * 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.jj2018-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.texi2018-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. -- H.J.
Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
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 wrote: > > On Wed, Apr 18, 2018 at 1:39 PM, H.J. Lu 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
Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
On Wed, Apr 18, 2018 at 2:09 PM, Jakub Jelinek 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 wrote: >> > > On Wed, Apr 18, 2018 at 1:39 PM, H.J. Lu 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 > > * 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.jj2018-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.texi2018-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
RE: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
> -Original Message- > From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > ow...@gcc.gnu.org] On Behalf Of Jakub Jelinek > Sent: Wednesday, April 18, 2018 2:10 PM > To: H.J. Lu > Cc: Uros Bizjak ; Richard Biener > ; gcc-patches@gcc.gnu.org; Tsimbalist, Igor > V > 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 > wrote: > > > > On Wed, Apr 18, 2018 at 1:39 PM, H.J. Lu > 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 > > * 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
RE: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
> -Original Message- > From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > ow...@gcc.gnu.org] On Behalf Of H.J. Lu > Sent: Wednesday, April 18, 2018 1:39 PM > To: Richard Biener > Cc: Uros Bizjak ; gcc-patches@gcc.gnu.org; Tsimbalist, > Igor V > Subject: Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs > > On Wed, Apr 18, 2018 at 4:35 AM, Richard Biener > wrote: > > On Wed, Apr 18, 2018 at 1:24 PM, H.J. Lu wrote: > >> On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu wrote: > >>> On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu > wrote: > >>>> On Tue, Apr 17, 2018 at 12:03 PM, H.J. Lu > wrote: > >>>>> On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak > wrote: > >>>>>> On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu > 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.
Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
On Wed, Apr 18, 2018 at 5:08 AM, Uros Bizjak wrote: > On Wed, Apr 18, 2018 at 1:57 PM, H.J. Lu wrote: >> On Wed, Apr 18, 2018 at 4:55 AM, Uros Bizjak wrote: >>> On Wed, Apr 18, 2018 at 1:39 PM, H.J. Lu 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. -- H.J.
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 wrote: > > > On Wed, Apr 18, 2018 at 1:39 PM, H.J. Lu 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 * 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.jj2018-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.texi2018-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
Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
On Wed, Apr 18, 2018 at 1:57 PM, H.J. Lu wrote: > On Wed, Apr 18, 2018 at 4:55 AM, Uros Bizjak wrote: >> On Wed, Apr 18, 2018 at 1:39 PM, H.J. Lu 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.
Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
On Wed, Apr 18, 2018 at 4:55 AM, Uros Bizjak wrote: > On Wed, Apr 18, 2018 at 1:39 PM, H.J. Lu 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. -- H.J.
Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
On Wed, Apr 18, 2018 at 1:39 PM, H.J. Lu 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.
Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
On Wed, Apr 18, 2018 at 4:35 AM, Richard Biener wrote: > On Wed, Apr 18, 2018 at 1:24 PM, H.J. Lu wrote: >> On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu wrote: >>> On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu wrote: On Tue, Apr 17, 2018 at 12:03 PM, H.J. Lu wrote: > On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak wrote: >> On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu 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? -- H.J.
Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
On Wed, Apr 18, 2018 at 1:24 PM, H.J. Lu wrote: > On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu wrote: >> On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu wrote: >>> On Tue, Apr 17, 2018 at 12:03 PM, H.J. Lu wrote: On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak wrote: > On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu 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.
Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu wrote: > On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu wrote: >> On Tue, Apr 17, 2018 at 12:03 PM, H.J. Lu wrote: >>> On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak wrote: >>>> On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu 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. -- H.J. From cb4446aa122dfb7711240612c013e4cdbd73971f Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Mon, 16 Apr 2018 14:13:10 -0700 Subject: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs -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 adds -mnop to enable multi-byte NOP code generation which can be used with -fcf-protection to implement indirect branch and return address tracking without -mcet. 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 (pass_insert_endbranch::gate): Don't check TARGET_IBT. (ix86_option_override_internal): Allow -fcf-protection with multi-byte NOPs unless CET is disabled explicitly. (ix86_trampoline_init): Don't check TARGET_IBT. (x86_output_mi_thunk): Likewise. (ix86_notrack_prefixed_insn_p): Likewise. * config/i386/i386.md (rdssp): Also enable for flag_nop. (incssp): Likewise. (nop_endbr): Likewise. * config/i386/i386.opt: Add -mnop. * doc/invoke.texi: Document -mnop. 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. * c-c++-common/fcf-protection-1.c: Compile with -mno-nop 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-notrack-icf-1.c: Compile with -fcf-protection=none. * gcc.target/i386/cet-notrack-icf-3.c: Likewise. * gcc.target/i386/cet-property-2.c: Likewise. * gcc.target/i386/indirect-thunk-attr-7.c: Likewise. * gcc.target/i386/indirect-thunk-extern-7.c: Likewise. * gcc.target/i386/ret-thunk-26.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/pr85417-2.c: Likewise. * gcc.target/i386/pr85403.c: Compile with -mno-nop. --- gcc/config/i386/cet.c
Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
On Tue, Apr 17, 2018 at 12:03 PM, H.J. Lu wrote: > On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak wrote: >> On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu 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? -- H.J.
Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu wrote: > On Tue, Apr 17, 2018 at 12:03 PM, H.J. Lu wrote: >> On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak wrote: >>> On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu 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. -- H.J.
Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak wrote: > On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu 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 -- H.J.
Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu 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.
[PATCH] x86: Allow -fcf-protection with multi-byte NOPs
-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? H.J. --- 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 (pass_insert_endbranch::gate): Don't check TARGET_IBT. (ix86_option_override_internal): For -fcf-protection, set x_flag_cet to 1 if not set and also check x_flag_cet. (ix86_trampoline_init): Don't check TARGET_IBT. (x86_output_mi_thunk): Likewise. (ix86_notrack_prefixed_insn_p): Likewise. * config/i386/i386.md (rdssp): Also enable for flag_cet. (incssp): Likewise. (nop_endbr): Also enable for flag_cet. * config/i386/i386.opt (flag_cet): Initialized to -1. 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. * 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: Remove dg-additional-options and dg-error for x86 targets. * c-c++-common/fcf-protection-7.c: Likewise. * gcc.target/i386/cet-notrack-icf-1.c: Compile with -fcf-protection=none -mno-cet. * gcc.target/i386/cet-notrack-icf-3.c: Likewise. * gcc.target/i386/cet-property-2.c: Compile with -fcf-protection=none. * gcc.target/i386/indirect-thunk-attr-7.c: Likewise. * gcc.target/i386/indirect-thunk-extern-7.c: Likewise. * gcc.target/i386/ret-thunk-26.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/pr85417-2.c: Likewise. --- gcc/config/i386/cet.c | 4 +- gcc/config/i386/i386.c | 23 +++ gcc/config/i386/i386.md| 6 +-- gcc/config/i386/i386.opt | 2 +- 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 | 3 +- gcc/testsuite/c-c++-common/fcf-protection-7.c | 3 +- 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/pr85417-1.c | 4 ++ gcc/testsuite/gcc.target/i386/pr85417-2.c | 17 gcc/testsuite/gcc.target/i386/ret-thunk-26.c | 2 +- 25 files changed, 129 insertions(+), 30 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 create mode 100644 gcc/testsuite/gcc.target/i386/pr85417-2.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