Auto merge of rust-lang#124636 - tbu-:pr_env_unsafe, r=petrochenkov

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 rust-lang#27970.
Fixes rust-lang#90308.
CC rust-lang#124866.

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:

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 <https://github.com/rust-lang/rust/issues/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 <https://github.com/rust-lang/rust/issues/27970>",
+ };
+}

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 ->

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;
@@ -110,14 +110,34 @@ 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) =>
+ {
+ 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,
  self.hir_context,
  unsafe_op_in_unsafe_fn_allowed,
- );
- }
+ ),
+ },
  }
  }
 

compiler/rustc_mir_build/src/errors.rs

@@ -20,6 +20,25 @@ pub struct UnconditionalRecursion {
  pub call_sites: Vec<Span>,
 }
 
+#[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]

compiler/rustc_span/src/symbol.rs

@@ -1579,6 +1579,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,

library/std/src/env.rs

@@ -318,22 +318,25 @@ 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
-/// 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`].
+/// 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, 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:
 ///
@@ -353,15 +356,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}")
  })
@@ -371,22 +385,25 @@ 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
-/// 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`].
+/// 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, 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:
 ///
@@ -403,22 +420,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}"))
 }

library/std/src/sys/pal/hermit/os.rs

@@ -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(())
 }
 

library/std/src/sys/pal/sgx/os.rs

@@ -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);
  }

library/std/src/sys/pal/solid/os.rs

@@ -191,7 +191,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| {
  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)

library/std/src/sys/pal/teeos/os.rs

@@ -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"))
 }