Add wcharP.h to hide internal wchar functions [BZ #18822]

  For some targets, like i386 and s390, internal IFUNC functions must be
called via PLT with a special register.  Add <wcharP.h> to allow targets,
which don't need a special register to call internal IFUNC functions via
PLT or have internal non-IFUNC wchar functions, to allow direct access to
internal wchar functions within libc.so and libc.a without using GOT nor
PLT.

Tested on i686 and x86-64.  It removed 11 PLT relocations on i686 and
29 PLT relocations on x86-64.

Any comments?

H.J.
---
	[BZ #18822]
	* include/wchar.h: Include <wcharP.h>.
	* sysdeps/generic/wcharP.h: New file.
	* sysdeps/i386/wcharP.h: Likewise.
	* sysdeps/x86_64/wcharP.h: Likewise.
---
 include/wchar.h          |  1 +
 sysdeps/generic/wcharP.h | 18 ++++++++++++++++++
 sysdeps/i386/wcharP.h    | 24 ++++++++++++++++++++++++
 sysdeps/x86_64/wcharP.h  | 25 +++++++++++++++++++++++++
 4 files changed, 68 insertions(+)
 create mode 100644 sysdeps/generic/wcharP.h
 create mode 100644 sysdeps/i386/wcharP.h
 create mode 100644 sysdeps/x86_64/wcharP.h
  

On Fri, Nov 24, 2017 at 4:13 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> For some targets, like i386 and s390, internal IFUNC functions must be
> called via PLT with a special register.  Add <wcharP.h> to allow targets,
> which don't need a special register to call internal IFUNC functions via
> PLT or have internal non-IFUNC wchar functions, to allow direct access to
> internal wchar functions within libc.so and libc.a without using GOT nor
> PLT.
>
> Tested on i686 and x86-64.  It removed 11 PLT relocations on i686 and
> 29 PLT relocations on x86-64.
>
> Any comments?

Any objections?

> H.J.
> ---
>         [BZ #18822]
>         * include/wchar.h: Include <wcharP.h>.
>         * sysdeps/generic/wcharP.h: New file.
>         * sysdeps/i386/wcharP.h: Likewise.
>         * sysdeps/x86_64/wcharP.h: Likewise.
> ---
>  include/wchar.h          |  1 +
>  sysdeps/generic/wcharP.h | 18 ++++++++++++++++++
>  sysdeps/i386/wcharP.h    | 24 ++++++++++++++++++++++++
>  sysdeps/x86_64/wcharP.h  | 25 +++++++++++++++++++++++++
>  4 files changed, 68 insertions(+)
>  create mode 100644 sysdeps/generic/wcharP.h
>  create mode 100644 sysdeps/i386/wcharP.h
>  create mode 100644 sysdeps/x86_64/wcharP.h
>
> diff --git a/include/wchar.h b/include/wchar.h
> index 1db0ac8278..4160322234 100644
> --- a/include/wchar.h
> +++ b/include/wchar.h
> @@ -2,6 +2,7 @@
>  # include <wcsmbs/wchar.h>
>  # ifndef _ISOMAC
>
> +#include <wcharP.h>
>  #include <bits/floatn.h>
>
>  extern __typeof (wcscasecmp_l) __wcscasecmp_l;
> diff --git a/sysdeps/generic/wcharP.h b/sysdeps/generic/wcharP.h
> new file mode 100644
> index 0000000000..1a37283bf1
> --- /dev/null
> +++ b/sysdeps/generic/wcharP.h
> @@ -0,0 +1,18 @@
> +/* Internal prototrypes for multibyte and wide character functions.
> +   Generic version.
> +   Copyright (C) 2017 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> diff --git a/sysdeps/i386/wcharP.h b/sysdeps/i386/wcharP.h
> new file mode 100644
> index 0000000000..deac12e352
> --- /dev/null
> +++ b/sysdeps/i386/wcharP.h
> @@ -0,0 +1,24 @@
> +/* Internal prototrypes for multibyte and wide character functions.
> +   i386 version.
> +   Copyright (C) 2017 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +extern __typeof (wcpncpy) __wcpncpy attribute_hidden;
> +extern __typeof (wcscat) __wcscat attribute_hidden __attribute_pure__;
> +extern __typeof (wcschrnul) __wcschrnul attribute_hidden __attribute_pure__;
> +extern __typeof (wcsncpy) __wcsncpy attribute_hidden;
> +extern __typeof (wcsnlen) __wcsnlen attribute_hidden __attribute_pure__;
> diff --git a/sysdeps/x86_64/wcharP.h b/sysdeps/x86_64/wcharP.h
> new file mode 100644
> index 0000000000..bd6272db9d
> --- /dev/null
> +++ b/sysdeps/x86_64/wcharP.h
> @@ -0,0 +1,25 @@
> +/* Internal prototrypes for multibyte and wide character functions.
> +   x86-64 version.
> +   Copyright (C) 2017 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +extern __typeof (wcpncpy) __wcpncpy attribute_hidden;
> +extern __typeof (wcscat) __wcscat attribute_hidden __attribute_pure__;
> +extern __typeof (wcschrnul) __wcschrnul attribute_hidden __attribute_pure__;
> +extern __typeof (wcsncpy) __wcsncpy attribute_hidden;
> +extern __typeof (wcslen) __wcslen attribute_hidden __attribute_pure__;
> +extern __typeof (wcsnlen) __wcsnlen attribute_hidden __attribute_pure__;
> --
> 2.14.3
>
  

On Mon, Dec 4, 2017 at 5:07 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Nov 24, 2017 at 4:13 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> For some targets, like i386 and s390, internal IFUNC functions must be
>> called via PLT with a special register.  Add <wcharP.h> to allow targets,
>> which don't need a special register to call internal IFUNC functions via
>> PLT or have internal non-IFUNC wchar functions, to allow direct access to
>> internal wchar functions within libc.so and libc.a without using GOT nor
>> PLT.
>>
>> Tested on i686 and x86-64.  It removed 11 PLT relocations on i686 and
>> 29 PLT relocations on x86-64.
>>
>> Any comments?
>
> Any objections?

I am checking it in today.

>> H.J.
>> ---
>>         [BZ #18822]
>>         * include/wchar.h: Include <wcharP.h>.
>>         * sysdeps/generic/wcharP.h: New file.
>>         * sysdeps/i386/wcharP.h: Likewise.
>>         * sysdeps/x86_64/wcharP.h: Likewise.
>> ---
>>  include/wchar.h          |  1 +
>>  sysdeps/generic/wcharP.h | 18 ++++++++++++++++++
>>  sysdeps/i386/wcharP.h    | 24 ++++++++++++++++++++++++
>>  sysdeps/x86_64/wcharP.h  | 25 +++++++++++++++++++++++++
>>  4 files changed, 68 insertions(+)
>>  create mode 100644 sysdeps/generic/wcharP.h
>>  create mode 100644 sysdeps/i386/wcharP.h
>>  create mode 100644 sysdeps/x86_64/wcharP.h
>>
>> diff --git a/include/wchar.h b/include/wchar.h
>> index 1db0ac8278..4160322234 100644
>> --- a/include/wchar.h
>> +++ b/include/wchar.h
>> @@ -2,6 +2,7 @@
>>  # include <wcsmbs/wchar.h>
>>  # ifndef _ISOMAC
>>
>> +#include <wcharP.h>
>>  #include <bits/floatn.h>
>>
>>  extern __typeof (wcscasecmp_l) __wcscasecmp_l;
>> diff --git a/sysdeps/generic/wcharP.h b/sysdeps/generic/wcharP.h
>> new file mode 100644
>> index 0000000000..1a37283bf1
>> --- /dev/null
>> +++ b/sysdeps/generic/wcharP.h
>> @@ -0,0 +1,18 @@
>> +/* Internal prototrypes for multibyte and wide character functions.
>> +   Generic version.
>> +   Copyright (C) 2017 Free Software Foundation, Inc.
>> +   This file is part of the GNU C Library.
>> +
>> +   The GNU C Library is free software; you can redistribute it and/or
>> +   modify it under the terms of the GNU Lesser General Public
>> +   License as published by the Free Software Foundation; either
>> +   version 2.1 of the License, or (at your option) any later version.
>> +
>> +   The GNU C Library is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> +   Lesser General Public License for more details.
>> +
>> +   You should have received a copy of the GNU Lesser General Public
>> +   License along with the GNU C Library; if not, see
>> +   <http://www.gnu.org/licenses/>.  */
>> diff --git a/sysdeps/i386/wcharP.h b/sysdeps/i386/wcharP.h
>> new file mode 100644
>> index 0000000000..deac12e352
>> --- /dev/null
>> +++ b/sysdeps/i386/wcharP.h
>> @@ -0,0 +1,24 @@
>> +/* Internal prototrypes for multibyte and wide character functions.
>> +   i386 version.
>> +   Copyright (C) 2017 Free Software Foundation, Inc.
>> +   This file is part of the GNU C Library.
>> +
>> +   The GNU C Library is free software; you can redistribute it and/or
>> +   modify it under the terms of the GNU Lesser General Public
>> +   License as published by the Free Software Foundation; either
>> +   version 2.1 of the License, or (at your option) any later version.
>> +
>> +   The GNU C Library is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> +   Lesser General Public License for more details.
>> +
>> +   You should have received a copy of the GNU Lesser General Public
>> +   License along with the GNU C Library; if not, see
>> +   <http://www.gnu.org/licenses/>.  */
>> +
>> +extern __typeof (wcpncpy) __wcpncpy attribute_hidden;
>> +extern __typeof (wcscat) __wcscat attribute_hidden __attribute_pure__;
>> +extern __typeof (wcschrnul) __wcschrnul attribute_hidden __attribute_pure__;
>> +extern __typeof (wcsncpy) __wcsncpy attribute_hidden;
>> +extern __typeof (wcsnlen) __wcsnlen attribute_hidden __attribute_pure__;
>> diff --git a/sysdeps/x86_64/wcharP.h b/sysdeps/x86_64/wcharP.h
>> new file mode 100644
>> index 0000000000..bd6272db9d
>> --- /dev/null
>> +++ b/sysdeps/x86_64/wcharP.h
>> @@ -0,0 +1,25 @@
>> +/* Internal prototrypes for multibyte and wide character functions.
>> +   x86-64 version.
>> +   Copyright (C) 2017 Free Software Foundation, Inc.
>> +   This file is part of the GNU C Library.
>> +
>> +   The GNU C Library is free software; you can redistribute it and/or
>> +   modify it under the terms of the GNU Lesser General Public
>> +   License as published by the Free Software Foundation; either
>> +   version 2.1 of the License, or (at your option) any later version.
>> +
>> +   The GNU C Library is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> +   Lesser General Public License for more details.
>> +
>> +   You should have received a copy of the GNU Lesser General Public
>> +   License along with the GNU C Library; if not, see
>> +   <http://www.gnu.org/licenses/>.  */
>> +
>> +extern __typeof (wcpncpy) __wcpncpy attribute_hidden;
>> +extern __typeof (wcscat) __wcscat attribute_hidden __attribute_pure__;
>> +extern __typeof (wcschrnul) __wcschrnul attribute_hidden __attribute_pure__;
>> +extern __typeof (wcsncpy) __wcsncpy attribute_hidden;
>> +extern __typeof (wcslen) __wcslen attribute_hidden __attribute_pure__;
>> +extern __typeof (wcsnlen) __wcsnlen attribute_hidden __attribute_pure__;
>> --
>> 2.14.3
>>
>
>
>
> --
> H.J.
  

On 11/25/2017 01:13 AM, H.J. Lu wrote:

> diff --git a/sysdeps/generic/wcharP.h b/sysdeps/generic/wcharP.h
> new file mode 100644
> index 0000000000..1a37283bf1
> --- /dev/null
> +++ b/sysdeps/generic/wcharP.h
> @@ -0,0 +1,18 @@
> +/* Internal prototrypes for multibyte and wide character functions.

Typo (in all files).

I think this is architecturally wrong.

We should enhance the toolchain to do this kind of optimization 
automatically (maybe using LTO?).  Or find another way to implement the 
markup in a more centralized fashion, so that once you define an IFUNC 
resolver, the necessary arrangements to avoid a direct call are 
performed automatically.

In any case, these files need documentation what exactly you are trying 
to do.  The wcharP.h name does not match existing usage of *P.h headers. 
  The intent here seems to be not to add any new declarations to 
wcharP.h, only optimize existing declarations.

It's also not clear why you need to replicate ___atribute_pure__ in 
those declarations.

Thanks,
Florian
  

On Mon, 11 Dec 2017, Florian Weimer wrote:

> We should enhance the toolchain to do this kind of optimization automatically
> (maybe using LTO?).  Or find another way to implement the markup in a more

glibc makes use of being written as "C with ABI boundaries" - that is, 
code does things that are undefined in C for the program as a whole, but 
defined on the basis that each separate translation unit must implement 
the semantics of the functions therein in a way that would be valid for 
any valid caller.  I think supporting LTO for glibc would be hard.

> It's also not clear why you need to replicate ___atribute_pure__ in those
> declarations.

Generally, an internal header can avoid repeating a function prototype 
using typeof, but typeof does not copy attributes, so making an internal 
declaration share the attributes of a public declaration requires 
duplicating them in both places.  (GCC bug 81824 requests warnings for 
missing attributes on aliases, where the attributes are properties of the 
function rather than a particular name of the function, and where the 
compiler can see both declarations and the alias definition.)
  

On 12/12/2017 12:40 AM, Joseph Myers wrote:
> On Mon, 11 Dec 2017, Florian Weimer wrote:
> 
>> We should enhance the toolchain to do this kind of optimization automatically
>> (maybe using LTO?).  Or find another way to implement the markup in a more
> 
> glibc makes use of being written as "C with ABI boundaries" - that is,
> code does things that are undefined in C for the program as a whole, but
> defined on the basis that each separate translation unit must implement
> the semantics of the functions therein in a way that would be valid for
> any valid caller.  I think supporting LTO for glibc would be hard.

Certainly for some parts of the library.  But not for high-level 
routines such as fnmatch or wcsxfrm_l.

Anyway, the point I was trying to make is that some optimizations need 
to happen at the compiler or linker level, otherwise the maintenance 
burden is too costly.  Functions in the public API already need to be 
declared in three places in many cases, and I think it's unwise to add 
an arbitrary number of architecture-specific header files into the mix.

>> It's also not clear why you need to replicate ___atribute_pure__ in those
>> declarations.
> 
> Generally, an internal header can avoid repeating a function prototype
> using typeof, but typeof does not copy attributes, so making an internal
> declaration share the attributes of a public declaration requires
> duplicating them in both places.

I thought that these attributes were cumulative, so attribute_hidden 
would just be added to the attributes declared in include/wchar.h for 
those internal aliases.

Florian
  

On Tue, Dec 12, 2017 at 2:57 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 12/12/2017 12:40 AM, Joseph Myers wrote:
>>
>> On Mon, 11 Dec 2017, Florian Weimer wrote:
>>
>>> We should enhance the toolchain to do this kind of optimization
>>> automatically
>>> (maybe using LTO?).  Or find another way to implement the markup in a
>>> more
>>
>>
>> glibc makes use of being written as "C with ABI boundaries" - that is,
>> code does things that are undefined in C for the program as a whole, but
>> defined on the basis that each separate translation unit must implement
>> the semantics of the functions therein in a way that would be valid for
>> any valid caller.  I think supporting LTO for glibc would be hard.
>
>
> Certainly for some parts of the library.  But not for high-level routines
> such as fnmatch or wcsxfrm_l.
>
> Anyway, the point I was trying to make is that some optimizations need to
> happen at the compiler or linker level, otherwise the maintenance burden is
> too costly.  Functions in the public API already need to be declared in
> three places in many cases, and I think it's unwise to add an arbitrary
> number of architecture-specific header files into the mix.

I don't believe GCC can do it on its own without attribute.  Even with
attribute, GCC can't do it properly for builtin functions:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67220

Since we need to support more than one architecture and each architecture
has different requirements, we can't avoid multiple header files.   At minimum,
for each architecture, a public header file, foo.h, may need

1. foo.h.  Public header file.
2. include/foo.h.  Common internal header file.
3. foo-hidden.h.   Architecture specific header file to hide more
internal functions.

Of course, we can just use the public foo.h and pay the price.

>>> It's also not clear why you need to replicate ___atribute_pure__ in those
>>> declarations.
>>
>>
>> Generally, an internal header can avoid repeating a function prototype
>> using typeof, but typeof does not copy attributes, so making an internal
>> declaration share the attributes of a public declaration requires
>> duplicating them in both places.
>
>
> I thought that these attributes were cumulative, so attribute_hidden would
> just be added to the attributes declared in include/wchar.h for those
> internal aliases.

That isn't the case for these attributes.
  

On 12/12/2017 01:23 PM, H.J. Lu wrote:
> I don't believe GCC can do it on its own without attribute.  Even with
> attribute, GCC can't do it properly for builtin functions:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67220
> 
> Since we need to support more than one architecture and each architecture
> has different requirements, we can't avoid multiple header files.   At minimum,
> for each architecture, a public header file, foo.h, may need
> 
> 1. foo.h.  Public header file.
> 2. include/foo.h.  Common internal header file.
> 3. foo-hidden.h.   Architecture specific header file to hide more
> internal functions.
> 
> Of course, we can just use the public foo.h and pay the price.

You mean the common internal header file?  The public header file does 
not contain the internal declarations, so its use would result in 
namespace issues.

There are better ways than separate header files.  We could add a lookup 
table based on the function name, to see which optimizations to apply, 
and make sure that if an IFUNC resolver is defined, the function name 
has compatible attributes.

(This ties into the idea to auto-generate header files, e.g. deriving 
the internal header files from the public ones.)

My suggestion is to wait with these optimizations until such a mechanism 
is available.

Thanks,
Florian