Stream: git-wasmtime

Topic: wasmtime / PR #4914 fuzzgen: Disable verifier after NaN C...

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

afonso360 opened PR #4914 from disable-nan-verifier to main:

We are currently running the verifier twice, once after the nan canonicalization pass, and again when JIT compiling the code.

The verifier first runs in the NaN Canonicalization pass. If it fails it prevents us from getting a nice cargo fuzz fmt test case.

So disable the verifier there, but ensure its enabled when JIT compiling.

@jameysharp I think this solves the issue in this comment: https://github.com/bytecodealliance/wasmtime/pull/4896#pullrequestreview-1104415283

I think it makes sense to keep NaN canonicalization in fuzzgen, otherwise the testcase reported in fmt is different than the one that is actually executed and that can be really confusing.

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

afonso360 submitted PR review.

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

afonso360 created PR review comment:

Instead of doing this we can also explicitly run the verifier here, I don't know which one is preferable.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 15 2022 at 14:51):

afonso360 edited PR #4914 from disable-nan-verifier to main:

We are currently running the verifier twice, once after the nan canonicalization pass, and again when JIT compiling the code.

The verifier first runs in the NaN Canonicalization pass. If it fails it prevents us from getting a nice cargo fuzz fmt test case.

So disable the verifier there, but ensure its enabled when JIT compiling.

@jameysharp I think this solves the issue in this comment: https://github.com/bytecodealliance/wasmtime/pull/4896#pullrequestreview-1104415283

I also think it makes sense to keep NaN canonicalization in fuzzgen, otherwise the testcase reported in fmt is different than the one that is actually executed and that can be really confusing.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 15 2022 at 16:20):

jameysharp submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 15 2022 at 16:43):

afonso360 updated PR #4914 from disable-nan-verifier to main.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 15 2022 at 16:45):

jameysharp has enabled auto merge for PR #4914.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 15 2022 at 17:18):

jameysharp merged PR #4914.

Last updated: Oct 23 2024 at 20:03 UTC