* PR84239, Reimplement CET intrinsics for rdssp/incssp insn @ 2018-02-09 12:51 Tsimbalist, Igor V 2018-02-09 18:42 ` Sandra Loosemore 0 siblings, 1 reply; 7+ messages in thread From: Tsimbalist, Igor V @ 2018-02-09 12:51 UTC (permalink / raw) To: gcc-patches; +Cc: Uros Bizjak, Sandra Loosemore, Tsimbalist, Igor V [-- Attachment #1: Type: text/plain, Size: 387 bytes --] Introduce a couple of new CET intrinsics for reading and updating a shadow stack pointer (_get_ssp and _inc_ssp), which are more user friendly. They replace the existing _rdssp[d|q] and _incssp[d|q] instrinsics. The _get_ssp intrinsic has more deterministic semantic: it returns a value of the shadow stack pointer if HW is CET capable and 0 otherwise. Ok for trunk? Igor [-- Attachment #2: 0001-Reimplement-CET-intrinsics-for-rdssp-incssp-insn.patch --] [-- Type: application/octet-stream, Size: 14601 bytes --] From b2628a08ca7d0a77f631ff1d93d61ee4cc248ab1 Mon Sep 17 00:00:00 2001 From: Igor Tsimbalist <igor.v.tsimbalist@intel.com> Date: Wed, 7 Feb 2018 19:31:32 +0300 Subject: [PATCH] Reimplement CET intrinsics for rdssp/incssp insn PR target/84239 --- gcc/ChangeLog | 16 +++++++ gcc/config/i386/cetintrin.h | 31 ++++++-------- gcc/config/i386/i386-builtin-types.def | 1 + gcc/config/i386/i386-builtin.def | 4 +- gcc/config/i386/i386.c | 3 +- gcc/config/i386/i386.md | 16 ++++--- gcc/doc/extend.texi | 62 +++++++++++++++++++++++++--- gcc/testsuite/ChangeLog | 9 ++++ gcc/testsuite/gcc.target/i386/cet-intrin-3.c | 10 ++--- gcc/testsuite/gcc.target/i386/cet-intrin-4.c | 25 +---------- gcc/testsuite/gcc.target/i386/cet-rdssp-1.c | 8 ++-- libgcc/ChangeLog | 6 +++ libgcc/config/i386/shadow-stack-unwind.h | 17 +++----- 13 files changed, 126 insertions(+), 82 deletions(-) diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 51c45c0..937a474 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,19 @@ +2018-02-08 Igor Tsimbalist <igor.v.tsimbalist@intel.com> + + PR target/84239 + * config/i386/cetintrin.h: Remove _rdssp[d|q] and + add _get_ssp intrinsics. Remove argument from + __builtin_ia32_rdssp[d|q]. + * config/i386/i386-builtin-types.def: Add UINT_FTYPE_VOID. + * config/i386/i386-builtin.def: Remove argument from + __builtin_ia32_rdssp[d|q]. + * config/i386/i386.c: Use UINT_FTYPE_VOID. Use + ix86_expand_special_args_builtin for _rdssp[d|q]. + * config/i386/i386.md: Remove argument from rdssp[si|di] insn. + Clear register before usage. + * doc/extend.texi: Remove argument from __builtin_ia32_rdssp[d|q]. + Add documentation for new _get_ssp and _inc_ssp intrinsics. + 2018-02-07 H.J. Lu <hongjiu.lu@intel.com> PR target/84248 diff --git a/gcc/config/i386/cetintrin.h b/gcc/config/i386/cetintrin.h index 7a4b4d8..e9abcf3 100644 --- a/gcc/config/i386/cetintrin.h +++ b/gcc/config/i386/cetintrin.h @@ -34,37 +34,32 @@ #define __DISABLE_SHSTK__ #endif /* __SHSTK__ */ -extern __inline unsigned int -__attribute__((__gnu_inline__, __always_inline__, __artificial__)) -_rdsspd (unsigned int __B) -{ - return __builtin_ia32_rdsspd (__B); -} - #ifdef __x86_64__ extern __inline unsigned long long __attribute__((__gnu_inline__, __always_inline__, __artificial__)) -_rdsspq (unsigned long long __B) +_get_ssp (void) { - return __builtin_ia32_rdsspq (__B); + return __builtin_ia32_rdsspq (); } -#endif - -extern __inline void +#else +extern __inline unsigned int __attribute__((__gnu_inline__, __always_inline__, __artificial__)) -_incsspd (unsigned int __B) +_get_ssp (void) { - __builtin_ia32_incsspd (__B); + return __builtin_ia32_rdsspd (); } +#endif -#ifdef __x86_64__ extern __inline void __attribute__((__gnu_inline__, __always_inline__, __artificial__)) -_incsspq (unsigned long long __B) +_inc_ssp (unsigned int __B) { - __builtin_ia32_incsspq (__B); -} +#ifdef __x86_64__ + __builtin_ia32_incsspq ((unsigned long long) __B); +#else + __builtin_ia32_incsspd (__B); #endif +} extern __inline void __attribute__((__gnu_inline__, __always_inline__, __artificial__)) diff --git a/gcc/config/i386/i386-builtin-types.def b/gcc/config/i386/i386-builtin-types.def index ba33549..08360d2 100644 --- a/gcc/config/i386/i386-builtin-types.def +++ b/gcc/config/i386/i386-builtin-types.def @@ -192,6 +192,7 @@ DEF_POINTER_TYPE (PCV64QI, V64QI, CONST) DEF_FUNCTION_TYPE (FLOAT128) DEF_FUNCTION_TYPE (UINT64) DEF_FUNCTION_TYPE (UNSIGNED) +DEF_FUNCTION_TYPE (UINT) DEF_FUNCTION_TYPE (USHORT) DEF_FUNCTION_TYPE (INT) DEF_FUNCTION_TYPE (VOID) diff --git a/gcc/config/i386/i386-builtin.def b/gcc/config/i386/i386-builtin.def index 2caac88..a510196 100644 --- a/gcc/config/i386/i386-builtin.def +++ b/gcc/config/i386/i386-builtin.def @@ -3043,7 +3043,7 @@ BDESC (OPTION_MASK_ISA_SHSTK, CODE_FOR_clrssbsy, "__builtin_ia32_clrssbsy", IX86 BDESC_END (CET, CET_NORMAL) BDESC_FIRST (cet_rdssp, CET_NORMAL, - OPTION_MASK_ISA_SHSTK, CODE_FOR_rdsspsi, "__builtin_ia32_rdsspd", IX86_BUILTIN_RDSSPD, UNKNOWN, (int) UINT_FTYPE_UINT) -BDESC (OPTION_MASK_ISA_SHSTK | OPTION_MASK_ISA_64BIT, CODE_FOR_rdsspdi, "__builtin_ia32_rdsspq", IX86_BUILTIN_RDSSPQ, UNKNOWN, (int) UINT64_FTYPE_UINT64) + OPTION_MASK_ISA_SHSTK, CODE_FOR_rdsspsi, "__builtin_ia32_rdsspd", IX86_BUILTIN_RDSSPD, UNKNOWN, (int) UINT_FTYPE_VOID) +BDESC (OPTION_MASK_ISA_SHSTK | OPTION_MASK_ISA_64BIT, CODE_FOR_rdsspdi, "__builtin_ia32_rdsspq", IX86_BUILTIN_RDSSPQ, UNKNOWN, (int) UINT64_FTYPE_VOID) BDESC_END (CET_NORMAL, MAX) diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index fc3d6f0..53f9cae 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -35701,6 +35701,7 @@ ix86_expand_special_args_builtin (const struct builtin_description *d, case INT_FTYPE_VOID: case USHORT_FTYPE_VOID: case UINT64_FTYPE_VOID: + case UINT_FTYPE_VOID: case UNSIGNED_FTYPE_VOID: nargs = 0; klass = load; @@ -38490,7 +38491,7 @@ s4fma_expand: && fcode <= IX86_BUILTIN__BDESC_CET_NORMAL_LAST) { i = fcode - IX86_BUILTIN__BDESC_CET_NORMAL_FIRST; - return ix86_expand_args_builtin (bdesc_cet_rdssp + i, exp, + return ix86_expand_special_args_builtin (bdesc_cet_rdssp + i, exp, target); } diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index a4832bf..3998053 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -18391,8 +18391,8 @@ reg_ssp = gen_reg_rtx (word_mode); emit_insn (gen_rtx_SET (reg_ssp, const0_rtx)); emit_insn ((word_mode == SImode) - ? gen_rdsspsi (reg_ssp, reg_ssp) - : gen_rdsspdi (reg_ssp, reg_ssp)); + ? gen_rdsspsi (reg_ssp) + : gen_rdsspdi (reg_ssp)); emit_move_insn (mem, reg_ssp); } DONE; @@ -18437,8 +18437,8 @@ reg_ssp = gen_reg_rtx (word_mode); emit_insn (gen_rtx_SET (reg_ssp, const0_rtx)); emit_insn ((word_mode == SImode) - ? gen_rdsspsi (reg_ssp, reg_ssp) - : gen_rdsspdi (reg_ssp, reg_ssp)); + ? gen_rdsspsi (reg_ssp) + : gen_rdsspdi (reg_ssp)); mem_buf = gen_rtx_MEM (word_mode, plus_constant (Pmode, operands[0], 3 * GET_MODE_SIZE (ptr_mode))); @@ -20167,12 +20167,10 @@ ;; CET instructions (define_insn "rdssp<mode>" [(set (match_operand:SWI48x 0 "register_operand" "=r") - (unspec_volatile:SWI48x - [(match_operand:SWI48x 1 "register_operand" "0")] - UNSPECV_NOP_RDSSP))] + (unspec_volatile:SWI48x [(const_int 0)] UNSPECV_NOP_RDSSP))] "TARGET_SHSTK" - "rdssp<mskmodesuffix>\t%0" - [(set_attr "length" "4") + "xor{l}\t%k0, %k0\n\trdssp<mskmodesuffix>\t%0" + [(set_attr "length" "6") (set_attr "type" "other")]) (define_insn "incssp<mode>" diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index cb9df97..9f25dd9 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -12461,6 +12461,7 @@ instructions, but allow the compiler to schedule those calls. * TILEPro Built-in Functions:: * x86 Built-in Functions:: * x86 transactional memory intrinsics:: +* x86 control-flow protection intrinsics:: @end menu @node AArch64 Built-in Functions @@ -21772,13 +21773,17 @@ void __builtin_ia32_wrpkru (unsigned int) unsigned int __builtin_ia32_rdpkru () @end smallexample -The following built-in functions are available when @option{-mcet} is used. -They are used to support Intel Control-flow Enforcment Technology (CET). -Each built-in function generates the machine instruction that is part of the -function's name. +The following built-in functions are available when @option{-mcet} or +@option{-mshstk} option is used. They support shadow stack +machine instructions from Intel Control-flow Enforcment Technology (CET). +Each built-in function generates the machine instruction that is part +of the function's name. These are the internal low level functions. +Normally the functions in @ref{x86 control-flow protection intrinsics} +should be used instead. + @smallexample -unsigned int __builtin_ia32_rdsspd (unsigned int) -unsigned long long __builtin_ia32_rdsspq (unsigned long long) +unsigned int __builtin_ia32_rdsspd (void) +unsigned long long __builtin_ia32_rdsspq (void) void __builtin_ia32_incsspd (unsigned int) void __builtin_ia32_incsspq (unsigned long long) void __builtin_ia32_saveprevssp(void); @@ -21885,6 +21890,51 @@ else Note that, in most cases, the transactional and non-transactional code must synchronize together to ensure consistency. +@node x86 control-flow protection intrinsics +@subsection x86 Control-Flow Protection Intrinsics + +@deftypefn {CET Function} {ret_type} _get_ssp (void) +The @code{ret_type} is @code{unsigned long long} for x86-64 platform +and @code{unsigned int} for x86 pltform. +Get the current value of shadow stack pointer if shadow stack support +from Intel CET is enabled in the HW or @code{0} otherwise. +@end deftypefn + +@deftypefn {CET Function} void _inc_ssp (unsigned int) +Increment the current shadow stack pointer by the size specified by the +function argument. For security reason only unsigned byte value is used +from the argument. Therefore for the size greater than @code{255} the +function should be called several times. +@end deftypefn + +The shadow stack unwind code looks like: + +@smallexample +#include <immintrin.h> + +/* Unwind the shadow stack for EH. */ +#define _Unwind_Frames_Extra(x) \ + do \ + @{ \ + _Unwind_Word ssp = _get_ssp (); \ + if (ssp != 0) \ + @{ \ + _Unwind_Word tmp = (x); \ + while (tmp > 255) \ + @{ \ + _inc_ssp (tmp); \ + tmp -= 255; \ + @} \ + _inc_ssp (tmp); \ + @} \ + @} \ + while (0) +@end smallexample + +@noindent +This code runs unconditionally on all x86-64 processors and all x86 +processors that support multi-byte NOP instructions. + @node Target Format Checks @section Format Checks Specific to Particular Target Machines diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index c90dc88..3fa26cc 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,12 @@ +2018-02-08 Igor Tsimbalist <igor.v.tsimbalist@intel.com> + + PR target/84239 + * testsuite/gcc.target/i386/cet-intrin-3.c: Use new _get_ssp and + _inc_ssp intrinsics. + * testsuite/gcc.target/i386/cet-intrin-4.c: Likewise. + * testsuite/gcc.target/i386/cet-rdssp-1.c: Remove argument from + __builtin_ia32_rdssp[d|q]. + 2018-02-07 Tom de Vries <tom@codesourcery.com> * gcc.dg/pr83844.c: Require effective target alloca. diff --git a/gcc/testsuite/gcc.target/i386/cet-intrin-3.c b/gcc/testsuite/gcc.target/i386/cet-intrin-3.c index bcd7203..b98c1e9 100644 --- a/gcc/testsuite/gcc.target/i386/cet-intrin-3.c +++ b/gcc/testsuite/gcc.target/i386/cet-intrin-3.c @@ -10,24 +10,22 @@ unsigned int f1 () { - unsigned int x = 0; - return _rdsspd (x); + return _get_ssp (); } void f3 (unsigned int _a) { - _incsspd (_a); + _inc_ssp (_a); } #ifdef __x86_64__ unsigned long long f2 () { - unsigned long long x = 0; - return _rdsspq (x); + return _get_ssp (); } void f4 (unsigned int _a) { - _incsspq (_a); + _inc_ssp (_a); } #endif diff --git a/gcc/testsuite/gcc.target/i386/cet-intrin-4.c b/gcc/testsuite/gcc.target/i386/cet-intrin-4.c index 437a4cd..86957b2 100644 --- a/gcc/testsuite/gcc.target/i386/cet-intrin-4.c +++ b/gcc/testsuite/gcc.target/i386/cet-intrin-4.c @@ -5,27 +5,4 @@ /* { dg-final { scan-assembler "incssp\[dq]\[ \t]+(%|)\[re]di" { target { ! ia32 } } } } */ #include <immintrin.h> - -unsigned int f1 () -{ - unsigned int x = 0; - return _rdsspd (x); -} - -void f3 (unsigned int _a) -{ - _incsspd (_a); -} - -#ifdef __x86_64__ -unsigned long long f2 () -{ - unsigned long long x = 0; - return _rdsspq (x); -} - -void f4 (unsigned int _a) -{ - _incsspq (_a); -} -#endif +#include "cet-intrin-3.c" diff --git a/gcc/testsuite/gcc.target/i386/cet-rdssp-1.c b/gcc/testsuite/gcc.target/i386/cet-rdssp-1.c index fb50ff4..6cd24f6 100644 --- a/gcc/testsuite/gcc.target/i386/cet-rdssp-1.c +++ b/gcc/testsuite/gcc.target/i386/cet-rdssp-1.c @@ -5,18 +5,18 @@ void _exit(int status) __attribute__ ((__noreturn__)); #ifdef __x86_64__ # define incssp(x) __builtin_ia32_incsspq (x) -# define rdssp(x) __builtin_ia32_rdsspq (x) +# define rdssp() __builtin_ia32_rdsspq () #else # define incssp(x) __builtin_ia32_incsspd (x) -# define rdssp(x) __builtin_ia32_rdsspd (x) +# define rdssp() __builtin_ia32_rdsspd () #endif static void __attribute__ ((noinline, noclone)) test (unsigned long frames) { - unsigned long ssp = 0; - ssp = rdssp (ssp); + unsigned long ssp; + ssp = rdssp (); if (ssp != 0) { unsigned long tmp = frames; diff --git a/libgcc/ChangeLog b/libgcc/ChangeLog index 1eb1663..692773d 100644 --- a/libgcc/ChangeLog +++ b/libgcc/ChangeLog @@ -1,3 +1,9 @@ +2018-02-08 Igor Tsimbalist <igor.v.tsimbalist@intel.com> + + PR target/84239 + * config/i386/shadow-stack-unwind.hi (_Unwind_Frames_Extra): + Use new _get_ssp and _inc_ssp intrinsics. + 2018-02-02 Julia Koval <julia.koval@intel.com> * config/i386/cpuinfo.h (processor_subtypes): Add INTEL_COREI7_ICELAKE. diff --git a/libgcc/config/i386/shadow-stack-unwind.h b/libgcc/config/i386/shadow-stack-unwind.h index ef75d97..416e061 100644 --- a/libgcc/config/i386/shadow-stack-unwind.h +++ b/libgcc/config/i386/shadow-stack-unwind.h @@ -22,30 +22,23 @@ a copy of the GCC Runtime Library Exception along with this program; see the files COPYING3 and COPYING.RUNTIME respectively. If not, see <http://www.gnu.org/licenses/>. */ -#ifdef __x86_64__ -# define incssp(x) __builtin_ia32_incsspq ((x)) -# define rdssp(x) __builtin_ia32_rdsspq (x) -#else -# define incssp(x) __builtin_ia32_incsspd ((x)) -# define rdssp(x) __builtin_ia32_rdsspd (x) -#endif +#include <x86intrin.h> /* Unwind the shadow stack for EH. */ #undef _Unwind_Frames_Extra #define _Unwind_Frames_Extra(x) \ do \ { \ - unsigned long ssp = 0; \ - ssp = rdssp (ssp); \ + _Unwind_Word ssp = _get_ssp (); \ if (ssp != 0) \ { \ - unsigned long tmp = (x); \ + _Unwind_Word tmp = (x); \ while (tmp > 255) \ { \ - incssp (tmp); \ + _inc_ssp (tmp); \ tmp -= 255; \ } \ - incssp (tmp); \ + _inc_ssp (tmp); \ } \ } \ while (0) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PR84239, Reimplement CET intrinsics for rdssp/incssp insn 2018-02-09 12:51 PR84239, Reimplement CET intrinsics for rdssp/incssp insn Tsimbalist, Igor V @ 2018-02-09 18:42 ` Sandra Loosemore 2018-02-12 14:16 ` Tsimbalist, Igor V 0 siblings, 1 reply; 7+ messages in thread From: Sandra Loosemore @ 2018-02-09 18:42 UTC (permalink / raw) To: Tsimbalist, Igor V, gcc-patches; +Cc: Uros Bizjak On 02/09/2018 05:50 AM, Tsimbalist, Igor V wrote: > Introduce a couple of new CET intrinsics for reading and updating a shadow stack > pointer (_get_ssp and _inc_ssp), which are more user friendly. They replace the existing > _rdssp[d|q] and _incssp[d|q] instrinsics. The _get_ssp intrinsic has more deterministic > semantic: it returns a value of the shadow stack pointer if HW is CET capable and > 0 otherwise. > > Ok for trunk? Just reviewing the documentation part: > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi > index cb9df97..9f25dd9 100644 > --- a/gcc/doc/extend.texi > +++ b/gcc/doc/extend.texi > @@ -12461,6 +12461,7 @@ instructions, but allow the compiler to schedule those calls. > * TILEPro Built-in Functions:: > * x86 Built-in Functions:: > * x86 transactional memory intrinsics:: > +* x86 control-flow protection intrinsics:: > @end menu > > @node AArch64 Built-in Functions > @@ -21772,13 +21773,17 @@ void __builtin_ia32_wrpkru (unsigned int) > unsigned int __builtin_ia32_rdpkru () > @end smallexample > > -The following built-in functions are available when @option{-mcet} is used. > -They are used to support Intel Control-flow Enforcment Technology (CET). > -Each built-in function generates the machine instruction that is part of the > -function's name. > +The following built-in functions are available when @option{-mcet} or > +@option{-mshstk} option is used. They support shadow stack > +machine instructions from Intel Control-flow Enforcment Technology (CET). s/Enforcment/Enforcement/ > +Each built-in function generates the machine instruction that is part > +of the function's name. These are the internal low level functions. s/low level/low-level/ > +Normally the functions in @ref{x86 control-flow protection intrinsics} > +should be used instead. > + > @smallexample > -unsigned int __builtin_ia32_rdsspd (unsigned int) > -unsigned long long __builtin_ia32_rdsspq (unsigned long long) > +unsigned int __builtin_ia32_rdsspd (void) > +unsigned long long __builtin_ia32_rdsspq (void) > void __builtin_ia32_incsspd (unsigned int) > void __builtin_ia32_incsspq (unsigned long long) > void __builtin_ia32_saveprevssp(void); > @@ -21885,6 +21890,51 @@ else > Note that, in most cases, the transactional and non-transactional code > must synchronize together to ensure consistency. > > +@node x86 control-flow protection intrinsics > +@subsection x86 Control-Flow Protection Intrinsics > + > +@deftypefn {CET Function} {ret_type} _get_ssp (void) > +The @code{ret_type} is @code{unsigned long long} for x86-64 platform > +and @code{unsigned int} for x86 pltform. I'd prefer the sentence about the return type be placed after the description of what the function does. And please fix typos: s/x86-64 platform/64-bit targets/ s/x86 pltform/32-bit targets/ > +Get the current value of shadow stack pointer if shadow stack support > +from Intel CET is enabled in the HW or @code{0} otherwise. s/HW/hardware,/ > +@end deftypefn > + > +@deftypefn {CET Function} void _inc_ssp (unsigned int) > +Increment the current shadow stack pointer by the size specified by the > +function argument. For security reason only unsigned byte value is used > +from the argument. Therefore for the size greater than @code{255} the > +function should be called several times. How about rephrasing the last two sentences: The argument is masked to a byte value for security reasons, so to increment by more than 255 bytes you must call the function multiple times. > +@end deftypefn > + > +The shadow stack unwind code looks like: > + > +@smallexample > +#include <immintrin.h> > + > +/* Unwind the shadow stack for EH. */ > +#define _Unwind_Frames_Extra(x) \ > + do \ > + @{ \ > + _Unwind_Word ssp = _get_ssp (); \ > + if (ssp != 0) \ > + @{ \ > + _Unwind_Word tmp = (x); \ > + while (tmp > 255) \ > + @{ \ > + _inc_ssp (tmp); \ > + tmp -= 255; \ > + @} \ > + _inc_ssp (tmp); \ > + @} \ > + @} \ > + while (0) > +@end smallexample Tabs in Texinfo input don't work well. Please use spaces to format code environments. > + > +@noindent > +This code runs unconditionally on all x86-64 processors and all x86 > +processors that support multi-byte NOP instructions. s/x86-64 and all x86/32-bit and 64-bit/ > + > @node Target Format Checks > @section Format Checks Specific to Particular Target Machines > -Sandra ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: PR84239, Reimplement CET intrinsics for rdssp/incssp insn 2018-02-09 18:42 ` Sandra Loosemore @ 2018-02-12 14:16 ` Tsimbalist, Igor V 2018-02-14 7:07 ` Jeff Law 2018-02-15 0:24 ` Joseph Myers 0 siblings, 2 replies; 7+ messages in thread From: Tsimbalist, Igor V @ 2018-02-12 14:16 UTC (permalink / raw) To: Sandra Loosemore, gcc-patches; +Cc: Uros Bizjak, Tsimbalist, Igor V [-- Attachment #1: Type: text/plain, Size: 5250 bytes --] > -----Original Message----- > From: Sandra Loosemore [mailto:sandra@codesourcery.com] > Sent: Friday, February 9, 2018 7:42 PM > To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>; gcc- > patches@gcc.gnu.org > Cc: Uros Bizjak <ubizjak@gmail.com> > Subject: Re: PR84239, Reimplement CET intrinsics for rdssp/incssp insn > > On 02/09/2018 05:50 AM, Tsimbalist, Igor V wrote: > > Introduce a couple of new CET intrinsics for reading and updating a > shadow stack > > pointer (_get_ssp and _inc_ssp), which are more user friendly. They replace > the existing > > _rdssp[d|q] and _incssp[d|q] instrinsics. The _get_ssp intrinsic has more > deterministic > > semantic: it returns a value of the shadow stack pointer if HW is CET > capable and > > 0 otherwise. > > > > Ok for trunk? > > Just reviewing the documentation part: > > > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi > > index cb9df97..9f25dd9 100644 > > --- a/gcc/doc/extend.texi > > +++ b/gcc/doc/extend.texi > > @@ -12461,6 +12461,7 @@ instructions, but allow the compiler to > schedule those calls. > > * TILEPro Built-in Functions:: > > * x86 Built-in Functions:: > > * x86 transactional memory intrinsics:: > > +* x86 control-flow protection intrinsics:: > > @end menu > > > > @node AArch64 Built-in Functions > > @@ -21772,13 +21773,17 @@ void __builtin_ia32_wrpkru (unsigned int) > > unsigned int __builtin_ia32_rdpkru () > > @end smallexample > > > > -The following built-in functions are available when @option{-mcet} is > used. > > -They are used to support Intel Control-flow Enforcment Technology (CET). > > -Each built-in function generates the machine instruction that is part of > the > > -function's name. > > +The following built-in functions are available when @option{-mcet} or > > +@option{-mshstk} option is used. They support shadow stack > > +machine instructions from Intel Control-flow Enforcment Technology > (CET). > > s/Enforcment/Enforcement/ > > > +Each built-in function generates the machine instruction that is part > > +of the function's name. These are the internal low level functions. > > s/low level/low-level/ > > > +Normally the functions in @ref{x86 control-flow protection intrinsics} > > +should be used instead. > > + > > @smallexample > > -unsigned int __builtin_ia32_rdsspd (unsigned int) > > -unsigned long long __builtin_ia32_rdsspq (unsigned long long) > > +unsigned int __builtin_ia32_rdsspd (void) > > +unsigned long long __builtin_ia32_rdsspq (void) > > void __builtin_ia32_incsspd (unsigned int) > > void __builtin_ia32_incsspq (unsigned long long) > > void __builtin_ia32_saveprevssp(void); > > @@ -21885,6 +21890,51 @@ else > > Note that, in most cases, the transactional and non-transactional code > > must synchronize together to ensure consistency. > > > > +@node x86 control-flow protection intrinsics > > +@subsection x86 Control-Flow Protection Intrinsics > > + > > +@deftypefn {CET Function} {ret_type} _get_ssp (void) > > +The @code{ret_type} is @code{unsigned long long} for x86-64 platform > > +and @code{unsigned int} for x86 pltform. > > I'd prefer the sentence about the return type be placed after the > description of what the function does. And please fix typos: > s/x86-64 platform/64-bit targets/ > s/x86 pltform/32-bit targets/ > > > +Get the current value of shadow stack pointer if shadow stack support > > +from Intel CET is enabled in the HW or @code{0} otherwise. > > s/HW/hardware,/ > > > +@end deftypefn > > + > > +@deftypefn {CET Function} void _inc_ssp (unsigned int) > > +Increment the current shadow stack pointer by the size specified by the > > +function argument. For security reason only unsigned byte value is used > > +from the argument. Therefore for the size greater than @code{255} the > > +function should be called several times. > > How about rephrasing the last two sentences: > > The argument is masked to a byte value for security reasons, so to > increment by more than 255 bytes you must call the function multiple times. > > > +@end deftypefn > > + > > +The shadow stack unwind code looks like: > > + > > +@smallexample > > +#include <immintrin.h> > > + > > +/* Unwind the shadow stack for EH. */ > > +#define _Unwind_Frames_Extra(x) \ > > + do \ > > + @{ \ > > + _Unwind_Word ssp = _get_ssp (); \ > > + if (ssp != 0) \ > > + @{ \ > > + _Unwind_Word tmp = (x); \ > > + while (tmp > 255) \ > > + @{ \ > > + _inc_ssp (tmp); \ > > + tmp -= 255; \ > > + @} \ > > + _inc_ssp (tmp); \ > > + @} \ > > + @} \ > > + while (0) > > +@end smallexample > > Tabs in Texinfo input don't work well. Please use spaces to format code > environments. > > > + > > +@noindent > > +This code runs unconditionally on all x86-64 processors and all x86 > > +processors that support multi-byte NOP instructions. > > s/x86-64 and all x86/32-bit and 64-bit/ > > > + > > @node Target Format Checks > > @section Format Checks Specific to Particular Target Machines > > All comments are fixed. The updated patch is attached. Igor > -Sandra [-- Attachment #2: 0001-Reimplement-CET-intrinsics-for-rdssp-incssp-insn.patch --] [-- Type: application/octet-stream, Size: 14910 bytes --] From f9453d2f1eec40c04812ba4059c329fbe6fa9309 Mon Sep 17 00:00:00 2001 From: Igor Tsimbalist <igor.v.tsimbalist@intel.com> Date: Wed, 7 Feb 2018 19:31:32 +0300 Subject: [PATCH] Reimplement CET intrinsics for rdssp/incssp insn PR target/84239 --- gcc/ChangeLog | 16 +++++++ gcc/config/i386/cetintrin.h | 31 ++++++-------- gcc/config/i386/i386-builtin-types.def | 1 + gcc/config/i386/i386-builtin.def | 4 +- gcc/config/i386/i386.c | 3 +- gcc/config/i386/i386.md | 16 ++++--- gcc/doc/extend.texi | 62 +++++++++++++++++++++++++--- gcc/testsuite/ChangeLog | 9 ++++ gcc/testsuite/gcc.target/i386/cet-intrin-3.c | 10 ++--- gcc/testsuite/gcc.target/i386/cet-intrin-4.c | 25 +---------- gcc/testsuite/gcc.target/i386/cet-rdssp-1.c | 8 ++-- libgcc/ChangeLog | 6 +++ libgcc/config/i386/shadow-stack-unwind.h | 17 +++----- 13 files changed, 126 insertions(+), 82 deletions(-) diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 51c45c0..937a474 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,19 @@ +2018-02-08 Igor Tsimbalist <igor.v.tsimbalist@intel.com> + + PR target/84239 + * config/i386/cetintrin.h: Remove _rdssp[d|q] and + add _get_ssp intrinsics. Remove argument from + __builtin_ia32_rdssp[d|q]. + * config/i386/i386-builtin-types.def: Add UINT_FTYPE_VOID. + * config/i386/i386-builtin.def: Remove argument from + __builtin_ia32_rdssp[d|q]. + * config/i386/i386.c: Use UINT_FTYPE_VOID. Use + ix86_expand_special_args_builtin for _rdssp[d|q]. + * config/i386/i386.md: Remove argument from rdssp[si|di] insn. + Clear register before usage. + * doc/extend.texi: Remove argument from __builtin_ia32_rdssp[d|q]. + Add documentation for new _get_ssp and _inc_ssp intrinsics. + 2018-02-07 H.J. Lu <hongjiu.lu@intel.com> PR target/84248 diff --git a/gcc/config/i386/cetintrin.h b/gcc/config/i386/cetintrin.h index 7a4b4d8..e9abcf3 100644 --- a/gcc/config/i386/cetintrin.h +++ b/gcc/config/i386/cetintrin.h @@ -34,37 +34,32 @@ #define __DISABLE_SHSTK__ #endif /* __SHSTK__ */ -extern __inline unsigned int -__attribute__((__gnu_inline__, __always_inline__, __artificial__)) -_rdsspd (unsigned int __B) -{ - return __builtin_ia32_rdsspd (__B); -} - #ifdef __x86_64__ extern __inline unsigned long long __attribute__((__gnu_inline__, __always_inline__, __artificial__)) -_rdsspq (unsigned long long __B) +_get_ssp (void) { - return __builtin_ia32_rdsspq (__B); + return __builtin_ia32_rdsspq (); } -#endif - -extern __inline void +#else +extern __inline unsigned int __attribute__((__gnu_inline__, __always_inline__, __artificial__)) -_incsspd (unsigned int __B) +_get_ssp (void) { - __builtin_ia32_incsspd (__B); + return __builtin_ia32_rdsspd (); } +#endif -#ifdef __x86_64__ extern __inline void __attribute__((__gnu_inline__, __always_inline__, __artificial__)) -_incsspq (unsigned long long __B) +_inc_ssp (unsigned int __B) { - __builtin_ia32_incsspq (__B); -} +#ifdef __x86_64__ + __builtin_ia32_incsspq ((unsigned long long) __B); +#else + __builtin_ia32_incsspd (__B); #endif +} extern __inline void __attribute__((__gnu_inline__, __always_inline__, __artificial__)) diff --git a/gcc/config/i386/i386-builtin-types.def b/gcc/config/i386/i386-builtin-types.def index ba33549..08360d2 100644 --- a/gcc/config/i386/i386-builtin-types.def +++ b/gcc/config/i386/i386-builtin-types.def @@ -192,6 +192,7 @@ DEF_POINTER_TYPE (PCV64QI, V64QI, CONST) DEF_FUNCTION_TYPE (FLOAT128) DEF_FUNCTION_TYPE (UINT64) DEF_FUNCTION_TYPE (UNSIGNED) +DEF_FUNCTION_TYPE (UINT) DEF_FUNCTION_TYPE (USHORT) DEF_FUNCTION_TYPE (INT) DEF_FUNCTION_TYPE (VOID) diff --git a/gcc/config/i386/i386-builtin.def b/gcc/config/i386/i386-builtin.def index 2caac88..a510196 100644 --- a/gcc/config/i386/i386-builtin.def +++ b/gcc/config/i386/i386-builtin.def @@ -3043,7 +3043,7 @@ BDESC (OPTION_MASK_ISA_SHSTK, CODE_FOR_clrssbsy, "__builtin_ia32_clrssbsy", IX86 BDESC_END (CET, CET_NORMAL) BDESC_FIRST (cet_rdssp, CET_NORMAL, - OPTION_MASK_ISA_SHSTK, CODE_FOR_rdsspsi, "__builtin_ia32_rdsspd", IX86_BUILTIN_RDSSPD, UNKNOWN, (int) UINT_FTYPE_UINT) -BDESC (OPTION_MASK_ISA_SHSTK | OPTION_MASK_ISA_64BIT, CODE_FOR_rdsspdi, "__builtin_ia32_rdsspq", IX86_BUILTIN_RDSSPQ, UNKNOWN, (int) UINT64_FTYPE_UINT64) + OPTION_MASK_ISA_SHSTK, CODE_FOR_rdsspsi, "__builtin_ia32_rdsspd", IX86_BUILTIN_RDSSPD, UNKNOWN, (int) UINT_FTYPE_VOID) +BDESC (OPTION_MASK_ISA_SHSTK | OPTION_MASK_ISA_64BIT, CODE_FOR_rdsspdi, "__builtin_ia32_rdsspq", IX86_BUILTIN_RDSSPQ, UNKNOWN, (int) UINT64_FTYPE_VOID) BDESC_END (CET_NORMAL, MAX) diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index fc3d6f0..53f9cae 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -35701,6 +35701,7 @@ ix86_expand_special_args_builtin (const struct builtin_description *d, case INT_FTYPE_VOID: case USHORT_FTYPE_VOID: case UINT64_FTYPE_VOID: + case UINT_FTYPE_VOID: case UNSIGNED_FTYPE_VOID: nargs = 0; klass = load; @@ -38490,7 +38491,7 @@ s4fma_expand: && fcode <= IX86_BUILTIN__BDESC_CET_NORMAL_LAST) { i = fcode - IX86_BUILTIN__BDESC_CET_NORMAL_FIRST; - return ix86_expand_args_builtin (bdesc_cet_rdssp + i, exp, + return ix86_expand_special_args_builtin (bdesc_cet_rdssp + i, exp, target); } diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index a4832bf..3998053 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -18391,8 +18391,8 @@ reg_ssp = gen_reg_rtx (word_mode); emit_insn (gen_rtx_SET (reg_ssp, const0_rtx)); emit_insn ((word_mode == SImode) - ? gen_rdsspsi (reg_ssp, reg_ssp) - : gen_rdsspdi (reg_ssp, reg_ssp)); + ? gen_rdsspsi (reg_ssp) + : gen_rdsspdi (reg_ssp)); emit_move_insn (mem, reg_ssp); } DONE; @@ -18437,8 +18437,8 @@ reg_ssp = gen_reg_rtx (word_mode); emit_insn (gen_rtx_SET (reg_ssp, const0_rtx)); emit_insn ((word_mode == SImode) - ? gen_rdsspsi (reg_ssp, reg_ssp) - : gen_rdsspdi (reg_ssp, reg_ssp)); + ? gen_rdsspsi (reg_ssp) + : gen_rdsspdi (reg_ssp)); mem_buf = gen_rtx_MEM (word_mode, plus_constant (Pmode, operands[0], 3 * GET_MODE_SIZE (ptr_mode))); @@ -20167,12 +20167,10 @@ ;; CET instructions (define_insn "rdssp<mode>" [(set (match_operand:SWI48x 0 "register_operand" "=r") - (unspec_volatile:SWI48x - [(match_operand:SWI48x 1 "register_operand" "0")] - UNSPECV_NOP_RDSSP))] + (unspec_volatile:SWI48x [(const_int 0)] UNSPECV_NOP_RDSSP))] "TARGET_SHSTK" - "rdssp<mskmodesuffix>\t%0" - [(set_attr "length" "4") + "xor{l}\t%k0, %k0\n\trdssp<mskmodesuffix>\t%0" + [(set_attr "length" "6") (set_attr "type" "other")]) (define_insn "incssp<mode>" diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index cb9df97..d012eef 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -12461,6 +12461,7 @@ instructions, but allow the compiler to schedule those calls. * TILEPro Built-in Functions:: * x86 Built-in Functions:: * x86 transactional memory intrinsics:: +* x86 control-flow protection intrinsics:: @end menu @node AArch64 Built-in Functions @@ -21772,13 +21773,17 @@ void __builtin_ia32_wrpkru (unsigned int) unsigned int __builtin_ia32_rdpkru () @end smallexample -The following built-in functions are available when @option{-mcet} is used. -They are used to support Intel Control-flow Enforcment Technology (CET). -Each built-in function generates the machine instruction that is part of the -function's name. +The following built-in functions are available when @option{-mcet} or +@option{-mshstk} option is used. They support shadow stack +machine instructions from Intel Control-flow Enforcement Technology (CET). +Each built-in function generates the machine instruction that is part +of the function's name. These are the internal low-level functions. +Normally the functions in @ref{x86 control-flow protection intrinsics} +should be used instead. + @smallexample -unsigned int __builtin_ia32_rdsspd (unsigned int) -unsigned long long __builtin_ia32_rdsspq (unsigned long long) +unsigned int __builtin_ia32_rdsspd (void) +unsigned long long __builtin_ia32_rdsspq (void) void __builtin_ia32_incsspd (unsigned int) void __builtin_ia32_incsspq (unsigned long long) void __builtin_ia32_saveprevssp(void); @@ -21885,6 +21890,51 @@ else Note that, in most cases, the transactional and non-transactional code must synchronize together to ensure consistency. +@node x86 control-flow protection intrinsics +@subsection x86 Control-Flow Protection Intrinsics + +@deftypefn {CET Function} {ret_type} _get_ssp (void) +Get the current value of shadow stack pointer if shadow stack support +from Intel CET is enabled in the hardware or @code{0} otherwise. +The @code{ret_type} is @code{unsigned long long} for 64-bit targets +and @code{unsigned int} for 32-bit targets. +@end deftypefn + +@deftypefn {CET Function} void _inc_ssp (unsigned int) +Increment the current shadow stack pointer by the size specified by the +function argument. The argument is masked to a byte value for security +reasons, so to increment by more than 255 bytes you must call the function +multiple times. +@end deftypefn + +The shadow stack unwind code looks like: + +@smallexample +#include <immintrin.h> + +/* Unwind the shadow stack for EH. */ +#define _Unwind_Frames_Extra(x) \ + do \ + @{ \ + _Unwind_Word ssp = _get_ssp (); \ + if (ssp != 0) \ + @{ \ + _Unwind_Word tmp = (x); \ + while (tmp > 255) \ + @{ \ + _inc_ssp (tmp); \ + tmp -= 255; \ + @} \ + _inc_ssp (tmp); \ + @} \ + @} \ + while (0) +@end smallexample + +@noindent +This code runs unconditionally on all 64-bit processors. For 32-bit +processors the code runs on those that support multi-byte NOP instructions. + @node Target Format Checks @section Format Checks Specific to Particular Target Machines diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index c90dc88..3fa26cc 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,12 @@ +2018-02-08 Igor Tsimbalist <igor.v.tsimbalist@intel.com> + + PR target/84239 + * testsuite/gcc.target/i386/cet-intrin-3.c: Use new _get_ssp and + _inc_ssp intrinsics. + * testsuite/gcc.target/i386/cet-intrin-4.c: Likewise. + * testsuite/gcc.target/i386/cet-rdssp-1.c: Remove argument from + __builtin_ia32_rdssp[d|q]. + 2018-02-07 Tom de Vries <tom@codesourcery.com> * gcc.dg/pr83844.c: Require effective target alloca. diff --git a/gcc/testsuite/gcc.target/i386/cet-intrin-3.c b/gcc/testsuite/gcc.target/i386/cet-intrin-3.c index bcd7203..b98c1e9 100644 --- a/gcc/testsuite/gcc.target/i386/cet-intrin-3.c +++ b/gcc/testsuite/gcc.target/i386/cet-intrin-3.c @@ -10,24 +10,22 @@ unsigned int f1 () { - unsigned int x = 0; - return _rdsspd (x); + return _get_ssp (); } void f3 (unsigned int _a) { - _incsspd (_a); + _inc_ssp (_a); } #ifdef __x86_64__ unsigned long long f2 () { - unsigned long long x = 0; - return _rdsspq (x); + return _get_ssp (); } void f4 (unsigned int _a) { - _incsspq (_a); + _inc_ssp (_a); } #endif diff --git a/gcc/testsuite/gcc.target/i386/cet-intrin-4.c b/gcc/testsuite/gcc.target/i386/cet-intrin-4.c index 437a4cd..86957b2 100644 --- a/gcc/testsuite/gcc.target/i386/cet-intrin-4.c +++ b/gcc/testsuite/gcc.target/i386/cet-intrin-4.c @@ -5,27 +5,4 @@ /* { dg-final { scan-assembler "incssp\[dq]\[ \t]+(%|)\[re]di" { target { ! ia32 } } } } */ #include <immintrin.h> - -unsigned int f1 () -{ - unsigned int x = 0; - return _rdsspd (x); -} - -void f3 (unsigned int _a) -{ - _incsspd (_a); -} - -#ifdef __x86_64__ -unsigned long long f2 () -{ - unsigned long long x = 0; - return _rdsspq (x); -} - -void f4 (unsigned int _a) -{ - _incsspq (_a); -} -#endif +#include "cet-intrin-3.c" diff --git a/gcc/testsuite/gcc.target/i386/cet-rdssp-1.c b/gcc/testsuite/gcc.target/i386/cet-rdssp-1.c index fb50ff4..6cd24f6 100644 --- a/gcc/testsuite/gcc.target/i386/cet-rdssp-1.c +++ b/gcc/testsuite/gcc.target/i386/cet-rdssp-1.c @@ -5,18 +5,18 @@ void _exit(int status) __attribute__ ((__noreturn__)); #ifdef __x86_64__ # define incssp(x) __builtin_ia32_incsspq (x) -# define rdssp(x) __builtin_ia32_rdsspq (x) +# define rdssp() __builtin_ia32_rdsspq () #else # define incssp(x) __builtin_ia32_incsspd (x) -# define rdssp(x) __builtin_ia32_rdsspd (x) +# define rdssp() __builtin_ia32_rdsspd () #endif static void __attribute__ ((noinline, noclone)) test (unsigned long frames) { - unsigned long ssp = 0; - ssp = rdssp (ssp); + unsigned long ssp; + ssp = rdssp (); if (ssp != 0) { unsigned long tmp = frames; diff --git a/libgcc/ChangeLog b/libgcc/ChangeLog index 1eb1663..692773d 100644 --- a/libgcc/ChangeLog +++ b/libgcc/ChangeLog @@ -1,3 +1,9 @@ +2018-02-08 Igor Tsimbalist <igor.v.tsimbalist@intel.com> + + PR target/84239 + * config/i386/shadow-stack-unwind.hi (_Unwind_Frames_Extra): + Use new _get_ssp and _inc_ssp intrinsics. + 2018-02-02 Julia Koval <julia.koval@intel.com> * config/i386/cpuinfo.h (processor_subtypes): Add INTEL_COREI7_ICELAKE. diff --git a/libgcc/config/i386/shadow-stack-unwind.h b/libgcc/config/i386/shadow-stack-unwind.h index ef75d97..416e061 100644 --- a/libgcc/config/i386/shadow-stack-unwind.h +++ b/libgcc/config/i386/shadow-stack-unwind.h @@ -22,30 +22,23 @@ a copy of the GCC Runtime Library Exception along with this program; see the files COPYING3 and COPYING.RUNTIME respectively. If not, see <http://www.gnu.org/licenses/>. */ -#ifdef __x86_64__ -# define incssp(x) __builtin_ia32_incsspq ((x)) -# define rdssp(x) __builtin_ia32_rdsspq (x) -#else -# define incssp(x) __builtin_ia32_incsspd ((x)) -# define rdssp(x) __builtin_ia32_rdsspd (x) -#endif +#include <x86intrin.h> /* Unwind the shadow stack for EH. */ #undef _Unwind_Frames_Extra #define _Unwind_Frames_Extra(x) \ do \ { \ - unsigned long ssp = 0; \ - ssp = rdssp (ssp); \ + _Unwind_Word ssp = _get_ssp (); \ if (ssp != 0) \ { \ - unsigned long tmp = (x); \ + _Unwind_Word tmp = (x); \ while (tmp > 255) \ { \ - incssp (tmp); \ + _inc_ssp (tmp); \ tmp -= 255; \ } \ - incssp (tmp); \ + _inc_ssp (tmp); \ } \ } \ while (0) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PR84239, Reimplement CET intrinsics for rdssp/incssp insn 2018-02-12 14:16 ` Tsimbalist, Igor V @ 2018-02-14 7:07 ` Jeff Law 2018-02-15 0:24 ` Joseph Myers 1 sibling, 0 replies; 7+ messages in thread From: Jeff Law @ 2018-02-14 7:07 UTC (permalink / raw) To: Tsimbalist, Igor V, Sandra Loosemore, gcc-patches; +Cc: Uros Bizjak On 02/12/2018 07:16 AM, Tsimbalist, Igor V wrote: >> -----Original Message----- >> From: Sandra Loosemore [mailto:sandra@codesourcery.com] >> Sent: Friday, February 9, 2018 7:42 PM >> To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>; gcc- >> patches@gcc.gnu.org >> Cc: Uros Bizjak <ubizjak@gmail.com> >> Subject: Re: PR84239, Reimplement CET intrinsics for rdssp/incssp insn >> >> On 02/09/2018 05:50 AM, Tsimbalist, Igor V wrote: >>> Introduce a couple of new CET intrinsics for reading and updating a >> shadow stack >>> pointer (_get_ssp and _inc_ssp), which are more user friendly. They replace >> the existing >>> _rdssp[d|q] and _incssp[d|q] instrinsics. The _get_ssp intrinsic has more >> deterministic >>> semantic: it returns a value of the shadow stack pointer if HW is CET >> capable and >>> 0 otherwise. >>> >>> Ok for trunk? >> Just reviewing the documentation part: >> >>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi >>> index cb9df97..9f25dd9 100644 >>> --- a/gcc/doc/extend.texi >>> +++ b/gcc/doc/extend.texi >>> @@ -12461,6 +12461,7 @@ instructions, but allow the compiler to >> schedule those calls. >>> * TILEPro Built-in Functions:: >>> * x86 Built-in Functions:: >>> * x86 transactional memory intrinsics:: >>> +* x86 control-flow protection intrinsics:: >>> @end menu >>> >>> @node AArch64 Built-in Functions >>> @@ -21772,13 +21773,17 @@ void __builtin_ia32_wrpkru (unsigned int) >>> unsigned int __builtin_ia32_rdpkru () >>> @end smallexample >>> >>> -The following built-in functions are available when @option{-mcet} is >> used. >>> -They are used to support Intel Control-flow Enforcment Technology (CET). >>> -Each built-in function generates the machine instruction that is part of >> the >>> -function's name. >>> +The following built-in functions are available when @option{-mcet} or >>> +@option{-mshstk} option is used. They support shadow stack >>> +machine instructions from Intel Control-flow Enforcment Technology >> (CET). >> >> s/Enforcment/Enforcement/ >> >>> +Each built-in function generates the machine instruction that is part >>> +of the function's name. These are the internal low level functions. >> s/low level/low-level/ >> >>> +Normally the functions in @ref{x86 control-flow protection intrinsics} >>> +should be used instead. >>> + >>> @smallexample >>> -unsigned int __builtin_ia32_rdsspd (unsigned int) >>> -unsigned long long __builtin_ia32_rdsspq (unsigned long long) >>> +unsigned int __builtin_ia32_rdsspd (void) >>> +unsigned long long __builtin_ia32_rdsspq (void) >>> void __builtin_ia32_incsspd (unsigned int) >>> void __builtin_ia32_incsspq (unsigned long long) >>> void __builtin_ia32_saveprevssp(void); >>> @@ -21885,6 +21890,51 @@ else >>> Note that, in most cases, the transactional and non-transactional code >>> must synchronize together to ensure consistency. >>> >>> +@node x86 control-flow protection intrinsics >>> +@subsection x86 Control-Flow Protection Intrinsics >>> + >>> +@deftypefn {CET Function} {ret_type} _get_ssp (void) >>> +The @code{ret_type} is @code{unsigned long long} for x86-64 platform >>> +and @code{unsigned int} for x86 pltform. >> I'd prefer the sentence about the return type be placed after the >> description of what the function does. And please fix typos: >> s/x86-64 platform/64-bit targets/ >> s/x86 pltform/32-bit targets/ >> >>> +Get the current value of shadow stack pointer if shadow stack support >>> +from Intel CET is enabled in the HW or @code{0} otherwise. >> s/HW/hardware,/ >> >>> +@end deftypefn >>> + >>> +@deftypefn {CET Function} void _inc_ssp (unsigned int) >>> +Increment the current shadow stack pointer by the size specified by the >>> +function argument. For security reason only unsigned byte value is used >>> +from the argument. Therefore for the size greater than @code{255} the >>> +function should be called several times. >> How about rephrasing the last two sentences: >> >> The argument is masked to a byte value for security reasons, so to >> increment by more than 255 bytes you must call the function multiple times. >> >>> +@end deftypefn >>> + >>> +The shadow stack unwind code looks like: >>> + >>> +@smallexample >>> +#include <immintrin.h> >>> + >>> +/* Unwind the shadow stack for EH. */ >>> +#define _Unwind_Frames_Extra(x) \ >>> + do \ >>> + @{ \ >>> + _Unwind_Word ssp = _get_ssp (); \ >>> + if (ssp != 0) \ >>> + @{ \ >>> + _Unwind_Word tmp = (x); \ >>> + while (tmp > 255) \ >>> + @{ \ >>> + _inc_ssp (tmp); \ >>> + tmp -= 255; \ >>> + @} \ >>> + _inc_ssp (tmp); \ >>> + @} \ >>> + @} \ >>> + while (0) >>> +@end smallexample >> Tabs in Texinfo input don't work well. Please use spaces to format code >> environments. >> >>> + >>> +@noindent >>> +This code runs unconditionally on all x86-64 processors and all x86 >>> +processors that support multi-byte NOP instructions. >> s/x86-64 and all x86/32-bit and 64-bit/ >> >>> + >>> @node Target Format Checks >>> @section Format Checks Specific to Particular Target Machines >>> > All comments are fixed. The updated patch is attached. > > Igor > >> -Sandra > > 0001-Reimplement-CET-intrinsics-for-rdssp-incssp-insn.patch > > > From f9453d2f1eec40c04812ba4059c329fbe6fa9309 Mon Sep 17 00:00:00 2001 > From: Igor Tsimbalist <igor.v.tsimbalist@intel.com> > Date: Wed, 7 Feb 2018 19:31:32 +0300 > Subject: [PATCH] Reimplement CET intrinsics for rdssp/incssp insn > > PR target/84239 > --- > gcc/ChangeLog | 16 +++++++ > gcc/config/i386/cetintrin.h | 31 ++++++-------- > gcc/config/i386/i386-builtin-types.def | 1 + > gcc/config/i386/i386-builtin.def | 4 +- > gcc/config/i386/i386.c | 3 +- > gcc/config/i386/i386.md | 16 ++++--- > gcc/doc/extend.texi | 62 +++++++++++++++++++++++++--- > gcc/testsuite/ChangeLog | 9 ++++ > gcc/testsuite/gcc.target/i386/cet-intrin-3.c | 10 ++--- > gcc/testsuite/gcc.target/i386/cet-intrin-4.c | 25 +---------- > gcc/testsuite/gcc.target/i386/cet-rdssp-1.c | 8 ++-- > libgcc/ChangeLog | 6 +++ > libgcc/config/i386/shadow-stack-unwind.h | 17 +++----- > 13 files changed, 126 insertions(+), 82 deletions(-) [ ... ] OK. Thanks, Jeff ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: PR84239, Reimplement CET intrinsics for rdssp/incssp insn 2018-02-12 14:16 ` Tsimbalist, Igor V 2018-02-14 7:07 ` Jeff Law @ 2018-02-15 0:24 ` Joseph Myers 2018-02-15 16:36 ` Tsimbalist, Igor V 1 sibling, 1 reply; 7+ messages in thread From: Joseph Myers @ 2018-02-15 0:24 UTC (permalink / raw) To: Tsimbalist, Igor V; +Cc: Sandra Loosemore, gcc-patches, Uros Bizjak This patch has broken bootstrap of a cross toolchain for x86_64 (the case where inhibit_libc is defined because there is no libc for the target available at that stage in the bootstrap process). In file included from /scratch/jmyers/glibc-bot/build/compilers/x86_64-linux-gnu/gcc-first/gcc/include/xmmintrin.h:34, from /scratch/jmyers/glibc-bot/build/compilers/x86_64-linux-gnu/gcc-first/gcc/include/x86intrin.h:33, from /scratch/jmyers/glibc-bot/src/gcc/libgcc/config/i386/shadow-stack-unwind.h:25, from ./md-unwind-support.h:27, from /scratch/jmyers/glibc-bot/src/gcc/libgcc/unwind-dw2.c:411: ../../.././gcc/mm_malloc.h:27:10: fatal error: stdlib.h: No such file or directory #include <stdlib.h> ^~~~~~~~~~ https://sourceware.org/ml/libc-testresults/2018-q1/msg00307.html The patch makes shadow-stack-unwind.h include <x86intrin.h>, which ends up including <mm_malloc.h>, which includes <stdlib.h> and <errno.h> unconditionally. You can't include any libc system headers unconditionally from libgcc (only when inhibit_libc is not defined - and <mm_malloc.h>, being an installed header, can't test inhibit_libc because it's in the user's namespace). So I think you need to avoid the mm_malloc.h include here somehow (without adding any inhibit_libc conditionals to installed headers). -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: PR84239, Reimplement CET intrinsics for rdssp/incssp insn 2018-02-15 0:24 ` Joseph Myers @ 2018-02-15 16:36 ` Tsimbalist, Igor V 2018-02-15 21:20 ` Joseph Myers 0 siblings, 1 reply; 7+ messages in thread From: Tsimbalist, Igor V @ 2018-02-15 16:36 UTC (permalink / raw) To: Joseph Myers Cc: Sandra Loosemore, gcc-patches, Uros Bizjak, Tsimbalist, Igor V Igor > -----Original Message----- > From: Joseph Myers [mailto:joseph@codesourcery.com] > Sent: Thursday, February 15, 2018 1:24 AM > To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com> > Cc: Sandra Loosemore <sandra@codesourcery.com>; gcc- > patches@gcc.gnu.org; Uros Bizjak <ubizjak@gmail.com> > Subject: RE: PR84239, Reimplement CET intrinsics for rdssp/incssp insn > > This patch has broken bootstrap of a cross toolchain for x86_64 (the case > where inhibit_libc is defined because there is no libc for the target > available at that stage in the bootstrap process). > > In file included from > /scratch/jmyers/glibc-bot/build/compilers/x86_64-linux-gnu/gcc- > first/gcc/include/xmmintrin.h:34, > from > /scratch/jmyers/glibc-bot/build/compilers/x86_64-linux-gnu/gcc- > first/gcc/include/x86intrin.h:33, > from > /scratch/jmyers/glibc-bot/src/gcc/libgcc/config/i386/shadow-stack- > unwind.h:25, > from ./md-unwind-support.h:27, > from /scratch/jmyers/glibc-bot/src/gcc/libgcc/unwind-dw2.c:411: > ../../.././gcc/mm_malloc.h:27:10: fatal error: stdlib.h: No such file or directory > #include <stdlib.h> > ^~~~~~~~~~ > > https://sourceware.org/ml/libc-testresults/2018-q1/msg00307.html > > The patch makes shadow-stack-unwind.h include <x86intrin.h>, which ends > up > including <mm_malloc.h>, which includes <stdlib.h> and <errno.h> > unconditionally. You can't include any libc system headers > unconditionally from libgcc (only when inhibit_libc is not defined - and > <mm_malloc.h>, being an installed header, can't test inhibit_libc because > it's in the user's namespace). So I think you need to avoid the > mm_malloc.h include here somehow (without adding any inhibit_libc > conditionals to installed headers). Here is a proposed patch diff --git a/libgcc/config/i386/shadow-stack-unwind.h b/libgcc/config/i386/shadow-stack-unwind.h index 416e061..b7c3d98 100644 --- a/libgcc/config/i386/shadow-stack-unwind.h +++ b/libgcc/config/i386/shadow-stack-unwind.h @@ -22,7 +22,14 @@ a copy of the GCC Runtime Library Exception along with this program; see the files COPYING3 and COPYING.RUNTIME respectively. If not, see <http://www.gnu.org/licenses/>. */ -#include <x86intrin.h> +/* NB: We need _get_ssp and _inc_ssp from <cetintrin.h>. But we can't + include <x86intrin.h> which ends up including <mm_malloc.h>, which + includes <stdlib.h> and <errno.h> unconditionally. But we can't + include any libc system headers unconditionally from libgcc. Avoid + including <mm_malloc.h> here by defining _IMMINTRIN_H_INCLUDED. */ +#define _IMMINTRIN_H_INCLUDED +#include <cetintrin.h> +#undef _IMMINTRIN_H_INCLUDED /* Unwind the shadow stack for EH. */ #undef _Unwind_Frames_Extra I haven't managed to run it through ./glibc/glibc.sourceware/scripts/build-many-glibcs.py. I did bootstrap and CET tests. Ok for trunk? Igor > -- > Joseph S. Myers > joseph@codesourcery.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: PR84239, Reimplement CET intrinsics for rdssp/incssp insn 2018-02-15 16:36 ` Tsimbalist, Igor V @ 2018-02-15 21:20 ` Joseph Myers 0 siblings, 0 replies; 7+ messages in thread From: Joseph Myers @ 2018-02-15 21:20 UTC (permalink / raw) To: Tsimbalist, Igor V; +Cc: Sandra Loosemore, gcc-patches, Uros Bizjak On Thu, 15 Feb 2018, Tsimbalist, Igor V wrote: > I haven't managed to run it through > ./glibc/glibc.sourceware/scripts/build-many-glibcs.py. I did bootstrap > and CET tests. > > Ok for trunk? OK. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-02-15 21:20 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-02-09 12:51 PR84239, Reimplement CET intrinsics for rdssp/incssp insn Tsimbalist, Igor V 2018-02-09 18:42 ` Sandra Loosemore 2018-02-12 14:16 ` Tsimbalist, Igor V 2018-02-14 7:07 ` Jeff Law 2018-02-15 0:24 ` Joseph Myers 2018-02-15 16:36 ` Tsimbalist, Igor V 2018-02-15 21:20 ` Joseph Myers
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).