[unix] Don't clone command-line args on startup

[mbrubeck]

Fixes part of #47164 and simplifies the args code on non-Apple Unix platforms.

Note: This could change behavior for programs that use both std::env::args and unsafe code that mutates argv directly. However, these programs already behave differently on different platforms. The new behavior on non-Apple platforms is closer to the existing behavior on Apple platforms.

[rust-highfive]

r? @shepmaster

(rust_highfive has picked a reviewer for you, use r? to override)

[alexcrichton]

Thanks! I think though this may run a risk of memory unsafety? There can be a brief moment after the main thread has returned that other threads could try to access env::args but after main has returned they may become invalid.

I'm not sure if there's a way around that other than cloning on startup. Does cloning on startup cause problems though?

[mbrubeck]

Thanks! I think though this may run a risk of memory unsafety? There can be a brief moment after the main thread has returned that other threads could try to access env::args but after main has returned they may become invalid.

Ah, this is because argv may point to data on the main thread's stack, right? Does this mean the current Mac and iOS implementations are unsafe, since they may try to read from argv after main thread exit?

I'm not sure if there's a way around that other than cloning on startup.

We could reset the pointer to null in cleanup, which is called on the main thread before it exits. When the pointer is null, std::env::args could either panic or returns an empty iterator. (The current Unix implementation returns an empty iterator in this situation, but a panic might be more appropriate.)

Does cloning on startup cause problems though?

It's a small time and memory overhead for every Rust binary that uses libstd, though probably too tiny to matter for most such programs. Removing it also simplifies the code somewhat.

[alexcrichton]

Ah yeah I dunno about the OSX implementation. I think we're praying that the library function gets it right? I'm not sure of the exact semantics of it though.

You're right though in that I think nulling out before main returns could work! It seems reasonable to me to panic in other threads at that time.

[mbrubeck]

Update: I re-added a cleanup function locally as discussed above, but now the unit test env::tests::args_debug seems to be deadlocking, and I don't yet know why.

[mbrubeck]

Oh, now I see why. :) Fix coming up.

[alexcrichton]

@bors: r+

[bors]

📌 Commit 91c3eee has been approved by alexcrichton