Bug 258463 - x11/libwacom: Fix build with clang/lld 13
Summary: x11/libwacom: Fix build with clang/lld 13
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-x11 (Nobody)
URL:
Keywords:
Depends on:
Blocks: 258209
  Show dependency treegraph
 
Reported: 2021-09-12 19:46 UTC by Dimitry Andric
Modified: 2021-10-02 11:10 UTC (History)
3 users (show)

See Also:
bugzilla: maintainer-feedback? (x11)


Attachments
work around lld 13 no longer supporting symver tricks (1.05 KB, patch)
2021-09-12 19:47 UTC, Dimitry Andric
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dimitry Andric freebsd_committer freebsd_triage 2021-09-12 19:46:06 UTC
Though x11/libwacom was not yet built during the exp-run for clang/llvm 13 (see bug 258209), due to some other dependencies not being available yet, I noticed that it failed to build with clang 13, or more specifically this is due to a behavior change in lld 13:

  ...
  [ 33% 10/30] cc  -o generate-hwdb generate-hwdb.p/tools_generate-hwdb.c.o -Wl,--as-needed -Wl,--no-undefined -fstack-protector-strong -O2 -pipe -g -fstack-protector-strong -fno-strict-aliasing '-Wl,-rpath,$ORIGIN/' -Wl,-rpath-link,/wrkdirs/share/dim/ports/x11/libwacom/work/libwacom-1.5/_build/ -Wl,--start-group libwacom.so.2.6.1 /usr/local/lib/libglib-2.0.so /usr/local/lib/libintl.so -Wl,--end-group
  [ 36% 11/30] /usr/local/bin/meson --internal exe --capture 65-libwacom.hwdb -- /wrkdirs/share/dim/ports/x11/libwacom/work/libwacom-1.5/_build/generate-hwdb
  FAILED: 65-libwacom.hwdb
  /usr/local/bin/meson --internal exe --capture 65-libwacom.hwdb -- /wrkdirs/share/dim/ports/x11/libwacom/work/libwacom-1.5/_build/generate-hwdb
  --- stderr ---

Unfortunately the meson build process doesn't really show you why it failed, but it turns out that running the 'generate-hwdb' command segfaults:

  Starting program: /wrkdirs/share/dim/ports/x11/libwacom/work/libwacom-1.5/_build/generate-hwdb

  Program received signal SIGSEGV, Segmentation fault.
  libwacom_add_match (device=device@entry=0x801031320, newmatch=0x0) at ../libwacom/libwacom.c:943
  943           device->matches[device->nmatches - 1] = libwacom_match_ref(newmatch);
  (gdb) bt
  #0  libwacom_add_match (device=device@entry=0x801031320, newmatch=0x0) at ../libwacom/libwacom.c:943
  #1  0x000000080024fc7d in libwacom_matchstr_to_match (device=device@entry=0x801031320, matchstr=<optimized out>) at ../libwacom/libwacom-database.c:207
  #2  0x000000080024e313 in libwacom_parse_tablet_keyfile (db=0x8010365a0, datadir=0x200b70 "/wrkdirs/share/dim/ports/x11/libwacom/work/libwacom-1.5/data", filename=<optimized out>) at ../libwacom/libwacom-database.c:652
  #3  load_tablet_files (db=0x8010365a0, datadir=0x200b70 "/wrkdirs/share/dim/ports/x11/libwacom/work/libwacom-1.5/data") at ../libwacom/libwacom-database.c:865
  #4  libwacom_database_new_for_path (datadir=0x200b70 "/wrkdirs/share/dim/ports/x11/libwacom/work/libwacom-1.5/data") at ../libwacom/libwacom-database.c:959
  #5  0x00000000002021b6 in main (argc=<optimized out>, argv=0x801036630) at ../tools/generate-hwdb.c:131

What happens is that an internal function 'libwacom_match_new' is supposed to be called, which returns a new 'WacomMatch' object. But instead, it calls a empty stub which returns NULL, resulting in this segfault. The empty stub was added as a rather nasty upstream hack to "Alias the accidentally exposed ABI into different functions", in https://github.com/linuxwacom/libwacom/commit/b9961dbe912fa096230460b194eebdc8a590d256:

