Alex Crichton (Apr 14 2020 at 21:05): Alex Crichton (Apr 14 2020 at 21:05):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
Alex Crichton (Apr 14 2020 at 21:06):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
Alex Crichton (Apr 14 2020 at 21:06):I thought that we'd want to use a 4gb guard size to remove all needs for a bounds check?
Alex Crichton (Apr 14 2020 at 21:07):b/c you could have a 2^32-1 offset from the 2^32-1 address
Alex Crichton (Apr 14 2020 at 21:07):which I think means we'd need 8gb of reserved address space?
Dan Gohman (Apr 14 2020 at 21:32):It's because x86 address offsets are encoded as signed 32-bit offsets, so the greatest offset we can codegen directly is 2 GiB
Alex Crichton (Apr 14 2020 at 21:33):hm so to make sure I understand
Dan Gohman (Apr 14 2020 at 21:33):The guard size is added to the static memory bound
Alex Crichton (Apr 14 2020 at 21:33):we only have a 2GB guard page, so we need a bounds check (conditional branch) for offsets >= 2GB
Alex Crichton (Apr 14 2020 at 21:33):but without that we would need a second instruction, right?
Alex Crichton (Apr 14 2020 at 21:34):so we're trading an add for a bounds check?
Dan Gohman (Apr 14 2020 at 21:34):We need extra instructions for wasm load/store offsets which are > 2 GiB
Dan Gohman (Apr 14 2020 at 21:34):which are very rare
Dan Gohman (Apr 14 2020 at 21:34):*offset immediates, to be specific
Alex Crichton (Apr 14 2020 at 21:34):oh for sure yeah
Alex Crichton (Apr 14 2020 at 21:35):but the codegen for >=2GB offsets would be more optimal if we had a bigger guard size
Alex Crichton (Apr 14 2020 at 21:35):like not saying we should, just making sure I understand the lay of the land
Dan Gohman (Apr 14 2020 at 21:35):yeah
Alex Crichton (Apr 14 2020 at 21:35):mk
Alex Crichton (Apr 14 2020 at 21:36):while you're here, @Dan Gohman do you know why there's a + 1 here?
Alex Crichton (Apr 14 2020 at 21:36):the comments don't really illuminate to me why it's there
Alex Crichton (Apr 14 2020 at 21:36):or rather I don't fully understand why that +1 is necessary
Alex Crichton (Apr 14 2020 at 21:36):(even with the comments)
Dan Gohman (Apr 14 2020 at 21:41):iirc, it's to support unaligned accesses, which might have a base which is in bounds, but extend out of bounds
Alex Crichton (Apr 14 2020 at 21:42):hm but even that I don't fully understand
Dan Gohman (Apr 14 2020 at 21:42):because we might generate multiple accesses, and ensure that even if we do the out-of-bounds one first, it still traps immediately
Alex Crichton (Apr 14 2020 at 21:42):or wait I guess it's trying to make sure check_size is always nonzero?
Alex Crichton (Apr 14 2020 at 21:43):I don't see how this is related to aligned stuff though
Alex Crichton (Apr 14 2020 at 21:43):I don't know why check_size needs to be nonzero though
Alex Crichton (Apr 14 2020 at 21:43):/me confused
Alex Crichton (Apr 14 2020 at 21:45):Put another way I'm removing this +1 and everything is still passing
Alex Crichton (Apr 14 2020 at 21:45):and I'm pretty sure we don't need this +1
Alex Crichton (Apr 14 2020 at 21:45):so this is a pre-flight check to see if I'm forgetting something
Dan Gohman (Apr 14 2020 at 21:46):/me is paging this code in
Dan Gohman (Apr 14 2020 at 21:47):The relevant comment is just above: "Add one to make sure that we check the pointer itself is in bounds."
Alex Crichton (Apr 14 2020 at 21:47):right yeah, but that doesn't make sense to me
Alex Crichton (Apr 14 2020 at 21:47):I don't know what the "pointer itself" is there, because we're doing a pointer check anyway
Alex Crichton (Apr 14 2020 at 21:47):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
Alex Crichton (Apr 14 2020 at 21:48):if there's no guard page then we need to check + width_of_access, not + 1 though
Alex Crichton (Apr 14 2020 at 21:48):so this may have just been a mistake where +1 was used in lieu of + width_of_access
Dan Gohman (Apr 14 2020 at 21:54):ok, yeah, I don't recall why it's written like that
Alex Crichton (Apr 14 2020 at 21:55):k no worries
Alex Crichton (Apr 14 2020 at 21:55):I'll send a PR and I'm trying to improve some comments along the way too
Last updated: Nov 22 2024 at 16:03 UTC