Wasmtime GitHub notifications bot (Oct 12 2023 at 11:58): Wasmtime GitHub notifications bot (Oct 12 2023 at 11:58):afonso360 opened PR #7225 from afonso360:fuzzgen-alias-analysis to bytecodealliance:main:
:wave: Hey,
This is a follow up to #7215 and @jameysharp's comments on yesterdays cranelift meeting. @jameysharp suggested that we could add heaps to clif to start testing our alias analysis pass.
But having thought about it, there is nothing special about the "Heap" flag on
MemFlags. It is only used by our alias analysis pass to optimize some accesses. The same happens with the other flags ("Table" and "VmCtx"). Additionally we already have a way to partition memory in the fuzzer, we call it stack slots.This PR tags each stack slot with a different Alias Analysis category (Although region might be a better name). Once we have that we just have to ensure that all accesses are correctly tagged, and we should be able to test all 4 categories.
I'm opening this as a draft since I still want to get some fuzzing on this. I suspect that if there is a bug to be found here, It's going to take a while to get those conditions.
Wasmtime GitHub notifications bot (Oct 12 2023 at 11:59):afonso360 edited PR #7225:
:wave: Hey,
This is a follow up to #7215 and @jameysharp's comments on yesterdays cranelift meeting. @jameysharp suggested that we could add heaps to clif to start testing our alias analysis pass.
But having thought about it, there is nothing special about the "Heap" flag on
MemFlags. It is only used by our alias analysis pass to optimize some accesses. The same happens with the other flags ("Table" and "VmCtx"). Additionally we already have a way to partition memory in the fuzzer, we call it stack slots. Additionally we already enforce that memory accesses can't cross stack slots, so we also don't run into the risk of doing a partial update on a different region.This PR tags each stack slot with a different Alias Analysis category (Although region might be a better name). Once we have that we just have to ensure that all accesses are correctly tagged, and we should be able to test all 4 categories.
I'm opening this as a draft since I still want to get some fuzzing on this. I suspect that if there is a bug to be found here, It's going to take a while to get those conditions.
Wasmtime GitHub notifications bot (Oct 12 2023 at 11:59):afonso360 edited PR #7225:
:wave: Hey,
This is a follow up to #7215 and @jameysharp's comments on yesterdays cranelift meeting. @jameysharp suggested that we could add heaps to clif to start testing our alias analysis pass.
But having thought about it, there is nothing special about the "Heap" flag on
MemFlags. It is only used by our alias analysis pass to optimize some accesses. The same happens with the other flags ("Table" and "VmCtx"). Additionally we already have a way to partition memory in the fuzzer, we call it stack slots. Additionally we already enforce that memory accesses can't cross stack slots, so we also don't run into the risk of doing a partial store/load on a different region.This PR tags each stack slot with a different Alias Analysis category (Although region might be a better name). Once we have that we just have to ensure that all accesses are correctly tagged, and we should be able to test all 4 categories.
I'm opening this as a draft since I still want to get some fuzzing on this. I suspect that if there is a bug to be found here, It's going to take a while to get those conditions.
Wasmtime GitHub notifications bot (Oct 12 2023 at 12:01):afonso360 edited PR #7225:
:wave: Hey,
This is a follow up to #7215 and @jameysharp's comments on yesterdays cranelift meeting. @jameysharp suggested that we could add heaps to clif to start testing our alias analysis pass.
But having thought about it, there is nothing special about the "Heap" flag on
MemFlags. It is only used by our alias analysis pass to optimize some accesses. The same happens with the other flags ("Table" and "VmCtx"). Additionally we already have a way to partition memory in the fuzzer, we call it stack slots. And we already enforce that memory accesses can't cross stack slots, so we also don't run into the risk of doing a partial store/load on a different region.This PR tags each stack slot with a different Alias Analysis category (Although region might be a better name). Once we have that we just have to ensure that all accesses are correctly tagged, and we should be able to test all 4 categories.
I'm opening this as a draft since I still want to get some fuzzing on this. I suspect that if there is a bug to be found here, It's going to take a while to get those conditions.
Wasmtime GitHub notifications bot (Oct 12 2023 at 12:15):afonso360 updated PR #7225.
Wasmtime GitHub notifications bot (Oct 12 2023 at 23:15):afonso360 has marked PR #7225 as ready for review.
Wasmtime GitHub notifications bot (Oct 12 2023 at 23:15):afonso360 requested wasmtime-compiler-reviewers for a review on PR #7225.
Wasmtime GitHub notifications bot (Oct 12 2023 at 23:15):afonso360 requested fitzgen for a review on PR #7225.
Wasmtime GitHub notifications bot (Oct 13 2023 at 17:28):fitzgen submitted PR review:
Nice!
Wasmtime GitHub notifications bot (Oct 13 2023 at 17:28):fitzgen submitted PR review:
Nice!
Wasmtime GitHub notifications bot (Oct 13 2023 at 17:28):fitzgen created PR review comment:
Are stack slots sorted by size? Could we document that in a comment on the field definition?
Wasmtime GitHub notifications bot (Oct 13 2023 at 17:28):fitzgen created PR review comment:
Okay, I see the sorting done here. Yeah, would be good to document this invariant at the field so that it is easier for other folks extending or reading this stuff to figure it out.
Wasmtime GitHub notifications bot (Oct 15 2023 at 14:53):afonso360 updated PR #7225.
Wasmtime GitHub notifications bot (Oct 15 2023 at 14:54):afonso360 has enabled auto merge for PR #7225.
Wasmtime GitHub notifications bot (Oct 15 2023 at 15:58):afonso360 merged PR #7225.
Last updated: Nov 22 2024 at 16:03 UTC