Re: [PATCH] Use libc_hidden_proto / _def for hidden wchar ifunc symbols.

["H.J. Lu" <hjl.tools@gmail.com>]

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.


-- 
H.J.