unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang
@ 2021-07-26  3:57 Fangrui Song via Libc-alpha
  2021-07-26  3:58 ` [PATCH 1/3] elf: Replace .tls_common with .tbss definition Fangrui Song via Libc-alpha
                   ` (5 more replies)
  0 siblings, 6 replies; 20+ messages in thread
From: Fangrui Song via Libc-alpha @ 2021-07-26  3:57 UTC (permalink / raw)
  To: libc-alpha; +Cc: Fangrui Song

The patches allow LLD 13.0.0 to build glibc. LLD's compatibility with
GNU ld is generally better than gold's compatibility with GNU ld.

The first two commits improve gold and clang compatibility as well.
(There is still a long way for clang to build glibc.)

About `make check` results:

I can't configure glibc --enable-static-pie with gold, so I use
--disable-static-pie with gold.

* gold (--disable-static-pie) has 160 FAIL.
* ld.bfd has 152 FAIL.
* ld.lld has 159 FAIL.

## I have investigated a few failures.

The tst-ifunc-isa-*.c failures are not ld.lld's fault.
The lld linked tst-ifunc-isa-* work with LD_BIND_NOW=1.
The tests happen to work with GNU ld because the IRELATIVE for foo_ifunc
is placed after JUMP_SLOT in .repa.plt.  The test needs to call
__x86_get_cpuid_feature_leaf which is defined in a different TU. IMHO
such ifunc does not guaranteed to work.

For gmon/tst-gmon-gprof*, ld.lld linked tst-gmon-gprof has a f3 line,
which appears more correct to me.  But the test considers it a failure.

% cat gmon/tst-gmon-gprof.out
--- expected
+++ actual
@@ -1,2 +1,3 @@
 f1 2000
 f2 1000
+f3 1
FAIL


For malloc/tst-compathooks-on,

    malloc/tst-compathooks-on: Symbol `__free_hook' has different size in shared object, consider re-linking

the root cause is that lld's symbol versioning is different from GNU ld in an unusal case:

    __asm__ (".symver " "__free_hook" "," "__free_hook" "@" "GLIBC_2.2.5");

This leaves two symbols __free_hook and __free_hook@GLIBC_2.2.5.
__free_hook is then attached a default version GLIBC_2.2.5.
I think malloc/malloc-debug.c uses a fragile versioned symbol here.
If the inline asm uses @@ the failure should go away.


In summary, I think the failed tests touch some dark corners of the toolchain.
These things do not really matter for real world applications.


Fangrui Song (3):
  elf: Replace .tls_common with .tbss definition
  elf: Skip tst-auditlogmod-* if the linker doesn't support --depaudit
  configure: Allow LD to be LLD 13.0.0 or above

 configure        | 111 +++++++++++++++++++++++++++++++++++++++++++++--
 configure.ac     |  24 +++++++---
 elf/Makefile     |   4 +-
 elf/tls-macros.h |   6 ++-
 4 files changed, 134 insertions(+), 11 deletions(-)

-- 
2.32.0.432.gabb21c7263-goog


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/3] elf: Replace .tls_common with .tbss definition
  2021-07-26  3:57 [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang Fangrui Song via Libc-alpha
@ 2021-07-26  3:58 ` Fangrui Song via Libc-alpha
  2021-07-29 14:26   ` H.J. Lu via Libc-alpha
  2021-07-26  3:58 ` [PATCH 2/3] elf: Skip tst-auditlogmod-* if the linker doesn't support --depaudit Fangrui Song via Libc-alpha
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Fangrui Song via Libc-alpha @ 2021-07-26  3:58 UTC (permalink / raw)
  To: libc-alpha; +Cc: Fangrui Song

.tls_common is obsoleted, not supported by clang -fintegrated-as or ld.lld.
Just change it to .tbss for portability.
---
 elf/tls-macros.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/elf/tls-macros.h b/elf/tls-macros.h
index e25e33b0f0..a637407417 100644
--- a/elf/tls-macros.h
+++ b/elf/tls-macros.h
@@ -1,7 +1,11 @@
 /* Macros to support TLS testing in times of missing compiler support.  */
 
 #define COMMON_INT_DEF(x) \
-  asm (".tls_common " #x ",4,4")
+  asm (".section .tbss\n\t" \
+       ".globl " #x "\n\t" \
+       ".balign 4\n\t" \
+       #x ":\t.space 4\n\t" \
+       ".previous")
 /* XXX Until we get compiler support we don't need declarations.  */
 #define COMMON_INT_DECL(x)
 
-- 
2.32.0.432.gabb21c7263-goog


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/3] elf: Skip tst-auditlogmod-* if the linker doesn't support --depaudit
  2021-07-26  3:57 [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang Fangrui Song via Libc-alpha
  2021-07-26  3:58 ` [PATCH 1/3] elf: Replace .tls_common with .tbss definition Fangrui Song via Libc-alpha
@ 2021-07-26  3:58 ` Fangrui Song via Libc-alpha
  2021-07-29 14:23   ` H.J. Lu via Libc-alpha
  2021-07-26  3:58 ` [PATCH 3/3] configure: Allow LD to be LLD 13.0.0 or above Fangrui Song via Libc-alpha
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Fangrui Song via Libc-alpha @ 2021-07-26  3:58 UTC (permalink / raw)
  To: libc-alpha; +Cc: Fangrui Song

gold and ld.lld do not support --audit or --depaudit.
---
 configure    | 34 ++++++++++++++++++++++++++++++++++
 configure.ac |  4 ++++
 elf/Makefile |  4 +++-
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 9619c10991..9b966196d4 100755
--- a/configure
+++ b/configure
@@ -5969,6 +5969,40 @@ $as_echo "$libc_linker_feature" >&6; }
 config_vars="$config_vars
 have-z-start-stop-gc = $libc_cv_z_start_stop_gc"
 
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --depaudit" >&5
+$as_echo_n "checking for linker that supports --depaudit... " >&6; }
+libc_linker_feature=no
+if test x"$gnu_ld" = x"yes"; then
+  libc_linker_check=`$LD -v --help 2>/dev/null | grep "\--depaudit"`
+  if test -n "$libc_linker_check"; then
+    cat > conftest.c <<EOF
+int _start (void) { return 42; }
+EOF
+    if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS $no_ssp
+				-Wl,--depaudit,x -nostdlib -nostartfiles
+				-fPIC -shared -o conftest.so conftest.c
+				1>&5'
+  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
+  (eval $ac_try) 2>&5
+  ac_status=$?
+  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+  test $ac_status = 0; }; }
+    then
+      libc_linker_feature=yes
+    fi
+    rm -f conftest*
+  fi
+fi
+if test $libc_linker_feature = yes; then
+  libc_cv_depaudit=yes
+else
+  libc_cv_depaudit=no
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_linker_feature" >&5
+$as_echo "$libc_linker_feature" >&6; }
+config_vars="$config_vars
+have-depaudit = $libc_cv_depaudit"
+
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --no-dynamic-linker" >&5
 $as_echo_n "checking for linker that supports --no-dynamic-linker... " >&6; }
 libc_linker_feature=no
diff --git a/configure.ac b/configure.ac
index 34ecbba540..17a4c9a1ab 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1337,6 +1337,10 @@ LIBC_LINKER_FEATURE([-z start-stop-gc], [-Wl,-z,start-stop-gc],
 		    [libc_cv_z_start_stop_gc=yes], [libc_cv_z_start_stop_gc=no])
 LIBC_CONFIG_VAR([have-z-start-stop-gc], [$libc_cv_z_start_stop_gc])
 
