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

Prioritize target libs when there's multiple candidates #14650

Closed
wants to merge 1 commit into from

Conversation

lilyball
Copy link
Contributor

@lilyball lilyball commented Jun 4, 2014

When multiple candidates for a library are found, if one (and only one)
candidate lives in the target library path, prioritize that one. This
allows rustc -L /usr/local/lib lib.rs to compile successfully, whereas
today it complains about multiple candidates for e.g. libstd.

Fixes #13733, #11195.

When multiple candidates for a library are found, if one (and only one)
candidate lives in the target library path, prioritize that one. This
allows `rustc -L /usr/local/lib lib.rs` to compile successfully, whereas
today it complains about multiple candidates for e.g. libstd.

Fixes rust-lang#13733, rust-lang#11195.
@japaric
Copy link
Member

japaric commented Jun 4, 2014

Would this also fix #13421? I'll test it.

@lilyball
Copy link
Contributor Author

lilyball commented Jun 4, 2014

@japaric Probably. I'm assuming setting the LLVM root like that is adding /usr/lib64 as a search path for rustc, and this PR should fix that.

@alexcrichton
Copy link
Member

A fact of bootstrapping is that the host libs have a chance of being different than the target libs, and the compiler is presenting to you a legitimate warning, as I stated in my earlier comment.

I agree that this is a problem that needs to be fixed, but I do not believe this is the right way to do it. The solution I am thinking of is to change our distribution strategy to not use the stage2 host libs at all, but rather only install the stage2 target libs.

@lilyball
Copy link
Contributor Author

lilyball commented Jun 4, 2014

@alexcrichton If we don't install the host libs, how do people compile syntax extensions?

@lilyball
Copy link
Contributor Author

lilyball commented Jun 4, 2014

@alexcrichton The legitimate warning assumes both libraries are valid candidates. This PR here makes the assumption that if a given library is found in the target lib path, then it's the correct one to use and any alternative matching candidates can be ignored. This assumes, of course, that the target lib path is only searched when building target libraries, which seems like a safe assumption (if I'm building a syntax extension dylib, I would not expect the target lib path to be searched, because I only want to link against host libs).

@alexcrichton
Copy link
Member

Just to be clear, I'll explain how I see this situation. The names "host lib" and "target lib" are misnomers here because this issue doesn't stem from cross compiling or host/targets at all. This issue is purely a staging problem.

The stage1 compiler, $target/stage1/bin/rustc will build a set of libraries for $target, located in $target/stage1/lib/rustlib/$target/lib. These libraries are then copied to become the stage2 compiler, located at $target/stage2/bin/rustc. I'll call these the stage1 artifacts. The stage1 artifacts will then be used to build the libraries again, placing them at $target/stage2/lib/rustlib/$target/lib. I'll call these the stage2 artifacts.

In terms of a system installation, we install the stage1 artifacts to /usr/lib and the stage2 artifacts to /usr/lib/rustlib/$target/lib. The stage1 artifacts will only generate binaries which link to stage2 artifacts normally, unless you pass -L /usr/lib on the command line. Note that this has the very important property that stages never link to code generated by another stage. This has in the past been viewed as necessary to bootstrap correctly.

What I was saying is that we stop distributing the stage1 entirely. The stage2 artifacts are the only ones distributed, and they are all placed into /usr/lib. This implies that /usr/lib is always on the library filesearch path.

You'll also note that this means that the stage2 artifacts, the stage3 compiler, will be linking to code that they didn't generate (an important property we had beforehand). Due to our need for syntax extensions, this is something that we already need to guarantee for the compiler to work properly, I believe.


Now that we've got that out of the way, I will address your questions.

If we don't install the host libs, how do people compile syntax extensions?

I think when you say host libs you are referring to the stage1 artifacts. Being able to compile syntax extensions doesn't really have anything to do with being able to find stage1 or stage2 artifacts. By not distributing stage1 artifacts, you will compile a syntax extension against the stage2 artifacts which are the exact libraries that stage2 artifact compiler is running with, so there is no problem here.

This PR here makes the assumption that if a given library is found in the target lib path, then it's the correct one to use and any alternative matching candidates can be ignored.

Yes, but assumptions always turn out to be wrong at some point. What you're trying to "fix" is something that I do not view as a problem to fix, but rather a fact of life with our current distribution and architecture. The compiler should need no changes to be able to pass -L /usr/lib on the command line.

I would not expect the target lib path to be searched, because I only want to link against host libs).

Can you rephrase this statement in terms of what I said above? There is no difference in host/target architecture, and the compiler today never links against the stage1 artifacts.

