Move last error into memory #977
Conversation
|
The rustfmt changes are a bit annoying intermingled like that. Can you create a rustfmt commit on master and rebase this PR over it? |
|
@bors r+ |
|
📌 Commit 459c65a has been approved by |
Move last error into memory These changes move the `Evaluator::last_error` into miri's memory and implement the `__errno_location()` shim (which is used by the file handling functions when they fail).
|
☀️ Test successful - checks-travis, status-appveyor |
| @@ -88,7 +92,7 @@ pub struct Evaluator<'tcx> { | |||
| pub(crate) cmd_line: Option<Pointer<Tag>>, | |||
|
|
|||
| /// Last OS error. | |||
There was a problem hiding this comment.
Now that this is a memory location, the comment should say e.g. how big it is (seems to always be 4 bytes?). Also, this was previously used for Windows stuff, errno_location is a Linux thing; can the two really share the same field?
There was a problem hiding this comment.
I'll add a comment about its size. It should work for windows in principle because windows uses an u32 and linux uses an i32.
I think only disadvantage of adding this is that we have the indirection of having to get the location pointer and then get the pointeé. But it shouldn't affect the capablities of setting and getting the error in either of the two platforms.
| .not_undef() | ||
| } | ||
|
|
||
| fn consume_io_error(&mut self, e: std::io::Error) -> InterpResult<'tcx> { |
There was a problem hiding this comment.
"consume" is an odd choice of words for a "set_last_error" convenience wrapper.
Is it really worth to have a helper for set_last_error(e.raw_os_error().unwrap())? Maybe it is, but then please call it something that indicates in its name that this sets the last_error variable.
There was a problem hiding this comment.
Well I think its useful for fs related stuff because it allows to use the raw error itself to set errno instead of having to match the io::Error and set the proper errno for each case. I'll think about a decent name and add some comments explaining its behavior.
|
This PR comes without any test... but looks like #976 some some errno things? |
Yes, there is no test in this PR. But the Edit: #992 |
These changes move the
Evaluator::last_errorinto miri's memory and implement the__errno_location()shim (which is used by the file handling functions when they fail).