From a43d1dc68fc24e71f726ec858db90f1731e96a11 Mon Sep 17 00:00:00 2001 From: Afonso Bordado Date: Thu, 29 Jun 2023 19:35:54 +0100 Subject: [PATCH] fuzzgen: Generate Tail Calls (#6641) * fuzzgen: Generate Tail Calls * fuzzgen: Use the ISA's pointer type when preparing indirect calls --- cranelift/fuzzgen/src/function_generator.rs | 145 ++++++++++++++++---- cranelift/fuzzgen/src/lib.rs | 9 +- fuzz/fuzz_targets/cranelift-fuzzgen.rs | 8 +- fuzz/fuzz_targets/cranelift-icache.rs | 7 +- 4 files changed, 127 insertions(+), 42 deletions(-) diff --git a/cranelift/fuzzgen/src/function_generator.rs b/cranelift/fuzzgen/src/function_generator.rs index 1e5626ded220..f4d0746e58e5 100644 --- a/cranelift/fuzzgen/src/function_generator.rs +++ b/cranelift/fuzzgen/src/function_generator.rs @@ -13,6 +13,7 @@ use cranelift::codegen::ir::{ }; use cranelift::codegen::isa::CallConv; use cranelift::frontend::{FunctionBuilder, FunctionBuilderContext, Switch, Variable}; +use cranelift::prelude::isa::OwnedTargetIsa; use cranelift::prelude::{ EntityRef, ExtFuncData, FloatCC, InstBuilder, IntCC, JumpTableData, MemFlags, StackSlotData, StackSlotKind, @@ -68,27 +69,32 @@ fn insert_opcode( Ok(()) } -fn insert_call( +fn insert_call_to_function( fgen: &mut FunctionGenerator, builder: &mut FunctionBuilder, - opcode: Opcode, - args: &[Type], - _rets: &[Type], + call_opcode: Opcode, + sig: &Signature, + sig_ref: SigRef, + func_ref: FuncRef, ) -> Result<()> { - assert!(matches!(opcode, Opcode::Call | Opcode::CallIndirect)); - let (sig, sig_ref, func_ref) = fgen.u.choose(&fgen.resources.func_refs)?.clone(); - let actuals = fgen.generate_values_for_signature( builder, sig.params.iter().map(|abi_param| abi_param.value_type), )?; - let call = if opcode == Opcode::Call { - builder.ins().call(func_ref, &actuals) - } else { - let addr_ty = args[0]; - let addr = builder.ins().func_addr(addr_ty, func_ref); - builder.ins().call_indirect(sig_ref, addr, &actuals) + let addr_ty = fgen.isa.pointer_type(); + let call = match call_opcode { + Opcode::Call => builder.ins().call(func_ref, &actuals), + Opcode::ReturnCall => builder.ins().return_call(func_ref, &actuals), + Opcode::CallIndirect => { + let addr = builder.ins().func_addr(addr_ty, func_ref); + builder.ins().call_indirect(sig_ref, addr, &actuals) + } + Opcode::ReturnCallIndirect => { + let addr = builder.ins().func_addr(addr_ty, func_ref); + builder.ins().return_call_indirect(sig_ref, addr, &actuals) + } + _ => unreachable!(), }; // Assign the return values to random variables @@ -102,6 +108,19 @@ fn insert_call( Ok(()) } +fn insert_call( + fgen: &mut FunctionGenerator, + builder: &mut FunctionBuilder, + opcode: Opcode, + _args: &[Type], + _rets: &[Type], +) -> Result<()> { + assert!(matches!(opcode, Opcode::Call | Opcode::CallIndirect)); + let (sig, sig_ref, func_ref) = fgen.u.choose(&fgen.resources.func_refs)?.clone(); + + insert_call_to_function(fgen, builder, opcode, &sig, sig_ref, func_ref) +} + fn insert_stack_load( fgen: &mut FunctionGenerator, builder: &mut FunctionBuilder, @@ -159,7 +178,7 @@ fn insert_cmp( // 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. - let unimplemented_cc = match (fgen.target_triple.architecture, cc) { + let unimplemented_cc = match (fgen.isa.triple().architecture, cc) { // Some FloatCC's are not implemented on AArch64, see: // https://github.com/bytecodealliance/wasmtime/issues/4850 (Architecture::Aarch64(_), FloatCC::OrderedNotEqual) => true, @@ -736,7 +755,12 @@ static OPCODE_SIGNATURES: Lazy> = Lazy::new(|| { .filter(|op| { match op { // Control flow opcodes should not be generated through `generate_instructions`. - Opcode::BrTable | Opcode::Brif | Opcode::Jump | Opcode::Return => false, + Opcode::BrTable + | Opcode::Brif + | Opcode::Jump + | Opcode::Return + | Opcode::ReturnCall + | Opcode::ReturnCallIndirect => false, // Constants are generated outside of `generate_instructions` Opcode::Iconst => false, @@ -814,8 +838,6 @@ static OPCODE_SIGNATURES: Lazy> = Lazy::new(|| { (Opcode::Trapnz), (Opcode::ResumableTrapnz), (Opcode::CallIndirect, &[I32]), - (Opcode::ReturnCall), - (Opcode::ReturnCallIndirect), (Opcode::FuncAddr), (Opcode::X86Pshufb), (Opcode::AvgRound), @@ -1447,7 +1469,7 @@ where u: &'r mut Unstructured<'data>, config: &'r Config, resources: Resources, - target_triple: Triple, + isa: OwnedTargetIsa, name: UserFuncName, signature: Signature, } @@ -1459,6 +1481,8 @@ enum BlockTerminator { Br(Block, Block), BrTable(Block, Vec), Switch(Type, Block, HashMap), + TailCall(FuncRef), + TailCallIndirect(FuncRef), } #[derive(Debug, Clone)] @@ -1468,6 +1492,8 @@ enum BlockTerminatorKind { Br, BrTable, Switch, + TailCall, + TailCallIndirect, } #[derive(Default)] @@ -1508,6 +1534,17 @@ impl Resources { let partition_point = self.blocks_without_params.partition_point(|b| *b <= block); &self.blocks_without_params[partition_point..] } + + /// Generates an iterator of all valid tail call targets. This includes all functions with both + /// the `tail` calling convention and the same return values as the caller. + fn tail_call_targets<'a>( + &'a self, + caller_sig: &'a Signature, + ) -> impl Iterator { + self.func_refs.iter().filter(|(sig, _, _)| { + sig.call_conv == CallConv::Tail && sig.returns == caller_sig.returns + }) + } } impl<'r, 'data> FunctionGenerator<'r, 'data> @@ -1517,7 +1554,7 @@ where pub fn new( u: &'r mut Unstructured<'data>, config: &'r Config, - target_triple: Triple, + isa: OwnedTargetIsa, name: UserFuncName, signature: Signature, usercalls: Vec<(UserExternalName, Signature)>, @@ -1531,7 +1568,7 @@ where libcalls, ..Resources::default() }, - target_triple, + isa, name, signature, } @@ -1613,7 +1650,7 @@ where // AArch64: https://github.com/bytecodealliance/wasmtime/issues/5483 // RISCV: https://github.com/bytecodealliance/wasmtime/issues/5882 let requires_aligned_atomics = matches!( - self.target_triple.architecture, + self.isa.triple().architecture, Architecture::Aarch64(_) | Architecture::Riscv64(_) ); let aligned = if is_atomic && requires_aligned_atomics { @@ -1784,6 +1821,23 @@ where switch.emit(builder, switch_val, default); } + BlockTerminator::TailCall(target) | BlockTerminator::TailCallIndirect(target) => { + let (sig, sig_ref, func_ref) = self + .resources + .func_refs + .iter() + .find(|(_, _, f)| *f == target) + .expect("Failed to find previously selected function") + .clone(); + + let opcode = match terminator { + BlockTerminator::TailCall(_) => Opcode::ReturnCall, + BlockTerminator::TailCallIndirect(_) => Opcode::ReturnCallIndirect, + _ => unreachable!(), + }; + + insert_call_to_function(self, builder, opcode, &sig, sig_ref, func_ref)?; + } } Ok(()) @@ -1797,7 +1851,7 @@ where // We filter out instructions that aren't supported by the target at this point instead // of building a single vector of valid instructions at the beginning of function // generation, to avoid invalidating the corpus when instructions are enabled/disabled. - if !valid_for_target(&self.target_triple, *op, &args, &rets) { + if !valid_for_target(&self.isa.triple(), *op, &args, &rets) { return Err(arbitrary::Error::IncorrectFormat.into()); } @@ -1827,7 +1881,7 @@ where .iter() .map(|libcall| { let pointer_type = Type::int_with_byte_size( - self.target_triple.pointer_width().unwrap().bytes().into(), + self.isa.triple().pointer_width().unwrap().bytes().into(), ) .unwrap(); let signature = libcall.signature(lib_callconv, pointer_type); @@ -1984,6 +2038,27 @@ where valid_terminators.push(BlockTerminatorKind::Switch); } + // Tail Calls are a block terminator, so we should insert them as any other block + // terminator. We should ensure that we can select at least one target before considering + // them as candidate instructions. + let has_tail_callees = self + .resources + .tail_call_targets(&self.signature) + .next() + .is_some(); + let is_tail_caller = self.signature.call_conv == CallConv::Tail; + // TODO: This is currently only supported on x86 + let supports_tail_calls = self.isa.triple().architecture == Architecture::X86_64; + // TODO: We currently require frame pointers for tail calls + let has_frame_pointers = self.isa.flags().preserve_frame_pointers(); + + if is_tail_caller && has_tail_callees && supports_tail_calls & has_frame_pointers { + valid_terminators.extend([ + BlockTerminatorKind::TailCall, + BlockTerminatorKind::TailCallIndirect, + ]); + } + let terminator = self.u.choose(&valid_terminators)?; // Choose block targets for the terminators that we picked above @@ -2042,6 +2117,22 @@ where BlockTerminator::Switch(_type, default_block, entries) } + BlockTerminatorKind::TailCall => { + let targets = self + .resources + .tail_call_targets(&self.signature) + .collect::>(); + let (_, _, funcref) = *self.u.choose(&targets[..])?; + BlockTerminator::TailCall(*funcref) + } + BlockTerminatorKind::TailCallIndirect => { + let targets = self + .resources + .tail_call_targets(&self.signature) + .collect::>(); + let (_, _, funcref) = *self.u.choose(&targets[..])?; + BlockTerminator::TailCallIndirect(*funcref) + } }) }) .collect::>()?; @@ -2054,7 +2145,7 @@ where let mut params = Vec::with_capacity(param_count); for _ in 0..param_count { - params.push(self.u._type(self.target_triple.architecture)?); + params.push(self.u._type(self.isa.triple().architecture)?); } Ok(params) } @@ -2074,7 +2165,7 @@ where // Create a pool of vars that are going to be used in this function for _ in 0..self.param(&self.config.vars_per_function)? { - let ty = self.u._type(self.target_triple.architecture)?; + let ty = self.u._type(self.isa.triple().architecture)?; let value = self.generate_const(builder, ty)?; vars.push((ty, value)); } @@ -2107,10 +2198,12 @@ where let mut builder = FunctionBuilder::new(&mut func, &mut fn_builder_ctx); + // Build the function references before generating the block CFG since we store + // function references in the CFG. + self.generate_funcrefs(&mut builder)?; self.generate_blocks(&mut builder)?; // Function preamble - self.generate_funcrefs(&mut builder)?; self.generate_stack_slots(&mut builder)?; // Main instruction generation loop diff --git a/cranelift/fuzzgen/src/lib.rs b/cranelift/fuzzgen/src/lib.rs index a09aa80f0215..57f3bfe3f030 100644 --- a/cranelift/fuzzgen/src/lib.rs +++ b/cranelift/fuzzgen/src/lib.rs @@ -8,11 +8,12 @@ use cranelift::codegen::ir::{types::*, UserExternalName, UserFuncName}; use cranelift::codegen::ir::{Function, LibCall}; use cranelift::codegen::isa::{self, Builder}; use cranelift::codegen::Context; +use cranelift::prelude::isa::OwnedTargetIsa; use cranelift::prelude::settings::SettingKind; use cranelift::prelude::*; use cranelift_arbitrary::CraneliftArbitrary; use cranelift_native::builder_with_options; -use target_lexicon::{Architecture, Triple}; +use target_lexicon::Architecture; mod config; mod cranelift_arbitrary; @@ -139,16 +140,16 @@ where pub fn generate_func( &mut self, name: UserFuncName, - target_triple: Triple, + isa: OwnedTargetIsa, usercalls: Vec<(UserExternalName, Signature)>, libcalls: Vec, ) -> Result { - let sig = self.generate_signature(target_triple.architecture)?; + let sig = self.generate_signature(isa.triple().architecture)?; let func = FunctionGenerator::new( &mut self.u, &self.config, - target_triple, + isa, name, sig, usercalls, diff --git a/fuzz/fuzz_targets/cranelift-fuzzgen.rs b/fuzz/fuzz_targets/cranelift-fuzzgen.rs index 131a88810a1f..3aa9cb0ef0fd 100644 --- a/fuzz/fuzz_targets/cranelift-fuzzgen.rs +++ b/fuzz/fuzz_targets/cranelift-fuzzgen.rs @@ -214,12 +214,8 @@ impl TestCase { }) .collect(); - let func = gen.generate_func( - fname, - isa.triple().clone(), - usercalls, - ALLOWED_LIBCALLS.to_vec(), - )?; + let func = + gen.generate_func(fname, isa.clone(), usercalls, ALLOWED_LIBCALLS.to_vec())?; functions.push(func); ctrl_planes.push(ControlPlane::arbitrary(gen.u)?); diff --git a/fuzz/fuzz_targets/cranelift-icache.rs b/fuzz/fuzz_targets/cranelift-icache.rs index f493eb7258ca..eb0afff42ea5 100644 --- a/fuzz/fuzz_targets/cranelift-icache.rs +++ b/fuzz/fuzz_targets/cranelift-icache.rs @@ -76,12 +76,7 @@ impl FunctionWithIsa { .map_err(|_| arbitrary::Error::IncorrectFormat)?; let func = gen - .generate_func( - fname, - isa.triple().clone(), - usercalls, - ALLOWED_LIBCALLS.to_vec(), - ) + .generate_func(fname, isa.clone(), usercalls, ALLOWED_LIBCALLS.to_vec()) .map_err(|_| arbitrary::Error::IncorrectFormat)?; Ok(FunctionWithIsa { isa, func })