diff --git a/cranelift/codegen/src/ir/jumptable.rs b/cranelift/codegen/src/ir/jumptable.rs index 7897dff845..3f2cdb8fb1 100644 --- a/cranelift/codegen/src/ir/jumptable.rs +++ b/cranelift/codegen/src/ir/jumptable.rs @@ -33,6 +33,10 @@ impl JumpTableData { table: Vec::with_capacity(capacity), } } + /// Create a new jump table with the provided blocks + pub fn with_blocks(table: Vec) -> Self { + Self { table } + } /// Get the number of table entries. pub fn len(&self) -> usize { diff --git a/cranelift/fuzzgen/src/config.rs b/cranelift/fuzzgen/src/config.rs index 1213a89dbf..6f1103b051 100644 --- a/cranelift/fuzzgen/src/config.rs +++ b/cranelift/fuzzgen/src/config.rs @@ -16,10 +16,7 @@ pub struct Config { /// This value does not apply to block0 which takes the function params /// and is thus governed by `signature_params` pub block_signature_params: RangeInclusive, - /// Max number of jump tables generated per function - /// Note, the actual number of jump tables may be larger if the Switch interface - /// decides to insert more. - pub jump_tables_per_function: RangeInclusive, + /// Max number of jump tables entries to generate pub jump_table_entries: RangeInclusive, /// The Switch API specializes either individual blocks or contiguous ranges. @@ -66,7 +63,6 @@ impl Default for Config { vars_per_function: 0..=16, blocks_per_function: 0..=16, block_signature_params: 0..=16, - jump_tables_per_function: 0..=4, jump_table_entries: 0..=16, switch_cases: 0..=64, // Ranges smaller than 2 don't make sense. diff --git a/cranelift/fuzzgen/src/function_generator.rs b/cranelift/fuzzgen/src/function_generator.rs index 8af3784d7c..3bd15f00e6 100644 --- a/cranelift/fuzzgen/src/function_generator.rs +++ b/cranelift/fuzzgen/src/function_generator.rs @@ -7,7 +7,7 @@ use cranelift::codegen::ir::instructions::InstructionFormat; use cranelift::codegen::ir::stackslot::StackSize; use cranelift::codegen::ir::{types::*, FuncRef, LibCall, UserExternalName, UserFuncName}; use cranelift::codegen::ir::{ - AbiParam, Block, ExternalName, Function, JumpTable, Opcode, Signature, StackSlot, Type, Value, + AbiParam, Block, ExternalName, Function, Opcode, Signature, StackSlot, Type, Value, }; use cranelift::codegen::isa::CallConv; use cranelift::frontend::{FunctionBuilder, FunctionBuilderContext, Switch, Variable}; @@ -18,6 +18,15 @@ use cranelift::prelude::{ use std::collections::HashMap; use std::ops::RangeInclusive; +/// Generates a Vec with `len` elements comprised of `options` +fn arbitrary_vec( + u: &mut Unstructured, + len: usize, + options: &[T], +) -> arbitrary::Result> { + (0..len).map(|_| u.choose(options).cloned()).collect() +} + type BlockSignature = Vec; fn insert_opcode( @@ -782,157 +791,6 @@ const OPCODE_SIGNATURES: &'static [( (Opcode::Call, &[], &[], insert_call), ]; -type BlockTerminator = fn( - fgen: &mut FunctionGenerator, - builder: &mut FunctionBuilder, - source_block: Block, -) -> Result<()>; - -fn insert_return( - fgen: &mut FunctionGenerator, - builder: &mut FunctionBuilder, - _source_block: Block, -) -> Result<()> { - let types: Vec = { - let rets = &builder.func.signature.returns; - rets.iter().map(|p| p.value_type).collect() - }; - let vals = fgen.generate_values_for_signature(builder, types.into_iter())?; - - builder.ins().return_(&vals[..]); - Ok(()) -} - -fn insert_jump( - fgen: &mut FunctionGenerator, - builder: &mut FunctionBuilder, - source_block: Block, -) -> Result<()> { - let (block, args) = fgen.generate_target_block(builder, source_block)?; - builder.ins().jump(block, &args[..]); - Ok(()) -} - -/// Generates a br_table into a random block -fn insert_br_table( - fgen: &mut FunctionGenerator, - builder: &mut FunctionBuilder, - source_block: Block, -) -> Result<()> { - let var = fgen.get_variable_of_type(I32)?; // br_table only supports I32 - let val = builder.use_var(var); - - let target_blocks = fgen.resources.forward_blocks_without_params(source_block); - let default_block = *fgen.u.choose(target_blocks)?; - - // We can still select a backwards branching jump table here! - let tables = fgen.resources.forward_jump_tables(builder, source_block); - let jt = *fgen.u.choose(&tables[..])?; - builder.ins().br_table(val, default_block, jt); - Ok(()) -} - -/// Generates a brz/brnz into a random block -fn insert_br( - fgen: &mut FunctionGenerator, - builder: &mut FunctionBuilder, - source_block: Block, -) -> Result<()> { - let (block, args) = fgen.generate_target_block(builder, source_block)?; - - let condbr_types = [I8, I16, I32, I64, I128, B1]; - let _type = *fgen.u.choose(&condbr_types[..])?; - let var = fgen.get_variable_of_type(_type)?; - let val = builder.use_var(var); - - if bool::arbitrary(fgen.u)? { - builder.ins().brz(val, block, &args[..]); - } else { - builder.ins().brnz(val, block, &args[..]); - } - - // After brz/brnz we must generate a jump - insert_jump(fgen, builder, source_block)?; - Ok(()) -} - -fn insert_bricmp( - fgen: &mut FunctionGenerator, - builder: &mut FunctionBuilder, - source_block: Block, -) -> Result<()> { - let (block, args) = fgen.generate_target_block(builder, source_block)?; - - let cc = *fgen.u.choose(IntCC::all())?; - let _type = *fgen.u.choose(&[I8, I16, I32, I64, I128])?; - - let lhs_var = fgen.get_variable_of_type(_type)?; - let lhs_val = builder.use_var(lhs_var); - - let rhs_var = fgen.get_variable_of_type(_type)?; - let rhs_val = builder.use_var(rhs_var); - - builder - .ins() - .br_icmp(cc, lhs_val, rhs_val, block, &args[..]); - - // After bricmp's we must generate a jump - insert_jump(fgen, builder, source_block)?; - Ok(()) -} - -fn insert_switch( - fgen: &mut FunctionGenerator, - builder: &mut FunctionBuilder, - source_block: Block, -) -> Result<()> { - let _type = *fgen.u.choose(&[I8, I16, I32, I64, I128][..])?; - let switch_var = fgen.get_variable_of_type(_type)?; - let switch_val = builder.use_var(switch_var); - - // TODO: We should also generate backwards branches in switches - let default_block = { - let target_blocks = fgen.resources.forward_blocks_without_params(source_block); - *fgen.u.choose(target_blocks)? - }; - - // Build this into a HashMap since we cannot have duplicate entries. - let mut entries = HashMap::new(); - for _ in 0..fgen.param(&fgen.config.switch_cases)? { - // The Switch API only allows for entries that are addressable by the index type - // so we need to limit the range of values that we generate. - let (ty_min, ty_max) = _type.bounds(false); - let range_start = fgen.u.int_in_range(ty_min..=ty_max)?; - - // We can either insert a contiguous range of blocks or a individual block - // This is done because the Switch API specializes contiguous ranges. - let range_size = if bool::arbitrary(fgen.u)? { - 1 - } else { - fgen.param(&fgen.config.switch_max_range_size)? - } as u128; - - // Build the switch entries - for i in 0..range_size { - let index = range_start.wrapping_add(i) % ty_max; - let block = { - let target_blocks = fgen.resources.forward_blocks_without_params(source_block); - *fgen.u.choose(target_blocks)? - }; - - entries.insert(index, block); - } - } - - let mut switch = Switch::new(); - for (entry, block) in entries.into_iter() { - switch.set_entry(entry, block); - } - switch.emit(builder, switch_val, default_block); - - Ok(()) -} - /// These libcalls need a interpreter implementation in `cranelift-fuzzgen.rs` const ALLOWED_LIBCALLS: &'static [LibCall] = &[ LibCall::CeilF32, @@ -952,33 +810,37 @@ where resources: Resources, } +#[derive(Debug, Clone)] +enum BlockTerminator { + Return, + Jump(Block), + Br(Block, Block), + BrIcmp(Block, Block), + BrTable(Block, Vec), + Switch(Type, Block, HashMap), +} + +#[derive(Debug, Clone)] +enum BlockTerminatorKind { + Return, + Jump, + Br, + BrIcmp, + BrTable, + Switch, +} + #[derive(Default)] struct Resources { vars: HashMap>, blocks: Vec<(Block, BlockSignature)>, blocks_without_params: Vec, - jump_tables: Vec, + block_terminators: Vec, func_refs: Vec<(Signature, FuncRef)>, stack_slots: Vec<(StackSlot, StackSize)>, } impl Resources { - /// Returns [JumpTable]'s where all blocks are forward of `block` - fn forward_jump_tables(&self, builder: &FunctionBuilder, block: Block) -> Vec { - // TODO: We can avoid allocating a Vec here by sorting self.jump_tables based - // on the minimum block and returning a slice based on that. - // See https://github.com/bytecodealliance/wasmtime/pull/4894#discussion_r971241430 for more details - - // Unlike with the blocks below jump table targets are not ordered, thus we do need - // to allocate a Vec here. - let jump_tables = &builder.func.jump_tables; - self.jump_tables - .iter() - .copied() - .filter(|jt| jump_tables[*jt].iter().all(|target| *target > block)) - .collect() - } - /// Partitions blocks at `block`. Only blocks that can be targeted by branches are considered. /// /// The first slice includes all blocks up to and including `block`. @@ -993,6 +855,12 @@ impl Resources { target_blocks.split_at(block.as_u32() as usize) } + /// Returns blocks forward of `block`. Only blocks that can be targeted by branches are considered. + fn forward_blocks(&self, block: Block) -> &[(Block, BlockSignature)] { + let (_, forward_blocks) = self.partition_target_blocks(block); + forward_blocks + } + /// Generates a slice of `blocks_without_params` ahead of `block` fn forward_blocks_without_params(&self, block: Block) -> &[Block] { let partition_point = self.blocks_without_params.partition_point(|b| *b <= block); @@ -1160,13 +1028,7 @@ where /// Chooses a random block which can be targeted by a jump / branch. /// This means any block that is not the first block. - /// - /// For convenience we also generate values that match the block's signature - fn generate_target_block( - &mut self, - builder: &mut FunctionBuilder, - source_block: Block, - ) -> Result<(Block, Vec)> { + fn generate_target_block(&mut self, source_block: Block) -> Result { // We try to mostly generate forward branches to avoid generating an excessive amount of // infinite loops. But they are still important, so give them a small chance of existing. let (backwards_blocks, forward_blocks) = @@ -1179,9 +1041,17 @@ where }; assert!(!block_targets.is_empty()); - let (block, signature) = self.u.choose(block_targets)?.clone(); - let args = self.generate_values_for_signature(builder, signature.into_iter())?; - Ok((block, args)) + let (block, _) = self.u.choose(block_targets)?.clone(); + Ok(block) + } + + fn generate_values_for_block( + &mut self, + builder: &mut FunctionBuilder, + block: Block, + ) -> Result> { + let (_, sig) = self.resources.blocks[block.as_u32() as usize].clone(); + self.generate_values_for_signature(builder, sig.iter().copied()) } fn generate_values_for_signature>( @@ -1198,48 +1068,77 @@ where .collect() } - /// We always need to exit safely out of a block. - /// This either means a jump into another block or a return. - fn finalize_block(&mut self, builder: &mut FunctionBuilder, source_block: Block) -> Result<()> { - let has_jump_tables = !self - .resources - .forward_jump_tables(builder, source_block) - .is_empty(); + /// The terminator that we need to insert has already been picked ahead of time + /// we just need to build the instructions for it + fn insert_terminator( + &mut self, + builder: &mut FunctionBuilder, + source_block: Block, + ) -> Result<()> { + let terminator = self.resources.block_terminators[source_block.as_u32() as usize].clone(); - let has_forward_blocks = { - let (_, forward_blocks) = self.resources.partition_target_blocks(source_block); - !forward_blocks.is_empty() - }; + match terminator { + BlockTerminator::Return => { + let types: Vec = { + let rets = &builder.func.signature.returns; + rets.iter().map(|p| p.value_type).collect() + }; + let vals = self.generate_values_for_signature(builder, types.into_iter())?; - let has_forward_blocks_without_params = !self - .resources - .forward_blocks_without_params(source_block) - .is_empty(); + builder.ins().return_(&vals[..]); + } + BlockTerminator::Jump(target) => { + let args = self.generate_values_for_block(builder, target)?; + builder.ins().jump(target, &args[..]); + } + BlockTerminator::Br(left, right) => { + let left_args = self.generate_values_for_block(builder, left)?; + let right_args = self.generate_values_for_block(builder, right)?; - let terminators: &[(BlockTerminator, bool)] = &[ - // Return is always a valid option - (insert_return, true), - // If we have forward blocks, we can allow generating jumps and branches - (insert_jump, has_forward_blocks), - (insert_br, has_forward_blocks), - (insert_bricmp, has_forward_blocks), - // Switches can only use blocks without params - (insert_switch, has_forward_blocks_without_params), - // We need both jump tables and a default block for br_table - ( - insert_br_table, - has_jump_tables && has_forward_blocks_without_params, - ), - ]; + let condbr_types = [I8, I16, I32, I64, I128, B1]; + let _type = *self.u.choose(&condbr_types[..])?; + let val = builder.use_var(self.get_variable_of_type(_type)?); - let terminators: Vec<_> = terminators - .into_iter() - .filter(|(_, valid)| *valid) - .map(|(term, _)| term) - .collect(); + if bool::arbitrary(self.u)? { + builder.ins().brz(val, left, &left_args[..]); + } else { + builder.ins().brnz(val, left, &left_args[..]); + } + builder.ins().jump(right, &right_args[..]); + } + BlockTerminator::BrIcmp(left, right) => { + let cc = *self.u.choose(IntCC::all())?; + let _type = *self.u.choose(&[I8, I16, I32, I64, I128])?; - let inserter = self.u.choose(&terminators[..])?; - inserter(self, builder, source_block)?; + let lhs = builder.use_var(self.get_variable_of_type(_type)?); + let rhs = builder.use_var(self.get_variable_of_type(_type)?); + + let left_args = self.generate_values_for_block(builder, left)?; + let right_args = self.generate_values_for_block(builder, right)?; + + builder.ins().br_icmp(cc, lhs, rhs, left, &left_args[..]); + builder.ins().jump(right, &right_args[..]); + } + BlockTerminator::BrTable(default, targets) => { + // Create jump tables on demand + let jt = builder.create_jump_table(JumpTableData::with_blocks(targets)); + + // br_table only supports I32 + let val = builder.use_var(self.get_variable_of_type(I32)?); + + builder.ins().br_table(val, default, jt); + } + BlockTerminator::Switch(_type, default, entries) => { + let mut switch = Switch::new(); + for (&entry, &block) in entries.iter() { + switch.set_entry(entry, block); + } + + let switch_val = builder.use_var(self.get_variable_of_type(_type)?); + + switch.emit(builder, switch_val, default); + } + } Ok(()) } @@ -1254,27 +1153,6 @@ where Ok(()) } - fn generate_jumptables(&mut self, builder: &mut FunctionBuilder) -> Result<()> { - // We shouldn't try to generate jumptables if we don't have any valid targets! - if self.resources.blocks_without_params.is_empty() { - return Ok(()); - } - - for _ in 0..self.param(&self.config.jump_tables_per_function)? { - let mut jt_data = JumpTableData::new(); - - for _ in 0..self.param(&self.config.jump_table_entries)? { - let block = *self.u.choose(&self.resources.blocks_without_params)?; - jt_data.push_entry(block); - } - - self.resources - .jump_tables - .push(builder.create_jump_table(jt_data)); - } - Ok(()) - } - fn generate_funcrefs(&mut self, builder: &mut FunctionBuilder) -> Result<()> { let count = self.param(&self.config.funcrefs_per_function)?; for func_index in 0..count.try_into().unwrap() { @@ -1396,6 +1274,118 @@ where .map(|(b, _)| *b) .collect(); + // Compute the block CFG + // + // cranelift-frontend requires us to never generate unreachable blocks + // To ensure this property we start by constructing a main "spine" of blocks. So block1 can + // always jump to block2, and block2 can always jump to block3, etc... + // + // That is not a very interesting CFG, so we introduce variations on that, but always + // ensuring that the property of pointing to the next block is maintained whatever the + // branching mechanism we use. + let blocks = self.resources.blocks.clone(); + self.resources.block_terminators = blocks + .iter() + .map(|&(block, _)| { + let next_block = Block::with_number(block.as_u32() + 1).unwrap(); + let forward_blocks = self.resources.forward_blocks(block); + let paramless_targets = self.resources.forward_blocks_without_params(block); + let has_paramless_targets = !paramless_targets.is_empty(); + let next_block_is_paramless = paramless_targets.contains(&next_block); + + let mut valid_terminators = vec![]; + + if forward_blocks.is_empty() { + // Return is only valid on the last block. + valid_terminators.push(BlockTerminatorKind::Return); + } else { + // If we have more than one block we can allow terminators that target blocks. + // TODO: We could add some kind of BrReturn/BrIcmpReturn here, to explore edges where we exit + // in the middle of the function + valid_terminators.extend_from_slice(&[ + BlockTerminatorKind::Jump, + BlockTerminatorKind::Br, + BlockTerminatorKind::BrIcmp, + ]); + } + + // BrTable and the Switch interface only allow targeting blocks without params + // we also need to ensure that the next block has no params, since that one is + // guaranteed to be picked in either case. + if has_paramless_targets && next_block_is_paramless { + valid_terminators.extend_from_slice(&[ + BlockTerminatorKind::BrTable, + BlockTerminatorKind::Switch, + ]); + } + + let terminator = self.u.choose(&valid_terminators[..])?; + + // Choose block targets for the terminators that we picked above + Ok(match terminator { + BlockTerminatorKind::Return => BlockTerminator::Return, + BlockTerminatorKind::Jump => BlockTerminator::Jump(next_block), + BlockTerminatorKind::Br => { + BlockTerminator::Br(next_block, self.generate_target_block(block)?) + } + BlockTerminatorKind::BrIcmp => { + BlockTerminator::BrIcmp(next_block, self.generate_target_block(block)?) + } + // TODO: Allow generating backwards branches here + BlockTerminatorKind::BrTable => { + // Make the default the next block, and then we don't have to worry + // that we can reach it via the targets + let default = next_block; + + let target_count = self.param(&self.config.jump_table_entries)?; + let targets = arbitrary_vec( + self.u, + target_count, + self.resources.forward_blocks_without_params(block), + )?; + + BlockTerminator::BrTable(default, targets) + } + BlockTerminatorKind::Switch => { + // Make the default the next block, and then we don't have to worry + // that we can reach it via the entries below + let default_block = next_block; + + let _type = *self.u.choose(&[I8, I16, I32, I64, I128][..])?; + + // Build this into a HashMap since we cannot have duplicate entries. + let mut entries = HashMap::new(); + for _ in 0..self.param(&self.config.switch_cases)? { + // The Switch API only allows for entries that are addressable by the index type + // so we need to limit the range of values that we generate. + let (ty_min, ty_max) = _type.bounds(false); + let range_start = self.u.int_in_range(ty_min..=ty_max)?; + + // We can either insert a contiguous range of blocks or a individual block + // This is done because the Switch API specializes contiguous ranges. + let range_size = if bool::arbitrary(self.u)? { + 1 + } else { + self.param(&self.config.switch_max_range_size)? + } as u128; + + // Build the switch entries + for i in 0..range_size { + let index = range_start.wrapping_add(i) % ty_max; + let block = *self + .u + .choose(self.resources.forward_blocks_without_params(block))?; + + entries.insert(index, block); + } + } + + BlockTerminator::Switch(_type, default_block, entries) + } + }) + }) + .collect::>()?; + Ok(()) } @@ -1463,7 +1453,6 @@ where self.generate_blocks(&mut builder, &sig)?; // Function preamble - self.generate_jumptables(&mut builder)?; self.generate_funcrefs(&mut builder)?; self.generate_stack_slots(&mut builder)?; @@ -1493,7 +1482,8 @@ where // Generate block instructions self.generate_instructions(&mut builder)?; - self.finalize_block(&mut builder, block)?; + // Insert a terminator to safely exit the block + self.insert_terminator(&mut builder, block)?; } builder.seal_all_blocks();