Patchwork Use libc_hidden_proto / _def for hidden wchar ifunc symbols.

login
register
mail settings
Submitter Stefan Liebler
Date Nov. 20, 2017, 3:51 p.m.
Message ID <709ddc70-da73-e352-03f6-3a9414a0a064@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/24369/
State Committed
Headers show

Comments

Stefan Liebler - Nov. 20, 2017, 3:51 p.m.
On 11/13/2017 06:30 PM, H.J. Lu wrote:
> On Mon, Nov 13, 2017 at 9:11 AM, Stefan Liebler <stli@linux.vnet.ibm.com> wrote:
>> On 11/13/2017 02:58 PM, H.J. Lu wrote:
>>>
>>> On Mon, Nov 13, 2017 at 3:55 AM, Florian Weimer <fweimer@redhat.com>
>>> wrote:
>>>>
>>>> On 11/13/2017 11:16 AM, Stefan Liebler wrote:
>>>>>
>>>>>
>>>>> This patch adds the libc_hidden_proto / libc_hidden_def construct.
>>>>> Then the __GI_* symbols are the default-ifunc-variants which can be
>>>>> called
>>>>> without PLT.
>>>>
>>>>
>>>>
>>>> attribute_hidden and *_hidden_{proto,def} conflict on some architectures.
>>>> You need to remove attribute_hidden as part of this change.
>>>
>>>
>>> That is true.  On i686, a hidden IFUNC function inside libc.so must be
>>> compiled
>>> with -fPIC via PLT since EBX must be loaded with GOT first.   This
>>> isn't an issue
>>> for x86-64 since PLT uses PC-relative addressing.  In this case, we
>>> should remove
>>> hidden attribute, instead of using __GI_* symbols, if we sill want to use
>>> IFUNC
>>> inside libc.so.
For the moment, I propose to remove the attribute_hidden in order to fix 
those internal IFUNC calls.
Is the attached patch okay to commit?

>>
>> On s390/s390x, just removing attribute_hidden in wchar.h is fine, too. Then
>> r12 will be setup with the GOT-pointer and IFUNC will be used inside
>> libc.so.
>>
>> What was your original intention? Getting rid of "PLT" or "PLT and IFUNC"
>> for calls inside libc.so?
> 
> The original intention is to remove PLT.  But it doesn't work for targets which
> need to set up a special register for PLT which is required by IFUNC.
> 
>>>
>>> Now I have question, is there a way to apply attribute_hidden to a
>>> function
>>> depending on architecture? For example, we remove attribute_hidden
>>> from __wcsnlen, __wcscat, __wcsncpy, __wcpncpy,  __wcschrnul in headers
>>> under include/.   For x86, we mark them hidden in a header file under
>>> sysdeps/x86?
>>
>> I don't know if duplicating the wchar.h file is such a good idea.
>>
> 
> Can we add
> 
> #include <wcharP.h>
> 
> to include/wchar.h and add a dummy sysdeps/generic/wcharP.h?  Then
> I can add sysdeps/x86_64/wcharP.h to hide __wcsnlen, __wcscat, __wcsncpy,
> _wcpncpy,  __wcschrnul
> 
> 
@H.J. Lu: Can you propose a separate patch for architecture dependent 
wcharP.h files?

Bye.
Stefan
H.J. Lu - Nov. 20, 2017, 3:59 p.m.
On Mon, Nov 20, 2017 at 7:51 AM, Stefan Liebler <stli@linux.vnet.ibm.com> wrote:
> On 11/13/2017 06:30 PM, H.J. Lu wrote:
>>
>> On Mon, Nov 13, 2017 at 9:11 AM, Stefan Liebler <stli@linux.vnet.ibm.com>
>> wrote:
>>>
>>> On 11/13/2017 02:58 PM, H.J. Lu wrote:
>>>>
>>>>
>>>> On Mon, Nov 13, 2017 at 3:55 AM, Florian Weimer <fweimer@redhat.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> On 11/13/2017 11:16 AM, Stefan Liebler wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> This patch adds the libc_hidden_proto / libc_hidden_def construct.
>>>>>> Then the __GI_* symbols are the default-ifunc-variants which can be
>>>>>> called
>>>>>> without PLT.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> attribute_hidden and *_hidden_{proto,def} conflict on some
>>>>> architectures.
>>>>> You need to remove attribute_hidden as part of this change.
>>>>
>>>>
>>>>
>>>> That is true.  On i686, a hidden IFUNC function inside libc.so must be
>>>> compiled
>>>> with -fPIC via PLT since EBX must be loaded with GOT first.   This
>>>> isn't an issue
>>>> for x86-64 since PLT uses PC-relative addressing.  In this case, we
>>>> should remove
>>>> hidden attribute, instead of using __GI_* symbols, if we sill want to
>>>> use
>>>> IFUNC
>>>> inside libc.so.
>
> For the moment, I propose to remove the attribute_hidden in order to fix
> those internal IFUNC calls.
> Is the attached patch okay to commit?

