From 6dcfc965928de39194bd91e804ef62286a7e0808 Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Mon, 13 May 2024 09:05:34 +0200 Subject: [PATCH] Make `std::env::{set_var, remove_var}` unsafe in edition 2024 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. --- compiler/rustc_feature/src/builtin_attrs.rs | 6 +++ .../rustc_mir_build/src/check_unsafety.rs | 13 +++-- compiler/rustc_span/src/symbol.rs | 1 + library/std/src/env.rs | 54 ++++++++++++------- library/std/src/sys/pal/hermit/os.rs | 14 ++--- library/std/src/sys/pal/sgx/os.rs | 4 +- library/std/src/sys/pal/solid/os.rs | 4 +- library/std/src/sys/pal/teeos/os.rs | 4 +- library/std/src/sys/pal/uefi/os.rs | 4 +- library/std/src/sys/pal/unix/os.rs | 8 +-- library/std/src/sys/pal/unsupported/os.rs | 4 +- library/std/src/sys/pal/wasi/os.rs | 4 +- library/std/src/sys/pal/windows/os.rs | 8 +-- library/std/src/sys/pal/xous/os.rs | 4 +- library/std/src/sys/pal/zkvm/os.rs | 4 +- tests/ui/rust-2024/unsafe-env.e2021.stderr | 23 ++++++++ tests/ui/rust-2024/unsafe-env.e2024.stderr | 39 ++++++++++++++ tests/ui/rust-2024/unsafe-env.rs | 31 +++++++++++ 18 files changed, 172 insertions(+), 57 deletions(-) create mode 100644 tests/ui/rust-2024/unsafe-env.e2021.stderr create mode 100644 tests/ui/rust-2024/unsafe-env.e2024.stderr create mode 100644 tests/ui/rust-2024/unsafe-env.rs diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index 0b4a871dd50c2..8b7e93fd55587 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -578,6 +578,12 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ "rustc_allowed_through_unstable_modules special cases accidental stabilizations of stable items \ through unstable paths" ), + rustc_attr!( + rustc_deprecated_safe_2024, Normal, template!(Word), WarnFollowing, + EncodeCrossCrate::Yes, + "rustc_deprecated_safe_2024 is supposed to be used in libstd only", + ), + // ========================================================================== // Internal attributes: Type system related: diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs index b5f7ffbd2afb1..403b7b31f1e5a 100644 --- a/compiler/rustc_mir_build/src/check_unsafety.rs +++ b/compiler/rustc_mir_build/src/check_unsafety.rs @@ -110,14 +110,19 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> { ); self.suggest_unsafe_block = false; } - SafetyContext::Safe => { - kind.emit_requires_unsafe_err( + SafetyContext::Safe => match kind { + // Allow calls to deprecated-safe unsafe functions if the + // caller is from an edition before 2024. + UnsafeOpKind::CallToUnsafeFunction(Some(id)) + if !span.at_least_rust_2024() + && self.tcx.has_attr(id, sym::rustc_deprecated_safe_2024) => {} + _ => kind.emit_requires_unsafe_err( self.tcx, span, self.hir_context, unsafe_op_in_unsafe_fn_allowed, - ); - } + ), + }, } } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index ace4dff46aa0a..532b355d4f792 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1573,6 +1573,7 @@ symbols! { rustc_def_path, rustc_default_body_unstable, rustc_deny_explicit_impl, + rustc_deprecated_safe_2024, rustc_diagnostic_item, rustc_diagnostic_macros, rustc_dirty, diff --git a/library/std/src/env.rs b/library/std/src/env.rs index 6f8ac17f12c70..95ee2a91d159e 100644 --- a/library/std/src/env.rs +++ b/library/std/src/env.rs @@ -318,11 +318,6 @@ impl Error for VarError { /// /// # Safety /// -/// Even though this function is currently not marked as `unsafe`, it needs to -/// be because invoking it can cause undefined behaviour. The function will be -/// marked `unsafe` in a future version of Rust. This is tracked in -/// [rust#27970](https://github.com/rust-lang/rust/issues/27970). -/// /// This function is safe to call in a single-threaded program. /// /// In multi-threaded programs, you must ensure that are no other threads @@ -331,7 +326,7 @@ impl Error for VarError { /// how to achieve this, but we strongly suggest not using `set_var` or /// `remove_var` in multi-threaded programs at all. /// -/// Most C libraries, including libc itself do not advertise which functions +/// Most C libraries, including libc itself, do not advertise which functions /// read from the environment. Even functions from the Rust standard library do /// that, e.g. for DNS lookups from [`std::net::ToSocketAddrs`]. /// @@ -353,15 +348,26 @@ impl Error for VarError { /// use std::env; /// /// let key = "KEY"; -/// env::set_var(key, "VALUE"); +/// unsafe { +/// env::set_var(key, "VALUE"); +/// } /// assert_eq!(env::var(key), Ok("VALUE".to_string())); /// ``` +#[cfg(not(bootstrap))] +#[rustc_deprecated_safe_2024] #[stable(feature = "env", since = "1.0.0")] -pub fn set_var, V: AsRef>(key: K, value: V) { +pub unsafe fn set_var, V: AsRef>(key: K, value: V) { _set_var(key.as_ref(), value.as_ref()) } -fn _set_var(key: &OsStr, value: &OsStr) { +#[cfg(bootstrap)] +#[allow(missing_docs)] +#[stable(feature = "env", since = "1.0.0")] +pub fn set_var, V: AsRef>(key: K, value: V) { + unsafe { _set_var(key.as_ref(), value.as_ref()) } +} + +unsafe fn _set_var(key: &OsStr, value: &OsStr) { os_imp::setenv(key, value).unwrap_or_else(|e| { panic!("failed to set environment variable `{key:?}` to `{value:?}`: {e}") }) @@ -371,11 +377,6 @@ fn _set_var(key: &OsStr, value: &OsStr) { /// /// # Safety /// -/// Even though this function is currently not marked as `unsafe`, it needs to -/// be because invoking it can cause undefined behaviour. The function will be -/// marked `unsafe` in a future version of Rust. This is tracked in -/// [rust#27970](https://github.com/rust-lang/rust/issues/27970). -/// /// This function is safe to call in a single-threaded program. /// /// In multi-threaded programs, you must ensure that are no other threads @@ -384,7 +385,7 @@ fn _set_var(key: &OsStr, value: &OsStr) { /// how to achieve this, but we strongly suggest not using `set_var` or /// `remove_var` in multi-threaded programs at all. /// -/// Most C libraries, including libc itself do not advertise which functions +/// Most C libraries, including libc itself, do not advertise which functions /// read from the environment. Even functions from the Rust standard library do /// that, e.g. for DNS lookups from [`std::net::ToSocketAddrs`]. /// @@ -403,22 +404,35 @@ fn _set_var(key: &OsStr, value: &OsStr) { /// /// # Examples /// -/// ``` +/// ```no_run /// use std::env; /// /// let key = "KEY"; -/// env::set_var(key, "VALUE"); +/// unsafe { +/// env::set_var(key, "VALUE"); +/// } /// assert_eq!(env::var(key), Ok("VALUE".to_string())); /// -/// env::remove_var(key); +/// unsafe { +/// env::remove_var(key); +/// } /// assert!(env::var(key).is_err()); /// ``` +#[cfg(not(bootstrap))] +#[rustc_deprecated_safe_2024] #[stable(feature = "env", since = "1.0.0")] -pub fn remove_var>(key: K) { +pub unsafe fn remove_var>(key: K) { _remove_var(key.as_ref()) } -fn _remove_var(key: &OsStr) { +#[cfg(bootstrap)] +#[allow(missing_docs)] +#[stable(feature = "env", since = "1.0.0")] +pub fn remove_var>(key: K) { + unsafe { _remove_var(key.as_ref()) } +} + +unsafe fn _remove_var(key: &OsStr) { os_imp::unsetenv(key) .unwrap_or_else(|e| panic!("failed to remove environment variable `{key:?}`: {e}")) } diff --git a/library/std/src/sys/pal/hermit/os.rs b/library/std/src/sys/pal/hermit/os.rs index cc6781238319b..91247d30462f8 100644 --- a/library/std/src/sys/pal/hermit/os.rs +++ b/library/std/src/sys/pal/hermit/os.rs @@ -172,18 +172,14 @@ pub fn getenv(k: &OsStr) -> Option { unsafe { ENV.as_ref().unwrap().lock().unwrap().get_mut(k).cloned() } } -pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> { - unsafe { - let (k, v) = (k.to_owned(), v.to_owned()); - ENV.as_ref().unwrap().lock().unwrap().insert(k, v); - } +pub unsafe fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> { + let (k, v) = (k.to_owned(), v.to_owned()); + ENV.as_ref().unwrap().lock().unwrap().insert(k, v); Ok(()) } -pub fn unsetenv(k: &OsStr) -> io::Result<()> { - unsafe { - ENV.as_ref().unwrap().lock().unwrap().remove(k); - } +pub unsafe fn unsetenv(k: &OsStr) -> io::Result<()> { + ENV.as_ref().unwrap().lock().unwrap().remove(k); Ok(()) } diff --git a/library/std/src/sys/pal/sgx/os.rs b/library/std/src/sys/pal/sgx/os.rs index 86f4c7d3d56d6..c021300d4ae33 100644 --- a/library/std/src/sys/pal/sgx/os.rs +++ b/library/std/src/sys/pal/sgx/os.rs @@ -157,13 +157,13 @@ pub fn getenv(k: &OsStr) -> Option { get_env_store().and_then(|s| s.lock().unwrap().get(k).cloned()) } -pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> { +pub unsafe fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> { let (k, v) = (k.to_owned(), v.to_owned()); create_env_store().lock().unwrap().insert(k, v); Ok(()) } -pub fn unsetenv(k: &OsStr) -> io::Result<()> { +pub unsafe fn unsetenv(k: &OsStr) -> io::Result<()> { if let Some(env) = get_env_store() { env.lock().unwrap().remove(k); } diff --git a/library/std/src/sys/pal/solid/os.rs b/library/std/src/sys/pal/solid/os.rs index ef35d8788a236..ac90aae4ebe46 100644 --- a/library/std/src/sys/pal/solid/os.rs +++ b/library/std/src/sys/pal/solid/os.rs @@ -191,7 +191,7 @@ pub fn getenv(k: &OsStr) -> Option { .flatten() } -pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> { +pub unsafe fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> { run_with_cstr(k.as_bytes(), &|k| { run_with_cstr(v.as_bytes(), &|v| { let _guard = ENV_LOCK.write(); @@ -200,7 +200,7 @@ pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> { }) } -pub fn unsetenv(n: &OsStr) -> io::Result<()> { +pub unsafe fn unsetenv(n: &OsStr) -> io::Result<()> { run_with_cstr(n.as_bytes(), &|nbuf| { let _guard = ENV_LOCK.write(); cvt_env(unsafe { libc::unsetenv(nbuf.as_ptr()) }).map(drop) diff --git a/library/std/src/sys/pal/teeos/os.rs b/library/std/src/sys/pal/teeos/os.rs index e54a92f01f86b..3be0846a6dd4d 100644 --- a/library/std/src/sys/pal/teeos/os.rs +++ b/library/std/src/sys/pal/teeos/os.rs @@ -109,11 +109,11 @@ pub fn getenv(_: &OsStr) -> Option { None } -pub fn setenv(_: &OsStr, _: &OsStr) -> io::Result<()> { +pub unsafe fn setenv(_: &OsStr, _: &OsStr) -> io::Result<()> { Err(io::Error::new(io::ErrorKind::Unsupported, "cannot set env vars on this platform")) } -pub fn unsetenv(_: &OsStr) -> io::Result<()> { +pub unsafe fn unsetenv(_: &OsStr) -> io::Result<()> { Err(io::Error::new(io::ErrorKind::Unsupported, "cannot unset env vars on this platform")) } diff --git a/library/std/src/sys/pal/uefi/os.rs b/library/std/src/sys/pal/uefi/os.rs index 58838c5876ebd..0b27977df2fde 100644 --- a/library/std/src/sys/pal/uefi/os.rs +++ b/library/std/src/sys/pal/uefi/os.rs @@ -203,11 +203,11 @@ pub fn getenv(_: &OsStr) -> Option { None } -pub fn setenv(_: &OsStr, _: &OsStr) -> io::Result<()> { +pub unsafe fn setenv(_: &OsStr, _: &OsStr) -> io::Result<()> { Err(io::const_io_error!(io::ErrorKind::Unsupported, "cannot set env vars on this platform")) } -pub fn unsetenv(_: &OsStr) -> io::Result<()> { +pub unsafe fn unsetenv(_: &OsStr) -> io::Result<()> { Err(io::const_io_error!(io::ErrorKind::Unsupported, "cannot unset env vars on this platform")) } diff --git a/library/std/src/sys/pal/unix/os.rs b/library/std/src/sys/pal/unix/os.rs index 8afc49f52274c..b5c7d30da7bc0 100644 --- a/library/std/src/sys/pal/unix/os.rs +++ b/library/std/src/sys/pal/unix/os.rs @@ -675,19 +675,19 @@ pub fn getenv(k: &OsStr) -> Option { .flatten() } -pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> { +pub unsafe fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> { run_with_cstr(k.as_bytes(), &|k| { run_with_cstr(v.as_bytes(), &|v| { let _guard = ENV_LOCK.write(); - cvt(unsafe { libc::setenv(k.as_ptr(), v.as_ptr(), 1) }).map(drop) + cvt(libc::setenv(k.as_ptr(), v.as_ptr(), 1)).map(drop) }) }) } -pub fn unsetenv(n: &OsStr) -> io::Result<()> { +pub unsafe fn unsetenv(n: &OsStr) -> io::Result<()> { run_with_cstr(n.as_bytes(), &|nbuf| { let _guard = ENV_LOCK.write(); - cvt(unsafe { libc::unsetenv(nbuf.as_ptr()) }).map(drop) + cvt(libc::unsetenv(nbuf.as_ptr())).map(drop) }) } diff --git a/library/std/src/sys/pal/unsupported/os.rs b/library/std/src/sys/pal/unsupported/os.rs index 248b34829f2ee..3be98898bbeb9 100644 --- a/library/std/src/sys/pal/unsupported/os.rs +++ b/library/std/src/sys/pal/unsupported/os.rs @@ -96,11 +96,11 @@ pub fn getenv(_: &OsStr) -> Option { None } -pub fn setenv(_: &OsStr, _: &OsStr) -> io::Result<()> { +pub unsafe fn setenv(_: &OsStr, _: &OsStr) -> io::Result<()> { Err(io::const_io_error!(io::ErrorKind::Unsupported, "cannot set env vars on this platform")) } -pub fn unsetenv(_: &OsStr) -> io::Result<()> { +pub unsafe fn unsetenv(_: &OsStr) -> io::Result<()> { Err(io::const_io_error!(io::ErrorKind::Unsupported, "cannot unset env vars on this platform")) } diff --git a/library/std/src/sys/pal/wasi/os.rs b/library/std/src/sys/pal/wasi/os.rs index ee377b6ef791d..e96296997e6a9 100644 --- a/library/std/src/sys/pal/wasi/os.rs +++ b/library/std/src/sys/pal/wasi/os.rs @@ -244,7 +244,7 @@ pub fn getenv(k: &OsStr) -> Option { .flatten() } -pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> { +pub unsafe fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> { run_with_cstr(k.as_bytes(), &|k| { run_with_cstr(v.as_bytes(), &|v| unsafe { let _guard = env_write_lock(); @@ -253,7 +253,7 @@ pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> { }) } -pub fn unsetenv(n: &OsStr) -> io::Result<()> { +pub unsafe fn unsetenv(n: &OsStr) -> io::Result<()> { run_with_cstr(n.as_bytes(), &|nbuf| unsafe { let _guard = env_write_lock(); cvt(libc::unsetenv(nbuf.as_ptr())).map(drop) diff --git a/library/std/src/sys/pal/windows/os.rs b/library/std/src/sys/pal/windows/os.rs index 64d8b72aed282..483b8b0072c8f 100644 --- a/library/std/src/sys/pal/windows/os.rs +++ b/library/std/src/sys/pal/windows/os.rs @@ -302,16 +302,16 @@ pub fn getenv(k: &OsStr) -> Option { .ok() } -pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> { +pub unsafe fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> { let k = to_u16s(k)?; let v = to_u16s(v)?; - cvt(unsafe { c::SetEnvironmentVariableW(k.as_ptr(), v.as_ptr()) }).map(drop) + cvt(c::SetEnvironmentVariableW(k.as_ptr(), v.as_ptr())).map(drop) } -pub fn unsetenv(n: &OsStr) -> io::Result<()> { +pub unsafe fn unsetenv(n: &OsStr) -> io::Result<()> { let v = to_u16s(n)?; - cvt(unsafe { c::SetEnvironmentVariableW(v.as_ptr(), ptr::null()) }).map(drop) + cvt(c::SetEnvironmentVariableW(v.as_ptr(), ptr::null())).map(drop) } pub fn temp_dir() -> PathBuf { diff --git a/library/std/src/sys/pal/xous/os.rs b/library/std/src/sys/pal/xous/os.rs index 8d2eaee8aa617..9be09eed62989 100644 --- a/library/std/src/sys/pal/xous/os.rs +++ b/library/std/src/sys/pal/xous/os.rs @@ -149,11 +149,11 @@ pub fn getenv(_: &OsStr) -> Option { None } -pub fn setenv(_: &OsStr, _: &OsStr) -> io::Result<()> { +pub unsafe fn setenv(_: &OsStr, _: &OsStr) -> io::Result<()> { Err(io::const_io_error!(io::ErrorKind::Unsupported, "cannot set env vars on this platform")) } -pub fn unsetenv(_: &OsStr) -> io::Result<()> { +pub unsafe fn unsetenv(_: &OsStr) -> io::Result<()> { Err(io::const_io_error!(io::ErrorKind::Unsupported, "cannot unset env vars on this platform")) } diff --git a/library/std/src/sys/pal/zkvm/os.rs b/library/std/src/sys/pal/zkvm/os.rs index 759beb2d306b9..e7d6cd52a258e 100644 --- a/library/std/src/sys/pal/zkvm/os.rs +++ b/library/std/src/sys/pal/zkvm/os.rs @@ -115,11 +115,11 @@ pub fn getenv(varname: &OsStr) -> Option { Some(OsString::from_inner(os_str::Buf { inner: u8s.to_vec() })) } -pub fn setenv(_: &OsStr, _: &OsStr) -> io::Result<()> { +pub unsafe fn setenv(_: &OsStr, _: &OsStr) -> io::Result<()> { Err(io::const_io_error!(io::ErrorKind::Unsupported, "cannot set env vars on this platform")) } -pub fn unsetenv(_: &OsStr) -> io::Result<()> { +pub unsafe fn unsetenv(_: &OsStr) -> io::Result<()> { Err(io::const_io_error!(io::ErrorKind::Unsupported, "cannot unset env vars on this platform")) } diff --git a/tests/ui/rust-2024/unsafe-env.e2021.stderr b/tests/ui/rust-2024/unsafe-env.e2021.stderr new file mode 100644 index 0000000000000..ebca04d348a36 --- /dev/null +++ b/tests/ui/rust-2024/unsafe-env.e2021.stderr @@ -0,0 +1,23 @@ +error[E0133]: call to unsafe function `unsafe_fn` is unsafe and requires unsafe function or block + --> $DIR/unsafe-env.rs:24:5 + | +LL | unsafe_fn(); + | ^^^^^^^^^^^ call to unsafe function + | + = note: consult the function's documentation for information on how to avoid undefined behavior + +error: unnecessary `unsafe` block + --> $DIR/unsafe-env.rs:27:5 + | +LL | unsafe { + | ^^^^^^ unnecessary `unsafe` block + | +note: the lint level is defined here + --> $DIR/unsafe-env.rs:12:8 + | +LL | #[deny(unused_unsafe)] + | ^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0133`. diff --git a/tests/ui/rust-2024/unsafe-env.e2024.stderr b/tests/ui/rust-2024/unsafe-env.e2024.stderr new file mode 100644 index 0000000000000..212ff1fca009a --- /dev/null +++ b/tests/ui/rust-2024/unsafe-env.e2024.stderr @@ -0,0 +1,39 @@ +error[E0133]: call to unsafe function `set_var` is unsafe and requires unsafe block + --> $DIR/unsafe-env.rs:14:5 + | +LL | env::set_var("FOO", "BAR"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ call to unsafe function + | + = note: consult the function's documentation for information on how to avoid undefined behavior + +error[E0133]: call to unsafe function `remove_var` is unsafe and requires unsafe block + --> $DIR/unsafe-env.rs:16:5 + | +LL | env::remove_var("FOO"); + | ^^^^^^^^^^^^^^^^^^^^^^ call to unsafe function + | + = note: consult the function's documentation for information on how to avoid undefined behavior + +error[E0133]: call to unsafe function `unsafe_fn` is unsafe and requires unsafe block + --> $DIR/unsafe-env.rs:24:5 + | +LL | unsafe_fn(); + | ^^^^^^^^^^^ call to unsafe function + | + = note: consult the function's documentation for information on how to avoid undefined behavior + +error: unnecessary `unsafe` block + --> $DIR/unsafe-env.rs:27:5 + | +LL | unsafe { + | ^^^^^^ unnecessary `unsafe` block + | +note: the lint level is defined here + --> $DIR/unsafe-env.rs:12:8 + | +LL | #[deny(unused_unsafe)] + | ^^^^^^^^^^^^^ + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0133`. diff --git a/tests/ui/rust-2024/unsafe-env.rs b/tests/ui/rust-2024/unsafe-env.rs new file mode 100644 index 0000000000000..2f20b69de9e0f --- /dev/null +++ b/tests/ui/rust-2024/unsafe-env.rs @@ -0,0 +1,31 @@ +//@ revisions: e2021 e2024 +//@[e2021] edition: 2021 +//@[e2024] edition: 2024 +//@[e2024] compile-flags: -Zunstable-options + +use std::env; +use std::mem; + +unsafe fn unsafe_fn() {} +fn safe_fn() {} + +#[deny(unused_unsafe)] +fn main() { + env::set_var("FOO", "BAR"); + //[e2024]~^ ERROR [E0133] + env::remove_var("FOO"); + //[e2024]~^ ERROR [E0133] + + unsafe { + env::set_var("FOO", "BAR"); + env::remove_var("FOO"); + } + + unsafe_fn(); + //~^ ERROR [E0133] + + unsafe { + //~^ ERROR unnecessary `unsafe` block + safe_fn(); + } +}