Skip to content

Commit

Permalink
Make std::env::{set_var, remove_var} unsafe in edition 2024
Browse files Browse the repository at this point in the history
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 rust-lang#27970.
Fixes rust-lang#90308.
CC rust-lang#124866.
  • Loading branch information
tbu- committed May 13, 2024
1 parent ba956ef commit 9358a14
Show file tree
Hide file tree
Showing 18 changed files with 174 additions and 59 deletions.
6 changes: 6 additions & 0 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,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:
Expand Down
13 changes: 9 additions & 4 deletions compiler/rustc_mir_build/src/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}
),
},
}
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1570,6 +1570,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,
Expand Down
54 changes: 34 additions & 20 deletions library/std/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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`].
///
Expand All @@ -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<K: AsRef<OsStr>, V: AsRef<OsStr>>(key: K, value: V) {
pub unsafe fn set_var<K: AsRef<OsStr>, V: AsRef<OsStr>>(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<K: AsRef<OsStr>, V: AsRef<OsStr>>(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}")
})
Expand All @@ -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
Expand All @@ -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`].
///
Expand All @@ -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<K: AsRef<OsStr>>(key: K) {
pub unsafe fn remove_var<K: AsRef<OsStr>>(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<K: AsRef<OsStr>>(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}"))
}
Expand Down
14 changes: 5 additions & 9 deletions library/std/src/sys/pal/hermit/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,18 +172,14 @@ pub fn getenv(k: &OsStr) -> Option<OsString> {
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(())
}

Expand Down
4 changes: 2 additions & 2 deletions library/std/src/sys/pal/sgx/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,13 +157,13 @@ pub fn getenv(k: &OsStr) -> Option<OsString> {
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);
}
Expand Down
8 changes: 4 additions & 4 deletions library/std/src/sys/pal/solid/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,19 +191,19 @@ pub fn getenv(k: &OsStr) -> Option<OsString> {
.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_env(unsafe { libc::setenv(k.as_ptr(), v.as_ptr(), 1) }).map(drop)
cvt_env(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_env(unsafe { libc::unsetenv(nbuf.as_ptr()) }).map(drop)
cvt_env(libc::unsetenv(nbuf.as_ptr())).map(drop)
})
}

Expand Down
4 changes: 2 additions & 2 deletions library/std/src/sys/pal/teeos/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,11 @@ pub fn getenv(_: &OsStr) -> Option<OsString> {
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"))
}

Expand Down
4 changes: 2 additions & 2 deletions library/std/src/sys/pal/uefi/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,11 +203,11 @@ pub fn getenv(_: &OsStr) -> Option<OsString> {
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"))
}

Expand Down
8 changes: 4 additions & 4 deletions library/std/src/sys/pal/unix/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -651,19 +651,19 @@ pub fn getenv(k: &OsStr) -> Option<OsString> {
.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)
})
}

Expand Down
4 changes: 2 additions & 2 deletions library/std/src/sys/pal/unsupported/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,11 @@ pub fn getenv(_: &OsStr) -> Option<OsString> {
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"))
}

Expand Down
4 changes: 2 additions & 2 deletions library/std/src/sys/pal/wasi/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ pub fn getenv(k: &OsStr) -> Option<OsString> {
.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();
Expand All @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions library/std/src/sys/pal/windows/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,16 +302,16 @@ pub fn getenv(k: &OsStr) -> Option<OsString> {
.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 {
Expand Down
4 changes: 2 additions & 2 deletions library/std/src/sys/pal/xous/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,11 @@ pub fn getenv(_: &OsStr) -> Option<OsString> {
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"))
}

Expand Down
4 changes: 2 additions & 2 deletions library/std/src/sys/pal/zkvm/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,11 @@ pub fn getenv(varname: &OsStr) -> Option<OsString> {
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"))
}

Expand Down
23 changes: 23 additions & 0 deletions tests/ui/rust-2024/unsafe-env.e2021.stderr
Original file line number Diff line number Diff line change
@@ -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`.
Loading

0 comments on commit 9358a14

Please sign in to comment.