[PATCH v2] libattr: Set symbol versions for legacy syscalls via attribute or asm

[Alexander Miller]

Currently, a linker script is used to define versioned symbols
for the legacy linux syscalls with simple assignments.

That isn't mentioned in ld's documentation as a valid method to
set symbol versions, and while one might assume that it should work
and the linker script is accepted, the result often isn't correct:
gold and lld always create broken binaries, and even the bfd linker
can create unusable symbols if used with --gc-sections or LTO.

For example, the result can be a library where the function has been
discarded and the versioned symbol is unusable, i.e.,
    23: 00000000     0 NOTYPE  GLOBAL DEFAULT  ABS getxattr@ATTR_1.0
instead of
    23: 000033c0     0 FUNC    GLOBAL DEFAULT   11 getxattr@ATTR_1.0
Meanwhile, lld doubles the version suffix: getxattr@ATTR_1.0@ATTR_1.0

Not that these issues may be viewed as linker bugs, and there may
or may not be some related improvements to linker script handling
since I tested this a few months ago (binutils-2.37, lld-14).
But in either case a more robust solution would be preferable.

So remove the linker script entirely and set symbol versions with
__attribute__((__symver__(...))) if available (i.e., in gcc >= 10,
but not in clang).  Otherwise use the traditional global asm solution
with a .symver directive.  These are the well documented methods (along
with version scripts, which don't apply here) to set symbol versions
and work correctly with all linkers, including older versions.

A workaround that is needed for gcc < 10 not to break LTO partitioning
with global asm is also included (__attribute__((__no_reorder__))),
but some very old versions may still need -flto-partition=none to build
correctly with LTO (not to going bother with completely obsolete versions).

Signed-off-by: Alexander Miller <alex.miller@gmx.de>
---
Changes since v1:
 * rebased on current master
 * add missing underscore in noreorder
 * use double underscore variants for attribute names
 * tried to improve title and commit message

 libattr/Makemodule.am | 10 ----------
 libattr/libattr.lds   | 12 ------------
 libattr/syscalls.c    | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 22 deletions(-)
 delete mode 100644 libattr/libattr.lds

diff --git a/libattr/Makemodule.am b/libattr/Makemodule.am
index 6a086e3..545da7c 100644
--- a/libattr/Makemodule.am
+++ b/libattr/Makemodule.am
@@ -13,10 +13,6 @@ LTVERSION = $(LT_CURRENT):$(LT_REVISION):$(LT_AGE)
 libattr_la_LIBADD = $(LTLIBINTL)
 libattr_la_DEPENDENCIES = \
        exports
-if OS_LINUX
-libattr_la_DEPENDENCIES += \
-       libattr/libattr.lds
-endif
 libattr_la_SOURCES = \
        libattr/attr_copy_action.c \
        libattr/attr_copy_check.c \
@@ -32,9 +28,3 @@ libattr_la_CFLAGS = -include libattr/libattr.h
 libattr_la_LDFLAGS = \
        -Wl,--version-script,$(top_srcdir)/exports \
        -version-info $(LTVERSION)
-if OS_LINUX
-libattr_la_LDFLAGS += \
-       -Wl,$(top_srcdir)/libattr/libattr.lds
-endif
-
-EXTRA_DIST += libattr/libattr.lds
diff --git a/libattr/libattr.lds b/libattr/libattr.lds
deleted file mode 100644
index 947f15d..0000000
--- a/libattr/libattr.lds
+++ /dev/null
@@ -1,12 +0,0 @@
-"fgetxattr@ATTR_1.0" = libattr_fgetxattr;
-"flistxattr@ATTR_1.0" = libattr_flistxattr;
-"fremovexattr@ATTR_1.0" = libattr_fremovexattr;
-"fsetxattr@ATTR_1.0" = libattr_fsetxattr;
-"getxattr@ATTR_1.0" = libattr_getxattr;
-"lgetxattr@ATTR_1.0" = libattr_lgetxattr;
-"listxattr@ATTR_1.0" = libattr_listxattr;
-"llistxattr@ATTR_1.0" = libattr_llistxattr;
-"lremovexattr@ATTR_1.0" = libattr_lremovexattr;
-"lsetxattr@ATTR_1.0" = libattr_lsetxattr;
-"removexattr@ATTR_1.0" = libattr_removexattr;
-"setxattr@ATTR_1.0" = libattr_setxattr;
diff --git a/libattr/syscalls.c b/libattr/syscalls.c
index 721ad7f..907560a 100644
--- a/libattr/syscalls.c
+++ b/libattr/syscalls.c
@@ -26,6 +26,27 @@
 #include <sys/syscall.h>
 #include <sys/xattr.h>

+/*
+ * Versioning of compat symbols:
+ * prefer symver attribute if available (since gcc 10),
+ * fall back to traditional .symver asm directive otherwise.
+ */
+#ifdef __has_attribute
+# if __has_attribute(__symver__)
+#  define SYMVER(cn, vn) __typeof(cn) cn __attribute__((__symver__(vn)))
+# elif __has_attribute(__no_reorder__)
+   /*
+    * Avoid wrong partitioning with older gcc and LTO. May not work reliably
+    * with all versions; use -flto-partition=none if you encounter problems.
+    */
+#  define SYMVER(cn, vn) __typeof(cn) cn __attribute__((__no_reorder__)); \
+                        __asm__(".symver " #cn "," vn)
+# endif
+#endif
+#ifndef SYMVER
+#  define SYMVER(cn, vn) __asm__(".symver " #cn "," vn)
+#endif
+
 #ifdef HAVE_VISIBILITY_ATTRIBUTE
 # pragma GCC visibility push(default)
 #endif
@@ -35,66 +56,78 @@ int libattr_setxattr(const char *path, const char *name,
 {
        return syscall(__NR_setxattr, path, name, value, size, flags);
 }
+SYMVER(libattr_setxattr, "setxattr@ATTR_1.0");

 int libattr_lsetxattr(const char *path, const char *name,
                      void *value, size_t size, int flags)
 {
        return syscall(__NR_lsetxattr, path, name, value, size, flags);
 }
+SYMVER(libattr_lsetxattr, "lsetxattr@ATTR_1.0");

 int libattr_fsetxattr(int filedes, const char *name,
                      void *value, size_t size, int flags)
 {
        return syscall(__NR_fsetxattr, filedes, name, value, size, flags);
 }
+SYMVER(libattr_fsetxattr, "fsetxattr@ATTR_1.0");

 ssize_t libattr_getxattr(const char *path, const char *name,
                         void *value, size_t size)
 {
        return syscall(__NR_getxattr, path, name, value, size);
 }
+SYMVER(libattr_getxattr, "getxattr@ATTR_1.0");

 ssize_t libattr_lgetxattr(const char *path, const char *name,
                          void *value, size_t size)
 {
        return syscall(__NR_lgetxattr, path, name, value, size);
 }
+SYMVER(libattr_lgetxattr, "lgetxattr@ATTR_1.0");

 ssize_t libattr_fgetxattr(int filedes, const char *name,
                          void *value, size_t size)
 {
        return syscall(__NR_fgetxattr, filedes, name, value, size);
 }
+SYMVER(libattr_fgetxattr, "fgetxattr@ATTR_1.0");

 ssize_t libattr_listxattr(const char *path, char *list, size_t size)
 {
        return syscall(__NR_listxattr, path, list, size);
 }
+SYMVER(libattr_listxattr, "listxattr@ATTR_1.0");

 ssize_t libattr_llistxattr(const char *path, char *list, size_t size)
 {
        return syscall(__NR_llistxattr, path, list, size);
 }
+SYMVER(libattr_llistxattr, "llistxattr@ATTR_1.0");

 ssize_t libattr_flistxattr(int filedes, char *list, size_t size)
 {
        return syscall(__NR_flistxattr, filedes, list, size);
 }
+SYMVER(libattr_flistxattr, "flistxattr@ATTR_1.0");

 int libattr_removexattr(const char *path, const char *name)
 {
        return syscall(__NR_removexattr, path, name);
 }
+SYMVER(libattr_removexattr, "removexattr@ATTR_1.0");

 int libattr_lremovexattr(const char *path, const char *name)
 {
        return syscall(__NR_lremovexattr, path, name);
 }
+SYMVER(libattr_lremovexattr, "lremovexattr@ATTR_1.0");

 int libattr_fremovexattr(int filedes, const char *name)
 {
        return syscall(__NR_fremovexattr, filedes, name);
 }
+SYMVER(libattr_fremovexattr, "fremovexattr@ATTR_1.0");

 #ifdef HAVE_VISIBILITY_ATTRIBUTE
 # pragma GCC visibility pop
--
2.38.2