[RFC 0/2] Transparent multi-version symbol support

[RFC 0/2] Transparent multi-version symbol support

[Florian Weimer]

This turned out to be way harder than it should be.  Older binutils does
not support multiple .symver directives for the same first symbol, so we
have various kludges like (from time/clock_settime.c):

| versioned_symbol (libc, __clock_settime, clock_settime, GLIBC_2_17);
| 
| /* clock_settime moved to libc in version 2.17;
|    old binaries may expect the symbol version it had in librt.  */
| #if SHLIB_COMPAT (libc, GLIBC_2_2, GLIBC_2_17)
| strong_alias (__clock_settime, __clock_settime_2);
| compat_symbol (libc, __clock_settime_2, clock_settime, GLIBC_2_2);
| #endif

I think I have found a way to do this with assembler hacks, so that it
applies to function and data symbols alike.  With this, the above
snipper becomes:

| versioned_symbol (libc, __clock_settime, clock_settime, GLIBC_2_17);
| 
| /* clock_settime moved to libc in version 2.17;
|    old binaries may expect the symbol version it had in librt.  */
| #if SHLIB_COMPAT (libc, GLIBC_2_2, GLIBC_2_17)
| compat_symbol (libc, __clock_settime, clock_settime, GLIBC_2_2);
| #endif

So the explicit aliases are gone.

Built with build-many-glibcs.py with binutils 2.36 and 2.34.  Tested on
i686-linux-gnu and x86_64-linux-gnu with binutils 2.34 and 2.35.

