Stream: git-wasmtime

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

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.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

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

Wasmtime GitHub notifications bot (Jan 05 2023 at 08:29):

elliottt requested afonso360 for a review on PR #5482.

Wasmtime GitHub notifications bot (Jan 05 2023 at 08:30):

elliottt requested jameysharp for a review on PR #5482.

Wasmtime GitHub notifications bot (Jan 05 2023 at 17:54):

jameysharp submitted PR review.

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.

Wasmtime GitHub notifications bot (Jan 05 2023 at 17:54):

jameysharp submitted PR review.

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?

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)? {

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)?;

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")?;

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] = &[

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.

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.

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(_))

Wasmtime GitHub notifications bot (Jan 05 2023 at 18:10):

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

Wasmtime GitHub notifications bot (Jan 05 2023 at 18:14):

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

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

elliottt requested jameysharp for a review on PR #5482.

Wasmtime GitHub notifications bot (Jan 05 2023 at 18:27):

afonso360 submitted PR review.

Wasmtime GitHub notifications bot (Jan 05 2023 at 18:28):

jameysharp submitted PR review.

Wasmtime GitHub notifications bot (Jan 05 2023 at 18:43):

elliottt has enabled auto merge for PR #5482.

Wasmtime GitHub notifications bot (Jan 05 2023 at 18:49):

elliottt merged PR #5482.

Last updated: Nov 22 2024 at 16:03 UTC