Skip to content

Commit

Permalink
fuzzgen: Use the correct ISA when running NaN Canonicalization pass
Browse files Browse the repository at this point in the history
cranelift-fuzzgen unconditionally runs the NaN Canonicalization pass on all functions that it generates. This is so that we can ensure that when running in the interpreter vs natively we get the same bitpattern for all NaN's.

Until now we just picked a random ISA (the host ISA), disabled the verifier and ran the pass with that. This was because the ISA didn't really matter for the passes that we wanted to run.

In bytecodealliance#8313 the ISA now drives some codgen decisions for the NaN Canonicalization pass. Namely, if the ISA supports Vectors, it tries to use that.

In bytecodealliance#8359 there was a fuzz bug reported where fuzzgen generated vector code for RISC-V without the `has_v` flag, something that should *never* happen, because we simply cannot compile that code.

It turns out that fuzzgen did not generate vector code itself. But since we were passing the host ISA to the nan canonicalization pass, it assumed that it could use vectors and did so. But the actual target isa did not support vectors.

To fix this, we now correctly pass the target isa that we are building a function for.
  • Loading branch information
afonso360 committed Apr 13, 2024
1 parent 2a6b132 commit 590afd4
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 6 deletions.
17 changes: 17 additions & 0 deletions cranelift/codegen/src/isa/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ use crate::ir::{self, Function, Type};
use crate::isa::unwind::{systemv::RegisterMappingError, UnwindInfoKind};
use crate::machinst::{CompiledCode, CompiledCodeStencil, TextSectionBuilder};
use crate::settings;
use crate::settings::Configurable;
use crate::settings::SetResult;
use crate::CodegenResult;
use alloc::{boxed::Box, sync::Arc, vec::Vec};
Expand Down Expand Up @@ -174,6 +175,22 @@ impl<T> IsaBuilder<T> {
}
}

/// Creates a new [Builder] from a [TargetIsa], copying all flags in the
/// process.
pub fn from_target_isa(target_isa: &dyn TargetIsa) -> Builder {
// We should always be able to find the builder for the TargetISA, since presumably we
// also generated the previous TargetISA at some point
let triple = target_isa.triple().clone();
let mut builder = self::lookup(triple).expect("Could not find triple for target ISA");

// Copy ISA Flags
for flag in target_isa.isa_flags() {
builder.set(&flag.name, &flag.value_string()).unwrap();
}

builder
}

/// Gets the triple for the builder.
pub fn triple(&self) -> &Triple {
&self.triple
Expand Down
14 changes: 8 additions & 6 deletions cranelift/fuzzgen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ where
Ok(inputs)
}

fn run_func_passes(&mut self, func: Function) -> Result<Function> {
fn run_func_passes(&mut self, func: Function, isa: &dyn TargetIsa) -> Result<Function> {
// Do a NaN Canonicalization pass on the generated function.
//
// Both IEEE754 and the Wasm spec are somewhat loose about what is allowed
Expand All @@ -115,7 +115,7 @@ where
// the interpreter won't get that version, so call that pass manually here.

let mut ctx = Context::for_function(func);
// Assume that we are generating this function for the current ISA.

// We disable the verifier here, since if it fails it prevents a test case from
// being generated and formatted by `cargo fuzz fmt`.
// We run the verifier before compiling the code, so it always gets verified.
Expand All @@ -125,11 +125,13 @@ where
builder
});

let isa = builder_with_options(false)
.expect("Unable to build a TargetIsa for the current host")
// Create a new TargetISA from the given ISA, this ensures that we copy all ISA
// flags, which may have an effect on the code generated by the passes below.
let isa = Builder::from_target_isa(isa)
.finish(flags)
.expect("Failed to build TargetISA");

// Finally run the NaN canonicalization pass
ctx.canonicalize_nans(isa.as_ref())
.expect("Failed NaN canonicalization pass");

Expand All @@ -156,15 +158,15 @@ where
let func = FunctionGenerator::new(
&mut self.u,
&self.config,
isa,
isa.clone(),
name,
sig,
usercalls,
libcalls,
)
.generate()?;

self.run_func_passes(func)
self.run_func_passes(func, &*isa)
}

/// Generate a random set of cranelift flags.
Expand Down

0 comments on commit 590afd4

Please sign in to comment.