Bug 250336

Summary: lang/clover: link failure after update to 20.2
Product: Ports & Packages Reporter: John Hein <jcfyecrayz>
Component: Individual Port(s)Assignee: freebsd-x11 (Nobody) <x11>
Status: Closed Overcome By Events    
Severity: Affects Some People CC: crahman, imp, x11, zeising
Priority: --- Flags: bugzilla: maintainer-feedback? (x11)
Version: Latest   
Hardware: Any   
OS: Any   
Description Flags
[patch] avoid old ld(1) due to link problems on 11.x (at least) jcfyecrayz: maintainer-approval? (x11)

Description John Hein 2020-10-14 10:47:22 UTC
.ang/clover fails to link on 11.x with the old default ld(1) - after the recent update to graphics/mesa* to 20.2 (ports r552109).

root@j11-clean:/wrkdirs/usr/ports/lang/clover/work/mesa-20.2.0/_build # c++ -o src/gallium/targets/pipe-loader/pipe_radeonsi.so src/gallium/targets/pipe-loader/pipe_radeonsi.so.p/pipe_radeonsi.c.o -Wl,--as-needed -Wl,--no-undefined -Wl,-O1 -shared -fPIC -Wl,--start-group -Wl,-soname,pipe_radeonsi.so -O2 -pipe -fstack-protector-strong -fno-strict-aliasing -B/usr/local/bin -Wl,-rpath=/usr/local/llvm10/lib -fstack-protector-strong src/gallium/auxiliary/libgallium.a src/compiler/nir/libnir.a src/compiler/libcompiler.a src/util/libmesa_util.a src/util/format/libmesa_format.a src/gallium/auxiliary/libgalliumvl_stub.a src/gallium/drivers/radeonsi/libradeonsi.a src/gallium/winsys/radeon/drm/libradeonwinsys.a src/gallium/winsys/amdgpu/drm/libamdgpuwinsys.a src/amd/addrlib/libaddrlib.a src/amd/common/libamd_common.a src/amd/llvm/libamd_common_llvm.a src/gallium/auxiliary/libgalliumvl.a src/util/libxmlconfig.a -Wl,--gc-sections /usr/lib/libz.so -pthread -lm /usr/local/lib/libexpat.so /usr/local/lib/libdrm.so -L/usr/local/llvm10/lib -lLLVM-10 /usr/local/lib/libzstd.so /usr/local/lib/libunwind.so -L/usr/local/llvm10/lib -lLLVM-10 /usr/local/lib/libdrm_radeon.so -L/usr/local/llvm10/lib -lLLVM-10 /usr/local/lib/libdrm_amdgpu.so -lelf -L/usr/local/llvm10/lib -lLLVM-10 -lelf -Wl,--end-group
/usr/local/bin/ld: src/gallium/auxiliary/libgalliumvl_stub.a(vl_vl_stubs.c.o): in function `vl_create_mpeg12_decoder':
vl_stubs.c:(.text.vl_create_mpeg12_decoder+0x0): multiple definition of `vl_create_mpeg12_decoder'; src/gallium/auxiliary/libgalliumvl.a(vl_vl_mpeg12_decoder.c.o):vl_mpeg12_decoder.c:(.text.vl_create_mpeg12_decoder+0x0): first defined here
/usr/local/bin/ld: src/gallium/auxiliary/libgalliumvl_stub.a(vl_vl_stubs.c.o): in function `vl_mpg12_bs_decode':
vl_stubs.c:(.text.vl_mpg12_bs_decode+0x0): multiple definition of `vl_mpg12_bs_decode'; src/gallium/auxiliary/libgalliumvl.a(vl_vl_mpeg12_bitstream.c.o):vl_mpeg12_bitstream.c:(.text.vl_mpg12_bs_decode+0x0): first defined here
/usr/local/bin/ld: src/gallium/auxiliary/libgalliumvl_stub.a(vl_vl_stubs.c.o): in function `vl_mpg12_bs_init':
vl_stubs.c:(.text.vl_mpg12_bs_init+0x0): multiple definition of `vl_mpg12_bs_init'; src/gallium/auxiliary/libgalliumvl.a(vl_vl_mpeg12_bitstream.c.o):vl_mpeg12_bitstream.c:(.text.vl_mpg12_bs_init+0x0): first defined here
/usr/local/bin/ld: src/gallium/auxiliary/libgalliumvl_stub.a(vl_vl_stubs.c.o):(.rodata.vl_zscan_alternate+0x0): multiple definition of `vl_zscan_alternate'; src/gallium/auxiliary/libgalliumvl.a(vl_vl_zscan.c.o):(.rodata.vl_zscan_alternate+0x0): first defined here
/usr/local/bin/ld: src/gallium/auxiliary/libgalliumvl_stub.a(vl_vl_stubs.c.o):(.rodata.vl_zscan_normal+0x0): multiple definition of `vl_zscan_normal'; src/gallium/auxiliary/libgalliumvl.a(vl_vl_zscan.c.o):(.rodata.vl_zscan_normal+0x0): first defined here
/usr/local/bin/ld: src/gallium/auxiliary/libgalliumvl.a(vl_vl_mpeg12_decoder.c.o): in function `vl_create_mpeg12_decoder':
vl_mpeg12_decoder.c:(.text.vl_create_mpeg12_decoder+0x4ba): undefined reference to `vl_video_buffer_create_ex'
/usr/local/bin/ld: vl_mpeg12_decoder.c:(.text.vl_create_mpeg12_decoder+0x548): undefined reference to `vl_video_buffer_create_ex'
/usr/local/bin/ld: vl_mpeg12_decoder.c:(.text.vl_create_mpeg12_decoder+0x660): undefined reference to `vl_video_buffer_create_ex'
/usr/local/bin/ld: src/gallium/auxiliary/libgalliumvl.a(vl_vl_mpeg12_decoder.c.o): in function `vl_mpeg12_end_frame':
vl_mpeg12_decoder.c:(.text.vl_mpeg12_end_frame+0x35e): undefined reference to `vl_video_buffer_plane_order'
c++: error: linker command failed with exit code 1 (use -v to see invocation)
Comment 1 John Hein 2020-10-14 11:00:22 UTC
Created attachment 218735 [details]
[patch] avoid old ld(1) due to link problems on 11.x (at least)

