From 3f9ccfc3b291196657edb2e920327f2d26804d6a 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 1/6] Med: qblog.h: better explanation + behaviour of

 QB_LOG_INIT_DATA

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

Based on better understanding how link-time callsite collection works,

put a better description for the macro.  Also based on poor user

experience in case that feature does not work well, say because

the linker deliberately changes the previously settled visibility

of the section boundary symbols (happened in ld from binutils-2.29,

fix is forthcoming), tweak the assertion message a bit, together

with an extension of the general intro to point that macro out.

Also play fair, include the <assert.h> header, which this macro

requires.

- use case:

  /usr/sbin/pacemakerd --features

- before:

  pacemakerd: utils.c:69: common:

  Assertion `0' failed

- after:

  pacemakerd: utils.c:69: common:

  Assertion `"non-empty callsite section" && QB_ATTR_SECTION_START != QB_ATTR_SECTION_STOP' failed.

Restructuring of the assertion inspired by the suggestion of Ferenc

Wц║gner (for the subsequent commit, actually).

And regarding:

> as a side effect, it can ensure the boundary-denoting symbols for

> the target collection area are kept alive with some otherwise unkind

> linkers

this was actually empirically discovered in one particular combination

with ld.bfd @ binutils 2.29, and extensively with 2.29.1 (placing here

a forward reference for the following commits that elaborate on the

libqb-target cross-combination matrix and the findings).

Signed-off-by: Jan Pokornц╫ <jpokorny@redhat.com>

---

 include/qb/qblog.h | 43 ++++++++++++++++++++++++++++++++++++++-----

 1 file changed, 38 insertions(+), 5 deletions(-)

diff --git a/include/qb/qblog.h b/include/qb/qblog.h

index 3cb4eef..7b6db8b 100644

--- a/include/qb/qblog.h

+++ b/include/qb/qblog.h

@@ -42,6 +42,10 @@ extern "C" {

 #undef QB_HAVE_ATTRIBUTE_SECTION

 #endif /* S_SPLINT_S */

+#ifdef QB_HAVE_ATTRIBUTE_SECTION

+#include <assert.h>  /* possibly needed for QB_LOG_INIT_DATA */

+#endif

+

 /**

  * @file qblog.h

  * The logging API provides four main parts (basics, filtering, threading & blackbox).

@@ -64,6 +68,17 @@ extern "C" {

  * }

  * @endcode

  *

+ * @note

+ * In practice, such a minimalistic approach hardly caters real use cases.

+ * Following section discusses the customization.  Moreover, it's quite

+ * vital to instrument the target user of this logging subsystem with

+ * @c QB_LOG_INIT_DATA() macro placed in the top file scope in an 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.

+ *

+ *

  * @par Configuring log targets.

  * A log target can be syslog, stderr, the blackbox, stdout, or a text file.

  * By default only syslog is enabled.

@@ -268,11 +283,29 @@ 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[];

-/* mere linker sanity check, possible future extension for internal purposes */

-#define QB_LOG_INIT_DATA(name)							\

-    void name(void);								\

-    void name(void) 								\

-    { if (QB_ATTR_SECTION_START == QB_ATTR_SECTION_STOP) assert(0); }	\

+/* 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 (but see below), which

+   is presumably assured by it's author as of relying on this very macro

+   [technically, the symbols that happen to be resolved under the respective

+   identifiers do not necessarily come from the same compilation unit level

+   as when it's not the end program (or by induction, a library higher in

+   the symbol resolution fall-through) but a shared library, the former takes

+   a precedence unless it lacks use of logging on its own making its very

+   section empty and hence without such boundary symbols provided at the

+   respective level and thus deferring to lower priority symbol resolution

+   stop -- despite this fuzzily targeted attestation, the check remains

+   reasonable and should not harm anything];

+   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 */

+#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 __attribute__ ((constructor)) name(void);

 #else

 #define QB_LOG_INIT_DATA(name)

-- 

2.14.2