Wasmtime GitHub notifications bot (May 17 2024 at 20:43): Wasmtime GitHub notifications bot (May 17 2024 at 20:43):afonso360 opened PR #8650 from afonso360:fuzz-stackslot-alignment to bytecodealliance:main:
:wave: Hey,
This PR is a follow up to #8635. It adds the bare minimum possible for the Cranelift Fuzzgen to start fuzzing with different stack slot alignments.
Now that we can specify alignments, we should be able to start fuzzing load sinking with stack addresses :tada: . That isn't done yet, but this is the first step.
Wasmtime GitHub notifications bot (May 17 2024 at 20:43):afonso360 requested elliottt for a review on PR #8650.
Wasmtime GitHub notifications bot (May 17 2024 at 20:43):afonso360 requested wasmtime-compiler-reviewers for a review on PR #8650.
Wasmtime GitHub notifications bot (May 18 2024 at 09:09):bjorn3 created PR review comment:
Currently the max working alignment is 2^4 (16 bytes): https://github.com/bytecodealliance/wasmtime/pull/8635#discussion_r1603954465
Wasmtime GitHub notifications bot (May 18 2024 at 09:09):bjorn3 submitted PR review.
Wasmtime GitHub notifications bot (May 18 2024 at 09:20):afonso360 submitted PR review.
Wasmtime GitHub notifications bot (May 18 2024 at 09:20):afonso360 created PR review comment:
:+1: I've been following that thread. We currently don't depend on any alignment larger than 8 bytes. And at most we'll need 16bytes (for sinking SIMD loads).
But I think it's still a good idea to test with larger values, even if we end up not having any guarantee past 16 bytes.
Wasmtime GitHub notifications bot (May 18 2024 at 09:23):afonso360 edited PR review comment.
Wasmtime GitHub notifications bot (May 18 2024 at 09:24):bjorn3 submitted PR review.
Wasmtime GitHub notifications bot (May 18 2024 at 09:24):bjorn3 created PR review comment:
I missed that you are not actually checking that the stack slot is correctly aligned at runtime.
Wasmtime GitHub notifications bot (May 22 2024 at 15:59):elliottt submitted PR review:
This looks good to me! Do you have a plan for verifying alignment in the future?
Wasmtime GitHub notifications bot (May 22 2024 at 21:59):afonso360 commented on PR #8650:
Yes, but hopefully we shouldn't need to.
The plan is to rely on the ABI alignment and generate loads/stores with the aligned flag if both the slot and the ABI support that alignment. That should allow us to start testing load sinking.
Now, I do plan on validating if these memory accesses are actually aligned with the interpreter. The interpreter already checks if stack accesses are aligned, when they contain the
alignedmemflag, but it currently doesn't support aligning the stack slots. Or align the stack at thecallboundary. So those are the two missing items.This hopefully shouldn't be needed since, if we're failing fuzz runs in the interpreter its not great for fuzzing efficiency.
Wasmtime GitHub notifications bot (May 22 2024 at 23:38):afonso360 merged PR #8650.
Last updated: Nov 22 2024 at 16:03 UTC