From 9bed19edc42fe7db30df8258d13598bfe7b1971b Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sun, 28 Apr 2024 17:45:08 -0400 Subject: [PATCH] Refactor UnixEnvVars::get so that it can be reused by getenv --- src/tools/miri/src/shims/env.rs | 12 ++++++++++-- src/tools/miri/src/shims/time.rs | 2 +- src/tools/miri/src/shims/unix/env.rs | 21 +++++---------------- src/tools/miri/src/shims/windows/env.rs | 2 +- 4 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/tools/miri/src/shims/env.rs b/src/tools/miri/src/shims/env.rs index 395a1ca62c6..b95abb484c8 100644 --- a/src/tools/miri/src/shims/env.rs +++ b/src/tools/miri/src/shims/env.rs @@ -102,11 +102,19 @@ impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { /// Try to get an environment variable from the interpreted program's environment. This is /// useful for implementing shims which are documented to read from the environment. - fn get_var(&mut self, name: &OsStr) -> InterpResult<'tcx, Option> { + fn get_env_var(&mut self, name: &OsStr) -> InterpResult<'tcx, Option> { let this = self.eval_context_ref(); match &this.machine.env_vars { EnvVars::Uninit => return Ok(None), - EnvVars::Unix(vars) => vars.get(this, name), + EnvVars::Unix(vars) => { + let var_ptr = vars.get(this, name)?; + if let Some(ptr) = var_ptr { + let var = this.read_os_str_from_c_str(ptr)?; + Ok(Some(var.to_owned())) + } else { + Ok(None) + } + } EnvVars::Windows(vars) => vars.get(name), } } diff --git a/src/tools/miri/src/shims/time.rs b/src/tools/miri/src/shims/time.rs index 05dbdef1ba1..8d1f07f916c 100644 --- a/src/tools/miri/src/shims/time.rs +++ b/src/tools/miri/src/shims/time.rs @@ -140,7 +140,7 @@ fn localtime_r( DateTime::from_timestamp(sec_since_epoch, 0).expect("Invalid timestamp"); // Figure out what time zone is in use - let tz = this.get_var(OsStr::new("TZ"))?.unwrap_or_else(|| OsString::from("UTC")); + let tz = this.get_env_var(OsStr::new("TZ"))?.unwrap_or_else(|| OsString::from("UTC")); let tz = match tz.into_string() { Ok(tz) => Tz::from_str(&tz).unwrap_or(Tz::UTC), _ => Tz::UTC, diff --git a/src/tools/miri/src/shims/unix/env.rs b/src/tools/miri/src/shims/unix/env.rs index f61a81b07cb..910e53260bf 100644 --- a/src/tools/miri/src/shims/unix/env.rs +++ b/src/tools/miri/src/shims/unix/env.rs @@ -71,13 +71,13 @@ pub(crate) fn environ(&self) -> Pointer> { self.environ.ptr() } - /// Implementation detail for [`InterpCx::get_var`]. This basically does `getenv`, complete + /// Implementation detail for [`InterpCx::get_env_var`]. This basically does `getenv`, complete /// with the reads of the environment, but returns an [`OsString`] instead of a pointer. pub(crate) fn get<'mir>( &self, ecx: &InterpCx<'mir, 'tcx, MiriMachine<'mir, 'tcx>>, name: &OsStr, - ) -> InterpResult<'tcx, Option> { + ) -> InterpResult<'tcx, Option>>> { // We don't care about the value as we have the `map` to keep track of everything, // but we do want to do this read so it shows up as a data race. let _vars_ptr = ecx.read_pointer(&self.environ)?; @@ -89,7 +89,7 @@ pub(crate) fn get<'mir>( Size::from_bytes(u64::try_from(name.len()).unwrap().checked_add(1).unwrap()), ecx, )?; - ecx.read_os_str_from_c_str(var_ptr).map(|s| Some(s.to_owned())) + Ok(Some(var_ptr)) } } @@ -137,19 +137,8 @@ fn getenv( let name_ptr = this.read_pointer(name_op)?; let name = this.read_os_str_from_c_str(name_ptr)?; - // We don't care about the value as we have the `map` to keep track of everything, - // but we do want to do this read so it shows up as a data race. - let _vars_ptr = this.read_pointer(&this.machine.env_vars.unix().environ)?; - Ok(match this.machine.env_vars.unix().map.get(name) { - Some(var_ptr) => { - // The offset is used to strip the "{name}=" part of the string. - var_ptr.offset( - Size::from_bytes(u64::try_from(name.len()).unwrap().checked_add(1).unwrap()), - this, - )? - } - None => Pointer::null(), - }) + let var_ptr = this.machine.env_vars.unix().get(this, name)?; + Ok(var_ptr.unwrap_or_else(Pointer::null)) } fn setenv( diff --git a/src/tools/miri/src/shims/windows/env.rs b/src/tools/miri/src/shims/windows/env.rs index a8b0a77b23b..b3bc5d4d852 100644 --- a/src/tools/miri/src/shims/windows/env.rs +++ b/src/tools/miri/src/shims/windows/env.rs @@ -27,7 +27,7 @@ pub(crate) fn new<'mir, 'tcx>( Ok(Self { map: env_vars }) } - /// Implementation detail for [`InterpCx::get_var`]. + /// Implementation detail for [`InterpCx::get_env_var`]. pub(crate) fn get<'tcx>(&self, name: &OsStr) -> InterpResult<'tcx, Option> { Ok(self.map.get(name).cloned()) }