> -----Original Message----- > From: Sandra Loosemore [mailto:san...@codesourcery.com] > Sent: Friday, February 9, 2018 7:42 PM > To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>; gcc- > patc...@gcc.gnu.org > Cc: Uros Bizjak <ubiz...@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
Description: 0001-Reimplement-CET-intrinsics-for-rdssp-incssp-insn.patch
