Use libc_hidden_proto / _def for hidden wchar ifunc symbols.
Commit Message
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
Comments
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.
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.
>
>
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.
@@ -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;