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(