[00/12] S390: Implement __fentry__

Message ID 20180802075735.3457-1-iii@linux.ibm.com
Headers show
Series
  • S390: Implement __fentry__
Related show

Message

Ilya Leoshkevich Aug. 2, 2018, 7:57 a.m.
This patch series adds the runtime support in glibc for the -mfentry
gcc feature introduced in [1] and [2].

Patches 1-9 deal with avoiding clobbering %r0 when calling lazily bound
functions, so that the new __fentry__ symbol could be called with return
address in that register.

Patch 10 removes the assumption that only Intel implements __fentry__.

Patch 11 adds __fentry__ implementation.

Patch 12 is a cleanup of problems in 31-bit mcount, which are similar to
those found while implementing 64-bit __fentry__.

[1] https://gcc.gnu.org/ml/gcc-patches/2018-07/msg00784.html
[2] https://gcc.gnu.org/ml/gcc-patches/2018-07/msg00912.html

Comments

Florian Weimer Aug. 2, 2018, 9:17 a.m. | #1
On 08/02/2018 09:57 AM, Ilya Leoshkevich wrote:
> This patch series adds the runtime support in glibc for the -mfentry

> gcc feature introduced in [1] and [2].

> 

> Patches 1-9 deal with avoiding clobbering %r0 when calling lazily bound

> functions, so that the new __fentry__ symbol could be called with return

> address in that register.


Should GCC arrange for suitable ABI markup if -mfentry is used?  I'm 
worried that unexpected clobbers of r0 could be quite difficult to 
figure out otherwise.

Thanks,
Florian
Ilya Leoshkevich Aug. 2, 2018, 10:43 a.m. | #2
> Am 02.08.2018 um 11:17 schrieb Florian Weimer <fweimer@redhat.com>:

> 

> On 08/02/2018 09:57 AM, Ilya Leoshkevich wrote:

>> This patch series adds the runtime support in glibc for the -mfentry

>> gcc feature introduced in [1] and [2].

>> Patches 1-9 deal with avoiding clobbering %r0 when calling lazily bound

>> functions, so that the new __fentry__ symbol could be called with return

>> address in that register.

> 

> Should GCC arrange for suitable ABI markup if -mfentry is used?  I'm worried that unexpected clobbers of r0 could be quite difficult to figure out otherwise.


I don’t think we need to do anything ABI-related at this point.

__fentry__ itself is being newly introduced, so there is no old ABI to
comply with. Instrumented functions’ ABI does not change, particularly,
%r0, being a volatile register, used to be clobbered anyway.

Are there other cases that I’m missing?

> Thanks,

> Florian

>
Florian Weimer Aug. 2, 2018, 6:32 p.m. | #3
On 08/02/2018 12:43 PM, Ilya Leoshkevich wrote:
> 

>> Am 02.08.2018 um 11:17 schrieb Florian Weimer <fweimer@redhat.com>:

>>

>> On 08/02/2018 09:57 AM, Ilya Leoshkevich wrote:

>>> This patch series adds the runtime support in glibc for the -mfentry

>>> gcc feature introduced in [1] and [2].

>>> Patches 1-9 deal with avoiding clobbering %r0 when calling lazily bound

>>> functions, so that the new __fentry__ symbol could be called with return

>>> address in that register.

>>

>> Should GCC arrange for suitable ABI markup if -mfentry is used?  I'm worried that unexpected clobbers of r0 could be quite difficult to figure out otherwise.

> 

> I don’t think we need to do anything ABI-related at this point.

> 

> __fentry__ itself is being newly introduced, so there is no old ABI to

> comply with. Instrumented functions’ ABI does not change, particularly,

> %r0, being a volatile register, used to be clobbered anyway.

> 

> Are there other cases that I’m missing?


Agreed, with the __fentry__ addition in the same release as the 
trampoline change, this should be safe and avoid the issue I was worried 
about.

Thanks,
Florian
Stefan Liebler Aug. 7, 2018, 8:27 a.m. | #4
On 08/02/2018 09:57 AM, Ilya Leoshkevich wrote:
> This patch series adds the runtime support in glibc for the -mfentry

> gcc feature introduced in [1] and [2].

> 

> Patches 1-9 deal with avoiding clobbering %r0 when calling lazily bound

> functions, so that the new __fentry__ symbol could be called with return

> address in that register.

> 

> Patch 10 removes the assumption that only Intel implements __fentry__.

> 

> Patch 11 adds __fentry__ implementation.

> 

> Patch 12 is a cleanup of problems in 31-bit mcount, which are similar to

> those found while implementing 64-bit __fentry__.

> 

> [1] https://gcc.gnu.org/ml/gcc-patches/2018-07/msg00784.html

> [2] https://gcc.gnu.org/ml/gcc-patches/2018-07/msg00912.html

> 

> 


Hi,

I've reviewed the s390-specific patches. They are all okay.

I think the move of the Intel-specific __fentry__ defintion into the 
Versions-files of the corresponding sysdeps directories is okay, too.
Do anybody complain about that?

If not, I plan to commit this patch-series at the end of this week.
Thank you for implementing the __fentry__ support for s390x.

Bye
Stefan
Stefan Liebler Aug. 10, 2018, 7:15 a.m. | #5
On 08/07/2018 10:27 AM, Stefan Liebler wrote:
> On 08/02/2018 09:57 AM, Ilya Leoshkevich wrote:

>> This patch series adds the runtime support in glibc for the -mfentry

>> gcc feature introduced in [1] and [2].

>>

>> Patches 1-9 deal with avoiding clobbering %r0 when calling lazily bound

>> functions, so that the new __fentry__ symbol could be called with return

>> address in that register.

>>

>> Patch 10 removes the assumption that only Intel implements __fentry__.

>>

>> Patch 11 adds __fentry__ implementation.

>>

>> Patch 12 is a cleanup of problems in 31-bit mcount, which are similar to

>> those found while implementing 64-bit __fentry__.

>>

>> [1] https://gcc.gnu.org/ml/gcc-patches/2018-07/msg00784.html

>> [2] https://gcc.gnu.org/ml/gcc-patches/2018-07/msg00912.html

>>

>>

> 

> Hi,

> 

> I've reviewed the s390-specific patches. They are all okay.

> 

> I think the move of the Intel-specific __fentry__ defintion into the 

> Versions-files of the corresponding sysdeps directories is okay, too.

> Do anybody complain about that?

> 

> If not, I plan to commit this patch-series at the end of this week.

> Thank you for implementing the __fentry__ support for s390x.

> 

> Bye

> Stefan

> 


Committed.

Thanks
Stefan