Allow bootstrapping llvm-tblgen on macOS and Linux

[arichardson]

Summary

This is needed in order to build various LLVM binutils (e.g. addr2line)
as well as clang/lld/lldb.

Co-authored-by: Jessica Clarke <[e-mail]>

Test Plan

compiles on ubuntu 18.04 (and worked on macOS when I last tried)

[arichardson]

lib/libc/db/db/db.c

58 ↗

(On Diff #91810)

This change is needed to make cross-build/include/linux/fcntl.h compatible with LLVM since that uses ::open(...) which doesn't work with the statement macro.
Can also split this into a separate commit?

[jrtc27]

https://reviews.freebsd.org/P512 is a more complete version I wrote, though presumably will fail on Linux currently due to the sysctl and fcntl changes you had to make. __has_include is probably a nicer way to do it than I did, too; I just compared FreeBSD, macOS and Linux config.h files.

[arichardson]

In D31057#698583, @jrtc27 wrote:

https://reviews.freebsd.org/P512 is a more complete version I wrote, though presumably will fail on Linux currently due to the sysctl and fcntl changes you had to make. __has_include is probably a nicer way to do it than I did, too; I just compared FreeBSD, macOS and Linux config.h files.

Ah nice, I didn't think about adding LDFLAGS+= -Wl,-dead_strip. Will try to merge your changes into this.

[arichardson]

merge @jrtc27's changes to the config file (I didn't include the LLDB changes since we never bootstrap it)

[emaste]

Looks OK to me but will defer to @dim for questions about maintainability / expected impact on future clang/llvm updates

[jrtc27]

In D31057#699067, @arichardson wrote:

merge @jrtc27's changes to the config file (I didn't include the LLDB changes since we never bootstrap it)

Yeah, the lldb and lldb-server change aren't needed as those aren't bootstrap tools, but what about lld?

[dim]

In D31057#699199, @emaste wrote:

Looks OK to me but will defer to @dim for questions about maintainability / expected impact on future clang/llvm updates

I'm not too troubled by the various hacks in the config.h files, as we already have a few of those. On the other hand, if the ifdef maze becomes too large, then it might start to make sense to have different pregenerated .h files for different OS/arch combinations.

That said, all the other stuff with sysctl.h and fcntl.h seems very ugly to me, and I'd rather leave it out if at all possible. Or at least, separate it out to a separate commit.

lib/clang/libllvmminimal/Makefile

84 ↗