@japaric
Copy link
Member

japaric commented Jun 4, 2014

FWIW, this didn't fix #13421. I made sure to remove my previous install, and then build and install rust with this patch applied, but when I tried to compile the patched rust (with the patched rustc), it failed with the same "multiple dylib candidates for libc" message.

@lilyball
Copy link
Contributor Author

lilyball commented Jun 4, 2014

@alexcrichton I was under the impression that the $prefix/lib vs $prefix/lib/rustlib/$triple/lib was due to the "host" libraries, used by the compiler, and the "target" libraries, used by the built product. It appears this is incorrect.

I take it that when cross-compiling right now you end up with stage1 artifacts in $prefix/lib, host libraries in $prefix/lib/rustlib/$host/lib, and target libraries in $prefix/lib/rustlib/$target/lib?

Given your explanation, it does seem that the correct thing to do is to stop distributing the stage1 artifacts. Why do we do that right now? Also, is the stage2 rustc actually built from the stage2 libraries, or is the stage2 rustc used to build the stage2 libraries? I thought it was the latter, because I thought the stage0 rustc was the snapshot, which was then used to build the stage0 libraries, which build the stage1 rustc, etc.

Incidentally, I'm not thrilled by the idea of putting all the stage2 artifacts in $prefix/lib. That's a lot of libraries. And this includes libcompiler-rt.a and libmorestack.a, which I would think we don't really want to install into a location that's used for compiling non-Rust things. I would be happier if we kept $prefix/lib/rustlib/$host/lib.

@lilyball
Copy link
Contributor Author

lilyball commented Jun 4, 2014

@japaric Can you run the final rustc build command (that failed) with RUST_LOG=rustc::metadata? That will provide useful info.

@alexcrichton
Copy link
Member

I take it that when cross-compiling right now you end up with stage1 artifacts in $prefix/lib, host libraries in $prefix/lib/rustlib/$host/lib, and target libraries in $prefix/lib/rustlib/$target/lib?

Yes.

Given your explanation, it does seem that the correct thing to do is to stop distributing the stage1 artifacts. Why do we do that right now?

I explained above as well, but it's to rely on the invariant that a compiler will never link code with a library that it did not itself generate. (more on this above)

Note that if we don't install the stage1 artifacts, we are breaking this invariant, and I believe this to be ok.

Also, is the stage2 rustc actually built from the stage2 libraries, or is the stage2 rustc used to build the stage2 libraries?

I'd be careful on naming here, as I'm not precisely sure what you mean by "stage2 rustc". I'll try to explain though.

I refer to the file $target/stageN/bin/rustc as the "stageN compiler". I'm not sure if this is the best name for this, but it's what I call it. The stageN compiler is the rustc executable that is linked against the stage{N-1} artifacts. The snapshot compiler is the same thing as the stage0 compiler.

With that in mind, the stage2 compiler is not built from the stage2 artifacts, but rather the stage1 artifacts. We currently install the stage2 compiler as $prefix/bin, which is additionally the reason why we install the stage1 artifacts as dylibs (so the stage2 compiler can run).

I would be happier if we kept $prefix/lib/rustlib/$host/lib.

morestack/compiler-rt are a good point, and this alternative would involve installing nothing to $prefix/lib other than the rustlib directory and all of its contents.

This will be tricky, however, with dynamic linking. When the rustc executable at $prefix/bin starts to run, it will need to load, for example, librustrt.so. In order to find this library, we currently predominately use a relative rpath which says "you'll find librustrt.so in ../lib/librustrt.so". This rpath statement would no longer be valid if we do not install libraries to $prefix/lib. We could rely on installations to modify LD_LIBRARY_PATH to point to $prefix/lib/rustlib/$host/lib, but that's another deviation from what we do today.

@japaric
Copy link
Member

japaric commented Jun 4, 2014

@kballard Sorry, my bad! My package manager wasn't passing the --enable-local-rust flag to the configure script. This does solve #13421.

@lilyball
Copy link
Contributor Author

lilyball commented Jun 4, 2014

@alexcrichton

I explained above as well, but it's to rely on the invariant that a compiler will never link code with a library that it did not itself generate. (more on this above)

I think I misunderstood. I thought you were talking about how syntax extensions loaded into rustc need to be linking against the same libraries rustc itself links against, but now I realize you are talking about executables/libraries produced by rustc linking against other libraries that were produced by the same rustc.

