cranelift: Validate iconst ranges (bytecodealliance#6850)

* cranelift: Validate `iconst` ranges

Add the following checks:

`iconst.i8`  immediate must be within 0 .. 2^8-1
`iconst.i16` immediate must be within 0 .. 2^16-1
`iconst.i32` immediate must be within 0 .. 2^32-1

Resolves bytecodealliance#3059

* cranelift: Parse `iconst` according to its type

Modifies the parser for textual CLIF so that V in `iconst.T V` is
parsed according to T.

Before this commit, something like `iconst.i32 0xffff_ffff_ffff` was
valid because all `iconst` were parsed the same as an
`iconst.i64`. Now the above example will throw an error.

Also, a negative immediate as in `iconst.iN -X` is now converted to
`2^N - X`.

This commit also fixes some broken tests.

* cranelift: Update tests to match new CLIF parser

cranelift/codegen/src/legalizer/mod.rs

@@ -16,7 +16,7 @@
 use crate::cursor::{Cursor, FuncCursor};
 use crate::flowgraph::ControlFlowGraph;
 use crate::ir::immediates::Imm64;
-use crate::ir::types::{I128, I64};
+use crate::ir::types::{self, I128, I64};
 use crate::ir::{self, InstBuilder, InstructionData, MemFlags, Value};
 use crate::isa::TargetIsa;
 use crate::trace;
@@ -38,7 +38,17 @@ fn imm_const(pos: &mut FuncCursor, arg: Value, imm: Imm64, is_signed: bool) -> V
  let imm = pos.ins().iconst(I64, imm);
  pos.ins().uextend(I128, imm)
  }
- _ => pos.ins().iconst(ty.lane_type(), imm),
+ _ => {
+ let bits = imm.bits();
+ let unsigned = match ty.lane_type() {
+ types::I8 => bits as u8 as i64,
+ types::I16 => bits as u16 as i64,
+ types::I32 => bits as u32 as i64,
+ types::I64 => bits,
+ _ => unreachable!(),
+ };
+ pos.ins().iconst(ty.lane_type(), unsigned)
+ }
  }
 }
 

cranelift/codegen/src/verifier/mod.rs

@@ -1659,6 +1659,39 @@ impl<'a> Verifier<'a> {
  }
  }
 
