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.
tbu- · May 2, 2024 · 49f8cf1 · 49f8cf1
1 parent 9084601
commit 49f8cf1
Show file tree

Hide file tree

Showing 27 changed files with 253 additions and 98 deletions.
diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs
@@ -1310,7 +1310,11 @@ fn ice_path() -> &'static Option<PathBuf> {
 /// internal features.
 ///
 /// A custom rustc driver can skip calling this to set up a custom ICE hook.
-pub fn install_ice_hook(
+///
+/// # Unsafety
+///
+/// This function modifies the environment and has the same safety as [`std::env::set_var`].
+pub unsafe fn install_ice_hook(
     bug_report_url: &'static str,
     extra_info: fn(&DiagCtxt),
 ) -> Arc<AtomicBool> {
@@ -1523,7 +1527,8 @@ pub fn main() -> ! {
     init_rustc_env_logger(&early_dcx);
     signal_handler::install();
     let mut callbacks = TimePassesCallbacks::default();
-    let using_internal_features = install_ice_hook(DEFAULT_BUG_REPORT_URL, |_| ());
+    // We're single-threaded at this point, so it's okay to call `install_ice_hook`.
+    let using_internal_features = unsafe { install_ice_hook(DEFAULT_BUG_REPORT_URL, |_| ()) };
     install_ctrlc_handler();
 
     let exit_code = catch_with_exit_code(|| {

diff --git a/compiler/rustc_hir/src/lang_items.rs b/compiler/rustc_hir/src/lang_items.rs
@@ -379,6 +379,9 @@ language_item_table! {
 
     String,                  sym::String,              string,                     Target::Struct,         GenericRequirement::None;
     CStr,                    sym::CStr,                c_str,                      Target::Struct,         GenericRequirement::None;
+
+    EnvSetVar,               sym::env_set_var,         env_set_var,                Target::Fn,             GenericRequirement::None;
+    EnvRemoveVar,            sym::env_remove_var,      env_remove_var,             Target::Fn,             GenericRequirement::None;
 }
 
 pub enum GenericRequirement {

diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs
@@ -178,13 +178,16 @@ fn configure_and_expand(
                     new_path.push(path);
                 }
             }
-            env::set_var(
-                "PATH",
-                &env::join_paths(
-                    new_path.iter().filter(|p| env::join_paths(iter::once(p)).is_ok()),
-                )
-                .unwrap(),
-            );
+            // FIXME: This looks unsafe.
+            unsafe {
+                env::set_var(
+                    "PATH",
+                    &env::join_paths(
+                        new_path.iter().filter(|p| env::join_paths(iter::once(p)).is_ok()),
+                    )
+                    .unwrap(),
+                );
+            }
         }
 
         // Create the config for macro expansion
@@ -223,7 +226,10 @@ fn configure_and_expand(
         }
 
         if cfg!(windows) {
-            env::set_var("PATH", &old_path);
+            unsafe {
+                // FIXME: This looks unsafe.
+                env::set_var("PATH", &old_path);
+            }
         }
 
         krate

diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs
@@ -41,6 +41,12 @@ struct UnsafetyVisitor<'a, 'tcx> {
     /// Flag to ensure that we only suggest wrapping the entire function body in
     /// an unsafe block once.
     suggest_unsafe_block: bool,
+
+    /// Functions that are only considered `unsafe` since Rust 2024.
+    ///
+    /// - `std::env::set_var`
+    /// - `std::env::remove_var`
+    rust_2024_unsafe_fns: &'a [DefId],
 }
 
 impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
@@ -109,14 +115,16 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
                 );
                 self.suggest_unsafe_block = false;
             }
-            SafetyContext::Safe => {
-                kind.emit_requires_unsafe_err(
+            SafetyContext::Safe => match kind {
+                UnsafeOpKind::CallToUnsafeFunction(Some(id))
+                    if !span.at_least_rust_2024() && self.rust_2024_unsafe_fns.contains(&id) => {}
+                _ => kind.emit_requires_unsafe_err(
                     self.tcx,
                     span,
                     self.hir_context,
                     unsafe_op_in_unsafe_fn_allowed,
-                );
-            }
+                ),
+            },
         }
     }
 
@@ -154,6 +162,7 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
                 inside_adt: false,
                 warnings: self.warnings,
                 suggest_unsafe_block: self.suggest_unsafe_block,
+                rust_2024_unsafe_fns: self.rust_2024_unsafe_fns,
             };
             inner_visitor.visit_expr(&inner_thir[expr]);
             // Unsafe blocks can be used in the inner body, make sure to take it into account
@@ -918,6 +927,13 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def: LocalDefId) {
         return;
     }
 
+    let mut rust_2024_unsafe_fns = Vec::new();
+    if let Some(def) = tcx.lang_items().env_set_var() {
+        rust_2024_unsafe_fns.push(def);
+    }
+    if let Some(def) = tcx.lang_items().env_remove_var() {
+        rust_2024_unsafe_fns.push(def);
+    }
     let hir_id = tcx.local_def_id_to_hir_id(def);
     let safety_context = tcx.hir().fn_sig_by_hir_id(hir_id).map_or(SafetyContext::Safe, |fn_sig| {
         if fn_sig.header.unsafety == hir::Unsafety::Unsafe {
@@ -940,6 +956,7 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def: LocalDefId) {
         inside_adt: false,
         warnings: &mut warnings,
         suggest_unsafe_block: true,
+        rust_2024_unsafe_fns: &rust_2024_unsafe_fns,
     };
     visitor.visit_expr(&thir[expr]);
 

diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs
@@ -756,6 +756,8 @@ symbols! {
         end,
         env,
         env_CFG_RELEASE: env!("CFG_RELEASE"),
+        env_remove_var,
+        env_set_var,
         eprint_macro,
         eprintln_macro,
         eq,

diff --git 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
@@ -353,15 +348,18 @@ 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_attr(not(any(bootstrap, test, doctest)), lang = "env_set_var")]
 #[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) {
+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 +369,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
@@ -403,22 +396,27 @@ 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_attr(not(any(bootstrap, test, doctest)), lang = "env_remove_var")]
 #[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) {
+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/process/tests.rs b/library/std/src/process/tests.rs
@@ -310,9 +310,16 @@ fn test_capture_env_at_spawn() {
 
     // This variable will not be present if the environment has already
     // been captured above.
-    env::set_var("RUN_TEST_NEW_ENV2", "456");
+    //
+    // FIXME: This looks unsafe.
+    unsafe {
+        env::set_var("RUN_TEST_NEW_ENV2", "456");
+    }
     let result = cmd.output().unwrap();
-    env::remove_var("RUN_TEST_NEW_ENV2");
+    // FIXME: This looks unsafe.
+    unsafe {
+        env::remove_var("RUN_TEST_NEW_ENV2");
+    }
 
     let output = String::from_utf8_lossy(&result.stdout).to_string();
 

diff --git 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<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(())
 }
 

diff --git 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<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);
     }

diff --git a/library/std/src/sys/pal/solid/os.rs b/library/std/src/sys/pal/solid/os.rs
@@ -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)
     })
 }
 

diff --git 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<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"))
 }
 

diff --git 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<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"))
 }
 

diff --git a/library/std/src/sys/pal/unix/os.rs b/library/std/src/sys/pal/unix/os.rs
@@ -667,19 +667,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)
     })
 }
 

diff --git 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<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"))
 }
 

diff --git 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<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();
@@ -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)