Bug 230214

Summary: multimedia/libx264: Fails to link with lld as /usr/bin/ld on i386
Product: Ports & Packages Reporter: Ed Maste <emaste>
Component: Individual Port(s)Assignee: Ed Maste <emaste>
Status: Closed FIXED    
Severity: Affects Some People CC: emaste, koobs, ndowens04
Priority: --- Keywords: easy
Version: LatestFlags: koobs: maintainer-feedback+
koobs: merge-quarterly?
Hardware: i386   
OS: Any   
URL: https://reviews.freebsd.org/D17201
See Also: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=214864
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=230208
Bug Depends on:    
Bug Blocks: 214864    
Attachments:
Description Flags
set LLD_UNSAFE on i386 and armv7 due to .text relocations koobs: maintainer-approval+

Description Ed Maste freebsd_committer freebsd_triage 2018-07-30 23:58:52 UTC
The migration to the LLVM project's lld linker as the system linker (/usr/bin/ld) for FreeBSD is in progress - it is the case for arm64 and amd64 today, and i386 will switch once ports issues are addressed - see exp-run in PR214864.

Linking multimedia/libx264 with lld fails with errors of the form:

/usr/bin/ld: error: can't create dynamic relocation R_386_PC32 against symbol: memcpy in readonly segment; recompile object files with -fPIC
>>> defined in /lib/libc.so.7
>>> referenced by mc.c
>>>               common/mc.o:(x264_plane_copy_c)

/usr/bin/ld: error: can't create dynamic relocation R_386_PC32 against symbol: memcpy in readonly segment; recompile object files with -fPIC
>>> defined in /lib/libc.so.7
>>> referenced by mc.c
>>>               common/mc.o:(x264_frame_init_lowres)

/usr/bin/ld: error: can't create dynamic relocation R_386_PC32 against symbol: x264_frame_expand_border_lowres in readonly segment; recompile object files with -fPIC
>>> defined in common/frame.o
>>> referenced by mc.c
>>>               common/mc.o:(x264_frame_init_lowres)

http://package18.nyi.freebsd.org/data/headi386PR214864-default/2018-07-30_18h09m59s/logs/errors/libx264-0.152.2854.log
Comment 1 Ed Maste freebsd_committer freebsd_triage 2018-08-01 12:35:02 UTC
I think this Makefile addition may work:

.if ${ARCH} == i386
# PR230215 Allow relocations against read-only segments (override lld default)
LDFLAGS+=-Wl,-z,notext
.endif
Comment 2 Kubilay Kocak freebsd_committer freebsd_triage 2018-08-06 01:05:53 UTC
(In reply to Ed Maste from comment #1)

Thanks for the report Ed

1) Presumably this (LDFLAGS) should be scoped not only to arch, but also to FreeBSD version's (where lld is the default?) and/or compiler/linker in use (only clang?)

2) Is this a short term fix or workaround until lld adds support / fixes for something in the future?

3) If this is an lld/i386 specific issue, what is the context behind the required  LDFLAGS not being a ports framework modification, rather than in specific ports? Is the fact that this is being addressed in the port, suggestive that there is a source/upstream specific issue ultimately here?
Comment 3 Kubilay Kocak freebsd_committer freebsd_triage 2018-08-06 01:13:02 UTC
Quote from https://lists.freebsd.org/pipermail/freebsd-ports/2017-December/111875.html

> lang/mono fails because lld defaults to -z text, disallowing
> relocations in read-only segments (like .text). A workaround is to add
> -z notext to the link command line, which turns off lld's error for
> this case and results in the same behaviour as ld.bfd and ld.gold
> provide by default.
Comment 4 Kubilay Kocak freebsd_committer freebsd_triage 2018-08-06 01:38:39 UTC
bug 230208 apparently had the same issue (error), but this was addressed with LLD_UNSAFE=yes
Comment 5 Ed Maste freebsd_committer freebsd_triage 2018-08-06 02:00:23 UTC
> 1) Presumably this (LDFLAGS) should be scoped not only to arch, but also to
> FreeBSD version's (where lld is the default?) and/or compiler/linker in use
> (only clang?)

It could be scoped to version, but it's not necessary: when GNU ld is used -znotext just reaffirms the default setting.

