From 11b9069098b4d4c23efb06c9457a085b784354d4 Mon Sep 17 00:00:00 2001 From: Alexander Miller Date: Thu, 22 Dec 2022 05:21:30 +0100 Subject: libattr: Set symbol versions for legacy syscalls via attribute or asm 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 going to bother with completely obsolete versions). Signed-off-by: Alexander Miller --- 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 #include +/* + * 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 -- cgit v1.2.1