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

Make rpath usage optional #5219

Closed
fabiand opened this issue Mar 4, 2013 · 32 comments · Fixed by #11744 or #11887
Closed

Make rpath usage optional #5219

fabiand opened this issue Mar 4, 2013 · 32 comments · Fixed by #11744 or #11887
Labels
A-linkage Area: linking into static, shared libraries and binaries
Milestone

Comments

@fabiand
Copy link
Contributor

fabiand commented Mar 4, 2013

Currently (something after 0.5) rustc (IIUIC) is using rpaths to point libs and binaries to the appropriate locations. This is problematic when rustc and the other libs/bins are package e.g. for Fedora.
The rpath usage should be made optional - /etc/ld.so.conf.d can be used instead - and LD_LIBRARY_PATH.

@brson
Copy link
Contributor

brson commented Mar 5, 2013

Why is this problematic for packaging? Is it because the rpath values end up with the wrong values?

Each crate's rpath has several values, one of which is supposed to be the final installation location (controlled with --prefix). Can we just provide more control over which locations are used during rpathing?

@fabiand
Copy link
Contributor Author

fabiand commented Mar 5, 2013

In general Fedora recommends to not use rpaths [0], because the paths are hardcoded in the binary. Fedora preferres to point the dynamic linker to the appropriate paths [1].
But yes, we are more flexible about internal libraries - which the case is here. So yes, it would also help us if we could controll the rpath path (which goes a bit into the direction if issue #5223)

[0] https://fedoraproject.org/wiki/Packaging:Guidelines#Beware_of_Rpath
[1] https://fedoraproject.org/wiki/Packaging:Guidelines#Alternatives_to_Rpath

@fabiand
Copy link
Contributor Author

fabiand commented Mar 5, 2013

The following rpath is used in e.g. the cargo binary:

ERROR   0002: file '/usr/bin/cargo' contains an invalid rpath '/home/fdeutsch/rpmbuild/BUILD/rust-0.6/x86_64-unknown-linux-gnu/stage1/lib/rustc/x86_64-unknown-linux-gnu/lib' in [$ORIGIN/../lib:/home/fdeutsch/rpmbuild/BUILD/rust-0.6/x86_64-unknown-linux-gnu/stage1/lib/rustc/x86_64-unknown-linux-gnu/lib:/usr/lib/rustc/x86_64-unknown-linux-gnu/lib]

The rpath referres to rpm's buildroot, where the lib won't stay (it will be instaled to --prefix)
Maybe the problem could adressed if the rpath would respect --prefix - but this could lead to problems at build time.
Maybe LD_LIBRARY_PATH can be used to point to the correct dir's at build time, and the rpath can be used to point to the correct dir's at runtime. Just my 2ct - but you ar ethe expert :)

@brson
Copy link
Contributor

brson commented Mar 8, 2013

Thanks for those links. We do generate rpaths with the expectation that some of them will be invalidated by moving the binaries around.

It's possible we could consider removing rpath, but it sure is nice to not think about dynamic loading when running binaries out of my build directory.

We could easily add a --disable-rpath flag to the configure script, to turn off the rpath for all the rust-provided libraries, with the assumption that package managers would modify ld.so.conf.d during install. If we did that I feel like we would still want the installed rustc to generate rpaths with user-built libraries - otherwise the experience would be different depending on the rust installation.

We could also just remove rpath completely - it doesn't work on windows anyway.

Another thing to consider is that, if we just used rustpkg for all builds, and let rustpkg install all libraries at a known location, then we might be able to conveniently not use rpath while still doing the right thing at load time. Can user directories like ~/.rustpkg be put into ld.so.conf.d?

@fabiand
Copy link
Contributor Author

fabiand commented Mar 8, 2013

Yes - I can fully understand how easy running rust in the build directory is when using rpaths :)
For the build directory use case it should be possible to create wrappers for rustc and friends which use LD_LIBRARY_PATH to point to the correct dirs.

You mention thet some libs might reside in ~/.rustpkg, could you explain the overall concept you are following with this?
In general I'd expect libraries to appear in /lib, /usr/lib some and some private /usr/lib/myapp. There are also some xdg specs that specify ~/.local/bin and ~/.local/lib - but let usfirst see what you say about ~/.rustpkg. Oh, I don't think that ~/.rustpkg would be expanded correctly if put into ld.so.conf.d

@fabiand
Copy link
Contributor Author

fabiand commented Mar 8, 2013

Just to summarize my thoughts: If rpath is used, the rpath path beeing used should only use directories below --prefix (to point to non-standard dirs containing libs relevant for a rust component).
The rpath path should not point to any build-time dir.

To include libraries at buildtime or when using the binaries in the build directory I'd suggest using wrappers (isn't autotools doing it like this?) which set LD_LIBRARY_PATH appropriately.

@fabiand
Copy link
Contributor Author

fabiand commented Mar 11, 2013

Any follow up thoughts?

@brson
Copy link
Contributor

brson commented Mar 11, 2013

