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

[Stefan Liebler <stli@linux.ibm.com>]

On 23.02.24 15:48, Adhemerval Zanella Netto wrote:
> 
> 
> On 21/02/24 11:56, Stefan Liebler wrote:
>> LLVM ld.lld now	also supports s390x and avoids unnecessary TPOFF
>> relocations for position independent executables.  Both recent
>> commits were also cherry-picked to LLVM 18.
>>
>> This patch just enables static PIE if build with supported lld.
>> ---
>>  sysdeps/s390/s390-64/configure    | 80 ++++++++++++++++++++++++++++++-
>>  sysdeps/s390/s390-64/configure.ac | 16 ++++++-
>>  2 files changed, 92 insertions(+), 4 deletions(-)
>>
>> diff --git a/sysdeps/s390/s390-64/configure b/sysdeps/s390/s390-64/configure
>> index 824ae9c129..d79a25ecf7 100644
>> --- a/sysdeps/s390/s390-64/configure
>> +++ b/sysdeps/s390/s390-64/configure
>> @@ -12,8 +12,84 @@ case $($LD --version) in
>>      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 lld patches are available with LLVM 18:
>> +    # - [lld] Add target support for SystemZ (s390x) #75643
>> +    # https://github.com/llvm/llvm-project/pull/75643
>> +    # 2024-02-13: https://github.com/llvm/llvm-project/commit/fe3406e349884e4ef61480dd0607f1e237102c74
>> +    # - [lld/ELF] Avoid unnecessary TPOFF relocations in GOT for -pie #81739
>> +    # https://github.com/llvm/llvm-project/pull/81739
>> +    # 2024-02-14: https://github.com/llvm/llvm-project/commit/6f907733e65d24edad65f763fb14402464bd578b
>> +    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
>> +    for ac_prog in $LD
>> +do
>> +  # Extract the first word of "$ac_prog", so it can be a program name with args.
>> +set dummy $ac_prog; ac_word=$2
>> +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
>> +printf %s "checking for $ac_word... " >&6; }
>> +if test ${ac_cv_prog_libc_cv_s390x_staticpie_req_LD+y}
>> +then :
>> +  printf %s "(cached) " >&6
>> +else $as_nop
>> +  if test -n "$libc_cv_s390x_staticpie_req_LD"; then
>> +  ac_cv_prog_libc_cv_s390x_staticpie_req_LD="$libc_cv_s390x_staticpie_req_LD" # Let the user override the test.
>> +else
>> +as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
>> +for as_dir in $PATH
>> +do
>> +  IFS=$as_save_IFS
>> +  case $as_dir in #(((
>> +    '') as_dir=./ ;;
>> +    */) ;;
>> +    *) as_dir=$as_dir/ ;;
>> +  esac
>> +    for ac_exec_ext in '' $ac_executable_extensions; do
>> +  if as_fn_executable_p "$as_dir$ac_word$ac_exec_ext"; then
>> +    ac_cv_prog_libc_cv_s390x_staticpie_req_LD="$ac_prog"
>> +    printf "%s\n" "$as_me:${as_lineno-$LINENO}: found $as_dir$ac_word$ac_exec_ext" >&5
>> +    break 2
>> +  fi
>> +done
>> +  done
>> +IFS=$as_save_IFS
>> +
>> +fi
>> +fi
>> +libc_cv_s390x_staticpie_req_LD=$ac_cv_prog_libc_cv_s390x_staticpie_req_LD
>> +if test -n "$libc_cv_s390x_staticpie_req_LD"; then
>> +  { printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $libc_cv_s390x_staticpie_req_LD" >&5
>> +printf "%s\n" "$libc_cv_s390x_staticpie_req_LD" >&6; }
>> +else
>> +  { printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: no" >&5
>> +printf "%s\n" "no" >&6; }
>> +fi
>> +
>> +
>> +  test -n "$libc_cv_s390x_staticpie_req_LD" && break
>> +done
>> +
>> +if test -z "$libc_cv_s390x_staticpie_req_LD"; then
>> +  ac_verc_fail=yes
>> +else
>> +  # Found it, now check the version.
>> +  { printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking version of $libc_cv_s390x_staticpie_req_LD" >&5
>> +printf %s "checking version of $libc_cv_s390x_staticpie_req_LD... " >&6; }
>> +  ac_prog_version=`$libc_cv_s390x_staticpie_req_LD --version 2>&1 | sed -n 's/^.*LLD.* \([0-9][0-9]*\.[0-9.]*\).*$/\1/p'`
>> +  case $ac_prog_version in
>> +    '') ac_prog_version="v. ?.??, bad"; ac_verc_fail=yes;;
>> +    1[8-9].*|[2-9][0-9].*)
>> +       ac_prog_version="$ac_prog_version, ok"; ac_verc_fail=no;;
>> +    *) ac_prog_version="$ac_prog_version, bad"; ac_verc_fail=yes;;
>> +
>> +  esac
>> +  { printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $ac_prog_version" >&5
>> +printf "%s\n" "$ac_prog_version" >&6; }
>> +fi
>> +if test $ac_verc_fail = yes; then
>> +  libc_cv_s390x_staticpie_req_version=no
>> +fi
>> +
>>      ;;
>>    *)
>>      # The required binutils patches are available with bintuils 2.39
>> diff --git a/sysdeps/s390/s390-64/configure.ac b/sysdeps/s390/s390-64/configure.ac
>> index 4657de0d37..0c1b276086 100644
>> --- a/sysdeps/s390/s390-64/configure.ac
>> +++ b/sysdeps/s390/s390-64/configure.ac
>> @@ -12,8 +12,20 @@ case $($LD --version) in
>>      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 lld patches are available with LLVM 18:
>> +    # - [lld] Add target support for SystemZ (s390x) #75643
>> +    # https://github.com/llvm/llvm-project/pull/75643
>> +    # 2024-02-13: https://github.com/llvm/llvm-project/commit/fe3406e349884e4ef61480dd0607f1e237102c74
>> +    # - [lld/ELF] Avoid unnecessary TPOFF relocations in GOT for -pie #81739
>> +    # https://github.com/llvm/llvm-project/pull/81739
>> +    # 2024-02-14: https://github.com/llvm/llvm-project/commit/6f907733e65d24edad65f763fb14402464bd578b
>> +    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,
>> +		      [LLD.* \([0-9][0-9]*\.[0-9.]*\)],
>> +		      [1[8-9].*|[2-9][0-9].*],
>> +		      libc_cv_s390x_staticpie_req_version=no)
>>      ;;
>>    *)
>>      # The required binutils patches are available with bintuils 2.39
> 
> 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.

> 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?
> 
> 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
> 
> diff --git a/sysdeps/s390/s390-64/configure.ac b/sysdeps/s390/s390-64/configure.ac
> index 4657de0d37..5117b424f3 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([! LC_ALL=C $READELF -Wr conftest | grep R_390_TLS_TPOFF])
> +     && AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -Wl,--no-dynamic-linker -pie -o conftest conftest1.o conftest2.o]) \
> +     && AC_TRY_COMMAND([! LC_ALL=C $READELF -Wr conftest | grep -E '\''R_390_TLS_DTPMOD|R_390_TLS_DTPOFF'\''])
>    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)