Make std::env::{set_var, remove_var} unsafe in edition 2024

[tbu-]

Allow calling these functions without unsafe blocks in editions up until 2021, but don't trigger the unused_unsafe lint for unsafe blocks containing these functions.

Fixes #27970.
Fixes #90308.
CC #124866.

[rustbot]

r? @joboet

rustbot has assigned @joboet.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rustbot]

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

Some changes occurred in src/tools/compiletest

cc @jieyouxu

The Miri subtree was changed

cc @rust-lang/miri

[rustbot]

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in src/tools/compiletest

cc @jieyouxu

[RalfJung]

Should there be a warning on older editions, to make this not come entirely out of the blue when doing edition migration?

[tbu-]

Should there be a warning on older editions, to make this not come entirely out of the blue when doing edition migration?

I agree that this would be quite useful. It seems hard to do right now, though, because a stage 0 compiler will detect a lot of unused unsafe blocks because it doesn't know about the std::env::set_var unsafety. Perhaps this could be postponed until after one bootstrap cycle?

[RalfJung]

I agree that this would be quite useful. It seems hard to do right now, though, because a stage 0 compiler will detect a lot of unused unsafe blocks because it doesn't know about the std::env::set_var unsafety. Perhaps this could be postponed until after one bootstrap cycle?

I wasn't talking about unused_unsafe, but a lint that warns against using set_var/remove_var outside an unsafe block on old editions.

Also, I am not sure if it's the best idea to add new lang items for these functions. In the past the proposal was to add an attribute for "deprecating safety". Then we could e.g. also add that attribute to before_exec.

[marcospb19]

so the only safe option is to not use set_var or remove_var in multi-threaded programs at all

Can't we use a static Mutex to guard env reads and writes? If so, saying that "the only option is not to use this" sounds misleading.

[ChrisDenton]

No we cannot. There's a lot of discussion about that on the linked issues but in short that would only make it safe for people sharing the same Mutex and then only if all FFI was guarded by that Mutex (because any libc function is allowed to read the environment).

[RalfJung]

Would have been good to add a comment explaining why this is no_run. Are you worried about UB in the doctest?

View changes since the review

[tbu-]

Yes, I'm worried about that. The doctest has no safety documentation, and I wouldn't know how I could prove that no other thread is running concurrently to it.

[ChrisDenton]

It is not safe to use in a doctest. We would need to use an integration test.

A doctest could work on platforms where set_var is thread safe but I'm not sure there's much utility in a test only for those platforms if we can have something that works cross-platform.