Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--enable-new-dtags should only be used for the GNU linker #46204

Closed
MartinHusemann opened this issue Nov 23, 2017 · 10 comments
Closed

--enable-new-dtags should only be used for the GNU linker #46204

MartinHusemann opened this issue Nov 23, 2017 · 10 comments
Labels
A-linkage Area: linking into static, shared libraries and binaries C-bug Category: This is a bug.

Comments

@MartinHusemann
Copy link
Contributor

AFAICT only Linux had broken DT_RPATH and replaced it with DT_RUNPATH. The latter is not implemented at all elsewhere, so while still using the gnu linker other platforms are broken by using RUNPATH instead of RPATH.

My attempts at trivially fixing it blindly (as bootstrap fails for me currently, see #45686) did not get far: #46202

But someone with a working setup should be able to fix it easily.

@kennytm kennytm added A-linkage Area: linking into static, shared libraries and binaries C-bug Category: This is a bug. labels Nov 23, 2017
@jethrogb
Copy link
Contributor

  1. Which platform are you using?
  2. I can't find any evidence in the glibc source that DT_RUNPATH is specific to Linux. Why is your platform using GNU ld but not GNU rtld?

@MartinHusemann
Copy link
Contributor Author

I (of course) am using NetBSD ;-)

I don't quite understand your question - binutils is very portable, and there are lots of ELF systems that are not Linux and do neither use glibc nor the Linux rtld.

