From f2a34eb0fc95c8a76ed1abe9278ff08ec352e6f0 Mon Sep 17 00:00:00 2001 From: Afonso Bordado Date: Fri, 19 Aug 2022 18:09:19 +0100 Subject: [PATCH 1/3] cranelift: Add assert to prevent wrong InstFormat being used for the wrong opcode --- cranelift/codegen/meta/src/gen_inst.rs | 3 +++ cranelift/codegen/src/ir/builder.rs | 16 +++++++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/cranelift/codegen/meta/src/gen_inst.rs b/cranelift/codegen/meta/src/gen_inst.rs index d85f6dc69b22..ca5afb47595b 100644 --- a/cranelift/codegen/meta/src/gen_inst.rs +++ b/cranelift/codegen/meta/src/gen_inst.rs @@ -902,6 +902,9 @@ fn gen_format_constructor(format: &InstructionFormat, fmt: &mut Formatter) { fmtln!(fmt, "data.sign_extend_immediates(ctrl_typevar);"); } + // Assert that this opcode belongs to this format + fmtln!(fmt, "assert_eq!(opcode.format(), InstructionFormat::from(&data), \"Wrong InstructionFormat for Opcode: {}\", opcode);"); + fmt.line("self.build(data, ctrl_typevar)"); }); fmtln!(fmt, "}"); diff --git a/cranelift/codegen/src/ir/builder.rs b/cranelift/codegen/src/ir/builder.rs index 3191f9dae159..4fd8b0665f3b 100644 --- a/cranelift/codegen/src/ir/builder.rs +++ b/cranelift/codegen/src/ir/builder.rs @@ -4,6 +4,7 @@ //! function. Many of its methods are generated from the meta language instruction definitions. use crate::ir; +use crate::ir::instructions::InstructionFormat; use crate::ir::types; use crate::ir::{DataFlowGraph, InstructionData}; use crate::ir::{Inst, Opcode, Type, Value}; @@ -217,7 +218,7 @@ mod tests { use crate::cursor::{Cursor, FuncCursor}; use crate::ir::condcodes::*; use crate::ir::types::*; - use crate::ir::{Function, InstBuilder, ValueDef}; + use crate::ir::{Function, InstBuilder, Opcode, TrapCode, ValueDef}; #[test] fn types() { @@ -262,4 +263,17 @@ mod tests { assert!(iadd != iconst); assert_eq!(pos.func.dfg.value_def(v0), ValueDef::Result(iconst, 0)); } + + #[test] + #[should_panic] + fn panics_when_inserting_wrong_opcode() { + let mut func = Function::new(); + let block0 = func.dfg.make_block(); + let mut pos = FuncCursor::new(&mut func); + pos.insert_block(block0); + + // We are trying to create a Opcode::Return with the InstData::Trap, which is obviously wrong + pos.ins() + .Trap(Opcode::Return, I32, TrapCode::BadConversionToInteger); + } } From 2875d9c16fe207082194df0e290456734df5d07c Mon Sep 17 00:00:00 2001 From: Afonso Bordado Date: Fri, 19 Aug 2022 18:59:38 +0100 Subject: [PATCH 2/3] cranelift: Use correct instruction format when inserting opcodes in fuzzgen --- cranelift/fuzzgen/src/function_generator.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/cranelift/fuzzgen/src/function_generator.rs b/cranelift/fuzzgen/src/function_generator.rs index c15a496fc007..0656d138a9c2 100644 --- a/cranelift/fuzzgen/src/function_generator.rs +++ b/cranelift/fuzzgen/src/function_generator.rs @@ -1,7 +1,8 @@ -use crate::codegen::ir::{ArgumentExtension, ArgumentPurpose, ValueList}; +use crate::codegen::ir::{ArgumentExtension, ArgumentPurpose}; use crate::config::Config; use anyhow::Result; use arbitrary::{Arbitrary, Unstructured}; +use cranelift::codegen::ir::instructions::InstructionFormat; use cranelift::codegen::ir::{types::*, FuncRef, LibCall, UserExternalName, UserFuncName}; use cranelift::codegen::ir::{ AbiParam, Block, ExternalName, Function, JumpTable, Opcode, Signature, StackSlot, Type, Value, @@ -24,11 +25,11 @@ fn insert_opcode( args: &'static [Type], rets: &'static [Type], ) -> Result<()> { - let mut arg_vals = ValueList::new(); + let mut vals = Vec::with_capacity(args.len()); for &arg in args.into_iter() { let var = fgen.get_variable_of_type(arg)?; let val = builder.use_var(var); - arg_vals.push(val, &mut builder.func.dfg.value_lists); + vals.push(val); } // For pretty much every instruction the control type is the return type @@ -42,7 +43,16 @@ fn insert_opcode( .copied() .unwrap_or(INVALID); - let (inst, dfg) = builder.ins().MultiAry(opcode, ctrl_type, arg_vals); + // Choose the appropriate instruction format for this opcode + let (inst, dfg) = match opcode.format() { + InstructionFormat::NullAry => builder.ins().NullAry(opcode, ctrl_type), + InstructionFormat::Unary => builder.ins().Unary(opcode, ctrl_type, vals[0]), + InstructionFormat::Binary => builder.ins().Binary(opcode, ctrl_type, vals[0], vals[1]), + InstructionFormat::Ternary => builder + .ins() + .Ternary(opcode, ctrl_type, vals[0], vals[1], vals[2]), + _ => unimplemented!(), + }; let results = dfg.inst_results(inst).to_vec(); for (val, &ty) in results.into_iter().zip(rets) { From 6cb178b214d42b9e639a9307463ff62bd50aaabf Mon Sep 17 00:00:00 2001 From: Afonso Bordado Date: Sat, 20 Aug 2022 01:07:04 +0100 Subject: [PATCH 3/3] cranelift: Use debug assert on InstFormat assert --- cranelift/codegen/meta/src/gen_inst.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cranelift/codegen/meta/src/gen_inst.rs b/cranelift/codegen/meta/src/gen_inst.rs index ca5afb47595b..819b553ce495 100644 --- a/cranelift/codegen/meta/src/gen_inst.rs +++ b/cranelift/codegen/meta/src/gen_inst.rs @@ -903,7 +903,7 @@ fn gen_format_constructor(format: &InstructionFormat, fmt: &mut Formatter) { } // Assert that this opcode belongs to this format - fmtln!(fmt, "assert_eq!(opcode.format(), InstructionFormat::from(&data), \"Wrong InstructionFormat for Opcode: {}\", opcode);"); + fmtln!(fmt, "debug_assert_eq!(opcode.format(), InstructionFormat::from(&data), \"Wrong InstructionFormat for Opcode: {}\", opcode);"); fmt.line("self.build(data, ctrl_typevar)"); });