rpms/libqb
5
0
Fork 0
Code Issues Pull Requests Releases Activity
95877b0e29
libqb/04-Med-add-extra-run-time-client-libqb-checks-that-logg.patch
Jan Pokorný 95877b0e29
1.0.2-10 - Evolution of the previous (rhbz#1478089, rhbz#1487787) 
Signed-off-by: Jan Pokorný <jpokorny@redhat.com>
2017-10-13 23:04:41 +02:00

491 lines
22 KiB
Diff
Raw Blame History

From 0c8500a0c15f54250ce93c724451c36cde8b4499 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 4/6] Med: add extra run-time (client, libqb) checks that
logging will work
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The table at the bottom concludes how the test matrix overview
introduced with a message for the preceding commit (also introducing
log_test_mock.sh runner which got reused here) looks as of this
refreshed sanity check, once QB_LOG_INIT_DATA macro at hand gets
applied (meaning "for non-libqb logging participants" so as not
complicate the matrix further). That macro is nothing triggered
directly, it will just plant a constructor-like function (to be
invoked automatically early in the execution) that will run through
the checks (one original and couple of new ones as of this changeset).
Note that for libqb users, this implies a new link dependency on libdl,
because they may opt-in for refreshed QB_LOG_INIT_DATA sanity check that
calls out to dlopen/dlsym/dladdr directly in case of "attribute section"
being available for the particular platform, and hence immediately needs
those symbols resolved in link time. Hence, add this conditional link
dependency to libqb.pc pkg-config file under Libs variable -- we
actually restore the occurrence of "-ldl" there as it used to be present
until commit 56754d0. While doing so, also move immediate link
dependencies of libqb (if any, currently not but that may be
a regression arising from the cleanup related to the mentioned commit)
represented with the LIBS autoconf variable under Libs.private variable
in libqb.pc, where it belongs per pkg-config documentation.
The promised table follows, 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 only 2.29)
goes like this (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 : BAD[*2] #<BAD[*E]>|<BAD[*F]>:<BAD[*G]>#
# x = b # BAD[*A] | BAD[*B] : BAD[*C] #<BAD[*E]>|<BAD[*F]>:<BAD[*G]>#
+=========+=========+=========+=========+=========+=========+=========+
whereas if we swap 2.29 for 2.29.1, i.e., X(b) .. ld.bfd = 2.29.1, we
can observe a somewhat simpler story (DEP ~ "depends"):
Woefully, nothing changes if we swap binutils 2.29.1 for 2.29, i.e.,
X(b) .. ld.bfd = 2.29.1. The second table from the previous commit
hence remains the same. The added sanity checks are useful nonetheless,
consider for example, that attribute-section-less libqb is what gets
run-time linked to an attribute-section-full target. The most precise
check we could use -- a custom logger function applied in a self-test
scheme -- is not available at the point the macro-defined function gets
invoked, simply because qb_log_init hasn't been invoked by the time
that constructor gets triggered. However, what we can do is to add
a non-trapping libqb-residing reverse-testing of the client space that
invokes qb_log_init (being intrusive all of a sudden at some unknown
execution point, as opposed to constructor-like function, seems pretty
bad idea + libqb as a library is a mere helper, not an undertaker :)
-- this check can at least announce, via syslog (the only
initially enabled logging target) that the target logging won't work.
After incorporating such a change (and extending log_test_mock.sh so as
to capture syslog stream within the container), nothing changes with the
table above, but desirably changes with "X(b) .. ld.bfd = 2.29.1" one:
+=========+=========+=========+=========+=========+=========+=========+
#client(x)# libqb(a) usage # libqb(b) usage #
# vvv #---------+---------+---------+---------+---------+---------+
# V # direct | libX(a) : libX(b) # direct | libX(a) : libX(b) #
+=========+=========+=========+=========+=========+=========+=========+
# x = a # OK | OK : DEP[*J] #<BAD[*M]>|<BAD[*M]>:<BAD[*L]>#
# x = b #<DEP[*N]>| DEP[*I] :<DEP[*O]>#<BAD[*M]>|<BAD[*M]>:<BAD[*L]>#
+=========+=========+=========+=========+=========+=========+=========+
[*1] client logging not working
[*2] interlib logging not working
[*3] both client and interlib logging not working
[*A] boils down to [*1], unless QB_LOG_INIT_DATA used on client side,
which then fails on
"non-empty implicit callsite section, otherwise target's linkage at
fault and logging would not work reliably"
assertion
[*B] boils down to [*1], unless QB_LOG_INIT_DATA used on interlib side,
which then fails on
"non-empty implicit callsite section, otherwise target's linkage at
fault and logging would not work reliably"
assertion
[*C] boils down to [*3], unless QB_LOG_INIT_DATA used on interlib side,
which then fails on
"non-empty implicit callsite section, otherwise target's linkage at
fault and logging would not work reliably"
assertion
[*E] boils down to [*1], unless QB_LOG_INIT_DATA used on client side,
which then fails on
"target's callsite section self-observable, otherwise target's
and/or libqb's linkage at fault and logging would not work reliably"
assertion
[*F] boils down to [*3], unless QB_LOG_INIT_DATA used on interlib side,
which then fails on
"libqb's callsite section non-empty, otherwise libqb's linkage at
fault and logging would not work reliably"
assertion
[*G] boils down to [*3], unless QB_LOG_INIT_DATA used on interlib side,
which then fails on
"target's callsite section self-observable, otherwise target's and/or
libqb's linkage at fault and logging would not work reliably"
assertion
[*I] boils down to [*1], unless QB_LOG_INIT_DATA used on client side,
which makes it, likely through self-reference keepalive (see
below) work OK
[*J] boils down to [*2], unless QB_LOG_INIT_DATA used on interlib side,
which makes it, likely through self-reference keepalive (see
below) work OK
[*K] boils down to [*3]
in addition, syslog carries this error:
"target chain supplied section not observed by libqb, target's and/or
libqb's linkage at fault and logging of the target will not work
reliably"
logged by libqb proper
[*L] boils down to [*3], unless QB_LOG_INIT_DATA used on interlib side
(sufficient?), which makes it, likely through self-reference
keepalive (see below) boil down just to [*1];
in addition, syslog carries this error:
"target chain supplied section not observed by libqb, target's and/or
libqb's linkage at fault and logging of the target will not work
reliably"
logged by libqb proper
[*M] boils down to [*1];
in addition, syslog carries this error:
"target chain supplied section not observed by libqb, target's and/or
libqb's linkage at fault and logging of the target will not work
reliably"
logged by libqb proper
[*N] boils down to [*M], unless QB_LOG_INIT_DATA used on client side,
which makes it, likely through self-reference keepalive (see
below) work OK
[*O] boils down to [*K], unless QB_LOG_INIT_DATA used on both client
and interlib side, which makes it, likely through self-reference
keepalive (see below) work OK (it's expected that this a mere
composite of situations [*I] and [*J] with consequences as stated)
Note: the only problematic (i.e. not captured automatically by the
QB_LOG_INIT_DATA macro presumably utilized at every non-libqb logging
system participant in the form of a discrete compilation unit)
combination with 2.29 is the one intersecting at "BAD[*2]" pertaining
"everything but interlib compiled with ld.bfd < 2.29". It would, of
course, be solvable as well, but presumably not in an easy way, and
that use case should not be as frequent.
Takeway: whenever your target (library or client program) actively
utilizes logging (meaning it emits at least a single log message),
YOU ARE strongly ENCOURAGED TO USE QB_LOG_INIT_DATA macro function
at (exactly) one of the source code files (presumably the main one)
per respective target's compilation unit. It will alleviate the
hassles possibly caused by downgrading libqb to the linker-vs-libqb
incompatibly compiled one or in similar circumstances arising merely
from the linker behaviour change, which the current build system/code
quake is all about.
Signed-off-by: Jan Pokorný <jpokorny@redhat.com>
---
configure.ac | 1 +
include/qb/qblog.h | 63 ++++++++++++++++++++++++++++---
lib/libqb.pc.in | 3 +-
lib/log.c | 44 +++++++++++++++++++++
tests/functional/Makefile.am | 3 +-
tests/functional/log.am | 2 +-
tests/functional/log_external/Makefile.am | 8 +++-
tests/functional/log_test_mock.sh | 13 ++++++-
tests/functional/syslog-stdout.py | 49 ++++++++++++++++++++++++
9 files changed, 173 insertions(+), 13 deletions(-)
create mode 100755 tests/functional/syslog-stdout.py
diff --git a/configure.ac b/configure.ac
index 274b49c..5769e60 100644
--- a/configure.ac
+++ b/configure.ac
@@ -642,6 +642,7 @@ if test "x${GCC}" = xyes; 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])
fi
fi
diff --git a/include/qb/qblog.h b/include/qb/qblog.h
index d39803f..9474c16 100644
--- a/include/qb/qblog.h
+++ b/include/qb/qblog.h
@@ -44,6 +44,7 @@ extern "C" {
#ifdef QB_HAVE_ATTRIBUTE_SECTION
#include <assert.h> /* possibly needed for QB_LOG_INIT_DATA */
+#include <dlfcn.h>
#endif
/**
@@ -283,20 +284,70 @@ typedef void (*qb_log_filter_fn)(struct qb_log_callsite * cs);
extern struct qb_log_callsite QB_ATTR_SECTION_START[];
extern struct qb_log_callsite QB_ATTR_SECTION_STOP[];
-/* optional on-demand self-check of (1) toolchain sanity (prerequisite for
+/* Related to the next macro that is -- unlike this one -- a public API */
+#ifndef _GNU_SOURCE
+#define QB_NONAPI_LOG_INIT_DATA_EXTRA_ \
+ _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)))
+#else
+#define QB_NONAPI_LOG_INIT_DATA_EXTRA_ \
+ /* libqb sanity (locating libqb by it's relatively unique \
+ -- and currently only such per-linkage global one -- \
+ non-functional symbol, due to possible confusion otherwise) */ \
+ if (dladdr(dlsym(RTLD_DEFAULT, "facilitynames"), &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("libqb's callsite section observable, otherwise libqb's \
+linkage at fault and logging would not work reliably" \
+ && work_s1 != NULL && work_s2 != NULL); \
+ assert("libqb's callsite section non-empty, otherwise libqb's \
+linkage at fault and logging would not work reliably" \
+ && work_s1 != work_s2); \
+ dlclose(work_handle); }
+#endif
+
+/**
+ * Optional on-demand self-check of (1) toolchain sanity (prerequisite for
* the logging subsystem to work properly) and (2) non-void active use of
* logging (satisfied with a justifying existence of a logging callsite as
* defined with a @c qb_logt invocation) at the target binary, which is
* presumably assured by it's author as of relying on this very macro;
* only effective when link-time ("run-time amortizing") callsite collection
* is; as a side effect, it can ensure the boundary-denoting symbols for the
- * target collection area are kept alive with some otherwise unkind linkers;
- * may be extended in future for more in-depth self-validation */
+ * target collection area are kept alive with some otherwise unkind linkers.
+ *
+ * Applying this macro in the target program/library is strongly recommended
+ * whenever the logging as framed by this header file is in use.
+ * Moreover, it's important to state that using this check while not ensuring
+ * @c _GNU_SOURCE macro definition is present at compile-time means only half
+ * of the available sanity checking will be performed, possibly resulting
+ * in libqb's own internally logged messages being lost without warning.
+ */
#define QB_LOG_INIT_DATA(name) \
void name(void); \
- void name(void) \
- { assert("non-empty callsite section" \
- && QB_ATTR_SECTION_START != QB_ATTR_SECTION_STOP); } \
+ void name(void) { \
+ void *work_handle; struct qb_log_callsite *work_s1, *work_s2; \
+ Dl_info work_dli; \
+ /* our own (target's) sanity */ \
+ if ((work_handle = dlopen(NULL, 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 callsite section self-observable, otherwise \
+target's and/or libqb's linkage at fault and logging would not work \
+reliably" \
+ && work_s1 != NULL && work_s2 != NULL); } \
+ QB_NONAPI_LOG_INIT_DATA_EXTRA_; \
+ /* original "cherry on the cake" check */ \
+ assert("non-empty implicit callsite section, otherwise target's \
+linkage at fault and logging would not work reliably" \
+ && QB_ATTR_SECTION_START != QB_ATTR_SECTION_STOP); } \
void __attribute__ ((constructor)) name(void);
#else
#define QB_LOG_INIT_DATA(name)
diff --git a/lib/libqb.pc.in b/lib/libqb.pc.in
index 8a8d0ba..37d27b7 100644
--- a/lib/libqb.pc.in
+++ b/lib/libqb.pc.in
@@ -7,5 +7,6 @@ Name: libqb
Version: @PACKAGE_VERSION@
Description: libqb
Requires:
-Libs: -L${libdir} -lqb @LIBS@
+Libs: -L${libdir} -lqb @client_dlopen_LIBS@
+Libs.private: @LIBS@
Cflags: -I${includedir}
diff --git a/lib/log.c b/lib/log.c
index bfd218f..f30b8d8 100644
--- a/lib/log.c
+++ b/lib/log.c
@@ -853,6 +853,18 @@ qb_log_init(const char *name, int32_t facility, uint8_t priority)
{
int32_t l;
enum qb_log_target_slot i;
+#ifdef QB_HAVE_ATTRIBUTE_SECTION
+ void *work_handle; struct qb_log_callsite *work_s1, *work_s2;
+ Dl_info work_dli;
+#endif /* QB_HAVE_ATTRIBUTE_SECTION */
+ /* cannot reuse single qb_log invocation in various contexts
+ through the variables (when section attribute in use),
+ hence this indirection */
+ enum {
+ preinit_err_none,
+ preinit_err_target_sec,
+ preinit_err_target_empty,
+ } preinit_err = preinit_err_none;
l = pthread_rwlock_init(&_listlock, NULL);
assert(l == 0);
@@ -871,6 +883,25 @@ qb_log_init(const char *name, int32_t facility, uint8_t priority)
qb_log_dcs_init();
#ifdef QB_HAVE_ATTRIBUTE_SECTION
+ /* sanity check that target chain supplied QB_ATTR_SECTION_ST{ART,OP}
+ symbols and hence the local references to them are not referencing
+ the proper libqb's ones (locating libqb-self by it's relatively
+ unique -- and currently only such per-linkage global one --
+ non-functional symbol, due to possible confusion otherwise) */
+ if (dladdr(dlsym(RTLD_DEFAULT, "facilitynames"), &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);
+ if (work_s1 == QB_ATTR_SECTION_START
+ || work_s2 == QB_ATTR_SECTION_STOP) {
+ preinit_err = preinit_err_target_sec;
+ } else if (work_s1 == work_s2) {
+ preinit_err = preinit_err_target_empty;
+ }
+ }
qb_log_callsites_register(QB_ATTR_SECTION_START, QB_ATTR_SECTION_STOP);
dl_iterate_phdr(_log_so_walk_callback, NULL);
_log_so_walk_dlnames();
@@ -884,6 +915,19 @@ qb_log_init(const char *name, int32_t facility, uint8_t priority)
_log_target_state_set(&conf[QB_LOG_SYSLOG], QB_LOG_STATE_ENABLED);
(void)qb_log_filter_ctl(QB_LOG_SYSLOG, QB_LOG_FILTER_ADD,
QB_LOG_FILTER_FILE, "*", priority);
+
+ if (preinit_err == preinit_err_target_sec)
+ qb_util_log(LOG_ERR, "target chain supplied section not"
+ " observed by libqb, target's and/or"
+ " libqb's linkage at fault and"
+ " logging of the target will not"
+ " work reliably");
+ else if (preinit_err == preinit_err_target_empty) {
+ qb_util_log(LOG_WARNING, "target chain supplied section empty,"
+ " target's linkage at fault and"
+ " logging of the target will not"
+ " work reliably");
+ }
}
void
diff --git a/tests/functional/Makefile.am b/tests/functional/Makefile.am
index 72f551e..c079009 100644
--- a/tests/functional/Makefile.am
+++ b/tests/functional/Makefile.am
@@ -19,5 +19,6 @@
MAINTAINERCLEANFILES = Makefile.in
EXTRA_DIST = GNUmakefile log_client.err log_interlib_client.err \
- log_test_client.sh log_test_interlib_client.sh log_test_mock.sh
+ log_test_client.sh log_test_interlib_client.sh \
+ log_test_mock.sh syslog-stdout.py
SUBDIRS = log_external log_internal
diff --git a/tests/functional/log.am b/tests/functional/log.am
index 5fe1ae7..0753fa5 100644
--- a/tests/functional/log.am
+++ b/tests/functional/log.am
@@ -20,7 +20,7 @@
MAINTAINERCLEANFILES = Makefile.in
CLEANFILES = log_client.err.real log_interlib_client.err.real
-AM_CPPFLAGS = -I$(top_builddir)/include -I$(top_srcdir)/include
+AM_CPPFLAGS = -D_GNU_SOURCE -I$(top_builddir)/include -I$(top_srcdir)/include
noinst_PROGRAMS = log_client log_interlib_client
# cannot use {check,noinst}_LTLIBRARIES because it leads to solely static lib
diff --git a/tests/functional/log_external/Makefile.am b/tests/functional/log_external/Makefile.am
index 36aa0af..ca1c8a5 100644
--- a/tests/functional/log_external/Makefile.am
+++ b/tests/functional/log_external/Makefile.am
@@ -19,5 +19,9 @@
include ../log.am
-log_client_LDFLAGS = -lqb
-liblog_inter_la_LIBADD = -lqb
+# while linking with system-wide version of libqb, we are still pursuing
+# local in-tree header file, hence we need to link with dynamic linking
+# library (which is a prerequisite for using QB_LOG_INIT_DATA defined
+# in qblog.h) explicitly
+log_client_LDFLAGS = -lqb @client_dlopen_LIBS@
+liblog_inter_la_LIBADD = -lqb @client_dlopen_LIBS@
diff --git a/tests/functional/log_test_mock.sh b/tests/functional/log_test_mock.sh
index 4489718..5900f6c 100755
--- a/tests/functional/log_test_mock.sh
+++ b/tests/functional/log_test_mock.sh
@@ -165,10 +165,19 @@ do_compile_and_test_client () {
-exec rm -- {} \;"
;;
esac
+ mock ${mock_args} --copyin "syslog-stdout.py" "builddir"
mock ${mock_args} --shell "( cd \"builddir/build/BUILD/$1\"; ./configure )"
mock ${mock_args} --shell \
- "make -C \"builddir/build/BUILD/$1/tests/functional/log_external\" \
- check-TESTS \"TESTS=../${_logfile}.sh\" $5" \
+ "python3 builddir/syslog-stdout.py \
+ >\"builddir/build/BUILD/$1/tests/functional/log_external/.syslog\" & \
+ { sleep 2; make -C \"builddir/build/BUILD/$1/tests/functional/log_external\" \
+ check-TESTS \"TESTS=../${_logfile}.sh\" $5; } \
+ && ! test -s \"builddir/build/BUILD/$1/tests/functional/log_external/.syslog\"; \
+ ret_ec=\$?; \
+ ( cd \"builddir/build/BUILD/$1/tests/functional/log_external\"; \
+ cat .syslog >> test-suite.log; \
+ echo SYSLOG-begin; cat .syslog; echo SYSLOG-end ); \
+ ret () { return \$1; }; ret \${ret_ec}" \
&& _result="${_result}_good" \
|| _result="${_result}_bad"
mock ${mock_args} --copyout \
diff --git a/tests/functional/syslog-stdout.py b/tests/functional/syslog-stdout.py
new file mode 100755
index 0000000..a0a761b
--- /dev/null
+++ b/tests/functional/syslog-stdout.py
@@ -0,0 +1,49 @@
+#!/usr/bin/python3
+# Copyright 2017 Red Hat, Inc.
+#
+# Author: Jan Pokorny <jpokorny@redhat.com>
+#
+# This file is part of libqb.
+#
+# libqb is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Lesser General Public License as published by
+# the Free Software Foundation, either version 2.1 of the License, or
+# (at your option) any later version.
+#
+# libqb is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with libqb. If not, see <http://www.gnu.org/licenses/>.
+
+import socket
+from atexit import register
+from os import remove
+from sys import argv
+
+# no locking, but anyway
+try:
+ remove("/dev/log")
+except FileNotFoundError:
+ pass
+sock = socket.socket(socket.AF_UNIX, socket.SOCK_DGRAM)
+sock.bind("/dev/log")
+
+def shutdown():
+ sock.close()
+ remove("/dev/log")
+
+def main(*argv):
+ register(shutdown)
+ while True:
+ try:
+ b = sock.recv(4096)
+ # flushing is crucial here
+ print(">>> " + str(b, 'ascii').split(' ', 3)[-1], flush=True)
+ except IOError:
+ pass
+
+if __name__ == '__main__':
+ main(*argv)
--
2.14.2
Reference in New Issue View Git Blame Copy Permalink