+LIBC_LINKER_FEATURE([--depaudit], [-Wl,--depaudit,x],
+		    [libc_cv_depaudit=yes], [libc_cv_depaudit=no])
+LIBC_CONFIG_VAR([have-depaudit], [$libc_cv_depaudit])
+
 LIBC_LINKER_FEATURE([--no-dynamic-linker],
 		    [-Wl,--no-dynamic-linker],
 		    [libc_cv_no_dynamic_linker=yes],
diff --git a/elf/Makefile b/elf/Makefile
index 87a70d6c7a..09f860a268 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -219,7 +219,6 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
 	 tst-dlopen-self tst-auditmany tst-initfinilazyfail tst-dlopenfail \
 	 tst-dlopenfail-2 \
 	 tst-filterobj tst-filterobj-dlopen tst-auxobj tst-auxobj-dlopen \
-	 tst-audit14 tst-audit15 tst-audit16 \
 	 tst-single_threaded tst-single_threaded-pthread \
 	 tst-tls-ie tst-tls-ie-dlmopen argv0test \
 	 tst-glibc-hwcaps tst-glibc-hwcaps-prepend tst-glibc-hwcaps-mask \
@@ -238,6 +237,9 @@ selinux-enabled := $(shell cat /selinux/enforce 2> /dev/null)
 ifneq ($(selinux-enabled),1)
 tests-execstack-yes = tst-execstack tst-execstack-needed tst-execstack-prog
 endif
+ifeq ($(have-depaudit),yes)
+tests += tst-audit14 tst-audit15 tst-audit16
+endif
 endif
 tests += $(tests-execstack-$(have-z-execstack))
 ifeq ($(run-built-tests),yes)
-- 
2.32.0.432.gabb21c7263-goog


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/3] configure: Allow LD to be LLD 13.0.0 or above
  2021-07-26  3:57 [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang Fangrui Song via Libc-alpha
  2021-07-26  3:58 ` [PATCH 1/3] elf: Replace .tls_common with .tbss definition Fangrui Song via Libc-alpha
  2021-07-26  3:58 ` [PATCH 2/3] elf: Skip tst-auditlogmod-* if the linker doesn't support --depaudit Fangrui Song via Libc-alpha
@ 2021-07-26  3:58 ` Fangrui Song via Libc-alpha
  2021-07-28 18:02 ` [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang H.J. Lu via Libc-alpha
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: Fangrui Song via Libc-alpha @ 2021-07-26  3:58 UTC (permalink / raw)
  To: libc-alpha; +Cc: Fangrui Song

When using LLD (LLVM linker) as the linker, configure prints a confusing
message.

    *** These critical programs are missing or too old: GNU ld

LLD>=13.0.0 can build glibc --enable-static-pie. (8.0.0 needs one
workaround for -Wl,-defsym=_begin=0. 9.0.0 works with
--disable-static-pie).
---
 configure    | 77 +++++++++++++++++++++++++++++++++++++++++++++++++---
 configure.ac | 20 ++++++++++----
 2 files changed, 88 insertions(+), 9 deletions(-)

diff --git a/configure b/configure
index 9b966196d4..050f1a29cf 100755
--- a/configure
+++ b/configure
@@ -4664,9 +4664,10 @@ if test $ac_verc_fail = yes; then
 fi
 
 
-if test -n "`$LD --version | sed -n 's/^GNU \(gold\).*$/\1/p'`"; then
+case $($LD --version) in
+  "GNU gold"*)
   # Accept gold 1.14 or higher
-  for ac_prog in $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
@@ -4729,8 +4730,75 @@ if test $ac_verc_fail = yes; then
   LD=: critic_missing="$critic_missing GNU gold"
 fi
 
+    ;;
+  "LLD"*)
+  # Accept LLD 13.0.0 or higher
+    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
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
+$as_echo_n "checking for $ac_word... " >&6; }
+if ${ac_cv_prog_LD+:} false; then :
+  $as_echo_n "(cached) " >&6
 else
-  for ac_prog in $LD
+  if test -n "$LD"; then
+  ac_cv_prog_LD="$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
+  test -z "$as_dir" && as_dir=.
+    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_LD="$ac_prog"
+    $as_echo "$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
+LD=$ac_cv_prog_LD
+if test -n "$LD"; then
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $LD" >&5
+$as_echo "$LD" >&6; }
+else
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+fi
+
+
+  test -n "$LD" && break
+done
+
+if test -z "$LD"; then
+  ac_verc_fail=yes
+else
+  # Found it, now check the version.
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking version of $LD" >&5
+$as_echo_n "checking version of $LD... " >&6; }
+  ac_prog_version=`$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[3-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
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_prog_version" >&5
+$as_echo "$ac_prog_version" >&6; }
+fi
+if test $ac_verc_fail = yes; then
+  LD=: critic_missing="$critic_missing LLD"
+fi
+
+    ;;
+  *)
+    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
@@ -4793,7 +4861,8 @@ if test $ac_verc_fail = yes; then
   LD=: critic_missing="$critic_missing GNU ld"
 fi
 
-fi
+    ;;
+esac
 
 # These programs are version sensitive.
 for ac_prog in gnumake gmake make
diff --git a/configure.ac b/configure.ac
index 17a4c9a1ab..5632277f9c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -995,18 +995,28 @@ AC_CHECK_PROG_VER(AS, $AS, --version,
 		  [2.1[0-9][0-9]*|2.2[5-9]*|2.[3-9][0-9]*|[3-9].*|[1-9][0-9]*],
 		  AS=: critic_missing="$critic_missing as")
 
-if test -n "`$LD --version | sed -n 's/^GNU \(gold\).*$/\1/p'`"; then
+case $($LD --version) in
+  "GNU gold"*)
   # Accept gold 1.14 or higher
-  AC_CHECK_PROG_VER(LD, $LD, --version,
+    AC_CHECK_PROG_VER(LD, $LD, --version,
 		    [GNU gold.* \([0-9][0-9]*\.[0-9.]*\)],
 		    [1.1[4-9]*|1.[2-9][0-9]*|1.1[0-9][0-9]*|[2-9].*|[1-9][0-9]*],
 		    LD=: critic_missing="$critic_missing GNU gold")
-else
-  AC_CHECK_PROG_VER(LD, $LD, --version,
+    ;;
+  "LLD"*)
+  # Accept LLD 13.0.0 or higher
+    AC_CHECK_PROG_VER(LD, $LD, --version,
+		    [LLD.* \([0-9][0-9]*\.[0-9.]*\)],
+		    [1[3-9].*|[2-9][0-9].*],
+		    LD=: critic_missing="$critic_missing LLD")
+    ;;
+  *)
+    AC_CHECK_PROG_VER(LD, $LD, --version,
 		    [GNU ld.* \([0-9][0-9]*\.[0-9.]*\)],
 		    [2.1[0-9][0-9]*|2.2[5-9]*|2.[3-9][0-9]*|[3-9].*|[1-9][0-9]*],
 		    LD=: critic_missing="$critic_missing GNU ld")
-fi
+    ;;
+esac
 
 # These programs are version sensitive.
 AC_CHECK_PROG_VER(MAKE, gnumake gmake make, --version,
-- 
2.32.0.432.gabb21c7263-goog


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang
  2021-07-26  3:57 [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang Fangrui Song via Libc-alpha
                   ` (2 preceding siblings ...)
  2021-07-26  3:58 ` [PATCH 3/3] configure: Allow LD to be LLD 13.0.0 or above Fangrui Song via Libc-alpha
@ 2021-07-28 18:02 ` H.J. Lu via Libc-alpha
  2021-07-28 21:52   ` Fangrui Song via Libc-alpha
  2021-07-30  7:57 ` Florian Weimer via Libc-alpha
  2021-08-02  4:02 ` Siddhesh Poyarekar
  5 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu via Libc-alpha @ 2021-07-28 18:02 UTC (permalink / raw)
  To: Fangrui Song; +Cc: GNU C Library

On Sun, Jul 25, 2021 at 8:58 PM Fangrui Song via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> The patches allow LLD 13.0.0 to build glibc. LLD's compatibility with
> GNU ld is generally better than gold's compatibility with GNU ld.
>
> The first two commits improve gold and clang compatibility as well.
> (There is still a long way for clang to build glibc.)
>
> About `make check` results:
>
> I can't configure glibc --enable-static-pie with gold, so I use
> --disable-static-pie with gold.
>
> * gold (--disable-static-pie) has 160 FAIL.
> * ld.bfd has 152 FAIL.
> * ld.lld has 159 FAIL.
>
> ## I have investigated a few failures.
>
> The tst-ifunc-isa-*.c failures are not ld.lld's fault.
> The lld linked tst-ifunc-isa-* work with LD_BIND_NOW=1.
> The tests happen to work with GNU ld because the IRELATIVE for foo_ifunc
> is placed after JUMP_SLOT in .repa.plt.  The test needs to call
> __x86_get_cpuid_feature_leaf which is defined in a different TU. IMHO
> such ifunc does not guaranteed to work.
>
> For gmon/tst-gmon-gprof*, ld.lld linked tst-gmon-gprof has a f3 line,
> which appears more correct to me.  But the test considers it a failure.
>
> % cat gmon/tst-gmon-gprof.out
> --- expected
> +++ actual
> @@ -1,2 +1,3 @@
>  f1 2000
>  f2 1000
> +f3 1
> FAIL
>
>
> For malloc/tst-compathooks-on,
>
>     malloc/tst-compathooks-on: Symbol `__free_hook' has different size in shared object, consider re-linking
>
> the root cause is that lld's symbol versioning is different from GNU ld in an unusal case:
>
>     __asm__ (".symver " "__free_hook" "," "__free_hook" "@" "GLIBC_2.2.5");
>
> This leaves two symbols __free_hook and __free_hook@GLIBC_2.2.5.
> __free_hook is then attached a default version GLIBC_2.2.5.
> I think malloc/malloc-debug.c uses a fragile versioned symbol here.
> If the inline asm uses @@ the failure should go away.
>
>
> In summary, I think the failed tests touch some dark corners of the toolchain.
> These things do not really matter for real world applications.
>

