build: Harden env var tracking in build scripts

Author: petrochenkov

src/build_helper/lib.rs

@@ -1,3 +1,5 @@
+use std::ffi::{OsStr, OsString};
+use std::fmt::Display;
 use std::path::{Path, PathBuf};
 use std::process::{Command, Stdio};
 use std::time::{SystemTime, UNIX_EPOCH};
@@ -28,6 +30,14 @@ macro_rules! t {
     };
 }
 
+/// Reads an environment variable and adds it to dependencies.
+/// Supposed to be used for all variables except those set for build scripts by cargo
+/// https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-build-scripts
+pub fn tracked_env_var_os<K: AsRef<OsStr> + Display>(key: K) -> Option<OsString> {
+    println!("cargo:rerun-if-env-changed={}", key);
+    env::var_os(key)
+}
+
 // Because Cargo adds the compiler's dylib path to our library search path, llvm-config may
 // break: the dylib path for the compiler, as of this writing, contains a copy of the LLVM
 // shared library, which means that when our freshly built llvm-config goes to load it's
@@ -37,10 +47,8 @@ macro_rules! t {
 // perfect -- we might actually want to see something from Cargo's added library paths -- but
 // for now it works.
 pub fn restore_library_path() {
-    println!("cargo:rerun-if-env-changed=REAL_LIBRARY_PATH_VAR");
-    println!("cargo:rerun-if-env-changed=REAL_LIBRARY_PATH");
-    let key = env::var_os("REAL_LIBRARY_PATH_VAR").expect("REAL_LIBRARY_PATH_VAR");
-    if let Some(env) = env::var_os("REAL_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);
     } else {
         env::remove_var(&key);

src/librustc_llvm/build.rs

@@ -2,21 +2,20 @@ use std::env;
 use std::path::{Path, PathBuf};
 use std::process::Command;
 
-use build_helper::output;
+use build_helper::{output, tracked_env_var_os};
 
 fn detect_llvm_link() -> (&'static str, &'static str) {
     // Force the link mode we want, preferring static by default, but
     // possibly overridden by `configure --enable-llvm-link-shared`.
-    if env::var_os("LLVM_LINK_SHARED").is_some() {
+    if tracked_env_var_os("LLVM_LINK_SHARED").is_some() {
         ("dylib", "--link-shared")
     } else {
         ("static", "--link-static")
     }
 }
 
 fn main() {
-    println!("cargo:rerun-if-env-changed=RUST_CHECK");
-    if env::var_os("RUST_CHECK").is_some() {
+    if tracked_env_var_os("RUST_CHECK").is_some() {
         // If we're just running `check`, there's no need for LLVM to be built.
         return;
     }
@@ -25,8 +24,8 @@ fn main() {
 
     let target = env::var("TARGET").expect("TARGET was not set");
     let llvm_config =
-        env::var_os("LLVM_CONFIG").map(|x| Some(PathBuf::from(x))).unwrap_or_else(|| {
-            if let Some(dir) = env::var_os("CARGO_TARGET_DIR").map(PathBuf::from) {
+        tracked_env_var_os("LLVM_CONFIG").map(|x| Some(PathBuf::from(x))).unwrap_or_else(|| {
+            if let Some(dir) = tracked_env_var_os("CARGO_TARGET_DIR").map(PathBuf::from) {
                 let to_test = dir
                     .parent()
                     .unwrap()
@@ -46,8 +45,6 @@ fn main() {
     }
     let llvm_config = llvm_config.unwrap_or_else(|| PathBuf::from("llvm-config"));
 
-    println!("cargo:rerun-if-env-changed=LLVM_CONFIG");
-
     // Test whether we're cross-compiling LLVM. This is a pretty rare case
     // currently where we're producing an LLVM for a different platform than
     // what this build script is currently running on.
@@ -163,12 +160,11 @@ fn main() {
         cfg.define(&flag, None);
     }
 
-    println!("cargo:rerun-if-env-changed=LLVM_RUSTLLVM");
-    if env::var_os("LLVM_RUSTLLVM").is_some() {
+    if tracked_env_var_os("LLVM_RUSTLLVM").is_some() {
         cfg.define("LLVM_RUSTLLVM", None);
     }
 
-    if env::var_os("LLVM_NDEBUG").is_some() {
+    if tracked_env_var_os("LLVM_NDEBUG").is_some() {
         cfg.define("NDEBUG", None);
         cfg.debug(false);
     }
@@ -255,7 +251,7 @@ fn main() {
     // librustc_llvm, for example when using static libc++, we may need to
     // manually specify the library search path and -ldl -lpthread as link
     // dependencies.
-    let llvm_linker_flags = env::var_os("LLVM_LINKER_FLAGS");
+    let llvm_linker_flags = tracked_env_var_os("LLVM_LINKER_FLAGS");
     if let Some(s) = llvm_linker_flags {
         for lib in s.into_string().unwrap().split_whitespace() {
             if lib.starts_with("-l") {
@@ -266,8 +262,8 @@ fn main() {
         }
     }
 
-    let llvm_static_stdcpp = env::var_os("LLVM_STATIC_STDCPP");
-    let llvm_use_libcxx = env::var_os("LLVM_USE_LIBCXX");
+    let llvm_static_stdcpp = tracked_env_var_os("LLVM_STATIC_STDCPP");
+    let llvm_use_libcxx = tracked_env_var_os("LLVM_USE_LIBCXX");
 
     let stdcppname = if target.contains("openbsd") {
         if target.contains("sparc64") { "estdc++" } else { "c++" }