On Wed, Nov 8, 2017 at 2:57 PM, H.J. Lu <hjl.to...@gmail.com> wrote: > On Wed, Nov 8, 2017 at 2:26 PM, Tsimbalist, Igor V > <igor.v.tsimbal...@intel.com> wrote: >>> -----Original Message----- >>> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- >>> ow...@gcc.gnu.org] On Behalf Of Jeff Law >>> Sent: Wednesday, November 8, 2017 7:31 PM >>> To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>; gcc- >>> patc...@gcc.gnu.org >>> Cc: trie...@redhat.com; Jakub Jelinek <ja...@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.tsimbal...@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.
