rustc_const_eval: Expose APIs for signalling foreign accesses to memory #141391
Conversation
rustbot
added
the
T-compiler
This comment has been minimized.
This comment has been minimized.
rustbot
added
the
S-waiting-on-review
| pub fn apply_accesses( | ||
| &mut self, | ||
| mut ids: Vec<AllocId>, | ||
| reads: Vec<std::ops::Range<u64>>, | ||
| writes: Vec<std::ops::Range<u64>>, | ||
| ) -> InterpResult<'tcx> { |
There was a problem hiding this comment.
I don't really understand what this function is supposed to do, but it seems to be doing the work of finding the allocation that needs to be adjusted? That logic can entirely live inside Miri; Miri is in control of picking the absolute addresses for all memory so it can do this easily. In fact it can do it more efficiently since it has a list of all allocations sorted by their absolute address.
I think the only change you need inside rustc is a version of prepare_for_native_write that takes a range.
| /// Initialise previously uninitialised bytes in the given range, and set provenance of | ||
| /// everything in it to `Wildcard`. Before calling this, make sure all provenance in this | ||
| /// range is exposed! | ||
| pub fn mark_foreign_write(&mut self, range: AllocRange) { |
There was a problem hiding this comment.
This seems to skip resetting unit memory to 0. That step must not be skipped!
We shouldn't need two operations here anyway. Just one operation, prepare_for_native_write with a range, that does all the things it used to do, but restricted to a range.
| /// | ||
| /// The allocations in `ids` are assumed to be already exposed. | ||
| pub fn prepare_for_native_call(&mut self, ids: Vec<AllocId>) -> InterpResult<'tcx> { | ||
| pub fn prepare_for_native_call( |
There was a problem hiding this comment.
So if paranoid is false, then this function does what exactly? It seems to do just absolutely nothing.^^
There was a problem hiding this comment.
This zeroes out the bytes of uninitialised memory without actually marking it as init. I initially didn't do this, but it resulted in mark_foreign_write overwriting the data we cared about with zeroes. That's also why the latter doesn't zero out anything. We first zero the memory, then call the foreign code, then without re-zeroing mark it as init if it was written to after being zeroed
There was a problem hiding this comment.
Where does it do that zeroing? prepare_for_native_write only gets called if paranoid is true. So what you say and what the code does do not seem to line up, or I am misunderstanding something.
But also, I think we shouldn't have such a 2-stage approach. This seems easier to reason about if we just fully delay everything until the memory gets accessed the first time.
|
@rustbot author |
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
This PR will allow Miri to internally update its state based on information about foreign accesses performed on its memory during FFI. Necessary as part of rust-lang/miri#4326 to make use of the extra information we gain; currently pending review of the design (see design document in the linked PR), so marked as draft for now.
r? @RalfJung