rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
94.88k stars 12.23k forks source link

posix_spawn_file_actions_addchdir_np lookup doesn't work on linux musl #99740

Open sunshowers opened 1 year ago

sunshowers commented 1 year ago

I tried this code:

use std::process::Command;

let mut cmd = Command::new("ls");
cmd.current_dir(".");
cmd.output().unwrap();

(compiled under x86_64-unknown-linux-musl)

I expected to see this happen:

The posix_spawn fast path is used.

Instead, this happened:

Rust fell back to the fork/exec slow path. (Inspected this using gdb and strace).

This is presumably because https://github.com/rust-lang/rust/blob/6dbae3ad19309bb541d9e76638e6aa4b5449f29a/library/std/src/sys/unix/process/process_unix.rs#L442-L447 does a dynamic symbol lookup but musl causes binaries to be statically linked.

Meta

rustc --version --verbose:

rustc 1.64.0-nightly (93ffde6f0 2022-07-23)
binary: rustc
commit-hash: 93ffde6f04d3d24327a4e17a2a2bf4f63c246235
commit-date: 2022-07-23
host: x86_64-unknown-linux-gnu
release: 1.64.0-nightly
LLVM version: 14.0.6
Backtrace

``` ```

cuviper commented 1 year ago

This is presumably because (use of weak!) does a dynamic symbol lookup

That used to call dlsym, but it has used #[linkage = "extern_weak"] since #90846, released in Rust 1.59.0.

I do see posix_spawn_file_actions_addchdir_np in x86_64-unknown-linux-musl/lib/self-contained/libc.a, and the corresponding weak symbol reference in its libstd-*.rlib, so I don't know why that isn't working.

sunshowers commented 1 year ago

Huh, thanks for looking at this. That's really weird then.

12101111 commented 1 year ago

I try this rust code and c code on musl and glibc linux system.

#![feature(linkage)]
use std::ffi::c_void;
extern "C" {
    #[linkage = "extern_weak"]
    static posix_spawn_file_actions_addchdir_np: *const c_void;
}
fn main() {
    println!("{}", unsafe {
        posix_spawn_file_actions_addchdir_np.is_null()
    });
}

musl:

> export RUSTFLAGS="-Ctarget-feature=-crt-static"
> cargo run
   Compiling test-weak-linkage v0.1.0 (/tmp/test-weak-linkage)
    Finished dev [unoptimized + debuginfo] target(s) in 0.11s
     Running `/var/tmp/rust/debug/test-weak-linkage`
false
> export RUSTFLAGS="-Ctarget-feature=+crt-static"
> cargo run
   Compiling test-weak-linkage v0.1.0 (/tmp/test-weak-linkage)
    Finished dev [unoptimized + debuginfo] target(s) in 0.12s
     Running `/var/tmp/rust/debug/test-weak-linkage`
true

glibc:

> export RUSTFLAGS="-Ctarget-feature=+crt-static"
> cargo run
   Compiling test-weak-linkage v0.1.0 (/tmp/test-weak-linkage)
    Finished dev [unoptimized + debuginfo] target(s) in 0.85s
     Running `target/debug/test-weak-linkage`
true
> export RUSTFLAGS="-Ctarget-feature=-crt-static"
> cargo run
   Compiling test-weak-linkage v0.1.0 (/tmp/test-weak-linkage)
    Finished dev [unoptimized + debuginfo] target(s) in 0.48s
     Running `target/debug/test-weak-linkage`
false
#include <stdio.h>
__attribute__((weak)) extern void *posix_spawn_file_actions_addchdir_np;

int main() {
    printf("%p\n", &posix_spawn_file_actions_addchdir_np);
}

musl:

> clang test.c && ./a.out
0x7fe282cb2800
> clang test.c -static && ./a.out
0

glibc:

> gcc test.c && ./a.out
0x7f7875c05550
> gcc test.c -static && ./a.out
(nil)

A blog https://maskray.me/blog/2021-04-25-weak-symbol say:

An undefined weak symbol does not extract archive members

So use weak symbol and checking null is not working in this case. However the old dlsym way also don't work with musl when the elf is statically linked . Maybe a compile-time check by rustc is needed when target-feature=+crt-static

cuviper commented 1 year ago

Hmm. Maybe instead we could add a defined weak function that returns ENOSYS, and then libc's global definition should override that if present.

sunshowers commented 1 year ago

If rustc bundles musl, can it just rely on the fact that it ships with this method?

cuviper commented 1 year ago

Well, in theory musl is supposed to allow external sysroots too, but it currently uses the self-contained mode for all crt-static builds. If this FIXME is addressed, then we can't just rely on what we ship.

https://github.com/rust-lang/rust/blob/0b79f758c9aa6646606662a6d623a0752286cd17/compiler/rustc_codegen_ssa/src/back/link.rs#L1570-L1573

Otherwise, we could ask the question in terms of minimum supported version -- posix_spawn_file_actions_addchdir_np was added in musl 1.1.24. #82632 has been working on musl 1.2 that would include changes for 64-bit time_t, so that might effectively become a new minimum for us.

In general, it would still be nice to have a weak! that works with crt-static, but that doesn't have to block here.

sunshowers commented 5 months ago

Still interested in this. My understanding is that once https://github.com/rust-lang/libc/pull/3068 lands and is incorporated into rustc, we'll set the minimum musl requirement to 1.2 -- and that at that point we can unconditionally assume that posix_spawn_file_actions_addchdir_np exists. (I could be wrong though, so please correct me if so!)

I did some reading on weak symbols and I think I understand how to use them, but if the above understanding is correct I think we can probably just wait.