Add helper alloc_os_str_as_c_str and use it in env_var emulation

[JOE1994]

First part of the plan laid out in #707 (comment).

[RalfJung]

I'm afraid there are some serious bugs in this code: this reads a UTF-16 string via &[u8] and treats it like UTF-8! That will not work.

[JOE1994]

My local build of MIRI was successful, but CI is failing. It seems that CI is buliding MIRI with the latest version of rustc specified in miri/rust-version, while I locally built MIRI with an older version of rustc.

Update : there were some changes made to my commit while merging conflicts with the master branch, and I had forgot about that... I will push an update soon to fix them.

[RalfJung]

Looks like CI is green. :) I'll put this onto my review queue.

However, I am sick, and this might be a longer thing, so don't hold your breath... it could take a while.

[JOE1994]

@RalfJung
I'm sorry to hear that you are sick 😢 I hope you feel better soon, and please take your time.

[RalfJung]

Please update the title and description of this PR; it has little to do with its content now. ;)

[JOE1994]

My local build compiles without errors, but there is a conflict in src/helpers.rs.

[RalfJung]

Well, then please rebase and resolve those conflicts. :)

[JOE1994]

The new function fn alloc_os_str_as_c_str in this PR uses fn os_str_to_bytes.
In the master branch, it seems like the definition of fn os_str_to_bytes has been moved into fn write_os_str_to_c_str, making it only callable inside write_os_str_to_c_str. Would it be okay if I move the definition of fn os_str_to_bytes out of fn write_os_str_to_c_str??

miri/src/helpers.rs

Lines 474 to 487 in aafb7c9

	#[cfg(target_os = "unix")]
	fn os_str_to_bytes<'tcx, 'a>(os_str: &'a OsStr) -> InterpResult<'tcx, &'a [u8]> {
	std::os::unix::ffi::OsStringExt::into_bytes(os_str)
	}
	#[cfg(not(target_os = "unix"))]
	fn os_str_to_bytes<'tcx, 'a>(os_str: &'a OsStr) -> InterpResult<'tcx, &'a [u8]> {
	// On non-unix platforms the best we can do to transform bytes from/to OS strings is to do the
	// intermediate transformation into strings. Which invalidates non-utf8 paths that are actually
	// valid.
	os_str
	.to_str()
	.map(|s| s.as_bytes())
	.ok_or_else(|| err_unsup_format!("{:?} is not a valid utf-8 string", os_str).into())
	}

[RalfJung]

Yeah I deliberately made it private as I thought nothing else would need it.

From what I can tell, the alloc function calls this just to be able to determine the length? Hm... I guess we could use OsStr::len for that? If the length is wrong, we'll either allocate too much memory or the write will fail, but nothing will silently go wrong. And I think the length is actually right for all platforms we support.

[RalfJung]

Please rebase instead of adding merge commits to a PR.

[JOE1994]

It's a little lame, but I had already made a merge in this PR with the MIRI master branch once, and I think this PR would mess up the commit history of the project. I will close this PR and instead will submit a new PR right away.

[RalfJung]

Wait, why that? Just rebase the branch and force-push.

[JOE1994]

Wait, why that? Just rebase the branch and force-push.

I didn't have a separate working branch in my local fork, and I wasn't sure on which point I should set as the rebase point. I'm not familiar with using rebase, so I thought it'd be better off to create a new PR with a fresh fork, rather than risking messing up the commit history.