Re: [PATCH v2] S390: Always enable static PIE if build with lld.

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 26/02/24 11:32, Stefan Liebler wrote:
> On 23.02.24 15:48, Adhemerval Zanella Netto wrote:
>>
>>
>> On 21/02/24 11:56, Stefan Liebler wrote:
>>
>> Sorry, but all the PIE tests on s390x are overcomplicated and brittle.  The version
>> check is really unecessary and I think we should move away from it, instead check
>> for either linker support and resulting relocations on generated code.
>>
>> The gold supports could be just filtered out by using -Wl,--no-dynamic-linker on
> Yes, when linking with "gcc -static-pie", gcc automatically adds
> --no-dynamic-linker and thus "-Wl,--no-dynamic-linker" can be added to
> the -pie check.

Indeed, I think all supported gcc version add --no-dynamic-linker for --static-pie.

> 
>> the -pie check.  Also, the relocation check does not seem correct: at least
>> with binutils 2.36 ld generates R_390_TLS_DTPMOD/R_390_TLS_DTPOFF for the
>> 'foo' access.
> Interesting. I don't see R_390_TLS_DTPMOD/R_390_TLS_DTPOFF with binutils
> 2.36, but:
> - conftest1.o
> 000000000000  000a00000033 R_390_TLS_LE64    0000000000000000 foo + 0
> - conftest2.o
> 00000000000a  000a00000031 R_390_TLS_IEENT   0000000000000000 foo + 2
> - conftest
> 000000001fe8  000800000038 R_390_TLS_TPOFF   0000000000000000 foo + 0
> 
> Can you please have a further look how the
> R_390_TLS_DTPMOD/R_390_TLS_DTPOFF were generated?

This was from the startup/standard library, the R_390_TLS_TPOFF is indeed
the correct relocation to check.

>>
>> So I think we should use something as the below.  The libc_cv_s390x_staticpie_req_runtime
>> is correctly set for old binutils and lld version 17, while being true for recent
>> binutils and lld version 18+.  I also think all the comments of the requirement
>> are confusing and does not help much (it also references others ABI, like i386
>> and ia64 which is not straightforward to understand why it matter for s390x),
>> so I would recommend to just remove them altogether.
> But, if you are bootstrapping the toolchain, then you don't have
> crt-files and the linking -pie check fails. Then you don't get static
> pie support and no rcrt1.o file.
> This is e.g. the case for the Ubuntu libc6-dev-s390x-cross package.
> 
> You also see it with build-many-glibcs.py script. Note that there glibc
> configure is called twice:
> -
> logs/compilers/s390x-linux-gnu/026-compilers-s390x-linux-gnu-glibc-s390-linux-gnu-configure-log.txt
> => called with empty sysroot. The following make install creates
> crt-files in the sysroot folder. Currently rcrt1.o is created. Without
> the version check, it would not be created.
> 
> - logs/glibcs/s390x-linux-gnu/003-glibcs-s390x-linux-gnu-configure-log.txt

You are right, we need to follow what loongarch and riscv does and use
-nostartfiles -nostdlib. Something like:

diff --git a/sysdeps/s390/s390-64/configure.ac b/sysdeps/s390/s390-64/configure.ac
index 4657de0d37..a8849599b8 100644
--- a/sysdeps/s390/s390-64/configure.ac
+++ b/sysdeps/s390/s390-64/configure.ac
@@ -1,34 +1,6 @@
 GLIBC_PROVIDES dnl See aclocal.m4 in the top level source directory.
 # Local configure fragment for sysdeps/s390/s390-64.

-# Bypass result of runtime configure check for known linker versions as
-# e.g. crt-files or libc.so might not be available in bootstrapping
-# environments.
-case $($LD --version) in
-  "GNU gold"*)
-    # As of 2023-08-07, gold does not support static PIE due to
-    # Bug 22221 - add --no-dynamic-linker option
-    # https://sourceware.org/bugzilla/show_bug.cgi?id=22221
-    libc_cv_s390x_staticpie_req_version=no
-    ;;
-  "LLD"*)
-    # As of 2023-08-07, there is no lld which supports s390x.
-    libc_cv_s390x_staticpie_req_version=no
-    ;;
-  *)
-    # The required binutils patches are available with bintuils 2.39
-    libc_cv_s390x_staticpie_req_version=yes
-    # Skip AC_CHECK_PROGS and just use the result from main configure.ac.
-    libc_cv_s390x_staticpie_req_LD=$LD
-    AC_CHECK_PROG_VER(libc_cv_s390x_staticpie_req_LD, $LD, --version,
-                     [GNU ld.* \([0-9][0-9]*\.[0-9.]*\)],
-                     [2.1[0-9][0-9]*|2.39*|2.[4-9][0-9]*|[3-9].*|[1-9][0-9]*],
-                     libc_cv_s390x_staticpie_req_version=no)
-    ;;
-esac
-AC_MSG_CHECKING([for s390-specific static PIE requirements (version check)])
-AC_MSG_RESULT($libc_cv_s390x_staticpie_req_version)
-
 # Minimal checking for static PIE support in ld.
 # Compare to ld testcase/bugzilla:
 # <binutils-source>/ld/testsuite/ld-elf/pr22263-1.rd
@@ -58,15 +30,14 @@ EOF
   libc_cv_s390x_staticpie_req_runtime=no
   if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -fPIE -c conftest1.c -o conftest1.o]) \
      && AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -fPIE -c conftest2.c -o conftest2.o]) \
-     && AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -pie -o conftest conftest1.o conftest2.o]) \
+     && AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -static-pie -nostartfiles -nostdlib -fPIE -o conftest conftest1.o conftest2.o]) \
      && AC_TRY_COMMAND([! LC_ALL=C $READELF -Wr conftest | grep R_390_TLS_TPOFF])
   then
     libc_cv_s390x_staticpie_req_runtime=yes
   fi
   rm -rf conftest.*])

-if test $libc_cv_s390x_staticpie_req_runtime = yes \
-   || test $libc_cv_s390x_staticpie_req_version = yes; then
+if test $libc_cv_s390x_staticpie_req_runtime = yes; then
    # Static PIE is supported only on 64bit.
    # Ensure you also have those patches for:
    # - binutils (ld)