+ fn iconst_bounds(&self, inst: Inst, errors: &mut VerifierErrors) -> VerifierStepResult<()> {
+ use crate::ir::instructions::InstructionData::UnaryImm;
+
+ let inst_data = &self.func.dfg.insts[inst];
+ if let UnaryImm {
+ opcode: Opcode::Iconst,
+ imm,
+ } = inst_data
+ {
+ let ctrl_typevar = self.func.dfg.ctrl_typevar(inst);
+ let bounds_mask = match ctrl_typevar {
+ types::I8 => u8::MAX.into(),
+ types::I16 => u16::MAX.into(),
+ types::I32 => u32::MAX.into(),
+ types::I64 => u64::MAX,
+ _ => unreachable!(),
+ };
+
+ let value = imm.bits() as u64;
+ if value & bounds_mask != value {
+ errors.fatal((
+ inst,
+ self.context(inst),
+ "constant immediate is out of bounds",
+ ))
+ } else {
+ Ok(())
+ }
+ } else {
+ Ok(())
+ }
+ }
+
  fn typecheck_function_signature(&self, errors: &mut VerifierErrors) -> VerifierStepResult<()> {
  let params = self
  .func
@@ -1735,6 +1768,7 @@ impl<'a> Verifier<'a> {
  self.instruction_integrity(inst, errors)?;
  self.typecheck(inst, errors)?;
  self.immediate_constraints(inst, errors)?;
+ self.iconst_bounds(inst, errors)?;
  }
 
  self.encodable_as_bb(block, errors)?;
@@ -1755,7 +1789,7 @@ impl<'a> Verifier<'a> {
 mod tests {
  use super::{Verifier, VerifierError, VerifierErrors};
  use crate::ir::instructions::{InstructionData, Opcode};
- use crate::ir::{types, AbiParam, Function};
+ use crate::ir::{types, AbiParam, Function, Type};
  use crate::settings;
 
  macro_rules! assert_err_with_msg {
@@ -1812,6 +1846,74 @@ mod tests {
  assert_err_with_msg!(errors, "instruction format");
  }
 
+ fn test_iconst_bounds(immediate: i64, ctrl_typevar: Type) -> VerifierErrors {
+ let mut func = Function::new();
+ let block0 = func.dfg.make_block();
+ func.layout.append_block(block0);
+
+ let test_inst = func.dfg.make_inst(InstructionData::UnaryImm {
+ opcode: Opcode::Iconst,
+ imm: immediate.into(),
+ });
+
+ let end_inst = func.dfg.make_inst(InstructionData::MultiAry {
+ opcode: Opcode::Return,
+ args: Default::default(),
+ });
+
+ func.dfg.append_result(test_inst, ctrl_typevar);
+ func.layout.append_inst(test_inst, block0);
+ func.layout.append_inst(end_inst, block0);
+
+ let flags = &settings::Flags::new(settings::builder());
+ let verifier = Verifier::new(&func, flags.into());
+ let mut errors = VerifierErrors::default();
+
+ let _ = verifier.run(&mut errors);
+ errors
+ }
+
+ fn test_iconst_bounds_err(immediate: i64, ctrl_typevar: Type) {
+ assert_err_with_msg!(
+ test_iconst_bounds(immediate, ctrl_typevar),
+ "constant immediate is out of bounds"
+ );
+ }
+
+ fn test_iconst_bounds_ok(immediate: i64, ctrl_typevar: Type) {
+ assert!(test_iconst_bounds(immediate, ctrl_typevar).is_empty());
+ }
+
+ #[test]
+ fn negative_iconst_8() {
+ test_iconst_bounds_err(-10, types::I8);
+ }
+
+ #[test]
+ fn negative_iconst_32() {
+ test_iconst_bounds_err(-1, types::I32);
+ }
+
+ #[test]
+ fn large_iconst_8() {
+ test_iconst_bounds_err(1 + u8::MAX as i64, types::I8);
+ }
+
+ #[test]
+ fn large_iconst_16() {
+ test_iconst_bounds_err(10 + u16::MAX as i64, types::I16);
+ }
+
+ #[test]
+ fn valid_iconst_8() {
+ test_iconst_bounds_ok(10, types::I8);
+ }
+
+ #[test]
+ fn valid_iconst_32() {
+ test_iconst_bounds_ok(u32::MAX as i64, types::I32);
+ }
+
  #[test]
  fn test_function_invalid_param() {
  let mut func = Function::new();

cranelift/filetests/filetests/egraph/extends.clif

@@ -4,7 +4,7 @@ target x86_64
 
 function %f1() -> i64 {
 block0:
- v0 = iconst.i32 0xffff_ffff_9876_5432
+ v0 = iconst.i32 0x9876_5432
  v1 = uextend.i64 v0
  return v1
  ; check: v2 = iconst.i64 0x9876_5432

cranelift/filetests/filetests/isa/s390x/constants.clif

@@ -9,12 +9,12 @@ block0:
 
 ; VCode:
 ; block0:
-; lhi %r2, -1
+; lhi %r2, 255
 ; br %r14
 ;
 ; Disassembled:
 ; block0: ; offset 0x0
-; lhi %r2, -1
+; lhi %r2, 0xff
 ; br %r14
 
 function %f() -> i16 {
@@ -189,11 +189,11 @@ block0:
 
 ; VCode:
 ; block0:
-; lhi %r2, -1
+; iilf %r2, 4294967295
 ; br %r14
 ;
 ; Disassembled:
 ; block0: ; offset 0x0
-; lhi %r2, -1
+; iilf %r2, 0xffffffff
 ; br %r14
 

cranelift/filetests/filetests/isa/x64/simd-lane-access-compile.clif

@@ -193,7 +193,7 @@ block0:
 ; pushq %rbp
 ; movq %rsp, %rbp
 ; block0:
-; movl $-1, %ecx
+; movl $65535, %ecx
 ; movd %ecx, %xmm1
 ; pshuflw $0, %xmm1, %xmm3
 ; pshufd $0, %xmm3, %xmm0
@@ -206,7 +206,7 @@ block0:
 ; pushq %rbp
 ; movq %rsp, %rbp
 ; block1: ; offset 0x4
-; movl $0xffffffff, %ecx
+; movl $0xffff, %ecx
 ; movd %ecx, %xmm1
 ; pshuflw $0, %xmm1, %xmm3
 ; pshufd $0, %xmm3, %xmm0

cranelift/filetests/filetests/parser/tiny.clif

@@ -36,7 +36,7 @@ block0:
 }
 ; sameln: function %bvalues() fast {
 ; nextln: block0:
-; nextln: v0 = iconst.i32 -1
+; nextln: v0 = iconst.i32 0xffff_ffff
 ; nextln: v1 = iconst.i8 0
 ; nextln: v2 = sextend.i32 v1
 ; nextln: v3 = bxor v0, v2

cranelift/reader/src/parser.rs

@@ -12,6 +12,7 @@ use cranelift_codegen::entity::{EntityRef, PrimaryMap};
 use cranelift_codegen::ir::entities::{AnyEntity, DynamicType};
 use cranelift_codegen::ir::immediates::{Ieee32, Ieee64, Imm64, Offset32, Uimm32, Uimm64};
 use cranelift_codegen::ir::instructions::{InstructionData, InstructionFormat, VariableArgs};
+use cranelift_codegen::ir::types;
 use cranelift_codegen::ir::types::INVALID;
 use cranelift_codegen::ir::types::*;
 use cranelift_codegen::ir::{self, UserExternalNameRef};
@@ -2456,10 +2457,25 @@ impl<'a> Parser<'a> {
  opcode,
  arg: self.match_value("expected SSA value operand")?,
  },
- InstructionFormat::UnaryImm => InstructionData::UnaryImm {
- opcode,
- imm: self.match_imm64("expected immediate integer operand")?,
- },
+ InstructionFormat::UnaryImm => {
+ let msg = |bits| format!("expected immediate {bits}-bit integer operand");
+ let unsigned = match explicit_control_type {
+ Some(types::I8) => self.match_imm8(&msg(8))? as u8 as i64,
+ Some(types::I16) => self.match_imm16(&msg(16))? as u16 as i64,
+ Some(types::I32) => self.match_imm32(&msg(32))? as u32 as i64,
+ Some(types::I64) => self.match_imm64(&msg(64))?.bits(),
+ _ => {
+ return err!(
+ self.loc,
+ "expected one of the following type: i8, i16, i32 or i64"
+ )
+ }
+ };
+ InstructionData::UnaryImm {
+ opcode,
+ imm: Imm64::new(unsigned),
+ }
+ }
  InstructionFormat::UnaryIeee32 => InstructionData::UnaryIeee32 {
  opcode,
  imm: self.match_ieee32("expected immediate 32-bit float operand")?,

cranelift/tests/bugpoint_test.clif

@@ -910,7 +910,7 @@ block51:
  v428 = iadd_imm.i64 v28, 8
  v429 = load.i16 v428
  v435 -> v429
- v430 = iconst.i16 0xffff_ffff_ffff_8000
+ v430 = iconst.i16 0x8000
  v431 = icmp eq v429, v430
  v433 = uextend.i32 v431
  brif v433, block154, block52

cranelift/wasm/src/code_translator.rs

@@ -928,7 +928,9 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
  translate_store(memarg, ir::Opcode::Store, builder, state, environ)?;
  }
  /****************************** Nullary Operators ************************************/
- Operator::I32Const { value } => state.push1(builder.ins().iconst(I32, i64::from(*value))),
+ Operator::I32Const { value } => {
+ state.push1(builder.ins().iconst(I32, *value as u32 as i64))
+ }
  Operator::I64Const { value } => state.push1(builder.ins().iconst(I64, *value)),
  Operator::F32Const { value } => {
  state.push1(builder.ins().f32const(f32_translation(*value)));

cranelift/wasm/src/environ/dummy.rs

@@ -509,7 +509,7 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ
  _heap: Heap,
  _val: ir::Value,
  ) -> WasmResult<ir::Value> {
- Ok(pos.ins().iconst(I32, -1))
+ Ok(pos.ins().iconst(I32, -1i32 as u32 as i64))
  }
 
  fn translate_memory_size(
@@ -518,7 +518,7 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ
  _index: MemoryIndex,
  _heap: Heap,
  ) -> WasmResult<ir::Value> {
- Ok(pos.ins().iconst(I32, -1))
+ Ok(pos.ins().iconst(I32, -1i32 as u32 as i64))
  }
 
  fn translate_memory_copy(
@@ -570,7 +570,7 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ
  _index: TableIndex,
  _table: ir::Table,
  ) -> WasmResult<ir::Value> {
- Ok(pos.ins().iconst(I32, -1))
+ Ok(pos.ins().iconst(I32, -1i32 as u32 as i64))
  }
 
  fn translate_table_grow(
@@ -581,7 +581,7 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ
  _delta: ir::Value,
  _init_value: ir::Value,
  ) -> WasmResult<ir::Value> {
- Ok(pos.ins().iconst(I32, -1))
+ Ok(pos.ins().iconst(I32, -1i32 as u32 as i64))
  }
 
  fn translate_table_get(
@@ -660,7 +660,7 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ
  mut pos: FuncCursor,
  _global_index: GlobalIndex,
  ) -> WasmResult<ir::Value> {
- Ok(pos.ins().iconst(I32, -1))
+ Ok(pos.ins().iconst(I32, -1i32 as u32 as i64))
  }
 
  fn translate_custom_global_set(
@@ -681,7 +681,7 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ
  _expected: ir::Value,
  _timeout: ir::Value,
  ) -> WasmResult<ir::Value> {
- Ok(pos.ins().iconst(I32, -1))
+ Ok(pos.ins().iconst(I32, -1i32 as u32 as i64))
  }
 
  fn translate_atomic_notify(