Please open a separate bug for each issue you are trying to fix and reference
the bug in your patch.

Thanks.

-- 
H.J.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang
  2021-07-28 18:02 ` [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang H.J. Lu via Libc-alpha
@ 2021-07-28 21:52   ` Fangrui Song via Libc-alpha
  2021-07-29 14:45     ` H.J. Lu via Libc-alpha
  0 siblings, 1 reply; 20+ messages in thread
From: Fangrui Song via Libc-alpha @ 2021-07-28 21:52 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GNU C Library

On 2021-07-28, H.J. Lu wrote:
>On Sun, Jul 25, 2021 at 8:58 PM Fangrui Song via Libc-alpha
><libc-alpha@sourceware.org> wrote:
>>
>> The patches allow LLD 13.0.0 to build glibc. LLD's compatibility with
>> GNU ld is generally better than gold's compatibility with GNU ld.
>>
>> The first two commits improve gold and clang compatibility as well.
>> (There is still a long way for clang to build glibc.)
>>
>> About `make check` results:
>>
>> I can't configure glibc --enable-static-pie with gold, so I use
>> --disable-static-pie with gold.
>>
>> * gold (--disable-static-pie) has 160 FAIL.
>> * ld.bfd has 152 FAIL.
>> * ld.lld has 159 FAIL.
>>
>> ## I have investigated a few failures.
>>
>> The tst-ifunc-isa-*.c failures are not ld.lld's fault.
>> The lld linked tst-ifunc-isa-* work with LD_BIND_NOW=1.
>> The tests happen to work with GNU ld because the IRELATIVE for foo_ifunc
>> is placed after JUMP_SLOT in .repa.plt.  The test needs to call
>> __x86_get_cpuid_feature_leaf which is defined in a different TU. IMHO
>> such ifunc does not guaranteed to work.
>>
>> For gmon/tst-gmon-gprof*, ld.lld linked tst-gmon-gprof has a f3 line,
>> which appears more correct to me.  But the test considers it a failure.
>>
>> % cat gmon/tst-gmon-gprof.out
>> --- expected
>> +++ actual
>> @@ -1,2 +1,3 @@
>>  f1 2000
>>  f2 1000
>> +f3 1
>> FAIL
>>
>>
>> For malloc/tst-compathooks-on,
>>
>>     malloc/tst-compathooks-on: Symbol `__free_hook' has different size in shared object, consider re-linking
>>
>> the root cause is that lld's symbol versioning is different from GNU ld in an unusal case:
>>
>>     __asm__ (".symver " "__free_hook" "," "__free_hook" "@" "GLIBC_2.2.5");
>>
>> This leaves two symbols __free_hook and __free_hook@GLIBC_2.2.5.
>> __free_hook is then attached a default version GLIBC_2.2.5.
>> I think malloc/malloc-debug.c uses a fragile versioned symbol here.
>> If the inline asm uses @@ the failure should go away.
>>
>>
>> In summary, I think the failed tests touch some dark corners of the toolchain.
>> These things do not really matter for real world applications.
>>
>
>Please open a separate bug for each issue you are trying to fix and reference
>the bug in your patch.
>
>Thanks.

I filed:

https://sourceware.org/bugzilla/show_bug.cgi?id=28151 elf: Some tst-audit* tests need ld --audit and --depaudit which do not work with gold or ld.lld
https://sourceware.org/bugzilla/show_bug.cgi?id=28152 elf: clang integrated assembler and ld.lld do not support .tls_common
https://sourceware.org/bugzilla/show_bug.cgi?id=28153 [test] gmon/tst-gmon-gprof* may have a f3 line when built with ld.lld
https://sourceware.org/bugzilla/show_bug.cgi?id=28154 [test] sysdeps/x86/tst-ifunc-isa-* lazy binding failure with ld.lld

