From 5d8f9b4dc1c913e62387e10762f018f182a34582 Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Mon, 13 May 2024 09:05:34 +0200 Subject: [PATCH 1/4] 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 0b4a871dd50..8b7e93fd555 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 b5f7ffbd2af..403b7b31f1e 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 90da220b3f5..c8670124169 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1575,6 +1575,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 6f8ac17f12c..95ee2a91d15 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 cc678123831..91247d30462 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 86f4c7d3d56..c021300d4ae 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 ef35d8788a2..ac90aae4ebe 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 e54a92f01f8..3be0846a6dd 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 58838c5876e..0b27977df2f 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 8afc49f5227..b5c7d30da7b 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 248b34829f2..3be98898bbe 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 ee377b6ef79..e96296997e6 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 64d8b72aed2..483b8b0072c 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 8d2eaee8aa6..9be09eed629 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 759beb2d306..e7d6cd52a25 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 00000000000..ebca04d348a --- /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 00000000000..212ff1fca00 --- /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 00000000000..e5c1d5778fd --- /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 call to unsafe function `set_var` is unsafe + env::remove_var("FOO"); + //[e2024]~^ ERROR call to unsafe function `remove_var` is unsafe + + unsafe { + env::set_var("FOO", "BAR"); + env::remove_var("FOO"); + } + + unsafe_fn(); + //~^ ERROR call to unsafe function `unsafe_fn` is unsafe + + unsafe { + //~^ ERROR unnecessary `unsafe` block + safe_fn(); + } +} From 8cf49806486133a1fae72ca22c732ed2800eb879 Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Fri, 24 May 2024 10:04:44 +0200 Subject: [PATCH 2/4] Add note about safety of `std::env::set_var` on Windows --- library/std/src/env.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/library/std/src/env.rs b/library/std/src/env.rs index 95ee2a91d15..d433caa9d2a 100644 --- a/library/std/src/env.rs +++ b/library/std/src/env.rs @@ -320,11 +320,14 @@ impl Error for VarError { /// /// This function is safe to call in a single-threaded program. /// -/// In multi-threaded programs, you must ensure that are no other threads -/// concurrently writing or *reading*(!) from the environment through functions -/// other than the ones in this module. You are responsible for figuring out -/// how to achieve this, but we strongly suggest not using `set_var` or -/// `remove_var` in multi-threaded programs at all. +/// This function is also always safe to call on Windows, in single-threaded +/// and multi-threaded programs. +/// +/// In multi-threaded programs on other operating systems, you must ensure that +/// are no other threads concurrently writing or *reading*(!) from the +/// environment through functions other than the ones in this module. You are +/// responsible for figuring out 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 /// read from the environment. Even functions from the Rust standard library do @@ -379,6 +382,9 @@ unsafe fn _set_var(key: &OsStr, value: &OsStr) { /// /// This function is safe to call in a single-threaded program. /// +/// This function is also always safe to call on Windows, in single-threaded +/// and multi-threaded programs. +/// /// In multi-threaded programs, you must ensure that are no other threads /// concurrently writing or *reading*(!) from the environment through functions /// other than the ones in this module. You are responsible for figuring out From d7680e355617f409db3cda62afe35fffbb52e70c Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Fri, 24 May 2024 10:26:04 +0200 Subject: [PATCH 3/4] Elaborate about modifying env vars in multi-threaded programs --- library/std/src/env.rs | 46 +++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/library/std/src/env.rs b/library/std/src/env.rs index d433caa9d2a..4d649f8a6f1 100644 --- a/library/std/src/env.rs +++ b/library/std/src/env.rs @@ -323,15 +323,20 @@ impl Error for VarError { /// This function is also always safe to call on Windows, in single-threaded /// and multi-threaded programs. /// -/// In multi-threaded programs on other operating systems, you must ensure that -/// are no other threads concurrently writing or *reading*(!) from the -/// environment through functions other than the ones in this module. You are -/// responsible for figuring out 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 -/// read from the environment. Even functions from the Rust standard library do -/// that, e.g. for DNS lookups from [`std::net::ToSocketAddrs`]. +/// In multi-threaded programs on other operating systems, we strongly suggest +/// not using `set_var` or `remove_var` at all. The exact requirement is: you +/// must ensure that there are no other threads concurrently writing or +/// *reading*(!) the environment through functions or global variables other +/// than the ones in this module. The problem is that these operating systems +/// do not provide a thread-safe way to read the environment, and most C +/// libraries, including libc itself, do not advertise which functions read +/// from the environment. Even functions from the Rust standard library may +/// read the environment without going through this module, e.g. for DNS +/// lookups from [`std::net::ToSocketAddrs`]. No stable guarantee is made about +/// which functions may read from the environment in future versions of a +/// library. All this makes it not practically possible for you to guarantee +/// that no other thread will read the environment, so the only safe option is +/// to not use `set_var` or `remove_var` in multi-threaded programs at all. /// /// Discussion of this unsafety on Unix may be found in: /// @@ -385,15 +390,20 @@ unsafe fn _set_var(key: &OsStr, value: &OsStr) { /// This function is also always safe to call on Windows, in single-threaded /// and multi-threaded programs. /// -/// In multi-threaded programs, you must ensure that are no other threads -/// concurrently writing or *reading*(!) from the environment through functions -/// other than the ones in this module. You are responsible for figuring out -/// 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 -/// read from the environment. Even functions from the Rust standard library do -/// that, e.g. for DNS lookups from [`std::net::ToSocketAddrs`]. +/// In multi-threaded programs on other operating systems, we strongly suggest +/// not using `set_var` or `remove_var` at all. The exact requirement is: you +/// must ensure that there are no other threads concurrently writing or +/// *reading*(!) the environment through functions or global variables other +/// than the ones in this module. The problem is that these operating systems +/// do not provide a thread-safe way to read the environment, and most C +/// libraries, including libc itself, do not advertise which functions read +/// from the environment. Even functions from the Rust standard library may +/// read the environment without going through this module, e.g. for DNS +/// lookups from [`std::net::ToSocketAddrs`]. No stable guarantee is made about +/// which functions may read from the environment in future versions of a +/// library. All this makes it not practically possible for you to guarantee +/// that no other thread will read the environment, so the only safe option is +/// to not use `set_var` or `remove_var` in multi-threaded programs at all. /// /// Discussion of this unsafety on Unix may be found in: /// From 44f9f8bc33c0b5537c52c9e18697b83f12f19604 Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Tue, 28 May 2024 15:16:25 +0200 Subject: [PATCH 4/4] Add `deprecated_safe` lint It warns about usages of `std::env::{set_var, remove_var}` with an automatic fix wrapping the call in an `unsafe` block. --- compiler/rustc_lint_defs/src/builtin.rs | 49 +++++++++++++++++++ compiler/rustc_mir_build/messages.ftl | 6 +++ .../rustc_mir_build/src/check_unsafety.rs | 19 ++++++- compiler/rustc_mir_build/src/errors.rs | 19 +++++++ .../ui/rust-2024/unsafe-env-suggestion.fixed | 20 ++++++++ tests/ui/rust-2024/unsafe-env-suggestion.rs | 20 ++++++++ .../ui/rust-2024/unsafe-env-suggestion.stderr | 33 +++++++++++++ tests/ui/rust-2024/unsafe-env.e2021.stderr | 6 +-- tests/ui/rust-2024/unsafe-env.e2024.stderr | 10 ++-- tests/ui/rust-2024/unsafe-env.rs | 1 - 10 files changed, 172 insertions(+), 11 deletions(-) create mode 100644 tests/ui/rust-2024/unsafe-env-suggestion.fixed create mode 100644 tests/ui/rust-2024/unsafe-env-suggestion.rs create mode 100644 tests/ui/rust-2024/unsafe-env-suggestion.stderr diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 13867319e5c..93995fe60a3 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -37,6 +37,7 @@ declare_lint_pass! { DEPRECATED, DEPRECATED_CFG_ATTR_CRATE_TYPE_NAME, DEPRECATED_IN_FUTURE, + DEPRECATED_SAFE, DEPRECATED_WHERE_CLAUSE_LOCATION, DUPLICATE_MACRO_ATTRIBUTES, ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT, @@ -4844,3 +4845,51 @@ declare_lint! { reference: "issue #124559 ", }; } + +declare_lint! { + /// The `deprecated_safe` lint detects unsafe functions being used as safe + /// functions. + /// + /// ### Example + /// + /// ```rust,edition2021,compile_fail + /// #![deny(deprecated_safe)] + /// // edition 2021 + /// use std::env; + /// fn enable_backtrace() { + /// env::set_var("RUST_BACKTRACE", "1"); + /// } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Rust [editions] allow the language to evolve without breaking backward + /// compatibility. This lint catches code that uses `unsafe` functions that + /// were declared as safe (non-`unsafe`) in earlier editions. If you switch + /// the compiler to a new edition without updating the code, then it + /// will fail to compile if you are using a function previously marked as + /// safe. + /// + /// You can audit the code to see if it suffices the preconditions of the + /// `unsafe` code, and if it does, you can wrap it in an `unsafe` block. If + /// you can't fulfill the preconditions, you probably need to switch to a + /// different way of doing what you want to achieve. + /// + /// This lint can automatically wrap the calls in `unsafe` blocks, but this + /// obviously cannot verify that the preconditions of the `unsafe` + /// functions are fulfilled, so that is still up to the user. + /// + /// The lint is currently "allow" by default, but that might change in the + /// future. + /// + /// [editions]: https://doc.rust-lang.org/edition-guide/ + pub DEPRECATED_SAFE, + Allow, + "detects unsafe functions being used as safe functions", + @future_incompatible = FutureIncompatibleInfo { + reason: FutureIncompatibilityReason::EditionError(Edition::Edition2024), + reference: "issue #27970 ", + }; +} diff --git a/compiler/rustc_mir_build/messages.ftl b/compiler/rustc_mir_build/messages.ftl index 4ba61226a3f..40a84a599af 100644 --- a/compiler/rustc_mir_build/messages.ftl +++ b/compiler/rustc_mir_build/messages.ftl @@ -28,6 +28,12 @@ mir_build_borrow_of_moved_value = borrow of moved value .value_borrowed_label = value borrowed here after move .suggestion = borrow this binding in the pattern to avoid moving the value +mir_build_call_to_deprecated_safe_fn_requires_unsafe = + call to deprecated safe function `{$function}` is unsafe and requires unsafe block + .note = consult the function's documentation for information on how to avoid undefined behavior + .label = call to unsafe function + .suggestion = you can wrap the call in an `unsafe` block if you can guarantee the code is only ever called from single-threaded code + mir_build_call_to_fn_with_requires_unsafe = call to function `{$function}` with `#[target_feature]` is unsafe and requires unsafe block .help = in order for the call to be safe, the context requires the following additional target {$missing_target_features_count -> diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs index 403b7b31f1e..24098282d93 100644 --- a/compiler/rustc_mir_build/src/check_unsafety.rs +++ b/compiler/rustc_mir_build/src/check_unsafety.rs @@ -9,7 +9,7 @@ use rustc_middle::thir::visit::Visitor; use rustc_middle::thir::*; use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_middle::ty::{self, ParamEnv, Ty, TyCtxt}; -use rustc_session::lint::builtin::{UNSAFE_OP_IN_UNSAFE_FN, UNUSED_UNSAFE}; +use rustc_session::lint::builtin::{DEPRECATED_SAFE, UNSAFE_OP_IN_UNSAFE_FN, UNUSED_UNSAFE}; use rustc_session::lint::Level; use rustc_span::def_id::{DefId, LocalDefId}; use rustc_span::symbol::Symbol; @@ -115,7 +115,22 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> { // 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) => {} + && self.tcx.has_attr(id, sym::rustc_deprecated_safe_2024) => + { + self.tcx.emit_node_span_lint( + DEPRECATED_SAFE, + self.hir_context, + span, + CallToDeprecatedSafeFnRequiresUnsafe { + span, + function: with_no_trimmed_paths!(self.tcx.def_path_str(id)), + sub: CallToDeprecatedSafeFnRequiresUnsafeSub { + left: span.shrink_to_lo(), + right: span.shrink_to_hi(), + }, + }, + ) + } _ => kind.emit_requires_unsafe_err( self.tcx, span, diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs index ae6e126a4e2..b29cc24cff8 100644 --- a/compiler/rustc_mir_build/src/errors.rs +++ b/compiler/rustc_mir_build/src/errors.rs @@ -20,6 +20,25 @@ pub struct UnconditionalRecursion { pub call_sites: Vec, } +#[derive(LintDiagnostic)] +#[diag(mir_build_call_to_deprecated_safe_fn_requires_unsafe)] +pub struct CallToDeprecatedSafeFnRequiresUnsafe { + #[label] + pub span: Span, + pub function: String, + #[subdiagnostic] + pub sub: CallToDeprecatedSafeFnRequiresUnsafeSub, +} + +#[derive(Subdiagnostic)] +#[multipart_suggestion(mir_build_suggestion, applicability = "machine-applicable")] +pub struct CallToDeprecatedSafeFnRequiresUnsafeSub { + #[suggestion_part(code = "unsafe {{ ")] + pub left: Span, + #[suggestion_part(code = " }}")] + pub right: Span, +} + #[derive(LintDiagnostic)] #[diag(mir_build_unsafe_op_in_unsafe_fn_call_to_unsafe_fn_requires_unsafe, code = E0133)] #[note] diff --git a/tests/ui/rust-2024/unsafe-env-suggestion.fixed b/tests/ui/rust-2024/unsafe-env-suggestion.fixed new file mode 100644 index 00000000000..d9c738edfac --- /dev/null +++ b/tests/ui/rust-2024/unsafe-env-suggestion.fixed @@ -0,0 +1,20 @@ +//@ run-rustfix + +#![deny(deprecated_safe)] + +use std::env; + +#[deny(unused_unsafe)] +fn main() { + unsafe { env::set_var("FOO", "BAR") }; + //~^ ERROR call to deprecated safe function + //~| WARN this is accepted in the current edition + unsafe { env::remove_var("FOO") }; + //~^ ERROR call to deprecated safe function + //~| WARN this is accepted in the current edition + + unsafe { + env::set_var("FOO", "BAR"); + env::remove_var("FOO"); + } +} diff --git a/tests/ui/rust-2024/unsafe-env-suggestion.rs b/tests/ui/rust-2024/unsafe-env-suggestion.rs new file mode 100644 index 00000000000..3bd169973e3 --- /dev/null +++ b/tests/ui/rust-2024/unsafe-env-suggestion.rs @@ -0,0 +1,20 @@ +//@ run-rustfix + +#![deny(deprecated_safe)] + +use std::env; + +#[deny(unused_unsafe)] +fn main() { + env::set_var("FOO", "BAR"); + //~^ ERROR call to deprecated safe function + //~| WARN this is accepted in the current edition + env::remove_var("FOO"); + //~^ ERROR call to deprecated safe function + //~| WARN this is accepted in the current edition + + unsafe { + env::set_var("FOO", "BAR"); + env::remove_var("FOO"); + } +} diff --git a/tests/ui/rust-2024/unsafe-env-suggestion.stderr b/tests/ui/rust-2024/unsafe-env-suggestion.stderr new file mode 100644 index 00000000000..90c91c2a474 --- /dev/null +++ b/tests/ui/rust-2024/unsafe-env-suggestion.stderr @@ -0,0 +1,33 @@ +error: call to deprecated safe function `std::env::set_var` is unsafe and requires unsafe block + --> $DIR/unsafe-env-suggestion.rs:9:5 + | +LL | env::set_var("FOO", "BAR"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ call to unsafe function + | + = warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2024! + = note: for more information, see issue #27970 +note: the lint level is defined here + --> $DIR/unsafe-env-suggestion.rs:3:9 + | +LL | #![deny(deprecated_safe)] + | ^^^^^^^^^^^^^^^ +help: you can wrap the call in an `unsafe` block if you can guarantee the code is only ever called from single-threaded code + | +LL | unsafe { env::set_var("FOO", "BAR") }; + | ++++++++ + + +error: call to deprecated safe function `std::env::remove_var` is unsafe and requires unsafe block + --> $DIR/unsafe-env-suggestion.rs:12:5 + | +LL | env::remove_var("FOO"); + | ^^^^^^^^^^^^^^^^^^^^^^ call to unsafe function + | + = warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2024! + = note: for more information, see issue #27970 +help: you can wrap the call in an `unsafe` block if you can guarantee the code is only ever called from single-threaded code + | +LL | unsafe { env::remove_var("FOO") }; + | ++++++++ + + +error: aborting due to 2 previous errors + diff --git a/tests/ui/rust-2024/unsafe-env.e2021.stderr b/tests/ui/rust-2024/unsafe-env.e2021.stderr index ebca04d348a..cc40ec2e466 100644 --- a/tests/ui/rust-2024/unsafe-env.e2021.stderr +++ b/tests/ui/rust-2024/unsafe-env.e2021.stderr @@ -1,5 +1,5 @@ error[E0133]: call to unsafe function `unsafe_fn` is unsafe and requires unsafe function or block - --> $DIR/unsafe-env.rs:24:5 + --> $DIR/unsafe-env.rs:23:5 | LL | unsafe_fn(); | ^^^^^^^^^^^ call to unsafe function @@ -7,13 +7,13 @@ LL | unsafe_fn(); = note: consult the function's documentation for information on how to avoid undefined behavior error: unnecessary `unsafe` block - --> $DIR/unsafe-env.rs:27:5 + --> $DIR/unsafe-env.rs:26:5 | LL | unsafe { | ^^^^^^ unnecessary `unsafe` block | note: the lint level is defined here - --> $DIR/unsafe-env.rs:12:8 + --> $DIR/unsafe-env.rs:11:8 | LL | #[deny(unused_unsafe)] | ^^^^^^^^^^^^^ diff --git a/tests/ui/rust-2024/unsafe-env.e2024.stderr b/tests/ui/rust-2024/unsafe-env.e2024.stderr index 212ff1fca00..b43f817cf72 100644 --- a/tests/ui/rust-2024/unsafe-env.e2024.stderr +++ b/tests/ui/rust-2024/unsafe-env.e2024.stderr @@ -1,5 +1,5 @@ error[E0133]: call to unsafe function `set_var` is unsafe and requires unsafe block - --> $DIR/unsafe-env.rs:14:5 + --> $DIR/unsafe-env.rs:13:5 | LL | env::set_var("FOO", "BAR"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ call to unsafe function @@ -7,7 +7,7 @@ LL | env::set_var("FOO", "BAR"); = 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 + --> $DIR/unsafe-env.rs:15:5 | LL | env::remove_var("FOO"); | ^^^^^^^^^^^^^^^^^^^^^^ call to unsafe function @@ -15,7 +15,7 @@ LL | env::remove_var("FOO"); = 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 + --> $DIR/unsafe-env.rs:23:5 | LL | unsafe_fn(); | ^^^^^^^^^^^ call to unsafe function @@ -23,13 +23,13 @@ LL | unsafe_fn(); = note: consult the function's documentation for information on how to avoid undefined behavior error: unnecessary `unsafe` block - --> $DIR/unsafe-env.rs:27:5 + --> $DIR/unsafe-env.rs:26:5 | LL | unsafe { | ^^^^^^ unnecessary `unsafe` block | note: the lint level is defined here - --> $DIR/unsafe-env.rs:12:8 + --> $DIR/unsafe-env.rs:11:8 | LL | #[deny(unused_unsafe)] | ^^^^^^^^^^^^^ diff --git a/tests/ui/rust-2024/unsafe-env.rs b/tests/ui/rust-2024/unsafe-env.rs index e5c1d5778fd..a882f077b9b 100644 --- a/tests/ui/rust-2024/unsafe-env.rs +++ b/tests/ui/rust-2024/unsafe-env.rs @@ -4,7 +4,6 @@ //@[e2024] compile-flags: -Zunstable-options use std::env; -use std::mem; unsafe fn unsafe_fn() {} fn safe_fn() {}