Fix: mark (again) section start/end symbols as hidden

Commit 1353b066072e ("Visibility hidden by default") broke the plugin
loading on RHEL 7 (and probably other Linux systems with an older
linker).  The ld version on RHEL 7 is 2.27-44.base.el7_9.1.

The symptom is that plugins aren't loaded:

    $ ./src/cli/babeltrace2 list-plugins
    From the following plugin paths:

      - /home/mjeanson/Git/babeltrace/src/plugins/ctf
      - /home/mjeanson/Git/babeltrace/src/plugins/text
      - /home/mjeanson/Git/babeltrace/src/plugins/utils
      - /home/mjeanson/Git/babeltrace/src/plugins/lttng-utils

    No plugins found.

While debugging, I found this strange behavior that I am unable to
explain:

    (gdb) frame
    #0  __bt_get_begin_section_plugin_descriptors () at /home/mjeanson/Git/babeltrace/src/plugins/ctf/plugin.cpp:16
    16	BT_PLUGIN_MODULE();
    (gdb) disassemble
    Dump of assembler code for function __bt_get_begin_section_plugin_descriptors():
       0x00007ffff57b0045 <+0>:	push   %rbp
       0x00007ffff57b0046 <+1>:	mov    %rsp,%rbp
    => 0x00007ffff57b0049 <+4>:	mov    0x2b8f68(%rip),%rax        # 0x7ffff5a68fb8
       0x00007ffff57b0050 <+11>:	pop    %rbp
       0x00007ffff57b0051 <+12>:	retq
    End of assembler dump.
    (gdb) si
    0x00007ffff57b0050	16	BT_PLUGIN_MODULE();
    (gdb) p/x $rax
    $2 = 0x7ffff7bc6dd0

