FR: relax bounds check for load/store of empty blobs

[crusso]

The region and probably ESM bounds check on load/storeBlob is too strict and will trap if the first offset is out-of-range, even if the size of the blob is zero.

https://dfinity.slack.com/archives/CPL67E7MX/p1713716838379599

Timo Hanke
:
I noticed an inconsistency with Region.storeBlob, Region.loadBlob. If you do storeBlob(216 - 1, "a") in a Region with one page then it works but storeBlob(216, "") doesn't. Same for loadBlob(216 - 1, 1) and loadBlob(216, 0). Is that intentional or accidental? One could argue that the end position matters, not the start position. (edited)
5 replies

Claudio Russo
:
I think that was a deliberate design decision, but we could indeed relax it to special case read/write s of empty blob. I think the end intention was to check the start and end offset are valid, regardless of data written. (edited)

Claudio Russo
:
In the extreme, if the size is empty, we could allow read/write from any offset, which might seems odd.

Timo Hanke
:
Yeah, but I would suggest something weaker. In loadBlob(pos, len) the condition would be pos + len <= 216 (if region has 1 page in this example). Instead of pos < 216 and pos + len <= 2**16 which it is now. So any pos would not go through.

Timo Hanke

Currently, len = 0 is special in the sense that loadBlob(pos, len) is valid if and only if pos + len <= 2**16 for all len > 0 . But that doesn't hold for len = 0.

Claudio Russo

motoko/rts/motoko-rts/src/region.rs

Line 155 in 760676a

pub unsafe fn check_relative_range(&self, offset: u64, len: u64) {

looks like the relevant line of code (and probably a similiar check for the old ExperimentalStableMemory). I'll open an issue. Is this urgent to fix or just icing on the cake?

region.rs
pub unsafe fn check_relative_range(&self, offset: u64, len: u64) {
https://github.com/[dfinity/motoko](https://github.com/dfinity/motoko)|dfinity/motokodfinity/motoko | Added by GitHub