Skip to content

[bootstrap] Don't reverse the order of dylib search path entries - #161377

Open
jnkel wants to merge 1 commit into
rust-lang:mainfrom
jnkel:fix-dylib-search-order
Open

[bootstrap] Don't reverse the order of dylib search path entries#161377
jnkel wants to merge 1 commit into
rust-lang:mainfrom
jnkel:fix-dylib-search-order

Conversation

@jnkel

@jnkel jnkel commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

The add_dylib_path helper function prepends paths to the beginning of the dynamic linker search path, but it reverses their order while doing so. This is surprising and undocumented, and seems to be unexpected by several callers of this function.

Particularly, rustc_lib_paths appends the ci-llvm path to the list it returns; reversing the order puts ci-llvm at a higher priority than the compiler's lib directory. On my system, this currently causes ./x test to fail (when building the unstable book), because the stage0 compiler is run using CI LLVM instead of stage0 LLVM (which are currently different because stage0 is on LLVM 22 while main is on LLVM 23).

Might be worth a try build as this change could potentially cause issues if there is somewhere we depend on this ordering reversal. I checked all the call-sites (and ran ./x test locally) and I don't think anyone intentionally relied on the ordering being reversed. I did find one snippet that concerned me (from #144303, cc @Kobzol):

// The `cargo` command configured above has dylib dir path set to the `build_compiler`'s
// libdir. That causes issues in cargo test, because the programs that cargo compiles are
// incorrectly picking that libdir, even though they should be picking the
// `tested_compiler`'s libdir. We thus have to override the precedence here.
let mut existing_dylib_paths = cargo
.get_envs()
.find(|(k, _)| *k == OsStr::new(dylib_path_var()))
.and_then(|(_, v)| v)
.map(|value| split_paths(value).collect::<Vec<PathBuf>>())
.unwrap_or_default();
existing_dylib_paths.insert(0, builder.rustc_libdir(tested_compiler));
add_dylib_path(existing_dylib_paths, &mut cargo);

The comment states we're inserting builder.rustc_libdir(tested_compiler) at the highest priority in the search path, but because add_dylib_path reversed the ordering, it's actually inserted at the lowest priority. I don't know how to reproduce the issue this was supposed to fix, so I can't be sure that this PR doesn't cause a regression.

Follow-up to #161335. r? @jieyouxu

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 19, 2026
@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Aug 19, 2026
@rust-log-analyzer

This comment has been minimized.

@jnkel
jnkel force-pushed the fix-dylib-search-order branch from 02b6017 to 67e8a19 Compare August 19, 2026 23:44
Comment on lines -145 to -148
for path in path {
list.insert(0, path);
}
cmd.env(dylib_path_var(), t!(env::join_paths(list)));

@jieyouxu jieyouxu Aug 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remark: I remember this was for a specific reason (or two), need to double-check

View changes since the review

@Kobzol

Kobzol commented Aug 20, 2026

Copy link
Copy Markdown
Member

The code in add_dylib is 11 years old and comes from bootstrap's first Rust commit. I agree that what you describe makes more sense.

Also, this is such a mess.. the ci-llvm directory really shouldn't be added in this function, that's super ad-hoc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants