rpms/libqb
5
0
Fork 0
Code Issues Pull Requests Releases Activity
c61edd4d30
libqb/05-High-bare-fix-for-libqb-logging-not-working-with-ld..patch
Jan Pokorný c61edd4d30
1.0.2-9 - Evolution of the previous (rhbz#1478089, rhbz#1487787) 
Signed-off-by: Jan Pokorný <jpokorny@redhat.com>
2017-10-09 23:06:05 +02:00

492 lines
22 KiB
Diff
Raw Blame History

From 509beb47f2494e7fe207180266401288da1b6b6d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= <jpokorny@redhat.com>
Date: Fri, 6 Oct 2017 17:17:26 +0200
Subject: [PATCH 5/6] High: bare fix for libqb logging not working with
ld.bfd/binutils 2.29+
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
(or rather [read on]: "bare" fix)
Initially with the help of the internal test suite and the failing log
test, it was eventually discovered[1] that these binutils commits going
to the recent 2.29 release affected the treatment of _start_SECNAME
and __stop_SECNAME symbols denoting the boundary start/stop addresses
of a SECNAME orphan section -- specifically in libqb context a custom
section (SECNAME=__verbose) used for link-time ("run-time amortizing")
callsite collection when there's a support in the toolchain[*]:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=cbd0eecf261c2447781f8c89b0d955ee66fae7e9
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=b27685f2016c510d03ac9a64f7b04ce8efcf95c4
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=7dba9362c172f1073487536eb137feb2da30b0ff
The first one explicitly states:
> Also __start_SECNAME and __stop_SECNAME symbols are marked as hidden
> by ELF linker so that __start_SECNAME and __stop_SECNAME symbols for
> section SECNAME in different modules are unique.
The problem is that libqb silently depends on the previous status quo
ld.bfd linker behaviour of keeping those symbols externally visible,
which was apparently not granted as it has deliberately changed per
above.
And then for 2.29.1 release of binutils once again, as someone actually
noticed something went overboard with the 2.29 changes:
https://sourceware.org/bugzilla/show_bug.cgi?id=21964
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=487b6440dad57440939fab7afdd84a218b612796
At least that change doesn't invalidate all the effort being put into
the original version of the changeset, only the configure script check
had to be refined so as not to miss the "orphan section magic not
working properly out of the box, without a bandaid" observation
(see the inline comment) -- the workaround arrangement needs
to be applied in that case as well.
* * *
So regarding the solution itself, the core of the fix was sketched at
the original Fedora targeted bug against binutils[2]. In short, we are
using a custom linker script that (re)describes the mentioned custom
orphan output section, or better yet, assuredly pushes that section,
and more importantly, it's own boundary denoting symbols, through into
the resulting binary when it's being linked (as in compile-time step).
This solution alone, while working for the non-libqb (more on that
below) logging participants, is not good enough, as it requires all
libqb targets to start using new incantation (namely "-Wl,foo.t" switch)
in the final link step during compilation, which might be solvable
with a tweak in libqb's pkg-config file under assumption that practice
of using "pkg-config --libs libqb" is rigidly followed. Which is likely
a false expectation, and furthermore only for the regular consumption
model, as it doesn't cover the least bit the developmental one (refer
to previous-but-one "tests" commit message), e.g. applied for internal
examples + tests (but no local sub-checkout tree usage can be excluded).
So further extensions were devised to cover both consumption models:
- a. regular:
courtesy of binutils maintainer[3], we follow an idea to make libqb.so
(i.e. what the targets link against) rather a linker script on its
own, which first include the version-specified (e.g. libqb.so.0) file
into the link, then lists, in situ, the content of the linker script
per above, hence -lqb linking has the same effect as having both
"-lqb -Wl,foo.t" explicitly in the link command prior to this trick
- b. developmental:
to eliminate any kind of race condition arising from the attempt
to post-modify libqb.la libtool archive file generated internally
by libtool, we sort of abuse "inherited_linker_flags" variable
within this file format, as it forms an accumulative value across
the whole transitive dependencies chain (if not impaired per the
note below), fitting exactly our purpose of injecting "-Wl,foo.t"
switch equivalent for those libtool-linking by L{D,IB}ADD'ing
libqb.la; it's then enough to craft a custom libtool archive file
declaring that value, and hook it into such dependency chain through
libqb_la_LIBADD, and with a little bit of further fiddling, it works
as desired (note that double occurrence of "-Wl,foo.t" equivalent
present at some stages of sorting this trick turned out to be,
surprisingly, counter-productive);
NOTE: some forms of libtool distribution (debian + derivatives ones
in particular) undermine natural transitive dependency propagation
with a deliberate cut off (https://bugs.debian.org/702737), so we
need to ensure the "impairment" is not happening by force (corosync
precedent: https://github.com/corosync/corosync/commit/0f1dc5c1)
^ something like this needs to be applied for any such "private
consumer" (although it hopefully goes without saying this way
of consuming libqb outside of it's own playground is hardly
the Right Thing) if portability is important, nonetheless!
Last but not least, libqb itself needs to be linked with the mentioned
"-Wl,foo.t" equivalent for its own outgoing log messages to be honoured,
which is already achieved with the arrangement for b. above, and by
experiments, further redefinition of those boundary denoting symbols
as weak was necessary so as to make them truly global within libqb.so
proper (at least with binutils 2.29).
* * *
Finally, let's have a look how the already well-known test matrix
overview changes as of this commit, but first as a recap,
"X(Y)" denotes "X linked with linker Y":
X(a) .. ld.bfd < 2.29
X(b) .. ld.bfd = 2.29 (and likely on)
and here you are (values in <angle brackets> denote non-trivial change
[not mere rewording] introduced as of this commit, in comparison to the
table stated in the preceding commit):
+=========+=========+=========+=========+=========+=========+=========+
#client(x)# libqb(a) usage # libqb(b) usage #
# vvv #---------+---------+---------+---------+---------+---------+
# V # direct | libX(a) : libX(b) # direct | libX(a) : libX(b) #
+=========+=========+=========+=========+=========+=========+=========+
# x = a # OK | OK : <OK> # <OK> | <OK> : <OK> #
# x = b # <OK> | <OK> : <OK> # <OK> | <OK> : <OK> #
+=========+=========+=========+=========+=========+=========+=========+
Everything is green \o/
Note: as of this fix, it is assumed that the non-green counterpart of
this table in the message for the preceding commit doubles as an
indicator how will mixing the logging participants wrt. linker+libqb
version work out, when "X(Y)" becomes read as "X linked with linker
Y under additional restriction on libqb version when compile-time link
is performed of the particular part":
X(a) .. ld.bfd < 2.29 OR [arbitrary ld.bfd AND libqb after this fix)
X(b) .. ld.bfd = 2.29 (and likely on) AND libqb up to, but excluding
this fix
* * *
Let's also state some imperfections and loops kept open:
Deficiencies:
* whenever anything is compiled against our install-time-modified
libqb.so so as to force the visibility of the discussed symbols
(or when compiling [with] libqb internally):
> /usr/bin/ld: warning: ../lib/qblog_script.ld contains output sections; did you forget -T?
- not solvable as long as we use the linker script, and there's
hardly any other way not requiring the libqb consumers to adapt
in any aspect
Open questions:
* what about binutils 2.99.1 partially mitigating the impact of the
above commits
* should we enable attribute((__section__)) for powerpc platforms?
it seems to work well there -- and related to that, we need to figure
out how static and dynamic call sites can live together -- say what
will happen to logging programs compiled with libqb w/o attribute
sections when the underlying libqb is flipped to the one with them...
* * *
[*] basically GCC's section("SECNAME") __attribute__ annotation of the
global variables + linker described behaviour previously mistakenly
taken for granted
References:
[1] http://oss.clusterlabs.org/pipermail/developers/2017-July/000503.html
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1477354#c2 + comment 8
[3] https://bugzilla.redhat.com/show_bug.cgi?id=1477354#c9
Signed-off-by: Jan Pokorný <jpokorny@redhat.com>
---
configure.ac | 90 +++++++++++++++++++++++++++++++++++++++++-------
lib/Makefile.am | 67 +++++++++++++++++++++++++++++++++++
lib/log.c | 7 ++++
lib/qblog_script.la.in | 15 ++++++++
lib/qblog_script.ld.in | 17 +++++++++
lib/qblog_script_noop.ld | 1 +
7 files changed, 184 insertions(+), 13 deletions(-)
create mode 100644 lib/qblog_script.la.in
create mode 100644 lib/qblog_script.ld.in
create mode 100644 lib/qblog_script_noop.ld
diff --git a/configure.ac b/configure.ac
index 5601302..6eda2dd 100644
--- a/configure.ac
+++ b/configure.ac
@@ -501,6 +501,12 @@ AC_ARG_ENABLE([debug],
AC_ARG_ENABLE([coverage],
[AS_HELP_STRING([--enable-coverage],[coverage analysis of the codebase])])
+AC_ARG_ENABLE([interlib-deps],
+ [AS_HELP_STRING([--disable-interlib-deps],
+ [disable inter-library dependencies (might break builds)])],
+ [enable_interlib_deps="$enableval"],
+ [enable_interlib_deps="yes"])
+
AC_ARG_ENABLE([slow-tests],
[AS_HELP_STRING([--enable-slow-tests],[build and run slow tests])])
@@ -609,6 +615,19 @@ else
COVERAGE_LDFLAGS=""
fi
+# --- inter-library dependencies ---
+# because of debian/ubuntu swimming against the stream
+# https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=702737,
+# override the libtool variable by force because the current
+# arrangement relies on transitive dependency propagation
+AC_MSG_NOTICE([Enabling inter-library dependencies: $enable_interlib_deps])
+if test "${enable_interlib_deps}" == "yes"; then
+ link_all_deplibs=yes
+else
+ link_all_deplibs=no
+fi
+
+# --- slow tests ---
if test "x${enable_slow_tests}" = xyes ; then
AC_DEFINE([HAVE_SLOW_TESTS], 1,[have slow tests])
AC_MSG_NOTICE([Enabling Slow tests])
@@ -618,31 +637,74 @@ AC_SUBST(HAVE_SLOW_TESTS)
# --- callsite sections ---
if test "x${GCC}" = xyes; then
+ AX_SAVE_FLAGS
AC_MSG_CHECKING([whether GCC supports __attribute__((section()) + ld supports orphan sections])
if test "x${ac_cv_link_attribute_section}" = x ; then
- AC_TRY_RUN(
- AC_LANG_PROGRAM(
+ LDFLAGS="${LDFLAGS_save} -shared -fPIC" # we are compiling shared lib
+ AC_LINK_IFELSE(
+ [AC_LANG_SOURCE(
[[#include <assert.h>
- extern void * __start___verbose, * __stop___verbose;]],
- [[static int my_var __attribute__((section("__verbose"))) = 5;
- assert(("non-empty data section", __start___verbose != __stop___verbose));
- assert(("no data section value loss", my_var == 5));
- ]]),
- [gcc_has_attribute_section=yes],
+ extern int __start___verbose[], __stop___verbose[];
+ int test(void) {
+ static int my_var __attribute__((section("__verbose"))) = 3;
+ assert(("L:non-empty data section", __start___verbose != __stop___verbose));
+ assert(("L:no data section value loss", my_var == 3 /* for 2.29.1+ */));
+ return *((int *) __start___verbose);
+ }]])],
+ [gcc_has_attribute_section=yes; cp "conftest${EXEEXT}" "lib_conftest.so"],
[gcc_has_attribute_section=no]
)
+ AX_RESTORE_FLAGS
else
gcc_has_attribute_section=${ac_cv_link_attribute_section}
fi
-
AC_MSG_RESULT($gcc_has_attribute_section)
- if test $gcc_has_attribute_section = yes; then
+
+ # in the failing case (e.g. with ld from binutils 2.29), it's likely the
+ # following will fail readily in linkage (hidden symbol `__stop___verbose'
+ # in conftest is referenced by DSO), but keep the sensible test (in-binary
+ # symbol is expected to be propagated into the library, and to draw the
+ # full circle back to binary through standard return value passing;
+ # -rpath passed because LD_LIBRARY_PATH exporting is unwieldy here);
+ # moreover, "my_var" == 3 assertion above is necessary so that binutils
+ # 2.29.1+ will not slip other parts of the overall is-workaround-needed
+ # harness, as it restored some (but not all) of the original behaviour,
+ # but the workaround is still provably needed
+ if test "x${gcc_has_attribute_section}" = xyes; then
+ AC_MSG_CHECKING([whether linker emits global boundary symbols for orphan sections])
+ LIBS="${LIBS} -L. -l_conftest -Wl,-rpath=$(pwd)"
+ AC_TRY_RUN(
+ AC_LANG_PROGRAM(
+ [[#include <assert.h>
+ extern int __start___verbose[], __stop___verbose[];
+ int test(void);]],
+ [[static int my_var __attribute__((section("__verbose"))) = 5;
+ assert(("P:non-empty data section", __start___verbose != __stop___verbose));
+ assert(("P:no data section value loss", my_var == test() /*5?*/));
+ ]]),
+ [gcc_has_attribute_section_visible=yes],
+ [gcc_has_attribute_section_visible=no]
+ )
+ AX_RESTORE_FLAGS
+ AC_MSG_RESULT($gcc_has_attribute_section_visible)
+ rm -f "lib_conftest.so"
+
AC_DEFINE([QB_HAVE_ATTRIBUTE_SECTION], 1,
[Enabling code using __attribute__((section))])
- PACKAGE_FEATURES="$PACKAGE_FEATURES attribute-section"
AC_SUBST([client_dlopen_LIBS],[$dlopen_LIBS])
+ if test "x${gcc_has_attribute_section_visible}" = xyes; then
+ PACKAGE_FEATURES="$PACKAGE_FEATURES attribute-section"
+ else
+ AC_DEFINE([QB_NEED_ATTRIBUTE_SECTION_WORKAROUND], 1,
+ [Enabling code using __attribute__((section))])
+ PACKAGE_FEATURES="$PACKAGE_FEATURES attribute-section-workaround"
+ fi
fi
fi
+AM_CONDITIONAL(HAVE_GCC_ATTRIBUTE_SECTION, [test "x${gcc_has_attribute_section}" = xyes])
+AM_CONDITIONAL(NEED_GCC_ATTRIBUTE_SECTION_WORKAROUND,
+ [test "x${gcc_has_attribute_section}" = xyes \
+ && test "x${gcc_has_attribute_section_visible}" != xyes])
# --- ansi ---
if test "x${enable_ansi}" = xyes && \
@@ -724,9 +786,11 @@ AC_CONFIG_FILES([Makefile
docs/Makefile
docs/common.dox
docs/html.dox
- docs/man.dox])
+ docs/man.dox
+ lib/qblog_script.la:lib/qblog_script.la.in])
-AC_CONFIG_LINKS([tests/functional/GNUmakefile:tests/functional/GNUmakefile])
+AC_CONFIG_LINKS([lib/qblog_script_noop.ld:lib/qblog_script_noop.ld
+ tests/functional/GNUmakefile:tests/functional/GNUmakefile])
AC_OUTPUT
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 0bebeb5..340cd85 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -19,6 +19,7 @@
# along with libqb. If not, see <http://www.gnu.org/licenses/>.
+CLEANFILES = qblog_script.ld
MAINTAINERCLEANFILES = Makefile.in
noinst_HEADERS = ipc_int.h util_int.h ringbuffer_int.h loop_int.h \
@@ -39,9 +40,48 @@ source_to_lint = util.c hdb.c ringbuffer.c ringbuffer_helper.c \
log_syslog.c log_dcs.c log_format.c \
map.c skiplist.c hashtable.c trie.c
+# Following two files related to linkage using classic ld from binutils 2.29+
+# with which we cannot afford to lose public access to section boundary symbols
+# (as the mentioned version started to scope them privately by default, see
+# the comment within the first of the files, ultimately leading to broken
+# logging functionality of libqb) deserve a bit of explanation:
+# * qblog_script.ld
+# - linker script that instructs the output section that those symbols should
+# be visible, i.e. supports the same behaviour regardless of ld version
+# - serves two purposes:
+# . local: libqb itself and its "private" (cf. examples) users need those
+# symbols visible, which is achieved with a help of the other file
+# . system-wide: whenever the non-private library users link against libqb
+# (it's development files), this linker script with
+# prepended INPUT command so as to refer to the actual
+# libqb library (it's numbered alias that is eventually
+# resolved to proper shared library) is masked as libqb.so,
+# this arrangement achieves the libqb's user will have
+# the discussed symbols visible alike
+# * qblog_script.la
+# - as mentioned earlier, this indirectly hooks into libtool machinery, with
+# the only true intention of injecting "-Wl,<path to qblog_script.ld>"
+# into "inherited_linker_flags" libtool archive variable, from where it's
+# subsequently spread into the build process of all the internal library
+# users, assuming they have their dep arranged as "user_LIBADD=libqb.la"
+# (this also alleviates the burden on getting things right if, e.g., any
+# libqb user consumes it directly like this from its own sub-checkout tree)
+# - it indirectly, once libtool prechew the original link command
+# originally referring to this file, it turns such reference into the
+# "real" library reference (here combining libdir and old_library
+# variables within the file), also ensures libqb itself will visibly
+# expose the discussed symbols, because such references point again to
+# the (not enriched) linker script file that will get interpreted just
+# like that during the last build step of the library
+EXTRA_libqb_la_DEPENDENCIES = qblog_script.ld qblog_script.la
+EXTRA_DIST = qblog_script.ld.in qblog_script.la.in qblog_script_noop.ld
+
libqb_la_SOURCES = $(source_to_lint) unix.c
libqb_la_CFLAGS = $(PTHREAD_CFLAGS)
libqb_la_LIBADD = $(LTLIBOBJS) $(dlopen_LIBS) $(PTHREAD_LIBS) $(socket_LIBS)
+if NEED_GCC_ATTRIBUTE_SECTION_WORKAROUND
+libqb_la_LIBADD += qblog_script.la
+endif
AM_LDFLAGS = $(LDFLAGS_COPY:-Bsymbolic-functions=)
@@ -60,6 +100,33 @@ else
endif
endif
+qblog_script.ld: %.ld: %.ld.in
+ $(AM_V_GEN)$(CPP) -xc -I$(top_srcdir)/include -D_GNU_SOURCE -C -P $< \
+ | sed -n "/$$(sed -n '/^[^#]/{s/[*\/]/\\\0/g;p;q}' $<)/{:r;p;n;br}" \
+ > $@
+
+# find the libqb.so symlink's target, if so, try to find out, iteratively,
+# its gradually shorter forms that likewise symlinks the same target as the
+# original libqb.so path, point to that file from the linker script using
+# qblog_script.ld as a template, storing result in place of original libqb.so
+# (e.g., libqb.so := "INPUT(libqb.so.0) " [...] "SECTIONS { " [...] "}")
+# NOTE: readlink nor realpath are POSIX; not chained links ready
+# NOTE: conservative check, i.e., not per NEED_GCC_ATTRIBUTE_SECTION_WORKAROUND
+if HAVE_GCC_ATTRIBUTE_SECTION
+install-exec-hook: qblog_script.ld
+ target=$$(ls -l "$(DESTDIR)$(libdir)/libqb.so" || :); \
+ target=$${target#* -> }; t1_bn=$$(basename "$${target}" || :); \
+ while test -n "$${t1_bn}"; do t2_bn=$${t1_bn%.*[0-9]*}; \
+ test "$${t2_bn}" != libqb.so || break; \
+ test -L "$${t2_bn}" || { t1_bn=$${t2_bn}; continue; }; \
+ t2_target=$$(ls -l "$${t2_bn}" || break); t2_target=$${t2_target#* -> }; \
+ test "$${t2_target}" = "$${target}" || break; \
+ t1_bn=$${t2_bn}; done; test -n "$${t1_bn}" || \
+ { echo "only applicable to SO symlink scheme"; exit 1; }; \
+ echo "INPUT($${t1_bn})" > "$(DESTDIR)$(libdir)/libqb.so-t"
+ cat $< >> "$(DESTDIR)$(libdir)/libqb.so-t"
+ mv -f "$(DESTDIR)$(libdir)/libqb.so-t" "$(DESTDIR)$(libdir)/libqb.so"
+endif
pkgconfigdir = $(libdir)/pkgconfig
pkgconfig_DATA = libqb.pc
diff --git a/lib/log.c b/lib/log.c
index bfd218f..4ed432f 100644
--- a/lib/log.c
+++ b/lib/log.c
@@ -40,6 +40,13 @@
#include "util_int.h"
#include <regex.h>
+#if defined(QB_NEED_ATTRIBUTE_SECTION_WORKAROUND) && !defined(S_SPLINT_S)
+/* following only needed to force these symbols be global
+ with ld 2.29: https://bugzilla.redhat.com/1477354 */
+struct qb_log_callsite __attribute__((weak)) QB_ATTR_SECTION_START[] = { 0 };
+struct qb_log_callsite __attribute__((weak)) QB_ATTR_SECTION_STOP[] = { 0 };
+#endif
+
static struct qb_log_target conf[QB_LOG_TARGET_MAX];
static uint32_t conf_active_max = 0;
static int32_t in_logger = QB_FALSE;
diff --git a/lib/qblog_script.la.in b/lib/qblog_script.la.in
new file mode 100644
index 0000000..f262df8
--- /dev/null
+++ b/lib/qblog_script.la.in
@@ -0,0 +1,15 @@
+# Generated by libtool
+# NOTE: above line is just to pass func_ltwrapper_script_p sanity check of
+# libtool script, as we are basically sort of abusing it so as to inject
+# our custom linker script to "private" (cf. examples) users of libqb.la
+
+# shall rather carry a location of old_library (possibly libdir or something
+# else, but installed=no needed to suppress 'library moved' warning then) as
+# it's together (with libtool implied prefix otherwise) used for linking libqb
+libdir=@abs_builddir@
+
+# avoids issues with library_names (spurious rpath emitting, relink-on-install)
+old_library=qblog_script_noop.ld
+
+# subject of our injection into libqb.la impacting build time for private users
+inherited_linker_flags=-Wl,@abs_builddir@/qblog_script.ld
diff --git a/lib/qblog_script.ld.in b/lib/qblog_script.ld.in
new file mode 100644
index 0000000..1f37976
--- /dev/null
+++ b/lib/qblog_script.ld.in
@@ -0,0 +1,17 @@
+#include <qb/qblog.h>
+/* GNU ld script
+ This atypical arrangement enforces global visibility of boundary symbols
+ (QB_ATTR_SECTION_START, QB_ATTR_SECTION_STOP) for the custom section
+ QB_ATTR_SECTION used for compile-time offloading of the logging call sites
+ tracking. While libqb relies on these being global, default linker from
+ binutils change the visibility as of version 2.29, making the logging
+ unusable without artificial stimulus: https://bugzilla.redhat.com/1477354 */
+SECTIONS {
+#ifdef QB_HAVE_ATTRIBUTE_SECTION
+ QB_ATTR_SECTION : {
+ QB_ATTR_SECTION_START = .;
+ *(QB_ATTR_SECTION);
+ QB_ATTR_SECTION_STOP = .;
+ }
+#endif
+}
diff --git a/lib/qblog_script_noop.ld b/lib/qblog_script_noop.ld
new file mode 100644
index 0000000..f037fca
--- /dev/null
+++ b/lib/qblog_script_noop.ld
@@ -0,0 +1 @@
+/* this is an empty linker script having a role of a NO-OP link object */
--
2.14.2
Reference in New Issue View Git Blame Copy Permalink