Use -fuse-ld=lld fixes this on 11.x

Note that this port already depends on (and uses) the ld from devel/binutils.  With the change suggested here, it may be that the dependency on devel/binutils is no longer necessary (including for other mesa ports).  I have not tried using -fuse-ld=lld instead of USES=binutils in the larger graphics/mesa* collection.  The patch here is just for lang/clover.

While here, address a couple portlint ordering warnings.

 - poudriere testport (ok)
 - portline (removes some warnings & no new warnings or errors)
Comment 2 Niclas Zeising freebsd_committer 2020-10-14 12:20:07 UTC
binutils is only used if ld is not ldd, which maybe is the case on FreeBSD 11 (I have to check).  Addfing -fuse-ld=lld might make sense where ld is not ldd, but I don't think we should add it unconditionally.
Comment 3 John Hein 2020-10-14 15:34:27 UTC
(In reply to Niclas Zeising from comment #2)
Agreed.  I was thinking of testing a replacement for the conditional USE_BINUTILS=yes in graphics/mesa-dri/Makefile.common - that is, keep the lld condition, but use -fuse-ld=lld instead of ports binutils.  If that works for all mesa based ports, then it saves a dependency.  That's not as important as fixing the build for 11, so patching just lang/clover for now is fine (and saving getting rid of binutils for another day - mainly after it can be tested).
Comment 4 John Hein 2020-10-14 16:53:06 UTC
(In reply to John Hein from comment #3)
Let me amend that last comment.  I don't think it hurts to add -fuse-ld=lld even if the default linker is lld already.  It may be better than adding conditional logic.

In other words, I think (a) might be better than (b) for simplicity.  -fuse-ld is certainly less expensive than a dependency on devel/binutils, so the largest(?) advantage of using the conditional logic is diminished.


LDFLAGS+= -fuse-ld=lld


.if ${/usr/bin/ld:L:tA} != /usr/bin/ld.lld
LDFLAGS+= -fuse-ld=lld

But (b) might be easier to grep for when 11.x goes away.  An OSVERSION test would have that advantage as well.

Either of the above can benefit from a comment and maybe even a USES=lld which can be pruned after 11.x goes away:

# The base ld(1) in 11.x has troubles for a number of ports
# where --as-needed and --start-group/--end-group is used
# with a mix of shared and static libraries.  Getting the
# order of libraries on the command line can sometimes
# work around ld(1)'s troubles.  But using the lld linker
# handles this issue more simply.
# After 11.x is no longer supported, this can be removed
# as the default linker on 12.x+ works fine in these cases.

A handful of other ports are having troubles with 11.x ld(1) after the recent update of various gnome related ports to use meson.  These have been failing for a couple weeks now in the freebsd package builder.

These bugs suggest the -fuse-ld=lld fix for some of them:

bug 249418
bug 249974
bug 249990
Comment 5 John Hein 2020-10-15 20:48:06 UTC
I ran a run in poudriere (11-stable/amd64) with the following patch on every port that included mesa-dri/Makefile.common, and they all built fine.

--- graphics/mesa-dri/Makefile.common   (revision 552118)
+++ graphics/mesa-dri/Makefile.common   (working copy)
@@ -61,8 +61,10 @@
 .if ${/usr/bin/ld:L:tA} != /usr/bin/ld.lld
 # --build-id isn't supported by old GNU ld.bfd in base
 # Also ld.bfd have problems that seems related to --as-needed
-USE_BINUTILS=          yes
-LDFLAGS+=              -B${LOCALBASE}/bin
+# Even the ld from the latest binutils port had some troubles (lang/clover - bug 250336).
+#USE_BINUTILS=         yes
+#LDFLAGS+=             -B${LOCALBASE}/bin
+LDFLAGS+=              -fuse-ld=lld

 # only have one port to check with portscout.

Using that instead of a patch only on lang/clover is probably better (reduces dependencies on devel/binutils).

Whether the committer keeps the ".if ${/usr/bin/ld:L:tA} != /usr/bin/ld.lld" (see comment 2, comment 3, comment 4) I leave to your discretion.  I think I slightly prefer the non-conditional simple case.  A simple comment in the Makefile - '# remove after 11.x support ends' - is probably worthwhile (along with a better comment in the commit message).

I don't think any of these possible changes require any PORTREVISION bumps.
Comment 6 Jan Beich freebsd_committer 2020-10-15 21:59:10 UTC
powerpc64 doesn't have LLD on FreeBSD < 13 [*], so you may want to prepend exists(/usr/bin/ld.lld) like mesa-devel did. lang/clover can probably work on powerpc64 (and aarch64) given amdgpu.ko support in drm-current-kmod.

[*] https://github.com/freebsd/freebsd/blob/stable/12/share/mk/src.opts.mk#L316
Comment 7 Niclas Zeising freebsd_committer 2020-10-16 08:21:48 UTC
I don't think we need to care about powerpc64 on other architectures than 13, but I'll ask the people working on the powerPC port.

I'd prefer to keep the workaround only where ld is not lld, so keeping the check in.  Adding a note about this being mainly for FreeBSD 11.4 is easily doable.

Since clover is restricted to amd64 and i386 for now, and since there might be problems if we add this to Makefile.common, I'd prefer if re restrict the change to clover for now, until I've had a chance to talk to manu about it.

Commenting out the old way we did things isn't needed, we can walk through the version history to see that.
Comment 8 John Hein 2020-11-05 13:30:55 UTC
(In reply to Niclas Zeising from comment #7)
I did not expect that we commit the commented out version.  Sorry for the extra cruft - it was just a quick proof-of-concept change (rather than a reflection of any final commit) to do the test in poudriere and report back here.

I have been run testing changes with the change to mesa-dri/Makefile.common vs. just lang/clover.  Seems fine so far.
Comment 9 crahman 2020-12-24 02:04:13 UTC
It would be great to get this fixed!  Thanks.
Comment 10 Warner Losh freebsd_committer 2021-06-28 17:51:56 UTC
FreeBSD 11 graphics support is a bit outdated right now.
FreeBSD 12 and newer is our focus since 11 support ends soon.