> A special "trick" is used here to hide the ABI from new versions:
> Usually when defining multiple versioned symbols, one would define one as the
> default one with @@
>     .symver _foo1,foo@VERSION1
>     .symver _foo2,foo@@version2 <-- default one

> By leaving out the default one, ld doesn't know which one to link to and
> fails with an unresolved symbol. rtld however can still figure it out, so
> anything compiled will continue to work. This way we can make a symbol
> disappear from the library for new builds but have old builds continue to
> work with the new version.

Unfortunately this trick/hack does not work anymore with lld 13, since https://github.com/llvm/llvm-project/commit/66d44304921, ("[ELF] Combine foo@v1 and foo with the same versionId if both are defined "). The idea behind the hack is to have the linker call the 'real' libwacom_match_new function whenever it is called from inside the library itself, but any external callers get the stubbed version which doesn't really do anything.

I think libwacom should have used a different approach here, but just renaming those accidentally exposed internal functions to something different. Then the tricks with .symver are completely unnecessary. Here I added a patch that is as simple as possible, which adds #defines for two affected functions in libwacomint.h, renaming then from 'libwacom_xxx' to 'libwacom_internal_xxx'. This does not affect the corresponding exposed functions in the libwacom.so, and makes the 'generate-hwdb' command work OK again. I also ran the complete libwacom test suite, including the deprecated functions test, and it works fine.
Comment 1 Dimitry Andric freebsd_committer freebsd_triage 2021-09-12 19:47:34 UTC
Created attachment 227861 [details]
work around lld 13 no longer supporting symver tricks
Comment 2 Niclas Zeising freebsd_committer freebsd_triage 2021-09-15 08:32:54 UTC
Can you submit this upstream as well, or at least had a discussion with them about the best solution for this?