The object files aren't unchanged due to the __malloc_initialize_hook
change (and presumably others, I haven't looked closely).

Thanks,
Florian

Florian Weimer (2):
  Change how the symbol_version_reference macro is defined
  Fold compat_symbol_unique functionality into compat_symbol

 config.h.in                   |  4 ++
 configure                     | 28 +++++++++++
 configure.ac                  | 21 +++++++++
 include/libc-symbols.h        | 21 +++------
 include/shlib-compat.h        | 55 +++++++---------------
 locale/lc-ctype.c             | 14 +++---
 malloc/malloc.c               |  2 +-
 nptl/libpthread-compat.c      | 16 +++----
 sysdeps/generic/libc-symver.h | 88 +++++++++++++++++++++++++++++++++++
 sysdeps/ia64/libc-symver.h    | 33 +++++++++++++
 time/clock_getcpuclockid.c    |  3 +-
 time/clock_getres.c           |  3 +-
 time/clock_gettime.c          |  3 +-
 time/clock_nanosleep.c        |  3 +-
 time/clock_settime.c          |  3 +-
 15 files changed, 219 insertions(+), 78 deletions(-)
 create mode 100644 sysdeps/generic/libc-symver.h
 create mode 100644 sysdeps/ia64/libc-symver.h

-- 
2.30.2

[RFC 1/2] Change how the symbol_version_reference macro is defined

[Florian Weimer]

A subsequent change will require including <config.h> for defining
symbol_version_reference.  <libc-symbol.h> should not include
<config.h> for _ISOMAC, so it cannot define symbol_version_reference
anymore, but symbol_version_reference is needed <shlib-compat.h> even
for _ISOMAC.  Moving the definition of symbol_version_reference to a
separate file <libc-symver.h> makes it possible to use a single
definition for both cases.
---
 include/libc-symbols.h | 16 +++-------------
 include/libc-symver.h  | 38 ++++++++++++++++++++++++++++++++++++++
 include/shlib-compat.h |  3 +++
 3 files changed, 44 insertions(+), 13 deletions(-)
 create mode 100644 include/libc-symver.h

diff --git a/include/libc-symbols.h b/include/libc-symbols.h
index c83e550b03..ce5f75a1a2 100644
--- a/include/libc-symbols.h
+++ b/include/libc-symbols.h
@@ -59,19 +59,6 @@
 # define IN_MODULE (-1)
 #endif
 
-/* Use symbol_version_reference to specify the version a symbol
-   reference should link to.  Use symbol_version or
-   default_symbol_version for the definition of a versioned symbol.
-   The difference is that the latter is a no-op in non-shared
-   builds.  */
-#ifdef __ASSEMBLER__
-# define symbol_version_reference(real, name, version) \
-     .symver real, name##@##version
-#else  /* !__ASSEMBLER__ */
-# define symbol_version_reference(real, name, version) \
-  __asm__ (".symver " #real "," #name "@" #version)
-#endif
-
 #ifndef _ISOMAC
 
 /* This is defined for the compilation of all C library code.  features.h
@@ -97,6 +84,9 @@
 
 #include <config.h>
 
+/* Obtain the definition of symbol_version_reference.  */
+#include <libc-symver.h>
+
 /* When PIC is defined and SHARED isn't defined, we are building PIE
    by default.  */
 #if defined PIC && !defined SHARED
diff --git a/include/libc-symver.h b/include/libc-symver.h
new file mode 100644
index 0000000000..11c77ae1cd
--- /dev/null
+++ b/include/libc-symver.h
@@ -0,0 +1,38 @@
+/* Symbol version management.
+   Copyright (C) 1995-2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+/* This file is included from <libc-symbols.h> for !_ISOMAC, and
+   unconditionally from <shlib-compat.h>.  */
+
+#ifndef _LIBC_SYMVER_H
+#define _LIBC_SYMVER_H 1
+
+/* Use symbol_version_reference to specify the version a symbol
+   reference should link to.  Use symbol_version or
+   default_symbol_version for the definition of a versioned symbol.
+   The difference is that the latter is a no-op in non-shared
+   builds.  */
+#ifdef __ASSEMBLER__
+# define symbol_version_reference(real, name, version) \
+     .symver real, name##@##version
+#else  /* !__ASSEMBLER__ */
+# define symbol_version_reference(real, name, version) \
+  __asm__ (".symver " #real "," #name "@" #version)
+#endif
+
+#endif /* _LIBC_SYMVER_H */
diff --git a/include/shlib-compat.h b/include/shlib-compat.h
index 28baef1ea4..b874e2588f 100644
--- a/include/shlib-compat.h
+++ b/include/shlib-compat.h
@@ -21,6 +21,9 @@
 
 # include <abi-versions.h>
 
+/* Obtain the definition of symbol_version_reference.  */
+#include <libc-symver.h>
+
 /* The file abi-versions.h (generated by scripts/abi-versions.awk) defines
    symbols like `ABI_libm_GLIBC_2_0' for each version set in the source
    code for each library.  For a version set that is subsumed by a later
-- 
2.30.2

Re: [RFC 1/2] Change how the symbol_version_reference macro is defined

[Adhemerval Zanella]

On 18/03/2021 17:07, Florian Weimer via Libc-alpha wrote:
> A subsequent change will require including <config.h> for defining
> symbol_version_reference.  <libc-symbol.h> should not include
> <config.h> for _ISOMAC, so it cannot define symbol_version_reference
> anymore, but symbol_version_reference is needed <shlib-compat.h> even
> for _ISOMAC.  Moving the definition of symbol_version_reference to a
> separate file <libc-symver.h> makes it possible to use a single
> definition for both cases.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  include/libc-symbols.h | 16 +++-------------
>  include/libc-symver.h  | 38 ++++++++++++++++++++++++++++++++++++++
>  include/shlib-compat.h |  3 +++
>  3 files changed, 44 insertions(+), 13 deletions(-)
>  create mode 100644 include/libc-symver.h
> 
> diff --git a/include/libc-symbols.h b/include/libc-symbols.h
> index c83e550b03..ce5f75a1a2 100644
> --- a/include/libc-symbols.h
> +++ b/include/libc-symbols.h
> @@ -59,19 +59,6 @@
>  # define IN_MODULE (-1)
>  #endif
>  
> -/* Use symbol_version_reference to specify the version a symbol
> -   reference should link to.  Use symbol_version or
> -   default_symbol_version for the definition of a versioned symbol.
> -   The difference is that the latter is a no-op in non-shared
> -   builds.  */
> -#ifdef __ASSEMBLER__
> -# define symbol_version_reference(real, name, version) \
> -     .symver real, name##@##version
> -#else  /* !__ASSEMBLER__ */
> -# define symbol_version_reference(real, name, version) \
> -  __asm__ (".symver " #real "," #name "@" #version)
> -#endif
> -
>  #ifndef _ISOMAC
>  
>  /* This is defined for the compilation of all C library code.  features.h

Ok.

> @@ -97,6 +84,9 @@
>  
>  #include <config.h>
>  
> +/* Obtain the definition of symbol_version_reference.  */
> +#include <libc-symver.h>
> +
>  /* When PIC is defined and SHARED isn't defined, we are building PIE
>     by default.  */
>  #if defined PIC && !defined SHARED

Ok.

> diff --git a/include/libc-symver.h b/include/libc-symver.h
> new file mode 100644
> index 0000000000..11c77ae1cd
> --- /dev/null
> +++ b/include/libc-symver.h
> @@ -0,0 +1,38 @@
> +/* Symbol version management.
> +   Copyright (C) 1995-2021 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +/* This file is included from <libc-symbols.h> for !_ISOMAC, and
> +   unconditionally from <shlib-compat.h>.  */
> +
> +#ifndef _LIBC_SYMVER_H
> +#define _LIBC_SYMVER_H 1
> +
> +/* Use symbol_version_reference to specify the version a symbol
> +   reference should link to.  Use symbol_version or
> +   default_symbol_version for the definition of a versioned symbol.
> +   The difference is that the latter is a no-op in non-shared
> +   builds.  */
> +#ifdef __ASSEMBLER__
> +# define symbol_version_reference(real, name, version) \
> +     .symver real, name##@##version
> +#else  /* !__ASSEMBLER__ */
> +# define symbol_version_reference(real, name, version) \
> +  __asm__ (".symver " #real "," #name "@" #version)
> +#endif
> +
> +#endif /* _LIBC_SYMVER_H */

Ok.

> diff --git a/include/shlib-compat.h b/include/shlib-compat.h
> index 28baef1ea4..b874e2588f 100644
> --- a/include/shlib-compat.h
> +++ b/include/shlib-compat.h
> @@ -21,6 +21,9 @@
>  
>  # include <abi-versions.h>
>  
> +/* Obtain the definition of symbol_version_reference.  */
> +#include <libc-symver.h>
> +
>  /* The file abi-versions.h (generated by scripts/abi-versions.awk) defines
>     symbols like `ABI_libm_GLIBC_2_0' for each version set in the source
>     code for each library.  For a version set that is subsumed by a later
> 

Ok.

[RFC 2/2] Fold compat_symbol_unique functionality into compat_symbol

[Florian Weimer]

This eliminates the need for intermediate aliases for defining
multiple symbol versions, for both compat_symbol and
versioned_symbol.  Some binutils versions do not suport multiple
versions per symbol on some targets, so aliases are automatically
introduced, similar to what compat_symbol_unique did.  To reduce
symbol table sizes, a configure check is added to avoid these
aliases if they are not needed.

The new mechanism works with data symbols as well as function symbols,
due to the way an assembler-level redirect is used.  It is not
compatible with weak symbols for old binutils versions, which is why
the definition of __malloc_initialize_hook had to be changed.  This
is not a loss of functionality because weak symbols do not matter
to dynamic linking.

The placeholder symbol needs repeating in nptl/libpthread-compat.c
now that compat_symbol is used, but that seems more obvious than
introducing yet another macro.

A subtle difference was that compat_symbol_unique made the symbol
global automatically.  compat_symbol does not do this, so static
had to be removed from the definition of
__libpthread_version_placeholder.

In locale/lc-ctype.c, it seems prudent to switch to
compat_symbol_reference.  It produces .symver directives directly, as
before.  This side-steps the question of copy relocations, as
mentioned in the comment.

To verify the versioned_symbol functionality, the explicit aliases
are eliminated from the clock_* function definitions.
---
 config.h.in                             |  4 ++
 configure                               | 28 ++++++++
 configure.ac                            | 21 ++++++
 include/libc-symbols.h                  |  5 +-
 include/shlib-compat.h                  | 52 ++++-----------
 locale/lc-ctype.c                       | 14 ++--
 malloc/malloc.c                         |  2 +-
 nptl/libpthread-compat.c                | 16 ++---
 sysdeps/generic/libc-symver.h           | 88 +++++++++++++++++++++++++
 {include => sysdeps/ia64}/libc-symver.h | 29 ++++----
 time/clock_getcpuclockid.c              |  3 +-
 time/clock_getres.c                     |  3 +-
 time/clock_gettime.c                    |  3 +-
 time/clock_nanosleep.c                  |  3 +-
 time/clock_settime.c                    |  3 +-
 15 files changed, 192 insertions(+), 82 deletions(-)
 create mode 100644 sysdeps/generic/libc-symver.h
 rename {include => sysdeps/ia64}/libc-symver.h (51%)

diff --git a/config.h.in b/config.h.in
index f21bf04e47..ca1547ae67 100644
--- a/config.h.in
+++ b/config.h.in
@@ -190,6 +190,10 @@
 /* Define if the linker defines __ehdr_start.  */
 #undef HAVE_EHDR_START
 
+/* Define to 1 if the assembler needs intermediate aliases to define
+   multiple symbol versions for one symbol.  */
+#define SYMVER_NEEDS_ALIAS 0
+
 /*
 \f */
 
diff --git a/configure b/configure
index 37cef37413..bf33439a7f 100755
--- a/configure
+++ b/configure
@@ -6590,6 +6590,34 @@ elif test "$libc_cv_ehdr_start" = broken; then
 $as_echo "$as_me: WARNING: linker is broken -- you should upgrade" >&2;}
 fi
 
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether the assembler requires one version per symbol" >&5
+$as_echo_n "checking whether the assembler requires one version per symbol... " >&6; }
+if ${libc_cv_symver_needs_alias+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+    cat > conftest.s <<EOF
+        .text
+testfunc:
+        .globl testfunc
+        .symver testfunc, testfunc1@VERSION1
+        .symver testfunc, testfunc1@VERSION2
+EOF
+  libc_cv_symver_needs_alias=no
+  if ${CC-cc} $ASFLAGS -c conftest.s 2>&5; then
+    libc_cv_symver_needs_alias=no
+  else
+    libc_cv_symver_needs_alias=yes
+  fi
+  rm conftest.*
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_symver_needs_alias" >&5
+$as_echo "$libc_cv_symver_needs_alias" >&6; }
+if test "$libc_cv_symver_needs_alias" = yes; then
+  $as_echo "#define SYMVER_NEEDS_ALIAS 1" >>confdefs.h
+
+fi
+
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for __builtin_trap with no external dependencies" >&5
 $as_echo_n "checking for __builtin_trap with no external dependencies... " >&6; }
 if ${libc_cv_builtin_trap+:} false; then :
diff --git a/configure.ac b/configure.ac
index 16b15b6f90..5b3440511b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1673,6 +1673,27 @@ elif test "$libc_cv_ehdr_start" = broken; then
   AC_MSG_WARN([linker is broken -- you should upgrade])
 fi
 
+AC_CACHE_CHECK(whether the assembler requires one version per symbol,
+               libc_cv_symver_needs_alias, [dnl
+  cat > conftest.s <<EOF
+        .text
+testfunc:
+        .globl testfunc
+        .symver testfunc, testfunc1@VERSION1
+        .symver testfunc, testfunc1@VERSION2
+EOF
+  libc_cv_symver_needs_alias=no
+  if ${CC-cc} $ASFLAGS -c conftest.s 2>&AS_MESSAGE_LOG_FD; then
+    libc_cv_symver_needs_alias=no
+  else
+    libc_cv_symver_needs_alias=yes
+  fi
+  rm conftest.*
+])
+if test "$libc_cv_symver_needs_alias" = yes; then
+  AC_DEFINE(SYMVER_NEEDS_ALIAS)
+fi
+
 AC_CACHE_CHECK(for __builtin_trap with no external dependencies,
 	       libc_cv_builtin_trap, [dnl
 libc_cv_builtin_trap=no
diff --git a/include/libc-symbols.h b/include/libc-symbols.h
index ce5f75a1a2..546fc26a7b 100644
--- a/include/libc-symbols.h
+++ b/include/libc-symbols.h
@@ -404,12 +404,13 @@ for linking")
   symbol_version_reference(real, name, version)
 # define default_symbol_version(real, name, version) \
      _default_symbol_version(real, name, version)
+/* See <libc-symver.h>.  */
 # ifdef __ASSEMBLER__
 #  define _default_symbol_version(real, name, version) \
-     .symver real, name##@##@##version
+  _set_symbol_version (real, name@@version)
 # else
 #  define _default_symbol_version(real, name, version) \
-     __asm__ (".symver " #real "," #name "@@" #version)
+  _set_symbol_version (real, #name "@@" #version)
 # endif
 
 /* Evalutes to a string literal for VERSION in LIB.  */
diff --git a/include/shlib-compat.h b/include/shlib-compat.h
index b874e2588f..f4978c05af 100644
--- a/include/shlib-compat.h
+++ b/include/shlib-compat.h
@@ -70,43 +70,20 @@
   default_symbol_version (local, symbol, name)
 
 # define compat_symbol(lib, local, symbol, version) \
-  compat_symbol_reference (lib, local, symbol, version)
-
-/* This is similar to compat_symbol, but allows versioning the same symbol
-   to multiple version without having multiple symbol definitions.  For
-   instance:
-
-   #if (SHLIB_COMPAT (libpthread, GLIBC_2_1_2, GLIBC_2_2))
-   compat_symbol_unique (libc, old_foo, GLIBC_2_1_2)
-   #endif
-
-   #if (SHLIB_COMPAT (libpthread, GLIBC_2_2_6, GLIBC_2_3))
-   compat_symbol_unique (libc, old_foo, GLIBC_2_2_6)
-   #endif
-
-   Internally it creates a unique strong alias to the input symbol and
-   creates one compat_symbol on the alias.  Using the above example,
-   it is similar to:
-
-   #if (SHLIB_COMPAT (libpthread, GLIBC_2_1_2, GLIBC_2_2))
-   strong_alias (old_foo, old_foo__COUNTER__)
-   compat_symbol (libc, old_foo__COUNTER__, foo, GLIBC_2_2)
-   #endif.
-
-   With __COUNTER__ being a monotonic number generated by the compiler.  */
-
-# define __compat_symbol_unique_concat(x, y) x ## y
-# define _compat_symbol_unique_concat(x, y) \
-  __compat_symbol_unique_concat (x, y)
-# define _compat_symbol_unique_alias(name) \
-  _compat_symbol_unique_concat (name, __COUNTER__)
-# define _compat_symbol_unique(lib, orig_name, name, version) \
-  strong_alias (orig_name, name) \
-  compat_symbol (lib, name, orig_name, version)
-# define compat_symbol_unique(lib, name, version) \
-  _compat_symbol_unique (lib, name, _compat_symbol_unique_alias (name), \
-                         version)
-
+  compat_symbol_1 (lib, local, symbol, version)
+# define compat_symbol_1(lib, local, symbol, version) \
+  compat_symbol_2 (local, symbol, VERSION_##lib##_##version)
+
+/* See <libc-symver.h>.  */
+# ifdef __ASSEMBLER__
+#define compat_symbol_2(local, symbol, name) \
+  _set_symbol_version (local, symbol@name)
+# else
+#  define compat_symbol_2(local, symbol, name) \
+  compat_symbol_3 (local, symbol, name)
+#  define compat_symbol_3(local, symbol, name) \
+  _set_symbol_version (local, #symbol "@" #name)
+# endif
 #else
 
 /* Not compiling ELF shared libraries at all, so never any old versions.  */
@@ -118,7 +95,6 @@
 
 /* This should not appear outside `#if SHLIB_COMPAT (...)'.  */
 # define compat_symbol(lib, local, symbol, version) ...
-# define compat_symbol_unique(lib, name, version) ...
 
 #endif
 
diff --git a/locale/lc-ctype.c b/locale/lc-ctype.c
index 1db0605c82..7c97480cbd 100644
--- a/locale/lc-ctype.c
+++ b/locale/lc-ctype.c
@@ -93,12 +93,14 @@ _nl_postload_ctype (void)
      We need those relocations so that a versioned definition with a COPY
      reloc in an executable will override the libc.so definition.  */
 
-compat_symbol (libc, __ctype_b, __ctype_b, GLIBC_2_0);
-compat_symbol (libc, __ctype_tolower, __ctype_tolower, GLIBC_2_0);
-compat_symbol (libc, __ctype_toupper, __ctype_toupper, GLIBC_2_0);
-compat_symbol (libc, __ctype32_b, __ctype32_b, GLIBC_2_0);
-compat_symbol (libc, __ctype32_tolower, __ctype32_tolower, GLIBC_2_2);
-compat_symbol (libc, __ctype32_toupper, __ctype32_toupper, GLIBC_2_2);
+compat_symbol_reference (libc, __ctype_b, __ctype_b, GLIBC_2_0);
+compat_symbol_reference (libc, __ctype_tolower, __ctype_tolower, GLIBC_2_0);
+compat_symbol_reference (libc, __ctype_toupper, __ctype_toupper, GLIBC_2_0);
+compat_symbol_reference (libc, __ctype32_b, __ctype32_b, GLIBC_2_0);
+compat_symbol_reference (libc, __ctype32_tolower, __ctype32_tolower,
+			  GLIBC_2_2);
+compat_symbol_reference (libc, __ctype32_toupper, __ctype32_toupper,
+			 GLIBC_2_2);
 
   __ctype_b = current (uint16_t, CLASS, 128);
   __ctype_toupper = current (int32_t, TOUPPER, 128);
diff --git a/malloc/malloc.c b/malloc/malloc.c
index 1f4bbd8edf..f23027c13d 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -1991,7 +1991,7 @@ static void *memalign_hook_ini (size_t alignment, size_t sz,
                                 const void *caller) __THROW;
 
 #if HAVE_MALLOC_INIT_HOOK
-void weak_variable (*__malloc_initialize_hook) (void) = NULL;
+void (*__malloc_initialize_hook) (void);
 compat_symbol (libc, __malloc_initialize_hook,
 	       __malloc_initialize_hook, GLIBC_2_0);
 #endif
diff --git a/nptl/libpthread-compat.c b/nptl/libpthread-compat.c
index 820dcd6a8f..da537af76e 100644
--- a/nptl/libpthread-compat.c
+++ b/nptl/libpthread-compat.c
@@ -20,10 +20,10 @@
 #include <shlib-compat.h>
 
 #ifdef SHARED
-static void
+void
 attribute_compat_text_section
 __attribute_used__
-__libpthread_version_placeholder (void)
+__libpthread_version_placeholder_1 (void)
 {
 }
 #endif
@@ -37,16 +37,16 @@ __libpthread_version_placeholder (void)
    there are plenty of other symbols which populate those later
    versions.  */
 #if (SHLIB_COMPAT (libpthread, GLIBC_2_1_2, GLIBC_2_2))
-compat_symbol_unique (libpthread,
-		      __libpthread_version_placeholder, GLIBC_2_1_2);
+compat_symbol (libpthread, __libpthread_version_placeholder_1,
+	       __libpthread_version_placeholder, GLIBC_2_1_2);
 #endif
 
 #if (SHLIB_COMPAT (libpthread, GLIBC_2_2_3, GLIBC_2_2_4))
-compat_symbol_unique (libpthread,
-		      __libpthread_version_placeholder, GLIBC_2_2_3);
+compat_symbol (libpthread, __libpthread_version_placeholder_1,
+	       __libpthread_version_placeholder, GLIBC_2_2_3);
 #endif
 
 #if (SHLIB_COMPAT (libpthread, GLIBC_2_2_6, GLIBC_2_3))
-compat_symbol_unique (libpthread,
-		      __libpthread_version_placeholder, GLIBC_2_2_6);
+compat_symbol (libpthread, __libpthread_version_placeholder_1,
+	       __libpthread_version_placeholder, GLIBC_2_2_6);
 #endif
diff --git a/sysdeps/generic/libc-symver.h b/sysdeps/generic/libc-symver.h
new file mode 100644
index 0000000000..69d147e2a8
--- /dev/null
+++ b/sysdeps/generic/libc-symver.h
@@ -0,0 +1,88 @@
+/* Symbol version management.
+   Copyright (C) 1995-2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+/* This file is included from <libc-symbols.h> for !_ISOMAC, and
+   unconditionally from <shlib-compat.h>.  */
+
+#ifndef _LIBC_SYMVER_H
+#define _LIBC_SYMVER_H 1
+
+#include <config.h>
+
+/* Use symbol_version_reference to specify the version a symbol
+   reference should link to.  Use symbol_version or
+   default_symbol_version for the definition of a versioned symbol.
+   The difference is that the latter is a no-op in non-shared
+   builds.
+
+   _set_symbol_version is similar to symbol_version_reference, except
+   that this macro expects the name and symbol version as a single
+   string or token sequence, with an @ or @@ separator.  (A string is
+   used in C mode and a token sequence in assembler mode.)
+   _set_symbol_version only be used for definitions because it may
+   introduce an alias symbol that would not be globally unique for
+   mere references.  The _set_symbol_version macro is used to define
+   default_symbol_version and compat_symbol.  */
+
+#ifdef __ASSEMBLER__
+# define symbol_version_reference(real, name, version) \
+     .symver real, name##@##version
+#else
+# define symbol_version_reference(real, name, version) \
+  __asm__ (".symver " #real "," #name "@" #version)
+#endif  /* !__ASSEMBLER__ */
+
+#if SYMVER_NEEDS_ALIAS
+/* If the assembler cannot support multiple versions for the same
+   symbol, introduce __SInnn_ aliases to which the symbol version is
+   attached.  */
+# define __symbol_version_unique_concat(x, y) __SI ## x ## _ ## y
+# define _symbol_version_unique_concat(x, y) \
+  __symbol_version_unique_concat (x, y)
+# define _symbol_version_unique_alias(name) \
+  _symbol_version_unique_concat (name, __COUNTER__)
+# ifdef __ASSEMBLER__
+#  define _set_symbol_version_2(real, alias, name_version) \
+  .globl alias ASM_LINE_SEP                                \
+  .equiv alias, real ASM_LINE_SEP                          \
+  .symver alias, name_version
+# else
+#  define _set_symbol_version_2(real, alias, name_version) \
+  __asm__ (".globl " #alias "\n\t"                         \
+           ".equiv " #alias ", " #real "\n\t"              \
+           ".symver " #alias "," name_version)
+# endif
+# define _set_symbol_version_1(real, alias, name_version) \
+  _set_symbol_version_2 (real, alias, name_version)
+/* REAL must be globally unique, so that the counter also produces
+   globally unique symbols.  */
+# define _set_symbol_version(real, name_version)                   \
+  _set_symbol_version_1 (real, _symbol_version_unique_alias (real), \
+                               name_version)
+# else  /* !SYMVER_NEEDS_ALIAS */
+# ifdef __ASSEMBLER__
+#  define _set_symbol_version(real, name_version) \
+  .symver real, name_version
+# else
+#  define _set_symbol_version(real, name_version) \
+  __asm__ (".symver " #real "," name_version)
+# endif
+#endif  /* !SYMVER_NEEDS_ALIAS */
+
+
+#endif /* _LIBC_SYMVER_H */
diff --git a/include/libc-symver.h b/sysdeps/ia64/libc-symver.h
similarity index 51%
rename from include/libc-symver.h
rename to sysdeps/ia64/libc-symver.h
index 11c77ae1cd..63a3e58dde 100644
--- a/include/libc-symver.h
+++ b/sysdeps/ia64/libc-symver.h
@@ -1,5 +1,5 @@
-/* Symbol version management.
-   Copyright (C) 1995-2021 Free Software Foundation, Inc.
+/* Symbol version management.  ia64 version.
+   Copyright (C) 2021 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -16,23 +16,18 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-/* This file is included from <libc-symbols.h> for !_ISOMAC, and
-   unconditionally from <shlib-compat.h>.  */
-
 #ifndef _LIBC_SYMVER_H
-#define _LIBC_SYMVER_H 1
 
-/* Use symbol_version_reference to specify the version a symbol
-   reference should link to.  Use symbol_version or
-   default_symbol_version for the definition of a versioned symbol.
-   The difference is that the latter is a no-op in non-shared
-   builds.  */
-#ifdef __ASSEMBLER__
-# define symbol_version_reference(real, name, version) \
-     .symver real, name##@##version
-#else  /* !__ASSEMBLER__ */
-# define symbol_version_reference(real, name, version) \
-  __asm__ (".symver " #real "," #name "@" #version)
+#include <sysdeps/generic/libc-symver.h>
+
+/* ia64 recognizes loc1 as a register name.  Add the # suffix to all
+   symbol references.  */
+#if !defined (__ASSEMBLER__) && SYMVER_NEEDS_ALIAS
+#undef _set_symbol_version_2
+# define _set_symbol_version_2(real, alias, name_version) \
+  __asm__ (".globl " #alias "#\n\t"                         \
+           ".equiv " #alias ", " #real "#\n\t"              \
+           ".symver " #alias "#," name_version)
 #endif
 
 #endif /* _LIBC_SYMVER_H */
diff --git a/time/clock_getcpuclockid.c b/time/clock_getcpuclockid.c
index c148d96c5c..220e2eb016 100644
--- a/time/clock_getcpuclockid.c
+++ b/time/clock_getcpuclockid.c
@@ -42,6 +42,5 @@ versioned_symbol (libc, __clock_getcpuclockid, clock_getcpuclockid, GLIBC_2_17);
 /* clock_getcpuclockid moved to libc in version 2.17;
    old binaries may expect the symbol version it had in librt.  */
 #if SHLIB_COMPAT (libc, GLIBC_2_2, GLIBC_2_17)
-strong_alias (__clock_getcpuclockid, __clock_getcpuclockid_2);
-compat_symbol (libc, __clock_getcpuclockid_2, clock_getcpuclockid, GLIBC_2_2);
+compat_symbol (libc, __clock_getcpuclockid, clock_getcpuclockid, GLIBC_2_2);
 #endif
diff --git a/time/clock_getres.c b/time/clock_getres.c
index 4b31893849..9099b62672 100644
--- a/time/clock_getres.c
+++ b/time/clock_getres.c
@@ -32,8 +32,7 @@ versioned_symbol (libc, __clock_getres, clock_getres, GLIBC_2_17);
 /* clock_getres moved to libc in version 2.17;
    old binaries may expect the symbol version it had in librt.  */
 #if SHLIB_COMPAT (libc, GLIBC_2_2, GLIBC_2_17)
-strong_alias (__clock_getres, __clock_getres_2);
-compat_symbol (libc, __clock_getres_2, clock_getres, GLIBC_2_2);
+compat_symbol (libc, __clock_getres, clock_getres, GLIBC_2_2);
 #endif
 
 stub_warning (clock_getres)
diff --git a/time/clock_gettime.c b/time/clock_gettime.c
index fdeaaca3f8..e8e129d201 100644
--- a/time/clock_gettime.c
+++ b/time/clock_gettime.c
@@ -33,8 +33,7 @@ versioned_symbol (libc, __clock_gettime, clock_gettime, GLIBC_2_17);
 /* clock_gettime moved to libc in version 2.17;
    old binaries may expect the symbol version it had in librt.  */
 #if SHLIB_COMPAT (libc, GLIBC_2_2, GLIBC_2_17)
-strong_alias (__clock_gettime, __clock_gettime_2);
-compat_symbol (libc, __clock_gettime_2, clock_gettime, GLIBC_2_2);
+compat_symbol (libc, __clock_gettime, clock_gettime, GLIBC_2_2);
 #endif
 
 stub_warning (clock_gettime)
diff --git a/time/clock_nanosleep.c b/time/clock_nanosleep.c
index 7ecb1cfcb8..57b3af2a70 100644
--- a/time/clock_nanosleep.c
+++ b/time/clock_nanosleep.c
@@ -38,8 +38,7 @@ versioned_symbol (libc, __clock_nanosleep, clock_nanosleep, GLIBC_2_17);
 /* clock_nanosleep moved to libc in version 2.17;
    old binaries may expect the symbol version it had in librt.  */
 #if SHLIB_COMPAT (libc, GLIBC_2_2, GLIBC_2_17)
-strong_alias (__clock_nanosleep, __clock_nanosleep_2);
-compat_symbol (libc, __clock_nanosleep_2, clock_nanosleep, GLIBC_2_2);
+compat_symbol (libc, __clock_nanosleep, clock_nanosleep, GLIBC_2_2);
 #endif
 
 stub_warning (clock_nanosleep)
diff --git a/time/clock_settime.c b/time/clock_settime.c
index 7676aaeb23..4df4ec56d1 100644
--- a/time/clock_settime.c
+++ b/time/clock_settime.c
@@ -33,8 +33,7 @@ versioned_symbol (libc, __clock_settime, clock_settime, GLIBC_2_17);
 /* clock_settime moved to libc in version 2.17;
    old binaries may expect the symbol version it had in librt.  */
 #if SHLIB_COMPAT (libc, GLIBC_2_2, GLIBC_2_17)
-strong_alias (__clock_settime, __clock_settime_2);
-compat_symbol (libc, __clock_settime_2, clock_settime, GLIBC_2_2);
+compat_symbol (libc, __clock_settime, clock_settime, GLIBC_2_2);
 #endif
 
 stub_warning (clock_settime)
-- 
2.30.2

Re: [RFC 2/2] Fold compat_symbol_unique functionality into compat_symbol

[Joseph Myers]

On Thu, 18 Mar 2021, Florian Weimer via Libc-alpha wrote:

> This eliminates the need for intermediate aliases for defining
> multiple symbol versions, for both compat_symbol and
> versioned_symbol.  Some binutils versions do not suport multiple
> versions per symbol on some targets, so aliases are automatically
> introduced, similar to what compat_symbol_unique did.  To reduce
> symbol table sizes, a configure check is added to avoid these
> aliases if they are not needed.

totalorder / totalordermag implementations also use something similar to 
compat_symbol_unique (but predating it) to create aliases and should 
probably also be changed not to do so, whether in this patch or a 
followup.

(Finding all the other places using aliases without __COUNTER__ for symbol 
version definitions may be harder.)

> diff --git a/configure.ac b/configure.ac
> index 16b15b6f90..5b3440511b 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1673,6 +1673,27 @@ elif test "$libc_cv_ehdr_start" = broken; then
>    AC_MSG_WARN([linker is broken -- you should upgrade])
>  fi
>  
> +AC_CACHE_CHECK(whether the assembler requires one version per symbol,
> +               libc_cv_symver_needs_alias, [dnl

I think either the configure test, or the code using it, should have a 
comment saying when the binutils issue was fixed, so it's easy to tell 
when the test and conditionals are obsolete and can be removed.  It's 
binutils bug 23840, so referencing that bug in the comment and saying it's 
fixed in binutils 2.35 seems appropriate.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC 2/2] Fold compat_symbol_unique functionality into compat_symbol

[Adhemerval Zanella]

On 18/03/2021 17:07, Florian Weimer via Libc-alpha wrote:
> This eliminates the need for intermediate aliases for defining
> multiple symbol versions, for both compat_symbol and
> versioned_symbol.  Some binutils versions do not suport multiple
> versions per symbol on some targets, so aliases are automatically
> introduced, similar to what compat_symbol_unique did.  To reduce
> symbol table sizes, a configure check is added to avoid these
> aliases if they are not needed.

This is indeed better and more flexible than compat_symbol_unique.
> 
> The new mechanism works with data symbols as well as function symbols,
> due to the way an assembler-level redirect is used.  It is not
> compatible with weak symbols for old binutils versions, which is why
> the definition of __malloc_initialize_hook had to be changed.  This
> is not a loss of functionality because weak symbols do not matter
> to dynamic linking.
> 
> The placeholder symbol needs repeating in nptl/libpthread-compat.c
> now that compat_symbol is used, but that seems more obvious than
> introducing yet another macro.
> 
> A subtle difference was that compat_symbol_unique made the symbol
> global automatically.  compat_symbol does not do this, so static
> had to be removed from the definition of
> __libpthread_version_placeholder.
> 
> In locale/lc-ctype.c, it seems prudent to switch to
> compat_symbol_reference.  It produces .symver directives directly, as
> before.  This side-steps the question of copy relocations, as
> mentioned in the comment.

Maybe move this change on a subsequent patch?

> 
> To verify the versioned_symbol functionality, the explicit aliases
> are eliminated from the clock_* function definitions.

LGTM with the Joseph's note about the comment referencing the binutils
bug.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  config.h.in                             |  4 ++
>  configure                               | 28 ++++++++
>  configure.ac                            | 21 ++++++
>  include/libc-symbols.h                  |  5 +-
>  include/shlib-compat.h                  | 52 ++++-----------
>  locale/lc-ctype.c                       | 14 ++--
>  malloc/malloc.c                         |  2 +-
>  nptl/libpthread-compat.c                | 16 ++---
>  sysdeps/generic/libc-symver.h           | 88 +++++++++++++++++++++++++
>  {include => sysdeps/ia64}/libc-symver.h | 29 ++++----
>  time/clock_getcpuclockid.c              |  3 +-
>  time/clock_getres.c                     |  3 +-
>  time/clock_gettime.c                    |  3 +-
>  time/clock_nanosleep.c                  |  3 +-
>  time/clock_settime.c                    |  3 +-
>  15 files changed, 192 insertions(+), 82 deletions(-)
>  create mode 100644 sysdeps/generic/libc-symver.h
>  rename {include => sysdeps/ia64}/libc-symver.h (51%)
> 
> diff --git a/config.h.in b/config.h.in
> index f21bf04e47..ca1547ae67 100644
> --- a/config.h.in
> +++ b/config.h.in
> @@ -190,6 +190,10 @@
>  /* Define if the linker defines __ehdr_start.  */
>  #undef HAVE_EHDR_START
>  
> +/* Define to 1 if the assembler needs intermediate aliases to define
> +   multiple symbol versions for one symbol.  */
> +#define SYMVER_NEEDS_ALIAS 0
> +
>  /*
>  \f */
>  

Ok.

> diff --git a/configure b/configure
> index 37cef37413..bf33439a7f 100755
> --- a/configure
> +++ b/configure
> @@ -6590,6 +6590,34 @@ elif test "$libc_cv_ehdr_start" = broken; then
>  $as_echo "$as_me: WARNING: linker is broken -- you should upgrade" >&2;}
>  fi
>  
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether the assembler requires one version per symbol" >&5
> +$as_echo_n "checking whether the assembler requires one version per symbol... " >&6; }
> +if ${libc_cv_symver_needs_alias+:} false; then :
> +  $as_echo_n "(cached) " >&6
> +else
> +    cat > conftest.s <<EOF
> +        .text
> +testfunc:
> +        .globl testfunc
> +        .symver testfunc, testfunc1@VERSION1
> +        .symver testfunc, testfunc1@VERSION2
> +EOF
> +  libc_cv_symver_needs_alias=no
> +  if ${CC-cc} $ASFLAGS -c conftest.s 2>&5; then
> +    libc_cv_symver_needs_alias=no
> +  else
> +    libc_cv_symver_needs_alias=yes
> +  fi
> +  rm conftest.*
> +
> +fi
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_symver_needs_alias" >&5
> +$as_echo "$libc_cv_symver_needs_alias" >&6; }
> +if test "$libc_cv_symver_needs_alias" = yes; then
> +  $as_echo "#define SYMVER_NEEDS_ALIAS 1" >>confdefs.h
> +
> +fi
> +
>  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for __builtin_trap with no external dependencies" >&5
>  $as_echo_n "checking for __builtin_trap with no external dependencies... " >&6; }
>  if ${libc_cv_builtin_trap+:} false; then :
> diff --git a/configure.ac b/configure.ac
> index 16b15b6f90..5b3440511b 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1673,6 +1673,27 @@ elif test "$libc_cv_ehdr_start" = broken; then
>    AC_MSG_WARN([linker is broken -- you should upgrade])
>  fi
>  
> +AC_CACHE_CHECK(whether the assembler requires one version per symbol,
> +               libc_cv_symver_needs_alias, [dnl
> +  cat > conftest.s <<EOF
> +        .text
> +testfunc:
> +        .globl testfunc
> +        .symver testfunc, testfunc1@VERSION1
> +        .symver testfunc, testfunc1@VERSION2
> +EOF
> +  libc_cv_symver_needs_alias=no
> +  if ${CC-cc} $ASFLAGS -c conftest.s 2>&AS_MESSAGE_LOG_FD; then
> +    libc_cv_symver_needs_alias=no
> +  else
> +    libc_cv_symver_needs_alias=yes
> +  fi
> +  rm conftest.*
> +])
> +if test "$libc_cv_symver_needs_alias" = yes; then
> +  AC_DEFINE(SYMVER_NEEDS_ALIAS)
> +fi
> +
>  AC_CACHE_CHECK(for __builtin_trap with no external dependencies,
>  	       libc_cv_builtin_trap, [dnl
>  libc_cv_builtin_trap=no

Ok, please add the comment about the binutills issue.

> diff --git a/include/libc-symbols.h b/include/libc-symbols.h
> index ce5f75a1a2..546fc26a7b 100644
> --- a/include/libc-symbols.h
> +++ b/include/libc-symbols.h
> @@ -404,12 +404,13 @@ for linking")
>    symbol_version_reference(real, name, version)
>  # define default_symbol_version(real, name, version) \
>       _default_symbol_version(real, name, version)
> +/* See <libc-symver.h>.  */
>  # ifdef __ASSEMBLER__
>  #  define _default_symbol_version(real, name, version) \
> -     .symver real, name##@##@##version
> +  _set_symbol_version (real, name@@version)
>  # else
>  #  define _default_symbol_version(real, name, version) \
> -     __asm__ (".symver " #real "," #name "@@" #version)
> +  _set_symbol_version (real, #name "@@" #version)
>  # endif
>  
>  /* Evalutes to a string literal for VERSION in LIB.  */

Ok.

> diff --git a/include/shlib-compat.h b/include/shlib-compat.h
> index b874e2588f..f4978c05af 100644
> --- a/include/shlib-compat.h
> +++ b/include/shlib-compat.h
> @@ -70,43 +70,20 @@
>    default_symbol_version (local, symbol, name)
>  
>  # define compat_symbol(lib, local, symbol, version) \
> -  compat_symbol_reference (lib, local, symbol, version)
> -
> -/* This is similar to compat_symbol, but allows versioning the same symbol
> -   to multiple version without having multiple symbol definitions.  For
> -   instance:
> -
> -   #if (SHLIB_COMPAT (libpthread, GLIBC_2_1_2, GLIBC_2_2))
> -   compat_symbol_unique (libc, old_foo, GLIBC_2_1_2)
> -   #endif
> -
> -   #if (SHLIB_COMPAT (libpthread, GLIBC_2_2_6, GLIBC_2_3))
> -   compat_symbol_unique (libc, old_foo, GLIBC_2_2_6)
> -   #endif
> -
> -   Internally it creates a unique strong alias to the input symbol and
> -   creates one compat_symbol on the alias.  Using the above example,
> -   it is similar to:
> -
> -   #if (SHLIB_COMPAT (libpthread, GLIBC_2_1_2, GLIBC_2_2))
> -   strong_alias (old_foo, old_foo__COUNTER__)
> -   compat_symbol (libc, old_foo__COUNTER__, foo, GLIBC_2_2)
> -   #endif.
> -
> -   With __COUNTER__ being a monotonic number generated by the compiler.  */
> -
> -# define __compat_symbol_unique_concat(x, y) x ## y
> -# define _compat_symbol_unique_concat(x, y) \
> -  __compat_symbol_unique_concat (x, y)
> -# define _compat_symbol_unique_alias(name) \
> -  _compat_symbol_unique_concat (name, __COUNTER__)
> -# define _compat_symbol_unique(lib, orig_name, name, version) \
> -  strong_alias (orig_name, name) \
> -  compat_symbol (lib, name, orig_name, version)
> -# define compat_symbol_unique(lib, name, version) \
> -  _compat_symbol_unique (lib, name, _compat_symbol_unique_alias (name), \
> -                         version)
> -

Ok.

> +  compat_symbol_1 (lib, local, symbol, version)
> +# define compat_symbol_1(lib, local, symbol, version) \
> +  compat_symbol_2 (local, symbol, VERSION_##lib##_##version)
> +
> +/* See <libc-symver.h>.  */
> +# ifdef __ASSEMBLER__
> +#define compat_symbol_2(local, symbol, name) \
> +  _set_symbol_version (local, symbol@name)
> +# else
> +#  define compat_symbol_2(local, symbol, name) \
> +  compat_symbol_3 (local, symbol, name)
> +#  define compat_symbol_3(local, symbol, name) \
> +  _set_symbol_version (local, #symbol "@" #name)
> +# endif
>  #else
>  

Ok.

>  /* Not compiling ELF shared libraries at all, so never any old versions.  */
> @@ -118,7 +95,6 @@
>  
>  /* This should not appear outside `#if SHLIB_COMPAT (...)'.  */
>  # define compat_symbol(lib, local, symbol, version) ...
> -# define compat_symbol_unique(lib, name, version) ...
>  
>  #endif
>  

Ok.

> diff --git a/locale/lc-ctype.c b/locale/lc-ctype.c
> index 1db0605c82..7c97480cbd 100644
> --- a/locale/lc-ctype.c
> +++ b/locale/lc-ctype.c
> @@ -93,12 +93,14 @@ _nl_postload_ctype (void)
>       We need those relocations so that a versioned definition with a COPY
>       reloc in an executable will override the libc.so definition.  */
>  
> -compat_symbol (libc, __ctype_b, __ctype_b, GLIBC_2_0);
> -compat_symbol (libc, __ctype_tolower, __ctype_tolower, GLIBC_2_0);
> -compat_symbol (libc, __ctype_toupper, __ctype_toupper, GLIBC_2_0);
> -compat_symbol (libc, __ctype32_b, __ctype32_b, GLIBC_2_0);
> -compat_symbol (libc, __ctype32_tolower, __ctype32_tolower, GLIBC_2_2);
> -compat_symbol (libc, __ctype32_toupper, __ctype32_toupper, GLIBC_2_2);
> +compat_symbol_reference (libc, __ctype_b, __ctype_b, GLIBC_2_0);
> +compat_symbol_reference (libc, __ctype_tolower, __ctype_tolower, GLIBC_2_0);
> +compat_symbol_reference (libc, __ctype_toupper, __ctype_toupper, GLIBC_2_0);
> +compat_symbol_reference (libc, __ctype32_b, __ctype32_b, GLIBC_2_0);
> +compat_symbol_reference (libc, __ctype32_tolower, __ctype32_tolower,
> +			  GLIBC_2_2);
> +compat_symbol_reference (libc, __ctype32_toupper, __ctype32_toupper,
> +			 GLIBC_2_2);
>  
>    __ctype_b = current (uint16_t, CLASS, 128);
>    __ctype_toupper = current (int32_t, TOUPPER, 128);

Ok.

> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index 1f4bbd8edf..f23027c13d 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -1991,7 +1991,7 @@ static void *memalign_hook_ini (size_t alignment, size_t sz,
>                                  const void *caller) __THROW;
>  
>  #if HAVE_MALLOC_INIT_HOOK
> -void weak_variable (*__malloc_initialize_hook) (void) = NULL;
> +void (*__malloc_initialize_hook) (void);
>  compat_symbol (libc, __malloc_initialize_hook,
>  	       __malloc_initialize_hook, GLIBC_2_0);
>  #endif

Ok.

> diff --git a/nptl/libpthread-compat.c b/nptl/libpthread-compat.c
> index 820dcd6a8f..da537af76e 100644
> --- a/nptl/libpthread-compat.c
> +++ b/nptl/libpthread-compat.c
> @@ -20,10 +20,10 @@
>  #include <shlib-compat.h>
>  
>  #ifdef SHARED
> -static void
> +void
>  attribute_compat_text_section
>  __attribute_used__
> -__libpthread_version_placeholder (void)
> +__libpthread_version_placeholder_1 (void)
>  {
>  }
>  #endif

I don't think there is the need to change the symbol name, on my
test the compat_symbol can origin and alias can be the same name.

> @@ -37,16 +37,16 @@ __libpthread_version_placeholder (void)
>     there are plenty of other symbols which populate those later
>     versions.  */
>  #if (SHLIB_COMPAT (libpthread, GLIBC_2_1_2, GLIBC_2_2))
> -compat_symbol_unique (libpthread,
> -		      __libpthread_version_placeholder, GLIBC_2_1_2);
> +compat_symbol (libpthread, __libpthread_version_placeholder_1,
> +	       __libpthread_version_placeholder, GLIBC_2_1_2);
>  #endif
>  
>  #if (SHLIB_COMPAT (libpthread, GLIBC_2_2_3, GLIBC_2_2_4))
> -compat_symbol_unique (libpthread,
> -		      __libpthread_version_placeholder, GLIBC_2_2_3);
> +compat_symbol (libpthread, __libpthread_version_placeholder_1,
> +	       __libpthread_version_placeholder, GLIBC_2_2_3);
>  #endif
>  
>  #if (SHLIB_COMPAT (libpthread, GLIBC_2_2_6, GLIBC_2_3))
> -compat_symbol_unique (libpthread,
> -		      __libpthread_version_placeholder, GLIBC_2_2_6);
> +compat_symbol (libpthread, __libpthread_version_placeholder_1,
> +	       __libpthread_version_placeholder, GLIBC_2_2_6);
>  #endif
> diff --git a/sysdeps/generic/libc-symver.h b/sysdeps/generic/libc-symver.h
> new file mode 100644
> index 0000000000..69d147e2a8
> --- /dev/null
> +++ b/sysdeps/generic/libc-symver.h
> @@ -0,0 +1,88 @@
> +/* Symbol version management.
> +   Copyright (C) 1995-2021 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +/* This file is included from <libc-symbols.h> for !_ISOMAC, and
> +   unconditionally from <shlib-compat.h>.  */
> +
> +#ifndef _LIBC_SYMVER_H
> +#define _LIBC_SYMVER_H 1
> +
> +#include <config.h>
> +
> +/* Use symbol_version_reference to specify the version a symbol
> +   reference should link to.  Use symbol_version or
> +   default_symbol_version for the definition of a versioned symbol.
> +   The difference is that the latter is a no-op in non-shared
> +   builds.
> +
> +   _set_symbol_version is similar to symbol_version_reference, except
> +   that this macro expects the name and symbol version as a single
> +   string or token sequence, with an @ or @@ separator.  (A string is
> +   used in C mode and a token sequence in assembler mode.)
> +   _set_symbol_version only be used for definitions because it may
> +   introduce an alias symbol that would not be globally unique for
> +   mere references.  The _set_symbol_version macro is used to define
> +   default_symbol_version and compat_symbol.  */
> +
> +#ifdef __ASSEMBLER__
> +# define symbol_version_reference(real, name, version) \
> +     .symver real, name##@##version
> +#else
> +# define symbol_version_reference(real, name, version) \
> +  __asm__ (".symver " #real "," #name "@" #version)
> +#endif  /* !__ASSEMBLER__ */
> +
> +#if SYMVER_NEEDS_ALIAS
> +/* If the assembler cannot support multiple versions for the same
> +   symbol, introduce __SInnn_ aliases to which the symbol version is
> +   attached.  */
> +# define __symbol_version_unique_concat(x, y) __SI ## x ## _ ## y
> +# define _symbol_version_unique_concat(x, y) \
> +  __symbol_version_unique_concat (x, y)
> +# define _symbol_version_unique_alias(name) \
> +  _symbol_version_unique_concat (name, __COUNTER__)
> +# ifdef __ASSEMBLER__
> +#  define _set_symbol_version_2(real, alias, name_version) \
> +  .globl alias ASM_LINE_SEP                                \
> +  .equiv alias, real ASM_LINE_SEP                          \
> +  .symver alias, name_version
> +# else
> +#  define _set_symbol_version_2(real, alias, name_version) \
> +  __asm__ (".globl " #alias "\n\t"                         \
> +           ".equiv " #alias ", " #real "\n\t"              \
> +           ".symver " #alias "," name_version)
> +# endif
> +# define _set_symbol_version_1(real, alias, name_version) \
> +  _set_symbol_version_2 (real, alias, name_version)
> +/* REAL must be globally unique, so that the counter also produces
> +   globally unique symbols.  */
> +# define _set_symbol_version(real, name_version)                   \
> +  _set_symbol_version_1 (real, _symbol_version_unique_alias (real), \
> +                               name_version)
> +# else  /* !SYMVER_NEEDS_ALIAS */
> +# ifdef __ASSEMBLER__
> +#  define _set_symbol_version(real, name_version) \
> +  .symver real, name_version
> +# else
> +#  define _set_symbol_version(real, name_version) \
> +  __asm__ (".symver " #real "," name_version)
> +# endif
> +#endif  /* !SYMVER_NEEDS_ALIAS */
> +
> +
> +#endif /* _LIBC_SYMVER_H */

Ok.

> diff --git a/include/libc-symver.h b/sysdeps/ia64/libc-symver.h
> similarity index 51%
> rename from include/libc-symver.h
> rename to sysdeps/ia64/libc-symver.h
> index 11c77ae1cd..63a3e58dde 100644
> --- a/include/libc-symver.h
> +++ b/sysdeps/ia64/libc-symver.h
> @@ -1,5 +1,5 @@
> -/* Symbol version management.
> -   Copyright (C) 1995-2021 Free Software Foundation, Inc.
> +/* Symbol version management.  ia64 version.
> +   Copyright (C) 2021 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>  
>     The GNU C Library is free software; you can redistribute it and/or
> @@ -16,23 +16,18 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> -/* This file is included from <libc-symbols.h> for !_ISOMAC, and
> -   unconditionally from <shlib-compat.h>.  */
> -
>  #ifndef _LIBC_SYMVER_H
> -#define _LIBC_SYMVER_H 1
>  
> -/* Use symbol_version_reference to specify the version a symbol
> -   reference should link to.  Use symbol_version or
> -   default_symbol_version for the definition of a versioned symbol.
> -   The difference is that the latter is a no-op in non-shared
> -   builds.  */
> -#ifdef __ASSEMBLER__
> -# define symbol_version_reference(real, name, version) \
> -     .symver real, name##@##version
> -#else  /* !__ASSEMBLER__ */
> -# define symbol_version_reference(real, name, version) \
> -  __asm__ (".symver " #real "," #name "@" #version)
> +#include <sysdeps/generic/libc-symver.h>
> +
> +/* ia64 recognizes loc1 as a register name.  Add the # suffix to all
> +   symbol references.  */
> +#if !defined (__ASSEMBLER__) && SYMVER_NEEDS_ALIAS
> +#undef _set_symbol_version_2
> +# define _set_symbol_version_2(real, alias, name_version) \
> +  __asm__ (".globl " #alias "#\n\t"                         \
> +           ".equiv " #alias ", " #real "#\n\t"              \
> +           ".symver " #alias "#," name_version)
>  #endif
>  
>  #endif /* _LIBC_SYMVER_H */

Ok.

> diff --git a/time/clock_getcpuclockid.c b/time/clock_getcpuclockid.c
> index c148d96c5c..220e2eb016 100644
> --- a/time/clock_getcpuclockid.c
> +++ b/time/clock_getcpuclockid.c
> @@ -42,6 +42,5 @@ versioned_symbol (libc, __clock_getcpuclockid, clock_getcpuclockid, GLIBC_2_17);
>  /* clock_getcpuclockid moved to libc in version 2.17;
>     old binaries may expect the symbol version it had in librt.  */
>  #if SHLIB_COMPAT (libc, GLIBC_2_2, GLIBC_2_17)
> -strong_alias (__clock_getcpuclockid, __clock_getcpuclockid_2);
> -compat_symbol (libc, __clock_getcpuclockid_2, clock_getcpuclockid, GLIBC_2_2);
> +compat_symbol (libc, __clock_getcpuclockid, clock_getcpuclockid, GLIBC_2_2);
>  #endif

Ok.

> diff --git a/time/clock_getres.c b/time/clock_getres.c
> index 4b31893849..9099b62672 100644
> --- a/time/clock_getres.c
> +++ b/time/clock_getres.c
> @@ -32,8 +32,7 @@ versioned_symbol (libc, __clock_getres, clock_getres, GLIBC_2_17);
>  /* clock_getres moved to libc in version 2.17;
>     old binaries may expect the symbol version it had in librt.  */
>  #if SHLIB_COMPAT (libc, GLIBC_2_2, GLIBC_2_17)
> -strong_alias (__clock_getres, __clock_getres_2);
> -compat_symbol (libc, __clock_getres_2, clock_getres, GLIBC_2_2);
> +compat_symbol (libc, __clock_getres, clock_getres, GLIBC_2_2);
>  #endif
>  
>  stub_warning (clock_getres)

Ok.

> diff --git a/time/clock_gettime.c b/time/clock_gettime.c
> index fdeaaca3f8..e8e129d201 100644
> --- a/time/clock_gettime.c
> +++ b/time/clock_gettime.c
> @@ -33,8 +33,7 @@ versioned_symbol (libc, __clock_gettime, clock_gettime, GLIBC_2_17);
>  /* clock_gettime moved to libc in version 2.17;
>     old binaries may expect the symbol version it had in librt.  */
>  #if SHLIB_COMPAT (libc, GLIBC_2_2, GLIBC_2_17)
> -strong_alias (__clock_gettime, __clock_gettime_2);
> -compat_symbol (libc, __clock_gettime_2, clock_gettime, GLIBC_2_2);
> +compat_symbol (libc, __clock_gettime, clock_gettime, GLIBC_2_2);
>  #endif
>  
>  stub_warning (clock_gettime)

Ok.

> diff --git a/time/clock_nanosleep.c b/time/clock_nanosleep.c
> index 7ecb1cfcb8..57b3af2a70 100644
> --- a/time/clock_nanosleep.c
> +++ b/time/clock_nanosleep.c
> @@ -38,8 +38,7 @@ versioned_symbol (libc, __clock_nanosleep, clock_nanosleep, GLIBC_2_17);
>  /* clock_nanosleep moved to libc in version 2.17;
>     old binaries may expect the symbol version it had in librt.  */
>  #if SHLIB_COMPAT (libc, GLIBC_2_2, GLIBC_2_17)
> -strong_alias (__clock_nanosleep, __clock_nanosleep_2);
> -compat_symbol (libc, __clock_nanosleep_2, clock_nanosleep, GLIBC_2_2);
> +compat_symbol (libc, __clock_nanosleep, clock_nanosleep, GLIBC_2_2);
>  #endif
>  
>  stub_warning (clock_nanosleep)

Ok.

> diff --git a/time/clock_settime.c b/time/clock_settime.c
> index 7676aaeb23..4df4ec56d1 100644
> --- a/time/clock_settime.c
> +++ b/time/clock_settime.c
> @@ -33,8 +33,7 @@ versioned_symbol (libc, __clock_settime, clock_settime, GLIBC_2_17);
>  /* clock_settime moved to libc in version 2.17;
>     old binaries may expect the symbol version it had in librt.  */
>  #if SHLIB_COMPAT (libc, GLIBC_2_2, GLIBC_2_17)
> -strong_alias (__clock_settime, __clock_settime_2);
> -compat_symbol (libc, __clock_settime_2, clock_settime, GLIBC_2_2);
> +compat_symbol (libc, __clock_settime, clock_settime, GLIBC_2_2);
>  #endif
>  
>  stub_warning (clock_settime)
> 

Ok.

Re: [RFC 2/2] Fold compat_symbol_unique functionality into compat_symbol

[Florian Weimer]

* Adhemerval Zanella:

>> In locale/lc-ctype.c, it seems prudent to switch to
>> compat_symbol_reference.  It produces .symver directives directly, as
>> before.  This side-steps the question of copy relocations, as
>> mentioned in the comment.
>
> Maybe move this change on a subsequent patch?

It's actually necessary to avoid build failures on some m68k variants,
which duplicate the inline asm (it's in a function, so the compiler can
do that), resulting in build failures due to .equiv instead of .set.  (I
switched to .equiv because some assemblers use .set for something
unrelated to symbols.)

I can make the change before this patch, though.

>> diff --git a/nptl/libpthread-compat.c b/nptl/libpthread-compat.c
>> index 820dcd6a8f..da537af76e 100644
>> --- a/nptl/libpthread-compat.c
>> +++ b/nptl/libpthread-compat.c
>> @@ -20,10 +20,10 @@
>>  #include <shlib-compat.h>
>>  
>>  #ifdef SHARED
>> -static void
>> +void
>>  attribute_compat_text_section
>>  __attribute_used__
>> -__libpthread_version_placeholder (void)
>> +__libpthread_version_placeholder_1 (void)
>>  {
>>  }
>>  #endif
>
> I don't think there is the need to change the symbol name, on my
> test the compat_symbol can origin and alias can be the same name.

It's necessary because the symbol version is no longer attached to the
symbol, so it's version-less and gets exported with the default
baseline, which could be something like GLIBC_2.17.  I can mention this
in the commit message.

Thanks,
Florian

Re: [RFC 2/2] Fold compat_symbol_unique functionality into compat_symbol

[Adhemerval Zanella]

On 19/03/2021 16:13, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>>> In locale/lc-ctype.c, it seems prudent to switch to
>>> compat_symbol_reference.  It produces .symver directives directly, as
>>> before.  This side-steps the question of copy relocations, as
>>> mentioned in the comment.
>>
>> Maybe move this change on a subsequent patch?
> 
> It's actually necessary to avoid build failures on some m68k variants,
> which duplicate the inline asm (it's in a function, so the compiler can
> do that), resulting in build failures due to .equiv instead of .set.  (I
> switched to .equiv because some assemblers use .set for something
> unrelated to symbols.)
> 
> I can make the change before this patch, though.

I think it would be better indeed.

> 
>>> diff --git a/nptl/libpthread-compat.c b/nptl/libpthread-compat.c
>>> index 820dcd6a8f..da537af76e 100644
>>> --- a/nptl/libpthread-compat.c
>>> +++ b/nptl/libpthread-compat.c
>>> @@ -20,10 +20,10 @@
>>>  #include <shlib-compat.h>
>>>  
>>>  #ifdef SHARED
>>> -static void
>>> +void
>>>  attribute_compat_text_section
>>>  __attribute_used__
>>> -__libpthread_version_placeholder (void)
>>> +__libpthread_version_placeholder_1 (void)
>>>  {
>>>  }
>>>  #endif
>>
>> I don't think there is the need to change the symbol name, on my
>> test the compat_symbol can origin and alias can be the same name.
> 
> It's necessary because the symbol version is no longer attached to the
> symbol, so it's version-less and gets exported with the default
> baseline, which could be something like GLIBC_2.17.  I can mention this
> in the commit message.

I think it should be documented on compat_symbol as well about this
limitation.

Re: [RFC 2/2] Fold compat_symbol_unique functionality into compat_symbol

[Adhemerval Zanella]

On 19/03/2021 15:59, Adhemerval Zanella wrote:
>> diff --git a/malloc/malloc.c b/malloc/malloc.c
>> index 1f4bbd8edf..f23027c13d 100644
>> --- a/malloc/malloc.c
>> +++ b/malloc/malloc.c
>> @@ -1991,7 +1991,7 @@ static void *memalign_hook_ini (size_t alignment, size_t sz,
>>                                  const void *caller) __THROW;
>>  
>>  #if HAVE_MALLOC_INIT_HOOK
>> -void weak_variable (*__malloc_initialize_hook) (void) = NULL;
>> +void (*__malloc_initialize_hook) (void);
>>  compat_symbol (libc, __malloc_initialize_hook,
>>  	       __malloc_initialize_hook, GLIBC_2_0);
>>  #endif
> 
> Ok.
> 

I am seeing this with binutils 2.33 on i686-linux-gnu:

x86_64-glibc-linux-gnu-gcc -m32 -march=i686 malloc.c -c -std=gnu11 -fgnu89-inline  -g -O2 -Wall -Wwrite-strings -Wundef -Werror -fmerge-all-constants -frounding-math -fno-stack-protector -Wstrict-prototypes -Wold-style-definition -fmath-errno   -fPIC -Wa,-mtune=i686  -DMORECORE_CLEARS=2  -ftls-model=initial-exec      -I../include -I/home/azanella/Projects/glibc/build/i686-linux-gnu/malloc  -I/home/azanella/Projects/glibc/build/i686-linux-gnu  -I../sysdeps/unix/sysv/linux/i386/i686  -I../sysdeps/i386/i686/nptl  -I../sysdeps/unix/sysv/linux/i386  -I../sysdeps/unix/sysv/linux/x86/include -I../sysdeps/unix/sysv/linux/x86  -I../sysdeps/x86/nptl  -I../sysdeps/i386/nptl  -I../sysdeps/unix/sysv/linux/include -I../sysdeps/unix/sysv/linux  -I../sysdeps/nptl  -I../sysdeps/pthread  -I../sysdeps/gnu  -I../sysdeps/unix/inet  -I../sysdeps/unix/sysv  -I../sysdeps/unix/i386  -I../sysdeps/unix  -I../sysdeps/posix  -I../sysdeps/i386/i686/fpu/multiarch  -I../sysdeps/i386/i686/fpu  -I../sysdeps/i386/i686/multiarch  -I../sysdeps/i386/i686  -I../sysdeps/i386/fpu  -I../sysdeps/x86/fpu  -I../sysdeps/i386  -I../sysdeps/x86/include -I../sysdeps/x86  -I../sysdeps/wordsize-32  -I../sysdeps/ieee754/float128  -I../sysdeps/ieee754/ldbl-96/include -I../sysdeps/ieee754/ldbl-96  -I../sysdeps/ieee754/dbl-64  -I../sysdeps/ieee754/flt-32  -I../sysdeps/ieee754  -I../sysdeps/generic  -I.. -I../libio -I.   -D_LIBC_REENTRANT -include /home/azanella/Projects/glibc/build/i686-linux-gnu/libc-modules.h -DMODULE_NAME=libc -include ../include/libc-symbols.h  -DPIC -DSHARED  -DUSE_TCACHE=1   -DTOP_NAMESPACE=glibc -o /home/azanella/Projects/glibc/build/i686-linux-gnu/malloc/malloc.os -MD -MP -MF /home/azanella/Projects/glibc/build/i686-linux-gnu/malloc/malloc.os.dt -MT /home/azanella/Projects/glibc/build/i686-linux-gnu/malloc/malloc.os
/tmp/ccrOfJvr.s: Assembler messages:
/tmp/ccrOfJvr.s: Error: `__SI__malloc_initialize_hook_0' can't be equated to common symbol `__malloc_initialize_hook'

The SYMVER_NEEDS_ALIAS is set to 1:

$ grep -w SYMVER_NEEDS_ALIAS config.h 
#define SYMVER_NEEDS_ALIAS 1

Re: [RFC 2/2] Fold compat_symbol_unique functionality into compat_symbol

[Florian Weimer]

* Adhemerval Zanella:

> I am seeing this with binutils 2.33 on i686-linux-gnu:
>
> x86_64-glibc-linux-gnu-gcc -m32 -march=i686 malloc.c -c -std=gnu11 -fgnu89-inline  -g -O2 -Wall -Wwrite-strings -Wundef -Werror -fmerge-all-constants -frounding-math -fno-stack-protector -Wstrict-prototypes -Wold-style-definition -fmath-errno   -fPIC -Wa,-mtune=i686  -DMORECORE_CLEARS=2  -ftls-model=initial-exec      -I../include -I/home/azanella/Projects/glibc/build/i686-linux-gnu/malloc  -I/home/azanella/Projects/glibc/build/i686-linux-gnu  -I../sysdeps/unix/sysv/linux/i386/i686  -I../sysdeps/i386/i686/nptl  -I../sysdeps/unix/sysv/linux/i386  -I../sysdeps/unix/sysv/linux/x86/include -I../sysdeps/unix/sysv/linux/x86  -I../sysdeps/x86/nptl  -I../sysdeps/i386/nptl  -I../sysdeps/unix/sysv/linux/include -I../sysdeps/unix/sysv/linux  -I../sysdeps/nptl  -I../sysdeps/pthread  -I../sysdeps/gnu  -I../sysdeps/unix/inet  -I../sysdeps/unix/sysv  -I../sysdeps/unix/i386  -I../sysdeps/unix  -I../sysdeps/posix  -I../sysdeps/i386/i686/fpu/multiarch  -I../sysdeps/i386/i686/fpu  -I../sysdeps/i386/i686/multiarch  -I../sysdeps/i386/i686  -I../sysdeps/i386/fpu  -I../sysdeps/x86/fpu  -I../sysdeps/i386  -I../sysdeps/x86/include -I../sysdeps/x86  -I../sysdeps/wordsize-32  -I../sysdeps/ieee754/float128  -I../sysdeps/ieee754/ldbl-96/include -I../sysdeps/ieee754/ldbl-96  -I../sysdeps/ieee754/dbl-64  -I../sysdeps/ieee754/flt-32  -I../sysdeps/ieee754  -I../sysdeps/generic  -I.. -I../libio -I.   -D_LIBC_REENTRANT -include /home/azanella/Projects/glibc/build/i686-linux-gnu/libc-modules.h -DMODULE_NAME=libc -include ../include/libc-symbols.h  -DPIC -DSHARED  -DUSE_TCACHE=1   -DTOP_NAMESPACE=glibc -o /home/azanella/Projects/glibc/build/i686-linux-gnu/malloc/malloc.os -MD -MP -MF /home/azanella/Projects/glibc/build/i686-linux-gnu/malloc/malloc.os.dt -MT /home/azanella/Projects/glibc/build/i686-linux-gnu/malloc/malloc.os
> /tmp/ccrOfJvr.s: Assembler messages:
> /tmp/ccrOfJvr.s: Error: `__SI__malloc_initialize_hook_0' can't be equated to common symbol `__malloc_initialize_hook'

Ahh, the GCC version is actually relevant here.  It needs __attribute__
((nocommon)).  I will at it.

Are there any other symbols with the same problem?

Thanks,
Florian

Re: [RFC 2/2] Fold compat_symbol_unique functionality into compat_symbol

[Adhemerval Zanella]

On 19/03/2021 17:17, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> I am seeing this with binutils 2.33 on i686-linux-gnu:
>>
>> x86_64-glibc-linux-gnu-gcc -m32 -march=i686 malloc.c -c -std=gnu11 -fgnu89-inline  -g -O2 -Wall -Wwrite-strings -Wundef -Werror -fmerge-all-constants -frounding-math -fno-stack-protector -Wstrict-prototypes -Wold-style-definition -fmath-errno   -fPIC -Wa,-mtune=i686  -DMORECORE_CLEARS=2  -ftls-model=initial-exec      -I../include -I/home/azanella/Projects/glibc/build/i686-linux-gnu/malloc  -I/home/azanella/Projects/glibc/build/i686-linux-gnu  -I../sysdeps/unix/sysv/linux/i386/i686  -I../sysdeps/i386/i686/nptl  -I../sysdeps/unix/sysv/linux/i386  -I../sysdeps/unix/sysv/linux/x86/include -I../sysdeps/unix/sysv/linux/x86  -I../sysdeps/x86/nptl  -I../sysdeps/i386/nptl  -I../sysdeps/unix/sysv/linux/include -I../sysdeps/unix/sysv/linux  -I../sysdeps/nptl  -I../sysdeps/pthread  -I../sysdeps/gnu  -I../sysdeps/unix/inet  -I../sysdeps/unix/sysv  -I../sysdeps/unix/i386  -I../sysdeps/unix  -I../sysdeps/posix  -I../sysdeps/i386/i686/fpu/multiarch  -I../sysdeps/i386/i686/fpu  -I../sysdeps/i386/i686/multiarch  -I../sysdeps/i386/i686  -I../sysdeps/i386/fpu  -I../sysdeps/x86/fpu  -I../sysdeps/i386  -I../sysdeps/x86/include -I../sysdeps/x86  -I../sysdeps/wordsize-32  -I../sysdeps/ieee754/float128  -I../sysdeps/ieee754/ldbl-96/include -I../sysdeps/ieee754/ldbl-96  -I../sysdeps/ieee754/dbl-64  -I../sysdeps/ieee754/flt-32  -I../sysdeps/ieee754  -I../sysdeps/generic  -I.. -I../libio -I.   -D_LIBC_REENTRANT -include /home/azanella/Projects/glibc/build/i686-linux-gnu/libc-modules.h -DMODULE_NAME=libc -include ../include/libc-symbols.h  -DPIC -DSHARED  -DUSE_TCACHE=1   -DTOP_NAMESPACE=glibc -o /home/azanella/Projects/glibc/build/i686-linux-gnu/malloc/malloc.os -MD -MP -MF /home/azanella/Projects/glibc/build/i686-linux-gnu/malloc/malloc.os.dt -MT /home/azanella/Projects/glibc/build/i686-linux-gnu/malloc/malloc.os
>> /tmp/ccrOfJvr.s: Assembler messages:
>> /tmp/ccrOfJvr.s: Error: `__SI__malloc_initialize_hook_0' can't be equated to common symbol `__malloc_initialize_hook'
> 
> Ahh, the GCC version is actually relevant here.  It needs __attribute__
> ((nocommon)).  I will at it.
> 
> Are there any other symbols with the same problem?
> 

The __attribute__ ((nocommon)) on the malloc hook fixed the build issue.

Re: [RFC 2/2] Fold compat_symbol_unique functionality into compat_symbol

[Florian Weimer]

* Adhemerval Zanella:

> The __attribute__ ((nocommon)) on the malloc hook fixed the build issue.

I've fixed this locally, and also added to versioned_symbol:

   If LOCAL is a data symbol and does not have a non-zero initializer,
   it should be defined with __attribute__ ((nocommon)) for
   compatibility with GCC versions that default to -fcommon.  */

Should I posted a v3?

Thanks,
Florian

Re: [RFC 2/2] Fold compat_symbol_unique functionality into compat_symbol

[Adhemerval Zanella]

On 19/03/2021 17:29, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> The __attribute__ ((nocommon)) on the malloc hook fixed the build issue.
> 
> I've fixed this locally, and also added to versioned_symbol:
> 
>    If LOCAL is a data symbol and does not have a non-zero initializer,
>    it should be defined with __attribute__ ((nocommon)) for
>    compatibility with GCC versions that default to -fcommon.  */
> 
> Should I posted a v3?

I don't think is a need, I will review the v2.

Re: [RFC 2/2] Fold compat_symbol_unique functionality into compat_symbol

[Adhemerval Zanella]

On 19/03/2021 17:29, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> The __attribute__ ((nocommon)) on the malloc hook fixed the build issue.
> 
> I've fixed this locally, and also added to versioned_symbol:
> 
>    If LOCAL is a data symbol and does not have a non-zero initializer,
>    it should be defined with __attribute__ ((nocommon)) for
>    compatibility with GCC versions that default to -fcommon.  */
> 
> Should I posted a v3?

I don't think is a need, I will review the v2.

Re: [RFC 0/2] Transparent multi-version symbol support

[Adhemerval Zanella]

On 18/03/2021 17:06, Florian Weimer via Libc-alpha wrote:
> This turned out to be way harder than it should be.  Older binutils does
> not support multiple .symver directives for the same first symbol, so we
> have various kludges like (from time/clock_settime.c):
> 
> | versioned_symbol (libc, __clock_settime, clock_settime, GLIBC_2_17);
> | 
> | /* clock_settime moved to libc in version 2.17;
> |    old binaries may expect the symbol version it had in librt.  */
> | #if SHLIB_COMPAT (libc, GLIBC_2_2, GLIBC_2_17)
> | strong_alias (__clock_settime, __clock_settime_2);
> | compat_symbol (libc, __clock_settime_2, clock_settime, GLIBC_2_2);
> | #endif
> 
> I think I have found a way to do this with assembler hacks, so that it
> applies to function and data symbols alike.  With this, the above
> snipper becomes:
> 
> | versioned_symbol (libc, __clock_settime, clock_settime, GLIBC_2_17);
> | 
> | /* clock_settime moved to libc in version 2.17;
> |    old binaries may expect the symbol version it had in librt.  */
> | #if SHLIB_COMPAT (libc, GLIBC_2_2, GLIBC_2_17)
> | compat_symbol (libc, __clock_settime, clock_settime, GLIBC_2_2);
> | #endif
> 
> So the explicit aliases are gone.
> 
> Built with build-many-glibcs.py with binutils 2.36 and 2.34.  Tested on
> i686-linux-gnu and x86_64-linux-gnu with binutils 2.34 and 2.35.
> 
> The object files aren't unchanged due to the __malloc_initialize_hook
> change (and presumably others, I haven't looked closely).

Thanks for working on this, I have checked on the all architectures that
generated abilist and I saw not regression on a make check (with 
run-built-tests=no).

The only detail I am not sure is if SYMVER_NEEDS_ALIAS does yields any
substantial gain (and it require some refactoring and code to handle it).
What kind of improvement do you expect?

Re: [RFC 0/2] Transparent multi-version symbol support

[Florian Weimer]

* Adhemerval Zanella:

> The only detail I am not sure is if SYMVER_NEEDS_ALIAS does yields any
> substantial gain (and it require some refactoring and code to handle it).
> What kind of improvement do you expect?

I think it's desirable not to pollute the symbol table if we can avoid
it because debuggers occasionally show the aliases instead of the main
symbol.

Thanks,
Florian