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.
Bart
> For instance:
>
> $ cat lib.cc
> #include <cstdio>
>
> #include "lib.h"
>
> Foo::Foo (void)
> {
> printf ("%s\n", __func__);
> }
>
> Foo::~Foo (void)
> {
> printf ("%s\n", __func__);
> }
> $ cat lib.h
> class __attribute__ ((visibility ("hidden"))) Foo
> {
> public:
> Foo (void);
> ~Foo (void);
> };
> $ cat main.cc
> #include "lib.h"
>
> int main (int argc, char *argv[])
> {
> Foo foo;
> return 0;
> }
>
> $ g++ -std=gnu++17 -Wall -O2 -fpic -shared lib.cc -o libfoo.so
> $ g++ -std=gnu++17 -Wall -O2 main.cc -o main -L. -lfoo
> /usr/bin/ld: /tmp/ccntd0J3.o: in function `main':
> main.cc:(.text.startup+0x22): undefined reference to `Foo::Foo()'
> /usr/bin/ld: main.cc:(.text.startup+0x2a): undefined reference to
> `Foo::~Foo()'
> /usr/bin/ld: main: hidden symbol `_ZN3FooD1Ev' isn't defined
> /usr/bin/ld: final link failed: bad value
> collect2: error: ld returned 1 exit status
>
> clang throws the exact same error, you need to use the default visibility
> for the exported ABI.
>
> Now, the hidden visibility might help you to optimize the class usage for
> private methods.
>
> For instance:
>
> $ cat lib.h
> class __attribute__ ((visibility ("default"))) Foo
> {
> public:
> Foo (void);
> ~Foo (void);
>
> void privateMethod();
> };
> $ cat lib.cc
> #include <cstdio>
>
> #include "lib.h"
>
> Foo::Foo (void)
> {
> privateMethod ();
> printf ("%s\n", __func__);
> }
>
> Foo::~Foo (void)
> {
> printf ("%s\n", __func__);
> }
>
> void Foo::privateMethod()
> {
> printf ("%s\n", __func__);
> }
> $ g++ -std=gnu++17 -Wall -O2 -fpic -shared lib.cc -o libfoo.so
>
> This will always generate a PLT call for 'privateMethod' due ELF interposition
> rules:
>
> $ objdump -d libfoo.so
> [...]
> 0000000000001170 <_ZN3FooC1Ev>:
> 1170: f3 0f 1e fa endbr64
> 1174: 48 83 ec 08 sub $0x8,%rsp
> 1178: e8 f3 fe ff ff call 1070
> <_ZN3Foo13privateMethodEv@plt>
> 117d: 48 8d 3d 7d 0e 00 00 lea 0xe7d(%rip),%rdi # 2001
> <_fini+0xe71>
> 1184: 48 83 c4 08 add $0x8,%rsp
> 1188: e9 d3 fe ff ff jmp 1060 <puts@plt>
> [...]
> $ objdump -R libfoo.so
> [...]
> 0000000000004020 R_X86_64_JUMP_SLOT _ZN3Foo13privateMethodEv@@Base
> [...]
>
> The R_X86_64_JUMP_SLOT is the dynamic relocation resolved by dynamic loader.
> If
> you define that privateMethod has hidden visibility, compile know that
> external
> call are not supported and will optimize it away:
>
> $ objdump -d libfoo.so:
> [...]
> 0000000000001150 <_ZN3FooD1Ev>:
> 1150: f3 0f 1e fa endbr64
> 1154: 48 83 ec 08 sub $0x8,%rsp
> 1158: 48 8d 3d af 0e 00 00 lea 0xeaf(%rip),%rdi # 200e
> <_fini+0xe8e>
> 115f: e8 ec fe ff ff call 1050 <puts@plt>
> 1164: 48 83 c4 08 add $0x8,%rsp
> 1168: c3 ret
> 1169: 90 nop
> 116a: 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1)
> [...]
>
> You can see that it even allows the compiler to inline the privateMethod.
> Recent GCC and clang also provides another option,
> -fno-semantic-interposition,
> which might also improve PIC code generation [1].
>
> [1] http://maskray.me/blog/2021-05-09-fno-semantic-interposition
>
> >
> > On top of that - if I build the examples with clang, they build
> > because they don't reference the symbols openly. But if I then make
> > one of those exceptions fly out of libgpiod unconditionally, their
> > typeid is correctly seen:
> >
> > terminate called after throwing an instance of 'gpiod::request_released'
> > what(): GPIO lines have been released
> > Aborted
> >
> > If on the other hand I add catch (const gpiod::request_released& ex)
> > somewhere, I get the same linking error.
> >
> > The same happens for any program that I'd build with -llgpiodcxx
> >
> > Bart
> >
> >>>
> >>> If I make the symbols unconditionally visible here, clang starts to
> >>> work but I have no idea why and would like to avoid re-exporting the
> >>> symbols if I can.
> >>>
> >>> I'm using the following version:
> >>> Ubuntu clang version 15.0.6
> >>> Target: x86_64-pc-linux-gnu
> >>> Thread model: posix
> >>> InstalledDir: /usr/bin
> >>>
> >>> Host is: x86_64 GNU/Linux
> >>>
> >>> It's not like gcc links fine but then fails to obtain typeid - I can
> >>> catch exceptions coming out from libgpiod just fine in external apps
> >>> linked using gcc and see their type.
> >>>
> >>> Any hints?
> >>
> >>
_______________________________________________
linaro-toolchain mailing list -- linaro-toolchain@lists.linaro.org
To unsubscribe send an email to linaro-toolchain-le...@lists.linaro.org
