The NetBSD clang driver relies on NetBSD ld knowing the default search
paths and never passes those paths explicitly. As a result of this
design requirement, implement appending default search paths within lld
as well.
Details
Diff Detail
Event Timeline
This doesn't seem a reasonable approach at all:
(1) It breaks cross-linking.
(2) It is not correct for any target architecture, e.g. /usr/local/lib certainly doesn't belong on this list and /lib doesn't either.
(3) The correct list depends not only on the target architecture, but also the active emulation.
lld's driver is intentionally designed to be agnostic of the host that the linker is running for the reasons described at the beginning of Driver.cpp: https://github.com/llvm-project/lld/blob/master/ELF/Driver.cpp#L13 I think I like that approach. If you need to do this, you can do this in the compiler driver rather than in the linker itself. Is there any reason you need to do this in the linker?
This breaks compat with GNU ld here and the linker is intended to be used standalone.
Is it acceptable to pass all the paths through configure/build phase of lld? It's done this way in GNU ld in the NetBSD distribution. If we need or want to hardcode all the specific paths it will be harder to maintain the proper list and behavior inside lld.
I don't think that would be better either. The main point is that it needs a lot more architectural knowledge than shown in the path. I would expect e.g. Linux distros have a similar problem nowadays.
I assume that we need to reimplement this in the context of lld:
NetBSD::NetBSD(const Driver &D, const llvm::Triple &Triple, const ArgList &Args) : Generic_ELF(D, Triple, Args) { if (!Args.hasArg(options::OPT_nostdlib)) { // When targeting a 32-bit platform, try the special directory used on // 64-bit hosts, and only fall back to the main library directory if that // doesn't work. // FIXME: It'd be nicer to test if this directory exists, but I'm not sure // what all logic is needed to emulate the '=' prefix here. switch (Triple.getArch()) { case llvm::Triple::x86: getFilePaths().push_back("=/usr/lib/i386"); break; case llvm::Triple::arm: case llvm::Triple::armeb: case llvm::Triple::thumb: case llvm::Triple::thumbeb: switch (Triple.getEnvironment()) { case llvm::Triple::EABI: case llvm::Triple::GNUEABI: getFilePaths().push_back("=/usr/lib/eabi"); break; case llvm::Triple::EABIHF: case llvm::Triple::GNUEABIHF: getFilePaths().push_back("=/usr/lib/eabihf"); break; default: getFilePaths().push_back("=/usr/lib/oabi"); break; } break; case llvm::Triple::mips64: case llvm::Triple::mips64el: if (tools::mips::hasMipsAbiArg(Args, "o32")) getFilePaths().push_back("=/usr/lib/o32"); else if (tools::mips::hasMipsAbiArg(Args, "64")) getFilePaths().push_back("=/usr/lib/64"); break; case llvm::Triple::ppc: getFilePaths().push_back("=/usr/lib/powerpc"); break; case llvm::Triple::sparc: getFilePaths().push_back("=/usr/lib/sparc"); break; default: break; } getFilePaths().push_back("=/usr/lib"); } }
This is where lld is not 100% compatible with GNU ld, but I'd think that's not a bad thing. I'd like to make lld agnostics of host OS so that the linker works exactly in the same way on any operating systems, which makes cross linking much easier to do. So, both a run-time detection of a host OS or a configure-time customization are I think undesirable.
Personally I have no strong opinion either way. Joerg is blocking a patch to handle it in clang: https://reviews.llvm.org/D33726
I find it very silly reason to brick NetBSD support in lld. I expect that it's easier to push paths into lld than convince @joerg to accept clang patch.
As you can see, lld doesn't really have any host-dependent knowledge so far, even though there are a few OSes that use lld as the default linker. We've added host-dependent knowledge to the compiler driver instead of to the linker for various operating systems. I guess that we could do the same thing for NetBSD, no?
Actually I find it frustrating and unfair as GNU ld doesn't have builtin knowledge either.. it's passed with gross 'super hack' comments from build scripts... but we are forced to push it to lld in order to move on.
I sympathize and understand your frustration, but I don't know what I can do to you. The thing that lld doesn't have host-specific config is a policy matter we've been maintaining so far for all operating systems. I don't think NetBSD should be the only exception of the policy. In addition to that, I strongly believe the fact that lld's behavior only depends on the command line options given to the linker is easier to understand and generally a good thing.
Joerg, what is special about NetBSD?
Nothing.
But the result is that we don't have GNU gold either and are stuck with 40min linking times of LLVM. It's destructive to productivity and damages any LLVM related development.
Talking from the perspective of having had to deal with thousands of packages in pkgsrc over the years: it is naive to believe that there isn't a lot of software that calls the linker directly for various reasons. As such, it is very important to have a useful configuration in the linker by default. Such a configuration is by its very nature target specific. This doesn't mean it can't be done in a cross-compiler friendly way, on the contrary. Over the years NetBSD has been pushing its toolchain to be as similar for the native build and a cross-target as reasonable possible. Modulo the build time choices in the config.h sense, the only difference between the native and cross tools is the built-in default of the former.
Clang works extremely well in that regard and perfectly supports a universal toolchain. LLD should as well. Consistent behavior of ELF across different OS is a fiction, as such some OS and/or CPU family specific defaults are naturally different. This can be grouped in general:
(1) The target operating system specified either from the builtin default, the name of the tool (triple-ld), an option like clang's --target or if desirable the name of the emulation when specified. I don't have a strong reason for supporting the last as it is typically not unique even with binutils and more a case of historic legacy than useful feature. For the perspective of supporting both native and cross toolchains, the first three options are enough and when using the compiler driver, the first two will generally work fine.
(2) The target CPU family. While it can be partially guessed from the object files, there are fun expects. ARM instruction encodings are one of the pecularities a linker has to deal with for example. Is BX a valid jump instruction?
(3) Whether the target CPU family is the primary ABI. This can generally not be determined from the object files and can affect the default search paths. Hard vs soft floating point is a good example here. Other cases are easier like linking i386 binaries on x86_64. N32 vs N64 on MIPS is more difficult again. This is a bit tricky in that it often can be drived only from the emulation name.
In terms of architecture, it doesn't need much, but it needs some thought. Identifying the target OS is likely the easiest. Most of the rest is OS specific adjustment. Having access to the binary name and the the arguments should be enough though. Doing it properly avoids the fun of having to patch a lot of software without costing that much in terms of code.
We will prepare a patch for i386 x86_64 only for now. Until we will get lld fully functional on NetBSD/amd64 we will skip other architectures.
ELF/Driver.cpp | ||
---|---|---|
378 | Please drop MIPS/PPC for now. |
ELF/Driver.cpp | ||
---|---|---|
370 | is it possible to use some Triple NetBSD target here? |
ELF/Driver.cpp | ||
---|---|---|
370 | Technically, you can get triple from process name but that obviously works only when lld is run as $CHOST-lld and not plain lld, which is not really the case right now. |
On 03.01.2019 21:19, Joerg Sonnenberger wrote:
On Thu, Jan 03, 2019 at 06:34:22PM +0000, Kamil Rytarowski via Phabricator via cfe-commits wrote:
krytarowski added a comment.
Actually I find it frustrating and unfair as GNU ld doesn't have builtin
knowledge either.. it's passed with gross 'super hack' comments from build scripts... but we are forced to push it to lld in order to move on.I'm puzzled. Seriously, when was the last time you actually checked how
much customization contains on a per-OS base in GNU ld? Yes, I'm
including the various build scripts because GNU ld is generally build by
a mix of hard-coded logic in the tool itself and various adjustments in
the linker scripts it is shipped with. But they are all a builtin part
of GNU ld as far as the end user is concerned. It is pretty much
irrelevant from a point of functionality where that logic is, but
skipping is a serious usability issue.Joerg
I'm referring to code '/usr/src/external/gpl3/binutils/usr.bin/ld/Makefile':
# XXX super hack . if (${BINUTILS_MACHINE_ARCH} == "x86_64" && \ ("${f}" == "elf_i386" || "${f}" == "i386nbsd")) EMUL_LIB_PATH.${f}=/usr/lib/i386 . elif (${BINUTILS_MACHINE_ARCH} == "sparc64" && \ ("${f}" == "elf32_sparc" || "${f}" == "sparcnbsd")) EMUL_LIB_PATH.${f}=/usr/lib/sparc . elif !empty(BINUTILS_MACHINE_ARCH:Mmips64*) . if "${f}" == "elf32ltsmip" || "${f}" == "elf32btsmip" EMUL_LIB_PATH.${f}:=/usr/lib/o32 . elif "${f}" == "elf64ltsmip" || "${f}" == "elf64btsmip" EMUL_LIB_PATH.${f}:=/usr/lib/64 . else EMUL_LIB_PATH.${f}=/usr/lib . endif . else EMUL_LIB_PATH.${f}=/usr/lib . endif e${f}.c: ${DIST}/ld/genscripts.sh ${.CURDIR}/Makefile stringify.sed ${_MKTARGET_CREATE} unset MACHINE || true; \ LIB_PATH=${EMUL_LIB_PATH.${f}} NATIVE=yes \ ${HOST_SH} ${DIST}/ld/genscripts.sh ${DIST}/ld \ ${LIBDIR} "/usr" "/usr/bin" \ ${G_target_alias} ${G_target_alias} ${G_target_alias} \ ${G_EMUL} ${LIBDIR} yes ${G_enable_initfini_array} \ ${f} "${G_target_alias}"
So we are now trying to put the logic directly into lld.
We've discussed this a bit and given other changes we need to do, and I see pretty much three options here:
- We hardcode stuff under defined(__NetBSD__) which kinda solves the problem, except lld won't be very cross-friendly.
- We try to do conditionals based on triple but this works only when we customize the install to include it in executable name. We probably would still need to default based on defined(__NetBSD__) when triple isn't available via process name.
- We create *nbsd* emulations for all arches (e.g. amd64/x86 don't have such emulations right now), and use that to switch logic. This would be closer to what FreeBSD does, I think. However, if we do this, then I suppose we should also add similar aliases to GNU ld.
I think that ifdefining the linker options with __NetBSD__ is no-go.
I think that ifdefining the linker options with __NetBSD__ is no-go. We expect to get lld to link NetBSD programs from any host, or from NetBSD to any other.
- We try to do conditionals based on triple but this works only when we customize the install to include it in executable name. We probably would still need to default based on defined(__NetBSD__) when triple isn't available via process name.
I think it won't work for us.
- We create *nbsd* emulations for all arches (e.g. amd64/x86 don't have such emulations right now), and use that to switch logic. This would be closer to what FreeBSD does, I think. However, if we do this, then I suppose we should also add similar aliases to GNU ld.
Probably too gross hack just for the gain redesigned model of a linker functional.
On Thu, Jan 03, 2019 at 08:31:53PM +0000, Kamil Rytarowski via Phabricator via cfe-commits wrote: > krytarowski added a comment. > > On 03.01.2019 21:19, Joerg Sonnenberger wrote: > >> On Thu, Jan 03, 2019 at 06:34:22PM +0000, Kamil Rytarowski via Phabricator via cfe-commits wrote: >> >>> krytarowski added a comment. >>> >>> Actually I find it frustrating and unfair as GNU ld doesn't have builtin >>> knowledge either.. it's passed with gross 'super hack' comments from build scripts... but we are forced to push it to lld in order to move on. >> >> I'm puzzled. Seriously, when was the last time you actually checked how >> much customization contains on a per-OS base in GNU ld? Yes, I'm >> including the various build scripts because GNU ld is generally build by >> a mix of hard-coded logic in the tool itself and various adjustments in >> the linker scripts it is shipped with. But they are all a builtin part >> of GNU ld as far as the end user is concerned. It is pretty much >> irrelevant from a point of functionality where that logic is, but >> skipping is a serious usability issue. >> >> Joerg > > I'm referring to code '/usr/src/external/gpl3/binutils/usr.bin/ld/Makefile': I think that's a left over from old times before the emulparams/* logic was done properly. But that's more a case of GNU ld's build system being gross than anything else. Joerg
Feel free to drop it.
On Thu, Jan 03, 2019 at 06:58:41PM +0000, Kamil Rytarowski via Phabricator wrote: > But the result is that we don't have GNU gold either and are stuck with > 40min linking times of LLVM. It's destructive to productivity and > damages any LLVM related development. The reason noone cared much about GNU gold is that it supports only a limited set of platforms and forces a lot of modern GNU "innovations" without any chance of fixing them. To a degree, both concerns apply to lld as well, but reasonable well integrated LTO support with Clang provides at least something in return. I have no idea about your link times, the only situation where linking is taking a really significant chunk of time is for full debug builds and the general solution for that is DWARF fission. Joerg
Actually I do cared to port gold but it didn't work and I wasn't able to spare more time. My work and a fixup for one feature is in pkgsrc-wip.
I think that this place is not the right place to bash GNU ld for performance issues.
I will refer just to slides and paper from Ian Lance Taylor to get overview why it is unacceptably slow:
https://www.airs.com/ian/gold-slides.pdf
https://ai.google/research/pubs/pub34417.pdf
I will add that (unless nothing changed recently) ignoring lack of features (like thinlto) GNU ld cannot produce >=4GB executables and this makes it even more unusable (this matters already for projects of webkit size in debug unoptimized build).
On 03.01.2019 23:15, Joerg Sonnenberger wrote: > On Thu, Jan 03, 2019 at 09:38:49PM +0000, Kamil Rytarowski via Phabricator via cfe-commits wrote: >> I think that this place is not the right place to bash GNU ld for performance issues. > > I didn't. > >> I will refer just to slides and paper from Ian Lance Taylor to get overview why it is unacceptably slow: >> >> https://www.airs.com/ian/gold-slides.pdf >> https://ai.google/research/pubs/pub34417.pdf > > We all know that gold and lld are faster. It's a huge step from > "somewhat faster" to "unacceptably slow". But that's again a completely > separate topic.
I used to waste like an hour daily average, any test-build of a local change and break of 5-10 min is unacceptably slow.
>> I will add that (unless nothing changed recently) ignoring lack of >> features (like thinlto) GNU ld cannot produce >=4GB executables and >> this makes it even more unusable. > > That sounds seriously like a troll. I already mentioned DWARF fission > for the one reasonable case for > 100MB binaries and that's in short > "don't touch most of the data"... >
I've edited the entry above that it's already more than 4GB of unoptimized webkit build with debuginfo, and gnu ld is truncating files on 4GB boundary.
It might be naive but I don't think it's too naive. Most programs use ld via cc, and I don't think it is too unreasonable to not implement a host-specific logic for a very small percentage of programs that directly use ld. If we do, that logic would be the same or very similar to the one that we already have in cc. I think we should avoid that duplication of the host-specific config in the toolchain.
I see there are pros and cons to not have host-specific config in ld. That said, if other operating systems can live without host-specific config in lld, why can't NetBSD do? I really don't like to be in a situation where only one operating system have a host-specific config while others don't.
Before we even can start a philosophical dispute, is there some way to even get that setup functional in lld? How to check if the target is supposed to be a NetBSD binary? @mgorny raised some proposals.. https://reviews.llvm.org/D56215#1345563 but all of them seem to be either dysfunctional (ifdefs) or hackish (renaming emul names or renaming lld binary to netbsd-*).
Looking at the code of lld, the detection of FreeBSD is done by checking emulation name (*fbsd) and anything else by a flag passed to a linker such as --do-something-for-android (e.g. "use-android-relr-tags")
Actually I think that we can handle two cases witch a combination of proposals:
- native mode
- cross mode
For the native mode we go for something like:
#if defined(__NetBSD__) #define NATIVE_TARGET LLD_NETBSD #else ...
and for cross mode read argv[0] to detect target triple in program name.
If argv[0] == ld we assume native mode and set target to native, otherwise try to parse target from argv[0], if the target is not parsable bail out with error.
This way we can transplant cc driver information into lld.
Not sure what I understand the point, but as to make lld work on/for NetBSD, I wonder if you can just add -L<path> to the command line in the compiler driver. Isn't a NetBSD triple passed to the compiler driver? If so, I wonder if you can add a few extra options when invoking the linker.
This describes the original patch of passing flags from compiler driver and breaks GNU ld compat. Joerg expects to pass no extra -L<path> or --disable-new-dtags options, treating lld as a drop-in replacement for GNU ld.
I'm trying to determine how the idea of Joerg can be implemented.
For the record, another option is to actually fix other software not to call LD directly.
Or if you really need to call the linker directly without specifying search paths you could also install a shell script as /usr/bin/ld that passes the appropriate flags to /usr/bin/ld.lld?
I don't think ifdefs in the source code are a good idea. Cross linking should just work as expected. But you could look at the OS field in the first input ELF file to choose default config options/a different emulation for NetBSD.
The approach we are using in CheriBSD to differentiate between MIPS and CHERI pure-capability to either pass an explicitly -m elf_btsmip_cheri_fbsd/ -m elf_btsmip_fbsd or infer whether it is CHERI or MIPS based on the e_flags field of the first input file.
This would be the best option to use a shell wrapper for someone who needs it, but we cannot convince Joerg for almost 2 years.
I don't think you would lose anything by passing -L<path> and --disable-new-dtags from compiler driver. These flags wouldn't do any harm to GNU linkers.
I've grepped FreeBSD, OpenBSD and pkgsrc.
OpenBSD ports
Total 2 packages there have issues with LLD.
$ grep -r USE_LLD . ./devel/libcoap/Makefile:USE_LLD= No ./sysutils/firmware/vmm/Makefile:USE_LLD= No
FreeBSD ports
There is a list of 140-150 unsafe (LLD_UNSAFE) packages with LLD. A
part of the entries on the list are magnified by the same
framework/piece-of-software, but different component of version.
6 ports have documented issues with search paths:
- pdcurses
- libds
- evangeline
- rexx-regutil
- otpw
- installwatch
The list might be incomplete.. but "thousands of monkeys fixing
packages" mentioned is strong overestimation.
pkgsrc
It was mentioned in 2017 (!) that rejecting teaching the driver because of packages
in pkgsrc that use LINKER_RPATH_FLAG (but probably not limited to this
option).
All of that looks like restricted to:
- GCC,
- Mercury,
- GPS,
- sather,
- wmutils-core
- aws
Most of that is probably about unneeded direct call of a linker.
Probably the list might be longer for trickier examples like emacs. (But
emacs seems to have no issues on OpenBSD and FreeBSD).
There is a list of 140-150 unsafe (LLD_UNSAFE) packages with LLD.
Some of these are tagged with a PR or comment explaining why they are LLD_UNSAFE, but we haven't done it consistently. Some of these 140-150 were probably added in the early days of bringing lld into FreeBSD, and added without much thought - some could be as easy as differences in -ztext/-znotext default.
Next version, based on recognizing NetBSD from triple.
@joerg, is this a better approach?
Thanks, this looks like a good starting point.
ELF/Driver.cpp | ||
---|---|---|
815 | See ToolChain::getTargetAndModeFromProgramName in clang. |
ELF/Driver.cpp | ||
---|---|---|
815 | Yes, I've based this on stripped down version of that. Most notably, I wasn't able to verify triple against TargetRegistry since it isn't initialized here. |
I'm sorry to say this but I don't think this is a good approach. Just like changing the default behavior depending on host hurts cross-build and is against the policy of the lld driver, changing the default behavior depending on the target hurts it too. Imagine that you are doing a cross-build on non-NetBSD system to create a NetBSD executable. You definitely want to ignore /usr/lib/i386 and /usr/lib on the host system. This patch does the opposite.
To be honest, I don't think I would lgtm a patch that changes lld's default behavior depending on host/target only for NetBSD, and it seems like a NetBSD's issue rather than an lld's issue. As I said, could you please make an effort to pass the flags you need from the compiler driver to the linker just like other systems do? That is easy to do, doesn't hurt anyone, probably a good thing to do anyway as it makes options explicit rather than implicit, and solves at least most of the problems you have.
@ruiu: No, it is exactly what you want, since it allows you to point lld into the normal sysroot. Cross-compiling is the default case for the NetBSD toolchain.
ELF/Driver.cpp | ||
---|---|---|
382 | Those need to be sysroot-relative of course. |
I want to handle NetBSD in the same way as the other operating systems. I'm sorry to have been saying no to a few recent patches for NetBSD, but I think that's for a good reason, and I don't think you presented a convincing reason why we had to handle only NetBSD differently.
ELF/Driver.cpp | ||
---|---|---|
806 | If I invoke an unprefixed ld.lld on NetBSD but want to target a different operating system, this will cause all the NetBSD defaults to apply to that binary and will possibly cause it to crash at runtime. I think any config changes based on a triple would need to use an explicit --target= flag instead. As @ruiu says, LLD's behaviour should not change depending on the host OS/default LLVM triple. Given the same input files and command line options the resulting binary should be identical on any host. |
ELF/Driver.cpp | ||
---|---|---|
817 | There is need to add a fallback for a native triple. |
@joerg if you think that this patch is OK, please click accept so we can be aware that this is what you want.
ELF/Driver.cpp | ||
---|---|---|
815 | initLLVM is called too late. That should be all that is needed to do a proper look up. |
Implemented checking the triple against target registry. Also made --version output the detected target.
ELF/Driver.cpp | ||
---|---|---|
806 | There needs to be a way to override the target triple that is not creating prefixed a symlink to ld.lld. Otherwise I can't use NetBSD ld.lld to build a non-NetBSD target without giving a value for every config option that lld supports. I think there should be a command line option to override the triple (e.g. --triple= or --target=). |
ELF/Driver.cpp | ||
---|---|---|
806 | OSABI field is not reliable way to detect OS/ABI. Everybody except FreeBSD sets UNIX SystemV. |
ELF/Driver.cpp | ||
---|---|---|
806 | Actually there is a FreeBSD specific hack to detect emulation name, and it has suffix fbsd.. if it is detected it sets FreeBSD OSABI. We don't have a chance to use a similar hack for NetBSD in other configuration options. |
@mgorny could you check if we can get crossbuilding functional for:
- to !NetBSD from NetBSD
- from !NetBSD to NetBSD.
- from NetBSD/amd64 to NetBSD/aarch64
I wonder whether it can work if we will keep using 'ld' file name for a linker.
ELF/Driver.cpp | ||
---|---|---|
369 | From a stylistic point of view, I would introduce a dedicated file for the NetBSD driver (DriverNetBSD.cpp?) that overloads generic LinkerDriver::appendDefaultSearchPaths(). I have no opinion what C++ semantics to use for it. The generic driver could be empty or use use Config->SearchPaths.push_back("=/usr/lib");. Probably empty is better as it would be more generic. |
Actually we have discussed it internally with @mgorny and we will propose a patch in Clang to handle this easier/better.
ELF/Driver.cpp | ||
---|---|---|
375 | As we have TargetTriple now, I would use it here instead of Config->EMachine, it will be easier to map knowledge from the Clang driver and handle special ABI cases. |
Right, I'm not aware of anyone but FreeBSD really using the OSABI field. For FreeBSD it was a long standing hack around limitations in the ELF kernel loader. I'm not even sure if FreeBSD use(d) to set the OSABI field for the intermediate object files.
Actually probably Linux MIPS (or SPARC?) used it and some proprietary OSes.. but we skip them for now. Only FreeBSD sets OSABI in lld and uses an emulation name detection hack. Once we will get merged TripleTarget detection we can switch it to use proper Triple check.
We will add proper mapping for other ELF targets and everybody will benefit from a standalone linker.
As mentioned previously it will be more than standalone as we need to introduce NetBSD specific customization into target binaries that is not easily mappable from linker options (unless someone wants to switch to Linux style ELF files on NetBSD, but it won't happen). From bigger changes we plan to change DT_NEEDED semantics for NetBSD and keep catching further integration issues.
Split target logic into D56650, switched to using target to determine which paths to apply. While at it, copied the code from clang since it now can match exactly.
From a stylistic point of view, I would introduce a dedicated file for the NetBSD driver (DriverNetBSD.cpp?) that overloads generic LinkerDriver::appendDefaultSearchPaths().
I have no opinion what C++ semantics to use for it.
The generic driver could be empty or use use Config->SearchPaths.push_back("=/usr/lib");. Probably empty is better as it would be more generic.