potentially confused mmap code in lucet-runtime internals

[froydnj]

I was trying to figure out what might be involved in adding Windows support to lucet-runtime-internals and came across these two pieces of code:

https://github.com/bytecodealliance/lucet/blob/0b51fe7b68a7c75b9d74197096ededeb44703238/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs#L167-L179

https://github.com/bytecodealliance/lucet/blob/0b51fe7b68a7c75b9d74197096ededeb44703238/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs#L199-L216

The two pieces are subtle enough that it's worth factoring out a separate function to handle them, but the code and the comments in the second one appear to be at odds with one another: zeroing the whole heap would seem to suggest zeroing more than just the currently accessible (?) heap. And the code itself in the second one appears to be at odds with itself as well: why the difference in what we memset vs. what we mprotect(NONE)/madvise?

Am I just insufficiently knowledgeable about what's going on, or is there a real problem here?

[iximeow]

I agree that these could be factored out into a separate function, the high-level intent is to quickly zero pages we know will have to be reset, so just calling it by sysdep::reset_pages or similar would be an improvement. Then we can gate that by OS to use whichever APIs are appropriate.

I suspect the difference in zeroing size when clearing the heap is an accident, and should either be heap_size in both places, or alloc.heap_accessible_size in both places. heap_accessible_size should be sufficient since excess heap ought to still be zeroed from its last reset, or instance creation, if memory serves for those limits.. It shouldn't be an error, but it's definitely not a good place to disagree on sizes!