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

[Stefan Liebler <stli at linux dot vnet dot ibm dot com>]

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.