Our package manager will by default not be installing libraries to a system location and will instead put them in the user's directory, ~/.rustpkg. This doesn't seem out of the ordinary to me; consider cabal, etc.

The behavior you recommend requires some deep changes to how Rust works, so need to be considered carefully. Does the current behavior prevent packaging in Fedora, and is there a minimal change we can make to unblock packaging?

@graydon You'll probably be interested in this subject.

@fabiand
Copy link
Contributor Author

fabiand commented Mar 13, 2013

Thanks for the rustpkg explanation.
Do I understand that rustpkg will create binaries and libs with rpaths pointing to the user's specific rustpkg dir ~/.rustpkg?
My feeling is that this user specific rpaths should be configurable at build time. The use case is obviously when the binary or lib is going to be installed systemwide. The only rpath exception is the rpath pointing to the system wide package specific libdir (e.g. /usr/lib64/rust).

I think that I'll spend a bit more time with looking at link.rs and rpath.rs and might get a better feeling for the problem.

Btw.: I noted that the libdir isn't configurabel. Fedora for example keeps x86_64 libs in (/usr)?/lib64, and i686 in (/usr)/lib. IIUIC this is currently not possible with rust as the libs will be installed to /usr/lib/arch-specific-path

@brson
Copy link
Contributor

brson commented Mar 14, 2013

@fabiand The point about libdir is well taken and I updated #5223.

I am not entirely sure how rustpkg does it's build now or how @catamorphism and @graydon intend for it to work, but based on the current language design I would expect that, yes, binaries installed to ~/.rustpkg would have rpaths pointing there.

Currently there are several rpaths configured for each linked library so that they hopefully fall back to something useful when the libraries are moved around during install or during the bootstrapping process, something like:

  • The relative path to the library
  • The absolute path to where the library was at build time
  • The system installation path

Under this scheme I would expect that rustpkg would, when linking against libraries in ~/.rustpkg, create rpaths to ~/.rustpkg.

@ghost ghost assigned catamorphism Mar 14, 2013
@fabiand
Copy link
Contributor Author

fabiand commented Mar 14, 2013

Yes, there are several rpaths used (seen in link.rs and rpath.rs) and I'd say - even with looking at rust itself, without the packaging glasses on - that it should be configurable what rpaths are used. Maybe it should be an option of rustc.

As said, system wide binaries are the best example for binaries where you don't want to many rpaths - including no rpaths pointing to some user dir.

@TJC
Copy link

TJC commented Mar 15, 2013

Please don't tag TJC :(

@brson
Copy link
Contributor

brson commented Mar 15, 2013

Sorry, TJC! Maybe it's time to give in and start working on Rust :) Seriously though, sorry. I'm sure it's a real PITA.

@brson
Copy link
Contributor

brson commented Mar 31, 2013

@thestinger mentioned that this is a security issue, which I didn't realize. Debian also says not to do this.

@thestinger
Copy link
Contributor

By the way, it's possible to strip this with chrpath -d the rust binaries and libraries but it would be much nicer to not require a third party tool.

@fabiand
Copy link
Contributor Author

fabiand commented Apr 2, 2013

@brson I generally wonder why you don't add a user-specific LD_LIBRARY_PATH to ~/.bashrc to point to the user specific rustpkg dir instead of using rpath. That makes it much easier to cope with - and that would also be in par with (at least) the Fedora guidelines. And I suppose with Debian's too, as it's the common way. (See 3rd party java binaries. They modify the .bashrc to point to some hidden dir in the userdir.)

@graydon
Copy link
Contributor

graydon commented Apr 2, 2013

We used to. Users kept forgetting and/or getting annoyed having to adjust it when running from a build dir. Rpaths made it possible to run foo/bar/rustc no matter which rustc you chose.

@fabiand
Copy link
Contributor Author

fabiand commented Apr 2, 2013

@graydon right. IIUIC - I always feel like a noob when it comes to autotools - autotools are generating wrappers for binaries which are created during a build process and moves the original binaries to hiddend irectories. The wrappers are then modifying the environment (setting env vars etc) to achieve the same goal (so e.g. that rustc can be run from the dev directory without messing with the env vars yourself).

I wonder if this could help.

@fabiand
Copy link
Contributor Author

fabiand commented Apr 2, 2013

Okay, it's actually not autools which creates these wrappers but libtool: http://www.flameeyes.eu/autotools-mythbuster/libtool/wrappers.html

@graydon
Copy link
Contributor

graydon commented Apr 2, 2013

Sorry, I didn't mean to imply a disinterest in fixing this better, just noting how it came to be.

Also strongly disinterested in using libtool. Would prefer going back to telling users to set env vars.

@thestinger
Copy link
Contributor

@graydon: How about defaulting to static linking (when it works)? Dynamic linking is valuable, but mostly for global installs handled by a package manager. Of course, if we end up with a bunch of tools using librustc that would be wasteful.

I think RPATH will just lead to confusion, and it's an easy security hole to leave open.

@graydon
Copy link
Contributor