Maybe a few of those have aliased DT_RUNPATH to DT_RPATH now (I don't know), but NetBSD has not and is not planing to do so.

So what is probably easiest here is: check for gnu linker (so it accepts the option) and for the linux kernel (as that is closed what you can easily get to check for the exact rtld in use).

Alternative: make use of RUNPATH vs RPATH explicitly selectable in the target definition.

@jethrogb
Copy link
Contributor

jethrogb commented Nov 24, 2017

binutils is very portable, and there are lots of ELF systems that are not Linux and do neither use glibc nor the Linux rtld.

There is no such thing as the "Linux rtld". For example, I'm fairly certain this issue is not an issue on Debian/kFreeBSD.

Maybe a few of those have aliased DT_RUNPATH to DT_RPATH now (I don't know), but NetBSD has not and is not planing to do so.

If an OS uses the GNU linker as the system linker and not the GNU rtld, I'd say that's asking for trouble. There is (as evidenced by this issue) no guarantee that the produced binaries are going to be compatible.

One more piece of evidence that this is not somehow “Linux's fault”: DT_RUNPATH has been part of the ELF specification for over 17 years: http://www.sco.com/developers/gabi/2000-07-17/ch5.dynamic.html

I'd prefer one of the following actions to be taken:

  1. Incompatible OSes update their system rtld to the latest ELF specification.
  2. Incompatible OSes patch whatever linker they use as the system linker to always produce binaries they can handle.
  3. Send a patch to binutils to disable the behavior of --enable-new-dtags (but still accept the flag) as a configuration option. Incompatible OSes should set this configuration option when they build binutils.

I looked at musl and it seems to be handling DT_RPATH and DT_RUNPATH equivalently. So I think everything would “just work” if Rust only passes --enable-new-dtags when target_env="gnu".

@MartinHusemann
Copy link
Contributor Author

OK, I am a bit reluctant to state the following - we never met in real life and I fear it might come across slightly strange. Let me do it anyway and I'll owe you a beverage of your choice when we should ever meet at a conference or wherever:

If I follow your line of thinking, I come to a different result:

  • following the obviously broken original ELF spec was a bad idea, so all the BSDs never did it and instead implemented DT_RPATH semantics just like they are now specified for DT_RUNPATH
  • when the ELF spec got fixed the way it was by introducing DT_RUNPATH there should not have been a ld(1) option added to fix new programs, if at all there should have been a negative option to generate compatible binaries for legacy systems
  • so Linux should make the --enable-new-dtags option a no-op in their linker, add a configure option and always use them on Linux
  • this should be upstreamed, of course
  • in any modern software, --enable-new-dtags should not be used ever

:-)

So back to rustc: nothing of this changes the way history happened, helps us support systems out there or improve rust portability.

If noone beats me to it, I will try again to come up with a working patch that makes use of the ld(1) option an explicit choice in the back/target definition. This will take some time due to my current bootstrap issues and the linker option handling being hardcoded in the zero stage bootstrap binaries.

Side note: I have to admit I was wrong at the start of this thread: DT_RUNPATH is not a Linuxism, Solaris (and maybe other SVr4 ELF systems) had DT_RPATH following the original spec too (but I don't know if their "native" linker made using DT_RUNPATH an option).

@geofft
Copy link
Contributor

geofft commented Nov 29, 2017

I'm a little confused why -Wl,--enable-new-dtags is generating DT_RUNPATH. In my mind, if NetBSD's ld.so doesn't support DT_RUNPATH, it doesn't make sense for GNU ld (or any linker) to generate DT_RUNPATH entires when targeting NetBSD, whether or not I ask for the "new" dtags. (I do agree that GNU should have just decided to default to DT_RUNPATH and the fact that we need to ask for "new" dtags forever is silly, but, what can I do.)

I installed a VM from the NetBSD 7.1 ISO on the website (GENERIC.201703111743Z) and I seem to get both DT_RPATH and DT_RUNPATH entries with the version of cc and ld I get:

netbsd# cat hello.c
#include <stdio.h>

char *response();

int main() {
    printf("%s\n", response());
}
netbsd# cat libfoo.c
char *response() {
        return "Hello world!";
}
netbsd# cc -fPIC -shared -o libfoo.so libfoo.c
netbsd# cc -L. -o hello hello.c -lfoo
netbsd# ./hello
Shared object "libfoo.so" not found
netbsd# cc -L. -o hello hello.c -lfoo -Wl,--enable-new-dtags -Wl,-rpath,/root
netbsd# ./hello
Hello world!
netbsd# readelf -d hello | grep /root
 0x000000000000000f (RPATH)              Library rpath: [/root]
 0x000000000000001d (RUNPATH)            Library runpath: [/root]
netbsd# ld --version
GNU ld (NetBSD Binutils nb1) 2.23.2
Copyright 2012 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) a later version.
This program has absolutely no warranty.

So I am a little confused that --enable-new-dtags breaks things for you - did the ld in NetBSD-current stop emitting both RPATH and RUNPATH, and if so, is that not a bug in ld?

(If you give me instructions on how to get an environment that matches yours, I'm happy to follow them, I just have not had the chance to learn as much about the BSDs as I'd like and so I don't really know how to install / upgrade things on this VM....)

@geofft
Copy link
Contributor

geofft commented Nov 29, 2017

Also to be clear - I'm not expressing a strong opinion either way on whether, if it is a bug in ld, Rust should / shouldn't work around it. I just want to make sure we understand what is happening and why for the sake of future people who come across this code.

(And if it's a bug in GNU ld that's hard to fix, I'd advocate for the Rust maintainers taking some sort of reasonable workaround like matching the target on the Linux kernel, because that happens to be the right heuristic for all supported platforms, even though the kernel technically isn't relevant, and I can likely be talked into writing such a patch.)

@MartinHusemann
Copy link
Contributor Author

Thanks for looking into it.

I am using NetBSD -current which has binutils 2.27 while NetBSD-7 had 2.23.
IIRC upstream changed ld starting with 2.24 or so to NOT emit DT_RPATH any more when using --enable-new-dtags.

I agree that this is a linker bug - but as you say, what can I do.

I also opened a discussion on NetBSD to additionaly support DT_RUNPATH and wrote a patch for it (one line change). With that, and a Cargo.toml hack (as suggested in #45686) I could complete bootstrap on amd64, so will look into writing a proper patch.

I'm sure you would be quicker with that, if you want to setup a test enviroment, just use NetBSD 8 beta, like this one:

http://nycdn.netbsd.org/pub/NetBSD-daily/netbsd-8/201711280120Z/images/NetBSD-8.0_BETA-amd64.iso

(This is the "daily" build, the exact timestamp will be invalid in a few days, but there will be newer ones following the same naming scheme)

@geofft
Copy link
Contributor

geofft commented Nov 29, 2017

Oh I see, this is binutils-gdb.git b1b00fcc, "ld: change --enable-new-dtags to only generate new dtags", from January 2013 / binutils 2.24.

I also see from Google that this has affected a few other pieces of software in NetBSD that have added -Wl,--enable-new-dtags to their Makefiles or something upstream. Blah. Supporting DT_RUNPATH in ld_elf.so is certainly an option, but a probably simpler thing you could do is have NetBSD's packaging of binutils revert that patch, or just unconditionally ignore the flag. (It would be better to have upstream binutils not generate the entry when targeting NetBSD, but fixing this in NetBSD's build seems like it would work.)

@jyn514
Copy link
Member

jyn514 commented Feb 3, 2023

@MartinHusemann are you still running into this issue? My understanding is that it's FreeBSD specific, and not actually a bug in rustc but in the linker itself ...

@jyn514 jyn514 changed the title DT_RUNPATH should not be used on != Linux --enable-new-dtags should only be used for the GNU linker Feb 3, 2023
@MartinHusemann
Copy link
Contributor Author

This can be closed - we agreed that this is an inconsistency between the linker and the runtime linker and I commited the patch I mentioned above , so NetBSD ld.elf_so treats DT_RUNPATH and DT_RPATH identical.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linkage Area: linking into static, shared libraries and binaries C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

5 participants