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

std::env::args_os always returns an empty iterator in a musl+gcompat environment #124126

Closed
rjsberry opened this issue Apr 18, 2024 · 16 comments · Fixed by #124447
Closed

std::env::args_os always returns an empty iterator in a musl+gcompat environment #124126

rjsberry opened this issue Apr 18, 2024 · 16 comments · Fixed by #124447
Labels
C-bug Category: This is a bug. O-musl Target: The musl libc T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@rjsberry
Copy link

I tried this code in Alpine Linux cross compiled to x86_64-unknown-linux-gnu:

use std::env;

fn main() {
    println!("{:?}", env::args_os().collect::<Vec<_>>());
}

For reference I ran this in a container on an M1 Macbook:

$ docker run --rm -it --platform linux/amd64 alpine
# apk add gcompat libgcc

I always see [] printed, even when I pass the program arguments.

I'm not sure this is an issue with the environment as the following C program works as expected:

#include <stdio.h>

int main(int argc, char **argv) {
    for (int i = 0; i < argc; i++) {
        printf("%d: %s\n", i, argv[i]);
    }
}

Meta

rustc +nightly --version --verbose:

rustc 1.79.0-nightly (becebb315 2024-04-17)
binary: rustc
commit-hash: becebb3158149a115cad8a402612e25436a7e37b
commit-date: 2024-04-17
host: aarch64-apple-darwin
release: 1.79.0-nightly
LLVM version: 18.1.3
@rjsberry rjsberry added the C-bug Category: This is a bug. label Apr 18, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Apr 18, 2024
@workingjubilee
Copy link
Member

uh.

@workingjubilee
Copy link
Member

workingjubilee commented Apr 19, 2024

@rjsberry Could you clarify: Does this problem arise with code ran on an ordinary gnu system? Say, a Debian container? Or x86_64-unknown-linux-musl itself?

@ChrisDenton
Copy link
Member

ChrisDenton commented Apr 19, 2024

I can take a guess at why this happens. For glibc targets we assume that the arguments can be retrieved without needing to store argc and argv. However if this musl compatibility layer doesn't use the .init_array then we can't retrieve them.

@saethlin saethlin added O-musl Target: The musl libc T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Apr 19, 2024
@workingjubilee
Copy link
Member

Ah yeah, it seems that behavior is a GNU extension that Rich Felker doesn't want to support?

@workingjubilee
Copy link
Member

Does env::args() just not work at all on musl if we're a cdylib, then?

@bjorn3
Copy link
Member

bjorn3 commented Apr 19, 2024

Correct. When libstd's start function isn't used, env::args() only works on Windows, macOS and Linux with Glibc. On any other target it returns an empty list.

@workingjubilee
Copy link
Member

Apparently we already have this comment:

pub unsafe fn init(_argc: isize, _argv: *const *const u8) {
// On Linux-GNU, we rely on `ARGV_INIT_ARRAY` below to initialize
// `ARGC` and `ARGV`. But in Miri that does not actually happen so we
// still initialize here.
#[cfg(any(miri, not(all(target_os = "linux", target_env = "gnu"))))]
really_init(_argc, _argv);
}

In other words, we are already hacking around this issue for the miri emulator, and not using the fact that we are still given argv "naturally" via execve.

We should probably just unconditionally call really_init.

@bjorn3
Copy link
Member

bjorn3 commented Apr 19, 2024

We should probably just unconditionally call really_init.

That would fix rust-lang/rustc_codegen_cranelift#1048 on Linux + glibc too.

@ChrisDenton
Copy link
Member

Note that this was intentionally done #66547 but there have been other issues #105999

@workingjubilee
Copy link
Member

workingjubilee commented Apr 19, 2024

@ChrisDenton I still think we can consider #105999 to still be fixed, as we just get our char **argv and it will be the same thing, even if the char*s behind the char** have been rearranged, right? So we'll set the atomics twice to the same value, which sounds very Working As Intended.

@workingjubilee
Copy link
Member

One other thought: if we initialize arguments this way, we don't also need to initialize them in our main function in binaries. Can you compile out that code on linux-gnu?

@joshtriplett in #66547 (review)

Well, given the remarks here seem to position this as a "wouldn't it be nice if we didn't do extra work?" and that this seems to be non-resilient in practice, as it has caused more than one open issue, I think we should be unafraid of doing a bit of extra computation.

@ChrisDenton
Copy link
Member

I more meant that the fact that libraries mess about with (what should be) immutable memory is a safety issue so ideally we would parse it at the point where it's least likely that the are multiple threads. But people would probably complain about doing unnecessary allocating and extra work pre-main.

Just storing argc and argv should be cheap though so I'm definitely not arguing against that.

@workingjubilee
Copy link
Member

workingjubilee commented Apr 20, 2024

the dilemma: strictly conforming posix programs shouldn't mutate argc and argv, but strictly conforming posix programs also shouldn't be aware of argc and argv if if they're dylibs, eh?

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 27, 2024
…nu, r=ChrisDenton

Unconditionally call `really_init` on GNU/Linux

This makes miri not diverge in behavior, it fixes running Rust linux-gnu binaries on musl with gcompat, it fixes dlopen edge-cases that cranelift somehow hits, etc.

Fixes rust-lang#124126

thou hast gazed into this abyss with me:
r? `@ChrisDenton`
@bors bors closed this as completed in be89760 Apr 28, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Apr 28, 2024
Rollup merge of rust-lang#124447 - workingjubilee:set-argv-twice-on-gnu, r=ChrisDenton

Unconditionally call `really_init` on GNU/Linux

This makes miri not diverge in behavior, it fixes running Rust linux-gnu binaries on musl with gcompat, it fixes dlopen edge-cases that cranelift somehow hits, etc.

Fixes rust-lang#124126

thou hast gazed into this abyss with me:
r? ``@ChrisDenton``
@joshtriplett
Copy link
Member

Could we check, first, whether we've already had these initialized, and only call this in the unusual case where they're not initialized? That would avoid the extra work in the common case, and do the work in the unusual case.

@workingjubilee
Copy link
Member

@joshtriplett I don't know what you mean? We aren't doing substantial extra work. The thing that costs the most in runtime startup, I expect, is having the static initializer at all, which we then don't get to make choices about whether its called. It simply is or isn't.

@joshtriplett
Copy link
Member

joshtriplett commented Jul 11, 2024

@workingjubilee Nevermind: #124447 (comment) ; forgot that really_init was just saving off the values and not doing anything with them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. O-musl Target: The musl libc T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants