Page MenuHomePhabricator

[LLD] Add NetBSD support as a new flavor of LLD (nb.lld)
Needs ReviewPublic

Authored by krytarowski on Nov 9 2019, 1:49 PM.

Details

Summary

The NetBSD target wraps the default Linux/ELF target with OS
specific customization. It is implemented as a light netbsd::link()
module wrapper that mutates arguments in argv[] and calls
elf::link().

This flavor detects the native/current and target Triple based on
argv[0] parsing. This is prerequisite for cross-compilation, in
particular the NetBSD distribution is cross-built always.

The default configuration of the ELF target is tuned for Linux and
there is no way to costomize in-place for the NetBSD target in the
same way as FreeBSD/OpenBSD. FreeBSD whenever needed can check
emulation name ("*_fbsd") and OpenBSD calls its extensions
"PT_OPENBSD_*".

This distinct flavor is needed for NetBSD as:

  • the linker MUST work in the standalone mode
  • it must be useful with gcc/pcc/others out of the box
  • clang NetBSD driver shall not hardcode LLD specific options
  • the linker must have support for cross-building
  • LLD shall be a drop-in replacement for (NetBSD-patched) GNU ld

There is no code-duplication between the NetBSD and ELF modules.
The NetBSD driver for debugging purposes prints the Target string
for the -v|-V|--version command line argument. There is no functional
or code maintenance change for other Operating Systems, especially
the ELF ones.

Equivalent customization is already done for the Darwin mode. For
instance there are hardcoded default search paths such as "/usr/lib"
and "/Library/Frameworks" in DarwinLdDriver.cpp. There is a prior
art with the MinGW target that similarly wraps coff:link().

This change is a starting point for development of NetBSD support
in LLD.

Diff Detail

Event Timeline

krytarowski created this revision.Nov 9 2019, 1:49 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptNov 9 2019, 1:49 PM

Another approach to achieve the same goal as previous attempts. It is already better as it avoids two forks.

Does this one look like fine for merging?

The previous one for the reference: https://reviews.llvm.org/D69755

  • fix a typo in a comment.
krytarowski added a comment.EditedNov 10 2019, 9:59 AM

Short summary:

  • FreeBSD/OpenBSD ship with mutations of the behavior of ELF/GNU/Linux in certain nits, we do the same with our driver.
  • MinGW ships with a frontend driver to COFF, we do the same with a driver to ELF/GNU/Linux.
  • Darwin ships with default search paths, we do the same in our driver.
  • NetBSD is not a GNU system, it is a BSD system that differs with ELF/GNU/Linux, GNU/Linux is at most an inspiration, none of our project goals is about reimplementing Linux. The same applies to MinGW, Darwin etc that do not reimplement Linux.
  • We already patch LLVM code for NetBSD support, especially LLDB and compiler-rt are the most heavily modified one.

The proposed change does not make anything without prior art.

LLD is practically the last piece of the LLVM toolchain stack without NetBSD support. We run the NetBSD buildbot for LLD since 2018 and there are no plans to detach it.

We intend to integrate LLD into our distribution, but we first must make it work in the upstream version out of the box.

Ping?

We are in the process of switching our buildbot to newer NetBSD snapshot (-8 to -9) and we keep waiting for this patch to land.

If we could get this patch merged, we could build and link the whole LLVM with lld on NetBSD and it would increase the productivity of the bot (better build times). Right now we need to maintain hacks to link at most with 2/3 cores, while 5/6 ones are idling doing nothing due to enormous RAM consumption of GNU ld.

We also upgraded LLVM in the current snapshot to a 10git version, and for the next snapshot merged (hopefully 10.0 or early 11git) we could ship it together with LLD aboard.

MaskRay added a comment.EditedDec 2 2019, 1:14 PM

If we could get this patch merged, we could build and link the whole LLVM with lld on NetBSD and it would increase the productivity of the bot (better build times). Right now we need to maintain hacks to link at most with 2/3 cores, while 5/6 ones are idling doing nothing due to enormous RAM consumption of GNU ld.

I will be very happy to see the productivity of NetBSD build bot gets improved. My comments below are about the approach you choose in this patch.

FreeBSD/OpenBSD ship with mutations of the behavior of ELF/GNU/Linux in certain nits, we do the same with our driver.

The FreeBSD specific code is just the following 4 lines in ELF/Driver.cpp:

if (s.endswith("_fbsd")) {
  s = s.drop_back(5);
  osabi = ELFOSABI_FREEBSD;
}

