Ravenbrook
Assert in protix.c triggered when working with large allocations
A user of Storm reported a crash triggered by the assertion below in code/protix.c (line 75 currently):
AVER(AddrOffset(base, limit) <= INT_MAX); /* should be redundant */
Due to another bug (which is now fixed), Storm allocated an array that was larger than 2 GiB, and when the MPS later attempted to change memory protection of the allocation, the assertion triggered.
I have created a test that triggers this bug. I will link it in a commit linked below. The test is quite slow due to the large allocation, so I leave it up to you if you find it worth including or not.
The issue seems to be using INT_MAX to limit a size_t (or ptrdiff_t) to a 32-bit value without really needing to do so. As far as I can see, the only time AddrOffset(base, limit) is used is as a direct parameter to mprotect, which accepts a size_t anyway. I think a solution would be to either remove the assertion (as the comment states, it should be redundant anyway), or to use PTRDIFF_MAX or something similar instead. Removing the assertion causes the attached test to pass.
Another way forward would be to disallow large allocations entirely (if other parts of the system are known to fail). I don't think this is the case, however, since the test passes without the assertion. As a sidenote, trying to allocate an object with a size close to SIZET_MAX sometimes causes internal assertions (due to violated assumptions due to overflow) rather than the AP functions just returning a failure due to not enough available memory as I would expect. Sizes below SIZET_MAX - 0x1000 behave properly.
I should perhaps also mention that I am happy to submit a patch if that helps. I am, however, not sure exactly what the preferred fix would be (removing the AVER entirely or changing the limit).