If there is a need to amend patches, I'll attach the BZ numbers.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/3] elf: Skip tst-auditlogmod-* if the linker doesn't support --depaudit
  2021-07-26  3:58 ` [PATCH 2/3] elf: Skip tst-auditlogmod-* if the linker doesn't support --depaudit Fangrui Song via Libc-alpha
@ 2021-07-29 14:23   ` H.J. Lu via Libc-alpha
  2021-07-29 16:28     ` Fangrui Song via Libc-alpha
  0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu via Libc-alpha @ 2021-07-29 14:23 UTC (permalink / raw)
  To: Fangrui Song; +Cc: GNU C Library

On Sun, Jul 25, 2021 at 9:00 PM Fangrui Song via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> gold and ld.lld do not support --audit or --depaudit.

Please update the commit log for BZ #28151.  We are in code freeze for glibc
2.34.   Please rebase and submit the v2 patch after glibc 2.34 has been
branched.

Thanks.

> ---
>  configure    | 34 ++++++++++++++++++++++++++++++++++
>  configure.ac |  4 ++++
>  elf/Makefile |  4 +++-
>  3 files changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index 9619c10991..9b966196d4 100755
> --- a/configure
> +++ b/configure
> @@ -5969,6 +5969,40 @@ $as_echo "$libc_linker_feature" >&6; }
>  config_vars="$config_vars
>  have-z-start-stop-gc = $libc_cv_z_start_stop_gc"
>
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --depaudit" >&5
> +$as_echo_n "checking for linker that supports --depaudit... " >&6; }
> +libc_linker_feature=no
> +if test x"$gnu_ld" = x"yes"; then
> +  libc_linker_check=`$LD -v --help 2>/dev/null | grep "\--depaudit"`
> +  if test -n "$libc_linker_check"; then
> +    cat > conftest.c <<EOF
> +int _start (void) { return 42; }
> +EOF
> +    if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS $no_ssp
> +                               -Wl,--depaudit,x -nostdlib -nostartfiles
> +                               -fPIC -shared -o conftest.so conftest.c
> +                               1>&5'
> +  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> +  (eval $ac_try) 2>&5
> +  ac_status=$?
> +  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> +  test $ac_status = 0; }; }
> +    then
> +      libc_linker_feature=yes
> +    fi
> +    rm -f conftest*
> +  fi
> +fi
> +if test $libc_linker_feature = yes; then
> +  libc_cv_depaudit=yes
> +else
> +  libc_cv_depaudit=no
> +fi
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_linker_feature" >&5
> +$as_echo "$libc_linker_feature" >&6; }
> +config_vars="$config_vars
> +have-depaudit = $libc_cv_depaudit"
> +
>  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --no-dynamic-linker" >&5
>  $as_echo_n "checking for linker that supports --no-dynamic-linker... " >&6; }
>  libc_linker_feature=no
> diff --git a/configure.ac b/configure.ac
> index 34ecbba540..17a4c9a1ab 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1337,6 +1337,10 @@ LIBC_LINKER_FEATURE([-z start-stop-gc], [-Wl,-z,start-stop-gc],
>                     [libc_cv_z_start_stop_gc=yes], [libc_cv_z_start_stop_gc=no])
>  LIBC_CONFIG_VAR([have-z-start-stop-gc], [$libc_cv_z_start_stop_gc])
>
> +LIBC_LINKER_FEATURE([--depaudit], [-Wl,--depaudit,x],
> +                   [libc_cv_depaudit=yes], [libc_cv_depaudit=no])
> +LIBC_CONFIG_VAR([have-depaudit], [$libc_cv_depaudit])
> +
>  LIBC_LINKER_FEATURE([--no-dynamic-linker],
>                     [-Wl,--no-dynamic-linker],
>                     [libc_cv_no_dynamic_linker=yes],
> diff --git a/elf/Makefile b/elf/Makefile
> index 87a70d6c7a..09f860a268 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -219,7 +219,6 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
>          tst-dlopen-self tst-auditmany tst-initfinilazyfail tst-dlopenfail \
>          tst-dlopenfail-2 \
>          tst-filterobj tst-filterobj-dlopen tst-auxobj tst-auxobj-dlopen \
> -        tst-audit14 tst-audit15 tst-audit16 \
>          tst-single_threaded tst-single_threaded-pthread \
>          tst-tls-ie tst-tls-ie-dlmopen argv0test \
>          tst-glibc-hwcaps tst-glibc-hwcaps-prepend tst-glibc-hwcaps-mask \
> @@ -238,6 +237,9 @@ selinux-enabled := $(shell cat /selinux/enforce 2> /dev/null)
>  ifneq ($(selinux-enabled),1)
>  tests-execstack-yes = tst-execstack tst-execstack-needed tst-execstack-prog
>  endif
> +ifeq ($(have-depaudit),yes)
> +tests += tst-audit14 tst-audit15 tst-audit16
> +endif
>  endif
>  tests += $(tests-execstack-$(have-z-execstack))
>  ifeq ($(run-built-tests),yes)
> --
> 2.32.0.432.gabb21c7263-goog
>


-- 
H.J.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/3] elf: Replace .tls_common with .tbss definition
  2021-07-26  3:58 ` [PATCH 1/3] elf: Replace .tls_common with .tbss definition Fangrui Song via Libc-alpha
@ 2021-07-29 14:26   ` H.J. Lu via Libc-alpha
  2021-07-29 16:21     ` Fangrui Song via Libc-alpha
  0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu via Libc-alpha @ 2021-07-29 14:26 UTC (permalink / raw)
  To: Fangrui Song; +Cc: GNU C Library

On Sun, Jul 25, 2021 at 8:59 PM Fangrui Song via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> .tls_common is obsoleted, not supported by clang -fintegrated-as or ld.lld.
> Just change it to .tbss for portability.

Please update the commit log for BZ #28152