PT_OPENBSD_* extensions are related to https://bugs.llvm.org//show_bug.cgi?id=31288 , which predated my involvement into lld.
It was not a good example of issue reports. Neither the bug nor the link it referenced provided rationale why such extensions were needed. That decision was made before my involvement into lld, though. If such patches were posted now, I would definitely ask if there were alternative ways to do things that would be less intrusive. CC @brad here. I will also ask in their IRC channel and/or send an email to their mailing list.

On the contrary, the NetBSD extension does much more than FreeBSD (4 lines of code) and OpenBSD (7 lines of code modulo comments). The patch introduces another toplevel driver nb.lld, and includes ~200 lines of change.
Moreover, I anticipate you will post another patch with probably more than 200 lines of code that does https://blog.netbsd.org/tnf/entry/the_first_report_on_lld#handling-of-indirect-shared-library-dependencies , which many people (probably even the GNU maintainers) feel no longer appropriate nowadays. IIRC an earlier review already got pushback from several lld contributors.

the linker MUST work in the standalone mode

By standalone mode, do you mean $(LD) or ld? I think for most such use cases, we should probably change them to do gcc/clang -nostdlib instead. In most cases users are not supposed to call ld directly. On FreeBSD (and Google servers and Android, if you consider them distributions), it has been proved many packages don't need ld customization to function properly. I personally fixed >10 instances of direct ld invocation in the internal Google code base. Many (including some third party or Google open sourced projects) use really brittle constructs and can break when different build configurations are considered. They are almost all fixed by changing to $(CC) alternatives, with only a few exceptions where GNU ld has to be used (tricky linker scripts that lld does not provide great compatibility with GNU ld).

If you can't migrate off those packages right now, a shell script that gets installed at "/usr/bin/ld" will probably work.

clang NetBSD driver shall not hardcode LLD specific options

Several toolchains in the clang/lib/Driver/ToolChains detect lld and make lld specific decisions there. While I hope linker specific options should be kept at a minimum, I understand that sometimes it is unavoidable. When we have no choice but to add linker specific options, adding such code to clangDriver is an established practice followed by almost every toolchain (see Android, Fuchsia, MinGW, MSVC, etc). Doing the driver thing in two places will more likely to cause conflicts.

the linker must have support for cross-building

I think this patch will actually harm cross building that targets NetBSD. Before, ld.lld is shared by all ELF platforms: Linux, FreeBSD, Fuchsia, Android, ChromeOS, etc. If an ELF change works on Linux, it will assuredly work on other platforms. This patch will introduce a new driver that naturally gets less test coverage.

MaskRay added a subscriber: brad.Dec 2 2019, 1:26 PM
ruiu added a comment.Dec 11 2019, 11:50 PM