> 2) Is this a short term fix or workaround until lld adds support / fixes for
> something in the future?

It's a workaround until the port can be fixed (either to avoid relocs against read-only segments) or for the upstream build infrastructure to take care of -znotext, assuming upstream wants to retain the RO segment relocs.

> 3) If this is an lld/i386 specific issue

It's not directly a lld/i386 issue; it's a combination of two things:
- lld disallows RO segment relocations by default because they're generally an indication that the software being linked was built incorrectly
- i386 has a large performance overhead from position-independent code
Comment 6 Ed Maste freebsd_committer freebsd_triage 2018-08-06 02:01:49 UTC
> bug 230208 apparently had the same issue (error), but this was addressed with
> LLD_UNSAFE=yes

Yeah, IMO this is a usable workaround, just punt on the issue and continue linking with GNU ld.
Comment 7 Kubilay Kocak freebsd_committer freebsd_triage 2018-08-06 05:04:08 UTC
(In reply to Ed Maste from comment #6)

Thank you for clarifying Ed. I'll defer to your analysis/expertise on the issue.

Happy to approve any change you recommend (and attach here, or in a review), pending/contingent on QA passing (poudriere {10,11,12}-i386) to your satisfaction.
Comment 8 Ed Maste freebsd_committer freebsd_triage 2018-08-19 15:03:18 UTC
Created attachment 196348 [details]
set LLD_UNSAFE on i386 and armv7 due to .text relocations
Comment 9 Kubilay Kocak freebsd_committer freebsd_triage 2018-08-20 01:23:40 UTC
Comment on attachment 196348 [details]
set LLD_UNSAFE on i386 and armv7 due to .text relocations

Approved by: koobs (maintainer)
MFH:         2018Q3
Comment 10 commit-hook freebsd_committer freebsd_triage 2018-09-05 14:15:33 UTC
A commit references this bug:

Author: emaste
Date: Wed Sep  5 14:15:29 UTC 2018
New revision: 479024
URL: https://svnweb.freebsd.org/changeset/ports/479024

Log:
  multimedia/libx264: set LLD_UNSAFE for i386 and armv7

  libx264 has relocations against readonly segment(s) and lld exits with
  an error suggesting recompiling with -fPIC.  As this may introduce a
  performance impact, for now just fall back to GNU ld.bfd.

  PR:		230214
  Approved by:	koobs (maintainer)
  MFH:		2018Q3

Changes:
  head/multimedia/libx264/Makefile
Comment 11 commit-hook freebsd_committer freebsd_triage 2018-09-05 14:48:59 UTC
A commit references this bug:

Author: emaste
Date: Wed Sep  5 14:48:13 UTC 2018
New revision: 479025
URL: https://svnweb.freebsd.org/changeset/ports/479025

Log:
  Revert r479024 due to build breakage

  PR:             230214

Changes:
  head/multimedia/libx264/Makefile
Comment 12 Nathan 2018-09-10 00:34:30 UTC
Adding :
LDFLAGS= -Wl,-z,notext 
worked for me on 12-current i386
Comment 13 Kubilay Kocak freebsd_committer freebsd_triage 2018-09-18 03:02:58 UTC
Comment on attachment 196348 [details]
set LLD_UNSAFE on i386 and armv7 due to .text relocations

Obsoleted by https://reviews.freebsd.org/D17201 (alternate method/solution)
Comment 14 commit-hook freebsd_committer freebsd_triage 2018-09-18 14:01:33 UTC
A commit references this bug:

Author: emaste
Date: Tue Sep 18 14:00:45 UTC 2018
New revision: 480024
URL: https://svnweb.freebsd.org/changeset/ports/480024

Log:
  multimedia/libx264: add -znotext to LDFLAGS on i386, for lld

  Example error:
  /usr/bin/ld: error: can't create dynamic relocation R_386_PC32 against
      symbol: gettimeofday in readonly segment; recompile object files
      with -fPIC

  This port links some non-PIC code, which fails with lld as it defaults
  to disallowing relocations against read-only segments.  For i386 we can
  just add -znotext unconditionally: for GNU BFD ld it just affirms BFD's
  existing default.

  PR:		214864, 230214
  Reviewed by:	koobs
  Approved by:	koobs
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D17201

Changes:
  head/multimedia/libx264/Makefile