Bug 81193 - PowerPC GCC __builtin_cpu_is and __builtin_cpu_supports should warn about old libraries
Summary: PowerPC GCC __builtin_cpu_is and __builtin_cpu_supports should warn about old...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 8.0
: P3 normal
Target Milestone: ---
Assignee: Michael Meissner
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-23 19:39 UTC by Michael Meissner
Modified: 2017-08-29 21:06 UTC (History)
8 users (show)

See Also:
Host:
Target: powerpc
Build:
Known to work:
Known to fail:
Last reconfirmed: 2017-06-26 00:00:00


Attachments
Patch to add ppc_cpu_supports_hw target support for tests (824 bytes, patch)
2017-06-27 22:14 UTC, Michael Meissner
Details | Diff
Proposed patch to fix the problem (2.36 KB, patch)
2017-07-11 19:50 UTC, Michael Meissner
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Meissner 2017-06-23 19:39:30 UTC
If GCC is configured with a new glibc, it supports the __builtin_cpu_is and __builtin_cpu_supports built-in functions.

However, if GCC is configured with an old glibc, the various __builtin_cpu* functions just return 0.

GCC should do either:

1) Warn that __builtin_cpu_is/__builtin_cpu_supports is not supported; (or)

2) It should call getauxval and check the HWCAP/HWCAP2 bits directly.  Perhaps there should be a helper function in libgcc that stands the built-in calls and the code generated for the built-in functions.
Comment 1 Peter Bergner 2017-06-23 20:45:00 UTC
I think 1) is doable, but I think there might be a problem with 2).  I seem to recall Alan mentioning that we have to be careful generating relocations in ifunc resolver functions and calling getauxval() would definitely generate one.  Alan, can you comment on what the issue is and whether it would be safe to call getauxval() in an ifunc resolver?
Comment 2 Michael Meissner 2017-06-23 21:54:29 UTC
One other possibility is to just remove the code that checks TARGET_LIBC_PROVIDES_HWCAP_IN_TCB, given that when we use it there is a reference to __parse_hwcap_and_convert_at_platform in the code.  That won't be satisified unless you have a new enough glibc.

Though it may be friendlier to warn the user if GCC wasn't configured to use the newer glibc.
Comment 3 Alan Modra 2017-06-24 06:06:31 UTC
Relocation problems with ifunc resolvers come about due to the fact that ifunc resolvers run at the same time other relocations are being processed.

So..
1) If the resolver runs before its code/data has been relocated, the resolver won't run correctly.
2) If the object requires text relocations then the text segment will be set read/write, but this makes it non-exec so any attempt to run an ifunc resolver will result in a segfault.

(1) is fixed for leaf function resolvers called from within the same object (shared lib or executable), as the linker sorts ifunc relocs last.  You will need to know the order in which an executable and its shared libraries are relocated by ld.so if you want to call a non-local resolver, or when the resolver itself makes non-local calls to functions that need their code/data relocating.  Generally, don't do that.  By inspection, getauxval loads values from the TOC/GOT, so it does need relocating, but if ld.so always relocates libc.so first then you might be OK calling getauxval..  I don't know enough about ld.so to answer that detail.  And, oh yeah, provided the user doesn't override libc.so getauxval with their own version.
(2) will give you an error/warning at link time.
Comment 4 Michael Meissner 2017-06-26 15:26:24 UTC
I have no problems with restricting use of __builtin_cpu_<xxx> and target_clone
to GLIBC 2.19 or newer (or whatever the release version is).  Now that I think
of it, yeah, we don't want to run ifunc resolvers if it has relocations.  I
don't know whether the linker should warn about problematic code (it probably
should, or at least we need to document it somewhere).  I was just surprised
that the compiler silently sets the test to 0.

Alternatively, we would need to do two passes.  On the first pass resolve all
of the normal functions and data locations, setting the location of ifunc
functions in the TOC to be an error function.  The second pass would resolve
the ifunc functions.  If an ifunc function calls another ifunc function that
hasn't been resolved yet, it would go to the error function.

