Stream: git-wasmtime

Topic: wasmtime / PR #5335 Cranelift: consider heap's guard page...

Wasmtime GitHub notifications bot (Nov 28 2022 at 23:26):

fitzgen opened PR #5335 from heap-addr-guard-pages to main:

Fixes #5328

FYI, I opted not to mess with or dedupe the similar logic in cranelift-wasm since that stuff is going to go away soon-ish when we remove heap_addr and collect all this logic into heap_{load,store}. Excited to get to a point where this logic isn't spread across crates and straddling interfaces.

Wasmtime GitHub notifications bot (Nov 28 2022 at 23:26):

fitzgen requested alexcrichton for a review on PR #5335.

Wasmtime GitHub notifications bot (Nov 29 2022 at 01:49):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Nov 29 2022 at 01:49):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Nov 29 2022 at 01:49):

cfallin created PR review comment:

Is it worth a comment here on the >= and the 0xffff_ffff (i.e., the minus-one aspect)?

In particular I think it's right because

• We trap if index > bound + guard_size - offset - access_size
• If the RHS of that is equal to 0xffff_ffff, index could be equal to it but not greater than it; so we cannot achieve index > RHS

I think it's a little unclear because above in the comment we have index < 4GiB <= RHS but here we are really relying on index <= (4GiB - 1) <= RHS, which is also valid (<= is also transitive). I guess aligning the two makes things clearer, at least for me...

Wasmtime GitHub notifications bot (Nov 29 2022 at 15:17):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Nov 29 2022 at 19:21):

fitzgen updated PR #5335 from heap-addr-guard-pages to main.

Wasmtime GitHub notifications bot (Nov 29 2022 at 19:23):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Nov 29 2022 at 19:23):

fitzgen created PR review comment:

Updated the comment above this to align more directly with the implementation.

Wasmtime GitHub notifications bot (Nov 29 2022 at 19:41):

fitzgen has enabled auto merge for PR #5335.

Wasmtime GitHub notifications bot (Nov 29 2022 at 19:54):

fitzgen merged PR #5335.

Last updated: Nov 22 2024 at 17:03 UTC