From 2db7d7a8e077fa9600c6d75f9bf482b2ecf754f9 Mon Sep 17 00:00:00 2001
From: Afonso Bordado <afonso360@users.noreply.github.com>
Date: Thu, 15 Sep 2022 18:18:15 +0100
Subject: [PATCH] fuzzgen: Disable verifier after NaN Canonicalization (#4914)

* fuzzgen: Disable verifier after NaN Canonicalization

We are currently running the verifier twice, once after the nan canonicalization pass, and again when JIT compiling the code.

The verifier first runs in the NaN Canonicalization pass. If it fails it prevents us from getting a nice `cargo fuzz fmt` test case.

So disable the verifier there, but ensure its enabled when JIT compiling.

* fuzzgen: Force enable verifier in cranelift-icache

This is already the default, but since we no longer run the verifier in `fuzzgen` its important to ensure that it runs in the fuzz targets.
---
 cranelift/fuzzgen/src/lib.rs           | 16 +++++++++++-----
 fuzz/fuzz_targets/cranelift-fuzzgen.rs |  4 ++++
 fuzz/fuzz_targets/cranelift-icache.rs  |  4 ++++
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/cranelift/fuzzgen/src/lib.rs b/cranelift/fuzzgen/src/lib.rs
index 6f0d0f64df..91f425be6c 100644
--- a/cranelift/fuzzgen/src/lib.rs
+++ b/cranelift/fuzzgen/src/lib.rs
@@ -183,17 +183,23 @@ 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
-        // this is only used for the verifier after `canonicalize_nans` so
-        // it's not too important.
-        let flags = settings::Flags::new(settings::builder());
+        // 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.
+        let flags = settings::Flags::new({
+            let mut builder = settings::builder();
+            builder.set("enable_verifier", "false").unwrap();
+            builder
+        });
+
         let isa = builder_with_options(false)
             .expect("Unable to build a TargetIsa for the current host")
             .finish(flags)
             .expect("Failed to build TargetISA");
 
         ctx.canonicalize_nans(isa.as_ref())
-            .expect("Failed validation after NaN canonicalization");
+            .expect("Failed NaN canonicalization pass");
 
         ctx.func
     }
diff --git a/fuzz/fuzz_targets/cranelift-fuzzgen.rs b/fuzz/fuzz_targets/cranelift-fuzzgen.rs
index c0d2a2418b..559b70d531 100644
--- a/fuzz/fuzz_targets/cranelift-fuzzgen.rs
+++ b/fuzz/fuzz_targets/cranelift-fuzzgen.rs
@@ -85,6 +85,10 @@ fuzz_target!(|testcase: TestCase| {
         let mut builder = settings::builder();
         // We need llvm ABI extensions for i128 values on x86
         builder.set("enable_llvm_abi_extensions", "true").unwrap();
+
+        // This is the default, but we should ensure that it wasn't accidentally turned off anywhere.
+        builder.set("enable_verifier", "true").unwrap();
+
         settings::Flags::new(builder)
     };
     let mut compiler = TestFileCompiler::with_host_isa(flags).unwrap();
diff --git a/fuzz/fuzz_targets/cranelift-icache.rs b/fuzz/fuzz_targets/cranelift-icache.rs
index abc3800e83..f239ef4aad 100644
--- a/fuzz/fuzz_targets/cranelift-icache.rs
+++ b/fuzz/fuzz_targets/cranelift-icache.rs
@@ -20,6 +20,10 @@ fuzz_target!(|func: SingleFunction| {
         let mut builder = settings::builder();
         // We need llvm ABI extensions for i128 values on x86
         builder.set("enable_llvm_abi_extensions", "true").unwrap();
+
+        // This is the default, but we should ensure that it wasn't accidentally turned off anywhere.
+        builder.set("enable_verifier", "true").unwrap();
+
         builder
     });