Bug 227918

Summary: [PATCH] remove exists check for CROSS_BINUTILS_PREFIX for external clang builds on secondary arches
Product: Base System Reporter: Kenneth Salerno <kennethsalerno>
Component: miscAssignee: freebsd-toolchain (Nobody) <toolchain>
Status: New ---    
Severity: Affects Only Me CC: bdrewery, brooks, marklmi26-fbsd
Priority: --- Keywords: patch
Version: 11.1-RELEASE   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
patch to remove exists check none

Description Kenneth Salerno 2018-05-02 11:04:49 UTC
Created attachment 192990 [details]
patch to remove exists check

Hi,

When using devel/llvm60 as external compiler on powerpc64, it is necessary to specify CROSS_BINUTILS_PREFIX if you want to use powerpc64-binutils and therefore requires BFLAGS+= -B/usr/local/bin/powerpc64-unknown-freebsd${OSREL}- set in Makefile.inc1.

Attached patch removes exists(${CROSS_BINUTILS_PREFIX}) directory check for CROSS_BINUTILS_PREFIX when it should add -B for all cases if the env variable is set, because it doesn't hurt in cases where it's unnecessary anyway.
Comment 1 Brooks Davis freebsd_committer freebsd_triage 2018-05-04 16:00:32 UTC
I think this makes sense.  I wonder if there's a sensible diagnostic we could add.
Comment 2 Bryan Drewery freebsd_committer freebsd_triage 2018-05-04 23:28:18 UTC
I'm missing something here... Do the llvm xtoolchain packages not "just work"
for cross-compiling already?
Comment 3 Kenneth Salerno 2018-05-05 11:20:06 UTC
(In reply to Bryan Drewery from comment #2)
Hello,

devel/xtoolchain-llvm* is just a no-build external compiler definition setting XCC variables etc., not an external /cross-compiler/ being built specifically for a single target arch like for example devel/powerpc64-gcc is. 

xtoolchain-llvm60 is therefore equivalent to what I am doing already in make.conf:
MYLLVMSUFFIX = 60
_OSRELEASE != uname -r
OSREL = ${_OSRELEASE:C/-.*//}
XCC = /usr/local/bin/clang${MYLLVMSUFFIX}
XCXX = /usr/local/bin/clang++${MYLLVMSUFFIX}
XCPP = /usr/local/bin/clang-cpp${MYLLVMSUFFIX}
CROSS_BINUTILS_PREFIX = /usr/local/bin/powerpc64-unknown-freebsd${OSREL}-
XCFLAGS += -B/usr/local/bin/powerpc64-unknown-freebsd${OSREL}-
XCXXFLAGS += -B/usr/local/bin/powerpc64-unknown-freebsd${OSREL}-
XLDFLAGS += -B/usr/local/bin/powerpc64-unknown-freebsd${OSREL}-
NO_WERROR =
WERROR =
WITHOUT_GCC = yes
WITHOUT_GCC_BOOTSTRAP = yes
WITHOUT_GNUCXX = yes
WITH_CLANG = yes
WITH_CLANG_BOOTSTRAP = yes
WITH_CLANG_FULL = yes
WITH_CLANG_IS_CC = yes
WITH_LLD = yes

So the problem with all of this is when you use llvm to cross-compile (e.g. amd64 build host -> ppc64 target), and you don't specify -B, it will try to use /usr/bin/ld which will default to the host machine arch.

To force the correct arch I am using powerpc64-binutils, which has a funky prefix (/usr/local/bin/powerpc64-unknown-freebsd11.1- that breaks this directly exists check in Makefile.inc1 that will always fail and not automatically add the correct -B from my CROSS_BINUTILS_PREFIX.
Comment 4 Mark Millard 2018-05-08 00:41:45 UTC
(In reply to Bryan Drewery from comment #2)

Note that the target is powerpc64. lld does not work
for that target yet. (Or did not last I tried and I've
not seen evidence of it being worked on for targeting
powerpc64.) The llvm* are not self supporting for this
target yet.

Thus the need to specify an alternate binutils for use.

(There are submittals at llvm about issues for lld
for targeting the powerpc families, and for the
compiler as well.)
Comment 5 Mark Millard 2018-05-08 01:39:25 UTC
(In reply to Kenneth Salerno from comment #0)

Quote:
therefore requires BFLAGS+= -B/usr/local/bin/powerpc64-unknown-freebsd${OSREL}-
 set in Makefile.inc1
End quote

Why not something based more on something like:
(the _altbinutils suffix is explained later)

CROSS_TOOLCHAIN=llvm60_altbinutils
CROSS_BINUTILS_PREFIX=/usr/local/powerpc64-unknown-freebsd${OSREL}/bin

# ls /usr/local/powerpc64-unknown-freebsd12.0/bin/
ar      as      ld      ld.bfd  nm      objcopy objdump ranlib  readelf size    strip

(So strings is missing and would need, say,
/usr/local/bin/powerpc64-unknown-freebsd${OSREL}-strings via, say, XSTRINGS.)

Of course there are lines in /usr/local/share/toolchains/llvm60.mk
that are a problem and need an alternative to avoid use of lld and
such:

# diff /usr/local/share/toolchains/llvm60.mk /usr/local/share/toolchains/llvm60_altbinutils.mk
4,5c4,5
< XLD=/usr/local/llvm60/bin/ld.lld
< CROSS_BINUTILS_PREFIX=/var/empty
---
> #XLD=/usr/local/llvm60/bin/ld.lld
> #CROSS_BINUTILS_PREFIX=/var/empty


I'll note that /usr/src/Makefile.inc1 already has logic for -B that
applies absent those when CROSS_BINUTILS_PREFIX is defined and
exists:

.if defined(CROSS_BINUTILS_PREFIX) && exists(${CROSS_BINUTILS_PREFIX})
# In the case of xdev-build tools, CROSS_BINUTILS_PREFIX won't be a
# directory, but the compiler will look in the right place for its
# tools so we don't need to tell it where to look.
BFLAGS+=        -B${CROSS_BINUTILS_PREFIX}
.endif


(I've got such a cross-build in process to test. I've done this
before but its been a while.)
Comment 6 Mark Millard 2018-05-08 01:58:14 UTC
(In reply to Mark Millard from comment #5)

Ultimately the build failed on assembler notation:
(note the double -B usage and the lib32 context, so
powerpc target locally, instead of powerpc64)

_ERROR_CMD='/usr/local/bin/clang60 -DCOMPAT_32BIT -mcpu=powerpc -m32 -L/usr/obj/powerpc64vtsc_xtoolchain-llvm/powerpc.powerpc64/usr/src/powerpc.powerpc64/obj-lib32/tmp/usr/lib32 --sysroot=/usr/obj/powerpc64vtsc_xtoolchain-llvm/powerpc.powerpc64/usr/src/powerpc.powerpc64/obj-lib32/tmp -B/usr/local/powerpc64-unknown-freebsd12.0/bin -B/usr/obj/powerpc64vtsc_xtoolchain-llvm/powerpc.powerpc64/usr/src/powerpc.powerpc64/obj-lib32/tmp/usr/lib32 -O2 -pipe -I/usr/src/lib/csu/common  -I/usr/src/lib/libc/include  -g -std=gnu99  -Wsystem-headers -Wall -Wno-format-y2k -W -Wno-unused-parameter -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wreturn-type -Wcast-qual -Wwrite-strings -Wswitch -Wshadow -Wunused-parameter -Wcast-align -Wchar-subscripts -Winline -Wnested-externs -Wredundant-decls -Wold-style-definition -Wno-pointer-sign -Wthread-safety -Wno-empty-body -Wno-string-plus-int -Wno-unused-const-variable  -Qunused-arguments    -c /usr/src/lib/csu/powerpc/crtn.S -o crtn.o; ;'

got errors such as:

/usr/src/lib/csu/powerpc/crtn.S:30:11: error: unexpected token in memory operand
--- lib/libcompiler_rt__L ---
Building /usr/obj/powerpc64vtsc_xtoolchain-llvm/powerpc.powerpc64/usr/src/powerpc.powerpc64/obj-lib32/lib/libcompiler_rt/ctzsi2.po
--- lib/csu__L ---
 lwz 11,0(1)
          ^
/usr/src/lib/csu/powerpc/crtn.S:31:10: error: unexpected token in memory operand
 lwz 0,4(11)
         ^
--- crti.o ---
/usr/src/lib/csu/powerpc/crti.S:29:2: warning: DWARF2 only supports one section per compilation unit
 .section .init,"ax",@progbits
 ^
/usr/src/lib/csu/powerpc/crti.S:34:13: error: unexpected token in memory operand
--- crtn.o ---
/usr/src/lib/csu/powerpc/crtn.S:32:2: error: invalid instruction mnemonic 'mtlr'
 mtlr 0
 ^~~~
. . .

I've restarted it using WITHOUT_LIB32= to see if the rest works.
Comment 7 Mark Millard 2018-05-08 02:11:32 UTC
(In reply to Mark Millard from comment #6)

The WITHOUT_LIB32= based build completed.

So it looks to me like the -B handling logic in the build
environment for FreeBSD may need some work for lib32 capable
contexts for cross-toolchain builds.


I'll note that gcc/g++ reports for its -B option:

QUOTE
`-B' prefixes that effectively specify directory names also apply to libraries in the linker, because the compiler translates these options into `-L' options for the linker. They also apply to includes files in the preprocessor, because the compiler translates these options into `-isystem' options for the preprocessor. In this case, the compiler appends `include' to the prefix.
END QUOTE

This would make using a non-directory different than pointing -B to a directory,
at least for gcc/g++ cross tools.


But I did not find any wording about the handling of multiple -B's on
the command line.
Comment 8 Mark Millard 2018-05-08 02:56:45 UTC
(In reply to Mark Millard from comment #7)

I will also note that /usr/local/bin/clang60 has code
generation problems for powerpc family members, in
particular, as an example, where the system library
source uses __builtin_eh_return it will be ignored.
A consequence is that any thrown C++ exceptions will
crash the program when the system is running what was
built via ignoring __builtin_eh_return .

The llvm60 (and 50) output for .ko's now also involves
.rela.plt and R_PPC64_JMP_SLOT that FreeBSD does not
handle, leading dynamic loading of kernel modules to
crash. (I did not have that issue back in the llvm40
days.) I've no clue if FreeBSD should handle such vs.
if clang should not cause such to be generated. I just
observe that they are mis-matched currently.

(I'm not claiming that those are the only problems.)

I've never gotten near being able to run devel/kyua
from a clang-based build yet, kyua uses throwing C++
exceptions extensively.

I took to building-in kernel modules that I wanted to
be able to use if I'm going to use a clang60-based
kernel.
Comment 9 Kenneth Salerno 2018-05-08 17:26:47 UTC
(In reply to Mark Millard from comment #8)
Agreed, Mark. devel/clang60 also has fatal issues generating processor/ISA-specific instructions(e.g. -mcpu=power9 is unusable) and even crashes itself during compilation, funnily enough.

I understand that -B is usually passed as a search directory, but it just so happens to also work as a binutils prefix for me :) and it has never harmed my builds passing multiple -B declarations with different arguments.
Comment 10 Mark Millard 2018-05-08 22:47:57 UTC
(In reply to Kenneth Salerno from comment #9)

The question for -B and finding binutils files when there are multiple -B's is:
which -B path ends up being used? In the lib32 failure example, the content of
the two places are not equivalent if I understand right. It appeared that the
one with the wrong content for lib32 was used for lib32 at the failure point,
if I understand right.

My understanding of -B is that it is primarily for finding binutils files
files but (for gcc/g++) also has -L and -isystem consequences when it
points to a directory (that is found), allowing such other types of files
to be bundled with the binutils files in the directory structure.

Sorry to hear that -mcpu=power9 has problems of its own. Nothing I've tested
is newer than an old PowerMac G5 so-called "Quad Core", so from 2006 or so.
Comment 11 Kenneth Salerno 2018-05-11 16:41:53 UTC
(In reply to Mark Millard from comment #10)
Mark,

The answer to your first question is, at least for clang, the first -B is used:

kens@freebsd-amd64$ ls -1 /usr/home/kens/test*/ld
/usr/home/kens/test1/ld
/usr/home/kens/test2/ld

kens@freebsd-amd64$ clang60 -B/usr/home/kens/test1/ -B/usr/home/kens/test2/ -print-prog-name=ld
/usr/home/kens/test1/ld

kens@freebsd-amd64$ clang60 -B/usr/home/kens/test2/ -B/usr/home/kens/test1/ -print-prog-name=ld
/usr/home/kens/test2/ld
Comment 12 Mark Millard 2018-05-11 18:21:52 UTC
(In reply to Kenneth Salerno from comment #11)

Actually my assumptions behind my interpretation were wrong. The
failed build step used (in order):

-B/usr/local/powerpc64-unknown-freebsd12.0/bin
-B/usr/obj/powerpc64vtsc_xtoolchain-llvm/powerpc.powerpc64/usr/src/powerpc.powerpc64/obj-lib32/tmp/usr/lib32

The content of the 2nd area was not what I was assuming. The two areas have:

$ ls -lT /usr/local/powerpc64-unknown-freebsd12.0/bin/
total 60000
-r-xr-xr-x  2 root  wheel  5619170 Apr 21 20:52:06 2018 ar
-r-xr-xr-x  2 root  wheel  7213389 Apr 21 20:52:12 2018 as
-r-xr-xr-x  4 root  wheel  7552620 Apr 21 20:52:17 2018 ld
-r-xr-xr-x  4 root  wheel  7552620 Apr 21 20:52:17 2018 ld.bfd
-r-xr-xr-x  2 root  wheel  5493387 Apr 21 20:52:06 2018 nm
-r-xr-xr-x  2 root  wheel  6411715 Apr 21 20:52:06 2018 objcopy
-r-xr-xr-x  2 root  wheel  7475890 Apr 21 20:52:06 2018 objdump
-r-xr-xr-x  2 root  wheel  5619185 Apr 21 20:52:06 2018 ranlib
-r-xr-xr-x  2 root  wheel  1584214 Apr 21 20:52:06 2018 readelf
lrwxr-xr-x  1 root  wheel       44 Apr 21 20:52:27 2018 size -> ../../bin/powerpc64-unknown-freebsd12.0-size
-r-xr-xr-x  2 root  wheel  6411722 Apr 21 20:52:06 2018 strip

vs.

$ ls -lT /usr/obj/powerpc64vtsc_xtoolchain-llvm/powerpc.powerpc64/usr/src/powerpc.powerpc64/obj-lib32/tmp/usr/lib32/
total 648
drwxr-xr-x  2 root  wheel     512 May  7 18:48:21 2018 dtrace
drwxr-xr-x  2 root  wheel     512 May  7 18:48:21 2018 i18n
-rwxr-xr-x  1 root  wheel   32538 May  7 18:48:28 2018 libc_nonshared.a
-rwxr-xr-x  1 root  wheel  409886 May  7 18:48:27 2018 libcompiler_rt.a
lrwxr-xr-x  1 root  wheel      16 May  7 18:48:27 2018 libgcc.a -> libcompiler_rt.a
-rwxr-xr-x  1 root  wheel  157554 May  7 18:48:27 2018 libgcc_eh.a
-rwxr-xr-x  1 root  wheel    2320 May  7 18:48:26 2018 libssp_nonshared.a

The rejection of some of the assembler notation for lib32's build steps may not be traceable to this at all.
Comment 13 Mark Millard 2018-05-11 22:27:20 UTC
(In reply to Mark Millard from comment #12)

Use of -v with the erring command reported in comment 6 shows that it
tries to run the i386 assembler (based on the -triple it uses
to run the assembler and the -target-cpu that it specifies as well):

. . . 
 "/usr/local/llvm60/bin/clang-6.0" -cc1as -triple i386-portbld-freebsd12.0 -filetype obj -main-file-name crtn.S -target-cpu i486 -I /usr/src/lib/csu/common -I /usr/src/lib/libc/include -fdebug-compilation-dir /usr/src -dwarf-debug-producer clang version 6.0.0 (tags/RELEASE_600/final) -I /usr/src/lib/csu/common -I /usr/src/lib/libc/include -debug-info-kind=limited -dwarf-version=2 -mrelocation-model static -o crtn.o /tmp/crtn-d80df9.s
/usr/src/lib/csu/powerpc/crtn.S:29:2: warning: DWARF2 only supports one section per compilation unit
 .section .init,"ax",@progbits
 ^
/usr/src/lib/csu/powerpc/crtn.S:30:11: error: unexpected token in memory operand
 lwz 11,0(1)
          ^
/usr/src/lib/csu/powerpc/crtn.S:31:10: error: unexpected token in memory operand
 lwz 0,4(11)
         ^
/usr/src/lib/csu/powerpc/crtn.S:32:2: error: invalid instruction mnemonic 'mtlr'
 mtlr 0
. . .


So -mcpu=powerpc -m32 on the clang60 command line was not enough
to cause targeting of powerpc for the assembler. May be a -triple
is required on the command line as well?
Comment 14 Kenneth Salerno 2018-05-11 23:02:54 UTC
(In reply to Mark Millard from comment #13)
Yes, please see my other bug report on that topic:

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=227920