libqb/0002-WIP-5-Experimental-fix-for-libqb-logging-not-working.patch

From 10cd5db8821e4d313ae5245b33061e02ba8d3ff5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= <jpokorny@redhat.com>
Date: Thu, 3 Aug 2017 17:40:09 +0200
Subject: [PATCH] WIP 5: Experimental fix for libqb logging not working with
 ld.bfd 2.29
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

What's missing:
* cross-testing programs vs. libqb prior/after this fix with ld.bfd 2.29
* program vs. library sanity attestation for good measure
  - code header-included in the program could/shall run-time one-off
    self-test the library code actually logs anything via custom callback
    log handler
  - library could/shall attest in qb_log_init that QB_ATTR_SECTION_START
    + STOP symbols belong to other address space than its own (dladdr?)
* related to that, we should really 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...

Version 2:
Simplify the compilation against libqb for its users that now no
longer need to worry to use a new flag for compilation with the
affected ld version, as semantically the same is already contained
in libqb.so itself (it's now a linker script instead of a common
symlink to the end versioned so file, which something known as an
implicit linker script:
https://sourceware.org/binutils/docs/ld/Implicit-Linker-Scripts.html).

Version 3:
Ditto for direct libqb.la users (incl. native examples and tests for
which the original Makefile.am form could have been restored thanks
to this -- not a subject of one-time configure check anymore),
to capture the case someone is using that libtool indirection directly
through a private checkout underneath the actual library user's repo.
The solutions is to slightly abuse libtool's library archive handling
and it's implicit dependency propagation within "dependency_libs"
variable, where we inject a reference to our own artificial library
archive that in turn eventually resolves to the discussed linker script.

Version 4:
Overcomes some unintended RPATH occurrences in qb-blackbox binary
or possibly libqb.so.*.  That's definitely not desired:
https://fedoraproject.org/wiki/Packaging:Guidelines#Beware_of_Rpath

Version 5:
Overcomes issues when HAVE_GCC_ATTRIBUTE_SECTION is not defined, hence
qblog_script.ld script used to carry unresolved symbolic references as
a consequence: https://bugzilla.redhat.com/show_bug.cgi?id=1487787
Now the script is not used at all in that case, and if accidentally was,
there is an extra guard conditionalizing the content referring to those
values to only the case they are expected to be defined.
Plus some more, missing in retrospect, conditionalizing is added.

Deficiencies of version 5 (some carried over, apparently):
- see What's missing above
- possibly needs and overhaul regarding documentation of the arrangement
  (now scattered throughout the files)

Deficiencies that are not solvable as long as we use the linker script
to participate in restoring boundary section symbols being global again:
- /usr/bin/ld: warning: ../lib/qblog_script.ld contains output sections; did you forget -T?

References:
http://oss.clusterlabs.org/pipermail/developers/2017-July/000503.html
https://bugzilla.redhat.com/show_bug.cgi?id=1477354#c8

Signed-off-by: Jan Pokorný <jpokorny@redhat.com>
---
 configure.ac           | 62 +++++++++++++++++++++++++++++++++++++---------
 lib/Makefile.am        | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/log.c              |  7 ++++++
 lib/qblog_script.la.in | 15 +++++++++++
 lib/qblog_script.ld.in | 17 +++++++++++++
 6 files changed, 156 insertions(+), 12 deletions(-)
 create mode 100644 lib/qblog_script.la.in
 create mode 100644 lib/qblog_script.ld.in

diff --git a/configure.ac b/configure.ac
index fd37466..f94af71 100644
--- a/configure.ac
+++ b/configure.ac
@@ -618,31 +618,68 @@ AC_SUBST(HAVE_SLOW_TESTS)
 
 # --- callsite sections ---
 if test "x${GCC}" = xyes; then
+	LDFLAGS_save="${LDFLAGS}"
 	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"
+		AC_LINK_IFELSE(
+			[AC_LANG_SOURCE(
+			    [[#include <assert.h>
+			     extern int __start___verbose[], __stop___verbose[];
+			     int test(void) {
+			       static int my_var __attribute__((section("__verbose"))) = 3;
+			       if (__start___verbose == __stop___verbose) assert(0);
+			       return *((int *) __start___verbose);
+			     }]])],
+			[gcc_has_attribute_section=yes; cp "conftest${EXEEXT}" "lib_conftest.so"],
+			[gcc_has_attribute_section=no]
+		)
+		LDFLAGS="${LDFLAGS_save}"
+	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 probable
+	# linking will fail readily (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 draw the
+	# full circle back to binary through standard return value passing
+	# (-rpath passed because LD_LIBRARY_PATH exporting is unwieldy here)
+	if test "x${gcc_has_attribute_section}" = xyes; then
+		AC_MSG_CHECKING([whether linker emits global symbols for orphan sections])
+		LDFLAGS="${LDFLAGS_save} -L. -l_conftest -Wl,-rpath=$(pwd)"
 		AC_TRY_RUN(
 			AC_LANG_PROGRAM(
 			    [[#include <assert.h>
-			     extern void * __start___verbose, * __stop___verbose;]],
+			     extern int __start___verbose[], __stop___verbose[];
+			     int test(void);]],
 			    [[static int my_var __attribute__((section("__verbose"))) = 5;
 			     if (__start___verbose == __stop___verbose) assert(0);
-			     if (my_var == 5) return 0;
-			     else return -1;
+			     return !(my_var == test() /*5?*/);
 			     ]]),
-			[gcc_has_attribute_section=yes],
-			[gcc_has_attribute_section=no]
+			[gcc_has_attribute_section_visible=yes],
+			[gcc_has_attribute_section_visible=no]
 		)
-	else
-		gcc_has_attribute_section=${ac_cv_link_attribute_section}
-	fi
+		LDFLAGS="${LDFLAGS_save}"
+		AC_MSG_RESULT($gcc_has_attribute_section_visible)
+		rm -f "lib_conftest.so"
 
-	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"
+		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 && \
@@ -721,7 +758,8 @@ 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_OUTPUT
 
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 0bebeb5..85dd434 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
+
 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 -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..b475835
--- /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.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
+}
-- 
2.14.1