This is an archive of the discontinued LLVM Phabricator instance.

[lld] [ELF] Include default search paths for NetBSD driver
Needs ReviewPublic

Authored by mgorny on Jan 2 2019, 12:38 PM.

Details

Summary

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.

Diff Detail

Event Timeline

mgorny created this revision.Jan 2 2019, 12:38 PM
joerg added a comment.Jan 2 2019, 1:34 PM

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.

ruiu added a comment.Jan 2 2019, 2:01 PM

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?

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.

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.

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.

joerg added a comment.Jan 2 2019, 3:26 PM

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.

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.

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.

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.

What's the expected solution?

This is a blocker to move on.

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");
  }
}

https://github.com/llvm-mirror/clang/blob/e030444510df2ffaf23eeae35692dc363bc28439/lib/Driver/ToolChains/NetBSD.cpp#L334-L379

ruiu added a comment.Jan 3 2019, 10:09 AM

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.

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.

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.

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.

ruiu added a comment.Jan 3 2019, 10:31 AM

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?

It's an option but Joerg insists on using lld standalone.

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.

ruiu added a comment.Jan 3 2019, 10:52 AM

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?

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.

joerg added a comment.Jan 3 2019, 11:16 AM

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.

mgorny updated this revision to Diff 180114.Jan 3 2019, 11:41 AM
mgorny edited the summary of this revision. (Show Details)

Updated to use clang's libdir logic.

krytarowski added inline comments.Jan 3 2019, 11:47 AM
ELF/Driver.cpp
377

Please drop MIPS/PPC for now.

krytarowski added inline comments.Jan 3 2019, 11:56 AM
ELF/Driver.cpp
369

is it possible to use some Triple NetBSD target here?

mgorny marked an inline comment as done.Jan 3 2019, 12:05 PM
mgorny added inline comments.
ELF/Driver.cpp
369

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.

mgorny updated this revision to Diff 180121.Jan 3 2019, 12:12 PM
mgorny edited the summary of this revision. (Show Details)

Removed non-x86.

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:

  1. We hardcode stuff under defined(__NetBSD__) which kinda solves the problem, except lld won't be very cross-friendly.
  2. 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.
  3. 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.

We've discussed this a bit and given other changes we need to do, and I see pretty much three options here:

  1. We hardcode stuff under defined(__NetBSD__) which kinda solves the problem, except lld won't be very cross-friendly.

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.

  1. 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.

  1. 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.

krytarowski added a comment.EditedJan 3 2019, 1:38 PM
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).

krytarowski added a comment.EditedJan 3 2019, 2:20 PM
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.

ruiu added a comment.Jan 3 2019, 2:26 PM

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.

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.

krytarowski added a comment.EditedJan 3 2019, 2:42 PM

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.

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.

ruiu added a comment.Jan 3 2019, 3:25 PM

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.

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.

mgorny added a comment.Jan 3 2019, 9:52 PM

For the record, another option is to actually fix other software not to call LD directly.

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.

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.

ruiu added a comment.Jan 4 2019, 10:30 AM

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 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:

  1. pdcurses
  2. libds
  3. evangeline
  4. rexx-regutil
  5. otpw
  6. 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:

  1. GCC,
  2. Mercury,
  3. GPS,
  4. sather,
  5. wmutils-core
  6. 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.

mgorny updated this revision to Diff 180717.Jan 8 2019, 12:40 PM

Next version, based on recognizing NetBSD from triple.

@joerg, is this a better approach?

joerg added a comment.Jan 8 2019, 12:58 PM

Thanks, this looks like a good starting point.

ELF/Driver.cpp
770

See ToolChain::getTargetAndModeFromProgramName in clang.

mgorny marked an inline comment as done.Jan 8 2019, 1:07 PM
mgorny added inline comments.
ELF/Driver.cpp
770

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.

ruiu added a comment.Jan 8 2019, 1:08 PM

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.

ruiu added a comment.Jan 8 2019, 1:30 PM

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.

Thanks, this looks like a good starting point.

What's the final state you want?

joerg added a comment.Jan 8 2019, 1:40 PM

@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.

joerg added inline comments.Jan 8 2019, 1:48 PM
ELF/Driver.cpp
381

Those need to be sysroot-relative of course.

ruiu added a comment.Jan 8 2019, 1:57 PM

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.

arichardson added inline comments.Jan 8 2019, 2:58 PM
ELF/Driver.cpp
761

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.

krytarowski added inline comments.Jan 8 2019, 4:19 PM
ELF/Driver.cpp
772

There is need to add a fallback for a native triple.

mgorny updated this revision to Diff 180886.Jan 9 2019, 11:12 AM
mgorny marked 3 inline comments as done.

Adjusted to make paths sysroot-relative.

@joerg if you think that this patch is OK, please click accept so we can be aware that this is what you want.

joerg added inline comments.Jan 11 2019, 5:57 AM
ELF/Driver.cpp
770

initLLVM is called too late. That should be all that is needed to do a proper look up.

mgorny updated this revision to Diff 181268.Jan 11 2019, 7:37 AM
mgorny marked 2 inline comments as done.

Implemented checking the triple against target registry. Also made --version output the detected target.

arichardson requested changes to this revision.Jan 11 2019, 9:36 AM
arichardson added inline comments.
ELF/Driver.cpp
761

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=).
Also how will the default this interact with input files that have the OSABI field set? I feel like the options based on the target OSABI should be used instead of the default triple.

This revision now requires changes to proceed.Jan 11 2019, 9:36 AM
krytarowski added inline comments.Jan 11 2019, 10:09 AM
ELF/Driver.cpp
761

OSABI field is not reliable way to detect OS/ABI. Everybody except FreeBSD sets UNIX SystemV.

krytarowski added inline comments.Jan 11 2019, 10:36 AM
ELF/Driver.cpp
761

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
368

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.

@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.

Actually we have discussed it internally with @mgorny and we will propose a patch in Clang to handle this easier/better.

krytarowski added inline comments.Jan 11 2019, 1:16 PM
ELF/Driver.cpp
374

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.

joerg added a comment.Jan 11 2019, 1:24 PM

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.

krytarowski added a comment.EditedJan 11 2019, 1:50 PM

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.

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.

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.

mgorny updated this revision to Diff 181484.Jan 13 2019, 2:03 PM
mgorny marked 7 inline comments as done.
mgorny set the repository for this revision to rLLD LLVM Linker.

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.