Bug 242171

Summary: dns/libidn2 is BROKEN on 12.0 amd64
Product: Ports & Packages Reporter: Antoine Brodin <antoine>
Component: Individual Port(s)Assignee: Sunpoet Po-Chuan Hsieh <sunpoet>
Status: Closed FIXED    
Severity: Affects Many People CC: decke, dereks, fw, ports, sascha
Priority: --- Flags: bugzilla: maintainer-feedback? (sunpoet)
antoine: exp-run+
Version: Latest   
Hardware: Any   
OS: Any   
See Also: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=242726
Attachments:
Description Flags
remove symver
none
Turn verioned puny into @@@ none

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 Sunpoet Po-Chuan Hsieh freebsd_committer 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 Sunpoet Po-Chuan Hsieh freebsd_committer 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 2019-12-12 12:52:05 UTC
sunpoet@ requests an exp-run
Comment 9 Antoine Brodin freebsd_committer 2019-12-14 07:00:17 UTC
Exp-run looks fine.
Comment 10 Bernhard Froehlich freebsd_committer 2019-12-22 12:56:41 UTC
Ping sunpoet@ - looks like exp-run was fine
Comment 11 commit-hook freebsd_committer 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 Sunpoet Po-Chuan Hsieh freebsd_committer 2019-12-22 15:07:36 UTC
Committed. Thanks!