Bug 242171 - dns/libidn2 is BROKEN on 12.0 amd64
Summary: dns/libidn2 is BROKEN on 12.0 amd64
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: Po-Chuan Hsieh
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-11-23 08:58 UTC by Antoine Brodin
Modified: 2019-12-22 15:07 UTC (History)
5 users (show)

See Also:
bugzilla: maintainer-feedback? (sunpoet)
antoine: exp-run+


Attachments
remove symver (750 bytes, patch)
2019-11-26 01:30 UTC, Derek Schrock
no flags Details | Diff
Turn verioned puny into @@@ (910 bytes, patch)
2019-11-26 01:31 UTC, Derek Schrock
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Derek Schrock 2019-11-26 01:25:15 UTC
So it appears from 12.0 to 12.1 ld.bfd was removed.  I can't pin down why ld.bfd is used using these builds but you can see "LD=/usr/bin/ld.bfd" and -fuse-ld=bfd during configure/make env.  It seems the condition to use bfd is powerpc, if the port sets LLD_UNSAFE, or USES=libtool?

Testing with ld.lld or just using clang to link with libidn2.so it doesn't seem to care about the duplicate symbols.

So from my understanding you use .symver if you want to use a different symbol depending on the platform/version.  So in this case _idn2_punycode_encode is both the current/default and _idn2_punycode_encode@IDN2_0.0.0 version hence the duplicate symbols.  But they're also exporting the non-versioned/default _idn2_punycode_encode so I'm wondering why they would need to export the versioned symbol at all.  However, maybe due to the age of ld.bfd in 12.0 this is an issue.

If you remove the .symver lines from the souce (nosymver_* patches):

https://gitlab.com/libidn/libidn2/commit/089616574679a017f5d45c6bd40b05816ae4db6b#31c565512a6403ad3bdbc9d48c45f9ecf339ab3c_224_231
https://gitlab.com/libidn/libidn2/commit/089616574679a017f5d45c6bd40b05816ae4db6b#f2b12ce27c4e0fd2a0b9324788d8be3f0b6a0594_225_232

Or if you turn the "..@IDN2_0.0.0" into a "@@@" the .symver is ignored an not exported since one already exists.  (atatat_* patches)

You end up with just the default version'ed symbol, the @@ version.

Current state:

  $ readelf -Ws /usr/local/lib/libidn2.so | fgrep puny                                                                                                                                                   
      62: 000000000001d2a0   977 FUNC    GLOBAL DEFAULT   13 _idn2_punycode_encode@@IDN2_0.0.0 (2)
      63: 000000000001d2a0   977 FUNC    GLOBAL DEFAULT   13 _idn2_punycode_encode@IDN2_0.0.0 (2)
      76: 000000000001d680  1189 FUNC    GLOBAL DEFAULT   13 _idn2_punycode_decode@@IDN2_0.0.0 (2)
      77: 000000000001d680  1189 FUNC    GLOBAL DEFAULT   13 _idn2_punycode_decode@IDN2_0.0.0 (2)

Trying to link an object file with libidn2.so and ld.bfd you get the multiple definitions error.

With either patch:

  $ readelf -Ws /usr /local/lib/libidn2.so | fgrep puny
      62: 000000000001d2a0   977 FUNC    GLOBAL DEFAULT   13 _idn2_punycode_encode@@IDN2_0.0.0 (2)
      75: 000000000001d680  1189 FUNC    GLOBAL DEFAULT   13 _idn2_punycode_decode@@IDN2_0.0.0 (2)

Here you can link with ld.bfd without issue.

Also, testing with clang/ld.lld works without issue as well on 12.0.

I knew/know virtually zero about this level of C/asm so I think I still do so I might have something wrong here but the attached patches allow libidn2.so to export the compat symbols and work with FreeBSD 12.0's ld.bfd.  Is this really an upstream bug or just the age of base's binutils not playing nice with "modern" code or...somethingelseihaventaclueabout.
Comment 2 Derek Schrock 2019-11-26 01:30:27 UTC
Created attachment 209434 [details]
remove symver

Remove the .symver leaving only one symbol.
Comment 3 Derek Schrock 2019-11-26 01:31:25 UTC
Created attachment 209435 [details]
Turn verioned puny into @@@
Comment 4 Florian Weimer 2019-11-26 13:05:38 UTC
Which link editor do you use? There should only be a compat symbol.

Does the FreeBSD dynamic loader support GNU-style per-symbol versions? If not, why has the assembler support for the .symver directive? Thanks.
Comment 5 Po-Chuan Hsieh freebsd_committer freebsd_triage 2019-12-05 16:30:56 UTC
(In reply to Antoine Brodin from comment #0)

I've prepared a patch [1] by disabling the symver. I guess it should work (at least for dns/knot2). I need some time to test it on poudriere for all 5 broken ports.

[1] https://people.freebsd.org/~sunpoet/patch/dns-libidn2.txt

(In reply to Derek Schrock from comment #1)

Thanks for your help. :)
Comment 6 Derek Schrock 2019-12-06 01:19:58 UTC
(In reply to Sunpoet Po-Chuan Hsieh from comment #5)

Seems like the most straight forward solution.

You can ignore net/samba410 since they moved to not using LLD_UNSAFE.  So it's no longer using ld.bfd.

However, I get the feeling there's something wrong with the amd64 clang/lld compiler when it comes to .symver.  If you look at libidn2 on i386 you only have two compat symbols.  Where as comment #1 shows amd64 gets both the @ and @@.  I don't know enough about this level of FreeBSD's clang/lld to even guess what might explain it.   I do see a couple recent phabricator reviews that seems to be related to improper handling or adding support of .symvers.
Comment 7 Po-Chuan Hsieh freebsd_committer freebsd_triage 2019-12-11 23:55:53 UTC
All 5 ports build fine in a 12.0-RELEASE-p12 jail. Request exp-run.
Comment 8 Bernhard Froehlich freebsd_committer freebsd_triage 2019-12-12 12:52:05 UTC
sunpoet@ requests an exp-run
Comment 9 Antoine Brodin freebsd_committer freebsd_triage 2019-12-14 07:00:17 UTC
Exp-run looks fine.
Comment 10 Bernhard Froehlich freebsd_committer freebsd_triage 2019-12-22 12:56:41 UTC
Ping sunpoet@ - looks like exp-run was fine
Comment 11 commit-hook freebsd_committer freebsd_triage 2019-12-22 15:01:30 UTC
A commit references this bug:

Author: sunpoet
Date: Sun Dec 22 15:00:51 UTC 2019
New revision: 520636
URL: https://svnweb.freebsd.org/changeset/ports/520636

Log:
  Fix dependent ports on FreeBSD 12.0 amd64

  - Bump PORTREVISION for package change

  PR:		242171
  Reported by:	antoine
  Exp-run by:	antoine

Changes:
  head/dns/libidn2/Makefile
Comment 12 Po-Chuan Hsieh freebsd_committer freebsd_triage 2019-12-22 15:07:36 UTC
Committed. Thanks!