From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 119811 invoked by alias); 14 Feb 2018 07:07:34 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 119801 invoked by uid 89); 14 Feb 2018 07:07:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 14 Feb 2018 07:07:32 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 67328C04AC60; Wed, 14 Feb 2018 07:07:30 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-16.rdu2.redhat.com [10.10.112.16]) by smtp.corp.redhat.com (Postfix) with ESMTP id B591E6060D; Wed, 14 Feb 2018 07:07:28 +0000 (UTC) Subject: Re: PR84239, Reimplement CET intrinsics for rdssp/incssp insn To: "Tsimbalist, Igor V" , Sandra Loosemore , "gcc-patches@gcc.gnu.org" Cc: Uros Bizjak References: <8f880caf-b053-781a-f5e3-aa6df33ae61d@codesourcery.com> From: Jeff Law Message-ID: <0e84cb53-764e-9cd9-4cda-5b8fec3fe47c@redhat.com> Date: Wed, 14 Feb 2018 07:07:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2018-02/txt/msg00812.txt.bz2 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 ; gcc- >> patches@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) \ >>> + @{ \ >>> + _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 > 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