RE: PR84239, Reimplement CET intrinsics for rdssp/incssp insn
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 jos...@codesourcery.com
RE: PR84239, Reimplement CET intrinsics for rdssp/incssp insn
Igor > -Original Message- > From: Joseph Myers [mailto:jos...@codesourcery.com] > Sent: Thursday, February 15, 2018 1:24 AM > To: Tsimbalist, Igor V > Cc: Sandra Loosemore ; gcc- > patc...@gcc.gnu.org; Uros Bizjak > 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 > ^~ > > https://sourceware.org/ml/libc-testresults/2018-q1/msg00307.html > > The patch makes shadow-stack-unwind.h include , which ends > up > including , which includes and > unconditionally. You can't include any libc system headers > unconditionally from libgcc (only when inhibit_libc is not defined - and > , 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 +/* NB: We need _get_ssp and _inc_ssp from . But we can't + include which ends up including , which + includes and unconditionally. But we can't + include any libc system headers unconditionally from libgcc. Avoid + including here by defining _IMMINTRIN_H_INCLUDED. */ +#define _IMMINTRIN_H_INCLUDED +#include +#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 > jos...@codesourcery.com
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 ^~ https://sourceware.org/ml/libc-testresults/2018-q1/msg00307.html The patch makes shadow-stack-unwind.h include , which ends up including , which includes and unconditionally. You can't include any libc system headers unconditionally from libgcc (only when inhibit_libc is not defined - and , 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 jos...@codesourcery.com
Re: PR84239, Reimplement CET intrinsics for rdssp/incssp insn
On 02/12/2018 07:16 AM, Tsimbalist, Igor V wrote:
>> -Original Message-
>> From: Sandra Loosemore [mailto:san...@codesourcery.com]
>> Sent: Friday, February 9, 2018 7:42 PM
>> To: Tsimbalist, Igor V ; gcc-
>> patc...@gcc.gnu.org
>> Cc: Uros Bizjak
>> 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
&g
RE: PR84239, Reimplement CET intrinsics for rdssp/incssp insn
> -Original Message-
> From: Sandra Loosemore [mailto:san...@codesourcery.com]
> Sent: Friday, February 9, 2018 7:42 PM
> To: Tsimbalist, Igor V ; gcc-
> patc...@gcc.gnu.org
> Cc: Uros Bizjak
> 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
> > +
> > +/* Unwind the shadow stack for EH. */
> > +#define _Unwind_Frames_Extra(x)\
> > + do \
> > +@{ \
> > + _Unwind_Word ssp = _get_ssp (); \
> > + if (ssp != 0)\
> > + @{
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
+
+/* 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
