Bug 239743
| Summary: | Mk/bsd.port.mk: don't switch to ld.bfd on powerpc64 with clang | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Ports & Packages | Reporter: | Piotr Kubaj <pkubaj> | ||||||
| Component: | Ports Framework | Assignee: | Port Management Team <portmgr> | ||||||
| Status: | Closed FIXED | ||||||||
| Severity: | Affects Many People | CC: | jbeich, linimon, ports-bugs | ||||||
| Priority: | --- | ||||||||
| Version: | Latest | ||||||||
| Hardware: | Any | ||||||||
| OS: | Any | ||||||||
| Attachments: |
|
||||||||
(In reply to Piotr Kubaj from comment #0) The LDFLAGS line doesn't need to be changed. Also the mis-indentation make me think the patch is wrong. And finally, I don't understand why powerpc64 needs a special treatment here. You probably want something like this?
%%%
LDFLAGS+= -fuse-ld=bfd
BINARY_ALIAS+= ld=${LD}
. if !defined(USE_BINUTILS)
-. if exists(/usr/bin/ld.bfd)
+. if exists(/usr/bin/ld.bfd) && (${ARCH} != powerpc64 || ${OSVERSION} < 1300038)
LD= /usr/bin/ld.bfd
CONFIGURE_ENV+= LD=${LD}
MAKE_ENV+= LD=${LD}
%%%
(In reply to Antoine Brodin from comment #3) I'll test this patch later and confirm whether it's ok. Regarding why powerpc64 needs special treatment: when it switches to clang, ld.bfd will still be installed. The reason is that lld currently doesn't support linking powerpc (32-bit) binaries. We will use lld for 64-bit binaries, however. So even after the switch /usr/bin/ld.bfd will still be installed (until lld is able to link powerpc binaries). (In reply to Piotr Kubaj from comment #4) This is still not enough (my patch works). misc/e2fsprogs-libuuid fails to configure: 1187 configure:3325: cc -O2 -pipe -fstack-protector-strong -fno-strict-aliasing -std=gnu99 -I/tmp/usr/ports/misc/e2fsprogs-libuuid/work/e2fsprogs-1.45.3/lib -I/usr/local/include -fstack-protector-strong -fuse-ld=bfd conftest.c >&5 1188 /usr/bin/ld.bfd: Dwarf Error: found dwarf version '4', this reader only handles version 2 information. 1189 /usr/lib/crt1.o: In function `_start': 1190 crt1.c:(.text+0x2): undefined reference to `.TOC.' The reason is that ld.bfd is used, which doesn't even know about elfv2. We need to either use lld (which works for this port on CURRENT but not on 12.0-RELEASE/amd64) or ld.bfd from ports. Since -fuse-ld allows for passing the whole patch to the linker, this is what we must do. Relevant base review: https://reviews.freebsd.org/D20261 (In reply to Antoine Brodin from comment #3) This patch is ok and much simpler: Index: Mk/bsd.port.mk =================================================================== --- Mk/bsd.port.mk (revision 508433) +++ Mk/bsd.port.mk (working copy) @@ -1820,10 +1820,10 @@ .endif .if defined(LLD_UNSAFE) && ${/usr/bin/ld:L:tA} == /usr/bin/ld.lld -LDFLAGS+= -fuse-ld=bfd +LDFLAGS+= -fuse-ld=${LD} BINARY_ALIAS+= ld=${LD} . if !defined(USE_BINUTILS) -. if exists(/usr/bin/ld.bfd) +. if exists(/usr/bin/ld.bfd) && (${ARCH} != powerpc64 || ${OSVERSION} < 1300038) LD= /usr/bin/ld.bfd CONFIGURE_ENV+= LD=${LD} MAKE_ENV+= LD=${LD} (In reply to Piotr Kubaj from comment #7) gcc doesn't understand this syntax (In reply to Antoine Brodin from comment #8) This is not about GCC, powerpc64 switches to Clang (both 32-bit and 64-bit). The problem is with LLD which can't link ppc32 binaries, that's why GNU ld needs to be used. So 32-bit binaries will be compiled by Clang and linked by GNU ld. (In reply to Piotr Kubaj from comment #9) Sure, but the ports tree is not just powerpc64 and some ports are compiled with gcc even on amd64 or i386 Does this patch work? %%% LDFLAGS+= -fuse-ld=bfd BINARY_ALIAS+= ld=${LD} . if !defined(USE_BINUTILS) -. if exists(/usr/bin/ld.bfd) +. if exists(/usr/bin/ld.bfd) && (${ARCH} != powerpc64 || ${OSVERSION} < 1300038) LD= /usr/bin/ld.bfd CONFIGURE_ENV+= LD=${LD} MAKE_ENV+= LD=${LD} . else +BINARY_ALIAS+= ld.bfd=${LOCALBASE}/bin/ld.bfd USE_BINUTILS= yes . endif . endif %%% The first patch you proposed ( https://bugs.freebsd.org/bugzilla/attachment.cgi?id=206395 ) was breaking ports on amd64 / i386 / aarch64 btw. (In reply to Antoine Brodin from comment #10) No, it still fails with the same error: 1187 configure:3325: cc -O2 -pipe -fstack-protector-strong -fno-strict-aliasing -std=gnu99 -I/tmp/usr/ports/misc/e2fsprogs-libuuid/work/e2fsprogs-1.45.3/lib -I/usr/local/include -fstack-protector-strong -fuse-ld=bfd conftest.c >&5 1188 /usr/bin/ld.bfd: Dwarf Error: found dwarf version '4', this reader only handles version 2 information. 1189 /usr/lib/crt1.o: In function `_start': 1190 crt1.c:(.text+0x2): undefined reference to `.TOC.' 1191 /usr/bin/ld.bfd: /usr/lib/crt1.o: unknown relocation type 252 for symbol .TOC. Note that since powerpc64 currently still use gcc in head, this PR is just to agree on a proper patch. Once we have a proper patch, this PR should wait until the switch to clang. Created attachment 206411 [details] patch Antoine, does this work for you? It worked for pkubaj@. I'm not sure if my commit message/inline comment are correct, though. - Only one extra conditional (easy to understand) - Comment why there (easy to refactor) - Drop OSVERSION to avoid redundancy with ld == lld conditional - Add -B${LOCALBASE}/bin because USE_BINUTILS alone doesn't (In reply to Piotr Kubaj from comment #4) > Regarding why powerpc64 needs special treatment: > when it switches to clang, ld.bfd will still be installed. > The reason is that lld currently doesn't support linking powerpc (32-bit) binaries. > We will use lld for 64-bit binaries, however. Do you mean -m64 uses ELFv2 while -m32 uses ELFv1? If so either review D20261 or review D20383 needs to make Clang default to BFD linker when -m32 is passed e.g., via getDefaultLinker(). (In reply to Jan Beich from comment #13) If it works on powerpc64, it's less intrusive for other archs. (In reply to Jan Beich from comment #13) There's no ELFv1 and ELFv2 for powerpc. powerpc has only one ABI and that doesn't change with the switch to Clang. ELFv2 is only defined for powerpc64. Regarding using GNU ld by default in Clang, I passed your suggestion to bdragon@, who currently leads the effort of switching to Clang, he said he would look at it. Since there's no OSVERSION check, I think this patch is ok to be committed now. OK? Can we get this patch to ports? I'd like it to be committed before the switch to clang is done, so that we don't get new breakages. Following our recent discussion on #powerpc64, is there any more information you need? I'm not on portmgr, so cannot approve. comment 14 can *probably* be treated as portmgr approval. When to land is up to you. Maybe adjust code comments and commit message if my understanding is incorrect/incomplete. A commit references this bug: Author: pkubaj Date: Sat Oct 12 20:49:35 UTC 2019 New revision: 514354 URL: https://svnweb.freebsd.org/changeset/ports/514354 Log: Mk/bsd.port.mk: use GNU LD from ports on powerpc64 elfv2 when GNU LD is required The reason is that on elfv2 systems we still have ld.bfd in base, but it's only used for 32-bit binaries (LLD currently doesn't support linking 32-bit PPC binaries). ld.bfd from base supports only elfv1 and using it breaks linking many ports that set LLD_UNSAFE. Use Binutils from ports in such case. PR: 239743 Submitted by: jbeich Approved by: portmgr Differential Revision: https://reviews.freebsd.org/D21996 Changes: head/Mk/bsd.port.mk |
Created attachment 206395 [details] patch With powerpc64 soon switching to clang, it doesn't make sense to use base ld.bfd when LLD_UNSAFE is defined.