Maintaining that invariant would require installing both "target" and "host" libraries even when target == host. But even with that, preserving this invariant for syntax extensions is only possible if rustc itself links against libraries that it produced, which a) seems like a bad idea, and b) actually breaks the invariant for the previous stage. Which is to say, this invariant can't possibly hold everywhere.

Note that if we don't install the stage1 artifacts, we are breaking this invariant, and I believe this to be ok.

I agree.

I refer to the file $target/stageN/bin/rustc as the "stageN compiler".

Ok, that's what I thought, but I wasn't entirely certain.

What is stage3 used for? I have a full stage3 tree in my build dir, but there are no files in it.

This will be tricky, however, with dynamic linking.

Why? Can't we just adjust the path to point to the correct rustlib dir?

I don't know how it works on other OS's, but on OS X it appears rustc uses e.g. @rpath/libstd-59beb4f7-0.11.0-pre.dylib. And I see two LC_RPATH load commands, one set to @loader_path/../lib (which resolves to /usr/local/lib) and a second one for /usr/local/lib/rustlib/x86_64-apple-darwin/lib. So it appears that the stage2 artifacts actually are already in the search path for rustc, it just uses the stage1 artifacts because they're found first. So in this case all we have to do is remove the LC_RPATH for @loader_path/../lib and the existing linker paths will continue to work. Or, since I assume that path is actually necessary for running rustc from the build dir, we just don't install the libraries into $prefix/lib and it will fall back to the second LC_RPATH.

If other OS's use different approaches, surely there's a way to deal with that (for example, to rewrite[1] the paths that rustc uses when copying rustc from the stage2 artifacts into its build location).

[1] OS X provides a tool install_name_tool that can do this job, although changing a path to be longer requires an ld flag -headerpad_max_install_names. I don't know how this works on Linux.

@lilyball
Copy link
Contributor Author

lilyball commented Jun 4, 2014

@alexcrichton Incidentally, if we swap those two LC_RPATH commands, then that prioritizes libraries in the rustlib dir, which, coincidentally, makes dyld act the same way this PR makes rustc act.