We are in the function that is supposed to return the beginning of the
__bt_plugin_descriptor section for the babeltrace-plugin-ctf.so shared
object (the address of the __start___bt_plugin_descriptor symbol).  The
address displayed by the disassembler (0x7ffff5a68fb8) appears correct.
It is the sum of 0x2b8f68 and the (next instruction's) PC,
0x7ffff57b0050.  However, after stepping, we see that rax contains a
different value, 0x7ffff7bc6dd0.  I do not understand how it's possible
for this instruction to give that result.  But this address
(0x7ffff7bc6dd0) is the beginning of the __bt_plugin_descriptor section
of libbabeltrace2.so.  So, the symbol from libbabeltrace2.so seems to
hide the symbol of the same name in babeltrace-plugin-ctf.so.  The
function that is supposed to return the start of the plugin descriptor
section in babeltrace-plugin-ctf.so actually returns the start of that
section but in libbabeltrace2.so.

I checked the properties of the special section start/end symbols, both
on RHEL7 and on a more recent system (Arch), for both before and after
1353b066072e.  Here are the results:

RHEL7 before:  00000000002c9768     0 NOTYPE  LOCAL  DEFAULT   26 __start___bt_plugin_descriptors
RHEL7 after :  00000000002c86c8     0 NOTYPE  GLOBAL DEFAULT   26 __start___bt_plugin_descriptors
Arch  before:  0000000000236a00     0 NOTYPE  GLOBAL HIDDEN    27 __start___bt_plugin_descriptors
Arch  after :  00000000002326c0     0 NOTYPE  GLOBAL PROTECTED   27 __start___bt_plugin_descriptors

On RHEL7 the symbols originally had LOCAL binding, meaning they weren't
used for relocation outside their object.  The
__start___bt_plugin_descriptor symbol from libbabeltrace2.so therefore
didn't override the one in babeltrace-plugin-ctf.so.  Similarly, on
Arch, the symbols had HIDDEN visibility, so they also didn't cross
shared object boundary.

After 1353b066072e, the symbols on RHEL7 were "GLOBAL DEFAULT", meaning
the symbols from libbabeltrace2.so would override the symbols with the
same names in plugin (which are necessarily loaded after the main
library).  On Arch, things kept working because the symbols use the
PROTECTED visibility.  ld started to use PROTECTED visibility at some
point for these special section start/end symbols, because it makes no
sense to make them overridable (which they are with the DEFAULT
visibility).  The code in a given file always wants to refer to the
section start/end symbols of its own file.

Note that starting with ld 2.35, we can use the "-z
start-stop-visibility" option to control the visibility of those
symbols.  In some way, it is the linker equivalent of
-fvisibility=hidden for the compiler (just for these start/stop
symbols).  But it doesn't help us for these older systems.

Here are the relevant changes I found in the ld history:

 * This commit (included in binutils 2.29) made the symbols hidden:

   Always define referenced __start_SECNAME/__stop_SECNAME
   https://inbox.sourceware.org/binutils/20170610224649.GA12339@gmail.com/
   https://gitlab.com/gnutools/binutils-gdb/-/commit/cbd0eecf261c2447781f8c89b0d955ee66fae7e9

 * This commit (included in binutils 2.29.1 and binutils 2.30) made the
   symbols protected, after people complained they couldn't look them
   up:

   Make __start/__stop symbols protected visibility
   https://inbox.sourceware.org/binutils/CAMe9rOq5vJzAw0-Quy-J_-UZH9tpoxa4jXWdks255nuFoph7zA@mail.gmail.com/
   https://gitlab.com/gnutools/binutils-gdb/-/commit/487b6440dad57440939fab7afdd84a218b612796

 * This commit (included in binutils 2.35) added the option to control
   the visibility:

   gold, ld: Implement -z start-stop-visibility=... option.
   https://inbox.sourceware.org/binutils/CAB=4xhqb63g_b8K_BJiKne8N+X9zRO_0sYZw+0UNTTFTbdV3OA@mail.gmail.com/
   https://gitlab.com/gnutools/binutils-gdb/-/commit/cae64165f47b64898c4f1982d294862cfae89a47

Fix all this by making these symbols explicitly hidden again, as they
were before 1353b066072e.

Change-Id: Iad6c07691bbfdcbd56948cdc7ccd241d3d8311e3
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/9726
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>

diff --git a/include/babeltrace2/plugin/plugin-dev.h b/include/babeltrace2/plugin/plugin-dev.h
index 7a862c05bb47071c6faa325a28958c6d798447f3..f4444f92040f7d96fd7a5414cef6419ef01762dd 100644 (file)
--- a/include/babeltrace2/plugin/plugin-dev.h
+++ b/include/babeltrace2/plugin/plugin-dev.h
@@ -17,6 +17,17 @@
 #include <babeltrace2/graph/message-iterator-class.h>
 #include <babeltrace2/types.h>
 
+/*
+ * _BT_HIDDEN: set the hidden attribute for internal functions
+ * On Windows, symbols are local unless explicitly exported,
+ * see https://gcc.gnu.org/wiki/Visibility
+ */
+#if defined(_WIN32) || defined(__CYGWIN__)
+#define _BT_HIDDEN
+#else
+#define _BT_HIDDEN __attribute__((visibility("hidden")))
+#endif
+
 /*
  * _BT_EXPORT: set the visibility for exported functions.
  */
@@ -319,20 +330,20 @@ other <code>BT_PLUGIN*()</code> macro.
 */
 #define BT_PLUGIN_MODULE() \
        static struct __bt_plugin_descriptor const * const __bt_plugin_descriptor_dummy __BT_PLUGIN_DESCRIPTOR_ATTRS = NULL; \
-       extern struct __bt_plugin_descriptor const *__BT_PLUGIN_DESCRIPTOR_BEGIN_SYMBOL __BT_PLUGIN_DESCRIPTOR_BEGIN_EXTRA; \
-       extern struct __bt_plugin_descriptor const *__BT_PLUGIN_DESCRIPTOR_END_SYMBOL __BT_PLUGIN_DESCRIPTOR_END_EXTRA; \
+       _BT_HIDDEN extern struct __bt_plugin_descriptor const *__BT_PLUGIN_DESCRIPTOR_BEGIN_SYMBOL __BT_PLUGIN_DESCRIPTOR_BEGIN_EXTRA; \
+       _BT_HIDDEN extern struct __bt_plugin_descriptor const *__BT_PLUGIN_DESCRIPTOR_END_SYMBOL __BT_PLUGIN_DESCRIPTOR_END_EXTRA; \
        \
        static struct __bt_plugin_descriptor_attribute const * const __bt_plugin_descriptor_attribute_dummy __BT_PLUGIN_DESCRIPTOR_ATTRIBUTES_ATTRS = NULL; \
-       extern struct __bt_plugin_descriptor_attribute const *__BT_PLUGIN_DESCRIPTOR_ATTRIBUTES_BEGIN_SYMBOL __BT_PLUGIN_DESCRIPTOR_ATTRIBUTES_BEGIN_EXTRA; \
-       extern struct __bt_plugin_descriptor_attribute const *__BT_PLUGIN_DESCRIPTOR_ATTRIBUTES_END_SYMBOL __BT_PLUGIN_DESCRIPTOR_ATTRIBUTES_END_EXTRA; \
+       _BT_HIDDEN extern struct __bt_plugin_descriptor_attribute const *__BT_PLUGIN_DESCRIPTOR_ATTRIBUTES_BEGIN_SYMBOL __BT_PLUGIN_DESCRIPTOR_ATTRIBUTES_BEGIN_EXTRA; \
+       _BT_HIDDEN extern struct __bt_plugin_descriptor_attribute const *__BT_PLUGIN_DESCRIPTOR_ATTRIBUTES_END_SYMBOL __BT_PLUGIN_DESCRIPTOR_ATTRIBUTES_END_EXTRA; \
        \
        static struct __bt_plugin_component_class_descriptor const * const __bt_plugin_component_class_descriptor_dummy __BT_PLUGIN_COMPONENT_CLASS_DESCRIPTOR_ATTRS = NULL; \
-       extern struct __bt_plugin_component_class_descriptor const *__BT_PLUGIN_COMPONENT_CLASS_DESCRIPTOR_BEGIN_SYMBOL __BT_PLUGIN_COMPONENT_CLASS_DESCRIPTOR_BEGIN_EXTRA; \
-       extern struct __bt_plugin_component_class_descriptor const *__BT_PLUGIN_COMPONENT_CLASS_DESCRIPTOR_END_SYMBOL __BT_PLUGIN_COMPONENT_CLASS_DESCRIPTOR_END_EXTRA; \
+       _BT_HIDDEN extern struct __bt_plugin_component_class_descriptor const *__BT_PLUGIN_COMPONENT_CLASS_DESCRIPTOR_BEGIN_SYMBOL __BT_PLUGIN_COMPONENT_CLASS_DESCRIPTOR_BEGIN_EXTRA; \
+       _BT_HIDDEN extern struct __bt_plugin_component_class_descriptor const *__BT_PLUGIN_COMPONENT_CLASS_DESCRIPTOR_END_SYMBOL __BT_PLUGIN_COMPONENT_CLASS_DESCRIPTOR_END_EXTRA; \
        \
        static struct __bt_plugin_component_class_descriptor_attribute const * const __bt_plugin_component_class_descriptor_attribute_dummy __BT_PLUGIN_COMPONENT_CLASS_DESCRIPTOR_ATTRIBUTES_ATTRS = NULL; \
-       extern struct __bt_plugin_component_class_descriptor_attribute const *__BT_PLUGIN_COMPONENT_CLASS_DESCRIPTOR_ATTRIBUTES_BEGIN_SYMBOL __BT_PLUGIN_COMPONENT_CLASS_DESCRIPTOR_ATTRIBUTES_BEGIN_EXTRA; \
-       extern struct __bt_plugin_component_class_descriptor_attribute const *__BT_PLUGIN_COMPONENT_CLASS_DESCRIPTOR_ATTRIBUTES_END_SYMBOL __BT_PLUGIN_COMPONENT_CLASS_DESCRIPTOR_ATTRIBUTES_END_EXTRA; \
+       _BT_HIDDEN extern struct __bt_plugin_component_class_descriptor_attribute const *__BT_PLUGIN_COMPONENT_CLASS_DESCRIPTOR_ATTRIBUTES_BEGIN_SYMBOL __BT_PLUGIN_COMPONENT_CLASS_DESCRIPTOR_ATTRIBUTES_BEGIN_EXTRA; \
+       _BT_HIDDEN extern struct __bt_plugin_component_class_descriptor_attribute const *__BT_PLUGIN_COMPONENT_CLASS_DESCRIPTOR_ATTRIBUTES_END_SYMBOL __BT_PLUGIN_COMPONENT_CLASS_DESCRIPTOR_ATTRIBUTES_END_EXTRA; \
        \
        _BT_EXPORT struct __bt_plugin_descriptor const * const *__bt_get_begin_section_plugin_descriptors(void) \
        { \