I think for the problem of using __builtin_cpu_<xxx>, we should issue a warning
(not a fatal error) if the configured GLIBC is too old saying you need to link
against a newer library, but generate the same code that we normally do.  Given
there is a reference to an external provided with the new glibc, it should be
safe, because you would get a linker error if you actually tried to use it with
an old library.  It would allow creation of libraries with functions using
__builtin_cpu_* and target_clone with an old compiler, providing the library is
linked appropriately.

It would also allow the tests for target_clone to work.  Though I probably will
switch my builds to use --with-advance-toolchain to always get the new library
(now that works with bootstrap).
Comment 5 Peter Bergner 2017-06-26 23:06:23 UTC
Given Alan's comments, I think the best thing to do here is to just emit a warning when using those builtins when the compiler was configured to use an old glibc.  

If someone disagrees, let me know now, as I'm going to start coding up the change to add the warning.
Comment 6 Alan Modra 2017-06-27 01:41:08 UTC
> Alternatively, we would need to do two passes.  On the first pass resolve all
> of the normal functions and data locations, setting the location of ifunc
> functions in the TOC to be an error function.  The second pass would resolve
> the ifunc functions.  If an ifunc function calls another ifunc function that
> hasn't been resolved yet, it would go to the error function.

Don't hold your breath waiting for this solution.  It was proposed first here:
https://gcc.gnu.org/ml/gcc-patches/2009-07/msg01307.html
Comment 7 Tulio Magno Quites Machado Filho 2017-06-27 14:57:37 UTC
(In reply to Michael Meissner from comment #4)
> I have no problems with restricting use of __builtin_cpu_<xxx> and
> target_clone to GLIBC 2.19 or newer (or whatever the release version is).

That's glibc 2.23.
Comment 8 Peter Bergner 2017-06-27 19:10:46 UTC
(In reply to Michael Meissner from comment #4)
> I think for the problem of using __builtin_cpu_<xxx>, we should issue a warning
> (not a fatal error) if the configured GLIBC is too old saying you need to link
> against a newer library, but generate the same code that we normally do.  Given
> there is a reference to an external provided with the new glibc, it should be
> safe, because you would get a linker error if you actually tried to use it with
> an old library.  It would allow creation of libraries with functions using
> __builtin_cpu_* and target_clone with an old compiler, providing the library is
> linked appropriately.

But with your patch and one being contemplated for float128-ifunc.c in libgcc, GCC will be a user of those builtins, so if we build on a system with an old GLIBC, we'll get a bootstrap / build failure.  We can't have that.

I'm also starting to wonder about the warning.  We cannot just always emit the warning, because the GCC bootstrap uses -Werror, so again, we'd end up with a bootstrap / build error.  I agree that it would be nice to tell normal users that the builtin they're using to always going to return false, but how can we turn it off for GCC builds?
Comment 9 Michael Meissner 2017-06-27 22:11:35 UTC
Well in theory we could add yet another switch to enable/disable the warning, but we have too many switches as it is.

Note, I've put a warning in my development version of the target_clones patches, and I think I will keep it (my warning is only if you try to use target_clones on an old system).

We probably should update the gcc documentation to mention this restriction.

Most users won't be able to use __builtin_cpu_<xxx> until their distro updates to a new glibc (i.e. years).  The Advance Toolchain users will be to use it.
Comment 10 Michael Meissner 2017-06-27 22:14:29 UTC
Created attachment 41642 [details]
Patch to add ppc_cpu_supports_hw target support for tests

BTW, the patch attached allows us to check whether __builtin_cpu_supports works on power7 and newer systems.
Comment 11 Michael Meissner 2017-06-28 13:07:44 UTC
Author: meissner
Date: Wed Jun 28 13:07:12 2017
New Revision: 249737

URL: https://gcc.gnu.org/viewcvs?rev=249737&root=gcc&view=rev
Log:
[gcc]
2017-06-28  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR ipa/81238
	* multiple_target.c (create_dispatcher_calls): Set the default
	clone to be static, not public.

[gcc/testsuite]
2017-06-28  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/81193
	* lib/target-supports.exp
	(check_ppc_cpu_supports_hw_available): New test to make sure
	__builtin_cpu_supports works on power7 and newer.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/multiple_target.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/lib/target-supports.exp
Comment 12 Michael Meissner 2017-07-11 19:50:11 UTC
Created attachment 41721 [details]
Proposed patch to fix the problem
Comment 13 Michael Meissner 2017-07-12 23:08:22 UTC
Author: meissner
Date: Wed Jul 12 23:07:50 2017
New Revision: 250165

URL: https://gcc.gnu.org/viewcvs?rev=250165&root=gcc&view=rev
Log:
[gcc]
2017-07-12  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/81193
	* config/rs6000/rs6000-c.c (rs6000_cpu_cpp_builtins): If GLIBC
	provides the hardware capability bits, define the macro
	__BUILTIN_CPU_SUPPORTS__.
	* config/rs6000/rs6000.c (cpu_expand_builtin): Generate a warning
	if GLIBC does not provide the hardware capability bits.  Add a
	gcc_unreachable call if the built-in cpu function is neither
	__builtin_cpu_is nor __builtin_cpu_supports.
	(rs6000_get_function_versions_dispatcher): Change the warning
	that an old GLIBC is used which does not export the capability
	bits to be an error.
	* doc/extend.texi (target_clones attribute): Document the
	restriction that GLIBC 2.23 or newer is needed on the PowerPC.
	(PowerPC built-in functions): Document that GLIBC 2.23 or newer is
	needed by __builtin_cpu_is and __builtin_cpu_supports.  Document
	the macros defined by GCC if the newer GLIBC is available.

[gcc/testsuite]
2017-07-12  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/81193
	* gcc.target/powerpc/bmi-andn-1.c: Add guard against using
	__builtin_cpu_supports with old GLIBC's.
	* gcc.target/powerpc/bmi-andn-2.c: Likewise.
	* gcc.target/powerpc/bmi-bextr-1.c: Likewise.
	* gcc.target/powerpc/bmi-bextr-2.c: Likewise.
	* gcc.target/powerpc/bmi-bextr-4.c: Likewise.
	* gcc.target/powerpc/bmi-bextr-5.c: Likewise.
	* gcc.target/powerpc/bmi-blsi-1.c: Likewise.
	* gcc.target/powerpc/bmi-blsi-2.c: Likewise.
	* gcc.target/powerpc/bmi-blsmsk-1.c: Likewise.
	* gcc.target/powerpc/bmi-blsmsk-2.c: Likewise.
	* gcc.target/powerpc/bmi-blsr-1.c: Likewise.
	* gcc.target/powerpc/bmi-blsr-2.c: Likewise.
	* gcc.target/powerpc/bmi-tzcnt-1.c: Likewise.
	* gcc.target/powerpc/bmi-tzcnt-2.c: Likewise.
	* gcc.target/powerpc/bmi2-bzhi32-1.c: Likewise.
	* gcc.target/powerpc/bmi2-bzhi64-1.c: Likewise.
	* gcc.target/powerpc/bmi2-mulx32-1.c: Likewise.
	* gcc.target/powerpc/bmi2-mulx32-2.c: Likewise.
	* gcc.target/powerpc/bmi2-mulx64-1.c: Likewise.
	* gcc.target/powerpc/bmi2-mulx64-2.c: Likewise.
	* gcc.target/powerpc/bmi2-pdep32-1.c: Likewise.
	* gcc.target/powerpc/bmi2-pdep64-1.c: Likewise.
	* gcc.target/powerpc/bmi2-pext32-1.c: Likewise.
	* gcc.target/powerpc/bmi2-pext64-1.c: Likewise.
	* gcc.target/powerpc/cpu-builtin-1.c: Likewise.

[libgcc]
2017-07-12  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/81193
	* configure.ac (PowerPC float128 hardware support): Test whether
	we can use __builtin_cpu_supports before enabling the ifunc
	handler.
	* configure: Regenerate.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/rs6000/rs6000-c.c
    trunk/gcc/config/rs6000/rs6000.c
    trunk/gcc/doc/extend.texi
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.target/powerpc/bmi-andn-1.c
    trunk/gcc/testsuite/gcc.target/powerpc/bmi-andn-2.c
    trunk/gcc/testsuite/gcc.target/powerpc/bmi-bextr-1.c
    trunk/gcc/testsuite/gcc.target/powerpc/bmi-bextr-2.c
    trunk/gcc/testsuite/gcc.target/powerpc/bmi-bextr-4.c
    trunk/gcc/testsuite/gcc.target/powerpc/bmi-bextr-5.c
    trunk/gcc/testsuite/gcc.target/powerpc/bmi-blsi-1.c
    trunk/gcc/testsuite/gcc.target/powerpc/bmi-blsi-2.c
    trunk/gcc/testsuite/gcc.target/powerpc/bmi-blsmsk-1.c
    trunk/gcc/testsuite/gcc.target/powerpc/bmi-blsmsk-2.c
    trunk/gcc/testsuite/gcc.target/powerpc/bmi-blsr-1.c
    trunk/gcc/testsuite/gcc.target/powerpc/bmi-blsr-2.c
    trunk/gcc/testsuite/gcc.target/powerpc/bmi-tzcnt-1.c
    trunk/gcc/testsuite/gcc.target/powerpc/bmi-tzcnt-2.c
    trunk/gcc/testsuite/gcc.target/powerpc/bmi2-bzhi32-1.c
    trunk/gcc/testsuite/gcc.target/powerpc/bmi2-bzhi64-1.c
    trunk/gcc/testsuite/gcc.target/powerpc/bmi2-mulx32-1.c
    trunk/gcc/testsuite/gcc.target/powerpc/bmi2-mulx32-2.c
    trunk/gcc/testsuite/gcc.target/powerpc/bmi2-mulx64-1.c
    trunk/gcc/testsuite/gcc.target/powerpc/bmi2-mulx64-2.c
    trunk/gcc/testsuite/gcc.target/powerpc/bmi2-pdep32-1.c
    trunk/gcc/testsuite/gcc.target/powerpc/bmi2-pdep64-1.c
    trunk/gcc/testsuite/gcc.target/powerpc/bmi2-pext32-1.c
    trunk/gcc/testsuite/gcc.target/powerpc/bmi2-pext64-1.c
    trunk/gcc/testsuite/gcc.target/powerpc/cpu-builtin-1.c
    trunk/libgcc/ChangeLog
    trunk/libgcc/configure
    trunk/libgcc/configure.ac
Comment 14 Michael Meissner 2017-07-19 20:32:26 UTC
Author: meissner
Date: Wed Jul 19 20:31:53 2017
New Revision: 250368

URL: https://gcc.gnu.org/viewcvs?rev=250368&root=gcc&view=rev
Log:
[gcc]
2017-07-19  Michael Meissner  <meissner@linux.vnet.ibm.com>

	Back port from trunk
	2017-07-12  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/81193
	* config/rs6000/rs6000-c.c (rs6000_cpu_cpp_builtins): If GLIBC
	provides the hardware capability bits, define the macro
	__BUILTIN_CPU_SUPPORTS__.
	* config/rs6000/rs6000.c (cpu_expand_builtin): Generate a warning
	if GLIBC does not provide the hardware capability bits.  Add a
	gcc_unreachable call if the built-in cpu function is neither
	__builtin_cpu_is nor __builtin_cpu_supports.
	* doc/extend.texi (PowerPC built-in functions): Document that
	GLIBC 2.23 or newer is needed by __builtin_cpu_is and
	__builtin_cpu_supports.  Document the macros defined by GCC if the
	newer GLIBC is available.

[gcc/testsuite]
2017-07-19  Michael Meissner  <meissner@linux.vnet.ibm.com>

	Back port from trunk
	2017-07-12  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/81193
	* gcc.target/powerpc/cpu-builtin-1.c: Change test to use #ifdef
	__BUILTIN_CPU_SUPPORTS to see if the GLIBC is new enough that
	__builtin_cpu_is and __builtin_cpu_supports are supported.


Modified:
    branches/gcc-7-branch/gcc/ChangeLog
    branches/gcc-7-branch/gcc/config/rs6000/rs6000-c.c
    branches/gcc-7-branch/gcc/config/rs6000/rs6000.c
    branches/gcc-7-branch/gcc/doc/extend.texi
    branches/gcc-7-branch/gcc/testsuite/ChangeLog
    branches/gcc-7-branch/gcc/testsuite/gcc.target/powerpc/cpu-builtin-1.c
Comment 15 Michael Meissner 2017-07-19 22:05:53 UTC
Author: meissner
Date: Wed Jul 19 22:05:20 2017
New Revision: 250371

URL: https://gcc.gnu.org/viewcvs?rev=250371&root=gcc&view=rev
Log:
[gcc]
2017-07-19  Michael Meissner  <meissner@linux.vnet.ibm.com>

	Back port from trunk
	2017-07-12  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/81193
	* config/rs6000/rs6000-c.c (rs6000_cpu_cpp_builtins): If GLIBC
	provides the hardware capability bits, define the macro
	__BUILTIN_CPU_SUPPORTS__.
	* config/rs6000/rs6000.c (cpu_expand_builtin): Generate a warning
	if GLIBC does not provide the hardware capability bits.  Add a
	gcc_unreachable call if the built-in cpu function is neither
	__builtin_cpu_is nor __builtin_cpu_supports.
	* doc/extend.texi (PowerPC built-in functions): Document that
	GLIBC 2.23 or newer is needed by __builtin_cpu_is and
	__builtin_cpu_supports.  Document the macros defined by GCC if the
	newer GLIBC is available.

[gcc/testsuite]
2017-07-19  Michael Meissner  <meissner@linux.vnet.ibm.com>

	Back port from trunk
	2017-07-12  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/81193
	* gcc.target/powerpc/cpu-builtin-1.c: Change test to use #ifdef
	__BUILTIN_CPU_SUPPORTS to see if the GLIBC is new enough that
	__builtin_cpu_is and __builtin_cpu_supports are supported.


Modified:
    branches/gcc-6-branch/gcc/config/rs6000/rs6000-c.c
    branches/gcc-6-branch/gcc/config/rs6000/rs6000.c
    branches/gcc-6-branch/gcc/doc/extend.texi
    branches/gcc-6-branch/gcc/testsuite/gcc.target/powerpc/cpu-builtin-1.c
Comment 16 Michael Meissner 2017-07-20 20:29:48 UTC
Fixed in trunk, and backported to GCC 7/6.
Comment 17 Michael Meissner 2017-08-29 21:06:53 UTC
Author: meissner
Date: Tue Aug 29 21:06:21 2017
New Revision: 251437

URL: https://gcc.gnu.org/viewcvs?rev=251437&root=gcc&view=rev
Log:
<add missing ChangeLogs>

[gcc]
2017-07-13  Michael Meissner  <meissner@linux.vnet.ibm.com>

	Back port from trunk
	2017-07-12  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/81193
	* config/rs6000/rs6000-c.c (rs6000_cpu_cpp_builtins): If GLIBC
	provides the hardware capability bits, define the macro
	__BUILTIN_CPU_SUPPORTS__.
	* config/rs6000/rs6000.c (cpu_expand_builtin): Generate a warning
	if GLIBC does not provide the hardware capability bits.  Add a
	gcc_unreachable call if the built-in cpu function is neither
	__builtin_cpu_is nor __builtin_cpu_supports.
	* doc/extend.texi (PowerPC built-in functions): Document that
	GLIBC 2.23 or newer is needed by __builtin_cpu_is and
	__builtin_cpu_supports.  Document the macros defined by GCC if the
	newer GLIBC is available.

[gcc/testsuite]
2017-07-13  Michael Meissner  <meissner@linux.vnet.ibm.com>

	Back port from trunk
	2017-06-28  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/81193
	* lib/target-supports.exp
	(check_ppc_cpu_supports_hw_available): New test to make sure
	__builtin_cpu_supports works on power7 and newer.

	Back port from trunk
	2017-07-12  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/81193
	* gcc.target/powerpc/cpu-builtin-1.c: Add guard against using
	__builtin_cpu_supports with old GLIBC's.


Modified:
    branches/gcc-6-branch/gcc/ChangeLog
    branches/gcc-6-branch/gcc/testsuite/ChangeLog