| Message ID | 20240221145610.962170-1-stli@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] S390: Always enable static PIE if build with lld. | expand |
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 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. 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. 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)
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)
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)
On 27.02.24 14:49, Adhemerval Zanella Netto wrote: > > > 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: > This solves the bootstrapping-case and the test is independent of used binutils/lld/gold linker. Then the version check is not needed anymore. As this is your effort, do you want to continue with this patch? Thanks a lot, Stefan > 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)
On 27/02/24 12:36, Stefan Liebler wrote: > On 27.02.24 14:49, Adhemerval Zanella Netto wrote: >> >> >> 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: >> > This solves the bootstrapping-case and the test is independent of used > binutils/lld/gold linker. Then the version check is not needed anymore. > > As this is your effort, do you want to continue with this patch? > I will send a update patch.
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