LGTM.

>>>
>>> On s390/s390x, just removing attribute_hidden in wchar.h is fine, too.
>>> Then
>>> r12 will be setup with the GOT-pointer and IFUNC will be used inside
>>> libc.so.
>>>
>>> What was your original intention? Getting rid of "PLT" or "PLT and IFUNC"
>>> for calls inside libc.so?
>>
>>
>> The original intention is to remove PLT.  But it doesn't work for targets
>> which
>> need to set up a special register for PLT which is required by IFUNC.
>>
>>>>
>>>> Now I have question, is there a way to apply attribute_hidden to a
>>>> function
>>>> depending on architecture? For example, we remove attribute_hidden
>>>> from __wcsnlen, __wcscat, __wcsncpy, __wcpncpy,  __wcschrnul in headers
>>>> under include/.   For x86, we mark them hidden in a header file under
>>>> sysdeps/x86?
>>>
>>>
>>> I don't know if duplicating the wchar.h file is such a good idea.
>>>
>>
>> Can we add
>>
>> #include <wcharP.h>
>>
>> to include/wchar.h and add a dummy sysdeps/generic/wcharP.h?  Then
>> I can add sysdeps/x86_64/wcharP.h to hide __wcsnlen, __wcscat, __wcsncpy,
>> _wcpncpy,  __wcschrnul
>>
>>
> @H.J. Lu: Can you propose a separate patch for architecture dependent
> wcharP.h files?

Sure.  I will submit one after your patch is checked in.
Stefan Liebler - Nov. 21, 2017, 7:48 a.m.
On 11/20/2017 04:59 PM, H.J. Lu wrote:
> On Mon, Nov 20, 2017 at 7:51 AM, Stefan Liebler <stli@linux.vnet.ibm.com> wrote:
>> On 11/13/2017 06:30 PM, H.J. Lu wrote:
>>>
>>> On Mon, Nov 13, 2017 at 9:11 AM, Stefan Liebler <stli@linux.vnet.ibm.com>
>>> wrote:
>>>>
>>>> On 11/13/2017 02:58 PM, H.J. Lu wrote:
>>>>>
>>>>>
>>>>> On Mon, Nov 13, 2017 at 3:55 AM, Florian Weimer <fweimer@redhat.com>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>> On 11/13/2017 11:16 AM, Stefan Liebler wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> This patch adds the libc_hidden_proto / libc_hidden_def construct.
>>>>>>> Then the __GI_* symbols are the default-ifunc-variants which can be
>>>>>>> called
>>>>>>> without PLT.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> attribute_hidden and *_hidden_{proto,def} conflict on some
>>>>>> architectures.
>>>>>> You need to remove attribute_hidden as part of this change.
>>>>>
>>>>>
>>>>>
>>>>> That is true.  On i686, a hidden IFUNC function inside libc.so must be
>>>>> compiled
>>>>> with -fPIC via PLT since EBX must be loaded with GOT first.   This
>>>>> isn't an issue
>>>>> for x86-64 since PLT uses PC-relative addressing.  In this case, we
>>>>> should remove
>>>>> hidden attribute, instead of using __GI_* symbols, if we sill want to
>>>>> use
>>>>> IFUNC
>>>>> inside libc.so.
>>
>> For the moment, I propose to remove the attribute_hidden in order to fix
>> those internal IFUNC calls.
>> Is the attached patch okay to commit?
> 
> LGTM.
> 
Committed.

>>>>
>>>> On s390/s390x, just removing attribute_hidden in wchar.h is fine, too.
>>>> Then
>>>> r12 will be setup with the GOT-pointer and IFUNC will be used inside
>>>> libc.so.
>>>>
>>>> What was your original intention? Getting rid of "PLT" or "PLT and IFUNC"
>>>> for calls inside libc.so?
>>>
>>>
>>> The original intention is to remove PLT.  But it doesn't work for targets
>>> which
>>> need to set up a special register for PLT which is required by IFUNC.
>>>
>>>>>
>>>>> Now I have question, is there a way to apply attribute_hidden to a
>>>>> function
>>>>> depending on architecture? For example, we remove attribute_hidden
>>>>> from __wcsnlen, __wcscat, __wcsncpy, __wcpncpy,  __wcschrnul in headers
>>>>> under include/.   For x86, we mark them hidden in a header file under
>>>>> sysdeps/x86?
>>>>
>>>>
>>>> I don't know if duplicating the wchar.h file is such a good idea.
>>>>
>>>
>>> Can we add
>>>
>>> #include <wcharP.h>
>>>
>>> to include/wchar.h and add a dummy sysdeps/generic/wcharP.h?  Then
>>> I can add sysdeps/x86_64/wcharP.h to hide __wcsnlen, __wcscat, __wcsncpy,
>>> _wcpncpy,  __wcschrnul
>>>
>>>
>> @H.J. Lu: Can you propose a separate patch for architecture dependent
>> wcharP.h files?
> 
> Sure.  I will submit one after your patch is checked in.
> 
>

