From 2fa07009f2a27c8d77ed48539cf66db6e2b5a676 Mon Sep 17 00:00:00 2001
From: JOE1994 <joseph942010@gmail.com>
Date: Fri, 13 Mar 2020 12:45:02 -0400
Subject: [PATCH 1/9] Windows shims for env var emulation
Shims for GetEnvironmentVariableW / SetEnvironmentVariableW /
GetEnvironmentStringsW.
Passes test 'tests/run-pass/env.rs'
---
src/shims/env.rs | 120 ++++++++++++++++++++++++++++-
src/shims/foreign_items/windows.rs | 30 ++++----
tests/run-pass/env.rs | 2 -
3 files changed, 136 insertions(+), 16 deletions(-)
diff --git a/src/shims/env.rs b/src/shims/env.rs
index 79c386e894..97d74ad0e0 100644
--- a/src/shims/env.rs
+++ b/src/shims/env.rs
@@ -1,6 +1,7 @@
use std::ffi::{OsString, OsStr};
use std::env;
use std::convert::TryFrom;
+use std::collections::hash_map::Values;
use crate::stacked_borrows::Tag;
use crate::rustc_target::abi::LayoutOf;
@@ -40,6 +41,10 @@ impl<'tcx> EnvVars<'tcx> {
}
ecx.update_environ()
}
+
+ pub(super) fn values(&self) -> InterpResult<'tcx, Values<'_, OsString, Pointer<Tag>>> {
+ Ok(self.map.values())
+ }
}
fn alloc_env_var_as_c_str<'mir, 'tcx>(
@@ -82,6 +87,79 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
})
}
+ fn getenvironmentvariablew(
+ &mut self,
+ name_op: OpTy<'tcx, Tag>, // LPCWSTR lpName
+ buf_op: OpTy<'tcx, Tag>, // LPWSTR lpBuffer
+ size_op: OpTy<'tcx, Tag>, // DWORD nSize
+ ) -> InterpResult<'tcx, u64> {
+ let this = self.eval_context_mut();
+ this.assert_target_os("windows", "GetEnvironmentVariableW");
+
+ let name_ptr = this.read_scalar(name_op)?.not_undef()?;
+ let name = this.read_os_str_from_wide_str(name_ptr)?;
+ Ok(match this.machine.env_vars.map.get(&name) {
+ Some(var_ptr) => {
+ // The offset is used to strip the "{name}=" part of the string.
+ let name_offset_bytes =
+ u64::try_from(name.len()).unwrap().checked_add(1).unwrap().checked_mul(2).unwrap();
+ let var_ptr = Scalar::from(var_ptr.offset(Size::from_bytes(name_offset_bytes), this)?);
+
+ let var_size = u64::try_from(this.read_os_str_from_wide_str(var_ptr)?.len()).unwrap();
+ let buf_size = u64::try_from(this.read_scalar(size_op)?.to_i32()?).unwrap();
+ let return_val = if var_size.checked_add(1).unwrap() > buf_size {
+ // If lpBuffer is not large enough to hold the data, the return value is the buffer size, in characters,
+ // required to hold the string and its terminating null character and the contents of lpBuffer are undefined.
+ var_size + 1
+ } else {
+ let buf_ptr = this.read_scalar(buf_op)?.not_undef()?;
+ for i in 0..var_size {
+ this.memory.copy(
+ this.force_ptr(var_ptr.ptr_offset(Size::from_bytes(i) * 2, this)?)?,
+ this.force_ptr(buf_ptr.ptr_offset(Size::from_bytes(i) * 2, this)?)?,
+ Size::from_bytes(2),
+ true,
+ )?;
+ }
+ // If the function succeeds, the return value is the number of characters stored in the buffer pointed to by lpBuffer,
+ // not including the terminating null character.
+ var_size
+ };
+ return_val
+ }
+ None => {
+ this.set_last_error(Scalar::from_u32(203))?; // ERROR_ENVVAR_NOT_FOUND
+ 0 // return zero upon failure
+ }
+ })
+ }
+
+ fn getenvironmentstringsw(&mut self) -> InterpResult<'tcx, Scalar<Tag>> {
+ let this = self.eval_context_mut();
+ this.assert_target_os("windows", "GetEnvironmentStringsW");
+
+ // Info on layout of environment blocks in Windows:
+ // https://docs.microsoft.com/en-us/windows/win32/procthread/environment-variables
+ let mut env_vars = std::ffi::OsString::new();
+ for &item in this.machine.env_vars.values()? {
+ let env_var = this.read_os_str_from_target_str(Scalar::from(item))?;
+ env_vars.push(env_var);
+ env_vars.push("\0");
+ }
+
+ // Allocate environment block
+ let tcx = this.tcx;
+ let env_block_size = env_vars.len().checked_add(1).unwrap();
+ let env_block_type = tcx.mk_array(tcx.types.u16, u64::try_from(env_block_size).unwrap());
+ let env_block_place = this.allocate(this.layout_of(env_block_type)?, MiriMemoryKind::WinHeap.into());
+
+ // Store environment variables to environment block
+ // Final null terminator(block terminator) is pushed by `write_os_str_to_wide_str`
+ this.write_os_str_to_wide_str(&env_vars, env_block_place, u64::try_from(env_block_size).unwrap())?;
+
+ Ok(env_block_place.ptr)
+ }
+
fn setenv(
&mut self,
name_op: OpTy<'tcx, Tag>,
@@ -118,6 +196,46 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
}
}
+ fn setenvironmentvariablew(
+ &mut self,
+ name_op: OpTy<'tcx, Tag>, // LPCWSTR lpName,
+ value_op: OpTy<'tcx, Tag>, // LPCWSTR lpValue,
+ ) -> InterpResult<'tcx, i32> {
+ let mut this = self.eval_context_mut();
+ this.assert_target_os("windows", "SetEnvironmentVariableW");
+
+ let name_ptr = this.read_scalar(name_op)?.not_undef()?;
+ let value_ptr = this.read_scalar(value_op)?.not_undef()?;
+
+ let mut new = None;
+ if !this.is_null(name_ptr)? {
+ let name = this.read_os_str_from_target_str(name_ptr)?;
+ if this.is_null(value_ptr)? {
+ // Delete environment variable `{name}`
+ if let Some(var) = this.machine.env_vars.map.remove(&name) {
+ this.memory.deallocate(var, None, MiriMemoryKind::Machine.into())?;
+ this.update_environ()?;
+ }
+ return Ok(1); // return non-zero on success
+ }
+ if !name.is_empty() && !name.to_string_lossy().contains('=') {
+ let value = this.read_os_str_from_target_str(value_ptr)?;
+ new = Some((name.to_owned(), value.to_owned()));
+ }
+ }
+ if let Some((name, value)) = new {
+ let var_ptr = alloc_env_var_as_target_str(&name, &value, &mut this)?;
+ if let Some(var) = this.machine.env_vars.map.insert(name, var_ptr) {
+ this.memory
+ .deallocate(var, None, MiriMemoryKind::Machine.into())?;
+ }
+ this.update_environ()?;
+ Ok(1) // return non-zero on success
+ } else {
+ Ok(0)
+ }
+ }
+
fn unsetenv(&mut self, name_op: OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
let this = self.eval_context_mut();
let target_os = &this.tcx.sess.target.target.target_os;
@@ -126,7 +244,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
let name_ptr = this.read_scalar(name_op)?.not_undef()?;
let mut success = None;
if !this.is_null(name_ptr)? {
- let name = this.read_os_str_from_c_str(name_ptr)?.to_owned();
+ let name = this.read_os_str_from_target_str(name_ptr)?.to_owned();
if !name.is_empty() && !name.to_string_lossy().contains('=') {
success = Some(this.machine.env_vars.map.remove(&name));
}
diff --git a/src/shims/foreign_items/windows.rs b/src/shims/foreign_items/windows.rs
index a35734573f..057f1c06a8 100644
--- a/src/shims/foreign_items/windows.rs
+++ b/src/shims/foreign_items/windows.rs
@@ -23,22 +23,26 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
// Environment related shims
"GetEnvironmentVariableW" => {
- // args[0] : LPCWSTR lpName (32-bit ptr to a const string of 16-bit Unicode chars)
- // args[1] : LPWSTR lpBuffer (32-bit pointer to a string of 16-bit Unicode chars)
- // lpBuffer : ptr to buffer that receives contents of the env_var as a null-terminated string.
- // Return `# of chars` stored in the buffer pointed to by lpBuffer, excluding null-terminator.
- // Return 0 upon failure.
-
- // This is not the env var you are looking for.
- this.set_last_error(Scalar::from_u32(203))?; // ERROR_ENVVAR_NOT_FOUND
- this.write_null(dest)?;
+ let result = this.getenvironmentvariablew(args[0], args[1], args[2])?;
+ this.write_scalar(Scalar::from_uint(result, dest.layout.size), dest)?;
}
"SetEnvironmentVariableW" => {
- // args[0] : LPCWSTR lpName (32-bit ptr to a const string of 16-bit Unicode chars)
- // args[1] : LPCWSTR lpValue (32-bit ptr to a const string of 16-bit Unicode chars)
- // Return nonzero if success, else return 0.
- throw_unsup_format!("can't set environment variable on Windows");
+ let result = this.setenvironmentvariablew(args[0], args[1])?;
+ this.write_scalar(Scalar::from_int(result, dest.layout.size), dest)?;
+ }
+
+ "GetEnvironmentStringsW" => {
+ let result = this.getenvironmentstringsw()?;
+ // If the function succeeds, the return value is a pointer to the environment block of the current process.
+ this.write_scalar(result, dest)?;
+ }
+
+ "FreeEnvironmentStringsW" => {
+ let old_vars_ptr = this.read_scalar(args[0])?.not_undef()?;
+ let result = this.memory.deallocate(this.force_ptr(old_vars_ptr)?, None, MiriMemoryKind::WinHeap.into()).is_ok();
+ // If the function succeeds, the return value is nonzero.
+ this.write_scalar(Scalar::from_int(result, dest.layout.size), dest)?;
}
// File related shims
diff --git a/tests/run-pass/env.rs b/tests/run-pass/env.rs
index c7506b23c1..23a3724ff7 100644
--- a/tests/run-pass/env.rs
+++ b/tests/run-pass/env.rs
@@ -1,5 +1,3 @@
-//ignore-windows: TODO env var emulation stubbed out on Windows
-
use std::env;
fn main() {
From cf5822af460f342721662a4dc959713a1cd7778c Mon Sep 17 00:00:00 2001
From: JOE1994 <joseph942010@gmail.com>
Date: Tue, 24 Mar 2020 16:00:11 -0400
Subject: [PATCH 2/9] exclude 'TERM' env_var to avoid terminfo trying to open
the termcap file
---
README.md | 2 +-
src/shims/env.rs | 6 +++++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/README.md b/README.md
index 59394830d7..03b995aa56 100644
--- a/README.md
+++ b/README.md
@@ -166,7 +166,7 @@ Several `-Z` flags are relevant for Miri:
* `-Zmiri-disable-stacked-borrows` disables checking the experimental
[Stacked Borrows] aliasing rules. This can make Miri run faster, but it also
means no aliasing violations will be detected.
-* `-Zmiri-disable-isolation` disables host host isolation. As a consequence,
+* `-Zmiri-disable-isolation` disables host isolation. As a consequence,
the program has access to host resources such as environment variables, file
systems, and randomness.
* `-Zmiri-ignore-leaks` disables the memory leak checker.
diff --git a/src/shims/env.rs b/src/shims/env.rs
index 97d74ad0e0..32ed65b398 100644
--- a/src/shims/env.rs
+++ b/src/shims/env.rs
@@ -24,8 +24,12 @@ pub struct EnvVars<'tcx> {
impl<'tcx> EnvVars<'tcx> {
pub(crate) fn init<'mir>(
ecx: &mut InterpCx<'mir, 'tcx, Evaluator<'tcx>>,
- excluded_env_vars: Vec<String>,
+ mut excluded_env_vars: Vec<String>,
) -> InterpResult<'tcx> {
+ if ecx.tcx.sess.target.target.target_os.to_lowercase() == "windows" {
+ // Exclude `TERM` var to avoid terminfo trying to open the termcap file.
+ excluded_env_vars.push("TERM".to_owned());
+ }
if ecx.machine.communicate {
let target_os = ecx.tcx.sess.target.target.target_os.as_str();
for (name, value) in env::vars() {
From 2051805e957d307f7f084172b61cf0a6d69edfc9 Mon Sep 17 00:00:00 2001
From: JOE1994 <joseph942010@gmail.com>
Date: Wed, 25 Mar 2020 00:52:53 -0400
Subject: [PATCH 3/9] follow-up to reviews
---
src/shims/env.rs | 110 +++++++++++++++++------------
src/shims/foreign_items/windows.rs | 9 ++-
2 files changed, 69 insertions(+), 50 deletions(-)
diff --git a/src/shims/env.rs b/src/shims/env.rs
index 32ed65b398..2e647589f4 100644
--- a/src/shims/env.rs
+++ b/src/shims/env.rs
@@ -1,3 +1,5 @@
+#![allow(non_snake_case)]
+
use std::ffi::{OsString, OsStr};
use std::env;
use std::convert::TryFrom;
@@ -26,7 +28,7 @@ impl<'tcx> EnvVars<'tcx> {
ecx: &mut InterpCx<'mir, 'tcx, Evaluator<'tcx>>,
mut excluded_env_vars: Vec<String>,
) -> InterpResult<'tcx> {
- if ecx.tcx.sess.target.target.target_os.to_lowercase() == "windows" {
+ if ecx.tcx.sess.target.target.target_os == "windows" {
// Exclude `TERM` var to avoid terminfo trying to open the termcap file.
excluded_env_vars.push("TERM".to_owned());
}
@@ -46,7 +48,7 @@ impl<'tcx> EnvVars<'tcx> {
ecx.update_environ()
}
- pub(super) fn values(&self) -> InterpResult<'tcx, Values<'_, OsString, Pointer<Tag>>> {
+ fn values(&self) -> InterpResult<'tcx, Values<'_, OsString, Pointer<Tag>>> {
Ok(self.map.values())
}
}
@@ -73,6 +75,28 @@ fn alloc_env_var_as_wide_str<'mir, 'tcx>(
Ok(ecx.alloc_os_str_as_wide_str(name_osstring.as_os_str(), MiriMemoryKind::Machine.into()))
}
+fn alloc_env_var_as_c_str<'mir, 'tcx>(
+ name: &OsStr,
+ value: &OsStr,
+ ecx: &mut InterpCx<'mir, 'tcx, Evaluator<'tcx>>,
+) -> InterpResult<'tcx, Pointer<Tag>> {
+ let mut name_osstring = name.to_os_string();
+ name_osstring.push("=");
+ name_osstring.push(value);
+ Ok(ecx.alloc_os_str_as_c_str(name_osstring.as_os_str(), MiriMemoryKind::Machine.into()))
+}
+
+fn alloc_env_var_as_wide_str<'mir, 'tcx>(
+ name: &OsStr,
+ value: &OsStr,
+ ecx: &mut InterpCx<'mir, 'tcx, Evaluator<'tcx>>,
+) -> InterpResult<'tcx, Pointer<Tag>> {
+ let mut name_osstring = name.to_os_string();
+ name_osstring.push("=");
+ name_osstring.push(value);
+ Ok(ecx.alloc_os_str_as_wide_str(name_osstring.as_os_str(), MiriMemoryKind::Machine.into()))
+}
+
impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {}
pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> {
fn getenv(&mut self, name_op: OpTy<'tcx, Tag>) -> InterpResult<'tcx, Scalar<Tag>> {
@@ -91,7 +115,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
})
}
- fn getenvironmentvariablew(
+ fn GetEnvironmentVariableW(
&mut self,
name_op: OpTy<'tcx, Tag>, // LPCWSTR lpName
buf_op: OpTy<'tcx, Tag>, // LPWSTR lpBuffer
@@ -110,21 +134,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
let var_ptr = Scalar::from(var_ptr.offset(Size::from_bytes(name_offset_bytes), this)?);
let var_size = u64::try_from(this.read_os_str_from_wide_str(var_ptr)?.len()).unwrap();
- let buf_size = u64::try_from(this.read_scalar(size_op)?.to_i32()?).unwrap();
+ // `buf_size` represent size in characters.
+ let buf_size = u64::try_from(this.read_scalar(size_op)?.to_u32()?).unwrap();
let return_val = if var_size.checked_add(1).unwrap() > buf_size {
// If lpBuffer is not large enough to hold the data, the return value is the buffer size, in characters,
// required to hold the string and its terminating null character and the contents of lpBuffer are undefined.
var_size + 1
} else {
let buf_ptr = this.read_scalar(buf_op)?.not_undef()?;
- for i in 0..var_size {
- this.memory.copy(
- this.force_ptr(var_ptr.ptr_offset(Size::from_bytes(i) * 2, this)?)?,
- this.force_ptr(buf_ptr.ptr_offset(Size::from_bytes(i) * 2, this)?)?,
- Size::from_bytes(2),
- true,
- )?;
- }
+ let bytes_to_be_copied = var_size.checked_add(1).unwrap().checked_mul(2).unwrap();
+ this.memory.copy(this.force_ptr(var_ptr)?, this.force_ptr(buf_ptr)?, Size::from_bytes(bytes_to_be_copied), true)?;
// If the function succeeds, the return value is the number of characters stored in the buffer pointed to by lpBuffer,
// not including the terminating null character.
var_size
@@ -138,7 +157,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
})
}
- fn getenvironmentstringsw(&mut self) -> InterpResult<'tcx, Scalar<Tag>> {
+ fn GetEnvironmentStringsW(&mut self) -> InterpResult<'tcx, Scalar<Tag>> {
let this = self.eval_context_mut();
this.assert_target_os("windows", "GetEnvironmentStringsW");
@@ -146,22 +165,23 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
// https://docs.microsoft.com/en-us/windows/win32/procthread/environment-variables
let mut env_vars = std::ffi::OsString::new();
for &item in this.machine.env_vars.values()? {
- let env_var = this.read_os_str_from_target_str(Scalar::from(item))?;
+ let env_var = this.read_os_str_from_wide_str(Scalar::from(item))?;
env_vars.push(env_var);
env_vars.push("\0");
}
+ // Allocate environment block & Store environment variables to environment block.
+ // Final null terminator(block terminator) is added by `alloc_os_str_to_wide_str`.
+ let envblock_ptr = this.alloc_os_str_as_wide_str(&env_vars, MiriMemoryKind::WinHeap.into());
- // Allocate environment block
- let tcx = this.tcx;
- let env_block_size = env_vars.len().checked_add(1).unwrap();
- let env_block_type = tcx.mk_array(tcx.types.u16, u64::try_from(env_block_size).unwrap());
- let env_block_place = this.allocate(this.layout_of(env_block_type)?, MiriMemoryKind::WinHeap.into());
-
- // Store environment variables to environment block
- // Final null terminator(block terminator) is pushed by `write_os_str_to_wide_str`
- this.write_os_str_to_wide_str(&env_vars, env_block_place, u64::try_from(env_block_size).unwrap())?;
+ Ok(envblock_ptr.into())
+ }
+
+ fn FreeEnvironmentStringsW(&mut self, env_block_op: OpTy<'tcx, Tag>) -> InterpResult<'tcx, bool> {
+ let this = self.eval_context_mut();
+ this.assert_target_os("windows", "FreeEnvironmentStringsW");
- Ok(env_block_place.ptr)
+ let env_block_ptr = this.read_scalar(env_block_op)?.not_undef()?;
+ Ok(this.memory.deallocate(this.force_ptr(env_block_ptr)?, None, MiriMemoryKind::WinHeap.into()).is_ok())
}
fn setenv(
@@ -200,7 +220,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
}
}
- fn setenvironmentvariablew(
+ fn SetEnvironmentVariableW(
&mut self,
name_op: OpTy<'tcx, Tag>, // LPCWSTR lpName,
value_op: OpTy<'tcx, Tag>, // LPCWSTR lpValue,
@@ -211,32 +231,32 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
let name_ptr = this.read_scalar(name_op)?.not_undef()?;
let value_ptr = this.read_scalar(value_op)?.not_undef()?;
- let mut new = None;
- if !this.is_null(name_ptr)? {
- let name = this.read_os_str_from_target_str(name_ptr)?;
- if this.is_null(value_ptr)? {
- // Delete environment variable `{name}`
- if let Some(var) = this.machine.env_vars.map.remove(&name) {
- this.memory.deallocate(var, None, MiriMemoryKind::Machine.into())?;
- this.update_environ()?;
- }
- return Ok(1); // return non-zero on success
- }
- if !name.is_empty() && !name.to_string_lossy().contains('=') {
- let value = this.read_os_str_from_target_str(value_ptr)?;
- new = Some((name.to_owned(), value.to_owned()));
- }
+ if this.is_null(name_ptr)? {
+ // ERROR CODE is not clearly explained in docs.. For now, throw UB instead.
+ throw_ub_format!("Pointer to environment variable name is NULL");
}
- if let Some((name, value)) = new {
- let var_ptr = alloc_env_var_as_target_str(&name, &value, &mut this)?;
+
+ let name = this.read_os_str_from_wide_str(name_ptr)?;
+ if name.is_empty() {
+ throw_unsup_format!("Environment variable name is an empty string");
+ } else if name.to_string_lossy().contains('=') {
+ throw_unsup_format!("Environment variable name contains '='");
+ } else if this.is_null(value_ptr)? {
+ // Delete environment variable `{name}`
+ if let Some(var) = this.machine.env_vars.map.remove(&name) {
+ this.memory.deallocate(var, None, MiriMemoryKind::Machine.into())?;
+ this.update_environ()?;
+ }
+ Ok(1) // return non-zero on success
+ } else {
+ let value = this.read_os_str_from_wide_str(value_ptr)?;
+ let var_ptr = alloc_env_var_as_wide_str(&name, &value, &mut this)?;
if let Some(var) = this.machine.env_vars.map.insert(name, var_ptr) {
this.memory
.deallocate(var, None, MiriMemoryKind::Machine.into())?;
}
this.update_environ()?;
Ok(1) // return non-zero on success
- } else {
- Ok(0)
}
}
@@ -248,7 +268,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
let name_ptr = this.read_scalar(name_op)?.not_undef()?;
let mut success = None;
if !this.is_null(name_ptr)? {
- let name = this.read_os_str_from_target_str(name_ptr)?.to_owned();
+ let name = this.read_os_str_from_c_str(name_ptr)?.to_owned();
if !name.is_empty() && !name.to_string_lossy().contains('=') {
success = Some(this.machine.env_vars.map.remove(&name));
}
diff --git a/src/shims/foreign_items/windows.rs b/src/shims/foreign_items/windows.rs
index 057f1c06a8..cc64057135 100644
--- a/src/shims/foreign_items/windows.rs
+++ b/src/shims/foreign_items/windows.rs
@@ -23,24 +23,23 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
// Environment related shims
"GetEnvironmentVariableW" => {
- let result = this.getenvironmentvariablew(args[0], args[1], args[2])?;
+ let result = this.GetEnvironmentVariableW(args[0], args[1], args[2])?;
this.write_scalar(Scalar::from_uint(result, dest.layout.size), dest)?;
}
"SetEnvironmentVariableW" => {
- let result = this.setenvironmentvariablew(args[0], args[1])?;
+ let result = this.SetEnvironmentVariableW(args[0], args[1])?;
this.write_scalar(Scalar::from_int(result, dest.layout.size), dest)?;
}
"GetEnvironmentStringsW" => {
- let result = this.getenvironmentstringsw()?;
+ let result = this.GetEnvironmentStringsW()?;
// If the function succeeds, the return value is a pointer to the environment block of the current process.
this.write_scalar(result, dest)?;
}
"FreeEnvironmentStringsW" => {
- let old_vars_ptr = this.read_scalar(args[0])?.not_undef()?;
- let result = this.memory.deallocate(this.force_ptr(old_vars_ptr)?, None, MiriMemoryKind::WinHeap.into()).is_ok();
+ let result = this.FreeEnvironmentStringsW(args[0])?;
// If the function succeeds, the return value is nonzero.
this.write_scalar(Scalar::from_int(result, dest.layout.size), dest)?;
}
From 4e38fbe6be6d5c8d67de5ca65ab763f9f314a398 Mon Sep 17 00:00:00 2001
From: JOE1994 <joseph942010@gmail.com>
Date: Wed, 25 Mar 2020 10:38:27 -0400
Subject: [PATCH 4/9] follow-up2 to review (few issues not resolved yet)
---
src/shims/env.rs | 45 +++++++++++++++++-------------
src/shims/foreign_items/windows.rs | 4 +--
2 files changed, 26 insertions(+), 23 deletions(-)
diff --git a/src/shims/env.rs b/src/shims/env.rs
index 2e647589f4..cc7c305596 100644
--- a/src/shims/env.rs
+++ b/src/shims/env.rs
@@ -1,9 +1,6 @@
-#![allow(non_snake_case)]
-
use std::ffi::{OsString, OsStr};
use std::env;
use std::convert::TryFrom;
-use std::collections::hash_map::Values;
use crate::stacked_borrows::Tag;
use crate::rustc_target::abi::LayoutOf;
@@ -47,10 +44,6 @@ impl<'tcx> EnvVars<'tcx> {
}
ecx.update_environ()
}
-
- fn values(&self) -> InterpResult<'tcx, Values<'_, OsString, Pointer<Tag>>> {
- Ok(self.map.values())
- }
}
fn alloc_env_var_as_c_str<'mir, 'tcx>(
@@ -115,11 +108,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
})
}
+ #[allow(non_snake_case)]
fn GetEnvironmentVariableW(
&mut self,
- name_op: OpTy<'tcx, Tag>, // LPCWSTR lpName
- buf_op: OpTy<'tcx, Tag>, // LPWSTR lpBuffer
- size_op: OpTy<'tcx, Tag>, // DWORD nSize
+ name_op: OpTy<'tcx, Tag>, // LPCWSTR
+ buf_op: OpTy<'tcx, Tag>, // LPWSTR
+ size_op: OpTy<'tcx, Tag>, // DWORD
) -> InterpResult<'tcx, u64> {
let this = self.eval_context_mut();
this.assert_target_os("windows", "GetEnvironmentVariableW");
@@ -134,7 +128,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
let var_ptr = Scalar::from(var_ptr.offset(Size::from_bytes(name_offset_bytes), this)?);
let var_size = u64::try_from(this.read_os_str_from_wide_str(var_ptr)?.len()).unwrap();
- // `buf_size` represent size in characters.
+ // `buf_size` represents the size in characters.
let buf_size = u64::try_from(this.read_scalar(size_op)?.to_u32()?).unwrap();
let return_val = if var_size.checked_add(1).unwrap() > buf_size {
// If lpBuffer is not large enough to hold the data, the return value is the buffer size, in characters,
@@ -157,6 +151,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
})
}
+ #[allow(non_snake_case)]
fn GetEnvironmentStringsW(&mut self) -> InterpResult<'tcx, Scalar<Tag>> {
let this = self.eval_context_mut();
this.assert_target_os("windows", "GetEnvironmentStringsW");
@@ -164,24 +159,33 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
// Info on layout of environment blocks in Windows:
// https://docs.microsoft.com/en-us/windows/win32/procthread/environment-variables
let mut env_vars = std::ffi::OsString::new();
- for &item in this.machine.env_vars.values()? {
+ for &item in this.machine.env_vars.map.values() {
let env_var = this.read_os_str_from_wide_str(Scalar::from(item))?;
env_vars.push(env_var);
env_vars.push("\0");
}
// Allocate environment block & Store environment variables to environment block.
// Final null terminator(block terminator) is added by `alloc_os_str_to_wide_str`.
+ // FIXME: MemoryKind should be `MiMemoryKind::Machine`,
+ // but using it results in a Stacked Borrows error when running MIRI on 'tests/run-pass/env.rs'
+ // For now, use `MiriMemoryKind::WinHeap` instead.
let envblock_ptr = this.alloc_os_str_as_wide_str(&env_vars, MiriMemoryKind::WinHeap.into());
-
+ // If the function succeeds, the return value is a pointer to the environment block of the current process.
Ok(envblock_ptr.into())
}
- fn FreeEnvironmentStringsW(&mut self, env_block_op: OpTy<'tcx, Tag>) -> InterpResult<'tcx, bool> {
+ #[allow(non_snake_case)]
+ fn FreeEnvironmentStringsW(&mut self, env_block_op: OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
let this = self.eval_context_mut();
this.assert_target_os("windows", "FreeEnvironmentStringsW");
let env_block_ptr = this.read_scalar(env_block_op)?.not_undef()?;
- Ok(this.memory.deallocate(this.force_ptr(env_block_ptr)?, None, MiriMemoryKind::WinHeap.into()).is_ok())
+ // FIXME: MemoryKind should be `MiMemoryKind::Machine`,
+ // but using it results in a Stacked Borrows error when running MIRI on 'tests/run-pass/env.rs'
+ // For now, use `MiriMemoryKind::WinHeap` instead.
+ let result = this.memory.deallocate(this.force_ptr(env_block_ptr)?, None, MiriMemoryKind::WinHeap.into());
+ // If the function succeeds, the return value is nonzero.
+ Ok(result.is_ok() as i32)
}
fn setenv(
@@ -220,10 +224,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
}
}
+ #[allow(non_snake_case)]
fn SetEnvironmentVariableW(
&mut self,
- name_op: OpTy<'tcx, Tag>, // LPCWSTR lpName,
- value_op: OpTy<'tcx, Tag>, // LPCWSTR lpValue,
+ name_op: OpTy<'tcx, Tag>, // LPCWSTR
+ value_op: OpTy<'tcx, Tag>, // LPCWSTR
) -> InterpResult<'tcx, i32> {
let mut this = self.eval_context_mut();
this.assert_target_os("windows", "SetEnvironmentVariableW");
@@ -233,14 +238,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
if this.is_null(name_ptr)? {
// ERROR CODE is not clearly explained in docs.. For now, throw UB instead.
- throw_ub_format!("Pointer to environment variable name is NULL");
+ throw_ub_format!("pointer to environment variable name is NULL");
}
let name = this.read_os_str_from_wide_str(name_ptr)?;
if name.is_empty() {
- throw_unsup_format!("Environment variable name is an empty string");
+ throw_unsup_format!("environment variable name is an empty string");
} else if name.to_string_lossy().contains('=') {
- throw_unsup_format!("Environment variable name contains '='");
+ throw_unsup_format!("environment variable name contains '='");
} else if this.is_null(value_ptr)? {
// Delete environment variable `{name}`
if let Some(var) = this.machine.env_vars.map.remove(&name) {
diff --git a/src/shims/foreign_items/windows.rs b/src/shims/foreign_items/windows.rs
index cc64057135..a64ef0f129 100644
--- a/src/shims/foreign_items/windows.rs
+++ b/src/shims/foreign_items/windows.rs
@@ -34,14 +34,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
"GetEnvironmentStringsW" => {
let result = this.GetEnvironmentStringsW()?;
- // If the function succeeds, the return value is a pointer to the environment block of the current process.
this.write_scalar(result, dest)?;
}
"FreeEnvironmentStringsW" => {
let result = this.FreeEnvironmentStringsW(args[0])?;
- // If the function succeeds, the return value is nonzero.
- this.write_scalar(Scalar::from_int(result, dest.layout.size), dest)?;
+ this.write_scalar(Scalar::from_i32(result), dest)?;
}
// File related shims
From f3e3af4beec260541dbeeead0a824caee7624e77 Mon Sep 17 00:00:00 2001
From: JOE1994 <joseph942010@gmail.com>
Date: Fri, 27 Mar 2020 09:18:13 -0400
Subject: [PATCH 5/9] adjust to change of 'fn write_os_str_to_wide_str'
---
src/shims/env.rs | 52 ++++++++++++++----------------------------------
1 file changed, 15 insertions(+), 37 deletions(-)
diff --git a/src/shims/env.rs b/src/shims/env.rs
index cc7c305596..794a1aab42 100644
--- a/src/shims/env.rs
+++ b/src/shims/env.rs
@@ -25,12 +25,12 @@ impl<'tcx> EnvVars<'tcx> {
ecx: &mut InterpCx<'mir, 'tcx, Evaluator<'tcx>>,
mut excluded_env_vars: Vec<String>,
) -> InterpResult<'tcx> {
- if ecx.tcx.sess.target.target.target_os == "windows" {
+ let target_os = ecx.tcx.sess.target.target.target_os.as_str();
+ if target_os == "windows" {
// Exclude `TERM` var to avoid terminfo trying to open the termcap file.
excluded_env_vars.push("TERM".to_owned());
}
if ecx.machine.communicate {
- let target_os = ecx.tcx.sess.target.target.target_os.as_str();
for (name, value) in env::vars() {
if !excluded_env_vars.contains(&name) {
let var_ptr = match target_os {
@@ -68,28 +68,6 @@ fn alloc_env_var_as_wide_str<'mir, 'tcx>(
Ok(ecx.alloc_os_str_as_wide_str(name_osstring.as_os_str(), MiriMemoryKind::Machine.into()))
}
-fn alloc_env_var_as_c_str<'mir, 'tcx>(
- name: &OsStr,
- value: &OsStr,
- ecx: &mut InterpCx<'mir, 'tcx, Evaluator<'tcx>>,
-) -> InterpResult<'tcx, Pointer<Tag>> {
- let mut name_osstring = name.to_os_string();
- name_osstring.push("=");
- name_osstring.push(value);
- Ok(ecx.alloc_os_str_as_c_str(name_osstring.as_os_str(), MiriMemoryKind::Machine.into()))
-}
-
-fn alloc_env_var_as_wide_str<'mir, 'tcx>(
- name: &OsStr,
- value: &OsStr,
- ecx: &mut InterpCx<'mir, 'tcx, Evaluator<'tcx>>,
-) -> InterpResult<'tcx, Pointer<Tag>> {
- let mut name_osstring = name.to_os_string();
- name_osstring.push("=");
- name_osstring.push(value);
- Ok(ecx.alloc_os_str_as_wide_str(name_osstring.as_os_str(), MiriMemoryKind::Machine.into()))
-}
-
impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {}
pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> {
fn getenv(&mut self, name_op: OpTy<'tcx, Tag>) -> InterpResult<'tcx, Scalar<Tag>> {
@@ -126,26 +104,26 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
let name_offset_bytes =
u64::try_from(name.len()).unwrap().checked_add(1).unwrap().checked_mul(2).unwrap();
let var_ptr = Scalar::from(var_ptr.offset(Size::from_bytes(name_offset_bytes), this)?);
+ let var = this.read_os_str_from_wide_str(var_ptr)?;
- let var_size = u64::try_from(this.read_os_str_from_wide_str(var_ptr)?.len()).unwrap();
+ let buf_ptr = this.read_scalar(buf_op)?.not_undef()?;
// `buf_size` represents the size in characters.
let buf_size = u64::try_from(this.read_scalar(size_op)?.to_u32()?).unwrap();
- let return_val = if var_size.checked_add(1).unwrap() > buf_size {
- // If lpBuffer is not large enough to hold the data, the return value is the buffer size, in characters,
- // required to hold the string and its terminating null character and the contents of lpBuffer are undefined.
- var_size + 1
- } else {
- let buf_ptr = this.read_scalar(buf_op)?.not_undef()?;
- let bytes_to_be_copied = var_size.checked_add(1).unwrap().checked_mul(2).unwrap();
- this.memory.copy(this.force_ptr(var_ptr)?, this.force_ptr(buf_ptr)?, Size::from_bytes(bytes_to_be_copied), true)?;
+ let (success, len) = this.write_os_str_to_wide_str(&var, buf_ptr, buf_size)?;
+
+ if success {
// If the function succeeds, the return value is the number of characters stored in the buffer pointed to by lpBuffer,
// not including the terminating null character.
- var_size
- };
- return_val
+ len
+ } else {
+ // If lpBuffer is not large enough to hold the data, the return value is the buffer size, in characters,
+ // required to hold the string and its terminating null character and the contents of lpBuffer are undefined.
+ len + 1
+ }
}
None => {
- this.set_last_error(Scalar::from_u32(203))?; // ERROR_ENVVAR_NOT_FOUND
+ let envvar_not_found = this.eval_path_scalar(&["std", "sys", "windows", "c", "ERROR_ENVVAR_NOT_FOUND"])?;
+ this.set_last_error(envvar_not_found.not_undef()?)?;
0 // return zero upon failure
}
})
From eaca17fcc31a74690bc89d6b2aa7c41b4aedb79a Mon Sep 17 00:00:00 2001
From: JOE1994 <joseph942010@gmail.com>
Date: Fri, 27 Mar 2020 09:59:42 -0400
Subject: [PATCH 6/9] add reference to issue#1013
---
src/shims/env.rs | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/shims/env.rs b/src/shims/env.rs
index 794a1aab42..eee9a53918 100644
--- a/src/shims/env.rs
+++ b/src/shims/env.rs
@@ -27,9 +27,11 @@ impl<'tcx> EnvVars<'tcx> {
) -> InterpResult<'tcx> {
let target_os = ecx.tcx.sess.target.target.target_os.as_str();
if target_os == "windows" {
- // Exclude `TERM` var to avoid terminfo trying to open the termcap file.
+ // Temporary hack: Exclude `TERM` var to avoid terminfo trying to open the termcap file.
+ // Can be removed once Issue#1013(Implement file system access for Windows) is resolved.
excluded_env_vars.push("TERM".to_owned());
}
+
if ecx.machine.communicate {
for (name, value) in env::vars() {
if !excluded_env_vars.contains(&name) {
From 3fe71dff5a58efaabd1686e2a25c09adb8d74092 Mon Sep 17 00:00:00 2001
From: Youngsuk Kim <joseph942010@gmail.com>
Date: Fri, 27 Mar 2020 10:15:35 -0400
Subject: [PATCH 7/9] Modify reference to issue 1013
Co-Authored-By: Ralf Jung <post@ralfj.de>
---
src/shims/env.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/shims/env.rs b/src/shims/env.rs
index eee9a53918..b6d7010263 100644
--- a/src/shims/env.rs
+++ b/src/shims/env.rs
@@ -28,7 +28,7 @@ impl<'tcx> EnvVars<'tcx> {
let target_os = ecx.tcx.sess.target.target.target_os.as_str();
if target_os == "windows" {
// Temporary hack: Exclude `TERM` var to avoid terminfo trying to open the termcap file.
- // Can be removed once Issue#1013(Implement file system access for Windows) is resolved.
+ // Can be removed once https://github.com/rust-lang/miri/issues/1013 is resolved.
excluded_env_vars.push("TERM".to_owned());
}
From ea836eeb0d72d5f983489f283ea7fc5f6a7ad442 Mon Sep 17 00:00:00 2001
From: JOE1994 <joseph942010@gmail.com>
Date: Fri, 27 Mar 2020 14:18:19 -0400
Subject: [PATCH 8/9] remove or update 'ignore-windows' annotations in some
tests
---
tests/compile-fail/environ-gets-deallocated.rs | 2 +-
tests/run-pass/env-exclude.rs | 1 -
tests/run-pass/env-without-isolation.rs | 1 -
3 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/tests/compile-fail/environ-gets-deallocated.rs b/tests/compile-fail/environ-gets-deallocated.rs
index dd00aef450..0f374a2e3f 100644
--- a/tests/compile-fail/environ-gets-deallocated.rs
+++ b/tests/compile-fail/environ-gets-deallocated.rs
@@ -1,4 +1,4 @@
-//ignore-windows: TODO env var emulation stubbed out on Windows
+//ignore-windows: Windows does not have a global environ list that the program can access directly
#[cfg(target_os="linux")]
fn get_environ() -> *const *const u8 {
diff --git a/tests/run-pass/env-exclude.rs b/tests/run-pass/env-exclude.rs
index efcf7a7561..1e251084f0 100644
--- a/tests/run-pass/env-exclude.rs
+++ b/tests/run-pass/env-exclude.rs
@@ -1,4 +1,3 @@
-// ignore-windows: TODO env var emulation stubbed out on Windows
// compile-flags: -Zmiri-disable-isolation -Zmiri-env-exclude=MIRI_ENV_VAR_TEST
fn main() {
diff --git a/tests/run-pass/env-without-isolation.rs b/tests/run-pass/env-without-isolation.rs
index 537e0923d2..6384762098 100644
--- a/tests/run-pass/env-without-isolation.rs
+++ b/tests/run-pass/env-without-isolation.rs
@@ -1,4 +1,3 @@
-// ignore-windows: TODO env var emulation stubbed out on Windows
// compile-flags: -Zmiri-disable-isolation
fn main() {
From 579b3c49dac9def1495f7d97d4f5b2771bbeefd8 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Fri, 27 Mar 2020 20:36:18 +0100
Subject: [PATCH 9/9] adjust MemoryKind comment
---
src/shims/env.rs | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/src/shims/env.rs b/src/shims/env.rs
index b6d7010263..ed689b1f42 100644
--- a/src/shims/env.rs
+++ b/src/shims/env.rs
@@ -146,9 +146,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
}
// Allocate environment block & Store environment variables to environment block.
// Final null terminator(block terminator) is added by `alloc_os_str_to_wide_str`.
- // FIXME: MemoryKind should be `MiMemoryKind::Machine`,
- // but using it results in a Stacked Borrows error when running MIRI on 'tests/run-pass/env.rs'
- // For now, use `MiriMemoryKind::WinHeap` instead.
+ // FIXME: MemoryKind should be `Machine`, blocked on https://github.com/rust-lang/rust/pull/70479.
let envblock_ptr = this.alloc_os_str_as_wide_str(&env_vars, MiriMemoryKind::WinHeap.into());
// If the function succeeds, the return value is a pointer to the environment block of the current process.
Ok(envblock_ptr.into())
@@ -160,9 +158,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
this.assert_target_os("windows", "FreeEnvironmentStringsW");
let env_block_ptr = this.read_scalar(env_block_op)?.not_undef()?;
- // FIXME: MemoryKind should be `MiMemoryKind::Machine`,
- // but using it results in a Stacked Borrows error when running MIRI on 'tests/run-pass/env.rs'
- // For now, use `MiriMemoryKind::WinHeap` instead.
+ // FIXME: MemoryKind should be `Machine`, blocked on https://github.com/rust-lang/rust/pull/70479.
let result = this.memory.deallocate(this.force_ptr(env_block_ptr)?, None, MiriMemoryKind::WinHeap.into());
// If the function succeeds, the return value is nonzero.
Ok(result.is_ok() as i32)