graydon commented Apr 2, 2013

I would be very happy to get static linking working well enough to make it a default in many such cases. But we ought to do the right thing when linking dynamically too. Is rpath-using-$ORIGIN considered acceptable? I.e. is it just the absolute paths that are problematic?

@thestinger
Copy link
Contributor

The absolute paths are the ones that can easily be a security issue, so $ORIGIN isn't that bad. I would still rather avoid it for a globally installed package but it's easy enough to strip for people who don't need it and won't be an issue if you don't know about it (like the absolute path could be).

Using the $ORIGIN rpath could be a ./configure flag, but the absolute path should never be there.

@pnkfelix
Copy link
Member

Nominating for milestone 2: backwards compatible. (The choice of whether one needs to put a setting in LD_LIBRARY_PATH or use some configure flag for certain systems, etc, seems like a backwards compatibility issue. Though maybe I misunderstand and these are issues only for people developing rustc itself, not for clients of rustc?)

It also could well be that this is more appropriately assocated with milestone 1: well-defined. I suspect @catamorphism and @graydon have a more concrete notion about this.

@graydon
Copy link
Contributor

graydon commented Aug 1, 2013

I don't have anything much to add to this despite it coming up for triage today. I think it has to be fixed still, I'm still happy to do it via whatever combination of static linking by default and use of $ORIGIN is sufficient for packagers, and I haven't seen any clear movement on it in the meantime.

@graydon
Copy link
Contributor

graydon commented Aug 8, 2013

accepted for backwards-compatible milestone

@brson
Copy link
Contributor

brson commented Jan 18, 2014

I'm getting worried that we won't resolve this for 1.0. I agree this is important to fix. Here's my current suggestion:

  • Modify the build system to once again set LD_LIBRARY_PATH/PATH to find the appropriate dynamic libraries.
  • Add a flag to rustc to disable rpath
  • Add a configure flag to disable rpathing the rust build itself. Packagers can use this to do whatever they want.

With this logic the rust package itself could avoid rpath where distros have their own way of dealing with library paths. The rustc installed by these distros would still be rpathing user's crates for the sake of convenience. Is this an acceptable compromise or do distros want us to completely disable rpath? I'm afraid that doing so would provide an inconsistent developer experience depending on how rustc is acquired; if we need to do that then we instead must find a way to completely eliminate rpath.

@brson
Copy link
Contributor

brson commented Jan 18, 2014

If we start to just statically link everything by default then rpath becomes much less important and we can just say it's up to users to figure it out if they opt into dynamic linking.

@thestinger
Copy link
Contributor

@brson: I think we should completely remove absolute rpaths. It's okay to have $ORIGIN-based ones but it may not be a good default. I'm not sure what the answer is to that, but agree that static linking basically resolves this.

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jan 23, 2014
By default, the compiler and libraries are all still built with rpaths, but this
can be opted out of with --disable-rpath to ./configure or --no-rpath to rustc.

cc rust-lang#5219
This was referenced Jan 23, 2014
bors added a commit that referenced this issue Jan 23, 2014
By default, the compiler and libraries are all still built with rpaths, but this
can be opted out of with --disable-rpath to ./configure or --no-rpath to rustc.

Closes #5219
bors added a commit that referenced this issue Jan 23, 2014
By default, the compiler and libraries are all still built with rpaths, but this
can be opted out of with --disable-rpath to ./configure or --no-rpath to rustc.

Closes #5219
alexcrichton added a commit to alexcrichton/rust that referenced this issue Jan 24, 2014
By default, the compiler and libraries are all still built with rpaths, but this
can be opted out of with --disable-rpath to ./configure or --no-rpath to rustc.

cc rust-lang#5219
@bors bors closed this as completed in a1d9d9e Jan 24, 2014
@thestinger
Copy link
Contributor

It doesn't appear that this is fully working. There are still rpaths in the binaries when configured with --disable-rpath even though the --no-rpath flag for rustc does work.

/usr/bin/rustc: RPATH=$ORIGIN/../lib:/build/rust-git/src/rust/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib:/usr/lib/rustlib/x86_64-unknown-linux-gnu/lib

@thestinger thestinger reopened this Jan 25, 2014
@alexcrichton
Copy link
Member

What steps did you follow to having the rpath show up? I thought I double checked on linux that the configure flag worked, but I may have overlooked something.

@thestinger
Copy link
Contributor

All I can tell you is that it still has rpaths when building with --disable-rpath passed to configure.

nathanielherman pushed a commit to nathanielherman/rust that referenced this issue Jan 30, 2014
By default, the compiler and libraries are all still built with rpaths, but this
can be opted out of with --disable-rpath to ./configure or --no-rpath to rustc.

cc rust-lang#5219
oli-obk pushed a commit to oli-obk/rust that referenced this issue May 2, 2020
If let else mutex

changelog: Adds lint to catch incorrect use of `Mutex::lock` in `if let` expressions with lock calls in any of the blocks.

closes: rust-lang#5219
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
Projects
None yet
8 participants