Stream: git-wasmtime

Topic: wasmtime / PR #5482 Fuzz multiple targets in cranelift-ic...


view this post on Zulip Wasmtime GitHub notifications bot (Jan 05 2023 at 05:35):

elliottt edited PR #5482 from trevor/compile-fuzz-all-targets to main:

Fuzz additional targets in the cranelift-icache target. The list of targets fuzzed is controlled by the targets enabled in fuzz/Cargo.toml.

This PR also reworks how instruction disabling is done in function generator, moving the deny-list to a function to make the decision at runtime instead of compile time.
<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Jan 05 2023 at 08:29):

elliottt requested afonso360 for a review on PR #5482.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 05 2023 at 08:30):

elliottt requested jameysharp for a review on PR #5482.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 05 2023 at 17:54):

jameysharp submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 05 2023 at 17:54):

jameysharp created PR review comment:

How about this, for a little more detail?

/// Returns true if we believe this `OpcodeSignature` should compile correctly
/// for the given target triple. We currently have a range of known issues
/// with specific lowerings on specific backends, and we don't want to get
/// fuzz bug reports for those. Over time our goal is to eliminate all of these
/// exceptions.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 05 2023 at 17:54):

jameysharp submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 05 2023 at 17:54):

jameysharp created PR review comment:

Would you mind factoring this "print non-default flags" logic out to a separate function and calling it from the Debug implementation for TestCase as well?

view this post on Zulip Wasmtime GitHub notifications bot (Jan 05 2023 at 17:54):

jameysharp created PR review comment:

This doesn't need to be a function. It probably didn't need to be before either, come to think of it...

        let supports_inline_probestack = match target_arch {
            Architecture::X86_64 => true,
            Architecture::Aarch64(_) => true,
            _ => false,
        };

        // Optionally test inline stackprobes on supported platforms
        // TODO: Test outlined stack probes.
        if supports_inline_probestack && bool::arbitrary(self.u)? {

view this post on Zulip Wasmtime GitHub notifications bot (Jan 05 2023 at 17:54):

jameysharp created PR review comment:

Let's add a comment here like the one in generate_instructions:

        // We filter out targets that aren't supported in the current build
        // configuration after randomly choosing one, instead of randomly choosing
        // a supported one, so that the same fuzz input works across different build
        // configurations.
        let target = u.choose(isa::ALL_ARCHITECTURES)?;

view this post on Zulip Wasmtime GitHub notifications bot (Jan 05 2023 at 17:54):

jameysharp created PR review comment:

The comment I'm about to make is an absurd nit-pick, but I have this thing about mixing different ways of emitting newlines, and I don't think the blank line is necessary anyway. Would you mind removing the \n here?

        writeln!(f, "test compile")?;

view this post on Zulip Wasmtime GitHub notifications bot (Jan 05 2023 at 17:54):

jameysharp created PR review comment:

Unimportant and pre-existing nit: I believe the 'static lifetime isn't required here, so I'd leave it out.

const OPCODE_SIGNATURES: &[OpcodeSignature] = &[

view this post on Zulip Wasmtime GitHub notifications bot (Jan 05 2023 at 17:54):

jameysharp created PR review comment:

I think this doesn't need to be pub; it looks like it's only called from this module now.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 05 2023 at 17:54):

jameysharp created PR review comment:

I think this doesn't need to be pub; it looks like it's only called from this module now.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 05 2023 at 17:54):

jameysharp created PR review comment:

Let's add a comment here like the one in generate_instructions:

        // We filter out condition codes that aren't supported by the target at
        // this point after randomly choosing one, instead of randomly choosing a
        // supported one, to avoid invalidating the corpus when these get implemented.
        if matches!(fgen.target_triple.architecture, Architecture::Aarch64(_))

view this post on Zulip Wasmtime GitHub notifications bot (Jan 05 2023 at 18:10):

elliottt updated PR #5482 from trevor/compile-fuzz-all-targets to main.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 05 2023 at 18:14):

elliottt updated PR #5482 from trevor/compile-fuzz-all-targets to main.

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

elliottt requested jameysharp for a review on PR #5482.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 05 2023 at 18:27):

afonso360 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 05 2023 at 18:28):

jameysharp submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 05 2023 at 18:43):

elliottt has enabled auto merge for PR #5482.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 05 2023 at 18:49):

elliottt merged PR #5482.


Last updated: Dec 23 2024 at 12:05 UTC