Patch

commit 70c17e8f6a07236aaa37e1aab67703d3bc1e09a2
Author: Stefan Liebler <stli@linux.vnet.ibm.com>
Date:   Mon Nov 20 12:26:24 2017 +0100

    Remove attribute_hidden for wchar ifunc symbols.
    
    On s390 (31bit) various debug/tst-*chk* testcases are failing as the tests
    are ending with a segmentation fault.
    
    One test is e.g. calling wcsnrtombs in debug/tst-chk1.c:1549.
    The function wcsnrtombs itself calls __wcsnlen. This function is called via
    PLT! The PLT-stub itself loads the address from GOT (r12 is assumed to be
    the GOT-pointer). In this case the loaded address is zero and the following
    branch leads to the segmentation fault.
    
    Due to the attribute_hidden in commit 44af8a32c341672b5160fdc2839767e9a837ad26
    "Mark internal wchar functions with attribute_hidden [BZ #18822]"
    for e.g. the __wcsnlen function, r12 is not loaded with the GOT-pointer
    in wcsnrtombs.
    
    On s390x (64bit), this __wcsnlen call is also using the PLT-stub. But it is
    not failing as the GOT-pointer is setup with larl-instruction by the PLT-stub
    itself.
    Note: On s390x/s390, __wcsnlen is an IFUNC symbol.
    
    On x86_64, __wcsnlen is also an IFUNC symbol and is called via PLT, too.
    
    Further IFUNC symbols on s390 which were marked as hidden by the mentioned
    commit are: __wcscat, __wcsncpy, __wcpncpy, __wcschrnul.
    
    This patch removes the attribute_hidden in wchar.h.
    Then the compiler setups e.g. r12 on s390 in order to call __wcsnlen via PLT.
    
    ChangeLog:
    
    	* include/wchar.h (__wcsnlen, __wcscat, __wcsncpy, __wcpncpy,
    	__wcschrnul): Remove attribute_hidden.

diff --git a/include/wchar.h b/include/wchar.h
index 24b2eaa..1db0ac8 100644
--- a/include/wchar.h
+++ b/include/wchar.h
@@ -152,9 +152,8 @@  extern int __wcsncasecmp (const wchar_t *__s1, const wchar_t *__s2,
      __attribute_pure__;
 extern size_t __wcslen (const wchar_t *__s) __attribute_pure__;
 extern size_t __wcsnlen (const wchar_t *__s, size_t __maxlen)
-     attribute_hidden __attribute_pure__;
-extern wchar_t *__wcscat (wchar_t *dest, const wchar_t *src)
-     attribute_hidden;
+     __attribute_pure__;
+extern wchar_t *__wcscat (wchar_t *dest, const wchar_t *src);
 extern wint_t __btowc (int __c) attribute_hidden;
 extern int __mbsinit (const __mbstate_t *__ps);
 extern size_t __mbrtowc (wchar_t *__restrict __pwc,
@@ -182,11 +181,10 @@  extern size_t __wcsnrtombs (char *__restrict __dst,
 			    __mbstate_t *__restrict __ps)
      attribute_hidden;
 extern wchar_t *__wcsncpy (wchar_t *__restrict __dest,
-			   const wchar_t *__restrict __src, size_t __n)
-     attribute_hidden;
+			   const wchar_t *__restrict __src, size_t __n);
 extern wchar_t *__wcpcpy (wchar_t *__dest, const wchar_t *__src);
 extern wchar_t *__wcpncpy (wchar_t *__dest, const wchar_t *__src,
-			   size_t __n) attribute_hidden;
+			   size_t __n);
 extern wchar_t *__wmemcpy (wchar_t *__s1, const wchar_t *s2,
 			   size_t __n) attribute_hidden;
 extern wchar_t *__wmempcpy (wchar_t *__restrict __s1,
@@ -195,7 +193,7 @@  extern wchar_t *__wmempcpy (wchar_t *__restrict __s1,
 extern wchar_t *__wmemmove (wchar_t *__s1, const wchar_t *__s2,
 			    size_t __n) attribute_hidden;
 extern wchar_t *__wcschrnul (const wchar_t *__s, wchar_t __wc)
-     attribute_hidden __attribute_pure__;
+     __attribute_pure__;
 
 extern wchar_t *__wmemset_chk (wchar_t *__s, wchar_t __c, size_t __n,
 			       size_t __ns) __THROW;