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 -L arguments given to linker always have the same order as the rustc -L arguments #16904

Merged
merged 2 commits into from
Sep 7, 2014

Conversation

inrustwetrust
Copy link
Contributor

Issue can be reproduced by the following:

$ cat main.rs
fn main() {}
$ rustc -Z print-link-args -Lfoo -Lbar main.rs

Run the rustc command a few times and observe that the order of the '-L' 'foo' '-L' 'bar' options randomly changes.

Actually hit this issue in practice on Windows when specifying two -L directories to rustc, one with rust-sdl2 in it and one with the C SDL2.dll. Since Windows file systems aren't case-sensitive, gcc randomly attempted to link against the rust sdl2.dll instead of SDL2.dll if that -L directory happened to come first.

The randomness was due to addl_lib_search_paths being a HashSet. Changed it to a Vec instead which maintains the ordering.
Unsure how to test this though since it is random by nature; suggestions very welcome.

@alexcrichton
Copy link
Member

Sorry for being a little slow to get to this, but this looks great!

It would be very nice to have a test for a change such as this, and the easiest one that I can think of is to generate libfoo.a into two separate directories, where each copy of libfoo.a has the same symbol (foo, for example). One library returns 1 while the other returns 2, and then you pass the appropriate -L paths and make sure the right one is picked up.

This would belong as a run-make test in src/test/run-make, and there are a number of other examples to draw from, but if you have any trouble feel free to just ping the PR or me directly!

@inrustwetrust
Copy link
Contributor Author

Thanks for the pointers. Added test under src/test/run-make/link-path-order.

@inrustwetrust
Copy link
Contributor Author

@alexcrichton: Hmm, that build failure can't be related to this patch, or?

@alexcrichton
Copy link
Member

LLVM assertions are likely to reproduce though, sadly. Have you ensured that this builds on a 64-bit platform?

This makes the extra library paths given to the gcc linker come in
the same order as the -L options on the rustc command line.
@inrustwetrust
Copy link
Contributor Author

Oops, sorry. Didn't realize the new commits would show up automatically in the pull request as soon as I pushed them. The original patch was built on 64-bit Ubuntu and that LLVM assertion failure didn't show up then. Rebuilding after rebase now to see if I can reproduce it against the current trunk code.

@inrustwetrust
Copy link
Contributor Author

Hmm, cannot reproduce this issue on 64-bit Ubuntu. I don't have a Mac available to test on, I'm afraid.
@alexcrichton: Any ideas on how to proceed here?

@alexcrichton
Copy link
Member

Let's try again and see what happens!

bors added a commit that referenced this pull request Sep 7, 2014
…hton

Issue can be reproduced by the following:
```
$ cat main.rs
fn main() {}
$ rustc -Z print-link-args -Lfoo -Lbar main.rs
```
Run the rustc command a few times and observe that the order of the '-L' 'foo' '-L' 'bar' options randomly changes.

Actually hit this issue in practice on Windows when specifying two -L directories to rustc, one with rust-sdl2 in it and one with the C SDL2.dll. Since Windows file systems aren't case-sensitive, gcc randomly attempted to link against the rust sdl2.dll instead of SDL2.dll if that -L directory happened to come first.

The randomness was due to addl_lib_search_paths being a HashSet. Changed it to a Vec instead which maintains the ordering.
Unsure how to test this though since it is random by nature; suggestions very welcome.
@bors bors closed this Sep 7, 2014
@bors bors merged commit e7a000e into rust-lang:master Sep 7, 2014
@inrustwetrust inrustwetrust deleted the link-path-order branch September 7, 2014 21:08
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 31, 2024
internal: Bump rust-cache action

Fixes a Node 16 deprecation warning and also pulls in Swatinem/rust-cache#147, which sounds interesting.
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.

3 participants