I've been poking around at the memory-related translation code in wasmtime/cranelift today. One thing I'm curious about (@Dan Gohman maybe you know the answer to this) is why the default guard size on 64-bit is only 2gb
I still need to figure out how to inspect machine code, but it looks like that means we still emit a bound check for all memory accesses from reading the code
I thought that we'd want to use a 4gb guard size to remove all needs for a bounds check?
b/c you could have a 2^32-1 offset from the 2^32-1 address
which I think means we'd need 8gb of reserved address space?
It's because x86 address offsets are encoded as signed 32-bit offsets, so the greatest offset we can codegen directly is 2 GiB
hm so to make sure I understand
The guard size is added to the static memory bound
we only have a 2GB guard page, so we need a bounds check (conditional branch) for offsets >= 2GB
but without that we would need a second instruction, right?
so we're trading an add
for a bounds check?
We need extra instructions for wasm load/store offsets which are > 2 GiB
which are very rare
*offset immediates, to be specific
oh for sure yeah
but the codegen for >=2GB offsets would be more optimal if we had a bigger guard size
like not saying we should, just making sure I understand the lay of the land
yeah
mk
while you're here, @Dan Gohman do you know why there's a + 1
here?
the comments don't really illuminate to me why it's there
or rather I don't fully understand why that +1 is necessary
(even with the comments)
iirc, it's to support unaligned accesses, which might have a base which is in bounds, but extend out of bounds
hm but even that I don't fully understand
because we might generate multiple accesses, and ensure that even if we do the out-of-bounds one first, it still traps immediately
or wait I guess it's trying to make sure check_size
is always nonzero?
I don't see how this is related to aligned stuff though
I don't know why check_size
needs to be nonzero though
/me confused
Put another way I'm removing this +1 and everything is still passing
and I'm pretty sure we don't need this +1
so this is a pre-flight check to see if I'm forgetting something
/me is paging this code in
The relevant comment is just above: "Add one to make sure that we check the pointer itself is in bounds."
right yeah, but that doesn't make sense to me
I don't know what the "pointer itself" is there, because we're doing a pointer check anyway
I know for a fact that there's a bug here if offset_guard_size == 0
, though, and this may have been trying to account for that at some point
if there's no guard page then we need to check + width_of_access
, not + 1
though
so this may have just been a mistake where +1
was used in lieu of + width_of_access
ok, yeah, I don't recall why it's written like that
k no worries
I'll send a PR and I'm trying to improve some comments along the way too
Last updated: Dec 23 2024 at 13:07 UTC