On Tue, 14 Feb 2023 at 11:52, Adhemerval Zanella Netto <adhemerval.zane...@linaro.org> wrote: > > > > On 13/02/23 19:04, Bartosz Golaszewski wrote: > > 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 > > On both clang and llvm the symbol is marked as local: > > $ objdump -t bindings/cxx/.libs/libgpiodcxx.so 2>/dev/null| grep > throw_from_errno > 0000000000010570 l F .text 0000000000000139 > _ZN5gpiod16throw_from_errnoERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE > c++filt > _ZN5gpiod16throw_from_errnoERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE > gpiod::throw_from_errno(std::__cxx11::basic_string<char, > std::char_traits<char>, std::allocator<char> > const&) > > Which won't be available to link. Are you seeing otherwise?
No, that's it. So if there's no __attribute__((visibility(x))) then a non-static symbol is *local*, but with __attribute__((visibility(x))), it's *hidden*? Anyway, thanks for the explanation, I will remove the guard now. Bart _______________________________________________ linaro-toolchain mailing list -- linaro-toolchain@lists.linaro.org To unsubscribe send an email to linaro-toolchain-le...@lists.linaro.org
