Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fuzzgen: Generate compiler flags #5020

Merged
merged 8 commits into from
Oct 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions cranelift/codegen/meta/src/gen_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,26 @@ fn gen_iterator(group: &SettingGroup, fmt: &mut Formatter) {
fmtln!(fmt, "}");
}

/// Generates a `all()` function with all options for this enum
fn gen_enum_all(name: &str, values: &[&'static str], fmt: &mut Formatter) {
fmtln!(
fmt,
"/// Returns a slice with all possible [{}] values.",
name
);
fmtln!(fmt, "pub fn all() -> &'static [{}] {{", name);
fmt.indent(|fmt| {
fmtln!(fmt, "&[");
fmt.indent(|fmt| {
for v in values.iter() {
fmtln!(fmt, "Self::{},", camel_case(v));
}
});
fmtln!(fmt, "]");
});
fmtln!(fmt, "}");
}

/// Emit Display and FromStr implementations for enum settings.
fn gen_to_and_from_str(name: &str, values: &[&'static str], fmt: &mut Formatter) {
fmtln!(fmt, "impl fmt::Display for {} {{", name);
Expand Down Expand Up @@ -158,6 +178,12 @@ fn gen_enum_types(group: &SettingGroup, fmt: &mut Formatter) {
});
fmtln!(fmt, "}");

fmtln!(fmt, "impl {} {{", name);
fmt.indent(|fmt| {
gen_enum_all(&name, values, fmt);
});
fmtln!(fmt, "}");

gen_to_and_from_str(&name, values, fmt);
}
}
Expand Down
2 changes: 2 additions & 0 deletions cranelift/codegen/meta/src/shared/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,5 +364,7 @@ pub(crate) fn define() -> SettingGroup {
false,
);

// When adding new settings please check if they can also be added
// in cranelift/fuzzgen/src/lib.rs for fuzzing.
settings.build()
}
15 changes: 13 additions & 2 deletions cranelift/fuzzgen/src/config.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
use std::collections::HashMap;
use std::ops::RangeInclusive;