(On Diff #91879)

I think this is all quite ugly, why don't we solve this at another level? I.e for the bootstrap tools phase, just don't use the tools/build/cross-build/include/common/sys/sysctl.h file at all?

lib/libc/db/db/db.c

58 ↗

(On Diff #91810)

Isn't it easier to work around this problem by defining O_EXLOCK and friends to 0 in the compat fcntl.h instead? Then you could leave this file alone.

tools/build/cross-build/include/common/sys/sysctl.h

41

As mentioned above, I'd rather solve this on a different level, but maybe it's not that easy...

usr.bin/clang/clang.prog.mk

24

Hm, we can't do LIBADD += ncurses here? Or do we miss a definition in bsd.libnames.mk (or whatever it's called)?

[jrtc27]

usr.bin/clang/clang.prog.mk

24

ncurses has a build-tools target so can't be built early enough to provide a bootstrap ncurses for the bootstrap-tools tblgen

[arichardson]

lib/libc/db/db/db.c

58 ↗

(On Diff #91810)

Yes it would be, but then we get silent success without a lock if one of the bootstrap tools uses these flags. I think not defining it in that case is safer.

[arichardson]

tools/build/cross-build/include/common/sys/sysctl.h

41

In general we want to make sure that bootstrap tools don't use sysctl/sysctlbyname() since they generally expect FreeBSD sysctl and then might not work as expected. However, in the case of LLVM we do need the sysctls() on macOS so we need some way of opting into using the real sysctl definitions.

usr.bin/clang/clang.prog.mk

24

It might be possible to build the bootstrap tools without linking ncurses. Will look into that.

[jrtc27]

usr.bin/clang/clang.prog.mk

24

That's what my patch did, worked just fine on macOS. On Linux I made it use -ltinfo since that's what LLVM's CMakeLists.txt auto-detects, though you might be able to get away without that too, I didn't try.

[arichardson]

lib/clang/include/llvm/Config/config.h

252–253

This is why ncurses is not needed for macos but is on FreeBSD/Linux. Will upload a new patch that avoids terminfo for bootstrap tools.

[arichardson]

More complete version. Now builds Clang+LLD successfully on Ubuntu and llvm-tblgen on macOS (clang probably works but I don't want to overheat my laptop)

[jrtc27]

lib/clang/include/llvm/Config/config.h

247

I think we should still enable it on FreeBSD, it works fine there already so this could potentially regress colours in certain weird cases. Simplifies the Makefiles too, you can use the same condition for execinfo as for ncursesw.

lib/clang/llvm.build.mk

103

Shouldn't this already be included thanks to src.opts.mk -> bsd.compiler.mk -> bsd.linker.mk? I didn't need it in my patch.

[jrtc27]

tools/build/make.py

171

[arichardson]

lib/clang/include/llvm/Config/config.h

247

That's fine too. Since ncurses is assumed to be available unconditionally on the host that seems fine.

The only problem I can think of is that it might cause issues for shared builds after upgrading ncurses soversion. Is that something we should worry about?

lib/clang/llvm.build.mk

103

I seem to recall that I got build errors without it, will see if I can drop it.

[jrtc27]

lib/clang/include/llvm/Config/config.h

247

Well that already needs to be handled carefully today, not a new issue. Plus bumping the ncurses soname is a rare event...

[arichardson]

Address latest @jrtc27 feedback

[jrtc27]

contrib/llvm-project/lld/tools/lld/lld.cpp

149–150

Given this is the vendored source, why not just #if 1 this? The condition should always be true in every situation we build this. Then (combined with my sysctl.h suggestion) you don't need LLVM_BOOTSTRAPPING.

lib/libc/db/db/db.c

58 ↗

(On Diff #91810)

This diff can be much more minimal if you just do #ifndef/#define 0 like for O_CLOEXEC above (i.e., that's all you need to do, then the existing code here just works). Not in the header, just this file, so it's still an error to try to use one of the missing flags. And yes, this should probably be a separate commit.

tools/build/cross-build/include/common/sys/sysctl.h

40

Hmm. You could make this a BOOTSTRAPPING_WANT_NATIVE_SYSCTL define? That way it's both clearer and cleaner IMO.

[arichardson]

tools/build/cross-build/include/common/sys/sysctl.h

40

Yeah that's what I had initially (with a different spelling), will update to use yours.

[arichardson]

Address more comments

[jrtc27]

Couple of comments, otherwise looks good from my perspective.

lib/clang/libllvm/Makefile

721

Why's this commented-out? It should build fine during buildworld?

lib/clang/llvm.build.mk

116–120

This no longer will be passed when building the cross-toolchain on FreeBSD, which I would guess risks breaking some old system GCCs from when libstdc++ was used. I suggest adding the old .if ${.MAKE.OS} == "FreeBSD" || !defined(BOOTSTRAPPING) to this rather than putting it in the .else.

[arichardson]

lib/clang/libllvm/Makefile

721

It is fine during buildworld (but not during bootstrap). However, we don't build clangd/clang-query so it just adds to the build time unnecessarily.

lib/clang/llvm.build.mk

116–120

Sounds good will fix.

[arichardson]

address latest review feedbkac

[arichardson]

ping?

[dim]

LGTM

tools/build/make.py

259

Pedantic note, maybe chop off one character more to have the PEP checker stop complaining? :)

[arichardson]

tools/build/make.py

259

Will push a pep8 fix commit for that script shortly.

[markj]

usr.bin/clang/llvm.prog.mk

25

A dumb question: when building bootstrap-tools during buildworld, llvm-tblgen is built and libllvmminimal.a is linked in. The latter contains references to backtrace() since config.h defines HAVE_BACKTRACE unconditionally and llvm::sys::PrintStackTrace() uses it, so the build fails since libexecinfo isn't linked. BOOTSTRAPPING is defined to 0 by the top-level makefile, so how is this supposed to work?

[jrtc27]

usr.bin/clang/llvm.prog.mk

25

glibc defines it, or at least it does in the 2.27 version present in Ubuntu 18.04, and macOS defines it in libsystem_c.dylib

[markj]

usr.bin/clang/llvm.prog.mk

25

I see, thanks.