Am 18.06.22 um 17:59 schrieb Will Godfrey:
Also does anyone know how to deal with the LV2 exposed symbols issue? Only Arch
Linux seem to test for this, but I'd like to keep them happy - they've been
very helpful in the past, and are usually the first to pick up new releases.

Hi Will,

as has been pointed out already, the recommended way to deal with this issue
today seems to generally link with visibility=hidden and then only to declare
explicitly some externally exposed functions to be exported for dynamic linking.

Now I've looked into that a bit more and found out some further info,
however I'm not really sure what's the actual problem reported by Arch Linux.


(1) generally speaking, it is highly recommended to hide all spurious exports
    also for the Main executable; this can reduce the size of the executable
    and allows modern compilers to do much more aggressive global optimisations
    and inlinings (because they can be sure the optimised functions can not
    be called from outside)

    See: https://gcc.gnu.org/wiki/Visibility


(2) However: we *did* already activate that link mode, albeit only for
    the LV2 plugin object allone. And we set it by directly spelling out
    the flag for GCC, instead of relying on CMake
    (the latter would be more portable).

    For context, this change was introduced by Kristian in 2021-1
    Commit: 422411854164c40c2


(3) As Kristian at that time also had found out, this works "out of the box",
    since the LV2-headres already define the necessary visibility attribute
    on the interface functions. These are:

    LV2_SYMBOL_EXPORT
    const LV2_Descriptor * lv2_descriptor(uint32_t index);

    LV2_SYMBOL_EXPORT
    const LV2UI_Descriptor* lv2ui_descriptor(uint32_t index);


    Where LV2_SYMBOL_EXPORT is defined as __attribute__((visibility("default")))


(4) Now I have tried the following: instead of only activating this for the
    Plugin object alone, we could rather activate it globally via a CMake
    policy. I have attached a patch for that change.

    As usual, you can apply this patch with

    git am 3d9b60-Avoid-exporting-dynamic-symbols.patch



(5) However, I am not sure this actually addresses the main problem indicated
    by the Arch Linux folks. After rebuilding with this change, indeed the
    exported symbols on the LV2-Plugin object are somewhat less (and drastically
    less on the main "yoshimi" executable). But there still remain some further
    dynamic symbols aside of the two interface functions

    These further symbols are almost all marked "weak" on my system, but these
    are essentially the same kind of stuff, which showed up in the error message
    forwarded by the Arch people: These are some helper functions generated from
    function templates of the C++ standard library for those custom types, which
    we now pass to pthreads and use with the C++ futures. It might well be that
    these functions must be indeed exported (and thus were marked specifically
    by the standard library) in order to allow for throwing some standard
    exceptions from those facilities and for similar "framework" stuff.
    But I am far from understanding this topic enough in-depth to come
    to a conclusion.

Thus, we could try the proposed change (because it is generally desirable always
to link with visibility=hidden), and then ask the Arch Linux folks, if this
changed the FATAL alarm on LV2 lint. On the other hand, changing this kind of
intricate stuff could always break some backwards compatibility in corner cases
and on some platforms, and so there remaines some risk.

-- Hermann
From 367ca89b6e4c5dd98fc8dd3d72e4511cd2f182b6 Mon Sep 17 00:00:00 2001
From: Ichthyostega <p...@ichthyostega.de>
Date: Wed, 22 Jun 2022 01:26:40 +0200
Subject: [PATCH] Avoid exporting dynamic symbols

Linking with visibility=hidden is highly recommended,
and can help with global optimisation.
See: https://gcc.gnu.org/wiki/Visibility


With this change, we enable this via a generic CMake setting,
instead of directly using an GCC flag -- thus it should be more portable.
And it is now also applied to the main executable itself (to improve optimisation)

Previously we did set this flag directly, see commit...
422411854164c40c2


Note: to load a LV2 plugin, we need two descriptor functions

LV2_SYMBOL_EXPORT
const LV2_Descriptor * lv2_descriptor(uint32_t index);

LV2_SYMBOL_EXPORT
const LV2UI_Descriptor* lv2ui_descriptor(uint32_t index);


These are already marked for Export in the LV2 headers defining them (lv2.h and ui.h).

#ifdef _WIN32
#    define LV2_SYMBOL_EXPORT LV2_SYMBOL_EXTERN __declspec(dllexport)
#else
#    define LV2_SYMBOL_EXPORT LV2_SYMBOL_EXTERN __attribute__((visibility("default")))
#endif



Beyond those two, we do not need to export any further entry points.
---
 src/CMakeLists.txt            | 7 +++++++
 src/LV2_Plugin/CMakeLists.txt | 1 -
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index ae09cb70..4ebce7f7 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -35,6 +35,13 @@ add_definitions(-std=gnu++14) # various versions of cmake
 
 add_definitions(-Wall)
 
+# avoid exporting any (spurious) symbols in EXE and Plugin
+# Note: the two LV2 descriptors are already marked visibility=default in LV2 headers
+cmake_policy (SET CMP0063 NEW)
+set(CMAKE_CXX_VISIBILITY_PRESET hidden)
+set(CMAKE_VISIBILITY_INLINES_HIDDEN 1)
+
+
 # vvv
 #add_definitions(-Wpedantic) # full ISO spec
 #add_definitions(-Werror) # warnings as errors
diff --git a/src/LV2_Plugin/CMakeLists.txt b/src/LV2_Plugin/CMakeLists.txt
index 51c1ce5f..3b069f5b 100644
--- a/src/LV2_Plugin/CMakeLists.txt
+++ b/src/LV2_Plugin/CMakeLists.txt
@@ -115,7 +115,6 @@ set (FltkUI_sources
 )
 
 add_definitions (-DYOSHIMI_LV2_PLUGIN=1)
-add_definitions (-fvisibility=hidden)
 
 add_library (yoshimi_lv2 MODULE
             ${yoshimi_lv2_plugin_files}
-- 
2.20.1

_______________________________________________
Yoshimi-devel mailing list
Yoshimi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/yoshimi-devel

Reply via email to