Stream: git-wasmtime

Topic: wasmtime / PR #10052 Fuzzing: Keep AVX flags enabled for ...

Wasmtime GitHub notifications bot (Jan 20 2025 at 18:05):

jeffcharles requested wasmtime-fuzz-reviewers for a review on PR #10052.

Wasmtime GitHub notifications bot (Jan 20 2025 at 18:05):

jeffcharles requested alexcrichton for a review on PR #10052.

Wasmtime GitHub notifications bot (Jan 20 2025 at 18:05):

jeffcharles opened PR #10052 from jeffcharles:fuzzing-leave-avx-on-for-winch to bytecodealliance:main:

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Winch relies on AVX and AVX2 CPU features for implementing SIMD instructions on x64. The fuzzer sometimes disables these flags in Cranelift but the SIMD Wast tests are marked as supported if the Rust standard library says the current CPU has AVX and AVX2 support. So when the fuzzer disables these flags, the test is still marked as supported and then fails with an error saying AVX or AVX2 is required when the test is executed.

Wasmtime GitHub notifications bot (Jan 20 2025 at 19:42):

jeffcharles updated PR #10052.

Wasmtime GitHub notifications bot (Jan 20 2025 at 20:44):

github-actions[bot] commented on PR #10052:

Subscribe to Label Action

cc @fitzgen

<details>
This issue or pull request has been labeled: "fuzzing"

Thus the following users have been cc'd because of the following labels:

• fitzgen: fuzzing

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Jan 21 2025 at 15:25):

alexcrichton commented on PR #10052:

Thanks for this! Figuring out how to navigate the fuzzers around this I always find a bit of a challenge. For example I thought we already disabled the simd proposal for winch, so how come Winch got some simd instructions?

Once we know simd is in the picture it also might make more sense to either accept this as a "this is allowed error" or fail earlier during the test case itself. We try to keep the fuzz test cases meaning the same thing across machines so instead of force-enabling AVX it might make more sense to see if AVX is either explicitly disabled or unavailable on hardware and in that case bail out and discard the fuzz test case basically

Wasmtime GitHub notifications bot (Jan 21 2025 at 16:49):

alexcrichton commented on PR #10052:

Oh I also see now the fuzz bug this came in as! Specifically wast_tests fails because fuzz configuration is turning off has_avx (explicitly) and then running a simd test. The host actually has avx though which is why the test is expected to pass but then it fails because has_avx is manually disabled otherwise.

I'm wary to force-enable has_avx in such a core location in fuzzing because it'll mean we basically never fuzz Winch without AVX which seems useful to preserve. Would it be possible to perhaps scope this to just the wast_tests fuzzer and that one oracle? That could test for Winch, see if the test itself needs simd (which I think is available in configuration), and then the fuzz input could be thrown out (aka skipped) if avx is explicitly disabled?

Wasmtime GitHub notifications bot (Jan 21 2025 at 20:41):

jeffcharles updated PR #10052.

Wasmtime GitHub notifications bot (Jan 21 2025 at 20:43):

jeffcharles commented on PR #10052:

I had to add at least one method to the config to get access to the codegen flags in the oracle. Or at least I couldn't figure out another way to get access to the flags. The checks are also a little ugly because the value for the flag is typed as a String as opposed to a boolean.

Wasmtime GitHub notifications bot (Jan 21 2025 at 20:48):

alexcrichton submitted PR review:

Looks good to me, and thanks! I was just looking at a similar-ish thing over at https://github.com/bytecodealliance/wasmtime/pull/10068 which jogged my memory and leads me to ask, mind adding a log::warn! call that the test case is being skipped like this log here?

Wasmtime GitHub notifications bot (Jan 21 2025 at 21:02):

jeffcharles updated PR #10052.

Wasmtime GitHub notifications bot (Jan 21 2025 at 21:39):

alexcrichton merged PR #10052.

Last updated: Jan 24 2025 at 00:11 UTC