On Mon, 13 Feb 2023 at 21:41, Adhemerval Zanella Netto <adhemerval.zane...@linaro.org> wrote: > > > > On 13/02/23 17:25, Bartosz Golaszewski wrote: > > On Mon, 13 Feb 2023 at 21:17, Adhemerval Zanella Netto > > <adhemerval.zane...@linaro.org> wrote: > >> > >> > >> > >> On 13/02/23 17:05, Bartosz Golaszewski wrote: > >>> On Mon, 13 Feb 2023 at 20:53, Adhemerval Zanella Netto > >>> <adhemerval.zane...@linaro.org> wrote: > >>>> > >>>> > >>>> > >>>> On 13/02/23 16:22, Bartosz Golaszewski wrote: > >>>>> On Mon, 13 Feb 2023 at 19:49, Adhemerval Zanella Netto > >>>>> <adhemerval.zane...@linaro.org> wrote: > >>>>>> > >>>>>> > >>>>>> > >>>>>> On 13/02/23 12:49, Bartosz Golaszewski wrote: > >>>>>>> Hey! > >>>>>>> > >>>>>>> I'm the author and maintainer of libgpiod. I'm currently getting ready > >>>>>>> to do a new major release. After giving some exposure to the release > >>>>>>> candidate, I noticed that when using clang, I can't link against the > >>>>>>> C++ bindings, while it works just fine in GCC. > >>>>>>> > >>>>>>> The tree in question is here: > >>>>>>> https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/log/ > >>>>>>> > >>>>>>> You can trigger the linking program by trying to build the C++ tests > >>>>>>> with clang like that: > >>>>>>> > >>>>>>> CC=clang CXX=clang++ ./autogen.sh --enable-bindings-cxx --enable-tests > >>>>>>> && make -j16 > >>>>>>> > >>>>>>> You'll get the following error: > >>>>>>> > >>>>>>> /usr/bin/ld: tests-chip.o:(.data+0x0): undefined reference to > >>>>>>> `typeinfo for gpiod::chip_closed' > >>>>>>> /usr/bin/ld: tests-line-request.o:(.data+0x0): undefined reference to > >>>>>>> `typeinfo for gpiod::request_released' > >>>>>>> /usr/bin/ld: .libs/gpiod-cxx-test: hidden symbol > >>>>>>> `_ZTIN5gpiod11chip_closedE' isn't defined > >>>>>>> /usr/bin/ld: final link failed: bad value > >>>>>>> > >>>>>>> The typoinfo is missing for exception types that should be visible to > >>>>>>> users of the library. > >>>>>>> > >>>>>>> The culprit is here: > >>>>>>> https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/tree/bindings/cxx/gpiod.hpp#n26 > >>>>>>> > >>>>>>> I added the GPIOD_CXX_BUILD macro in order to not re-export the > >>>>>>> visible symbols if any user of the library would include the gpiod.hpp > >>>>>>> header. When the library is being built, the symbols are visible, when > >>>>>>> someone includes the header, the symbols are hidden. > >>>>>> > >>>>>> But the typeid of the symbol, for instance gpiod, will still be > >>>>>> provided > >>>>>> by shared library if I understood correctly: > >>>>>> > >>>>>> libgpiod-llvm$ objdump -t ./bindings/cxx/tests/tests-chip.o > >>>>>> 2>/dev/null| grep -w _ZTIN5gpiod11chip_closedE > >>>>>> 0000000000000000 *UND* 0000000000000000 .hidden > >>>>>> _ZTIN5gpiod11chip_closedE > >>>>>> libgpiod-llvm$ objdump -t bindings/cxx/.libs/libgpiodcxx.so > >>>>>> 2>/dev/null| grep _ZTIN5gpiod11chip_closedE > >>>>>> 0000000000024b50 g O .data.rel.ro 0000000000000018 > >>>>>> _ZTIN5gpiod11chip_closedE > >>>>>> > >>>>>> However, it seems that GCC is not applying the hidden attribute on the > >>>>>> typeid class: > >>>>>> > >>>>>> libgpiod-gcc$ objdump -t ./bindings/cxx/tests/tests-chip.o > >>>>>> 2>/dev/null| grep -w _ZTIN5gpiod11chip_closedE > >>>>>> 0000000000000000 w O > >>>>>> .data.rel.local.DW.ref._ZTIN5gpiod11chip_closedE > >>>>>> 0000000000000008 .hidden DW.ref._ZTIN5gpiod11chip_closedE > >>>>>> 0000000000000000 *UND* 0000000000000000 > >>>>>> _ZTIN5gpiod11chip_closedE > >>>>>> > >>>>>> When it creates create the vague linking weak symbol > >>>>>> .data.rel.local.DW.ref._ZTIN5gpiod11chip_closedE. > >>>>>> > >>>>>> I am not sure why GCC is being permissive here, in fact IMHO this is > >>>>>> gcc issue. If I add the visibility explicitly using pragmas: > >>>>>> > >>>>>> diff --git a/bindings/cxx/gpiodcxx/exception.hpp > >>>>>> b/bindings/cxx/gpiodcxx/exception.hpp > >>>>>> index 98b7bc4..24ae698 100644 > >>>>>> --- a/bindings/cxx/gpiodcxx/exception.hpp > >>>>>> +++ b/bindings/cxx/gpiodcxx/exception.hpp > >>>>>> @@ -17,6 +17,8 @@ > >>>>>> > >>>>>> namespace gpiod { > >>>>>> > >>>>>> +#pragma GCC visibility push(hidden) > >>>>>> + > >>>>>> /** > >>>>>> * @ingroup gpiod_cxx > >>>>>> * @{ > >>>>>> @@ -25,7 +27,7 @@ namespace gpiod { > >>>>>> /** > >>>>>> * @brief Exception thrown when an already closed chip is used. > >>>>>> */ > >>>>>> -class GPIOD_CXX_API chip_closed : public ::std::logic_error > >>>>>> +class /*GPIOD_CXX_API*/ chip_closed : public ::std::logic_error > >>>>>> { > >>>>>> public: > >>>>>> > >>>>>> @@ -64,6 +66,8 @@ public: > >>>>>> virtual ~chip_closed(); > >>>>>> }; > >>>>>> > >>>>>> +#pragma GCC visibility pop > >>>>>> + > >>>>>> /** > >>>>>> * @brief Exception thrown when an already released line request is > >>>>>> used. > >>>>>> */ > >>>>>> > >>>>>> I get an explicit linking error: > >>>>>> > >>>>>> /usr/bin/ld: > >>>>>> tests-chip.o:(.data.rel.local.DW.ref._ZTIN5gpiod11chip_closedE[DW.ref._ZTIN5gpiod11chip_closedE]+0x0): > >>>>>> undefined reference to `typeinfo for gpiod::chip_closed' > >>>>>> > >>>>>> Which is what I expect. So I suggest you to avoid adding the hidden > >>>>>> visibility on tests because since there are not linking static, they > >>>>>> should follow the default rules of ABI and hidden in this case does > >>>>>> not really make much sense. > >>>>>> > >>>>> > >>>>> I'm not sure I understand this. The tests are linked dynamically - > >>>>> just like any other program would. IIUC: I build libgpiodcxx making > >>>>> the exception symbols visible, then if anyone else (a program linking > >>>>> against libgpiodcxx) includes the header, the symbol is hidden. > >>>> > >>>> Hidden symbols linkage only works for TU within the same ELF object, > >>>> for instance within a shared library or a binary itself. They are used, > >>>> besides to trim the dynamic section, to also avoid symbol interposition > >>>> through PLT. > >>>> > >>>> So using hidden visibility for libgpiodcxx only makes sense if you > >>>> other TU reference the exported ABI (for instance if another class > >>>> from libgpiodcxx uses the chip_closed class). It does not make sense > >>>> for external ELF objects to use visibility hidden for public ABIs because > >>>> by ELF rules such symbols are not visible at static linking time. > >>>> > >>> > >>> Yes, I get that. Isn't this what I'm doing here though (and possibly > >>> failing)? When building libgpiodcxx, I pass -DGPIOD_CXX_BUILD to the > >>> compiler. > >>> > >>> This makes GPIOD_CXX_API become __attribute__((visibility("default"))) > >>> and the resulting shared object has: > >>> > >>> $ nm bindings/cxx/.libs/libgpiodcxx.so | grep chip_closed > >>> 000000000000f770 T _ZN5gpiod11chip_closedaSEOS0_ > >>> 000000000000f760 T _ZN5gpiod11chip_closedaSERKS0_ > >>> 000000000000f740 T _ZN5gpiod11chip_closedC1EOS0_ > >>> 000000000000f700 T > >>> _ZN5gpiod11chip_closedC1ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE > >>> 000000000000f720 T _ZN5gpiod11chip_closedC1ERKS0_ > >>> 000000000000f740 T _ZN5gpiod11chip_closedC2EOS0_ > >>> 000000000000f700 T > >>> _ZN5gpiod11chip_closedC2ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE > >>> 000000000000f720 T _ZN5gpiod11chip_closedC2ERKS0_ > >>> 000000000000f790 T _ZN5gpiod11chip_closedD0Ev > >>> 000000000000f780 T _ZN5gpiod11chip_closedD1Ev > >>> 000000000000f780 T _ZN5gpiod11chip_closedD2Ev > >>> 0000000000024b38 D _ZTIN5gpiod11chip_closedE > >>> 000000000001b5d0 R _ZTSN5gpiod11chip_closedE > >>> 0000000000024ac0 D _ZTVN5gpiod11chip_closedE > >>> > >>> Specifically: > >>> > >>> $ nm bindings/cxx/.libs/libgpiodcxx.so | grep _ZTIN5gpiod11chip_closedE > >>> 0000000000024b38 D _ZTIN5gpiod11chip_closedE > >>> > >>> Looks good for linking. Now if someone includes the exception.hpp > >>> header, GPIOD_CXX_API will become > >>> __attribute__((visibility("hidden"))) but that shouldn't matter now > >>> for the program that's being built because libgpiocxx has this symbol > >>> and IT IS visible? > >>> > >>> I admit, I'm not an expert on this but I really can't see what I'm > >>> missing here. > >> > >> That's I am trying to explain in the first email, where the test-chip.o > >> object > >> is binding with the typeinfo as hidden: > >> > >> $ objdump -t ./bindings/cxx/tests/tests-chip.o 2>/dev/null| grep -w > >> _ZTIN5gpiod11chip_closedE > >> 0000000000000000 *UND* 0000000000000000 *.hidden* > >> _ZTIN5gpiod11chip_closedE > >> > >> The libgpiodcxx.so is correct, it exports the symbol visibility as > >> expected. > >> But, you also need to setup the correct symbol visibility on ABI headers > >> definition when building the tests. C/C++ visibility is not inferred from > >> the input objects, but rather from the TU itself; meaning that if you > >> define > >> hidden visibility for the tests, it will try to bind against hidden symbol > >> and not the global ones from the library. > > > > OH now I get it! tests-chip.o is the TU here and the final link fails. I > > see. > > Yeap, I might not have being clear about that (sorry for the confusion). > > > > > So is my whole re-export guard just pointless? Don't libraries do that > > at all? And so if some other library includes this header in its > > header it will be visible in this library too? What is the best > > practice here? > > For ELF afaik there is no much you can do for exported symbols, the best > practices are only to export that you really need and use namespace to avoid > symbol names clashes and ODR violations. Another option is to use static > linking, so you can always hidden visibility everywhere (and it still makes > a difference for PIE binaries, since you need to build for PIC). > > But libraries do use the hidden visibility to optimize internal calls (it > does show a lot of improvement and plays better with LTO). For instance, > CPython 3.10 used the -fno-semantic-interposition [1] to get a really nice > improvement. > > [1] > https://developers.redhat.com/blog/2020/06/25/red-hat-enterprise-linux-8-2-brings-faster-python-3-8-run-speeds >
One thing I don't get is: what about this function: https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/tree/bindings/cxx/internal.cpp#n12 which clearly is visible outside its translation unit despite the default passed to the compiler being -fvisibility=hidden? Bart _______________________________________________ linaro-toolchain mailing list -- linaro-toolchain@lists.linaro.org To unsubscribe send an email to linaro-toolchain-le...@lists.linaro.org
