* [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
@ 2017-11-07 16:42 Tsimbalist, Igor V
2017-11-08 18:30 ` H.J. Lu
2017-11-08 19:05 ` Jeff Law
0 siblings, 2 replies; 9+ messages in thread
From: Tsimbalist, Igor V @ 2017-11-07 16:42 UTC (permalink / raw)
To: Jeff Law, gcc-patches; +Cc: triegel, Tsimbalist, Igor V
[-- Attachment #1: Type: text/plain, Size: 2599 bytes --]
I decided to split my previous patch "Enable building libitm with Intel CET "
into two different patches. The first patch will add a new field to sjlj.S and
target.h files. The second one will add Intel CET support on the top of the
first one. In this case the further changes for adding Intel CET support are
seen clearly.
Ok for trunk?
Igor
> -----Original Message-----
> From: Tsimbalist, Igor V
> Sent: Tuesday, October 31, 2017 5:30 PM
> To: Jeff Law <law@redhat.com>; gcc-patches@gcc.gnu.org
> Cc: triegel@redhat.com; Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>
> Subject: RE: [PATCH 21/22] Enable building libitm with Intel CET
>
> > -----Original Message-----
> > From: Jeff Law [mailto:law@redhat.com]
> > Sent: Tuesday, October 31, 2017 12:21 AM
> > To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>; gcc-
> > patches@gcc.gnu.org
> > Cc: triegel@redhat.com
> > Subject: Re: [PATCH 21/22] Enable building libitm with Intel CET
> >
> > On 10/12/2017 03:21 PM, Tsimbalist, Igor V wrote:
> > > Enable building libitm with Intel CET options.
> > >
> > > libitm/
> > > * Makefile.in: Regenerate.
> > > * acinclude.m4: Add enable.m4 and cet.m4.
> > > * config/x86/sjlj.S
> > > (_ITM_beginTransaction): Save Shadow Stack pointer.
> > > (GTM_longjmp): Restore Shadow Stack pointer.
> > > * config/x86/target.h (struct gtm_jmpbuf):
> > > Add Shadow Stack pointer.
> > > * configure: Regenerate.
> > > * configure.ac: Set CET_FLAGS. Update XCFLAGS, libtool_VERSION.
> > > * testsuite/Makefile.in: Regenerate.
> > >
> > > * config/cet.m4: Define ENABLE_CET_COMPATIBILITY. Set
> > > enable_cet_compatibility.
> > >
> > Would it make sense to avoid having different sizes of gtm_jmpbuf by
> > simply having the ssp slot always defined, even if we're not using it?
>
> Yes, it make sense. We can do it for x86 configuration. In that case the
> conditional code will be limited to Shadow Stack pointer read/write.
>
> > Along the same lines, would it make sense to have that field at the end
> > of the structure so that the amount of conditional code in in sjlj.S is
> > minimized (ie, all the offests are the same, so in the CET case you just
> > have a single extra store).
>
> The comments says the buffer is specially located in such a way that eip/rip
> field overlap with a return address on the stack. That means the field can be
> added anywhere before eip/rip and in turn the offsets will change.
>
> I will re-implement the fixes with adding the new field for x86 only.
>
> Igor
>
> >
> > Jeff
[-- Attachment #2: 0021-Add-extra-field-to-gtm_jmpbuf-on-x86-only.patch --]
[-- Type: application/octet-stream, Size: 4429 bytes --]
From a6361c78bf774f2b4dbeeaf4147c286cff4ae5a4 Mon Sep 17 00:00:00 2001
From: Igor Tsimbalist <igor.v.tsimbalist@intel.com>
Date: Tue, 7 Nov 2017 17:00:24 +0300
Subject: [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
Expand the gtm_jmpbuf structure by one word field to add
Intel CET support further. The code in sjlj.S already
allocates more space on the stack then gtm_jmpbuf needs.
Use this extra space to absorb the new field.
The structure is allocated on the stack in such a way
that eip/rsp field is overlapped with return address on
the stack. Locate the new field right before eip/rsp so
code that accesses buffer fields relative to address of
gtm_jmpbuf has its offsets unchanged.
The libtool_VERSION is updated for x86 due to extending
the gtm_jmpbuf structure.
* libitm/config/x86/target.h: Add new field (ssp).
* libitm/config/x86/sjlj.S: Change offsets.
* libitm/configure.tgt: Update libtool_VERSION.
---
libitm/config/x86/sjlj.S | 44 ++++++++++++++++++++++++--------------------
libitm/config/x86/target.h | 2 ++
libitm/configure.tgt | 12 ++++++++++++
3 files changed, 38 insertions(+), 20 deletions(-)
diff --git a/libitm/config/x86/sjlj.S b/libitm/config/x86/sjlj.S
index 21ca9d7..1c8597a 100644
--- a/libitm/config/x86/sjlj.S
+++ b/libitm/config/x86/sjlj.S
@@ -126,20 +126,22 @@ SYM(_ITM_beginTransaction):
/* Store edi for future HTM fast path retries. We use a stack slot
lower than the jmpbuf so that the jmpbuf's rip field will overlap
with the proper return address on the stack. */
- movl %edi, 8(%rsp)
+ movl %edi, (%rsp)
/* Save the jmpbuf for any non-HTM-fastpath execution method.
Because rsp-based addressing is 1 byte larger and we've got rax
handy, use it. */
- movq %rax, -64(%rax)
- movq %rbx, -56(%rax)
- movq %rbp, -48(%rax)
- movq %r12, -40(%rax)
- movq %r13, -32(%rax)
- movq %r14, -24(%rax)
- movq %r15, -16(%rax)
- leaq -64(%rax), %rsi
+ movq %rax, -72(%rax)
+ movq %rbx, -64(%rax)
+ movq %rbp, -56(%rax)
+ movq %r12, -48(%rax)
+ movq %r13, -40(%rax)
+ movq %r14, -32(%rax)
+ movq %r15, -24(%rax)
+ xorq %rdx, %rdx
+ movq %rdx, -16(%rax)
+ leaq -72(%rax), %rsi
call SYM(GTM_begin_transaction)
- movl 8(%rsp), %edi
+ movl (%rsp), %edi
addq $72, %rsp
cfi_adjust_cfa_offset(-72)
#ifdef HAVE_AS_RTM
@@ -162,12 +164,14 @@ SYM(_ITM_beginTransaction):
movl 4(%esp), %eax
subl $28, %esp
cfi_def_cfa_offset(32)
- movl %ecx, 8(%esp)
- movl %ebx, 12(%esp)
- movl %esi, 16(%esp)
- movl %edi, 20(%esp)
- movl %ebp, 24(%esp)
- leal 8(%esp), %edx
+ movl %ecx, 4(%esp)
+ movl %ebx, 8(%esp)
+ movl %esi, 12(%esp)
+ movl %edi, 16(%esp)
+ movl %ebp, 20(%esp)
+ xorl %edx, %edx
+ movl %edx, 24(%eax)
+ leal 4(%esp), %edx
#if defined HAVE_ATTRIBUTE_VISIBILITY || !defined __PIC__
call SYM(GTM_begin_transaction)
#elif defined __ELF__
@@ -203,10 +207,10 @@ SYM(GTM_longjmp):
movq 48(%rsi), %r15
movl %edi, %eax
cfi_def_cfa(%rsi, 0)
- cfi_offset(%rip, 56)
+ cfi_offset(%rip, 64)
cfi_register(%rsp, %rcx)
movq %rcx, %rsp
- jmp *56(%rsi)
+ jmp *64(%rsi)
#else
movl (%edx), %ecx
movl 4(%edx), %ebx
@@ -214,10 +218,10 @@ SYM(GTM_longjmp):
movl 12(%edx), %edi
movl 16(%edx), %ebp
cfi_def_cfa(%edx, 0)
- cfi_offset(%eip, 20)
+ cfi_offset(%eip, 24)
cfi_register(%esp, %ecx)
movl %ecx, %esp
- jmp *20(%edx)
+ jmp *24(%edx)
#endif
cfi_endproc
diff --git a/libitm/config/x86/target.h b/libitm/config/x86/target.h
index 1b79dc0..5a4b820 100644
--- a/libitm/config/x86/target.h
+++ b/libitm/config/x86/target.h
@@ -39,12 +39,14 @@ typedef struct gtm_jmpbuf
unsigned long long r13;
unsigned long long r14;
unsigned long long r15;
+ unsigned long long ssp;
unsigned long long rip;
#else
unsigned long ebx;
unsigned long esi;
unsigned long edi;
unsigned long ebp;
+ unsigned long ssp;
unsigned long eip;
#endif
} gtm_jmpbuf;
diff --git a/libitm/configure.tgt b/libitm/configure.tgt
index 4ea71c8..49e4109 100644
--- a/libitm/configure.tgt
+++ b/libitm/configure.tgt
@@ -149,3 +149,15 @@ case "${target}" in
UNSUPPORTED=1
;;
esac
+
+# Update libtool_VERSION since the size of struct gtm_jmpbuf is
+# changed for x86.
+case "${host}" in
+
+ # For x86, we use slots in the TCB head for most of our TLS.
+ # The setup of those slots in beginTransaction can afford to
+ # use the global-dynamic model.
+ i[456]86-*-* | x86_64-*-*)
+ libtool_VERSION=2:0:0
+ ;;
+esac
--
1.8.3.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
2017-11-07 16:42 [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only Tsimbalist, Igor V
@ 2017-11-08 18:30 ` H.J. Lu
2017-11-08 21:29 ` Tsimbalist, Igor V
2017-11-08 19:05 ` Jeff Law
1 sibling, 1 reply; 9+ messages in thread
From: H.J. Lu @ 2017-11-08 18:30 UTC (permalink / raw)
To: Tsimbalist, Igor V; +Cc: Jeff Law, gcc-patches, triegel
On Tue, Nov 7, 2017 at 8:22 AM, Tsimbalist, Igor V
<igor.v.tsimbalist@intel.com> wrote:
> I decided to split my previous patch "Enable building libitm with Intel CET "
> into two different patches. The first patch will add a new field to sjlj.S and
> target.h files. The second one will add Intel CET support on the top of the
> first one. In this case the further changes for adding Intel CET support are
> seen clearly.
>
> Ok for trunk?
>
libitm/configure.tgt should check ${target} like the other places:
+# Update libtool_VERSION since the size of struct gtm_jmpbuf is
+# changed for x86.
+case "${host}" in
Did these come from cut and paste?
+ # For x86, we use slots in the TCB head for most of our TLS.
+ # The setup of those slots in beginTransaction can afford to
+ # use the global-dynamic model.
I think the whole thing should be:
case "${target}" in
# Update libtool_VERSION since the size of struct gtm_jmpbuf is
# changed for x86.
i[456]86-*-* | x86_64-*-*)
libtool_VERSION=2:0:0
;;
esac
--
H.J.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
2017-11-07 16:42 [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only Tsimbalist, Igor V
2017-11-08 18:30 ` H.J. Lu
@ 2017-11-08 19:05 ` Jeff Law
2017-11-08 22:31 ` Tsimbalist, Igor V
1 sibling, 1 reply; 9+ messages in thread
From: Jeff Law @ 2017-11-08 19:05 UTC (permalink / raw)
To: Tsimbalist, Igor V, gcc-patches; +Cc: triegel, Jakub Jelinek
On 11/07/2017 09:22 AM, Tsimbalist, Igor V wrote:
> I decided to split my previous patch "Enable building libitm with Intel CET "
> into two different patches. The first patch will add a new field to sjlj.S and
> target.h files. The second one will add Intel CET support on the top of the
> first one. In this case the further changes for adding Intel CET support are
> seen clearly.
>
> Ok for trunk?
>
[ ... snip ... ]
>
>
> 0021-Add-extra-field-to-gtm_jmpbuf-on-x86-only.patch
>
>
> From a6361c78bf774f2b4dbeeaf4147c286cff4ae5a4 Mon Sep 17 00:00:00 2001
> From: Igor Tsimbalist <igor.v.tsimbalist@intel.com>
> Date: Tue, 7 Nov 2017 17:00:24 +0300
> Subject: [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
>
> Expand the gtm_jmpbuf structure by one word field to add
> Intel CET support further. The code in sjlj.S already
> allocates more space on the stack then gtm_jmpbuf needs.
> Use this extra space to absorb the new field.
>
> The structure is allocated on the stack in such a way
> that eip/rsp field is overlapped with return address on
> the stack. Locate the new field right before eip/rsp so
> code that accesses buffer fields relative to address of
> gtm_jmpbuf has its offsets unchanged.
>
> The libtool_VERSION is updated for x86 due to extending
> the gtm_jmpbuf structure.
>
> * libitm/config/x86/target.h: Add new field (ssp).
> * libitm/config/x86/sjlj.S: Change offsets.
> * libitm/configure.tgt: Update libtool_VERSION.
So if I understand correctly, given the desire to to have the eip/rip
field overlap with the return address on the stack offset changes are
inevitable if we add fields.
> esac
> +
> +# Update libtool_VERSION since the size of struct gtm_jmpbuf is
> +# changed for x86.
> +case "${host}" in
> +
> + # For x86, we use slots in the TCB head for most of our TLS.
> + # The setup of those slots in beginTransaction can afford to
> + # use the global-dynamic model.
> + i[456]86-*-* | x86_64-*-*)
> + libtool_VERSION=2:0:0
What's the plan for supporting existing code that may have linked
dynamically against libitm?
One approach is to force the distros to carry the old libitm DSO.
THe other would be to try and support both within the same DSO using
symbol versioning. That would seem to imply that we'd need to the
before/after code to build that single library that supported both.
Thoughts? Jakub, any interest in chiming in here?
jeff
> + ;;
> +esac
> -- 1.8.3.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
2017-11-08 18:30 ` H.J. Lu
@ 2017-11-08 21:29 ` Tsimbalist, Igor V
0 siblings, 0 replies; 9+ messages in thread
From: Tsimbalist, Igor V @ 2017-11-08 21:29 UTC (permalink / raw)
To: H.J. Lu; +Cc: Jeff Law, gcc-patches, triegel, Tsimbalist, Igor V
[-- Attachment #1: Type: text/plain, Size: 1856 bytes --]
Igor
> -----Original Message-----
> From: H.J. Lu [mailto:hjl.tools@gmail.com]
> Sent: Wednesday, November 8, 2017 7:18 PM
> To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>
> Cc: Jeff Law <law@redhat.com>; gcc-patches@gcc.gnu.org;
> triegel@redhat.com
> Subject: Re: [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
>
> On Tue, Nov 7, 2017 at 8:22 AM, Tsimbalist, Igor V
> <igor.v.tsimbalist@intel.com> wrote:
> > I decided to split my previous patch "Enable building libitm with Intel CET "
> > into two different patches. The first patch will add a new field to sjlj.S and
> > target.h files. The second one will add Intel CET support on the top of the
> > first one. In this case the further changes for adding Intel CET support are
> > seen clearly.
> >
> > Ok for trunk?
> >
>
> libitm/configure.tgt should check ${target} like the other places:
>
> +# Update libtool_VERSION since the size of struct gtm_jmpbuf is
> +# changed for x86.
> +case "${host}" in
>
> Did these come from cut and paste?
>
> + # For x86, we use slots in the TCB head for most of our TLS.
> + # The setup of those slots in beginTransaction can afford to
> + # use the global-dynamic model.
>
> I think the whole thing should be:
>
> case "${target}" in
> # Update libtool_VERSION since the size of struct gtm_jmpbuf is
> # changed for x86.
> i[456]86-*-* | x86_64-*-*)
> libtool_VERSION=2:0:0
> ;;
> esac
There was a feedback from Joseph (email attached) with the comment about
similar case in cet.m4:
> This file is checking $target. That's only ever appropriate in directories
> building compilers and similar tools; target library directories should check
> $host, as the host for target libraries is the target for the compiler.
Igor
>
>
>
> --
> H.J.
[-- Attachment #2: Type: message/rfc822, Size: 18253 bytes --]
[-- Attachment #2.1.1: Type: text/plain, Size: 1535 bytes --]
> -----Original Message-----
> From: Joseph Myers [mailto:joseph@codesourcery.com]
> Sent: Thursday, October 12, 2017 10:36 PM
> To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>
> Cc: gcc-patches@gcc.gnu.org; Jeff Law <law@redhat.com>; ian@airs.com
> Subject: Re: [PATCH 07/22] Enable building libgcc with CET options.
>
> On Thu, 12 Oct 2017, Tsimbalist, Igor V wrote:
>
> > Enable building libgcc with CET options by default on Linux/x86 if
> > binutils supports CET v2.0.
> > It can be disabled with --disable-cet. It is an error to configure
> > GCC with --enable-cet if bintuiils doesn't support CET v2.0.
> >
> > config/
> > * cet.m4: New file
>
> This file is checking $target. That's only ever appropriate in directories
> building compilers and similar tools; target library directories should check
> $host, as the host for target libraries is the target for the compiler.
Fixed.
> This file has a comment
>
> > +dnl GCC_CET_LIBRARY
> > +dnl (SHELL-CODE_HANDLER)
>
> which doesn't seem to match the subsequent definition of GCC_CET_FLAGS.
Fixed.
> I don't see any documentation of the new configure option. I'd expect the
> first patch adding such an option to document it in install.texi, and then
> subsequent patches to update that documentation if those patches extend
> the option to cover more things.
Added the description of this configure option to install.texi.
The updated patch is attached.
Igor
> --
> Joseph S. Myers
> joseph@codesourcery.com
[-- Attachment #2.1.2: 0007-Enable-building-libgcc-with-CET-options.patch --]
[-- Type: application/octet-stream, Size: 10767 bytes --]
From d3c4f2673d9606c1e101c20ab42610b733c6ab4e Mon Sep 17 00:00:00 2001
From: Igor Tsimbalist <igor.v.tsimbalist@intel.com>
Date: Fri, 7 Jul 2017 18:33:15 +0300
Subject: [PATCH 07/22] Enable building libgcc with CET options.
Enable building libgcc with CET options by default on Linux/x86 if
binutils supports CET v2.0.
It can be disabled with --disable-cet. It is an error to configure
GCC with --enable-cet if bintuiils doesn't support CET v2.0.
config/
* cet.m4: New file
gcc/
* config.gcc (extra_headers): Add cet.h for Linux/x86 targets.
* config/i386/cet.h: New file.
* doc/install.texi: Add --enable-cet/--disable-cet.
libgcc/
* Makefile.in (configure_deps): Add $(srcdir)/../config/cet.m4.
(CET_FLAGS): New.
* config/i386/t-linux
(HOST_LIBGCC2_CFLAGS): Add $(CET_FLAGS).
(CRTSTUFF_T_CFLAGS): Add $(CET_FLAGS).
* configure.ac: Include ../config/cet.m4.
Set and substitute CET_FLAGS.
* configure: Regenerated.
---
config/cet.m4 | 38 ++++++++++++++++++++++++
gcc/config.gcc | 1 +
gcc/config/i386/cet.h | 74 ++++++++++++++++++++++++++++++++++++++++++++++
gcc/doc/install.texi | 13 ++++++++
libgcc/Makefile.in | 5 +++-
libgcc/config/i386/t-linux | 3 +-
libgcc/configure | 72 ++++++++++++++++++++++++++++++++++++++++++++
libgcc/configure.ac | 4 +++
8 files changed, 208 insertions(+), 2 deletions(-)
create mode 100644 config/cet.m4
create mode 100644 gcc/config/i386/cet.h
mode change 100644 => 100755 libgcc/configure
diff --git a/config/cet.m4 b/config/cet.m4
new file mode 100644
index 0000000..ca457ee
--- /dev/null
+++ b/config/cet.m4
@@ -0,0 +1,38 @@
+dnl
+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)
+case "$host" in
+ i[34567]86-*-linux* | x86_64-*-linux*)
+ case "$enable_cet" in
+ default)
+ # Check if assembler supports CET.
+ AC_COMPILE_IFELSE(
+ [AC_LANG_PROGRAM(
+ [],
+ [asm ("setssbsy");])],
+ [enable_cet=yes],
+ [enable_cet=no])
+ ;;
+ yes)
+ # Check if assembler supports CET.
+ AC_COMPILE_IFELSE(
+ [AC_LANG_PROGRAM(
+ [],
+ [asm ("setssbsy");])],
+ [],
+ [AC_MSG_ERROR([assembler with CET support is required for --enable-cet])])
+ ;;
+ esac
+ ;;
+ *)
+ enable_cet=no
+ ;;
+esac
+if test x$enable_cet = xyes; then
+ $1="-fcf-protection -mcet -include cet.h"
+fi
+])
diff --git a/gcc/config.gcc b/gcc/config.gcc
index cdda262..d2bee8e 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4548,6 +4548,7 @@ case ${target} in
;;
i[34567]86-*-linux* | x86_64-*-linux*)
extra_objs="${extra_objs} cet.o"
+ extra_headers="${extra_headers} cet.h"
tmake_file="$tmake_file i386/t-linux i386/t-cet"
;;
i[34567]86-*-kfreebsd*-gnu | x86_64-*-kfreebsd*-gnu)
diff --git a/gcc/config/i386/cet.h b/gcc/config/i386/cet.h
new file mode 100644
index 0000000..759f360
--- /dev/null
+++ b/gcc/config/i386/cet.h
@@ -0,0 +1,74 @@
+/* ELF program property for Intel CET.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+
+ This file is free software; you can redistribute it and/or modify it
+ under the terms of the GNU General Public License as published by the
+ Free Software Foundation; either version 3, or (at your option) any
+ later version.
+
+ This file is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ General Public License for more details.
+
+ Under Section 7 of GPL version 3, you are granted additional
+ permissions described in the GCC Runtime Library Exception, version
+ 3.1, as published by the Free Software Foundation.
+
+ You should have received a copy of the GNU General Public License and
+ 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/>.
+ */
+
+/* Add x86 feature with IBT and/or SHSTK bits to ELF program property
+ if they are enabled. Otherwise, contents in this header file are
+ unused. */
+
+#ifdef __ASSEMBLER__
+
+# ifdef __CET__
+# ifdef __IBT__
+/* GNU_PROPERTY_X86_FEATURE_1_IBT. */
+# define __PROPERTY_IBT 0x1
+# else
+# define __PROPERTY_IBT 0x0
+# endif
+
+# ifdef __SHSTK__
+/* GNU_PROPERTY_X86_FEATURE_1_SHSTK. */
+# define __PROPERTY_SHSTK 0x2
+# else
+# define __PROPERTY_SHSTK 0x0
+# endif
+
+# define __PROPERTY_BITS (__PROPERTY_IBT | __PROPERTY_SHSTK)
+
+# ifdef __LP64__
+# define __PROPERTY_ALIGN 3
+# else
+# define __PROPERTY_ALIGN 2
+# endif
+
+ .pushsection ".note.gnu.property", "a"
+ .p2align __PROPERTY_ALIGN
+ .long 1f - 0f /* name length. */
+ .long 4f - 1f /* data length. */
+ /* NT_GNU_PROPERTY_TYPE_0. */
+ .long 5 /* note type. */
+0:
+ .asciz "GNU" /* vendor name. */
+1:
+ .p2align __PROPERTY_ALIGN
+ /* GNU_PROPERTY_X86_FEATURE_1_AND. */
+ .long 0xc0000002 /* pr_type. */
+ .long 3f - 2f /* pr_datasz. */
+2:
+ /* GNU_PROPERTY_X86_FEATURE_1_XXX. */
+ .long __PROPERTY_BITS
+3:
+ .p2align __PROPERTY_ALIGN
+4:
+ .popsection
+# endif
+#endif
diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index da360da..6639eb2 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -2084,6 +2084,19 @@ explicitly specify the directory where they are installed. The
shorthand for
@option{--with-hsa-runtime-lib=@/@var{hsainstalldir}/lib} and
@option{--with-hsa-runtime-include=@/@var{hsainstalldir}/include}.
+
+@item --enable-cet
+@itemx --disable-cet
+Enable building target run-time libraries with control-flow
+instrumentation, see @option{-fcf-protection} option. When
+@code{--enable-cet} is specified target libraries are configured
+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.
@end table
@subheading Cross-Compiler-Specific Options
diff --git a/libgcc/Makefile.in b/libgcc/Makefile.in
index a1a392d..eaa68b5 100644
--- a/libgcc/Makefile.in
+++ b/libgcc/Makefile.in
@@ -171,7 +171,8 @@ configure_deps = \
$(srcdir)/../config/dfp.m4 \
$(srcdir)/../config/unwind_ipinfo.m4 \
$(srcdir)/../config/gthr.m4 \
- $(srcdir)/../config/sjlj.m4
+ $(srcdir)/../config/sjlj.m4 \
+ $(srcdir)/../config/cet.m4
$(srcdir)/configure: @MAINT@ $(srcdir)/configure.ac $(configure_deps)
cd $(srcdir) && $(AUTOCONF)
@@ -254,6 +255,8 @@ HOST_LIBGCC2_CFLAGS =
PICFLAG = @PICFLAG@
+CET_FLAGS = @CET_FLAGS@
+
# Defined in libgcc2.c, included only in the static library.
LIB2FUNCS_ST = _eprintf __gcc_bcmp
diff --git a/libgcc/config/i386/t-linux b/libgcc/config/i386/t-linux
index 11bb46e..8506a63 100644
--- a/libgcc/config/i386/t-linux
+++ b/libgcc/config/i386/t-linux
@@ -3,4 +3,5 @@
# t-slibgcc-elf-ver and t-linux
SHLIB_MAPFILES = libgcc-std.ver $(srcdir)/config/i386/libgcc-glibc.ver
-HOST_LIBGCC2_CFLAGS += -mlong-double-80 -DUSE_ELF_SYMVER
+HOST_LIBGCC2_CFLAGS += -mlong-double-80 -DUSE_ELF_SYMVER $(CET_FLAGS)
+CRTSTUFF_T_CFLAGS += $(CET_FLAGS)
diff --git a/libgcc/configure b/libgcc/configure
old mode 100644
new mode 100755
index 15d34b2..078d0d5
--- a/libgcc/configure
+++ b/libgcc/configure
@@ -573,6 +573,7 @@ vis_hide
real_host_noncanonical
accel_dir_suffix
force_explicit_eh_registry
+CET_FLAGS
fixed_point
enable_decimal_float
decimal_float
@@ -675,6 +676,7 @@ with_build_libsubdir
enable_largefile
enable_decimal_float
with_system_libunwind
+enable_cet
enable_explicit_exception_frame_registration
with_glibc_version
enable_tls
@@ -1314,6 +1316,8 @@ Optional Features:
enable decimal float extension to C. Selecting 'bid'
or 'dpd' choses which decimal floating point format
to use
+ --enable-cet enable Intel CET in target libraries
+ [default=default]
--enable-explicit-exception-frame-registration
register exception tables explicitly at module
start, for use e.g. for compatibility with
@@ -4773,6 +4777,74 @@ fi
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_sjlj_exceptions" >&5
$as_echo "$ac_cv_sjlj_exceptions" >&6; }
+ # Check whether --enable-cet was given.
+if test "${enable_cet+set}" = set; then :
+ enableval=$enable_cet;
+ case "$enableval" in
+ yes|no|default) ;;
+ *) as_fn_error "Unknown argument to enable/disable cet" "$LINENO" 5 ;;
+ esac
+
+else
+ enable_cet=default
+fi
+
+
+case "$target" in
+ i3456786-*-linux* | x86_64-*-linux*)
+ case "$enable_cet" in
+ default)
+ # Check if assembler supports CET.
+ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+int
+main ()
+{
+asm ("setssbsy");
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+ enable_cet=yes
+else
+ enable_cet=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+ ;;
+ yes)
+ # Check if assembler supports CET.
+ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+int
+main ()
+{
+asm ("setssbsy");
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+
+else
+ as_fn_error "assembler with CET support is required for --enable-cet" "$LINENO" 5
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+ ;;
+ esac
+ ;;
+ *)
+ enable_cet=no
+ ;;
+esac
+if test x$enable_cet = xyes; then
+ CET_FLAGS="-fcf-protection -mcet -include cet.h"
+fi
+
+
+
# Check whether --enable-explicit-exception-frame-registration was given.
if test "${enable_explicit_exception_frame_registration+set}" = set; then :
enableval=$enable_explicit_exception_frame_registration;
diff --git a/libgcc/configure.ac b/libgcc/configure.ac
index da49971..f23661a 100644
--- a/libgcc/configure.ac
+++ b/libgcc/configure.ac
@@ -11,6 +11,7 @@ sinclude(../config/dfp.m4)
sinclude(../config/unwind_ipinfo.m4)
sinclude(../config/gthr.m4)
sinclude(../config/sjlj.m4)
+sinclude(../config/cet.m4)
AC_PREREQ(2.64)
AC_INIT([GNU C Runtime Library], 1.0,,[libgcc])
@@ -236,6 +237,9 @@ GCC_CHECK_UNWIND_GETIPINFO
# Check if the compiler is configured for setjmp/longjmp exceptions.
GCC_CHECK_SJLJ_EXCEPTIONS
+GCC_CET_FLAGS(CET_FLAGS)
+AC_SUBST(CET_FLAGS)
+
AC_ARG_ENABLE([explicit-exception-frame-registration],
[AC_HELP_STRING([--enable-explicit-exception-frame-registration],
[register exception tables explicitly at module start, for use
--
1.8.3.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
2017-11-08 19:05 ` Jeff Law
@ 2017-11-08 22:31 ` Tsimbalist, Igor V
2017-11-08 23:04 ` H.J. Lu
0 siblings, 1 reply; 9+ messages in thread
From: Tsimbalist, Igor V @ 2017-11-08 22:31 UTC (permalink / raw)
To: Jeff Law, gcc-patches; +Cc: triegel, Jakub Jelinek, Tsimbalist, Igor V
> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Jeff Law
> Sent: Wednesday, November 8, 2017 7:31 PM
> To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>; gcc-
> patches@gcc.gnu.org
> Cc: triegel@redhat.com; Jakub Jelinek <jakub@redhat.com>
> Subject: Re: [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
>
> On 11/07/2017 09:22 AM, Tsimbalist, Igor V wrote:
> > I decided to split my previous patch "Enable building libitm with Intel CET "
> > into two different patches. The first patch will add a new field to sjlj.S and
> > target.h files. The second one will add Intel CET support on the top of the
> > first one. In this case the further changes for adding Intel CET support are
> > seen clearly.
> >
> > Ok for trunk?
> >
>
> [ ... snip ... ]
>
> >
> >
> > 0021-Add-extra-field-to-gtm_jmpbuf-on-x86-only.patch
> >
> >
> > From a6361c78bf774f2b4dbeeaf4147c286cff4ae5a4 Mon Sep 17 00:00:00
> 2001
> > From: Igor Tsimbalist <igor.v.tsimbalist@intel.com>
> > Date: Tue, 7 Nov 2017 17:00:24 +0300
> > Subject: [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
> >
> > Expand the gtm_jmpbuf structure by one word field to add
> > Intel CET support further. The code in sjlj.S already
> > allocates more space on the stack then gtm_jmpbuf needs.
> > Use this extra space to absorb the new field.
> >
> > The structure is allocated on the stack in such a way
> > that eip/rsp field is overlapped with return address on
> > the stack. Locate the new field right before eip/rsp so
> > code that accesses buffer fields relative to address of
> > gtm_jmpbuf has its offsets unchanged.
> >
> > The libtool_VERSION is updated for x86 due to extending
> > the gtm_jmpbuf structure.
> >
> > * libitm/config/x86/target.h: Add new field (ssp).
> > * libitm/config/x86/sjlj.S: Change offsets.
> > * libitm/configure.tgt: Update libtool_VERSION.
> So if I understand correctly, given the desire to to have the eip/rip
> field overlap with the return address on the stack offset changes are
> inevitable if we add fields.
Yes, that's exactly the case.
> > esac
> > +
> > +# Update libtool_VERSION since the size of struct gtm_jmpbuf is
> > +# changed for x86.
> > +case "${host}" in
> > +
> > + # For x86, we use slots in the TCB head for most of our TLS.
> > + # The setup of those slots in beginTransaction can afford to
> > + # use the global-dynamic model.
> > + i[456]86-*-* | x86_64-*-*)
> > + libtool_VERSION=2:0:0
> What's the plan for supporting existing code that may have linked
> dynamically against libitm?
This should just work.
> One approach is to force the distros to carry the old libitm DSO.
>
> THe other would be to try and support both within the same DSO using
> symbol versioning. That would seem to imply that we'd need to the
> before/after code to build that single library that supported both.
>
> Thoughts? Jakub, any interest in chiming in here?
My thought is that the buffer is encapsulated in the library, only sjlj.S
functions allocate the buffer and access the fields of the buffer, it's
sort of a black box. If an app loads the library it will work with the
buffer through the library's functions from sjlj.S , which are compiled
together.
Igor
> jeff
> > + ;;
> > +esac
> > -- 1.8.3.1
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
2017-11-08 22:31 ` Tsimbalist, Igor V
@ 2017-11-08 23:04 ` H.J. Lu
2017-11-09 13:49 ` H.J. Lu
0 siblings, 1 reply; 9+ messages in thread
From: H.J. Lu @ 2017-11-08 23:04 UTC (permalink / raw)
To: Tsimbalist, Igor V; +Cc: Jeff Law, gcc-patches, triegel, Jakub Jelinek
On Wed, Nov 8, 2017 at 2:26 PM, Tsimbalist, Igor V
<igor.v.tsimbalist@intel.com> wrote:
>> -----Original Message-----
>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
>> owner@gcc.gnu.org] On Behalf Of Jeff Law
>> Sent: Wednesday, November 8, 2017 7:31 PM
>> To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>; gcc-
>> patches@gcc.gnu.org
>> Cc: triegel@redhat.com; Jakub Jelinek <jakub@redhat.com>
>> Subject: Re: [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
>>
>> On 11/07/2017 09:22 AM, Tsimbalist, Igor V wrote:
>> > I decided to split my previous patch "Enable building libitm with Intel CET "
>> > into two different patches. The first patch will add a new field to sjlj.S and
>> > target.h files. The second one will add Intel CET support on the top of the
>> > first one. In this case the further changes for adding Intel CET support are
>> > seen clearly.
>> >
>> > Ok for trunk?
>> >
>>
>> [ ... snip ... ]
>>
>> >
>> >
>> > 0021-Add-extra-field-to-gtm_jmpbuf-on-x86-only.patch
>> >
>> >
>> > From a6361c78bf774f2b4dbeeaf4147c286cff4ae5a4 Mon Sep 17 00:00:00
>> 2001
>> > From: Igor Tsimbalist <igor.v.tsimbalist@intel.com>
>> > Date: Tue, 7 Nov 2017 17:00:24 +0300
>> > Subject: [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
>> >
>> > Expand the gtm_jmpbuf structure by one word field to add
>> > Intel CET support further. The code in sjlj.S already
>> > allocates more space on the stack then gtm_jmpbuf needs.
>> > Use this extra space to absorb the new field.
>> >
>> > The structure is allocated on the stack in such a way
>> > that eip/rsp field is overlapped with return address on
>> > the stack. Locate the new field right before eip/rsp so
>> > code that accesses buffer fields relative to address of
>> > gtm_jmpbuf has its offsets unchanged.
>> >
>> > The libtool_VERSION is updated for x86 due to extending
>> > the gtm_jmpbuf structure.
>> >
>> > * libitm/config/x86/target.h: Add new field (ssp).
>> > * libitm/config/x86/sjlj.S: Change offsets.
>> > * libitm/configure.tgt: Update libtool_VERSION.
>> So if I understand correctly, given the desire to to have the eip/rip
>> field overlap with the return address on the stack offset changes are
>> inevitable if we add fields.
>
> Yes, that's exactly the case.
>
>> > esac
>> > +
>> > +# Update libtool_VERSION since the size of struct gtm_jmpbuf is
>> > +# changed for x86.
>> > +case "${host}" in
>> > +
>> > + # For x86, we use slots in the TCB head for most of our TLS.
>> > + # The setup of those slots in beginTransaction can afford to
>> > + # use the global-dynamic model.
>> > + i[456]86-*-* | x86_64-*-*)
>> > + libtool_VERSION=2:0:0
>> What's the plan for supporting existing code that may have linked
>> dynamically against libitm?
>
> This should just work.
>
>> One approach is to force the distros to carry the old libitm DSO.
>>
>> THe other would be to try and support both within the same DSO using
>> symbol versioning. That would seem to imply that we'd need to the
>> before/after code to build that single library that supported both.
>>
>> Thoughts? Jakub, any interest in chiming in here?
>
> My thought is that the buffer is encapsulated in the library, only sjlj.S
> functions allocate the buffer and access the fields of the buffer, it's
> sort of a black box. If an app loads the library it will work with the
> buffer through the library's functions from sjlj.S , which are compiled
> together.
It isn't the black box since gtm_jmpbuf is used in:
struct gtm_transaction_cp
{
gtm_jmpbuf jb;
size_t undolog_size;
If we don't want to change libtool_VERSION, we need to add symbol
versioning to libitm.so. But from what I can see, libitm.so wasn't designed
with symbol versioning. Instead, it changes libtool_VERSION when
ABI is changed.
--
H.J.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
2017-11-08 23:04 ` H.J. Lu
@ 2017-11-09 13:49 ` H.J. Lu
2017-11-13 14:03 ` Tsimbalist, Igor V
0 siblings, 1 reply; 9+ messages in thread
From: H.J. Lu @ 2017-11-09 13:49 UTC (permalink / raw)
To: Tsimbalist, Igor V; +Cc: Jeff Law, gcc-patches, triegel, Jakub Jelinek
On Wed, Nov 8, 2017 at 2:57 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Nov 8, 2017 at 2:26 PM, Tsimbalist, Igor V
> <igor.v.tsimbalist@intel.com> wrote:
>>> -----Original Message-----
>>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
>>> owner@gcc.gnu.org] On Behalf Of Jeff Law
>>> Sent: Wednesday, November 8, 2017 7:31 PM
>>> To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>; gcc-
>>> patches@gcc.gnu.org
>>> Cc: triegel@redhat.com; Jakub Jelinek <jakub@redhat.com>
>>> Subject: Re: [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
>>>
>>> On 11/07/2017 09:22 AM, Tsimbalist, Igor V wrote:
>>> > I decided to split my previous patch "Enable building libitm with Intel CET "
>>> > into two different patches. The first patch will add a new field to sjlj.S and
>>> > target.h files. The second one will add Intel CET support on the top of the
>>> > first one. In this case the further changes for adding Intel CET support are
>>> > seen clearly.
>>> >
>>> > Ok for trunk?
>>> >
>>>
>>> [ ... snip ... ]
>>>
>>> >
>>> >
>>> > 0021-Add-extra-field-to-gtm_jmpbuf-on-x86-only.patch
>>> >
>>> >
>>> > From a6361c78bf774f2b4dbeeaf4147c286cff4ae5a4 Mon Sep 17 00:00:00
>>> 2001
>>> > From: Igor Tsimbalist <igor.v.tsimbalist@intel.com>
>>> > Date: Tue, 7 Nov 2017 17:00:24 +0300
>>> > Subject: [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
>>> >
>>> > Expand the gtm_jmpbuf structure by one word field to add
>>> > Intel CET support further. The code in sjlj.S already
>>> > allocates more space on the stack then gtm_jmpbuf needs.
>>> > Use this extra space to absorb the new field.
>>> >
>>> > The structure is allocated on the stack in such a way
>>> > that eip/rsp field is overlapped with return address on
>>> > the stack. Locate the new field right before eip/rsp so
>>> > code that accesses buffer fields relative to address of
>>> > gtm_jmpbuf has its offsets unchanged.
>>> >
>>> > The libtool_VERSION is updated for x86 due to extending
>>> > the gtm_jmpbuf structure.
>>> >
>>> > * libitm/config/x86/target.h: Add new field (ssp).
>>> > * libitm/config/x86/sjlj.S: Change offsets.
>>> > * libitm/configure.tgt: Update libtool_VERSION.
>>> So if I understand correctly, given the desire to to have the eip/rip
>>> field overlap with the return address on the stack offset changes are
>>> inevitable if we add fields.
>>
>> Yes, that's exactly the case.
>>
>>> > esac
>>> > +
>>> > +# Update libtool_VERSION since the size of struct gtm_jmpbuf is
>>> > +# changed for x86.
>>> > +case "${host}" in
>>> > +
>>> > + # For x86, we use slots in the TCB head for most of our TLS.
>>> > + # The setup of those slots in beginTransaction can afford to
>>> > + # use the global-dynamic model.
>>> > + i[456]86-*-* | x86_64-*-*)
>>> > + libtool_VERSION=2:0:0
>>> What's the plan for supporting existing code that may have linked
>>> dynamically against libitm?
>>
>> This should just work.
>>
>>> One approach is to force the distros to carry the old libitm DSO.
>>>
>>> THe other would be to try and support both within the same DSO using
>>> symbol versioning. That would seem to imply that we'd need to the
>>> before/after code to build that single library that supported both.
>>>
>>> Thoughts? Jakub, any interest in chiming in here?
>>
>> My thought is that the buffer is encapsulated in the library, only sjlj.S
>> functions allocate the buffer and access the fields of the buffer, it's
>> sort of a black box. If an app loads the library it will work with the
>> buffer through the library's functions from sjlj.S , which are compiled
>> together.
>
> It isn't the black box since gtm_jmpbuf is used in:
>
> struct gtm_transaction_cp
> {
> gtm_jmpbuf jb;
> size_t undolog_size;
>
> If we don't want to change libtool_VERSION, we need to add symbol
> versioning to libitm.so. But from what I can see, libitm.so wasn't designed
> with symbol versioning. Instead, it changes libtool_VERSION when
> ABI is changed.
>
I was wrong on 2 counts:
1. libitm does have symbol versioning.
2. libitm does look like a black box since there is no installed header
and internal is opaque.
Igor, please do
1. Build libitm with the old gtm_jmpbuf.
2. Build libitm with the new gtm_jmpbuf and the same libtool_VERSION.
3. Replace the the old libitm with the new libitm.
4. Run libitm tetsts in the old libitm build tree.
If there are no regressions, we don't need to change libtool_VERSION.
--
H.J.
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
2017-11-09 13:49 ` H.J. Lu
@ 2017-11-13 14:03 ` Tsimbalist, Igor V
2017-11-17 0:18 ` Jeff Law
0 siblings, 1 reply; 9+ messages in thread
From: Tsimbalist, Igor V @ 2017-11-13 14:03 UTC (permalink / raw)
To: 'H.J. Lu', Jeff Law
Cc: gcc-patches, triegel, Jakub Jelinek, Tsimbalist, Igor V
> -----Original Message-----
> From: H.J. Lu [mailto:hjl.tools@gmail.com]
> Sent: Thursday, November 9, 2017 2:37 PM
> To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>
> Cc: Jeff Law <law@redhat.com>; gcc-patches@gcc.gnu.org;
> triegel@redhat.com; Jakub Jelinek <jakub@redhat.com>
> Subject: Re: [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
>
> On Wed, Nov 8, 2017 at 2:57 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> > On Wed, Nov 8, 2017 at 2:26 PM, Tsimbalist, Igor V
> > <igor.v.tsimbalist@intel.com> wrote:
> >>> -----Original Message-----
> >>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> >>> owner@gcc.gnu.org] On Behalf Of Jeff Law
> >>> Sent: Wednesday, November 8, 2017 7:31 PM
> >>> To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>; gcc-
> >>> patches@gcc.gnu.org
> >>> Cc: triegel@redhat.com; Jakub Jelinek <jakub@redhat.com>
> >>> Subject: Re: [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
> >>>
> >>> On 11/07/2017 09:22 AM, Tsimbalist, Igor V wrote:
> >>> > I decided to split my previous patch "Enable building libitm with Intel
> CET "
> >>> > into two different patches. The first patch will add a new field to sjlj.S
> and
> >>> > target.h files. The second one will add Intel CET support on the top of
> the
> >>> > first one. In this case the further changes for adding Intel CET support
> are
> >>> > seen clearly.
> >>> >
> >>> > Ok for trunk?
> >>> >
> >>>
> >>> [ ... snip ... ]
> >>>
> >>> >
> >>> >
> >>> > 0021-Add-extra-field-to-gtm_jmpbuf-on-x86-only.patch
> >>> >
> >>> >
> >>> > From a6361c78bf774f2b4dbeeaf4147c286cff4ae5a4 Mon Sep 17
> 00:00:00
> >>> 2001
> >>> > From: Igor Tsimbalist <igor.v.tsimbalist@intel.com>
> >>> > Date: Tue, 7 Nov 2017 17:00:24 +0300
> >>> > Subject: [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
> >>> >
> >>> > Expand the gtm_jmpbuf structure by one word field to add
> >>> > Intel CET support further. The code in sjlj.S already
> >>> > allocates more space on the stack then gtm_jmpbuf needs.
> >>> > Use this extra space to absorb the new field.
> >>> >
> >>> > The structure is allocated on the stack in such a way
> >>> > that eip/rsp field is overlapped with return address on
> >>> > the stack. Locate the new field right before eip/rsp so
> >>> > code that accesses buffer fields relative to address of
> >>> > gtm_jmpbuf has its offsets unchanged.
> >>> >
> >>> > The libtool_VERSION is updated for x86 due to extending
> >>> > the gtm_jmpbuf structure.
> >>> >
> >>> > * libitm/config/x86/target.h: Add new field (ssp).
> >>> > * libitm/config/x86/sjlj.S: Change offsets.
> >>> > * libitm/configure.tgt: Update libtool_VERSION.
> >>> So if I understand correctly, given the desire to to have the eip/rip
> >>> field overlap with the return address on the stack offset changes are
> >>> inevitable if we add fields.
> >>
> >> Yes, that's exactly the case.
> >>
> >>> > esac
> >>> > +
> >>> > +# Update libtool_VERSION since the size of struct gtm_jmpbuf is
> >>> > +# changed for x86.
> >>> > +case "${host}" in
> >>> > +
> >>> > + # For x86, we use slots in the TCB head for most of our TLS.
> >>> > + # The setup of those slots in beginTransaction can afford to
> >>> > + # use the global-dynamic model.
> >>> > + i[456]86-*-* | x86_64-*-*)
> >>> > + libtool_VERSION=2:0:0
> >>> What's the plan for supporting existing code that may have linked
> >>> dynamically against libitm?
> >>
> >> This should just work.
> >>
> >>> One approach is to force the distros to carry the old libitm DSO.
> >>>
> >>> THe other would be to try and support both within the same DSO using
> >>> symbol versioning. That would seem to imply that we'd need to the
> >>> before/after code to build that single library that supported both.
> >>>
> >>> Thoughts? Jakub, any interest in chiming in here?
> >>
> >> My thought is that the buffer is encapsulated in the library, only sjlj.S
> >> functions allocate the buffer and access the fields of the buffer, it's
> >> sort of a black box. If an app loads the library it will work with the
> >> buffer through the library's functions from sjlj.S , which are compiled
> >> together.
> >
> > It isn't the black box since gtm_jmpbuf is used in:
> >
> > struct gtm_transaction_cp
> > {
> > gtm_jmpbuf jb;
> > size_t undolog_size;
> >
> > If we don't want to change libtool_VERSION, we need to add symbol
> > versioning to libitm.so. But from what I can see, libitm.so wasn't designed
> > with symbol versioning. Instead, it changes libtool_VERSION when
> > ABI is changed.
> >
>
> I was wrong on 2 counts:
>
> 1. libitm does have symbol versioning.
> 2. libitm does look like a black box since there is no installed header
> and internal is opaque.
>
> Igor, please do
>
> 1. Build libitm with the old gtm_jmpbuf.
> 2. Build libitm with the new gtm_jmpbuf and the same libtool_VERSION.
> 3. Replace the the old libitm with the new libitm.
> 4. Run libitm tetsts in the old libitm build tree.
I did
1. On the top of my workspace with all the fixes I discarded the changes
libitm/configure.tgt: Update libtool_VERSION.
2. Built the libitm. The library has the suffix 1.0.0.
3. Checked out the sources right before my fixes in libitm. Libitm has an old
gtm_jmpbuf (without new field).
4. Built the libitm and run the tests.
5. Replaced the library with the library from the step 2 and run tests
All tests passed in both runs.
> If there are no regressions, we don't need to change libtool_VERSION.
I will revert the changes regarding libtool_VERSION in libitm/configure.tgt.
Igor
> --
> H.J.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
2017-11-13 14:03 ` Tsimbalist, Igor V
@ 2017-11-17 0:18 ` Jeff Law
0 siblings, 0 replies; 9+ messages in thread
From: Jeff Law @ 2017-11-17 0:18 UTC (permalink / raw)
To: Tsimbalist, Igor V, 'H.J. Lu'; +Cc: gcc-patches, triegel, Jakub Jelinek
On 11/13/2017 06:53 AM, Tsimbalist, Igor V wrote:
>> -----Original Message-----
>> From: H.J. Lu [mailto:hjl.tools@gmail.com]
>> Sent: Thursday, November 9, 2017 2:37 PM
>> To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>
>> Cc: Jeff Law <law@redhat.com>; gcc-patches@gcc.gnu.org;
>> triegel@redhat.com; Jakub Jelinek <jakub@redhat.com>
>> Subject: Re: [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
>>
>> On Wed, Nov 8, 2017 at 2:57 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Wed, Nov 8, 2017 at 2:26 PM, Tsimbalist, Igor V
>>> <igor.v.tsimbalist@intel.com> wrote:
>>>>> -----Original Message-----
>>>>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
>>>>> owner@gcc.gnu.org] On Behalf Of Jeff Law
>>>>> Sent: Wednesday, November 8, 2017 7:31 PM
>>>>> To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>; gcc-
>>>>> patches@gcc.gnu.org
>>>>> Cc: triegel@redhat.com; Jakub Jelinek <jakub@redhat.com>
>>>>> Subject: Re: [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
>>>>>
>>>>> On 11/07/2017 09:22 AM, Tsimbalist, Igor V wrote:
>>>>>> I decided to split my previous patch "Enable building libitm with Intel
>> CET "
>>>>>> into two different patches. The first patch will add a new field to sjlj.S
>> and
>>>>>> target.h files. The second one will add Intel CET support on the top of
>> the
>>>>>> first one. In this case the further changes for adding Intel CET support
>> are
>>>>>> seen clearly.
>>>>>>
>>>>>> Ok for trunk?
>>>>>>
>>>>>
>>>>> [ ... snip ... ]
>>>>>
>>>>>>
>>>>>>
>>>>>> 0021-Add-extra-field-to-gtm_jmpbuf-on-x86-only.patch
>>>>>>
>>>>>>
>>>>>> From a6361c78bf774f2b4dbeeaf4147c286cff4ae5a4 Mon Sep 17
>> 00:00:00
>>>>> 2001
>>>>>> From: Igor Tsimbalist <igor.v.tsimbalist@intel.com>
>>>>>> Date: Tue, 7 Nov 2017 17:00:24 +0300
>>>>>> Subject: [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only
>>>>>>
>>>>>> Expand the gtm_jmpbuf structure by one word field to add
>>>>>> Intel CET support further. The code in sjlj.S already
>>>>>> allocates more space on the stack then gtm_jmpbuf needs.
>>>>>> Use this extra space to absorb the new field.
>>>>>>
>>>>>> The structure is allocated on the stack in such a way
>>>>>> that eip/rsp field is overlapped with return address on
>>>>>> the stack. Locate the new field right before eip/rsp so
>>>>>> code that accesses buffer fields relative to address of
>>>>>> gtm_jmpbuf has its offsets unchanged.
>>>>>>
>>>>>> The libtool_VERSION is updated for x86 due to extending
>>>>>> the gtm_jmpbuf structure.
>>>>>>
>>>>>> * libitm/config/x86/target.h: Add new field (ssp).
>>>>>> * libitm/config/x86/sjlj.S: Change offsets.
>>>>>> * libitm/configure.tgt: Update libtool_VERSION.
>>>>> So if I understand correctly, given the desire to to have the eip/rip
>>>>> field overlap with the return address on the stack offset changes are
>>>>> inevitable if we add fields.
>>>>
>>>> Yes, that's exactly the case.
>>>>
>>>>>> esac
>>>>>> +
>>>>>> +# Update libtool_VERSION since the size of struct gtm_jmpbuf is
>>>>>> +# changed for x86.
>>>>>> +case "${host}" in
>>>>>> +
>>>>>> + # For x86, we use slots in the TCB head for most of our TLS.
>>>>>> + # The setup of those slots in beginTransaction can afford to
>>>>>> + # use the global-dynamic model.
>>>>>> + i[456]86-*-* | x86_64-*-*)
>>>>>> + libtool_VERSION=2:0:0
>>>>> What's the plan for supporting existing code that may have linked
>>>>> dynamically against libitm?
>>>>
>>>> This should just work.
>>>>
>>>>> One approach is to force the distros to carry the old libitm DSO.
>>>>>
>>>>> THe other would be to try and support both within the same DSO using
>>>>> symbol versioning. That would seem to imply that we'd need to the
>>>>> before/after code to build that single library that supported both.
>>>>>
>>>>> Thoughts? Jakub, any interest in chiming in here?
>>>>
>>>> My thought is that the buffer is encapsulated in the library, only sjlj.S
>>>> functions allocate the buffer and access the fields of the buffer, it's
>>>> sort of a black box. If an app loads the library it will work with the
>>>> buffer through the library's functions from sjlj.S , which are compiled
>>>> together.
>>>
>>> It isn't the black box since gtm_jmpbuf is used in:
>>>
>>> struct gtm_transaction_cp
>>> {
>>> gtm_jmpbuf jb;
>>> size_t undolog_size;
>>>
>>> If we don't want to change libtool_VERSION, we need to add symbol
>>> versioning to libitm.so. But from what I can see, libitm.so wasn't designed
>>> with symbol versioning. Instead, it changes libtool_VERSION when
>>> ABI is changed.
>>>
>>
>> I was wrong on 2 counts:
>>
>> 1. libitm does have symbol versioning.
>> 2. libitm does look like a black box since there is no installed header
>> and internal is opaque.
>>
>> Igor, please do
>>
>> 1. Build libitm with the old gtm_jmpbuf.
>> 2. Build libitm with the new gtm_jmpbuf and the same libtool_VERSION.
>> 3. Replace the the old libitm with the new libitm.
>> 4. Run libitm tetsts in the old libitm build tree.
>
> I did
>
> 1. On the top of my workspace with all the fixes I discarded the changes
> libitm/configure.tgt: Update libtool_VERSION.
> 2. Built the libitm. The library has the suffix 1.0.0.
> 3. Checked out the sources right before my fixes in libitm. Libitm has an old
> gtm_jmpbuf (without new field).
> 4. Built the libitm and run the tests.
> 5. Replaced the library with the library from the step 2 and run tests
>
> All tests passed in both runs.
FWIW, I wandered through libitm a bit and I think gtm_jmpbuf is supposed
to be an internal implementation detail. I don't see references to it
or any structure that contains a gtm_jmpbuf in libitm.h. So I don't see
any direct way for it to escape.
I think with that analysis as well as your testing we can just drop the
chunk that changes the version and the rest is OK.
Jeff
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-11-16 23:51 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-07 16:42 [PATCH 21/22] Add extra field to gtm_jmpbuf on x86 only Tsimbalist, Igor V
2017-11-08 18:30 ` H.J. Lu
2017-11-08 21:29 ` Tsimbalist, Igor V
2017-11-08 19:05 ` Jeff Law
2017-11-08 22:31 ` Tsimbalist, Igor V
2017-11-08 23:04 ` H.J. Lu
2017-11-09 13:49 ` H.J. Lu
2017-11-13 14:03 ` Tsimbalist, Igor V
2017-11-17 0:18 ` Jeff Law
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).