This is an archive of the discontinued LLVM Phabricator instance.

[driver][netbsd] Build and pass `-L` arguments to the linker
AbandonedPublic

Authored by atanasyan on May 31 2017, 9:13 AM.

Details

Reviewers
joerg
Summary

This patch builds and passes -L arguments to the linker in case of NetBSD target. That makes possible to use LLD linker which does not have any built-in per-OS paths configuration.

To do so the patch a) constructs paths to the library directories explicitly without using = placeholder. That allows to get correct paths whether --sysroot argument passed to the driver or not; b) calls ToolChain::AddFilePathLibArgs method to build a list of -L options from the paths list.

I hope this changes fix the following bug:
https://bugs.llvm.org/show_bug.cgi?id=33155

Diff Detail

Repository
rL LLVM

Event Timeline

atanasyan created this revision.May 31 2017, 9:13 AM

Looks great! I'm going to test it.

krytarowski added inline comments.May 31 2017, 9:52 AM
lib/Driver/ToolChains/NetBSD.cpp
324

What's the rationale for this condition? Will it work for LLVM libc++?

This works for me!

joerg edited edge metadata.May 31 2017, 3:12 PM

I'm against it. I consider this a strong bug on the LLD side and the behavior of clang correct.

I'm against it. I consider this a strong bug on the LLD side and the behavior of clang correct.

Do you suggest to add knowledge about various OS (linux/*bsd) and arch (x86,arm,mips,...) specific paths to the LLD?

Regardless of LLD, --sysroot is still needed I think.

joerg added a comment.May 31 2017, 3:38 PM

Such knowledge is necessary anyway. There is enough software that wants to use the linker directly.

joerg added a comment.May 31 2017, 3:39 PM

sysroot is already handled in NetBSD.cpp line 118 or so.

What software in particular, besides compilers?

joerg added a comment.Jun 1 2017, 11:37 AM

A small subset can be found by searching for LINKER_RPATH_FLAG in pkgsrc. A classic offender is Emacs. For more, I would have to systematically search.

@ruiu what's your opinion on this?

ruiu added a comment.Jun 6 2017, 9:40 AM

I'm totally against adding per-OS path knowledge to our linker. Compilers already know include paths and I don't want to maintain another list of paths in the linker. Also this can be more confusing than useful when you are doing cross-linking.

For all OSes other than NetBSD, LLD works fine with the clang driver as the driver passes include paths to the linker. I don't see any reason not to do the same thing for NetBSD. That stands even if the linker has to have a list of include paths.

I concur this, linkers are to used through a compiler frontend and libtool (which wraps a compiler).

joerg added a comment.Jun 7 2017, 6:55 AM
In D33726#774105, @ruiu wrote:

I'm totally against adding per-OS path knowledge to our linker. Compilers already know include paths and I don't want to maintain another list of paths in the linker. Also this can be more confusing than useful when you are doing cross-linking.

The only reason for compilers to maintain that list is for finding crt*.o. They otherwise don't care about the library paths at all. There is no confusion for cross-linking as long as proper sysroot support is used. Which we have been doing on NetBSD for ages.

For all OSes other than NetBSD, LLD works fine with the clang driver as the driver passes include paths to the linker. I don't see any reason not to do the same thing for NetBSD. That stands even if the linker has to have a list of include paths.

Sorry, but this is again ignorant and wrong. The very same problem of build systems calling ld directly apply on most other systems. Even then, the list of linker paths is not the only OS-specific knowledge. Things like the DT_RPATH vs DT_RUNPATH mess, init vs init_array all belong into this category. The list goes on.

atanasyan abandoned this revision.Sep 21 2017, 6:52 AM