From 590afd49889822ded547957df1ee7af66499f12f Mon Sep 17 00:00:00 2001
From: Afonso Bordado <afonsobordado@az8.co>
Date: Sat, 13 Apr 2024 13:30:42 +0100
Subject: [PATCH] fuzzgen: Use the correct ISA when running NaN
 Canonicalization pass

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 #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 #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.
---
 cranelift/codegen/src/isa/mod.rs | 17 +++++++++++++++++
 cranelift/fuzzgen/src/lib.rs     | 14 ++++++++------
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/cranelift/codegen/src/isa/mod.rs b/cranelift/codegen/src/isa/mod.rs
index 8e68e7aaba27..662e80302887 100644
--- a/cranelift/codegen/src/isa/mod.rs
+++ b/cranelift/codegen/src/isa/mod.rs
@@ -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};
@@ -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
diff --git a/cranelift/fuzzgen/src/lib.rs b/cranelift/fuzzgen/src/lib.rs
index dd3a7662da95..a4b4dfbda8a5 100644
--- a/cranelift/fuzzgen/src/lib.rs
+++ b/cranelift/fuzzgen/src/lib.rs
@@ -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
@@ -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.
@@ -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");
 
@@ -156,7 +158,7 @@ where
         let func = FunctionGenerator::new(
             &mut self.u,
             &self.config,
-            isa,
+            isa.clone(),
             name,
             sig,
             usercalls,
@@ -164,7 +166,7 @@ where
         )
         .generate()?;
 
-        self.run_func_passes(func)
+        self.run_func_passes(func, &*isa)
     }
 
     /// Generate a random set of cranelift flags.