lld is not that Linux-centric as you implied in your comment. We have code for FreeBSD, OpenBSD, AMDGPU and Fuchsia (which isn't even a POSIX system), and adding code for NetBSD is OK and is appreciated. However, the thing we've been trying to avoid is to hard-code a platform-specific knowledge such as library paths to the linker. Combined with the fact that lld doesn't provide a way to enable/disable targets at build-time, all ld.lld executables (sometimes installed as /usr/bin/ld) work the same way no matter where they are run on. That helps those who do cross compilation a lot. That is a very appreciated property of our linker, and I don't think I want make an exception for NetBSD. Even on NetBSD, lld should behave the same as running on other kind of hosts.

Also, frankly speaking, having a driver only for NetBSD seems very odd to me. We would have 5 drivers for MSVC link.exe, MinGW, ELF-based systems, macOS, and NetBSD? That doesn't feel right. If you strongly feel that the linker should know the library paths and the like, it shouldn't be too hard to write a shell script wrapper to wrap lld.

We attempted to patch internally LLD rebasing our code for the buildbot purposes but LLD without being used rots quickly, see: https://reviews.llvm.org/D58892

We use LD as ld and there are no plans neither intentions to break it. It was already decided internally in the project.

If you insist, I can rewrite nb.lld from C++ to shell and upstream to the LLD repository. We require -fuse-ld=lld to be rerouted through this shell script and lld invocation call it so we can revert back to the concept from https://reviews.llvm.org/D69755. Personally I see no gain in this as I prefer to rely on LLVM libs at least for Target Triplet detection.

As I can understand that Linux/GNU behavior is promoted (and enforced to other ELF OSs) it does not work for us. I propose a patch that keeps intact the GNU/Linux code.

If 200 LOC for NetBSD is a show stopper, please see e.g. compiler-rt where we maintain 2 orders of magnitude more NetBSD-only LOC.

I try to avoid forking LLD for downstream and this shall be appreciated.

MaskRay added a comment.EditedDec 13 2019, 2:00 PM

I know I'm late to the party but this change thoroughly destroyed NetBSD support. The NetBSD loader doesn't support having more than two PT_LOAD sections. -z norosegment helped with that so far but after this change practically everything fails to run.

This seems to be a very serious limitation. I have difficult to understand how such limitation could exist at all, but I think this should be straightforward to fix.

Yes, it is a serious limitation, and no, it's not straightforward to fix. The loader has making a lot of assumptions, especially what PT_LOAD segments to expect and in which order. I've previously made a patch that allowed third segment but it was rejected as apparently 'not doing it the right way'. Almost a year has passed, and I am entirely powerless to fix it.

@mgorny told me that NetBSD ld.so has a limitation that it can only handle 2 PT_LOAD segments.

I think the NetBSD toolchain team's attitude toward certain ELF things should probably be made more flexible. For that particular PT_LOAD limitation issue (a.out heritage?), if @mgorny don't mind giving me a pointer, I'd like to help argue fixing the limitation.

If 200 LOC for NetBSD is a show stopper, please see e.g. compiler-rt where we maintain 2 orders of magnitude more NetBSD-only LOC.

I am not sure compiler-rt is a good example here. The "2 orders of magnitude more" OS specific things make compiler-rt difficult to maintain. Innovations can be easily constrained by the numerous OS specific customization. I am glad that lld hasn't entered that situation.

I feel that NetBSD's stubborn attitude is adding complexity to many other projects.

I try to avoid forking LLD for downstream and this shall be appreciated.

If you can accept DT_RUNPATH and fix the PT_LOAD limitation, then it seems NetBSD just needs its own ld wrapper that adds -L, and probably -znognustack which saves you 56 bytes. I don't understand why such customization should still be upstreamed.

krytarowski added a comment.EditedDec 13 2019, 2:58 PM

@joerg works on the ELF loader and we just need to wait for him. We wrote our temporary patch, but it was not ideal as the long term solution.

compiler-rt is nicely maintained from our perspective and we have almost feature parity with Linux.

We maintain and develop around 4000 NetBSD specific LOC in LLDB. That number grows, especially with each CPU architecture gaining support [soon in upstream].

it seems NetBSD just needs

This is not everything, but I don't want to derail yet another review into offtopic or generic discussion. Getting lld functional standalone is merely a starting point for us.

We need a linker that is a GNU ld drop-in replacement. As we have no interest in fixing it for Linux, we want this property for NetBSD.

I don't understand why such customization should still be upstreamed.

LLD is by default bricked, without upstreaming standalone LLD support it will be broken for NetBSD and we will keep observing that people attempt to cross-build malformed binaries for our OS (as they will be at least tuned for Linux). Additionally this forces us to maintain a subproject (LLD Standalone) externally.

krytarowski added a comment.EditedDec 13 2019, 5:14 PM

Stating it from a different side. If you do not care about GNU ld drop-in replacement property for Linux it's not our business, but we do care about this in NetBSD and we restrict this to our project only (from ELF targets). If you do not care about NetBSD, you shall not care how and whether we use LLD. We try hard to not interfere with Linux looking for a consensus.

You are not forced to use the NetBSD driver neither define it in LLD_SYMLINKS_TO_CREATE. If you have a strong disrespect to this GNU ld drop-in replacement approach, I can patch the die("") invocation in main() to conditionally print "nb.lld". There are around 20 lines patches in the generic code and around 1/3 of that is in the code comments. This shall not be burden for any of you... there is a larger NetBSD-only single code block even in LLVM OpenMP.

@MaskRay I will mail you off-list with one question.

krytarowski added a comment.EditedJan 2 2020, 8:14 PM

LLVM Release Schedule: 10.0.0: Jan 15, 2020: branch, then rc1

It would be nice to get a reply to my mail proposal before the branch.. so we could push the code for LLVM 10. (If it was lost, I can resend it) It can likely upset both sides, but with the proposal in the end we get a bfd.ld drop-in replacement. (One of LLVM goals was to be a drop-in replacement for GNU toolchain components, even if some of them are irrelevant on a number of platforms.)