[7/7] s390x: Use <gcc-macros.h> in early HWCAP check

Message ID 201e517c6a751bebe505af99e35a9f8643066f31.1642162312.git.fweimer@redhat.com
State Superseded
Headers
Series Reliable CPU compatibility diagnostics in ld.so |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Florian Weimer Jan. 14, 2022, 12:41 p.m. UTC
  This is required so that the checks still work if $(early-cflags)
selects a different ISA level.

---
 sysdeps/s390/s390-64/dl-hwcap-check.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
  

Comments

Stefan Liebler Jan. 18, 2022, 12:42 p.m. UTC | #1
On 14/01/2022 13:41, Florian Weimer wrote:
> This is required so that the checks still work if $(early-cflags)
> selects a different ISA level.
> 
> ---
>  sysdeps/s390/s390-64/dl-hwcap-check.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/sysdeps/s390/s390-64/dl-hwcap-check.h b/sysdeps/s390/s390-64/dl-hwcap-check.h
> index 53e02250b8..f769932325 100644
> --- a/sysdeps/s390/s390-64/dl-hwcap-check.h
> +++ b/sysdeps/s390/s390-64/dl-hwcap-check.h
> @@ -19,17 +19,18 @@
>  #ifndef _DL_HWCAP_CHECK_H
>  #define _DL_HWCAP_CHECK_H
>  
> +#include <gcc-macros.h>
>  #include <ldsodefs.h>
>  
>  static inline void
>  dl_hwcap_check (void)
>  {
>  #if defined __ARCH__
> -# if __ARCH__ >= 13
> +# if GCCMACRO__ARCH__ >= 13
>    if (!(GLRO(dl_hwcap) & HWCAP_S390_VXRS_EXT2))
>      _dl_fatal_printf ("\
>  Fatal glibc error: CPU lacks VXRS_EXT2 support (z15 or later required)\n");
> -# elif __ARCH__ >= 12
> +# elif GCCMACRO__ARCH__ >= 12
>    if (!(GLRO(dl_hwcap) & HWCAP_S390_VXE))
>      _dl_fatal_printf ("\
>  Fatal glibc error: CPU lacks VXE support (z14 or later required)\n");



Hi Florian,

I'm not quite sure if all your patches are already committed. I've just
give it a try with commit f8b765bec44e6c464a7eabf80e58c6851ca15ac3:

- configure glibc with --with-rtld-early-cflags=-march=zEC12 and
CFLAGS=-march=z15 on a z15.

- Rebooted with novx-kernel-parameter => vector-related HWCAPs are
disabled and executing vector-instructions leads to a crash

- run a helloworld-program: crash due to vector-instruction in
_dl_setup_hash, which is called in _dl_start_final before
_dl_sysdep_start is called which runs dl_hwcap_check.

I've checked the build-log and see that the following files are compiled
with -march=zEC12:
- dl-printf.c
- ../sysdeps/unix/sysv/linux/dl-write.c
- dl-tunables.c
- ../sysdeps/unix/sysv/linux/dl-sysdep.c
- rtld.c

Bye
Stefan
  
Florian Weimer Jan. 18, 2022, 12:54 p.m. UTC | #2
* Stefan Liebler:

> I'm not quite sure if all your patches are already committed. I've just
> give it a try with commit f8b765bec44e6c464a7eabf80e58c6851ca15ac3:
>
> - configure glibc with --with-rtld-early-cflags=-march=zEC12 and
> CFLAGS=-march=z15 on a z15.
>
> - Rebooted with novx-kernel-parameter => vector-related HWCAPs are
> disabled and executing vector-instructions leads to a crash
>
> - run a helloworld-program: crash due to vector-instruction in
> _dl_setup_hash, which is called in _dl_start_final before
> _dl_sysdep_start is called which runs dl_hwcap_check.
>
> I've checked the build-log and see that the following files are compiled
> with -march=zEC12:
> - dl-printf.c
> - ../sysdeps/unix/sysv/linux/dl-write.c
> - dl-tunables.c
> - ../sysdeps/unix/sysv/linux/dl-sysdep.c
> - rtld.c

Sorry, I missed that requirement.

Would you mind testing the attached patch?

Thanks,
Florian
  
Stefan Liebler Jan. 18, 2022, 1:31 p.m. UTC | #3
On 18/01/2022 13:54, Florian Weimer wrote:
> * Stefan Liebler:
> 
>> I'm not quite sure if all your patches are already committed. I've just
>> give it a try with commit f8b765bec44e6c464a7eabf80e58c6851ca15ac3:
>>
>> - configure glibc with --with-rtld-early-cflags=-march=zEC12 and
>> CFLAGS=-march=z15 on a z15.
>>
>> - Rebooted with novx-kernel-parameter => vector-related HWCAPs are
>> disabled and executing vector-instructions leads to a crash
>>
>> - run a helloworld-program: crash due to vector-instruction in
>> _dl_setup_hash, which is called in _dl_start_final before
>> _dl_sysdep_start is called which runs dl_hwcap_check.
>>
>> I've checked the build-log and see that the following files are compiled
>> with -march=zEC12:
>> - dl-printf.c
>> - ../sysdeps/unix/sysv/linux/dl-write.c
>> - dl-tunables.c
>> - ../sysdeps/unix/sysv/linux/dl-sysdep.c
>> - rtld.c
> 
> Sorry, I missed that requirement.
> 
> Would you mind testing the attached patch?
> 
> Thanks,
> Florian

Sure. Now it works fine and I get the expected:
Fatal glibc error: CPU lacks VXRS_EXT2 support (z15 or later required)

Thanks,
Stefan
  
Florian Weimer Jan. 18, 2022, 1:33 p.m. UTC | #4
* Stefan Liebler:

> On 18/01/2022 13:54, Florian Weimer wrote:
>> * Stefan Liebler:
>> 
>>> I'm not quite sure if all your patches are already committed. I've just
>>> give it a try with commit f8b765bec44e6c464a7eabf80e58c6851ca15ac3:
>>>
>>> - configure glibc with --with-rtld-early-cflags=-march=zEC12 and
>>> CFLAGS=-march=z15 on a z15.
>>>
>>> - Rebooted with novx-kernel-parameter => vector-related HWCAPs are
>>> disabled and executing vector-instructions leads to a crash
>>>
>>> - run a helloworld-program: crash due to vector-instruction in
>>> _dl_setup_hash, which is called in _dl_start_final before
>>> _dl_sysdep_start is called which runs dl_hwcap_check.
>>>
>>> I've checked the build-log and see that the following files are compiled
>>> with -march=zEC12:
>>> - dl-printf.c
>>> - ../sysdeps/unix/sysv/linux/dl-write.c
>>> - dl-tunables.c
>>> - ../sysdeps/unix/sysv/linux/dl-sysdep.c
>>> - rtld.c
>> 
>> Sorry, I missed that requirement.
>> 
>> Would you mind testing the attached patch?
>> 
>> Thanks,
>> Florian
>
> Sure. Now it works fine and I get the expected:
> Fatal glibc error: CPU lacks VXRS_EXT2 support (z15 or later required)

Thanks.  Should I push it with your Reviewed-by: and Tested-by:?

Florian
  
Stefan Liebler Jan. 18, 2022, 1:38 p.m. UTC | #5
On 18/01/2022 14:33, Florian Weimer wrote:
> * Stefan Liebler:
> 
>> On 18/01/2022 13:54, Florian Weimer wrote:
>>> * Stefan Liebler:
>>>
>>>> I'm not quite sure if all your patches are already committed. I've just
>>>> give it a try with commit f8b765bec44e6c464a7eabf80e58c6851ca15ac3:
>>>>
>>>> - configure glibc with --with-rtld-early-cflags=-march=zEC12 and
>>>> CFLAGS=-march=z15 on a z15.
>>>>
>>>> - Rebooted with novx-kernel-parameter => vector-related HWCAPs are
>>>> disabled and executing vector-instructions leads to a crash
>>>>
>>>> - run a helloworld-program: crash due to vector-instruction in
>>>> _dl_setup_hash, which is called in _dl_start_final before
>>>> _dl_sysdep_start is called which runs dl_hwcap_check.
>>>>
>>>> I've checked the build-log and see that the following files are compiled
>>>> with -march=zEC12:
>>>> - dl-printf.c
>>>> - ../sysdeps/unix/sysv/linux/dl-write.c
>>>> - dl-tunables.c
>>>> - ../sysdeps/unix/sysv/linux/dl-sysdep.c
>>>> - rtld.c
>>>
>>> Sorry, I missed that requirement.
>>>
>>> Would you mind testing the attached patch?
>>>
>>> Thanks,
>>> Florian
>>
>> Sure. Now it works fine and I get the expected:
>> Fatal glibc error: CPU lacks VXRS_EXT2 support (z15 or later required)
> 
> Thanks.  Should I push it with your Reviewed-by: and Tested-by:?
> 
> Florian
> 

Yes, this if fine for me.
I've had a look at the patch and it just extracted the _dl_setup_hash
function into a new file which is then build with the rtld-early-cflags.

Reviewed-by: Stefan Liebler <stli@linux.ibm.com>
Tested-by: Stefan Liebler <stli@linux.ibm.com>
  
Joseph Myers Jan. 18, 2022, 9:03 p.m. UTC | #6
The "elf: Move _dl_setup_hash to its own file" change (commit 
c90363403b57b3b7919061851cb3e6d9c85e784a) appears to have broken the build 
for MIPS (all ABIs).

In file included from ../sysdeps/gnu/ldsodefs.h:46,
                 from ../sysdeps/unix/sysv/linux/ldsodefs.h:25,
                 from ../sysdeps/unix/sysv/linux/mips/ldsodefs.h:22,
                 from dl-setup_hash.c:21:
dl-setup_hash.c: In function '_dl_setup_hash':
../sysdeps/mips/ldsodefs.h:39:33: error: implicit declaration of function 'DT_MIPS'; did you mean 'EM_MIPS'? [-Werror=implicit-function-declaration]
   39 |       (hash32) += (map)->l_info[DT_MIPS (SYMTABNO)]->d_un.d_val - (symbias); \
      |                                 ^~~~~~~
dl-setup_hash.c:48:7: note: in expansion of macro 'ELF_MACHINE_XHASH_SETUP'
   48 |       ELF_MACHINE_XHASH_SETUP (hash32, symbias, map);
      |       ^~~~~~~~~~~~~~~~~~~~~~~
../sysdeps/mips/ldsodefs.h:39:42: error: 'SYMTABNO' undeclared (first use in this function)
   39 |       (hash32) += (map)->l_info[DT_MIPS (SYMTABNO)]->d_un.d_val - (symbias); \
      |                                          ^~~~~~~~
dl-setup_hash.c:48:7: note: in expansion of macro 'ELF_MACHINE_XHASH_SETUP'
   48 |       ELF_MACHINE_XHASH_SETUP (hash32, symbias, map);
      |       ^~~~~~~~~~~~~~~~~~~~~~~
../sysdeps/mips/ldsodefs.h:39:42: note: each undeclared identifier is reported only once for each function it appears in
   39 |       (hash32) += (map)->l_info[DT_MIPS (SYMTABNO)]->d_un.d_val - (symbias); \
      |                                          ^~~~~~~~
dl-setup_hash.c:48:7: note: in expansion of macro 'ELF_MACHINE_XHASH_SETUP'
   48 |       ELF_MACHINE_XHASH_SETUP (hash32, symbias, map);
      |       ^~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
  
Florian Weimer Jan. 18, 2022, 9:21 p.m. UTC | #7
* Joseph Myers:

> The "elf: Move _dl_setup_hash to its own file" change (commit 
> c90363403b57b3b7919061851cb3e6d9c85e784a) appears to have broken the build 
> for MIPS (all ABIs).

Sorry, I didn't run a full build of a glibcs this time. 8-/

I did check that ELF_MACHINE_XHASH_SETUP was defined in mips
<ldsodefs.h>, but missed its macro dependency.

I'm build-testing a patch right now, will post it once it finishes
building.

Thanks,
Florian
  

Patch

diff --git a/sysdeps/s390/s390-64/dl-hwcap-check.h b/sysdeps/s390/s390-64/dl-hwcap-check.h
index 53e02250b8..f769932325 100644
--- a/sysdeps/s390/s390-64/dl-hwcap-check.h
+++ b/sysdeps/s390/s390-64/dl-hwcap-check.h
@@ -19,17 +19,18 @@ 
 #ifndef _DL_HWCAP_CHECK_H
 #define _DL_HWCAP_CHECK_H
 
+#include <gcc-macros.h>
 #include <ldsodefs.h>
 
 static inline void
 dl_hwcap_check (void)
 {
 #if defined __ARCH__
-# if __ARCH__ >= 13
+# if GCCMACRO__ARCH__ >= 13
   if (!(GLRO(dl_hwcap) & HWCAP_S390_VXRS_EXT2))
     _dl_fatal_printf ("\
 Fatal glibc error: CPU lacks VXRS_EXT2 support (z15 or later required)\n");
-# elif __ARCH__ >= 12
+# elif GCCMACRO__ARCH__ >= 12
   if (!(GLRO(dl_hwcap) & HWCAP_S390_VXE))
     _dl_fatal_printf ("\
 Fatal glibc error: CPU lacks VXE support (z14 or later required)\n");