rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.79k stars 2.42k forks source link

`cargo run` exports bad DYLD_LIBRARY_PATH on macOS #3366

Closed casey closed 7 years ago

casey commented 7 years ago

I've been going down the rabbit hole for a while on this one. Thankfully though, it seems like it's actually hopefully a pretty simple fix. Or, if it isn't, at least maybe I'm reporting the issue in the right repo ^_^

I originally thought it was a git2-rs issue, and reported it in https://github.com/alexcrichton/git2-rs/issues/175. Then I thought it was a rustc issue, and reported it here: https://github.com/rust-lang/rust/issues/36250.

Finally though, I've gotten to the bottom of it, and it appears to be a cargo bug.

A few things need to happen to trigger the issue:

  1. We are linking ina dynamic library that is not provided by the system, for example something from homebrew in /usr/local/lib, or from macports in /opt/local/lib.

  2. We are also linking in a system framework.

  3. In the nonstandard location in 1., there is a dynamically linked library which the system framework from 2. needs. However, this library in 1. is not the version that the framework expects.

  4. We run the binary with cargo run.

To make everything concrete, let's say that we're linking in openssl provided by macports in /opt/local/lib, the framework we're using is the ApplicationServices framework, and we also happen to have libiconv installed in /opt/local/lib.

Cargo will export DYLD_LIBRARY_PATH=/opt/local/lib:..., and we'll get the following error:

: cargo run
   Compiling rust-dynamic-linker-error v0.0.0 (file:///Users/rodarmor/src/rust-dynamic-linker-error)
    Finished debug [unoptimized + debuginfo] target(s) in 0.34 secs
     Running `target/debug/rust-dynamic-linker-error`
dyld: Symbol not found: _iconv
  Referenced from: /System/Library/PrivateFrameworks/LanguageModeling.framework/Versions/A/LanguageModeling
  Expected in: /opt/local/lib/libiconv.2.dylib
 in /System/Library/PrivateFrameworks/LanguageModeling.framework/Versions/A/LanguageModeling

What's happening is that the LanguageModeling framework, which is a transitive dependency of the ApplicationServices framework also depends on libiconv. Normally it finds it in /usr/lib/libiconv.dylib. However, because of the value of DYLD_LIBRARY_PATH, it finds it in /usr/local/lib/libiconv.dylib, and we get the error because it is the wrong version with a missing symbol.

If we run the binary directly, without the environment variable, it works:

: ./target/debug/rust-dynamic-linker-error
Hello, world!

Although the conditions needed to trigger this bug may seem esoteric, they are actually very common on mac development machines. Developers often have a ton of stuff installed via macports or homebrew.

There are a few ways that this might be resolved:

The DYLD_LIBRARY_PATH value seems to be unnecessary at least in this case, since the binary works when run directly. If so, perhaps we can avoid exporting it so as not to cause this breakage? However, perhaps there are other situations where the variable is necessary to find libraries? What's the purpose of the export?

If the DYLD_LIBRARY_PATH value is necessary, then we need to communicate that to developers somehow. It would be very frustrating to have a binary which runs with cargo run, but which cannot be run directly.

And, if we do need DYLD_LIBRARY_PATH, then we should consider if DYLD_FALLBACK_LIBRARY_PATH might suffice. DYLD_FALLBACK_LIBRARY_PATH does not disrupt the normal order of dynamic library lookup, but instead is only consulted if the dynamic linker cannot find a library in the normal places.

alexcrichton commented 7 years ago

This is a pretty tricky issue but I think the solution is that we should probably just not try to add any "system directories" to DYLD_LIBRARY_PATH. We should basically just assume that any directory outside the build directory is up to the user to be in DYLD_LIBRARY_PATH as it's not generated by Cargo itself.

@casey would you be willing to help test out a patch for this? I think the relevant section of code is here and we likely just want to filter out paths that aren't in the build directory.

casey commented 7 years ago

Sure, I'd be happy to.

One thing I'm curious about, when is DYLD_LIBRARY_PATH necessary for a binary to work? All my binaries seem to run fine without it.

alexcrichton commented 7 years ago

It's sometimes needed for plugins but it's primarily used for build scripts which generate dynamic native libraries.

pkgw commented 7 years ago

I've run into the same problem with cargo test (example) and am happy to test, or to try to come up with a patch that implements the logic to filter out paths outside of the build directory.

alexcrichton commented 7 years ago

@pkgw ok! If you need any help just let me know.

I think the basic patch here is to just only append paths that look like they're in the cargo build directory, and ignore others.

pkgw commented 7 years ago

OK, I've create a pull request, #3651, that applies this strategy. I was even able to test it and it works in my test case!

However, I have to admit that I mostly forget what's even going on in this code, so I'm far from confident that it's doing the right thing. Fortunately it's only a few lines' worth of changes.

My test case:

  1. Get a Mac with Homebrew
  2. Install at least libjpeg and libpng using Homebrew.
  3. Create a Cargo project with contents as follows.
  4. For Cargo.toml:
[package]
name = "issue3366"
version = "0.0.1"
authors = ["Peter Williams <peter@newton.cx>"]
build = "build.rs"

[lib]
name = "mytest"
crate-type = ["rlib"]

[build-dependencies]
gcc = "^0.3"
  1. For build.rs:
extern crate gcc;

fn main() {
  let mut cfg = gcc::Config::new();
  cfg.file("blah.c");
  println!("cargo:rustc-link-lib=framework=ImageIO");
  println!("cargo:rustc-link-lib=dylib=jpeg");
  println!("cargo:rustc-link-search=/usr/local/lib");
  cfg.compile("libfoo.a");
}
  1. For blah.c:

    int myfunction (void) { return 3; } 
  2. For src/lib.rs:

    pub fn hello() -> usize { 3 }
  3. Run cargo test. With the buggy version, I get a dyld: Symbol not found __cg_jpeg_resync_to_restart error. With the patched version, it just works.

However, I just ran the test suite and the patch definitely breaks the run_with_library_paths test. If the discussion in this issue is correct, the test should just be deleted, I think.

sgrif commented 7 years ago

Oh hey, I was updating the README for pq-sys to have more information about this and noticed I couldn't reproduce the issue anymore. Just wanted to confirm that #3651 did in fact fix a long standing issue for us.

sgrif commented 7 years ago

I do agree with the assessment that setting DYLD_FALLBACK_LIBRARY_PATH is the right thing to do. Here's another case that can trigger the problem on macOS. Many people on Mac install PostgreSQL using Postgres.app. That program places libpq.dylib in a subdirectory of the executable, and also vendors libjpeg.dylib. If we link against libpq when someone has installed it using that method, that directory will be added to DYLD_LIBRARY_PATH, and the same issue will occur with dyld: Symbol not found __cg_jpeg_resync_to_restart

alexcrichton commented 7 years ago

@sgrif thanks for confirming the fix! I'd also be totally up for leveraging DYLD_FALLBACK_LIBRARY_PATH. It's unfortunate that it's OSX-specific, but hey if it fixes a platform it fixes a platform.

alexcrichton commented 7 years ago

I believe this was fixed in https://github.com/rust-lang/cargo/pull/3651

ns-cweber commented 5 years ago

I'm seeing this on stable (cargo 1.31.0 (339d9f9c8 2018-11-16)) and nightly (cargo 1.32.0-nightly (5e85ba14a 2018-12-02)) when I try to cargo run immediately after cargo new --bin hello. Not sure why it needs libJPEG...

dyld: Symbol not found: __cg_jpeg_resync_to_restart
  Referenced from: /System/Library/Frameworks/ImageIO.framework/Versions/A/ImageIO
  Expected in: /usr/local/lib/libJPEG.dylib

...

Caused by:
  process didn't exit successfully: `rustc --edition=2018 --crate-name mycli src/main.rs --color always --crate-type bin --emit=dep-info,link -C debuginfo=2 -C metadata=d508f6b37c4f5b6c -C extra-filename=-d508f6b37c4f5b6c --out-dir /Users/cweber/temp/rust-cli/mycli/target/debug/deps -C incremental=/Users/cweber/temp/rust-cli/mycli/target/debug/incremental -L dependency=/Users/cweber/temp/rust-cli/mycli/target/debug/deps` (signal: 6, SIGABRT: process abort signal)
aredier commented 5 years ago

Hi, new to rust and having the same problem (downloading everything via rustup.sh and then cargo new helloWorld, cargo build throws the same error (verbose version):


Compiling helloWorld v0.1.0 (/.../helloWorld)
     Running `rustc --edition=2018 --crate-name helloWorld src/main.rs --color always --crate-type bin --emit=dep-info,link -C debuginfo=2 -C metadata=be4ec7492c887d5d -C extra-filename=-be4ec7492c887d5d --out-dir /.../helloWorld/target/debug/deps -C incremental=/.../helloWorld/target/debug/incremental -L dependency=/.../helloWorld/target/debug/deps`
dyld: Symbol not found: __cg_jpeg_resync_to_restart
  Referenced from: /System/Library/Frameworks/ImageIO.framework/Versions/A/ImageIO
  Expected in: /usr/local/lib/libJPEG.dylib
 in /System/Library/Frameworks/ImageIO.framework/Versions/A/ImageIO
error: Could not compile `helloWorld`.

Caused by:
  process didn't exit successfully: `rustc --edition=2018 --crate-name helloWorld src/main.rs --color always --crate-type bin --emit=dep-info,link -C debuginfo=2 -C metadata=be4ec7492c887d5d -C extra-filename=-be4ec7492c887d5d --out-dir /.../helloWorld/target/debug/deps -C incremental=/.../helloWorld/target/debug/incremental -L dependency=/.../helloWorld/target/debug/deps` (signal: 6, SIGABRT: process abort signal)
ehuss commented 5 years ago

@ns-cweber or @aredier do you know what installed /usr/local/bin/libJPEG.dylib? (Are you using homebrew or macports?) Which version of MacOS are you using? Which version of Xcode do you have? Can you include the output from these commands?

xcode-select  -p
ld -v
cc -v

And maybe try running this inside your hello-world project:

`rustup which cargo` run

I'm wondering if this might get fixed by #6355. I'm unable to reproduce, so I'd like to get to the bottom of it (this has been reported a few times — rust-lang/rust#34869 rust-lang/rust#36250). EDIT: Just FYI, jpeg comes in from a long dependency chain Security → Foundation → CoreServices → QuartzCore →... → ImageIO. I'm a little confused how /usr/local/lib is getting picked up first. Neither cargo nor rustup should be adding that (without any build scripts).

aredier commented 5 years ago

@ehuss It seemed I used the wrong version of rustup (curl -sf -L https://static.rust-lang.org/rustup.sh | sh) which was one of the first link when I searched for "install Rust" ( https://doc.rust-lang.org/1.0.0/book/installing-rust.html). but installing rustup via curl https://sh.rustup.rs -sSf | sh solved the problem. Sorry about that.

however if you still want the info on my system:

libjpeg was installed as a dependency by brew on my system (MacOS 10.14.1). xcode-select -p gave (when still having a problem): /Library/Developer/CommandLineTools ld -v:

@(#)PROGRAM:ld  PROJECT:ld64-409.12
BUILD 17:47:51 Sep 25 2018
configured to support archs: armv6 armv7 armv7s arm64 i386 x86_64 x86_64h armv6m armv7k armv7m armv7em
LTO support using: LLVM version 10.0.0, (clang-1000.10.44.4) (static support for 21, runtime is 21)
TAPI support using: Apple TAPI version 10.0.0 (tapi-1000.10.8)

and cc -v gives me

Apple LLVM version 10.0.0 (clang-1000.10.44.4)
Target: x86_64-apple-darwin18.2.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin
Found CUDA installation: /usr/local/cuda, version 9.0

as for

`rustup which cargo` run

it gave a command not found

dwijnand commented 5 years ago

We just fixed that URL yesterday, btw: https://github.com/rust-lang/cargo/pull/6455

ehuss commented 5 years ago

See also https://github.com/rust-lang/rustup.rs/issues/1507

@alexcrichton would it be possible to add a redirect on https://static.rust-lang.org/rustup.sh to https://sh.rustup.rs? It has an ancient 3 year old version from https://github.com/rust-lang-deprecated/rustup.sh. Considering some documentation (like Cargo) still points at that old version (links in TRPL up to 1.13 link to the old version — https://doc.rust-lang.org/1.13.0/book/getting-started.html — fixed in rust-lang/rust#38122).

alexcrichton commented 5 years ago

An excellent idea, done now!

ehuss commented 5 years ago

Hm, maybe that wasn't the best idea. curl without -L does nothing and prints nothing.

curl -sSf https://static.rust-lang.org/rustup.sh

If it can't point directly at the up-to-date file, maybe replace it with something like this:

#!/bin/bash
echo "The location of rustup.sh has moved."
echo "Run the following command to install from the new location:"
echo "    curl https://sh.rustup.rs -sSf | sh"

(or whatever wording sounds good)

alexcrichton commented 5 years ago

Yeah it's true that without -L it doesn't do anything, but I didn't see some examples that didn't have the -L flag?

ehuss commented 5 years ago

The old book had -L, but cargo does not. None of the modern documentation uses it either (book, rustup).

alexcrichton commented 5 years ago

Oh for sure sh.rustup.rs with a HTTP redirect would break everyone so I'm not too worried about that, it looks like the main one is the Cargo documentation which doesn't pass -L? That's a bummer :(

I think I've figured out a good middle-ground though, the HTTP redirect response body now contains the shell script you recommended, so if -L is passed it "just works", but if it's not passed you get a message

dwijnand commented 5 years ago

Genius.

benw commented 5 years ago

@alexcrichton Just a suggestion, to add exit 1 to the end of the response body. That will help out anyone running this from a script.

Lord-Kamina commented 5 years ago

Hi; just came here to say this is still very much an issue. I am trying to port rust/cargo to mxe (I'm on MacOS) and am seeing the exact same issue with ImageIO.framework looking for the wrong libJPEG.dylib. I don't use Homebrew but rather MacPorts. Manually replacing instances of DYLD_LIBRARY_PATH to DYLD_FALLBACK_LIBRARY_PATH fixes it but then cargo fails elsewhere due to the replaces modifying the checksum of some files.

Really, DYLD_LIBRARY_PATH shouldn't be used essentially ever, since it messes with default loading paths. If necessary, you should switch to DYLD_FALLBACK_LIBRARY_PATH instead.

There must be some way to get around this (because MacPorts itself doesn't have this issue when building Rust) but I haven't yet found the way to do it.

Sent with GitHawk

ehuss commented 5 years ago

@Lord-Kamina DYLD_LIBRARY_PATH is no longer used in Cargo since 1.33 and rustup since rustup 1.18.0. If you are having issues with other tools, like the rust-lang build system, I would suggest looking there.

Lord-Kamina commented 5 years ago

@ehuss I am not using rustup; I am trying to build and seeing these issues with rust 1.36 and cargo 0.37 (downloaded from https://static.rust-lang.org/dist/2019-07-04), running grep -R DYLD_LIBRARY_PATH./* on the sources should confirm what I'm saying.

EDIT: Ignore me, you're correct; cargo at least is correctly setting only DYLD_LIBRARY_FALLBACK_PATH

Sent with GitHawk