On that note, something to consider is making rustc explicitly take the first crate it finds, instead of special-casing the target lib dir. It would have to search the target lib dir first (right now it searches additional paths before the target lib dir, although I don't know why), but that shouldn't be a problem.

@alexcrichton
Copy link
Member

What is stage3 used for?

Snapshots are stage3 compilers. I think that's the only use case of stage3

This will be tricky, however, with dynamic linking.

Why? Can't we just adjust the path to point to the correct rustlib dir?

Interesting! Your findings surprise me, but I suppose I should have also seen that coming. I'll digress for a second to talk about rpaths:

The rpath values are passed to the system linker via the -rpath flag. This does not work on windows. This means that at link-time is when the rpath values are settled upon. We used to insert many rpath values, but today we only insert two. One is the relative path from the output path to each library being linked. This is the primary rpath relied upon to have a binary find its libraries. The second is an absolute path to the target's library directory. This is done so when you compile an executable, you can move it around on the system and have it still work.

So, with that in mind, what you discovered was first the relative path, then the absolute path (as expected). Note though that this absolute path is the absolute path with the --prefix that rustc was configured with. This absolute path will not work for installations that don't install to the same --prefix.

If we were to not install libs into $prefix/lib, I think we'd have to go the route of modifying the rpath in the libraries (which I had not considered before!). As a wrench into this, however, windows depends on the stage2 artifacts all being in the same directory, $prefix/bin. Windows does not have rpaths, but it will load a dynamic library that is next to the executable.

On that note, something to consider is making rustc explicitly take the first crate it finds, instead of special-casing the target lib dir.

The extract_one function is operating over a set of crates that all have the same filename hash. This means that they have the same crate ID. As an example, this could mean that you just updated libstd. It has an entirely different ABI, but the same filename. I believe that the compiler is legitimately generating an error informing you that the two libraries are distinct candidates that could both be linked.

@lilyball
Copy link
Contributor Author

lilyball commented Jun 5, 2014

This absolute path will not work for installations that don't install to the same --prefix.

Why would you install to a different location than your --prefix?

Or are you talking right now about compiling third-party libraries and executables that depend on those libraries? If so, I take it you're saying that the prefix rustc was configured with would end up as an LC_RPATH in the third-party executable as well, even though it's not actually where the third-party libraries would get installed to? That seems fine, because if my third-party executable wants to dynamically link against libstd (either directly or because a library it uses does), then it needs that path anyway to find the installed libstd on the system. As for finding the dependent third-party library, it seems we already have that problem today. If my rustc has a prefix of /usr/local, but I install the third-party library into /opt/local, and the executable that uses it into ~/bin, then it won't be able to find the library.

Of course, if we remove the LC_RPATH: @loader_path/../lib entry, then that makes the third-party story worse, but we don't have to remove it. We should swap the order of the two, so the $prefix/rustlib/$target/lib is the first LC_RPATH, and @loader_path/../lib is the second. That way the rust-installed libraries take precedence, but otherwise, third-party libraries in a relative location will still work.

I don't believe we need to modify the rpath in libraries with this approach.

windows depends on the stage2 artifacts all being in the same directory, $prefix/bin

Doesn't that mean we can basically just ignore windows? We just install the stage2 artifacts there instead of the stage1 artifacts. Although how do third-party executables find the rust libraries? Do they also have to be installed into $prefix to work? However it works, I don't think switching from using stage1 artifacts to stage2 artifacts should affect windows in any meaningful way.

I believe that the compiler is legitimately generating an error informing you that the two libraries are distinct candidates that could both be linked.

Two libraries with the same crate ID but different hashes in the same directory seems like a legitimate error.

Two libraries with the same crate ID in two different directories (regardless of hash) is not necessarily an error. If I just installed a new version of libstd with a different ABI, it would presumably have a different hash, and live in the same directory as my old libstd. rustc can then report an error when I try and use it. But the only reason why this might cause a problem when considering different directories is precisely when making the switch to no longer installing stage1 artifacts, if the user passes -L $prefix/lib. But that's resolved by searching the target lib dir before any -L dirs (which means the newer libraries would have precedence over the older stage1 artifact libraries).

@alexcrichton
Copy link
Member

Why would you install to a different location than your --prefix?

The nightly installer is configured with some --prefix, and you can then pass --prefix to the install script to specify an arbitrary location to put it. No, I'm not talking about third-party libraries.

We cannot swap the order because then rust itself could not be built if rust is installed.

Doesn't that mean we can basically just ignore windows?

Not if we can find a solution for both worlds.

Although how do third-party executables find the rust libraries?

Can you define what you mean by "third-party executable"?

I don't think switching from using stage1 artifacts to stage2 artifacts should affect windows in any meaningful way.

Neither do I, what I meant was that our distribution and organization strategy of libraries does affect windows.

Two libraries with the same crate ID in two different directories (regardless of hash) is not necessarily an error

This paragraph doesn't make sense to me, sorry. I'm assuming that we should remove stage1 artifacts, in which case the associated bug is fixed and this PR is not necessary.

@lilyball
Copy link
Contributor Author

lilyball commented Jun 5, 2014

The nightly installer is configured with some --prefix, and you can then pass --prefix to the install script to specify an arbitrary location to put it. No, I'm not talking about third-party libraries.

Huh, ok.

We cannot swap the order because then rust itself could not be built if rust is installed.

I see. Well, if we stop installing stage1 artifacts, then it doesn't really matter, except we'll need to teach the Makefile and nightly installer script to delete the old installed artifacts (as they won't be overwritten with new ones).

Not if we can find a solution for both worlds.

I didn't mean don't make this work on Windows, I mean, that it sounds like Windows isn't really affected by this change.

Can you define what you mean by "third-party executable"?

Executables built by users of rust, as opposed to the executables built when building rust itself (e.g. rustc, rustdoc).

Neither do I, what I meant was that our distribution and organization strategy of libraries does affect windows.

It sounds to me like Windows does not have an issue that's equivalent to the -L/usr/local/lib issue. Replacing stage1 artifacts with stage2 artifacts when installing should not affect anything as far as I can tell.

This paragraph doesn't make sense to me, sorry. I'm assuming that we should remove stage1 artifacts, in which case the associated bug is fixed and this PR is not necessary.

If we remove the stage1 artifacts, then the precise issue that prompted this PR is fixed, and yes, I suppose we can close the PR. But it's entirely possible for this issue to crop up again in the future with third-party libraries, just not I guess with the Rust standard libraries (e.g. with multiple -L flags and the same library installed in multiple places). If that crops up, we can deal with that then.

@alexcrichton
Copy link
Member

But it's entirely possible for this issue to crop up again in the future with third-party libraries, just not I guess with the Rust standard libraries (e.g. with multiple -L flags and the same library installed in multiple places).

If the libraries that are located have the exact same SVH, the compiler can safely choose either one because they should both have the same ABI in that case (if they were exactly duplicates).

Ok, in that case, I'm going to close this for now, and open an issue about our distribution tactics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot link to third-party Rust libraries in the same prefix rustc was installed to
3 participants