Re: [PATCH] lib: Use attribute symver when available to define symbol versioning.

[Mark Wielaard <mark@klomp.org>]

[-- Attachment #1: Type: text/plain, Size: 1511 bytes --]

On Tue, 2020-04-14 at 17:25 +0200, Mark Wielaard wrote:
> On Fri, 2020-04-10 at 21:42 +0200, Mark Wielaard wrote:
> > GCC 10 introduces a function attribute to define symbol versioning.
> > Add a configure check to see if __attribute__((symver)) is supported.
> > If it is then define the OLD_VERSION, NEW_VERSION, COMPAT_VERSION
> > and COMPAT_VERSION_NEWPROTO macros using just attribute symver,
> > attribute alias and typeof function names. And avoid defining symbols
> > in asm statements.
> 
> Turns out this almost, but not completely work.
> 
> > +#define NEW_VERSION(name, version) \
> > +  __attribute__((__symver__(#name "@@" #version))) \
> > +  __typeof__ ( name ) name ;
> 
> Using the two @@ variant keeps the original name in the symbol table,
> which will cause that symbol to be doubly marked as a version symbol my
> the linker. We really need the tripple @@@ variant which will actually
> rename and remove the original non-versioned name. But gcc10
> attribute((symver)) only accepts the single and double @ variants.
> 
> I am looking how to work around this. The gcc function attribute
> documentation implies that you shouldn't use the bare non-versioned
> symbol name, but that is precisely what we do (so we don't have to mark
> up each symbol, but can simply rely on the linker version map script).
> 
> Back to the drawing board...

OK, I found a workaround, as attached.
It feels a bit like a hack, but seems to do what is expected.

Cheers,

Mark

[-- Attachment #2: 0001-lib-Use-attribute-symver-when-available-to-define-sy.patch --]
[-- Type: text/x-patch, Size: 6695 bytes --]

From 60f18cd09d771eceb8762af2c46f27ae75b72422 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Wed, 8 Apr 2020 16:11:41 +0200
Subject: [PATCH] lib: Use attribute symver when available to define symbol
 versioning.

GCC 10 introduces a function attribute to define symbol versioning.
Add a configure check to see if __attribute__((symver)) is supported.
If it is then define the OLD_VERSION, NEW_VERSION, COMPAT_VERSION
and COMPAT_VERSION_NEWPROTO macros using just attribute symver,
attribute alias and typeof function names. And avoid defining symbols
in asm statements.

The attribute doesn't accept triple @ symver definitions.
The special foo@@@VERSION_XY version replaces all symbol references to
foo with foo@@VERSION_XY in the file, so there is no bare foo symbol
anymore. When there is still a bare foo symbol the linker creates
a default version for the first section it sees in the version script.
This might create duplicate default versions of the symbol.
we use a trick by placing a non-default alias of the symbol in
ELFUTILS_0 version section (which doesn't have any global symbols).
This prevents the linker from adding an (extra) default version of
the symbol.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 ChangeLog       |  4 ++++
 configure.ac    | 13 +++++++++++++
 lib/ChangeLog   |  5 +++++
 lib/eu-config.h | 23 +++++++++++++++++++++--
 libdw/ChangeLog | 10 ++++++++++
 libdw/libdw.map | 17 ++++-------------
 6 files changed, 57 insertions(+), 15 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 854568e0..bff3bdef 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2020-04-10  Mark Wielaard  <mark@klomp.org>
+
+	* configure.ac: Add __attribute__((symver)) check.
+
 2020-03-30  Mark Wielaard  <mark@klomp.org>
 
 	* configure.ac: Set version to 0.179.
diff --git a/configure.ac b/configure.ac
index a39e800f..2a0aaca8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -158,6 +158,19 @@ if test "$ac_cv_gcc_struct" = "yes"; then
 		  [Defined if __attribute__((gcc_struct)) is supported])
 fi
 
+AC_CACHE_CHECK([whether gcc supports __attribute__((symver))],
+	ac_cv_gcc_symver, [dnl
+save_CFLAGS="$CFLAGS"
+CFLAGS="$save_CFLAGS -Werror"
+AC_COMPILE_IFELSE([AC_LANG_SOURCE([dnl
+__attribute__ ((__symver__ ("foo@VERS_1"))) int foo_v1 (void) { }
+])], ac_cv_gcc_symver=yes, ac_cv_gcc_symver=no)
+CFLAGS="$save_CFLAGS"])
+if test "$ac_cv_gcc_symver" = "yes"; then
+	AC_DEFINE([HAVE_GCC_SYMVER], [1],
+		  [Defined if __attribute__((symver)) is supported])
+fi
+
 AC_CACHE_CHECK([whether gcc supports -fPIC], ac_cv_fpic, [dnl
 save_CFLAGS="$CFLAGS"
 CFLAGS="$save_CFLAGS -fPIC -Werror"
diff --git a/lib/ChangeLog b/lib/ChangeLog
index 51c79841..07837cce 100644
--- a/lib/ChangeLog
+++ b/lib/ChangeLog
@@ -1,3 +1,8 @@
+2020-04-10  Mark Wielaard  <mark@klomp.org>
+
+	* eu-config.h: If HAVE_GCC_SYMVER define symbol versioning macros
+	using attribute symver.
+
 2019-08-25  Srđan Milaković  <sm108@rice.edu>
 
 	* dynamicsizehash_concurrent.{c,h}: New files.
diff --git a/lib/eu-config.h b/lib/eu-config.h
index 84b22d7c..f5a6be22 100644
--- a/lib/eu-config.h
+++ b/lib/eu-config.h
@@ -177,6 +177,24 @@ asm (".section predict_data, \"aw\"; .previous\n"
 
 
 #ifdef SYMBOL_VERSIONING
+#ifdef HAVE_GCC_SYMVER
+# define OLD_VERSION(name, version) \
+  __attribute__((__symver__(#name "@" #version))) \
+  __attribute__((__alias__(#name))) \
+  __typeof__(name) _compat_##version_##name;
+#define NEW_VERSION(name, version) \
+  __attribute__((__symver__(#name "@@" #version))) \
+  __typeof__(name) name; \
+  __attribute__((__symver__(#name "@ELFUTILS_0"))) \
+  __attribute__((__alias__(#name))) \
+  __typeof__(name) _local_##name;
+# define COMPAT_VERSION_NEWPROTO(name, version, prefix) \
+  __attribute__((__symver__(#name "@" #version))) \
+  __typeof__(_compat_##prefix##_##name) _compat_##prefix##_##name;
+# define COMPAT_VERSION(name, version, prefix) \
+  __attribute__((__symver__(#name "@" #version))) \
+  __typeof__(name) _compat_##prefix##_##name;
+#else /* HAVE_GCC_SYMVER */
 # define OLD_VERSION(name, version) \
   asm (".globl _compat." #version "." #name "\n" \
        "_compat." #version "." #name " = " #name "\n" \
@@ -190,13 +208,14 @@ asm (".section predict_data, \"aw\"; .previous\n"
 # define COMPAT_VERSION(name, version, prefix) \
   asm (".symver _compat." #version "." #name "," #name "@" #version); \
   __typeof (name) _compat_##prefix##_##name asm ("_compat." #version "." #name);
-#else
+#endif /* HAVE_GCC_SYMVER */
+#else /* SYMBOL_VERSIONING */
 # define OLD_VERSION(name, version) /* Nothing for static linking.  */
 # define NEW_VERSION(name, version) /* Nothing for static linking.  */
 # define COMPAT_VERSION_NEWPROTO(name, version, prefix) \
   error "should use #ifdef SYMBOL_VERSIONING"
 # define COMPAT_VERSION(name, version, prefix) error "should use #ifdef SYMBOL_VERSIONING"
-#endif
+#endif /* SYMBOL_VERSIONING */
 
 #ifndef FALLTHROUGH
 # ifdef HAVE_FALLTHROUGH
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 59f33f9e..dae71b78 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,13 @@
+2020-04-14  Mark Wielaard  <mark@klomp.org>
+
+	* libdw.map (ELFUTILS_0): Explicitly mark everything local.
+	(ELFUTILS_0.122): Remove local wildcard.
+	(ELFUTILS_0.126): Likewise.
+	(ELFUTILS_0.127): Likewise.
+	(ELFUTILS_0.130): Likewise.
+	(ELFUTILS_0.136): Likewise.
+	(ELFUTILS_0.138): Likewise.
+
 2019-10-28  Aaron Merey  <amerey@redhat.com>
 
 	* Makefile.am (libdw_so_LDLIBS): Add -ldl for libdebuginfod.so dlopen.
diff --git a/libdw/libdw.map b/libdw/libdw.map
index decac05c..20009868 100644
--- a/libdw/libdw.map
+++ b/libdw/libdw.map
@@ -1,4 +1,7 @@
-ELFUTILS_0 { };
+ELFUTILS_0 {
+  local: *;
+};
+
 ELFUTILS_0.122 {
   global:
     dwarf_abbrevhaschildren;
@@ -141,16 +144,12 @@ ELFUTILS_0.122 {
     dwfl_standard_find_debuginfo;
     dwfl_version;
 
-  local:
-    *;
 } ELFUTILS_0;
 
 ELFUTILS_0.126 {
   global:
     dwarf_getelf;
 
-  local:
-    *;
 } ELFUTILS_0.122;
 
 ELFUTILS_0.127 {
@@ -161,8 +160,6 @@ ELFUTILS_0.127 {
     dwfl_report_begin_add;
     dwfl_module_address_section;
 
-  local:
-    *;
 } ELFUTILS_0.126;
 
 ELFUTILS_0.130 {
@@ -172,8 +169,6 @@ ELFUTILS_0.130 {
     dwfl_module_build_id;
     dwfl_module_report_build_id;
 
-  local:
-    *;
 } ELFUTILS_0.127;
 
 ELFUTILS_0.136 {
@@ -181,8 +176,6 @@ ELFUTILS_0.136 {
     dwfl_addrsegment;
     dwfl_report_segment;
 
-  local:
-    *;
 } ELFUTILS_0.130;
 
 ELFUTILS_0.138 {
@@ -190,8 +183,6 @@ ELFUTILS_0.138 {
     # Replaced ELFUTILS_0.130 version, which has bug-compatibility wrapper.
     dwfl_module_build_id;
 
-  local:
-    *;
 } ELFUTILS_0.136;
 
 ELFUTILS_0.142 {
-- 
2.18.2