Re: [PATCH v2 3/3] elf: Enable TLS descriptor tests on aarch64

["H.J. Lu" <hjl.tools@gmail.com>]

On Tue, Mar 12, 2024 at 9:21 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
> The aarch64 uses 'trad' for traditional tls and 'desc' for tls
> descriptors, but unlike other targets it defaults to 'desc'.  The
> gnutls2 configure check does not set aarch64 as an ABI that uses
> TLS descriptors, which then disable somes stests.
>
> Also rename the internal machinery fron gnu2 to tls descriptors.
>
> Checked on aarch64-linux-gnu.
> ---
>  configure                    | 23 +++++++++++++----------
>  configure.ac                 | 15 +++++++++------
>  elf/Makefile                 | 26 +++++++++++++-------------
>  sysdeps/aarch64/preconfigure |  1 +
>  sysdeps/arm/Makefile         |  8 ++++----
>  5 files changed, 40 insertions(+), 33 deletions(-)
>
> diff --git a/configure b/configure
> index 117b48a421..432e40a592 100755
> --- a/configure
> +++ b/configure
> @@ -653,7 +653,7 @@ LIBGD
>  libc_cv_cc_loop_to_function
>  libc_cv_cc_submachine
>  libc_cv_cc_nofma
> -libc_cv_mtls_dialect_gnu2
> +libc_cv_mtls_descriptor
>  libc_cv_has_glob_dat
>  libc_cv_fpie
>  libc_cv_z_execstack
> @@ -4760,6 +4760,9 @@ libc_config_ok=no
>  # whether to use such directories.
>  with_fp_cond=1
>
> +# A preconfigure script may define another name to TLS descriptor variant
> +mtls_descriptor=gnu2
> +
>  if frags=`ls -d $srcdir/sysdeps/*/preconfigure 2> /dev/null`
>  then
>    { printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for sysdeps preconfigure fragments" >&5
> @@ -7006,9 +7009,9 @@ fi
>  printf "%s\n" "$libc_cv_has_glob_dat" >&6; }
>
>
> -{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for -mtls-dialect=gnu2" >&5
> -printf %s "checking for -mtls-dialect=gnu2... " >&6; }
> -if test ${libc_cv_mtls_dialect_gnu2+y}
> +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for tls descriptor support" >&5
> +printf %s "checking for tls descriptor support... " >&6; }
> +if test ${libc_cv_mtls_descriptor+y}
>  then :
>    printf %s "(cached) " >&6
>  else $as_nop
> @@ -7019,7 +7022,7 @@ void foo (void)
>    i = 10;
>  }
>  EOF
> -if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS -fPIC -mtls-dialect=gnu2 -nostdlib -nostartfiles
> +if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS -fPIC -mtls-dialect=$mtls_descriptor -nostdlib -nostartfiles
>                    -shared conftest.c -o conftest 1>&5'
>    { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
>    (eval $ac_try) 2>&5
> @@ -7027,17 +7030,17 @@ if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS -fPIC -mtls-dialect=gnu2 -nostdlib -nost
>    printf "%s\n" "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
>    test $ac_status = 0; }; }
>  then
> -  libc_cv_mtls_dialect_gnu2=yes
> +  libc_cv_mtls_descriptor=$mtls_descriptor
>  else
> -  libc_cv_mtls_dialect_gnu2=no
> +  libc_cv_mtls_descriptor=no
>  fi
>  rm -f conftest*
>  fi
> -{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $libc_cv_mtls_dialect_gnu2" >&5
> -printf "%s\n" "$libc_cv_mtls_dialect_gnu2" >&6; }
> +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $libc_cv_mtls_descriptor" >&5
> +printf "%s\n" "$libc_cv_mtls_descriptor" >&6; }
>
>  config_vars="$config_vars
> -have-mtls-dialect-gnu2 = $libc_cv_mtls_dialect_gnu2"
> +have-mtls-descriptor = $libc_cv_mtls_descriptor"
>
>  { printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking if -Wno-ignored-attributes is required for aliases" >&5
>  printf %s "checking if -Wno-ignored-attributes is required for aliases... " >&6; }
> diff --git a/configure.ac b/configure.ac
> index 19b88a47a5..bdc385d03c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -442,6 +442,9 @@ libc_config_ok=no
>  # whether to use such directories.
>  with_fp_cond=1
>
> +# A preconfigure script may define another name to TLS descriptor variant
> +mtls_descriptor=gnu2
> +
>  dnl Let sysdeps/*/preconfigure act here.
>  LIBC_PRECONFIGURE([$srcdir], [for sysdeps])
>
> @@ -1287,7 +1290,7 @@ fi
>  rm -f conftest*])
>  AC_SUBST(libc_cv_has_glob_dat)
>
> -AC_CACHE_CHECK([for -mtls-dialect=gnu2], libc_cv_mtls_dialect_gnu2,
> +AC_CACHE_CHECK([for tls descriptor support], libc_cv_mtls_descriptor,
>  [dnl
>  cat > conftest.c <<EOF
>  __thread int i;
> @@ -1296,16 +1299,16 @@ void foo (void)
>    i = 10;
>  }
>  EOF
> -if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS -fPIC -mtls-dialect=gnu2 -nostdlib -nostartfiles
> +if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS -fPIC -mtls-dialect=$mtls_descriptor -nostdlib -nostartfiles
>                    -shared conftest.c -o conftest 1>&AS_MESSAGE_LOG_FD])
>  then
> -  libc_cv_mtls_dialect_gnu2=yes
> +  libc_cv_mtls_descriptor=$mtls_descriptor
>  else
> -  libc_cv_mtls_dialect_gnu2=no
> +  libc_cv_mtls_descriptor=no
>  fi
>  rm -f conftest*])
> -AC_SUBST(libc_cv_mtls_dialect_gnu2)
> -LIBC_CONFIG_VAR([have-mtls-dialect-gnu2], [$libc_cv_mtls_dialect_gnu2])
> +AC_SUBST(libc_cv_mtls_descriptor)
> +LIBC_CONFIG_VAR([have-mtls-descriptor], [$libc_cv_mtls_descriptor])
>
>  dnl clang emits an warning for a double alias redirection, to warn the
>  dnl original symbol is sed even when weak definition overrides it.
> diff --git a/elf/Makefile b/elf/Makefile
> index 393a27ef2a..4f1903391a 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -1001,13 +1001,13 @@ modules-names-tests = $(filter-out ifuncmod% tst-tlsmod%,\
>  # For +depfiles in Makerules.
>  extra-test-objs += tst-auditmod17.os
>
> -ifeq (yes,$(have-mtls-dialect-gnu2))
> +ifneq (no,$(have-mtls-descriptor))
>  tests += tst-gnu2-tls1
>  modules-names += tst-gnu2-tls1mod
>  $(objpfx)tst-gnu2-tls1: $(objpfx)tst-gnu2-tls1mod.so
>  tst-gnu2-tls1mod.so-no-z-defs = yes
> -CFLAGS-tst-gnu2-tls1mod.c += -mtls-dialect=gnu2
> -endif # $(have-mtls-dialect-gnu2)
> +CFLAGS-tst-gnu2-tls1mod.c += -mtls-dialect=$(have-mtls-descriptor)
> +endif # $(have-mtls-descriptor)
>
>  ifeq (yes,$(have-protected-data))
>  modules-names += tst-protected1moda tst-protected1modb
> @@ -2975,11 +2975,11 @@ $(objpfx)tst-tls-allocation-failure-static-patched.out: \
>  $(objpfx)tst-audit-tlsdesc: $(objpfx)tst-audit-tlsdesc-mod1.so \
>                             $(objpfx)tst-audit-tlsdesc-mod2.so \
>                             $(shared-thread-library)
> -ifeq (yes,$(have-mtls-dialect-gnu2))
> +ifneq (no,$(have-mtls-descriptor))
>  # The test is valid for all TLS types, but we want to exercise GNU2
>  # TLS if possible.
> -CFLAGS-tst-audit-tlsdesc-mod1.c += -mtls-dialect=gnu2
> -CFLAGS-tst-audit-tlsdesc-mod2.c += -mtls-dialect=gnu2
> +CFLAGS-tst-audit-tlsdesc-mod1.c += -mtls-dialect=$(have-mtls-descriptor)
> +CFLAGS-tst-audit-tlsdesc-mod2.c += -mtls-dialect=$(have-mtls-descriptor)
>  endif
>  $(objpfx)tst-audit-tlsdesc-dlopen: $(shared-thread-library)
>  $(objpfx)tst-audit-tlsdesc-dlopen.out: $(objpfx)tst-audit-tlsdesc-mod1.so \
> @@ -3058,11 +3058,11 @@ $(objpfx)tst-gnu2-tls2.out: \
>    $(objpfx)tst-gnu2-tls2mod1.so \
>    $(objpfx)tst-gnu2-tls2mod2.so
>
> -ifeq (yes,$(have-mtls-dialect-gnu2))
> -CFLAGS-tst-tlsgap-mod0.c += -mtls-dialect=gnu2
> -CFLAGS-tst-tlsgap-mod1.c += -mtls-dialect=gnu2
> -CFLAGS-tst-tlsgap-mod2.c += -mtls-dialect=gnu2
> -CFLAGS-tst-gnu2-tls2mod0.c += -mtls-dialect=gnu2
> -CFLAGS-tst-gnu2-tls2mod1.c += -mtls-dialect=gnu2
> -CFLAGS-tst-gnu2-tls2mod2.c += -mtls-dialect=gnu2
> +ifneq (no,$(have-mtls-descriptor))
> +CFLAGS-tst-tlsgap-mod0.c += -mtls-dialect=$(have-mtls-descriptor)
> +CFLAGS-tst-tlsgap-mod1.c += -mtls-dialect=$(have-mtls-descriptor)
> +CFLAGS-tst-tlsgap-mod2.c += -mtls-dialect=$(have-mtls-descriptor)
> +CFLAGS-tst-gnu2-tls2mod0.c += -mtls-dialect=$(have-mtls-descriptor)
> +CFLAGS-tst-gnu2-tls2mod1.c += -mtls-dialect=$(have-mtls-descriptor)
> +CFLAGS-tst-gnu2-tls2mod2.c += -mtls-dialect=$(have-mtls-descriptor)
>  endif
> diff --git a/sysdeps/aarch64/preconfigure b/sysdeps/aarch64/preconfigure
> index d9bd1f8558..19657b627b 100644
> --- a/sysdeps/aarch64/preconfigure
> +++ b/sysdeps/aarch64/preconfigure
> @@ -2,5 +2,6 @@ case "$machine" in
>  aarch64*)
>         base_machine=aarch64
>         machine=aarch64
> +       mtls_descriptor=desc
>         ;;
>  esac
> diff --git a/sysdeps/arm/Makefile b/sysdeps/arm/Makefile
> index d5cea717a9..619474eca9 100644
> --- a/sysdeps/arm/Makefile
> +++ b/sysdeps/arm/Makefile
> @@ -13,15 +13,15 @@ $(objpfx)libgcc-stubs.a: $(objpfx)aeabi_unwind_cpp_pr1.os
>  lib-noranlib: $(objpfx)libgcc-stubs.a
>
>  ifeq ($(build-shared),yes)
> -ifeq (yes,$(have-mtls-dialect-gnu2))
> +ifneq (no,$(have-mtls-descriptor))
>  tests += tst-armtlsdescloc tst-armtlsdescextnow tst-armtlsdescextlazy
>  modules-names += tst-armtlsdesclocmod
>  modules-names += tst-armtlsdescextlazymod tst-armtlsdescextnowmod
>  CPPFLAGS-tst-armtlsdescextnowmod.c += -Dstatic=
>  CPPFLAGS-tst-armtlsdescextlazymod.c += -Dstatic=
> -CFLAGS-tst-armtlsdesclocmod.c += -mtls-dialect=gnu2
> -CFLAGS-tst-armtlsdescextnowmod.c += -mtls-dialect=gnu2
> -CFLAGS-tst-armtlsdescextlazymod.c += -mtls-dialect=gnu2
> +CFLAGS-tst-armtlsdesclocmod.c += -mtls-dialect=$(have-mtls-descriptor)
> +CFLAGS-tst-armtlsdescextnowmod.c += -mtls-dialect=$(have-mtls-descriptor)
> +CFLAGS-tst-armtlsdescextlazymod.c += -mtls-dialect=$(have-mtls-descriptor)
>  LDFLAGS-tst-armtlsdescextnowmod.so += -Wl,-z,now
>  tst-armtlsdescloc-ENV = LD_BIND_NOW=1
>  tst-armtlsdescextnow-ENV = LD_BIND_NOW=1
> --
> 2.34.1
>


LGTM.

Reviewed-by: H.J. Lu <hjl.tools@gmail.com>

Thanks.
-- 
H.J.