Experiment: Apply #[deprecated_safe] to env::set_var/env::remove_var

compiler/rustc_codegen_cranelift/src/bin/cg_clif_build_sysroot.rs

@@ -55,7 +55,11 @@ impl rustc_driver::Callbacks for CraneliftPassesCallbacks {
 
 fn main() {
     rustc_driver::init_rustc_env_logger();
-    rustc_driver::install_ice_hook();
+    // SAFETY: in main(), no other threads could be reading or writing the environment
+    // FIXME(skippy) are there definitely zero threads here? other functions have been called
+    unsafe {
+        rustc_driver::install_ice_hook();
+    }
     let exit_code = rustc_driver::catch_with_exit_code(|| {
         let mut use_clif = false;
 

compiler/rustc_driver/src/lib.rs

@@ -1235,7 +1235,11 @@ pub fn report_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) {
 /// Installs a panic hook that will print the ICE message on unexpected panics.
 ///
 /// A custom rustc driver can skip calling this to set up a custom ICE hook.
-pub fn install_ice_hook() {
+///
+/// # Safety
+///
+/// Must not be called when any other thread could be reading or writing the environment.
+pub unsafe fn install_ice_hook() {
     // If the user has not explicitly overridden "RUST_BACKTRACE", then produce
     // full backtraces. When a compiler ICE happens, we want to gather
     // as much information as possible to present in the issue opened
@@ -1320,7 +1324,11 @@ pub fn main() -> ! {
     init_rustc_env_logger();
     signal_handler::install();
     let mut callbacks = TimePassesCallbacks::default();
-    install_ice_hook();
+    // SAFETY: in main(), no other threads could be reading or writing the environment
+    // FIXME(skippy) are there definitely zero threads here? other functions have been called
+    unsafe {
+        install_ice_hook();
+    }
     let exit_code = catch_with_exit_code(|| {
         let args = env::args_os()
             .enumerate()

compiler/rustc_feature/src/tests.rs

@@ -3,6 +3,8 @@ use super::UnstableFeatures;
 #[test]
 fn rustc_bootstrap_parsing() {
     let is_bootstrap = |env, krate| {
+        // FIXME(skippy) there's no fix for deprecated_safe until tests can be run single threaded
+        #[cfg_attr(not(bootstrap), allow(deprecated_safe))]
         std::env::set_var("RUSTC_BOOTSTRAP", env);
         matches!(UnstableFeatures::from_environment(krate), UnstableFeatures::Cheat)
     };

compiler/rustc_interface/src/passes.rs

@@ -315,13 +315,17 @@ pub 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(),
-            );
+            // SAFETY: we're in a cfg!(windows) section, set_var is always sound
+            #[cfg_attr(bootstrap, allow(unused_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
@@ -367,7 +371,11 @@ pub fn configure_and_expand(
             resolver.lint_buffer().buffer_lint(lint, node_id, span, msg);
         }
         if cfg!(windows) {
-            env::set_var("PATH", &old_path);
+            // SAFETY: we're in a cfg!(windows) section, set_var is always sound
+            #[cfg_attr(bootstrap, allow(unused_unsafe))]
+            unsafe {
+                env::set_var("PATH", &old_path);
+            }
         }
 
         if recursion_limit_hit {

compiler/rustc_llvm/build.rs

@@ -22,7 +22,8 @@ fn detect_llvm_link() -> (&'static str, &'static str) {
 // the one we want to use. As such, we restore the environment to what bootstrap saw. This isn't
 // perfect -- we might actually want to see something from Cargo's added library paths -- but
 // for now it works.
-fn restore_library_path() {
+// SAFETY: must not be called when any other thread could be reading or writing the environment
+unsafe fn restore_library_path() {
     let key = tracked_env_var_os("REAL_LIBRARY_PATH_VAR").expect("REAL_LIBRARY_PATH_VAR");
     if let Some(env) = tracked_env_var_os("REAL_LIBRARY_PATH") {
         env::set_var(&key, &env);
@@ -81,7 +82,10 @@ fn main() {
         return;
     }
 
-    restore_library_path();
+    // SAFETY: in main(), no other threads could be reading or writing the environment
+    unsafe {
+        restore_library_path();
+    }
 
     let target = env::var("TARGET").expect("TARGET was not set");
     let llvm_config =

library/std/src/env.rs

@@ -542,7 +542,10 @@ pub struct JoinPathsError {
 ///         let mut paths = env::split_paths(&path).collect::<Vec<_>>();
 ///         paths.push(PathBuf::from("/home/xyz/bin"));
 ///         let new_path = env::join_paths(paths)?;
-///         env::set_var("PATH", &new_path);
+///         // SAFETY: in main(), no other threads could be reading or writing the environment
+///         unsafe {
+///             env::set_var("PATH", &new_path);
+///         }
 ///     }
 ///
 ///     Ok(())

library/std/src/process/tests.rs

@@ -278,8 +278,12 @@ fn test_capture_env_at_spawn() {
 
     // This variable will not be present if the environment has already
     // been captured above.
+    // FIXME(skippy) there's no fix for deprecated_safe until tests can be run single threaded
+    #[cfg_attr(not(bootstrap), allow(deprecated_safe))]
     env::set_var("RUN_TEST_NEW_ENV2", "456");
     let result = cmd.output().unwrap();
+    // FIXME(skippy) there's no fix for deprecated_safe until tests can be run single threaded
+    #[cfg_attr(not(bootstrap), allow(deprecated_safe))]
     env::remove_var("RUN_TEST_NEW_ENV2");
 
     let output = String::from_utf8_lossy(&result.stdout).to_string();

library/std/tests/env.rs

@@ -19,24 +19,36 @@ fn eq(a: Option<OsString>, b: Option<&str>) {
 #[test]
 fn test_set_var() {
     let n = make_rand_name();
+    // FIXME(skippy) there's no fix for deprecated_safe until tests can be run single threaded
+    #[cfg_attr(not(bootstrap), allow(deprecated_safe))]
     set_var(&n, "VALUE");
     eq(var_os(&n), Some("VALUE"));
 }
 
 #[test]
 fn test_remove_var() {
     let n = make_rand_name();
+    // FIXME(skippy) there's no fix for deprecated_safe until tests can be run single threaded
+    #[cfg_attr(not(bootstrap), allow(deprecated_safe))]
     set_var(&n, "VALUE");
+    // FIXME(skippy) there's no fix for deprecated_safe until tests can be run single threaded
+    #[cfg_attr(not(bootstrap), allow(deprecated_safe))]
     remove_var(&n);
     eq(var_os(&n), None);
 }
 
 #[test]
 fn test_set_var_overwrite() {
     let n = make_rand_name();
+    // FIXME(skippy) there's no fix for deprecated_safe until tests can be run single threaded
+    #[cfg_attr(not(bootstrap), allow(deprecated_safe))]
     set_var(&n, "1");
+    // FIXME(skippy) there's no fix for deprecated_safe until tests can be run single threaded
+    #[cfg_attr(not(bootstrap), allow(deprecated_safe))]
     set_var(&n, "2");
     eq(var_os(&n), Some("2"));
+    // FIXME(skippy) there's no fix for deprecated_safe until tests can be run single threaded
+    #[cfg_attr(not(bootstrap), allow(deprecated_safe))]
     set_var(&n, "");
     eq(var_os(&n), Some(""));
 }
@@ -51,6 +63,8 @@ fn test_var_big() {
         i += 1;
     }
     let n = make_rand_name();
+    // FIXME(skippy) there's no fix for deprecated_safe until tests can be run single threaded
+    #[cfg_attr(not(bootstrap), allow(deprecated_safe))]
     set_var(&n, &s);
     eq(var_os(&n), Some(&s));
 }
@@ -60,8 +74,12 @@ fn test_var_big() {
 fn test_env_set_get_huge() {
     let n = make_rand_name();
     let s = "x".repeat(10000);
+    // FIXME(skippy) there's no fix for deprecated_safe until tests can be run single threaded
+    #[cfg_attr(not(bootstrap), allow(deprecated_safe))]
     set_var(&n, &s);
     eq(var_os(&n), Some(&s));
+    // FIXME(skippy) there's no fix for deprecated_safe until tests can be run single threaded
+    #[cfg_attr(not(bootstrap), allow(deprecated_safe))]
     remove_var(&n);
     eq(var_os(&n), None);
 }
@@ -71,6 +89,8 @@ fn test_env_set_var() {
     let n = make_rand_name();
 
     let mut e = vars_os();
+    // FIXME(skippy) there's no fix for deprecated_safe until tests can be run single threaded
+    #[cfg_attr(not(bootstrap), allow(deprecated_safe))]
     set_var(&n, "VALUE");
     assert!(!e.any(|(k, v)| { &*k == &*n && &*v == "VALUE" }));
 
@@ -95,9 +115,13 @@ fn env_home_dir() {
         if #[cfg(unix)] {
             let oldhome = var_to_os_string(var("HOME"));
 
+            // FIXME(skippy) there's no fix for deprecated_safe until tests can be run single threaded
+            #[cfg_attr(not(bootstrap), allow(deprecated_safe))]
             set_var("HOME", "/home/MountainView");
             assert_eq!(home_dir(), Some(PathBuf::from("/home/MountainView")));
 
+            // FIXME(skippy) there's no fix for deprecated_safe until tests can be run single threaded
+            #[cfg_attr(not(bootstrap), allow(deprecated_safe))]
             remove_var("HOME");
             if cfg!(target_os = "android") {
                 assert!(home_dir().is_none());
@@ -108,33 +132,59 @@ fn env_home_dir() {
                 assert_ne!(home_dir(), Some(PathBuf::from("/home/MountainView")));
             }
 
+            // FIXME(skippy) there's no fix for deprecated_safe until tests can be run single threaded
+            #[cfg_attr(not(bootstrap), allow(deprecated_safe))]
             if let Some(oldhome) = oldhome { set_var("HOME", oldhome); }
         } else if #[cfg(windows)] {
             let oldhome = var_to_os_string(var("HOME"));
             let olduserprofile = var_to_os_string(var("USERPROFILE"));
 
-            remove_var("HOME");
-            remove_var("USERPROFILE");
+            // SAFETY: inside a cfg!(windows) section, remove_var is always sound
+            #[cfg_attr(bootstrap, allow(unused_unsafe))]
+            unsafe {
+                remove_var("HOME");
+                remove_var("USERPROFILE");
+            }
 
             assert!(home_dir().is_some());
 
             set_var("HOME", "/home/MountainView");
             assert_eq!(home_dir(), Some(PathBuf::from("/home/MountainView")));
 
-            remove_var("HOME");
+            // SAFETY: inside a cfg!(windows) section, remove_var is always sound
+            #[cfg_attr(bootstrap, allow(unused_unsafe))]
+            unsafe {
+                remove_var("HOME");
+            }
 
-            set_var("USERPROFILE", "/home/MountainView");
+            // SAFETY: inside a cfg!(windows) section, set_var is always sound
+            #[cfg_attr(bootstrap, allow(unused_unsafe))]
+            unsafe {
+                set_var("USERPROFILE", "/home/MountainView");
+            }
             assert_eq!(home_dir(), Some(PathBuf::from("/home/MountainView")));
 
-            set_var("HOME", "/home/MountainView");
-            set_var("USERPROFILE", "/home/PaloAlto");
+            // SAFETY: inside a cfg!(windows) section, set_var is always sound
+            #[cfg_attr(bootstrap, allow(unused_unsafe))]
+            unsafe {
+                set_var("HOME", "/home/MountainView");
+                set_var("USERPROFILE", "/home/PaloAlto");
+            }
             assert_eq!(home_dir(), Some(PathBuf::from("/home/MountainView")));
 
-            remove_var("HOME");
-            remove_var("USERPROFILE");
+            // SAFETY: inside a cfg!(windows) section, remove_var is always sound
+            #[cfg_attr(bootstrap, allow(unused_unsafe))]
+            unsafe {
+                remove_var("HOME");
+                remove_var("USERPROFILE");
+            }
 
-            if let Some(oldhome) = oldhome { set_var("HOME", oldhome); }
-            if let Some(olduserprofile) = olduserprofile { set_var("USERPROFILE", olduserprofile); }
+            // SAFETY: inside a cfg!(windows) section, set_var is always sound
+            #[cfg_attr(bootstrap, allow(unused_unsafe))]
+            unsafe {
+                if let Some(oldhome) = oldhome { set_var("HOME", oldhome); }
+                if let Some(olduserprofile) = olduserprofile { set_var("USERPROFILE", olduserprofile); }
+            }
         }
     }
 }

library/test/src/lib.rs

@@ -145,6 +145,8 @@ pub fn test_main_static_abort(tests: &[&TestDescAndFn]) {
     // If we're being run in SpawnedSecondary mode, run the test here. run_test
     // will then exit the process.
     if let Ok(name) = env::var(SECONDARY_TEST_INVOKER_VAR) {
+        // FIXME(skippy) deprecate_safe: should be a single thread here, assert that somehow?
+        #[cfg_attr(not(bootstrap), allow(deprecated_safe))]
         env::remove_var(SECONDARY_TEST_INVOKER_VAR);
         let test = tests
             .iter()

library/test/src/term/terminfo/searcher/tests.rs

@@ -13,7 +13,11 @@ fn test_get_dbpath_for_term() {
     }
     assert!(x("screen") == "/usr/share/terminfo/s/screen");
     assert!(get_dbpath_for_term("") == None);
+    // FIXME(skippy) there's no fix for deprecated_safe until tests can be run single threaded
+    #[cfg_attr(not(bootstrap), allow(deprecated_safe))]
     env::set_var("TERMINFO_DIRS", ":");
     assert!(x("screen") == "/usr/share/terminfo/s/screen");
+    // FIXME(skippy) there's no fix for deprecated_safe until tests can be run single threaded
+    #[cfg_attr(not(bootstrap), allow(deprecated_safe))]
     env::remove_var("TERMINFO_DIRS");
 }

src/librustdoc/lib.rs

@@ -163,7 +163,11 @@ pub fn main() {
     }
 
     rustc_driver::set_sigpipe_handler();
-    rustc_driver::install_ice_hook();
+    // SAFETY: in main(), no other threads could be reading or writing the environment
+    // FIXME(skippy) are there definitely zero threads here? other functions have been called
+    unsafe {
+        rustc_driver::install_ice_hook();
+    }
 
     // When using CI artifacts (with `download_stage1 = true`), tracing is unconditionally built
     // with `--features=static_max_level_info`, which disables almost all rustdoc logging. To avoid

src/test/ui/command/command-exec.rs

@@ -41,7 +41,11 @@ fn main() {
             }
 
             "exec-test5" => {
-                env::set_var("VARIABLE", "ABC");
+                // SAFETY: in main(), no other threads could be reading or writing the environment
+                #[cfg_attr(bootstrap, allow(unused_unsafe))]
+                unsafe {
+                    env::set_var("VARIABLE", "ABC");
+                }
                 Command::new("definitely-not-a-real-binary").env("VARIABLE", "XYZ").exec();
                 assert_eq!(env::var("VARIABLE").unwrap(), "ABC");
                 println!("passed");

src/test/ui/process/process-envs.rs

@@ -29,7 +29,11 @@ fn main() {
     // save original environment
     let old_env = env::var_os("RUN_TEST_NEW_ENV");
 
-    env::set_var("RUN_TEST_NEW_ENV", "123");
+    // SAFETY: in main(), no other threads could be reading or writing the environment
+    #[cfg_attr(bootstrap, allow(unused_unsafe))]
+    unsafe {
+        env::set_var("RUN_TEST_NEW_ENV", "123");
+    }
 
     // create filtered environment vector
     let filtered_env : HashMap<String, String> =
@@ -40,9 +44,13 @@ fn main() {
     cmd.envs(&filtered_env);
 
     // restore original environment
-    match old_env {
-        None => env::remove_var("RUN_TEST_NEW_ENV"),
-        Some(val) => env::set_var("RUN_TEST_NEW_ENV", &val)
+    // SAFETY: in main(), no other threads could be reading or writing the environment
+    #[cfg_attr(bootstrap, allow(unused_unsafe))]
+    unsafe {
+        match old_env {
+            None => env::remove_var("RUN_TEST_NEW_ENV"),
+            Some(val) => env::set_var("RUN_TEST_NEW_ENV", &val)
+        }
     }
 
     let result = cmd.output().unwrap();

src/test/ui/process/process-remove-from-env.rs

@@ -28,15 +28,23 @@ fn main() {
     // save original environment
     let old_env = env::var_os("RUN_TEST_NEW_ENV");
 
-    env::set_var("RUN_TEST_NEW_ENV", "123");
+    // SAFETY: in main(), no other threads could be reading or writing the environment
+    #[cfg_attr(bootstrap, allow(unused_unsafe))]
+    unsafe {
+        env::set_var("RUN_TEST_NEW_ENV", "123");
+    }
 
     let mut cmd = env_cmd();
     cmd.env_remove("RUN_TEST_NEW_ENV");
 
     // restore original environment
-    match old_env {
-        None => env::remove_var("RUN_TEST_NEW_ENV"),
-        Some(val) => env::set_var("RUN_TEST_NEW_ENV", &val)
+    // SAFETY: in main(), no other threads could be reading or writing the environment
+    #[cfg_attr(bootstrap, allow(unused_unsafe))]
+    unsafe {
+        match old_env {
+            None => env::remove_var("RUN_TEST_NEW_ENV"),
+            Some(val) => env::set_var("RUN_TEST_NEW_ENV", &val)
+        }
     }
 
     let result = cmd.output().unwrap();