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.

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

Reply via email to