/// Holds the range of acceptable values to use during the generation of testcases
pub struct Config {
pub test_case_inputs: RangeInclusive<usize>,
/// Maximum allowed test case inputs.
/// We build test case inputs from the rest of the bytes that the fuzzer provides us
/// so we allow the fuzzer to control this by feeding us more or less bytes.
/// The upper bound here is to prevent too many inputs that cause long test times
pub max_test_case_inputs: usize,
pub signature_params: RangeInclusive<usize>,
pub signature_rets: RangeInclusive<usize>,
pub instructions_per_block: RangeInclusive<usize>,
Expand Down Expand Up @@ -51,12 +56,17 @@ pub struct Config {
/// We insert a checking sequence to guarantee that those inputs never make
/// it to the instruction, but sometimes we want to allow them.
pub allowed_fcvt_traps_ratio: (usize, usize),

/// Some flags really impact compile performance, we still want to test
/// them, but probably at a lower rate, so that overall execution time isn't
/// impacted as much
pub compile_flag_ratio: HashMap<&'static str, (usize, usize)>,
}

impl Default for Config {
fn default() -> Self {
Config {
test_case_inputs: 1..=10,
max_test_case_inputs: 100,
signature_params: 0..=16,
signature_rets: 0..=16,
instructions_per_block: 0..=64,
Expand All @@ -75,6 +85,7 @@ impl Default for Config {
backwards_branch_ratio: (1, 1000),
allowed_int_divz_ratio: (1, 1_000_000),
allowed_fcvt_traps_ratio: (1, 1_000_000),
compile_flag_ratio: [("regalloc_checker", (1usize, 1000))].into_iter().collect(),
}
}
}
106 changes: 93 additions & 13 deletions cranelift/fuzzgen/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::config::Config;
use crate::function_generator::FunctionGenerator;
use crate::settings::{Flags, OptLevel};
use anyhow::Result;
use arbitrary::{Arbitrary, Unstructured};
use cranelift::codegen::data_value::DataValue;
Expand Down Expand Up @@ -30,6 +31,9 @@ impl<'a> Arbitrary<'a> for SingleFunction {
}

pub struct TestCase {
/// [Flags] to use when compiling this test case
pub flags: Flags,
/// Function under test
pub func: Function,
/// Generate multiple test inputs for each test case.
/// This allows us to get more coverage per compilation, which may be somewhat expensive.
Expand All @@ -38,19 +42,24 @@ pub struct TestCase {

impl fmt::Debug for TestCase {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(
f,
r#";; Fuzzgen test case
writeln!(f, ";; Fuzzgen test case\n")?;
writeln!(f, "test interpret")?;
writeln!(f, "test run")?;

test interpret
test run
set enable_llvm_abi_extensions
target aarch64
target s390x
target x86_64
// Print only non default flags
let default_flags = Flags::new(settings::builder());
for (default, flag) in default_flags.iter().zip(self.flags.iter()) {
assert_eq!(default.name, flag.name);

"#
)?;
if default.value_string() != flag.value_string() {
writeln!(f, "set {}={}", flag.name, flag.value_string())?;
}
}

writeln!(f, "target aarch64")?;
writeln!(f, "target s390x")?;
writeln!(f, "target riscv64")?;
writeln!(f, "target x86_64\n")?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to add riscv64 here too (or are you worried it would find too many issues)?

Actually in the spirit of your enum .all() method above -- perhaps we can define a cranelift_codegen::isa::all() that returns all Triples compiled into the build, and use that here? (Feel free to drop that idea though if it's more than ~20 lines of complexity or so.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah the all() idea sounds good, I've been adding them where it think they would be useful for others.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ended up not being very easy to do, I might try it again later. But either way I've added riscv64 to the target list.


writeln!(f, "{}", self.func)?;

Expand Down Expand Up @@ -140,7 +149,10 @@ where
fn generate_test_inputs(mut self, signature: &Signature) -> Result<Vec<TestCaseInput>> {
let mut inputs = Vec::new();

loop {
// Generate up to "max_test_case_inputs" inputs, we need an upper bound here since
// the fuzzer at some point starts trying to feed us way too many inputs. (I found one
// test case with 130k inputs!)
for _ in 0..self.config.max_test_case_inputs {
let last_len = self.u.len();

let test_args = signature
Expand Down Expand Up @@ -217,14 +229,82 @@ where
self.run_func_passes(func)
}

/// Generate a random set of cranelift flags.
/// Only semantics preserving flags are considered
fn generate_flags(&mut self) -> Result<Flags> {
let mut builder = settings::builder();

let opt = self.u.choose(OptLevel::all())?;
builder.set("opt_level", &format!("{}", opt)[..])?;

// Boolean flags
// TODO: probestack is semantics preserving, but only works inline and on x64
// TODO: enable_pinned_reg does not work with our current trampolines. See: #4376
// TODO: is_pic has issues:
// x86: https://github.com/bytecodealliance/wasmtime/issues/5005
// aarch64: https://github.com/bytecodealliance/wasmtime/issues/2735
let bool_settings = [
afonso360 marked this conversation as resolved.
Show resolved Hide resolved
"enable_alias_analysis",
"enable_safepoints",
"unwind_info",
"preserve_frame_pointers",
"enable_jump_tables",
"enable_heap_access_spectre_mitigation",
"enable_table_access_spectre_mitigation",
"enable_incremental_compilation_cache_checks",
"regalloc_checker",
"enable_llvm_abi_extensions",
"use_egraphs",
afonso360 marked this conversation as resolved.
Show resolved Hide resolved
];
for flag_name in bool_settings {
let enabled = self
.config
.compile_flag_ratio
.get(&flag_name)
.map(|&(num, denum)| self.u.ratio(num, denum))
.unwrap_or_else(|| bool::arbitrary(self.u))?;

let value = format!("{}", enabled);
builder.set(flag_name, value.as_str())?;
}

// Fixed settings

// We need llvm ABI extensions for i128 values on x86, so enable it regardless of
// what we picked above.
if cfg!(target_arch = "x86_64") {
builder.enable("enable_llvm_abi_extensions")?;
}

// This is the default, but we should ensure that it wasn't accidentally turned off anywhere.
builder.enable("enable_verifier")?;

// These settings just panic when they're not enabled and we try to use their respective functionality
// so they aren't very interesting to be automatically generated.
builder.enable("enable_atomics")?;
builder.enable("enable_float")?;
builder.enable("enable_simd")?;

// `machine_code_cfg_info` generates additional metadata for the embedder but this doesn't feed back
// into compilation anywhere, we leave it on unconditionally to make sure the generation doesn't panic.
builder.enable("machine_code_cfg_info")?;

Ok(Flags::new(builder))
}

pub fn generate_test(mut self) -> Result<TestCase> {
// If we're generating test inputs as well as a function, then we're planning to execute
// this function. That means that any function references in it need to exist. We don't yet
// have infrastructure for generating multiple functions, so just don't generate funcrefs.
self.config.funcrefs_per_function = 0..=0;

let flags = self.generate_flags()?;
let func = self.generate_func()?;
let inputs = self.generate_test_inputs(&func.signature)?;
Ok(TestCase { func, inputs })
Ok(TestCase {
flags,
func,
inputs,
})
}
}
18 changes: 4 additions & 14 deletions fuzz/fuzz_targets/cranelift-fuzzgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ use std::sync::atomic::Ordering;

use cranelift_codegen::data_value::DataValue;
use cranelift_codegen::ir::{LibCall, TrapCode};
use cranelift_codegen::settings;
use cranelift_codegen::settings::Configurable;
use cranelift_filetests::function_runner::{TestFileCompiler, Trampoline};
use cranelift_fuzzgen::*;
use cranelift_interpreter::environment::FuncIndex;
Expand Down Expand Up @@ -167,24 +165,16 @@ fn build_interpreter(testcase: &TestCase) -> Interpreter {
static STATISTICS: Lazy<Statistics> = Lazy::new(Statistics::default);

fuzz_target!(|testcase: TestCase| {
// This is the default, but we should ensure that it wasn't accidentally turned off anywhere.
assert!(testcase.flags.enable_verifier());

// Periodically print statistics
let valid_inputs = STATISTICS.valid_inputs.fetch_add(1, Ordering::SeqCst);
if valid_inputs != 0 && valid_inputs % 10000 == 0 {
STATISTICS.print(valid_inputs);
}

// Native fn
let flags = {
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();
let mut compiler = TestFileCompiler::with_host_isa(testcase.flags.clone()).unwrap();
compiler.declare_function(&testcase.func).unwrap();
compiler.define_function(testcase.func.clone()).unwrap();
compiler
Expand Down