I am OK with this patch for now, but would prefer that we get a solution from upstream eventually.
Comment 3 Dimitry Andric freebsd_committer freebsd_triage 2021-09-15 09:13:26 UTC
(In reply to Niclas Zeising from comment #2)
Upstream already has a pull request to completely get rid of the deprecated symbols: https://github.com/linuxwacom/libwacom/pull/290

However, this will break their ABI and bump the .so version, so clients will have to be recompiled.

I guess it is nicer for the time being to not break the ABI and get rid of the symbol trickery. I will also put in a pull request.
Comment 4 Dimitry Andric freebsd_committer freebsd_triage 2021-09-15 11:11:03 UTC
Submitted https://github.com/linuxwacom/libwacom/pull/426
Comment 5 Ghost 2021-09-30 16:49:00 UTC
Thanks, using this here on a FreeBSD 14/amd64 desktop with devel/llvm13 as system toolchain.
Comment 6 commit-hook freebsd_committer freebsd_triage 2021-10-02 11:09:42 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=a1edd535d35d03e2d54a49bffac39035b990a716

commit a1edd535d35d03e2d54a49bffac39035b990a716
Author:     Dimitry Andric <dim@FreeBSD.org>
AuthorDate: 2021-09-12 19:44:36 +0000
Commit:     Dimitry Andric <dim@FreeBSD.org>
CommitDate: 2021-10-02 10:58:25 +0000

    x11/libwacom: work around lld 13 no longer supporting symver tricks

    Though x11/libwacom was not yet built during the exp-run for clang/llvm
    13 (see bug 258209), due to some other dependencies not being available
    yet, I noticed that it failed to build with clang 13, or more
    specifically this is due to a behavior change in lld 13:

      ...
      [ 33% 10/30] cc  -o generate-hwdb generate-hwdb.p/tools_generate-hwdb.c.o -Wl,--as-needed -Wl,--no-undefined -fstack-protector-strong -O2 -pipe -g -fstack-protector-strong -fno-strict-aliasing '-Wl,-rpath,$ORIGIN/' -Wl,-rpath-link,/wrkdirs/share/dim/ports/x11/libwacom/work/libwacom-1.5/_build/ -Wl,--start-group libwacom.so.2.6.1 /usr/local/lib/libglib-2.0.so /usr/local/lib/libintl.so -Wl,--end-group
      [ 36% 11/30] /usr/local/bin/meson --internal exe --capture 65-libwacom.hwdb -- /wrkdirs/share/dim/ports/x11/libwacom/work/libwacom-1.5/_build/generate-hwdb
      FAILED: 65-libwacom.hwdb
      /usr/local/bin/meson --internal exe --capture 65-libwacom.hwdb -- /wrkdirs/share/dim/ports/x11/libwacom/work/libwacom-1.5/_build/generate-hwdb
      --- stderr ---

    Unfortunately the meson build process doesn't really show you why it
    failed, but it turns out that running the 'generate-hwdb' command
    segfaults:

      Starting program: /wrkdirs/share/dim/ports/x11/libwacom/work/libwacom-1.5/_build/generate-hwdb

      Program received signal SIGSEGV, Segmentation fault.
      libwacom_add_match (device=device@entry=0x801031320, newmatch=0x0) at ../libwacom/libwacom.c:943
      943           device->matches[device->nmatches - 1] = libwacom_match_ref(newmatch);
      (gdb) bt
      #0  libwacom_add_match (device=device@entry=0x801031320, newmatch=0x0) at ../libwacom/libwacom.c:943
      #1  0x000000080024fc7d in libwacom_matchstr_to_match (device=device@entry=0x801031320, matchstr=<optimized out>) at ../libwacom/libwacom-database.c:207
      #2  0x000000080024e313 in libwacom_parse_tablet_keyfile (db=0x8010365a0, datadir=0x200b70 "/wrkdirs/share/dim/ports/x11/libwacom/work/libwacom-1.5/data", filename=<optimized out>) at ../libwacom/libwacom-database.c:652
      #3  load_tablet_files (db=0x8010365a0, datadir=0x200b70 "/wrkdirs/share/dim/ports/x11/libwacom/work/libwacom-1.5/data") at ../libwacom/libwacom-database.c:865
      #4  libwacom_database_new_for_path (datadir=0x200b70 "/wrkdirs/share/dim/ports/x11/libwacom/work/libwacom-1.5/data") at ../libwacom/libwacom-database.c:959
      #5  0x00000000002021b6 in main (argc=<optimized out>, argv=0x801036630) at ../tools/generate-hwdb.c:131

    What happens is that an internal function 'libwacom_match_new' is
    supposed to be called, which returns a new 'WacomMatch' object. But
    instead, it calls a empty stub which returns NULL, resulting in this
    segfault. The empty stub was added as a rather nasty upstream hack to
    "Alias the accidentally exposed ABI into different functions", in
    https://github.com/linuxwacom/libwacom/commit/b9961dbe912fa096230460b194eebdc8a590d256:

    > A special "trick" is used here to hide the ABI from new versions:
    > Usually when defining multiple versioned symbols, one would define one as the
    > default one with @@
    >     .symver _foo1,foo@VERSION1
    >     .symver _foo2,foo@@version2 <-- default one

    > By leaving out the default one, ld doesn't know which one to link to and
    > fails with an unresolved symbol. rtld however can still figure it out, so
    > anything compiled will continue to work. This way we can make a symbol
    > disappear from the library for new builds but have old builds continue to
    > work with the new version.

    Unfortunately this trick/hack does not work anymore with lld 13, since
    https://github.com/llvm/llvm-project/commit/66d44304921, ("[ELF] Combine
    foo@v1 and foo with the same versionId if both are defined "). The idea
    behind the hack is to have the linker call the 'real' libwacom_match_new
    function whenever it is called from inside the library itself, but any
    external callers get the stubbed version which doesn't really do
    anything.

    I think libwacom should have used a different approach here, but just
    renaming those accidentally exposed internal functions to something
    different. Then the tricks with .symver are completely unnecessary. Here
    I added a patch that is as simple as possible, which adds #defines for
    two affected functions in libwacomint.h, renaming then from
    'libwacom_xxx' to 'libwacom_internal_xxx'. This does not affect the
    corresponding exposed functions in the libwacom.so, and makes the
    'generate-hwdb' command work OK again. I also ran the complete libwacom
    test suite, including the deprecated functions test, and it works fine.

    PR:             258463
    Approved by:    zeising (maintainer)
    MFH:            2021Q4

 x11/libwacom/files/patch-libwacom_libwacomint.h (new) | 12 ++++++++++++
 1 file changed, 12 insertions(+)