>
> ---
>  elf/tls-macros.h | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/elf/tls-macros.h b/elf/tls-macros.h
> index e25e33b0f0..a637407417 100644
> --- a/elf/tls-macros.h
> +++ b/elf/tls-macros.h
> @@ -1,7 +1,11 @@
>  /* Macros to support TLS testing in times of missing compiler support.  */
>
>  #define COMMON_INT_DEF(x) \
> -  asm (".tls_common " #x ",4,4")
> +  asm (".section .tbss\n\t" \
> +       ".globl " #x "\n\t" \
> +       ".balign 4\n\t" \
> +       #x ":\t.space 4\n\t" \
> +       ".previous")
>  /* XXX Until we get compiler support we don't need declarations.  */
>  #define COMMON_INT_DECL(x)
>
> --
> 2.32.0.432.gabb21c7263-goog
>


-- 
H.J.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang
  2021-07-28 21:52   ` Fangrui Song via Libc-alpha
@ 2021-07-29 14:45     ` H.J. Lu via Libc-alpha
  0 siblings, 0 replies; 20+ messages in thread
From: H.J. Lu via Libc-alpha @ 2021-07-29 14:45 UTC (permalink / raw)
  To: Fangrui Song; +Cc: GNU C Library

On Wed, Jul 28, 2021 at 2:52 PM Fangrui Song <maskray@google.com> wrote:
>
> On 2021-07-28, H.J. Lu wrote:
> >On Sun, Jul 25, 2021 at 8:58 PM Fangrui Song via Libc-alpha
> ><libc-alpha@sourceware.org> wrote:
> >>
> >> The patches allow LLD 13.0.0 to build glibc. LLD's compatibility with
> >> GNU ld is generally better than gold's compatibility with GNU ld.
> >>
> >> The first two commits improve gold and clang compatibility as well.
> >> (There is still a long way for clang to build glibc.)
> >>
> >> About `make check` results:
> >>
> >> I can't configure glibc --enable-static-pie with gold, so I use
> >> --disable-static-pie with gold.
> >>
> >> * gold (--disable-static-pie) has 160 FAIL.
> >> * ld.bfd has 152 FAIL.

There should be no unexpected failures on x86 with ld.bfd.   If
it isn't the case for you, you should open a binutils bug.

> >> * ld.lld has 159 FAIL.
> >>
> >> ## I have investigated a few failures.
> >>
> >> The tst-ifunc-isa-*.c failures are not ld.lld's fault.
> >> The lld linked tst-ifunc-isa-* work with LD_BIND_NOW=1.
> >> The tests happen to work with GNU ld because the IRELATIVE for foo_ifunc
> >> is placed after JUMP_SLOT in .repa.plt.  The test needs to call
> >> __x86_get_cpuid_feature_leaf which is defined in a different TU. IMHO
> >> such ifunc does not guaranteed to work.
> >>
> >> For gmon/tst-gmon-gprof*, ld.lld linked tst-gmon-gprof has a f3 line,
> >> which appears more correct to me.  But the test considers it a failure.
> >>
> >> % cat gmon/tst-gmon-gprof.out
> >> --- expected
> >> +++ actual
> >> @@ -1,2 +1,3 @@
> >>  f1 2000
> >>  f2 1000
> >> +f3 1
> >> FAIL
> >>
> >>
> >> For malloc/tst-compathooks-on,
> >>
> >>     malloc/tst-compathooks-on: Symbol `__free_hook' has different size in shared object, consider re-linking
> >>
> >> the root cause is that lld's symbol versioning is different from GNU ld in an unusal case:
> >>
> >>     __asm__ (".symver " "__free_hook" "," "__free_hook" "@" "GLIBC_2.2.5");
> >>
> >> This leaves two symbols __free_hook and __free_hook@GLIBC_2.2.5.
> >> __free_hook is then attached a default version GLIBC_2.2.5.
> >> I think malloc/malloc-debug.c uses a fragile versioned symbol here.
> >> If the inline asm uses @@ the failure should go away.
> >>
> >>
> >> In summary, I think the failed tests touch some dark corners of the toolchain.

Please open a separate bug for each distinct failed test with lld.

Thanks.

> >> These things do not really matter for real world applications.
> >>
> >
> >Please open a separate bug for each issue you are trying to fix and reference
> >the bug in your patch.
> >
> >Thanks.
>
> I filed:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=28151 elf: Some tst-audit* tests need ld --audit and --depaudit which do not work with gold or ld.lld
> https://sourceware.org/bugzilla/show_bug.cgi?id=28152 elf: clang integrated assembler and ld.lld do not support .tls_common
> https://sourceware.org/bugzilla/show_bug.cgi?id=28153 [test] gmon/tst-gmon-gprof* may have a f3 line when built with ld.lld
> https://sourceware.org/bugzilla/show_bug.cgi?id=28154 [test] sysdeps/x86/tst-ifunc-isa-* lazy binding failure with ld.lld
>
> If there is a need to amend patches, I'll attach the BZ numbers.



-- 
H.J.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/3] elf: Replace .tls_common with .tbss definition
  2021-07-29 14:26   ` H.J. Lu via Libc-alpha
@ 2021-07-29 16:21     ` Fangrui Song via Libc-alpha
  0 siblings, 0 replies; 20+ messages in thread
From: Fangrui Song via Libc-alpha @ 2021-07-29 16:21 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GNU C Library

On 2021-07-29, H.J. Lu wrote:
>On Sun, Jul 25, 2021 at 8:59 PM Fangrui Song via Libc-alpha
><libc-alpha@sourceware.org> wrote:
>>
>> .tls_common is obsoleted, not supported by clang -fintegrated-as or ld.lld.
>> Just change it to .tbss for portability.
>
>Please update the commit log for BZ #28152

I will be mindful when committing.

>>
>> ---
>>  elf/tls-macros.h | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/elf/tls-macros.h b/elf/tls-macros.h
>> index e25e33b0f0..a637407417 100644
>> --- a/elf/tls-macros.h
>> +++ b/elf/tls-macros.h
>> @@ -1,7 +1,11 @@
>>  /* Macros to support TLS testing in times of missing compiler support.  */
>>
>>  #define COMMON_INT_DEF(x) \
>> -  asm (".tls_common " #x ",4,4")
>> +  asm (".section .tbss\n\t" \
>> +       ".globl " #x "\n\t" \
>> +       ".balign 4\n\t" \
>> +       #x ":\t.space 4\n\t" \
>> +       ".previous")
>>  /* XXX Until we get compiler support we don't need declarations.  */
>>  #define COMMON_INT_DECL(x)
>>
>> --
>> 2.32.0.432.gabb21c7263-goog
>>
>
>
>-- 
>H.J.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/3] elf: Skip tst-auditlogmod-* if the linker doesn't support --depaudit
  2021-07-29 14:23   ` H.J. Lu via Libc-alpha
@ 2021-07-29 16:28     ` Fangrui Song via Libc-alpha
  0 siblings, 0 replies; 20+ messages in thread
From: Fangrui Song via Libc-alpha @ 2021-07-29 16:28 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GNU C Library

On 2021-07-29, H.J. Lu wrote:
>On Sun, Jul 25, 2021 at 9:00 PM Fangrui Song via Libc-alpha
><libc-alpha@sourceware.org> wrote:
>>
>> gold and ld.lld do not support --audit or --depaudit.
>
>Please update the commit log for BZ #28151.  We are in code freeze for glibc
>2.34.   Please rebase and submit the v2 patch after glibc 2.34 has been
>branched.

Will mail v2 after presumably this

"The current development version of glibc 2.34, releasing on or around August 1st, 2021."

>Thanks.
>
>> ---
>>  configure    | 34 ++++++++++++++++++++++++++++++++++
>>  configure.ac |  4 ++++
>>  elf/Makefile |  4 +++-
>>  3 files changed, 41 insertions(+), 1 deletion(-)
>>
>> diff --git a/configure b/configure
>> index 9619c10991..9b966196d4 100755
>> --- a/configure
>> +++ b/configure
>> @@ -5969,6 +5969,40 @@ $as_echo "$libc_linker_feature" >&6; }
>>  config_vars="$config_vars
>>  have-z-start-stop-gc = $libc_cv_z_start_stop_gc"
>>
>> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --depaudit" >&5
>> +$as_echo_n "checking for linker that supports --depaudit... " >&6; }
>> +libc_linker_feature=no
>> +if test x"$gnu_ld" = x"yes"; then
>> +  libc_linker_check=`$LD -v --help 2>/dev/null | grep "\--depaudit"`
>> +  if test -n "$libc_linker_check"; then
>> +    cat > conftest.c <<EOF
>> +int _start (void) { return 42; }
>> +EOF
>> +    if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS $no_ssp
>> +                               -Wl,--depaudit,x -nostdlib -nostartfiles
>> +                               -fPIC -shared -o conftest.so conftest.c
>> +                               1>&5'
>> +  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
>> +  (eval $ac_try) 2>&5
>> +  ac_status=$?
>> +  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
>> +  test $ac_status = 0; }; }
>> +    then
>> +      libc_linker_feature=yes
>> +    fi
>> +    rm -f conftest*
>> +  fi
>> +fi
>> +if test $libc_linker_feature = yes; then
>> +  libc_cv_depaudit=yes
>> +else
>> +  libc_cv_depaudit=no
>> +fi
>> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_linker_feature" >&5
>> +$as_echo "$libc_linker_feature" >&6; }
>> +config_vars="$config_vars
>> +have-depaudit = $libc_cv_depaudit"
>> +
>>  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --no-dynamic-linker" >&5
>>  $as_echo_n "checking for linker that supports --no-dynamic-linker... " >&6; }
>>  libc_linker_feature=no
>> diff --git a/configure.ac b/configure.ac
>> index 34ecbba540..17a4c9a1ab 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -1337,6 +1337,10 @@ LIBC_LINKER_FEATURE([-z start-stop-gc], [-Wl,-z,start-stop-gc],
>>                     [libc_cv_z_start_stop_gc=yes], [libc_cv_z_start_stop_gc=no])
>>  LIBC_CONFIG_VAR([have-z-start-stop-gc], [$libc_cv_z_start_stop_gc])
>>
>> +LIBC_LINKER_FEATURE([--depaudit], [-Wl,--depaudit,x],
>> +                   [libc_cv_depaudit=yes], [libc_cv_depaudit=no])
>> +LIBC_CONFIG_VAR([have-depaudit], [$libc_cv_depaudit])
>> +
>>  LIBC_LINKER_FEATURE([--no-dynamic-linker],
>>                     [-Wl,--no-dynamic-linker],
>>                     [libc_cv_no_dynamic_linker=yes],
>> diff --git a/elf/Makefile b/elf/Makefile
>> index 87a70d6c7a..09f860a268 100644
>> --- a/elf/Makefile
>> +++ b/elf/Makefile
>> @@ -219,7 +219,6 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
>>          tst-dlopen-self tst-auditmany tst-initfinilazyfail tst-dlopenfail \
>>          tst-dlopenfail-2 \
>>          tst-filterobj tst-filterobj-dlopen tst-auxobj tst-auxobj-dlopen \
>> -        tst-audit14 tst-audit15 tst-audit16 \
>>          tst-single_threaded tst-single_threaded-pthread \
>>          tst-tls-ie tst-tls-ie-dlmopen argv0test \
>>          tst-glibc-hwcaps tst-glibc-hwcaps-prepend tst-glibc-hwcaps-mask \
>> @@ -238,6 +237,9 @@ selinux-enabled := $(shell cat /selinux/enforce 2> /dev/null)
>>  ifneq ($(selinux-enabled),1)
>>  tests-execstack-yes = tst-execstack tst-execstack-needed tst-execstack-prog
>>  endif
>> +ifeq ($(have-depaudit),yes)
>> +tests += tst-audit14 tst-audit15 tst-audit16
>> +endif
>>  endif
>>  tests += $(tests-execstack-$(have-z-execstack))
>>  ifeq ($(run-built-tests),yes)
>> --
>> 2.32.0.432.gabb21c7263-goog
>>
>
>
>-- 
>H.J.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang
  2021-07-26  3:57 [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang Fangrui Song via Libc-alpha
                   ` (3 preceding siblings ...)
  2021-07-28 18:02 ` [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang H.J. Lu via Libc-alpha
@ 2021-07-30  7:57 ` Florian Weimer via Libc-alpha
  2021-07-31  6:34   ` Fāng-ruì Sòng via Libc-alpha
  2021-08-02  4:02 ` Siddhesh Poyarekar
  5 siblings, 1 reply; 20+ messages in thread
From: Florian Weimer via Libc-alpha @ 2021-07-30  7:57 UTC (permalink / raw)
  To: Fangrui Song via Libc-alpha; +Cc: Fangrui Song

* Fangrui Song via Libc-alpha:

> For malloc/tst-compathooks-on,
>
>     malloc/tst-compathooks-on: Symbol `__free_hook' has different size in shared object, consider re-linking
>
> the root cause is that lld's symbol versioning is different from GNU ld in an unusal case:
>
>     __asm__ (".symver " "__free_hook" "," "__free_hook" "@" "GLIBC_2.2.5");
>
> This leaves two symbols __free_hook and __free_hook@GLIBC_2.2.5.
> __free_hook is then attached a default version GLIBC_2.2.5.
> I think malloc/malloc-debug.c uses a fragile versioned symbol here.
> If the inline asm uses @@ the failure should go away.

But we want to produce a compat symbol here.  With the current version
scripts, BFD ld will not export a symbol unless it is listed in the
version script.  That is, if I remove __free_hook from libc_malloc_debug
in malloc/Versions, I get an ABI check failure:

--- ../sysdeps/unix/sysv/linux/x86_64/64/libc_malloc_debug.abilist      2021-07-27 16:14:51.516781791 +0200
+++ …/malloc/libc_malloc_debug.symlist  2021-07-30 09:55:09.818875449 +0200
@@ -3 +2,0 @@ GLIBC_2.16 aligned_alloc F
-GLIBC_2.2.5 __free_hook D 0x8

If this works with a linker, it appears to ignore “local: *;” in version
nodes for versioned symbols.  That looks like a linker bug.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang
  2021-07-30  7:57 ` Florian Weimer via Libc-alpha
@ 2021-07-31  6:34   ` Fāng-ruì Sòng via Libc-alpha
  2021-07-31  6:41     ` Florian Weimer via Libc-alpha
  2021-08-02 20:55     ` Fāng-ruì Sòng via Libc-alpha
  0 siblings, 2 replies; 20+ messages in thread
From: Fāng-ruì Sòng via Libc-alpha @ 2021-07-31  6:34 UTC (permalink / raw)
  To: Florian Weimer; +Cc: Fangrui Song via Libc-alpha

On Fri, Jul 30, 2021 at 12:57 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Fangrui Song via Libc-alpha:
>
> > For malloc/tst-compathooks-on,
> >
> >     malloc/tst-compathooks-on: Symbol `__free_hook' has different size in shared object, consider re-linking
> >
> > the root cause is that lld's symbol versioning is different from GNU ld in an unusal case:
> >
> >     __asm__ (".symver " "__free_hook" "," "__free_hook" "@" "GLIBC_2.2.5");
> >
> > This leaves two symbols __free_hook and __free_hook@GLIBC_2.2.5.
> > __free_hook is then attached a default version GLIBC_2.2.5.
> > I think malloc/malloc-debug.c uses a fragile versioned symbol here.
> > If the inline asm uses @@ the failure should go away.
>
> But we want to produce a compat symbol here.  With the current version
> scripts, BFD ld will not export a symbol unless it is listed in the
> version script.  That is, if I remove __free_hook from libc_malloc_debug
> in malloc/Versions, I get an ABI check failure:
>
> --- ../sysdeps/unix/sysv/linux/x86_64/64/libc_malloc_debug.abilist      2021-07-27 16:14:51.516781791 +0200
> +++ …/malloc/libc_malloc_debug.symlist  2021-07-30 09:55:09.818875449 +0200
> @@ -3 +2,0 @@ GLIBC_2.16 aligned_alloc F
> -GLIBC_2.2.5 __free_hook D 0x8
>
> If this works with a linker, it appears to ignore “local: *;” in version
> nodes for versioned symbols.  That looks like a linker bug.
>
> Thanks,
> Florian
>

I have a comment on https://sourceware.org/bugzilla/show_bug.cgi?id=23328#c6

How does removing __free_hook from malloc/Versions break the ABI check test?

% cat a.s
.symver __free_hook, __free_hook@GLIBC_2.2.5
.globl __free_hook
__free_hook:
  nop
% cat a.ver
GLIBC_2.2.5 {};  /* should not list the non-default version __free_hook */
local { local: *; };
% cc -c a.s
% ld.bfd -shared --version-script=a.ver a.o -o a.so
% readelf -W --dyn- a.so | grep free_hook
     3: 0000000000001000     0 NOTYPE  GLOBAL DEFAULT    7
__free_hook@GLIBC_2.2.5

One non-default version symbol, as expected.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang
  2021-07-31  6:34   ` Fāng-ruì Sòng via Libc-alpha
@ 2021-07-31  6:41     ` Florian Weimer via Libc-alpha
  2021-08-02 20:55     ` Fāng-ruì Sòng via Libc-alpha
  1 sibling, 0 replies; 20+ messages in thread
From: Florian Weimer via Libc-alpha @ 2021-07-31  6:41 UTC (permalink / raw)
  To: Fāng-ruì Sòng; +Cc: Fangrui Song via Libc-alpha

* Fāng-ruì Sòng:

> On Fri, Jul 30, 2021 at 12:57 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * Fangrui Song via Libc-alpha:
>>
>> > For malloc/tst-compathooks-on,
>> >
>> >     malloc/tst-compathooks-on: Symbol `__free_hook' has different size in shared object, consider re-linking
>> >
>> > the root cause is that lld's symbol versioning is different from GNU ld in an unusal case:
>> >
>> >     __asm__ (".symver " "__free_hook" "," "__free_hook" "@" "GLIBC_2.2.5");
>> >
>> > This leaves two symbols __free_hook and __free_hook@GLIBC_2.2.5.
>> > __free_hook is then attached a default version GLIBC_2.2.5.
>> > I think malloc/malloc-debug.c uses a fragile versioned symbol here.
>> > If the inline asm uses @@ the failure should go away.
>>
>> But we want to produce a compat symbol here.  With the current version
>> scripts, BFD ld will not export a symbol unless it is listed in the
>> version script.  That is, if I remove __free_hook from libc_malloc_debug
>> in malloc/Versions, I get an ABI check failure:
>>
>> --- ../sysdeps/unix/sysv/linux/x86_64/64/libc_malloc_debug.abilist      2021-07-27 16:14:51.516781791 +0200
>> +++ …/malloc/libc_malloc_debug.symlist  2021-07-30 09:55:09.818875449 +0200
>> @@ -3 +2,0 @@ GLIBC_2.16 aligned_alloc F
>> -GLIBC_2.2.5 __free_hook D 0x8
>>
>> If this works with a linker, it appears to ignore “local: *;” in version
>> nodes for versioned symbols.  That looks like a linker bug.
>>
>> Thanks,
>> Florian
>>
>
> I have a comment on https://sourceware.org/bugzilla/show_bug.cgi?id=23328#c6
>
> How does removing __free_hook from malloc/Versions break the ABI check test?
>
> % cat a.s
> .symver __free_hook, __free_hook@GLIBC_2.2.5
> .globl __free_hook
> __free_hook:
>   nop
> % cat a.ver
> GLIBC_2.2.5 {};  /* should not list the non-default version __free_hook */
> local { local: *; };

We currently generate this:

GLIBC_2.2.5 {
  global:
    __malloc_hook;
    __memalign_hook;
    __realloc_hook;
    calloc;
    free;
    mallinfo;
    malloc;
    malloc_get_state;
    malloc_set_state;
    malloc_stats;
    malloc_trim;
    malloc_usable_size;
    mallopt;
    mcheck;
    mcheck_check_all;
    mcheck_pedantic;
    memalign;
    mprobe;
    mtrace;
    muntrace;
    posix_memalign;
    pvalloc;
    realloc;
    valloc;
  local:
    *;
};

See libc_malloc_debug.map in the build tree.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang
  2021-07-26  3:57 [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang Fangrui Song via Libc-alpha
                   ` (4 preceding siblings ...)
  2021-07-30  7:57 ` Florian Weimer via Libc-alpha
@ 2021-08-02  4:02 ` Siddhesh Poyarekar
  2021-08-02  4:23   ` Siddhesh Poyarekar
  2021-08-08  2:54   ` Fāng-ruì Sòng via Libc-alpha
  5 siblings, 2 replies; 20+ messages in thread
From: Siddhesh Poyarekar @ 2021-08-02  4:02 UTC (permalink / raw)
  To: Fangrui Song, libc-alpha

On 7/26/21 9:27 AM, Fangrui Song via Libc-alpha wrote:
> For malloc/tst-compathooks-on,
> 
>      malloc/tst-compathooks-on: Symbol `__free_hook' has different size in shared object, consider re-linking
> 
> the root cause is that lld's symbol versioning is different from GNU ld in an unusal case:
> 
>      __asm__ (".symver " "__free_hook" "," "__free_hook" "@" "GLIBC_2.2.5");
> 
> This leaves two symbols __free_hook and __free_hook@GLIBC_2.2.5.
> __free_hook is then attached a default version GLIBC_2.2.5.
> I think malloc/malloc-debug.c uses a fragile versioned symbol here.
> If the inline asm uses @@ the failure should go away.

The entire point of link time deprecation is to not emit a @@ symbol. 
If lld emits __free_hook@@GLIBC_2.2.5 in this case then this needs to be 
resolved.  How does one do symbol deprecation in lld?

> In summary, I think the failed tests touch some dark corners of the toolchain.
> These things do not really matter for real world applications.

Not having a default symbol version is a documented mechanism[1] to 
deprecate symbols, so this is not some obscure use case.  Symbol 
deprecation matters for real world applications.

Siddhesh

[1] See section 3.3 in https://www.akkadia.org/drepper/dsohowto.pdf:

"Every versioned DSO has at most one version
of every API which can be used at link-time. An API
(not ABI) can also vanish completely: this is a way to
deprecate APIs without affecting binary compatibility."

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang
  2021-08-02  4:02 ` Siddhesh Poyarekar
@ 2021-08-02  4:23   ` Siddhesh Poyarekar
  2021-08-08 16:50     ` Fāng-ruì Sòng via Libc-alpha
  2021-08-08  2:54   ` Fāng-ruì Sòng via Libc-alpha
  1 sibling, 1 reply; 20+ messages in thread
From: Siddhesh Poyarekar @ 2021-08-02  4:23 UTC (permalink / raw)
  To: Fangrui Song, libc-alpha

On 8/2/21 9:32 AM, Siddhesh Poyarekar wrote:
> The entire point of link time deprecation is to not emit a @@ symbol. If 
> lld emits __free_hook@@GLIBC_2.2.5 in this case then this needs to be 
> resolved.  How does one do symbol deprecation in lld?

Sorry I just realized that you may have implied that having a .globl in 
the object file ought to be sufficient to indicate that the symbol needs 
to be exported.  The glibc linker script however overrides this with 
{local: *} and if the linker doesn't honour that then ISTM that this 
ought to be a bug in the linker.  ld.bfd always gives preference to the 
linker script over symbol definitions in source and it seems like a 
simpler and more consistent rule.

Siddhesh

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang
  2021-07-31  6:34   ` Fāng-ruì Sòng via Libc-alpha
  2021-07-31  6:41     ` Florian Weimer via Libc-alpha
@ 2021-08-02 20:55     ` Fāng-ruì Sòng via Libc-alpha
  1 sibling, 0 replies; 20+ messages in thread
From: Fāng-ruì Sòng via Libc-alpha @ 2021-08-02 20:55 UTC (permalink / raw)
  To: Florian Weimer; +Cc: Fangrui Song via Libc-alpha

On 2021-07-30, Fāng-ruì Sòng wrote:
>On Fri, Jul 30, 2021 at 12:57 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * Fangrui Song via Libc-alpha:
>>
>> > For malloc/tst-compathooks-on,
>> >
>> >     malloc/tst-compathooks-on: Symbol `__free_hook' has different size in shared object, consider re-linking
>> >
>> > the root cause is that lld's symbol versioning is different from GNU ld in an unusal case:
>> >
>> >     __asm__ (".symver " "__free_hook" "," "__free_hook" "@" "GLIBC_2.2.5");
>> >
>> > This leaves two symbols __free_hook and __free_hook@GLIBC_2.2.5.
>> > __free_hook is then attached a default version GLIBC_2.2.5.
>> > I think malloc/malloc-debug.c uses a fragile versioned symbol here.
>> > If the inline asm uses @@ the failure should go away.
>>
>> But we want to produce a compat symbol here.  With the current version
>> scripts, BFD ld will not export a symbol unless it is listed in the
>> version script.  That is, if I remove __free_hook from libc_malloc_debug
>> in malloc/Versions, I get an ABI check failure:
>>
>> --- ../sysdeps/unix/sysv/linux/x86_64/64/libc_malloc_debug.abilist      2021-07-27 16:14:51.516781791 +0200
>> +++ …/malloc/libc_malloc_debug.symlist  2021-07-30 09:55:09.818875449 +0200
>> @@ -3 +2,0 @@ GLIBC_2.16 aligned_alloc F
>> -GLIBC_2.2.5 __free_hook D 0x8
>>
>> If this works with a linker, it appears to ignore “local: *;” in version
>> nodes for versioned symbols.  That looks like a linker bug.
>>
>> Thanks,
>> Florian
>>
>
>I have a comment on https://sourceware.org/bugzilla/show_bug.cgi?id=23328#c6
>
>How does removing __free_hook from malloc/Versions break the ABI check test?
>
>% cat a.s
>.symver __free_hook, __free_hook@GLIBC_2.2.5
>.globl __free_hook
>__free_hook:
>  nop
>% cat a.ver
>GLIBC_2.2.5 {};  /* should not list the non-default version __free_hook */
>local { local: *; };
>% cc -c a.s
>% ld.bfd -shared --version-script=a.ver a.o -o a.so
>% readelf -W --dyn- a.so | grep free_hook
>     3: 0000000000001000     0 NOTYPE  GLOBAL DEFAULT    7
>__free_hook@GLIBC_2.2.5
>
>One non-default version symbol, as expected.

I'll implement the behavior in https://reviews.llvm.org/D107234 and
https://reviews.llvm.org/D107235

ld.lld linked libc.so has exactly the same set of non-SHN_ABS dynamic
symbols as ld.bfd linked libc.so.
(ld.bfd places version nodes into SHN_ABS symbols but they are unused by ld.so)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang
  2021-08-02  4:02 ` Siddhesh Poyarekar
  2021-08-02  4:23   ` Siddhesh Poyarekar
@ 2021-08-08  2:54   ` Fāng-ruì Sòng via Libc-alpha
  2021-08-08 16:45     ` Siddhesh Poyarekar
  1 sibling, 1 reply; 20+ messages in thread
From: Fāng-ruì Sòng via Libc-alpha @ 2021-08-08  2:54 UTC (permalink / raw)
  To: Siddhesh Poyarekar; +Cc: libc-alpha

On Sun, Aug 1, 2021 at 9:03 PM Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>
> On 7/26/21 9:27 AM, Fangrui Song via Libc-alpha wrote:
> > For malloc/tst-compathooks-on,
> >
> >      malloc/tst-compathooks-on: Symbol `__free_hook' has different size in shared object, consider re-linking
> >
> > the root cause is that lld's symbol versioning is different from GNU ld in an unusal case:
> >
> >      __asm__ (".symver " "__free_hook" "," "__free_hook" "@" "GLIBC_2.2.5");
> >
> > This leaves two symbols __free_hook and __free_hook@GLIBC_2.2.5.
> > __free_hook is then attached a default version GLIBC_2.2.5.
> > I think malloc/malloc-debug.c uses a fragile versioned symbol here.
> > If the inline asm uses @@ the failure should go away.
>
> The entire point of link time deprecation is to not emit a @@ symbol.
> If lld emits __free_hook@@GLIBC_2.2.5 in this case then this needs to be
> resolved.  How does one do symbol deprecation in lld?
>
> > In summary, I think the failed tests touch some dark corners of the toolchain.
> > These things do not really matter for real world applications.
>
> Not having a default symbol version is a documented mechanism[1] to
> deprecate symbols, so this is not some obscure use case.  Symbol
> deprecation matters for real world applications.
>
> Siddhesh

See
https://sourceware.org/bugzilla/show_bug.cgi?id=28197 ("compat_symbol
should migrate to .symver *, *, remove")

This is an assembler design flaw. Keeping the original symbol makes
all linker implementations difficult.
Ian Lance Taylor's complaint is probably this:
https://www.airs.com/blog/archives/220

Anyway, I installed a workaround for LLD 13.0.0. This is no longer an issue.

> [1] See section 3.3 in https://www.akkadia.org/drepper/dsohowto.pdf:
>
> "Every versioned DSO has at most one version
> of every API which can be used at link-time. An API
> (not ABI) can also vanish completely: this is a way to
> deprecate APIs without affecting binary compatibility."

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang
  2021-08-08  2:54   ` Fāng-ruì Sòng via Libc-alpha
@ 2021-08-08 16:45     ` Siddhesh Poyarekar
  0 siblings, 0 replies; 20+ messages in thread
From: Siddhesh Poyarekar @ 2021-08-08 16:45 UTC (permalink / raw)
  To: Fāng-ruì Sòng; +Cc: libc-alpha

On 8/8/21 8:24 AM, Fāng-ruì Sòng wrote:
> See
> https://sourceware.org/bugzilla/show_bug.cgi?id=28197 ("compat_symbol
> should migrate to .symver *, *, remove")

Thanks for digging deeper into this.  Off the top of my head I don't 
remember any compat_symbol() use where we may be using the 
implementation function name so ", remove" may in fact be acceptable.

Siddhesh

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang
  2021-08-02  4:23   ` Siddhesh Poyarekar
@ 2021-08-08 16:50     ` Fāng-ruì Sòng via Libc-alpha
  0 siblings, 0 replies; 20+ messages in thread
From: Fāng-ruì Sòng via Libc-alpha @ 2021-08-08 16:50 UTC (permalink / raw)
  To: Siddhesh Poyarekar; +Cc: libc-alpha

On Sun, Aug 1, 2021 at 9:23 PM Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>
> On 8/2/21 9:32 AM, Siddhesh Poyarekar wrote:
> > The entire point of link time deprecation is to not emit a @@ symbol. If
> > lld emits __free_hook@@GLIBC_2.2.5 in this case then this needs to be
> > resolved.  How does one do symbol deprecation in lld?
>
> Sorry I just realized that you may have implied that having a .globl in
> the object file ought to be sufficient to indicate that the symbol needs
> to be exported.  The glibc linker script however overrides this with
> {local: *} and if the linker doesn't honour that then ISTM that this
> ought to be a bug in the linker.  ld.bfd always gives preference to the
> linker script over symbol definitions in source and it seems like a
> simpler and more consistent rule.
>
> Siddhesh

Your conjecture was correct:) LLD<13.0.0 did not implement: a "local:"
pattern can localize a non-default version symbol.
I apologize for the previous confusion I may caused.
(I was not aware that this is possible, but this behavior does make
sense to me. I implemented it and cherry picked
https://reviews.llvm.org/D107234 to 13.0.0)

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2021-08-08 16:50 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-26  3:57 [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang Fangrui Song via Libc-alpha
2021-07-26  3:58 ` [PATCH 1/3] elf: Replace .tls_common with .tbss definition Fangrui Song via Libc-alpha
2021-07-29 14:26   ` H.J. Lu via Libc-alpha
2021-07-29 16:21     ` Fangrui Song via Libc-alpha
2021-07-26  3:58 ` [PATCH 2/3] elf: Skip tst-auditlogmod-* if the linker doesn't support --depaudit Fangrui Song via Libc-alpha
2021-07-29 14:23   ` H.J. Lu via Libc-alpha
2021-07-29 16:28     ` Fangrui Song via Libc-alpha
2021-07-26  3:58 ` [PATCH 3/3] configure: Allow LD to be LLD 13.0.0 or above Fangrui Song via Libc-alpha
2021-07-28 18:02 ` [PATCH 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang H.J. Lu via Libc-alpha
2021-07-28 21:52   ` Fangrui Song via Libc-alpha
2021-07-29 14:45     ` H.J. Lu via Libc-alpha
2021-07-30  7:57 ` Florian Weimer via Libc-alpha
2021-07-31  6:34   ` Fāng-ruì Sòng via Libc-alpha
2021-07-31  6:41     ` Florian Weimer via Libc-alpha
2021-08-02 20:55     ` Fāng-ruì Sòng via Libc-alpha
2021-08-02  4:02 ` Siddhesh Poyarekar
2021-08-02  4:23   ` Siddhesh Poyarekar
2021-08-08 16:50     ` Fāng-ruì Sòng via Libc-alpha
2021-08-08  2:54   ` Fāng-ruì Sòng via Libc-alpha
2021-08-08 16:45     ` Siddhesh Poyarekar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).