Stream: git-wasmtime

Topic: wasmtime / PR #5167 cranelift: Forbid zero sized `heap_ad...

view this post on Zulip Wasmtime GitHub notifications bot (Nov 01 2022 at 16:02):

afonso360 opened PR #5167 from interp-heap-sat-sub to main:

:wave: Hey,

I don't know if we should merge this or not!

This PR started on https://github.com/bytecodealliance/wasmtime/pull/5155#discussion_r1009872167 where @jameysharp pointed out that it was a bit weird that the interpreter used a saturating_sub when calculating heap_addr sizes.

I added that because we do have some 0 sized heap_addr's in runtests. However when fixing that, I added a verifier rule, and it turns out we have a lot of 0 sized heap_addr's in our test suite. Are these legal?

The instruction docs state the following:

Verify that the offset range p .. p + Size - 1 is in bounds for the heap H, and generate an absolute address that is safe to dereference.

So it sounds like Size must always be larger than 1?

But it also says:

If p + Size is not greater than the heap bound

Which would allow 0 as a Size.

So, lets make a decision about this! I don't have any preference for either, I'm just opening this as a PR because when I started fixing the interpreter I thought it would just be that one runtest that was wrong!

Also, there are some regressions in the x64 codegen that probably shouldn't be there.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 01 2022 at 16:08):

afonso360 updated PR #5167 from interp-heap-sat-sub to main.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 01 2022 at 16:13):

afonso360 updated PR #5167 from interp-heap-sat-sub to main.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 01 2022 at 17:10):

jameysharp requested cfallin for a review on PR #5167.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 02 2022 at 15:38):

afonso360 updated PR #5167 from interp-heap-sat-sub to main.

Last updated: Nov 22 2024 at 16:03 UTC