Stream: git-wasmtime

Topic: wasmtime / PR #4467 fuzzgen: Add scalar float support

view this post on Zulip Wasmtime GitHub notifications bot (Jul 19 2022 at 18:21):

afonso360 opened PR #4467 from fuzzgen-float to main:

:wave: Hey,

This PR adds support for generating floats in fuzzgen and adds a bunch of instructions.

I think we have all the float ops, except bit ops that operate on floats.

So far this has found #4446 and #4460. We can merge this before #4460 but it will probably start crashing in oss-fuzz.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 19 2022 at 18:24):

afonso360 edited PR #4467 from fuzzgen-float to main:

:wave: Hey,

This PR adds support for generating floats in fuzzgen and adds a bunch of instructions.

I think we have all the float ops, except bit ops that operate on floats.

So far this has found #4446 and #4460. We can merge this before #4460 but it will probably start crashing in oss-fuzz.

cc: @jameysharp

view this post on Zulip Wasmtime GitHub notifications bot (Jul 19 2022 at 19:52):

jameysharp requested jameysharp for a review on PR #4467.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 19 2022 at 23:29):

jameysharp submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 19 2022 at 23:29):

jameysharp submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 19 2022 at 23:29):

jameysharp created PR review comment:

The second args[1] should be args[2].

It'd be nice to not duplicate all this code for each arity. Looking through InstBuilder, I see there's a MultiAry case. Can that replace the NullAry/Unary/Binary/Ternary split?

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 06:10):

afonso360 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 06:10):

afonso360 created PR review comment:

Good Catch! I missed the existence of MultiAry but it looks like a much better solution. I'll replace it.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 06:49):

afonso360 updated PR #4467 from fuzzgen-float to main.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 18:05):

jameysharp submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 18:26):

cfallin created PR review comment:

Hmm, I'm surprised that this works, and I suspect it does only accidentally: we should be building the argument list in the pool owned by the DataFlowGraph. The ValueList is just an index into the pool, so ValueLists need to be built with the proper pool and will silently fail if referring to another pool. (This is a kind of unfortunate logical-unsafety but the type system doesn't give us a way to tie the list to a particular pool.)

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 18:26):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 19:09):

afonso360 updated PR #4467 from fuzzgen-float to main.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 19:11):

afonso360 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 19:11):

afonso360 created PR review comment:

Fixed. I wonder how the fuzzer didn't immediately catch this, I only ran it for a couple of minutes after making this change, but I would have expected it to fail instantly.

Thanks!

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 19:27):

jameysharp created PR review comment:

Thanks for fixing the use of ValueList! I think you don't need a dependency on cranelift-entity now, right?

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 19:27):

jameysharp submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 19:36):

afonso360 updated PR #4467 from fuzzgen-float to main.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 19:36):

afonso360 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 19:36):

afonso360 created PR review comment:

Right, Fixed!

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 20:19):

jameysharp has enabled auto merge for PR #4467.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 22:30):

jameysharp has disabled auto merge for PR #4467.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 22:30):

afonso360 updated PR #4467 from fuzzgen-float to main.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2022 at 22:45):

jameysharp has enabled auto merge for PR #4467.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 21 2022 at 00:33):

jameysharp merged PR #4467.

Last updated: Nov 22 2024 at 17:03 UTC