From 20246f544f74866885f4d9491f74152bd95e7786 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Fri, 6 Oct 2017 17:17:26 +0200 Subject: [PATCH] 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, now that we established means to analyse the impact of the linker-dependent misbehaviour and to detect some of its symptoms in preceding two commits, respectively) 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: http://lists.gnu.org/archive/html/bug-binutils/2017-08/msg00195.html (overview of the original bug discussion, rather than directly https://sourceware.org/bugzilla/show_bug.cgi?id=21964, which is a result of a conflct resolution when restoring bugzilla backup) 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 band aid" 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 executable 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, which should now demistify the very existence of effectively empty qblog_script_noop.ld file); 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! * * * On the address of linker script workaround, there are linkers out there that do not support the trick, for instance: - ld.gold from binutils (but it has hardly ever been working with orphan sections, anyway: https://sourceware.org/bugzilla/show_bug.cgi?id=22291) - ancient versions of ld.bfd, e.g. 10+ years old one used as a native system linker even in the most recent releases of FreeBSD, unless GCC toolchain is used instead If these are hit when (because) the compiler has already demonstrated it supports "section" attribute, the build system configuration is forcibly stopped, simply to stay conceptually compatible with the prior state in which the affinity to leverage that feature hasn't been called off under any circumstances. One is, however, able to achieve exactly this behaviour with --enable-nosection-fallback switch, but if some other participants in the logging, possibly linked with a more friendly linker, do utilize this orphan section, logging may silently break (another reason to require an explicit sign-off). Another note, the particular self-check change slightly touched in the previous commit but otherwise predating this whole effort by far needs to be modified now once again, this time because linker-script-based workaround for newer linkers as stated causes the section boundary symbols to be present regardless if that section is utilized, leading to a self-inflicted breakage due to these empty section symbols suddenly winning in the symbol resolution mechanism (previously the empty section would be dropped incl. the boundary symbols), causing problems down the line. It also makes this very check self-contained in the same compilation unit that trigggers it, whereas previously it used to be the said "arbitrary" winner and things kept silently working just because failure condition -- empty section -- would be implicitly isolated. 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 under all circumstances, 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). * * * To provide a high-level prioritized overview of what drove the approach: - PRESERVATION OF BINARY COMPATIBILITY (ABI), which is achieved except for a single "ABI nongracefulness" I am aware of but that's more a consequence of slightly incorrect assumptions in the logic of QB_LOG_INIT_DATA macro function predating this whole affair by a long shot and which the patchset finally rectifies: if in the run-time dynamic link, following is combined: (. libqb, arbitrary variant: pre-/post-fix, binutils < / >= 2.29) . an "intermediate" library (something that the end executable links with) triggering QB_LOG_INIT_DATA macro and being built with pre-fix libqb (and perhaps only with binutils < 2.29) . end executable using no libqb's logging at all, but being built with post-fix libqb (and arbitrary binutils < / >= 2.29) then, unlike when executable is built with pre-fix libqb, the special callsite data containing section in the ELF structure of the executable is created + its boundary denoting symbols defined within, despite the section being empty (did not happen with pre-fix libqb), and because the symbols defined within the target program have priority over that of shared libraries in the symbol resolution fallback scheme, the assertion of QB_LOG_INIT_DATA of the mentioned intermediate library will actually be evaluating the inequality of boundaries for the section of the executable(!) rather than it's own (or whatever higher prio symbols are hit, presumably only present if the section at that level is non-empty, basically a generalization of the story so far); the problem then manifests as unability to run said executable as it will fail because of the intermediate library inflicted assertion (sadly with very unhelpful "Assertion `0' failed" message); fortunately, there's enough flexibility so as how to fix this, either should be fine: . have everything in the executable's library dependency closure that links against libqb assurably (compile-time) linked with one variant of libqb only (either all pre-fix or post-fix, mind the apparent limitation of binutils' versions with the former) . have the end executable (that does not use logging at all as discussed precondition) linked using substitution like this: s/-lqb/-l:libqb.so.0/ (you may need to adapt the number later) and you may also need to add this CPPFLAG for the executable: -DQB_KILL_ATTRIBUTE_SECTION - as high level of isolation of the client space from the linker (respectively toolchain) subtleties as possible (no new compilation flags and such required, plus there's no way to hook any dynamic computational ad-hoc decision when the compilation is about to happen, anyway), and in turn, versatility is preserved as much as possible * * * 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 (+ 2.29.1 and hopefully on) and here you are (values in 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 : # | : # # x = b # | : # | : # +=========+=========+=========+=========+=========+=========+=========+ 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 (loosely though[!], as the occurrence of empty callsite section can no longer be attributed to something bad going on as of this fix that enforces its presence unconditionally, whereas it would be suppressed when unused before with kind linkers, hence some other conditions can be witnessed especially when QB_LOG_INIT_DATA misused in no-logging context) 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 * as already mentioned, lacking compatibility with ld.gold linker and won't foreseeably be (cf. https://bugzilla.redhat.com/1500898#c7) - please stick with ld.bfd (i.e. default ld linker), which you had to do in the past anyway (at least for compiling libqb itself) Open questions: * should we enable attribute((__section__)) for powerpc and other minor platforms if the feature is proved to be working there as well? and if/when that's going to happen, we need to figure out the transition plan to be spread throughout an extended period to keep the transition smooth -- notably when now-with-callsite-section clients will get run-time linked with callsite-section-not-a-default libqb (say upon it's downgrade), and for that, the libqb's support alone should be enabled year(s) ahead of the actual client space... * * * [*] 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ý --- .gitignore | 4 + check | 2 + configure.ac | 156 ++++++++++++++++++--- include/qb/qblog.h | 32 ++++- lib/Makefile.am | 79 ++++++++++- lib/libqb.pc.in | 11 ++ lib/log.c | 7 + lib/qblog_script.la.in | 15 ++ lib/qblog_script.ld.in | 30 ++++ lib/qblog_script_noop.ld | 1 + tests/functional/log_callsite_bench_gen.py | 2 +- tests/functional/log_external/Makefile.am | 2 +- 12 files changed, 312 insertions(+), 29 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/.gitignore b/.gitignore index d36173734..48d733624 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,7 @@ aclocal.m4 autom4te.cache/ config.status configure +conftest.* .libs # since we use subdir-objects as automake option @@ -28,6 +29,9 @@ configure /.tarball-version /tag-* +/lib/qblog_script.ld +# already captured with wildcard: /lib/qblog_script.la + libtool .version libqb.spec diff --git a/check b/check index 6a2f29d40..79062c570 100755 --- a/check +++ b/check @@ -90,6 +90,8 @@ check_nosection() { echo "checking nosection" echo "=======================" # no __attribute__((section)) + # NOTE: alternatively CPPFLAGS=-DQB_KILL_ATTRIBUTE_SECTION, but + # that won't shortcut many initial/configure decisions check "ac_cv_link_attribute_section=no" } diff --git a/configure.ac b/configure.ac index 8df2c29b4..540712b7e 100644 --- a/configure.ac +++ b/configure.ac @@ -78,6 +78,7 @@ AM_CONDITIONAL(HAVE_DOXYGEN, test -n "${DOXYGEN}") AC_CHECK_PROGS([SPLINT], [splint]) AM_CONDITIONAL(HAVE_SPLINT, test -n "${SPLINT}") AC_CHECK_TOOLS([NM], [eu-nm nm], [:]) +AC_CHECK_TOOLS([READELF], [eu-readelf readelf], [:]) AM_PATH_PYTHON([2.6],, [:]) AM_CONDITIONAL([HAVE_PYTHON], [test "$PYTHON" != :]) @@ -504,6 +505,14 @@ 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)])]) + +AC_ARG_ENABLE([nosection-fallback], + [AS_HELP_STRING([--enable-nosection-fallback], + [allow (logging compat-breaking?) fallback when orphan section dead-ended])]) + AC_ARG_ENABLE([slow-tests], [AS_HELP_STRING([--enable-slow-tests],[build and run slow tests])]) @@ -612,6 +621,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 "x${enable_interlib_deps}" = xno; then + link_all_deplibs=no +else + link_all_deplibs=yes +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]) @@ -621,20 +643,60 @@ 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 + LDFLAGS="${LDFLAGS_save} -shared -fPIC" # we are compiling shared lib + AC_LINK_IFELSE( + [AC_LANG_SOURCE( + [[#include + 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${ac_exeext}" "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) + + # 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-executable symbol is expected to be propagated into the library, + # and to draw the full circle back to the executable through standard + # return value passing (respectively no-exec probing to spot the issue); + # -rpath passed because LD_LIBRARY_PATH exporting is unwieldy here); + # moreover, "my_var" == 3 assertion above (respectively checking if the + # boundary symbol visibility differs from DEFAULT in readelf output) 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${shrext_cmds} -Wl,-rpath=$(pwd)" dnl could be turned to AC_TRY_RUN (first assertion is equivalent to dnl the further check in action-if-true), but that would prevent dnl cross-building AC_LINK_IFELSE( [AC_LANG_PROGRAM( [[#include - extern void * __start___verbose, * __stop___verbose;]], + extern int __start___verbose[], __stop___verbose[]; + int test(void);]], [[static int my_var __attribute__((section("__verbose"))) = 5; - assert("non-empty data section" + assert("E:non-empty data section" && __start___verbose != __stop___verbose); - assert("no data section value loss" - && my_var == 5);]] + assert("E:no data section value loss" + && my_var == test() /*5?*/);]] )], [# alternatively something like (but requires number parsing): # readelf -SW "conftest${ac_exeext}" \ @@ -644,22 +706,78 @@ if test "x${GCC}" = xyes; then verbose_stop_addr=$(${NM} -g --portability -- "conftest${ac_exeext}" \ | grep __stop___verbose | cut -d" " -f 3) test "${verbose_start_addr}" = "${verbose_stop_addr}" \ - && gcc_has_attribute_section=no \ - || gcc_has_attribute_section=yes], - [gcc_has_attribute_section=no] + && gcc_has_attribute_section_visible=no \ + || { verbose_start_type=$(${READELF} -s backup \ + | sed -n '/__start___verbose/{s/^\s*//p;q}' \ + | tr -s ' ' \ + | cut -d" " -f6) + test "${verbose_start_type}" = DEFAULT \ + && gcc_has_attribute_section_visible=yes \ + || gcc_has_attribute_section_visible=no; }], + [gcc_has_attribute_section_visible=no] ) - else - gcc_has_attribute_section=${ac_cv_link_attribute_section} - fi + AX_RESTORE_FLAGS + AC_MSG_RESULT($gcc_has_attribute_section_visible) + rm -f "conftest${shrext_cmds}" + + if test "x${gcc_has_attribute_section_visible}" = xno; then + # check if the linker script based workaround is + # feasible at all, otherwise fallback to using no + # section attribute while making some noise about it + # as combining with-without accustomed logging + # participants is currently uncharted waters + AC_MSG_CHECKING([whether linker workaround for orphan sections usable]) + >conftest.ld cat <<-EOF + SECTIONS { + __verbose : { + __start___verbose = .; + *(__verbose); + __stop___verbose = .; + } + } + EOF + LDFLAGS="${LDFLAGS} -Wl,conftest.ld" + AC_LINK_IFELSE( + [AC_LANG_PROGRAM( + [[#include + extern int __start___verbose[], __stop___verbose[]; + int test(void);]], + [[static int my_var __attribute__((section("__verbose"))) = 5; + assert("E:non-empty data section" + && __start___verbose != __stop___verbose); + assert("E:no data section value loss" + && my_var == 5);]] + )], + [], + [gcc_has_attribute_section=no] + ) + AX_RESTORE_FLAGS + AC_MSG_RESULT([$gcc_has_attribute_section]) + rm -f "conftest.ld" + fi - AC_MSG_RESULT($gcc_has_attribute_section) - if test $gcc_has_attribute_section = yes; then - 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}" = xyes; then + AC_DEFINE([QB_HAVE_ATTRIBUTE_SECTION], 1, + [Enabling code using __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 + elif test "x${enable_nosection_fallback}" = xyes; then + AC_MSG_NOTICE([Falling back to not using orphan section]) + else + AC_MSG_ERROR([Would use section attribute, cannot; see --enable-nosection-fallback]) + 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 && \ @@ -741,9 +859,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/include/qb/qblog.h b/include/qb/qblog.h index 7be1390fd..0af7d62ed 100644 --- a/include/qb/qblog.h +++ b/include/qb/qblog.h @@ -79,7 +79,10 @@ extern "C" { * scope in exactly one source file (preferably the main one) to be mixed into * the resulting compilation unit. This is a self-defensive measure for when * the linker-assisted collection of callsite data silently fails, which could - * otherwise go unnoticed, causing troubles down the road. + * otherwise go unnoticed, causing troubles down the road, but alas it cannot + * discern misuse of @c QB_LOG_INIT_DATA() macro in no-logging context from + * broken callsite section handling assumptions owing to overboard fancy + * linker -- situation that the self-check aims to detect in the first place. * * @par Configuring log targets. * A log target can be syslog, stderr, the blackbox, stdout, or a text file. @@ -287,12 +290,13 @@ extern struct qb_log_callsite QB_ATTR_SECTION_STOP[]; /* Related to the next macro that is -- unlike this one -- a public API */ #ifndef _GNU_SOURCE -#define QB_NONAPI_LOG_INIT_DATA_EXTRA_ \ +#define QB_NONAPI_LOG_INIT_DATA_EXTRA_(name) \ _Pragma(QB_PP_STRINGIFY(GCC warning QB_PP_STRINGIFY( \ without "_GNU_SOURCE" defined (directly or not) \ - QB_LOG_INIT_DATA cannot check sanity of libqb proper))) + QB_LOG_INIT_DATA cannot check sanity of libqb proper \ + nor of the target site originating this check alone))) #else -#define QB_NONAPI_LOG_INIT_DATA_EXTRA_ \ +#define QB_NONAPI_LOG_INIT_DATA_EXTRA_(name) \ { Dl_info work_dli; \ /* libqb sanity (locating libqb by it's relatively unique \ -- and currently only such per-linkage global one -- \ @@ -310,6 +314,23 @@ libqb's build is at fault, preventing reliable logging" \ assert("libqb's callsite section is populated, otherwise \ libqb's build is at fault, preventing reliable logging" \ && work_s1 != work_s2); \ + dlclose(work_handle); } \ + /* sanity of the target site originating this check alone */ \ + if (dladdr(dlsym(RTLD_DEFAULT, QB_PP_STRINGIFY(name)), &work_dli) \ + && (work_handle = dlopen(work_dli.dli_fname, \ + RTLD_LOCAL|RTLD_LAZY)) != NULL) { \ + work_s1 = (struct qb_log_callsite *) \ + dlsym(work_handle, QB_ATTR_SECTION_START_STR); \ + work_s2 = (struct qb_log_callsite *) \ + dlsym(work_handle, QB_ATTR_SECTION_STOP_STR); \ + assert("target's own callsite section observable, otherwise \ +target's own linkage at fault and logging would not work reliably \ +(unless QB_LOG_INIT_DATA macro used unexpectedly in no-logging context)"\ + && work_s1 != NULL && work_s2 != NULL); \ + assert("target's own callsite section non-empty, otherwise \ +target's own linkage at fault and logging would not work reliably \ +(unless QB_LOG_INIT_DATA macro used unexpectedly in no-logging context)"\ + && work_s1 != work_s2); \ dlclose(work_handle); } } #endif /* _GNU_SOURCE */ @@ -354,8 +375,7 @@ libqb's build is at fault, preventing reliable logging" \ target's and/or libqb's build is at fault, preventing reliable logging" \ && work_s1 != NULL && work_s2 != NULL); \ dlclose(work_handle); /* perhaps overly eager thing to do */ } \ - /* better targeted attestations when available */ \ - QB_NONAPI_LOG_INIT_DATA_EXTRA_; \ + QB_NONAPI_LOG_INIT_DATA_EXTRA_(name); \ /* finally, original, straightforward check */ \ assert("implicit callsite section is populated, otherwise \ target's build is at fault, preventing reliable logging" \ diff --git a/lib/Makefile.am b/lib/Makefile.am index 0bebeb5e7..1572cff5f 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -19,6 +19,7 @@ # along with libqb. If not, see . +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," +# 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,9 +100,42 @@ else endif endif - -pkgconfigdir = $(libdir)/pkgconfig -pkgconfig_DATA = libqb.pc +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/[*\/]/\\&/g;p;q;}' $<)/,$$ p" \ + > $@ + +# sadly, there's a distinction between "exec" and "data" install, and it's hard +# to decouple install-exec-hook below (.pc file is platform-dependent, anyway) +pkgconfigexecdir = $(libdir)/pkgconfig +pkgconfigexec_DATA = libqb.pc + +# 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 "$${t1_bn}" > "$(DESTDIR)$(libdir)/libqb.so-t" + so_ver=$$(cat "$(DESTDIR)$(libdir)/libqb.so-t"); \ + echo "INPUT($${so_ver})" > "$(DESTDIR)$(libdir)/libqb.so-t"; \ + cat $< >> "$(DESTDIR)$(libdir)/libqb.so-t"; \ + sed -i -- "s/libqb.so./$${so_ver}/" \ + "$(DESTDIR)$(libdir)/libqb.so-t" "$(DESTDIR)$(pkgconfigexecdir)/libqb.pc" + mv -f "$(DESTDIR)$(libdir)/libqb.so-t" "$(DESTDIR)$(libdir)/libqb.so" +endif if HAVE_SPLINT check_SCRIPTS = run_splint.sh diff --git a/lib/libqb.pc.in b/lib/libqb.pc.in index 37d27b74e..d9839bf41 100644 --- a/lib/libqb.pc.in +++ b/lib/libqb.pc.in @@ -8,5 +8,16 @@ Version: @PACKAGE_VERSION@ Description: libqb Requires: Libs: -L${libdir} -lqb @client_dlopen_LIBS@ +# NOTE: If -lqb not usable for linking (e.g. linker not compatible with +# linker scripts ad-hoc modifying output sections), try recent +# ld.bfd/binutils linker first when available, otherwise you can +# try "-l:libqb.so." link switch that bypasses said linker +# script -- but beware, logging may be less efficient and may lack +# possible future optimizations and extra features. Consequently, +# logging issues (typically bound to QB_LOG_INIT_DATA macro) can be +# mitigated with QB_KILL_ATTRIBUTE_SECTION macro defined for a build. +# NOTE: when concerned about a warning coming from the build process like +# warning: [...]libqb.so contains output sections; did you forget -T? +# and the build finishes OK, take it merely as a harmless side-effect Libs.private: @LIBS@ Cflags: -I${includedir} diff --git a/lib/log.c b/lib/log.c index e9cff8873..769087171 100644 --- a/lib/log.c +++ b/lib/log.c @@ -40,6 +40,13 @@ #include "util_int.h" #include +#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 000000000..f262df8cf --- /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 000000000..a188ac233 --- /dev/null +++ b/lib/qblog_script.ld.in @@ -0,0 +1,30 @@ +#include +/* 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 + + NOTE: If -lqb not usable for linking (e.g. linker not compatible with + linker scripts ad-hoc modifying output sections), try recent + ld.bfd/binutils linker first when available, otherwise you can + try "-l:libqb.so." link switch that bypasses said linker + script -- but beware, logging may be less efficient and may lack + possible future optimizations and extra features. Consequently, + logging issues (typically bound to QB_LOG_INIT_DATA macro) can be + mitigated with QB_KILL_ATTRIBUTE_SECTION macro defined for a build. + NOTE: When concerned about a warning coming from the build process like + warning: [...]libqb.so contains output sections; did you forget -T? + while it finishes OK, consider it merely a harmless side-effect. + */ +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 000000000..f037fcace --- /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 */ diff --git a/tests/functional/log_callsite_bench_gen.py b/tests/functional/log_callsite_bench_gen.py index d80011e20..4cb7e2968 100755 --- a/tests/functional/log_callsite_bench_gen.py +++ b/tests/functional/log_callsite_bench_gen.py @@ -112,7 +112,7 @@ def main(opts, args): width = int(floor(log10(CALLSITE_COUNT))) + 1 print('\n'.join([ - "/* compile with -lqb OR with -DQB_KILL_ATTRIBUTE_SECTION -lqb */", + "/* compile with -lqb OR with -DQB_KILL_ATTRIBUTE_SECTION -l:libqb.so.0 */", "#include ", ] + list_to_c_source(worklist, FNC_PREFIX, width) + [ "int main(int argc, char *argv[]) {", diff --git a/tests/functional/log_external/Makefile.am b/tests/functional/log_external/Makefile.am index b1b568a6d..ac0a2ea8e 100644 --- a/tests/functional/log_external/Makefile.am +++ b/tests/functional/log_external/Makefile.am @@ -36,5 +36,5 @@ log_callsite_bench_sectionfull_SOURCES = ../log_callsite_bench.c log_callsite_bench_sectionfull_LDFLAGS = -lqb log_callsite_bench_sectionless_SOURCES = ../log_callsite_bench.c -log_callsite_bench_sectionless_LDFLAGS = -lqb +log_callsite_bench_sectionless_LDFLAGS = -l:libqb.so.0 log_callsite_bench_sectionless_CPPFLAGS = -DQB_KILL_ATTRIBUTE_SECTION