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.
_______________________________________________
linaro-toolchain mailing list -- linaro-toolchain@lists.linaro.org
To unsubscribe send an email to linaro-toolchain-le...@lists.linaro.org
