From 4bac38446e56312491c227d26ca819af043e906f Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Tue, 24 Oct 2023 14:34:17 -0700 Subject: [PATCH 1/3] PCC: support x86-64. This PR extends the proof-carrying-code infrastructure to support x86-64 as well as aarch64. In the process, many of the mechanisms had to be made a little more general. One important change is that the PCC leaves more "breadcrumbs" on the frontend now, avoiding the need for magic handling of facts on constant values, etc., in the backend. For the first time a lowering rule also gains the ability to add a fact to a vreg to preserve the chain as well. With these changes, we can validate compilation of SpiderMonkey.wasm with Wasm static memories on x86-64 and aarch64: ``` cfallin@fastly2:~/work/wasmtime% target/release/wasmtime compile -C pcc=yes --target x86_64 ../wasm-tests/spidermonkey.wasm cfallin@fastly2:~/work/wasmtime% target/release/wasmtime compile -C pcc=yes --target aarch64 ../wasm-tests/spidermonkey.wasm cfallin@fastly2:~/work/wasmtime% ``` --- cranelift/codegen/src/ir/pcc.rs | 128 +++- .../codegen/src/isa/aarch64/inst/imms.rs | 4 +- cranelift/codegen/src/isa/aarch64/pcc.rs | 323 ++++------ cranelift/codegen/src/isa/x64/inst.isle | 2 +- cranelift/codegen/src/isa/x64/inst/args.rs | 25 +- cranelift/codegen/src/isa/x64/lower.rs | 11 + cranelift/codegen/src/isa/x64/mod.rs | 1 + cranelift/codegen/src/isa/x64/pcc.rs | 600 ++++++++++++++++++ .../codegen/src/legalizer/globalvalue.rs | 13 +- cranelift/codegen/src/machinst/compile.rs | 3 +- cranelift/codegen/src/machinst/isle.rs | 5 + cranelift/codegen/src/machinst/lower.rs | 22 +- cranelift/codegen/src/machinst/mod.rs | 1 + cranelift/codegen/src/machinst/pcc.rs | 160 +++++ cranelift/codegen/src/machinst/vcode.rs | 8 + cranelift/codegen/src/prelude_lower.isle | 5 + .../filetests/filetests/pcc/fail/add.clif | 1 + .../filetests/pcc/fail/blockparams.clif | 1 + .../filetests/filetests/pcc/fail/extend.clif | 12 +- .../filetests/filetests/pcc/fail/load.clif | 1 + .../filetests/pcc/fail/memtypes.clif | 1 + .../filetests/filetests/pcc/fail/shift.clif | 1 + .../filetests/filetests/pcc/fail/simple.clif | 1 + .../filetests/filetests/pcc/fail/struct.clif | 1 + .../filetests/filetests/pcc/fail/vmctx.clif | 1 + .../filetests/filetests/pcc/succeed/add.clif | 1 + .../filetests/pcc/succeed/blockparams.clif | 1 + .../filetests/pcc/succeed/extend.clif | 7 + .../filetests/pcc/succeed/gv_fact.clif | 5 +- .../filetests/filetests/pcc/succeed/load.clif | 5 +- .../filetests/pcc/succeed/memtypes.clif | 1 + .../filetests/filetests/pcc/succeed/opt.clif | 1 + .../filetests/pcc/succeed/shift.clif | 1 + .../filetests/pcc/succeed/simple.clif | 1 + .../filetests/pcc/succeed/struct.clif | 1 + .../filetests/pcc/succeed/vmctx.clif | 1 + .../wasm/src/code_translator/bounds_checks.rs | 10 + 37 files changed, 1124 insertions(+), 242 deletions(-) create mode 100644 cranelift/codegen/src/isa/x64/pcc.rs create mode 100644 cranelift/codegen/src/machinst/pcc.rs diff --git a/cranelift/codegen/src/ir/pcc.rs b/cranelift/codegen/src/ir/pcc.rs index f2506e3ee519..7a6cd840f5ab 100644 --- a/cranelift/codegen/src/ir/pcc.rs +++ b/cranelift/codegen/src/ir/pcc.rs @@ -220,6 +220,26 @@ impl Fact { } } + /// Create a range fact that specifies the maximum range for a + /// value of the given bit-width, zero-extended into a wider + /// width. + pub const fn max_range_for_width_extended(from_width: u16, to_width: u16) -> Self { + debug_assert!(from_width <= to_width); + match from_width { + from_width if from_width < 64 => Fact::Range { + bit_width: to_width, + min: 0, + max: (1u64 << from_width) - 1, + }, + 64 => Fact::Range { + bit_width: to_width, + min: 0, + max: u64::MAX, + }, + _ => panic!("bit width too large!"), + } + } + /// Try to infer a minimal fact for a value of the given IR type. pub fn infer_from_type(ty: ir::Type) -> Option<&'static Self> { static FACTS: [Fact; 4] = [ @@ -362,13 +382,14 @@ impl<'a> FactContext<'a> { }, ) => { // If the bitwidths we're claiming facts about are the - // same, and if the right-hand-side range is larger - // than the left-hand-side range, than the LHS + // same, or the left-hand-side makes a claim about a + // wider bitwidth, and if the right-hand-side range is + // larger than the left-hand-side range, than the LHS // subsumes the RHS. // // In other words, we can always expand the claimed // possible value range. - bw_lhs == bw_rhs && max_lhs <= max_rhs && min_lhs >= min_rhs + bw_lhs >= bw_rhs && max_lhs <= max_rhs && min_lhs >= min_rhs } ( @@ -472,28 +493,38 @@ impl<'a> FactContext<'a> { /// Computes the `uextend` of a value with the given facts. pub fn uextend(&self, fact: &Fact, from_width: u16, to_width: u16) -> Option { + trace!( + "uextend: fact {:?} from {} to {}", + fact, + from_width, + to_width + ); + if from_width == to_width { + return Some(fact.clone()); + } + match fact { - // If we have a defined value in bits 0..bit_width, and we - // are filling zeroes into from_bits..to_bits, and - // bit_width and from_bits are exactly contiguous, then we - // have defined values in 0..to_bits (and because this is - // a zero-extend, the max value is the same). + // If the claim is already for a same-or-wider value and the min + // and max are within range of the narrower value, we can + // claim the same range. Fact::Range { bit_width, min, max, - } if *bit_width == from_width => Some(Fact::Range { - bit_width: to_width, - min: *min, - max: *max, - }), + } if *bit_width >= from_width + && *min <= max_value_for_width(from_width) + && *max <= max_value_for_width(from_width) => + { + Some(Fact::Range { + bit_width: to_width, + min: *min, + max: *max, + }) + } // Otherwise, we can at least claim that the value is - // within the range of `to_width`. - Fact::Range { .. } => Some(Fact::Range { - bit_width: to_width, - min: 0, - max: max_value_for_width(to_width), - }), + // within the range of `from_width`. + Fact::Range { .. } => Some(Fact::max_range_for_width_extended(from_width, to_width)), + _ => None, } } @@ -517,6 +548,44 @@ impl<'a> FactContext<'a> { } } + /// Computes the bit-truncation of a value with the given fact. + pub fn truncate(&self, fact: &Fact, from_width: u16, to_width: u16) -> Option { + if from_width == to_width { + return Some(fact.clone()); + } + + trace!( + "truncate: fact {:?} from {} to {}", + fact, + from_width, + to_width + ); + + match fact { + Fact::Range { + bit_width, + min, + max, + } if *bit_width == from_width => { + let max_val = (1u64 << to_width) - 1; + if *min <= max_val && *max <= max_val { + Some(Fact::Range { + bit_width: to_width, + min: *min, + max: *max, + }) + } else { + Some(Fact::Range { + bit_width: to_width, + min: 0, + max: max_val, + }) + } + } + _ => None, + } + } + /// Scales a value with a fact by a known constant. pub fn scale(&self, fact: &Fact, width: u16, factor: u32) -> Option { match fact { @@ -551,11 +620,6 @@ impl<'a> FactContext<'a> { /// Offsets a value with a fact by a known amount. pub fn offset(&self, fact: &Fact, width: u16, offset: i64) -> Option { - // Any negative offset could underflow, and removes - // all claims of constrained range, so for now we only - // support positive offsets. - let offset = u64::try_from(offset).ok()?; - trace!( "FactContext::offset: {:?} + {} in width {}", fact, @@ -563,14 +627,22 @@ impl<'a> FactContext<'a> { width ); + let compute_offset = |base: u64| -> Option { + if offset >= 0 { + base.checked_add(u64::try_from(offset).unwrap()) + } else { + base.checked_sub(u64::try_from(-offset).unwrap()) + } + }; + match fact { Fact::Range { bit_width, min, max, } if *bit_width == width => { - let min = min.checked_add(offset)?; - let max = max.checked_add(offset)?; + let min = compute_offset(*min)?; + let max = compute_offset(*max)?; Some(Fact::Range { bit_width: *bit_width, @@ -583,8 +655,8 @@ impl<'a> FactContext<'a> { min_offset: mem_min_offset, max_offset: mem_max_offset, } => { - let min_offset = mem_min_offset.checked_add(offset)?; - let max_offset = mem_max_offset.checked_add(offset)?; + let min_offset = compute_offset(*mem_min_offset)?; + let max_offset = compute_offset(*mem_max_offset)?; Some(Fact::Mem { ty: *ty, min_offset, diff --git a/cranelift/codegen/src/isa/aarch64/inst/imms.rs b/cranelift/codegen/src/isa/aarch64/inst/imms.rs index 54f6bd804bcf..197c87aa18ee 100644 --- a/cranelift/codegen/src/isa/aarch64/inst/imms.rs +++ b/cranelift/codegen/src/isa/aarch64/inst/imms.rs @@ -309,8 +309,8 @@ impl Imm12 { } /// Get the actual value that this immediate corresponds to. - pub fn value(&self) -> u64 { - let base = self.bits as u64; + pub fn value(&self) -> u32 { + let base = self.bits as u32; if self.shift12 { base << 12 } else { diff --git a/cranelift/codegen/src/isa/aarch64/pcc.rs b/cranelift/codegen/src/isa/aarch64/pcc.rs index e1bb383829c1..a84604aa0810 100644 --- a/cranelift/codegen/src/isa/aarch64/pcc.rs +++ b/cranelift/codegen/src/isa/aarch64/pcc.rs @@ -8,44 +8,11 @@ use crate::isa::aarch64::inst::args::{PairAMode, ShiftOp}; use crate::isa::aarch64::inst::Inst; use crate::isa::aarch64::inst::{ALUOp, MoveWideOp}; use crate::isa::aarch64::inst::{AMode, ExtendOp}; +use crate::machinst::pcc::*; use crate::machinst::Reg; use crate::machinst::{InsnIndex, VCode}; use crate::trace; -fn get_fact_or_default(vcode: &VCode, reg: Reg) -> PccResult<&Fact> { - vcode - .vreg_fact(reg.into()) - .or_else(|| Fact::infer_from_type(vcode.vreg_type(reg.into()))) - .ok_or(PccError::MissingFact) -} - -fn has_fact(vcode: &VCode, reg: Reg) -> bool { - vcode.vreg_fact(reg.into()).is_some() -} - -fn fail_if_missing(fact: Option) -> PccResult { - fact.ok_or(PccError::UnsupportedFact) -} - -fn check_subsumes(ctx: &FactContext, subsumer: &Fact, subsumee: &Fact) -> PccResult<()> { - trace!( - "checking if derived fact {:?} subsumes stated fact {:?}", - subsumer, - subsumee - ); - - // For now, allow all `mem` facts to validate. - if matches!(subsumee, Fact::Mem { .. }) { - return Ok(()); - } - - if ctx.subsumes(subsumer, subsumee) { - Ok(()) - } else { - Err(PccError::UnsupportedFact) - } -} - fn extend_fact(ctx: &FactContext, value: &Fact, mode: ExtendOp) -> Option { match mode { ExtendOp::UXTB => ctx.uextend(value, 8, 64), @@ -59,77 +26,6 @@ fn extend_fact(ctx: &FactContext, value: &Fact, mode: ExtendOp) -> Option } } -fn check_output) -> PccResult>( - ctx: &FactContext, - vcode: &mut VCode, - out: Reg, - ins: &[Reg], - f: F, -) -> PccResult<()> { - if let Some(fact) = vcode.vreg_fact(out.into()) { - let result = f(vcode)?; - check_subsumes(ctx, &result, fact) - } else if ins.iter().any(|r| { - vcode - .vreg_fact(r.into()) - .map(|fact| fact.propagates()) - .unwrap_or(false) - }) { - if let Ok(fact) = f(vcode) { - trace!("setting vreg {:?} to {:?}", out, fact); - vcode.set_vreg_fact(out.into(), fact); - } - Ok(()) - } else { - Ok(()) - } -} - -fn check_unop PccResult>( - ctx: &FactContext, - vcode: &mut VCode, - out: Reg, - ra: Reg, - f: F, -) -> PccResult<()> { - check_output(ctx, vcode, out, &[ra], |vcode| { - let ra = get_fact_or_default(vcode, ra)?; - f(ra) - }) -} - -fn check_binop PccResult>( - ctx: &FactContext, - vcode: &mut VCode, - out: Reg, - ra: Reg, - rb: Reg, - f: F, -) -> PccResult<()> { - check_output(ctx, vcode, out, &[ra, rb], |vcode| { - let ra = get_fact_or_default(vcode, ra)?; - let rb = get_fact_or_default(vcode, rb)?; - f(ra, rb) - }) -} - -fn check_constant( - ctx: &FactContext, - vcode: &mut VCode, - out: Reg, - bit_width: u16, - value: u64, -) -> PccResult<()> { - let result = Fact::constant(bit_width, value); - if let Some(fact) = vcode.vreg_fact(out.into()) { - check_subsumes(ctx, &result, fact) - } else { - trace!("setting vreg {:?} to {:?}", out, result); - vcode.set_vreg_fact(out.into(), result); - Ok(()) - } -} - pub(crate) fn check( ctx: &FactContext, vcode: &mut VCode, @@ -145,51 +41,51 @@ pub(crate) fn check( Ok(()) } Inst::ULoad8 { rd, ref mem, flags } | Inst::SLoad8 { rd, ref mem, flags } => { - check_load(&ctx, Some(rd.to_reg()), flags, mem, vcode, I8) + check_load(ctx, Some(rd.to_reg()), flags, mem, vcode, I8) } Inst::ULoad16 { rd, ref mem, flags } | Inst::SLoad16 { rd, ref mem, flags } => { - check_load(&ctx, Some(rd.to_reg()), flags, mem, vcode, I16) + check_load(ctx, Some(rd.to_reg()), flags, mem, vcode, I16) } Inst::ULoad32 { rd, ref mem, flags } | Inst::SLoad32 { rd, ref mem, flags } => { - check_load(&ctx, Some(rd.to_reg()), flags, mem, vcode, I32) + check_load(ctx, Some(rd.to_reg()), flags, mem, vcode, I32) } Inst::ULoad64 { rd, ref mem, flags } => { - check_load(&ctx, Some(rd.to_reg()), flags, mem, vcode, I64) + check_load(ctx, Some(rd.to_reg()), flags, mem, vcode, I64) } - Inst::FpuLoad32 { ref mem, flags, .. } => check_load(&ctx, None, flags, mem, vcode, F32), - Inst::FpuLoad64 { ref mem, flags, .. } => check_load(&ctx, None, flags, mem, vcode, F64), - Inst::FpuLoad128 { ref mem, flags, .. } => check_load(&ctx, None, flags, mem, vcode, I8X16), - Inst::LoadP64 { ref mem, flags, .. } => check_load_pair(&ctx, flags, mem, vcode, 16), - Inst::FpuLoadP64 { ref mem, flags, .. } => check_load_pair(&ctx, flags, mem, vcode, 16), - Inst::FpuLoadP128 { ref mem, flags, .. } => check_load_pair(&ctx, flags, mem, vcode, 32), + Inst::FpuLoad32 { ref mem, flags, .. } => check_load(ctx, None, flags, mem, vcode, F32), + Inst::FpuLoad64 { ref mem, flags, .. } => check_load(ctx, None, flags, mem, vcode, F64), + Inst::FpuLoad128 { ref mem, flags, .. } => check_load(ctx, None, flags, mem, vcode, I8X16), + Inst::LoadP64 { ref mem, flags, .. } => check_load_pair(ctx, flags, mem, vcode, 16), + Inst::FpuLoadP64 { ref mem, flags, .. } => check_load_pair(ctx, flags, mem, vcode, 16), + Inst::FpuLoadP128 { ref mem, flags, .. } => check_load_pair(ctx, flags, mem, vcode, 32), Inst::VecLoadReplicate { rn, flags, size, .. - } => check_load_addr(&ctx, flags, rn, vcode, size.lane_size().ty()), + } => check_load_addr(ctx, flags, rn, vcode, size.lane_size().ty()), Inst::LoadAcquire { access_ty, rn, flags, .. - } => check_load_addr(&ctx, flags, rn, vcode, access_ty), - - Inst::Store8 { rd, ref mem, flags } => check_store(&ctx, Some(rd), flags, mem, vcode, I8), - Inst::Store16 { rd, ref mem, flags } => check_store(&ctx, Some(rd), flags, mem, vcode, I16), - Inst::Store32 { rd, ref mem, flags } => check_store(&ctx, Some(rd), flags, mem, vcode, I32), - Inst::Store64 { rd, ref mem, flags } => check_store(&ctx, Some(rd), flags, mem, vcode, I64), - Inst::FpuStore32 { ref mem, flags, .. } => check_store(&ctx, None, flags, mem, vcode, F32), - Inst::FpuStore64 { ref mem, flags, .. } => check_store(&ctx, None, flags, mem, vcode, F64), + } => check_load_addr(ctx, flags, rn, vcode, access_ty), + + Inst::Store8 { rd, ref mem, flags } => check_store(ctx, Some(rd), flags, mem, vcode, I8), + Inst::Store16 { rd, ref mem, flags } => check_store(ctx, Some(rd), flags, mem, vcode, I16), + Inst::Store32 { rd, ref mem, flags } => check_store(ctx, Some(rd), flags, mem, vcode, I32), + Inst::Store64 { rd, ref mem, flags } => check_store(ctx, Some(rd), flags, mem, vcode, I64), + Inst::FpuStore32 { ref mem, flags, .. } => check_store(ctx, None, flags, mem, vcode, F32), + Inst::FpuStore64 { ref mem, flags, .. } => check_store(ctx, None, flags, mem, vcode, F64), Inst::FpuStore128 { ref mem, flags, .. } => { - check_store(&ctx, None, flags, mem, vcode, I8X16) + check_store(ctx, None, flags, mem, vcode, I8X16) } - Inst::StoreP64 { ref mem, flags, .. } => check_store_pair(&ctx, flags, mem, vcode, 16), - Inst::FpuStoreP64 { ref mem, flags, .. } => check_store_pair(&ctx, flags, mem, vcode, 16), - Inst::FpuStoreP128 { ref mem, flags, .. } => check_store_pair(&ctx, flags, mem, vcode, 32), + Inst::StoreP64 { ref mem, flags, .. } => check_store_pair(ctx, flags, mem, vcode, 16), + Inst::FpuStoreP64 { ref mem, flags, .. } => check_store_pair(ctx, flags, mem, vcode, 16), + Inst::FpuStoreP128 { ref mem, flags, .. } => check_store_pair(ctx, flags, mem, vcode, 32), Inst::StoreRelease { access_ty, rn, flags, .. - } => check_store_addr(&ctx, flags, rn, vcode, access_ty), + } => check_store_addr(ctx, flags, rn, vcode, access_ty), Inst::AluRRR { alu_op: ALUOp::Add, @@ -197,8 +93,13 @@ pub(crate) fn check( rd, rn, rm, - } => check_binop(&ctx, vcode, rd.to_reg(), rn, rm, |rn, rm| { - fail_if_missing(ctx.add(rn, rm, size.bits().into())) + } => check_binop(ctx, vcode, 64, rd, rn, rm, |rn, rm| { + clamp_range( + ctx, + 64, + size.bits().into(), + ctx.add(rn, rm, size.bits().into()), + ) }), Inst::AluRRImm12 { alu_op: ALUOp::Add, @@ -206,9 +107,29 @@ pub(crate) fn check( rd, rn, imm12, - } => check_unop(&ctx, vcode, rd.to_reg(), rn, |rn| { - let imm_fact = Fact::constant(size.bits().into(), imm12.value()); - fail_if_missing(ctx.add(&rn, &imm_fact, size.bits().into())) + } => check_unop(ctx, vcode, 64, rd, rn, |rn| { + let imm12: i64 = imm12.value().into(); + clamp_range( + ctx, + 64, + size.bits().into(), + ctx.offset(&rn, size.bits().into(), imm12), + ) + }), + Inst::AluRRImm12 { + alu_op: ALUOp::Sub, + size, + rd, + rn, + imm12, + } => check_unop(ctx, vcode, 64, rd, rn, |rn| { + let imm12: i64 = imm12.value().into(); + clamp_range( + ctx, + 64, + size.bits().into(), + ctx.offset(&rn, size.bits().into(), -imm12), + ) }), Inst::AluRRRShift { alu_op: ALUOp::Add, @@ -218,13 +139,18 @@ pub(crate) fn check( rm, shiftop, } if shiftop.op() == ShiftOp::LSL && has_fact(vcode, rn) && has_fact(vcode, rm) => { - check_binop(&ctx, vcode, rd.to_reg(), rn, rm, |rn, rm| { + check_binop(ctx, vcode, 64, rd, rn, rm, |rn, rm| { let rm_shifted = fail_if_missing(ctx.shl( &rm, size.bits().into(), shiftop.amt().value().into(), ))?; - fail_if_missing(ctx.add(&rn, &rm_shifted, size.bits().into())) + clamp_range( + ctx, + 64, + size.bits().into(), + ctx.add(&rn, &rm_shifted, size.bits().into()), + ) }) } Inst::AluRRRExtend { @@ -235,9 +161,14 @@ pub(crate) fn check( rm, extendop, } if has_fact(vcode, rn) && has_fact(vcode, rm) => { - check_binop(&ctx, vcode, rd.to_reg(), rn, rm, |rn, rm| { - let rm_extended = fail_if_missing(extend_fact(&ctx, rm, extendop))?; - fail_if_missing(ctx.add(&rn, &rm_extended, size.bits().into())) + check_binop(ctx, vcode, 64, rd, rn, rm, |rn, rm| { + let rm_extended = fail_if_missing(extend_fact(ctx, rm, extendop))?; + clamp_range( + ctx, + 64, + size.bits().into(), + ctx.add(&rn, &rm_extended, size.bits().into()), + ) }) } Inst::AluRRImmShift { @@ -246,8 +177,13 @@ pub(crate) fn check( rd, rn, immshift, - } if has_fact(vcode, rn) => check_unop(&ctx, vcode, rd.to_reg(), rn, |rn| { - fail_if_missing(ctx.shl(&rn, size.bits().into(), immshift.value().into())) + } if has_fact(vcode, rn) => check_unop(ctx, vcode, 64, rd, rn, |rn| { + clamp_range( + ctx, + 64, + size.bits().into(), + ctx.shl(&rn, size.bits().into(), immshift.value().into()), + ) }), Inst::Extend { rd, @@ -255,21 +191,32 @@ pub(crate) fn check( signed: false, from_bits, to_bits, - } if has_fact(vcode, rn) => check_unop(&ctx, vcode, rd.to_reg(), rn, |rn| { - fail_if_missing(ctx.uextend(&rn, from_bits.into(), to_bits.into())) + } if has_fact(vcode, rn) => check_unop(ctx, vcode, 64, rd, rn, |rn| { + clamp_range( + ctx, + 64, + to_bits.into(), + ctx.uextend(&rn, from_bits.into(), to_bits.into()), + ) }), - Inst::AluRRR { size, rd, .. } + + Inst::AluRRR { rd, size, .. } | Inst::AluRRImm12 { rd, size, .. } | Inst::AluRRRShift { rd, size, .. } | Inst::AluRRRExtend { rd, size, .. } | Inst::AluRRImmLogic { rd, size, .. } - | Inst::AluRRImmShift { rd, size, .. } => { - // Any ALU op can validate a max-value fact where the - // value is the maximum for its bit-width. - check_output(&ctx, vcode, rd.to_reg(), &[], |_vcode| { - Ok(Fact::max_range_for_width(size.bits().into())) - }) - } + | Inst::AluRRImmShift { rd, size, .. } => check_output(ctx, vcode, rd, &[], |_vcode| { + clamp_range(ctx, 64, size.bits().into(), None) + }), + + Inst::Extend { + rd, + from_bits, + to_bits, + .. + } => check_output(ctx, vcode, rd, &[], |_vcode| { + clamp_range(ctx, to_bits.into(), from_bits.into(), None) + }), Inst::MovWide { op: MoveWideOp::MovZ, @@ -278,7 +225,7 @@ pub(crate) fn check( rd, } => { let constant = u64::from(imm.bits) << (imm.shift * 16); - check_constant(&ctx, vcode, rd.to_reg(), 64, constant) + check_constant(ctx, vcode, rd, 64, constant) } Inst::MovWide { @@ -288,20 +235,20 @@ pub(crate) fn check( rd, } => { let constant = !(u64::from(imm.bits) << (imm.shift * 16)) & size.max_value(); - check_constant(&ctx, vcode, rd.to_reg(), 64, constant) + check_constant(ctx, vcode, rd, 64, constant) } Inst::MovK { rd, rn, imm, .. } => { - let input = get_fact_or_default(vcode, rn)?; + let input = get_fact_or_default(vcode, rn, 64); trace!("MovK: input = {:?}", input); if let Some(input_constant) = input.as_const(64) { trace!(" -> input_constant: {}", input_constant); let constant = u64::from(imm.bits) << (imm.shift * 16); let constant = input_constant | constant; trace!(" -> merged constant: {}", constant); - check_constant(&ctx, vcode, rd.to_reg(), 64, constant) + check_constant(ctx, vcode, rd, 64, constant) } else { - check_output(ctx, vcode, rd.to_reg(), &[], |_vcode| { + check_output(ctx, vcode, rd, &[], |_vcode| { Ok(Fact::max_range_for_width(64)) }) } @@ -313,18 +260,6 @@ pub(crate) fn check( } } -/// The operation we're checking against an amode: either -/// -/// - a *load*, and we need to validate that the field's fact subsumes -/// the load result's fact, OR -/// -/// - a *store*, and we need to validate that the stored data's fact -/// subsumes the field's fact. -enum LoadOrStore<'a> { - Load { result_fact: Option<&'a Fact> }, - Store { stored_fact: Option<&'a Fact> }, -} - fn check_load( ctx: &FactContext, rd: Option, @@ -334,13 +269,18 @@ fn check_load( ty: Type, ) -> PccResult<()> { let result_fact = rd.and_then(|rd| vcode.vreg_fact(rd.into())); + let bits = u16::try_from(ty.bits()).unwrap(); check_addr( ctx, flags, addr, vcode, ty, - LoadOrStore::Load { result_fact }, + LoadOrStore::Load { + result_fact, + from_bits: bits, + to_bits: bits, + }, ) } @@ -379,12 +319,17 @@ fn check_addr<'a>( let check = |addr: &Fact, ty: Type| -> PccResult<()> { match op { - LoadOrStore::Load { result_fact } => { - let loaded_fact = ctx.load(addr, ty)?; + LoadOrStore::Load { + result_fact, + from_bits, + to_bits, + } => { + let loaded_fact = + clamp_range(ctx, to_bits, from_bits, ctx.load(addr, ty)?.cloned())?; trace!( "checking a load: loaded_fact = {loaded_fact:?} result_fact = {result_fact:?}" ); - if ctx.subsumes_fact_optionals(loaded_fact, result_fact) { + if ctx.subsumes_fact_optionals(Some(&loaded_fact), result_fact) { Ok(()) } else { Err(PccError::UnsupportedFact) @@ -396,15 +341,15 @@ fn check_addr<'a>( match addr { &AMode::RegReg { rn, rm } => { - let rn = get_fact_or_default(vcode, rn)?; - let rm = get_fact_or_default(vcode, rm)?; + let rn = get_fact_or_default(vcode, rn, 64); + let rm = get_fact_or_default(vcode, rm, 64); let sum = fail_if_missing(ctx.add(&rn, &rm, 64))?; trace!("rn = {rn:?} rm = {rm:?} sum = {sum:?}"); check(&sum, ty) } &AMode::RegScaled { rn, rm, ty } => { - let rn = get_fact_or_default(vcode, rn)?; - let rm = get_fact_or_default(vcode, rm)?; + let rn = get_fact_or_default(vcode, rn, 64); + let rm = get_fact_or_default(vcode, rm, 64); let rm_scaled = fail_if_missing(ctx.scale(&rm, 64, ty.bytes()))?; let sum = fail_if_missing(ctx.add(&rn, &rm_scaled, 64))?; check(&sum, ty) @@ -415,28 +360,28 @@ fn check_addr<'a>( ty, extendop, } => { - let rn = get_fact_or_default(vcode, rn)?; - let rm = get_fact_or_default(vcode, rm)?; - let rm_extended = fail_if_missing(extend_fact(ctx, rm, extendop))?; + let rn = get_fact_or_default(vcode, rn, 64); + let rm = get_fact_or_default(vcode, rm, 64); + let rm_extended = fail_if_missing(extend_fact(ctx, &rm, extendop))?; let rm_scaled = fail_if_missing(ctx.scale(&rm_extended, 64, ty.bytes()))?; let sum = fail_if_missing(ctx.add(&rn, &rm_scaled, 64))?; check(&sum, ty) } &AMode::RegExtended { rn, rm, extendop } => { - let rn = get_fact_or_default(vcode, rn)?; - let rm = get_fact_or_default(vcode, rm)?; - let rm_extended = fail_if_missing(extend_fact(ctx, rm, extendop))?; + let rn = get_fact_or_default(vcode, rn, 64); + let rm = get_fact_or_default(vcode, rm, 64); + let rm_extended = fail_if_missing(extend_fact(ctx, &rm, extendop))?; let sum = fail_if_missing(ctx.add(&rn, &rm_extended, 64))?; check(&sum, ty)?; Ok(()) } &AMode::Unscaled { rn, simm9 } => { - let rn = get_fact_or_default(vcode, rn)?; + let rn = get_fact_or_default(vcode, rn, 64); let sum = fail_if_missing(ctx.offset(&rn, 64, simm9.value.into()))?; check(&sum, ty) } &AMode::UnsignedOffset { rn, uimm12 } => { - let rn = get_fact_or_default(vcode, rn)?; + let rn = get_fact_or_default(vcode, rn, 64); // N.B.: the architecture scales the immediate in the // encoded instruction by the size of the loaded type, so // e.g. an offset field of 4095 can mean a load of offset @@ -456,7 +401,7 @@ fn check_addr<'a>( Ok(()) } &AMode::RegOffset { rn, off, .. } => { - let rn = get_fact_or_default(vcode, rn)?; + let rn = get_fact_or_default(vcode, rn, 64); let sum = fail_if_missing(ctx.offset(&rn, 64, off))?; check(&sum, ty) } @@ -502,8 +447,8 @@ fn check_load_addr( if !flags.checked() { return Ok(()); } - let fact = get_fact_or_default(vcode, reg)?; - let _output_fact = ctx.load(fact, ty)?; + let fact = get_fact_or_default(vcode, reg, 64); + let _output_fact = ctx.load(&fact, ty)?; Ok(()) } @@ -517,7 +462,7 @@ fn check_store_addr( if !flags.checked() { return Ok(()); } - let fact = get_fact_or_default(vcode, reg)?; - let _output_fact = ctx.store(fact, ty, None)?; + let fact = get_fact_or_default(vcode, reg, 64); + let _output_fact = ctx.store(&fact, ty, None)?; Ok(()) } diff --git a/cranelift/codegen/src/isa/x64/inst.isle b/cranelift/codegen/src/isa/x64/inst.isle index e5cb338a40cf..e55867ab45ab 100644 --- a/cranelift/codegen/src/isa/x64/inst.isle +++ b/cranelift/codegen/src/isa/x64/inst.isle @@ -2163,7 +2163,7 @@ ;; even not generate a zero-extended move in this case. (rule 1 (extend_to_gpr src @ (value_type $I32) $I64 (ExtendKind.Zero)) (if-let $true (value32_zeros_upper32 src)) - src) + (add_range_fact src 64 0 0xffff_ffff)) (rule (extend_to_gpr (and val (value_type from_ty)) to_ty diff --git a/cranelift/codegen/src/isa/x64/inst/args.rs b/cranelift/codegen/src/isa/x64/inst/args.rs index a9a393be1693..da74f0ca15ca 100644 --- a/cranelift/codegen/src/isa/x64/inst/args.rs +++ b/cranelift/codegen/src/isa/x64/inst/args.rs @@ -3,6 +3,7 @@ use super::regs::{self}; use super::EmitState; use crate::ir::condcodes::{FloatCC, IntCC}; +use crate::ir::types::*; use crate::ir::{MemFlags, Type}; use crate::isa::x64::inst::regs::pretty_print_reg; use crate::isa::x64::inst::Inst; @@ -107,7 +108,7 @@ macro_rules! newtype_of_reg { $( /// A newtype wrapper around `RegMem` for general-purpose registers. #[derive(Clone, Debug)] - pub struct $newtype_reg_mem(RegMem); + pub struct $newtype_reg_mem(pub RegMem); impl From<$newtype_reg_mem> for RegMem { fn from(rm: $newtype_reg_mem) -> Self { @@ -168,7 +169,7 @@ macro_rules! newtype_of_reg { $( /// A newtype wrapper around `RegMemImm`. #[derive(Clone, Debug)] - pub struct $newtype_reg_mem_imm(RegMemImm); + pub struct $newtype_reg_mem_imm(pub RegMemImm); impl From<$newtype_reg_mem_imm> for RegMemImm { fn from(rmi: $newtype_reg_mem_imm) -> RegMemImm { @@ -232,7 +233,7 @@ macro_rules! newtype_of_reg { /// A newtype wrapper around `Imm8Reg`. #[derive(Clone, Debug)] #[allow(dead_code)] // Used by some newtypes and not others. - pub struct $newtype_imm8_reg(Imm8Reg); + pub struct $newtype_imm8_reg(pub Imm8Reg); impl From<$newtype_reg> for $newtype_imm8_reg { fn from(r: $newtype_reg) -> Self { @@ -1930,6 +1931,15 @@ impl ExtMode { ExtMode::BQ | ExtMode::WQ | ExtMode::LQ => 8, } } + + /// Source size, as an integer type. + pub(crate) fn src_type(&self) -> Type { + match self { + ExtMode::BL | ExtMode::BQ => I8, + ExtMode::WL | ExtMode::WQ => I16, + ExtMode::LQ => I32, + } + } } impl fmt::Debug for ExtMode { @@ -2227,6 +2237,15 @@ impl OperandSize { pub(crate) fn to_bits(&self) -> u8 { self.to_bytes() * 8 } + + pub(crate) fn to_type(&self) -> Type { + match self { + Self::Size8 => I8, + Self::Size16 => I16, + Self::Size32 => I32, + Self::Size64 => I64, + } + } } /// An x64 memory fence kind. diff --git a/cranelift/codegen/src/isa/x64/lower.rs b/cranelift/codegen/src/isa/x64/lower.rs index c76a57f7c3fc..6b071c48ce34 100644 --- a/cranelift/codegen/src/isa/x64/lower.rs +++ b/cranelift/codegen/src/isa/x64/lower.rs @@ -3,10 +3,12 @@ // ISLE integration glue. pub(super) mod isle; +use crate::ir::pcc::{FactContext, PccResult}; use crate::ir::{types, ExternalName, Inst as IRInst, LibCall, Opcode, Type}; use crate::isa::x64::abi::*; use crate::isa::x64::inst::args::*; use crate::isa::x64::inst::*; +use crate::isa::x64::pcc; use crate::isa::{x64::X64Backend, CallConv}; use crate::machinst::abi::SmallInstVec; use crate::machinst::lower::*; @@ -337,4 +339,13 @@ impl LowerBackend for X64Backend { fn maybe_pinned_reg(&self) -> Option { Some(regs::pinned_reg()) } + + fn check_fact( + &self, + ctx: &FactContext<'_>, + vcode: &mut VCode, + inst: InsnIndex, + ) -> PccResult<()> { + pcc::check(ctx, vcode, inst) + } } diff --git a/cranelift/codegen/src/isa/x64/mod.rs b/cranelift/codegen/src/isa/x64/mod.rs index 75fbe2980abb..38a9c668c9d3 100644 --- a/cranelift/codegen/src/isa/x64/mod.rs +++ b/cranelift/codegen/src/isa/x64/mod.rs @@ -24,6 +24,7 @@ mod abi; pub mod encoding; mod inst; mod lower; +mod pcc; pub mod settings; /// An X64 backend. diff --git a/cranelift/codegen/src/isa/x64/pcc.rs b/cranelift/codegen/src/isa/x64/pcc.rs new file mode 100644 index 000000000000..ca39788290c4 --- /dev/null +++ b/cranelift/codegen/src/isa/x64/pcc.rs @@ -0,0 +1,600 @@ +//! Proof-carrying-code validation for x64 VCode. + +use crate::ir::pcc::*; +use crate::ir::types::*; +use crate::ir::Type; +use crate::isa::x64::inst::args::{ + AluRmiROpcode, Amode, GprMem, GprMemImm, Imm8Gpr, Imm8Reg, RegMem, RegMemImm, ShiftKind, + SyntheticAmode, ToWritableReg, XmmMem, XmmMemAligned, XmmMemAlignedImm, XmmMemImm, +}; +use crate::isa::x64::inst::Inst; +use crate::machinst::pcc::*; +use crate::machinst::{InsnIndex, VCode}; +use crate::machinst::{Reg, Writable}; +use crate::trace; + +pub(crate) fn check( + ctx: &FactContext, + vcode: &mut VCode, + inst_idx: InsnIndex, +) -> PccResult<()> { + trace!("Checking facts on inst: {:?}", vcode[inst_idx]); + + match vcode[inst_idx] { + Inst::Args { .. } => { + // Defs on the args have "axiomatic facts": we trust the + // ABI code to pass through the values unharmed, so the + // facts given to us in the CLIF should still be true. + Ok(()) + } + + Inst::AluRmiR { + size, + op: AluRmiROpcode::Add, + src1, + src2: GprMemImm(RegMemImm::Reg { reg: src2 }), + dst, + } => { + let bits = size.to_bits().into(); + check_binop( + ctx, + vcode, + 64, + dst.to_writable_reg(), + src1.to_reg(), + src2, + |src1, src2| clamp_range(ctx, 64, bits, ctx.add(src1, src2, bits)), + ) + } + + Inst::AluRmiR { + size, + op: AluRmiROpcode::Add, + src1, + src2: GprMemImm(RegMemImm::Imm { simm32 }), + dst, + } => { + let bits = size.to_bits().into(); + check_unop( + ctx, + vcode, + 64, + dst.to_writable_reg(), + src1.to_reg(), + |src1| { + let simm32: i64 = simm32.into(); + clamp_range(ctx, 64, bits, ctx.offset(src1, bits, simm32)) + }, + ) + } + Inst::AluRmiR { + size, + op: AluRmiROpcode::Add, + src1, + src2: GprMemImm(RegMemImm::Mem { ref addr }), + dst, + } => { + let bits: u16 = size.to_bits().into(); + let loaded = check_load(ctx, None, addr, vcode, size.to_type(), bits)?; + check_unop(ctx, vcode, 64, dst.to_writable_reg(), src1.into(), |src1| { + let sum = loaded.and_then(|loaded| ctx.add(src1, &loaded, bits)); + clamp_range(ctx, 64, bits, sum) + }) + } + + Inst::AluRmiR { + size, + op: AluRmiROpcode::Sub, + src1, + src2: GprMemImm(RegMemImm::Imm { simm32 }), + dst, + } => { + let bits = size.to_bits().into(); + check_unop( + ctx, + vcode, + 64, + dst.to_writable_reg(), + src1.to_reg(), + |src1| { + let simm32: i64 = simm32.into(); + clamp_range(ctx, 64, bits, ctx.offset(src1, bits, -simm32)) + }, + ) + } + + Inst::AluRmiR { + size, + src2: GprMemImm(RegMemImm::Mem { ref addr }), + dst, + .. + } + | Inst::AluRmRVex { + size, + src2: GprMem(RegMem::Mem { ref addr }), + dst, + .. + } => { + let loaded = check_load(ctx, None, addr, vcode, size.to_type(), 64)?; + check_output(ctx, vcode, dst.to_writable_reg(), &[], |_vcode| { + clamp_range(ctx, 64, size.to_bits().into(), loaded) + }) + } + Inst::CmpRmiR { + size, + src: GprMemImm(RegMemImm::Mem { ref addr }), + .. + } => { + check_load(ctx, None, addr, vcode, size.to_type(), 64)?; + Ok(()) + } + + Inst::Cmove { + size, + consequent: GprMem(RegMem::Mem { ref addr, .. }), + .. + } => { + check_load(ctx, None, addr, vcode, size.to_type(), 64)?; + Ok(()) + } + + Inst::Imm { simm64, dst, .. } => { + check_output(ctx, vcode, dst.to_writable_reg(), &[], |_vcode| { + Ok(Fact::constant(64, simm64)) + }) + } + + Inst::AluConstOp { + op: AluRmiROpcode::Xor, + dst, + .. + } => check_output(ctx, vcode, dst.to_writable_reg(), &[], |_vcode| { + Ok(Fact::constant(64, 0)) + }), + + Inst::AluRmiR { + size, + dst, + src2: GprMemImm(RegMemImm::Reg { .. } | RegMemImm::Imm { .. }), + .. + } + | Inst::AluConstOp { size, dst, .. } + | Inst::MovRR { size, dst, .. } + | Inst::AluRmRVex { + size, + src2: GprMem(RegMem::Reg { .. }), + dst, + .. + } => { + let bits: u16 = size.to_bits().into(); + trace!("generic case: bits = {}", bits); + check_output(ctx, vcode, dst.to_writable_reg(), &[], |_vcode| { + clamp_range(ctx, 64, bits, None) + }) + } + + Inst::MovzxRmR { + ref ext_mode, + src: GprMem(RegMem::Reg { reg: src }), + dst, + } => { + let from_bytes: u16 = ext_mode.src_size().into(); + let to_bytes: u16 = ext_mode.dst_size().into(); + check_unop(ctx, vcode, 64, dst.to_writable_reg(), src, |src| { + let extended = ctx.uextend(src, from_bytes * 8, to_bytes * 8); + trace!("src = {:?} extended = {:?}", src, extended); + clamp_range(ctx, 64, from_bytes * 8, extended) + }) + } + + Inst::LoadEffectiveAddress { + ref addr, + dst, + size, + } => { + let addr = addr.clone(); + let bits: u16 = size.to_bits().into(); + check_output(ctx, vcode, dst.to_writable_reg(), &[], |vcode| { + trace!("checking output: addr = {:?}", addr); + let clamped = clamp_range(ctx, 64, bits, compute_addr(ctx, vcode, &addr, bits)); + trace!("clamped = {:?}", clamped); + clamped + }) + } + + Inst::MovRM { size, src, ref dst } => { + check_store(ctx, Some(src.to_reg()), dst, vcode, size.to_type()) + } + Inst::MovImmM { + size, + simm32: _, + ref dst, + } => check_store(ctx, None, dst, vcode, size.to_type()), + Inst::Mov64MR { ref src, dst } => { + check_load(ctx, Some(dst.to_writable_reg()), src, vcode, I64, 64)?; + Ok(()) + } + Inst::MovzxRmR { + ref ext_mode, + src: GprMem(RegMem::Mem { ref addr }), + dst, + } => { + check_load( + ctx, + Some(dst.to_writable_reg()), + addr, + vcode, + ext_mode.src_type(), + 64, + )?; + Ok(()) + } + + Inst::AluRM { + size, + op: _, + ref src1_dst, + src2: _, + } => { + check_load(ctx, None, src1_dst, vcode, size.to_type(), 64)?; + check_store(ctx, None, src1_dst, vcode, size.to_type()) + } + + Inst::UnaryRmR { + size, + src: GprMem(RegMem::Mem { ref addr }), + dst, + .. + } + | Inst::UnaryRmRVex { + size, + src: GprMem(RegMem::Mem { ref addr }), + dst, + .. + } + | Inst::UnaryRmRImmVex { + size, + src: GprMem(RegMem::Mem { ref addr }), + dst, + .. + } => { + check_load(ctx, None, addr, vcode, size.to_type(), 64)?; + check_output(ctx, vcode, dst.to_writable_reg(), &[], |_vcode| { + clamp_range(ctx, 64, size.to_bits().into(), None) + }) + } + + Inst::Div { + divisor: GprMem(RegMem::Mem { ref addr }), + .. + } + | Inst::Div8 { + divisor: GprMem(RegMem::Mem { ref addr }), + .. + } + | Inst::MulHi { + src2: GprMem(RegMem::Mem { ref addr }), + .. + } + | Inst::UMulLo { + src2: GprMem(RegMem::Mem { ref addr }), + .. + } + | Inst::MovsxRmR { + src: GprMem(RegMem::Mem { ref addr }), + .. + } => { + // Round up on size: some of the above will take 32- or + // 8-bit mem args, but if we can validate assuming a + // 64-bit load, we're still (conservatively) safe. + check_load(ctx, None, addr, vcode, I64, 64)?; + Ok(()) + } + + Inst::ShiftR { + size, + kind: ShiftKind::ShiftLeft, + src, + num_bits: Imm8Gpr(Imm8Reg::Imm8 { imm }), + dst, + } => check_unop(ctx, vcode, 64, dst.to_writable_reg(), src.to_reg(), |src| { + clamp_range( + ctx, + 64, + size.to_bits().into(), + ctx.shl(src, size.to_bits().into(), imm.into()), + ) + }), + + Inst::ShiftR { size, dst, .. } => { + check_output(ctx, vcode, dst.to_writable_reg(), &[], |_vcode| { + clamp_range(ctx, 64, size.to_bits().into(), None) + }) + } + + Inst::Push64 { + src: GprMemImm(RegMemImm::Mem { ref addr }), + } => { + check_load(ctx, None, addr, vcode, I64, 64)?; + Ok(()) + } + + Inst::XmmMovRMVex { ref dst, .. } + | Inst::XmmMovRMImmVex { ref dst, .. } + | Inst::XmmMovRM { ref dst, .. } + | Inst::XmmMovRMImm { ref dst, .. } => { + check_store(ctx, None, dst, vcode, I8X16)?; + Ok(()) + } + + Inst::XmmRmiReg { + src2: XmmMemAlignedImm(RegMemImm::Mem { ref addr }), + .. + } + | Inst::XmmRmiRVex { + src2: XmmMemImm(RegMemImm::Mem { ref addr }), + .. + } + | Inst::XmmRmRImmVex { + src2: XmmMem(RegMem::Mem { ref addr }), + .. + } + | Inst::XmmRmR { + src2: XmmMemAligned(RegMem::Mem { ref addr }), + .. + } + | Inst::XmmRmRUnaligned { + src2: XmmMem(RegMem::Mem { ref addr }), + .. + } + | Inst::XmmRmRBlend { + src2: XmmMemAligned(RegMem::Mem { ref addr }), + .. + } + | Inst::XmmRmRVex3 { + src3: XmmMem(RegMem::Mem { ref addr }), + .. + } + | Inst::XmmRmRBlendVex { + src2: XmmMem(RegMem::Mem { ref addr }), + .. + } + | Inst::XmmUnaryRmR { + src: XmmMemAligned(RegMem::Mem { ref addr }), + .. + } + | Inst::XmmUnaryRmRUnaligned { + src: XmmMem(RegMem::Mem { ref addr }), + .. + } + | Inst::XmmUnaryRmRImm { + src: XmmMemAligned(RegMem::Mem { ref addr }), + .. + } + | Inst::XmmUnaryRmREvex { + src: XmmMem(RegMem::Mem { ref addr }), + .. + } + | Inst::XmmUnaryRmRVex { + src: XmmMem(RegMem::Mem { ref addr }), + .. + } + | Inst::XmmUnaryRmRImmVex { + src: XmmMem(RegMem::Mem { ref addr }), + .. + } + | Inst::XmmRmREvex { + src2: XmmMem(RegMem::Mem { ref addr }), + .. + } + | Inst::XmmUnaryRmRImmEvex { + src: XmmMem(RegMem::Mem { ref addr }), + .. + } + | Inst::XmmRmREvex3 { + src3: XmmMem(RegMem::Mem { ref addr }), + .. + } => { + check_load(ctx, None, addr, vcode, I8X16, 128)?; + Ok(()) + } + + Inst::XmmVexPinsr { + src2: GprMem(RegMem::Mem { ref addr }), + .. + } + | Inst::GprToXmm { + src: GprMem(RegMem::Mem { ref addr }), + .. + } + | Inst::GprToXmmVex { + src: GprMem(RegMem::Mem { ref addr }), + .. + } => { + check_load(ctx, None, addr, vcode, I64, 64)?; + Ok(()) + } + + Inst::XmmCmove { + consequent: XmmMemAligned(RegMem::Mem { ref addr }), + .. + } => { + check_load(ctx, None, addr, vcode, I8X16, 128)?; + Ok(()) + } + + Inst::XmmCmpRmR { + src: XmmMemAligned(RegMem::Mem { ref addr }), + .. + } + | Inst::XmmRmRImm { + src2: RegMem::Mem { ref addr }, + .. + } => { + check_load(ctx, None, addr, vcode, I8X16, 128)?; + Ok(()) + } + + Inst::CvtIntToFloat { + src2: GprMem(RegMem::Mem { ref addr }), + .. + } + | Inst::CvtIntToFloatVex { + src2: GprMem(RegMem::Mem { ref addr }), + .. + } => { + check_load(ctx, None, addr, vcode, I64, 64)?; + Ok(()) + } + + Inst::LockCmpxchg { mem: ref dst, .. } | Inst::AtomicRmwSeq { mem: ref dst, .. } => { + check_store(ctx, None, dst, vcode, I64)?; + Ok(()) + } + + Inst::CallUnknown { + dest: RegMem::Mem { ref addr }, + .. + } + | Inst::ReturnCallUnknown { + callee: RegMem::Mem { ref addr }, + .. + } + | Inst::JmpUnknown { + target: RegMem::Mem { ref addr }, + .. + } => { + check_load(ctx, None, addr, vcode, I64, 64)?; + Ok(()) + } + + _ if vcode.inst_defines_facts(inst_idx) => { + trace!( + "Unsupported inst during PCC validation: {:?}", + vcode[inst_idx] + ); + Err(PccError::UnsupportedFact) + } + + _ => Ok(()), + } +} + +fn check_load( + ctx: &FactContext, + dst: Option>, + src: &SyntheticAmode, + vcode: &VCode, + ty: Type, + to_bits: u16, +) -> PccResult> { + let result_fact = dst.and_then(|dst| vcode.vreg_fact(dst.to_reg().into())); + let from_bits = u16::try_from(ty.bits()).unwrap(); + check_mem( + ctx, + src, + vcode, + ty, + LoadOrStore::Load { + result_fact, + from_bits, + to_bits, + }, + ) +} + +fn check_store( + ctx: &FactContext, + data: Option, + dst: &SyntheticAmode, + vcode: &VCode, + ty: Type, +) -> PccResult<()> { + let stored_fact = data.and_then(|data| vcode.vreg_fact(data.into())); + check_mem(ctx, dst, vcode, ty, LoadOrStore::Store { stored_fact }).map(|_| ()) +} + +fn check_mem<'a>( + ctx: &FactContext, + amode: &SyntheticAmode, + vcode: &VCode, + ty: Type, + op: LoadOrStore<'a>, +) -> PccResult> { + match amode { + SyntheticAmode::Real(amode) if !amode.get_flags().checked() => return Ok(None), + SyntheticAmode::NominalSPOffset { .. } | SyntheticAmode::ConstantOffset(_) => { + return Ok(None) + } + _ => {} + } + + let addr = compute_addr(ctx, vcode, amode, 64).ok_or(PccError::MissingFact)?; + + match op { + LoadOrStore::Load { + result_fact, + from_bits, + to_bits, + } => { + let loaded_fact = clamp_range(ctx, to_bits, from_bits, ctx.load(&addr, ty)?.cloned())?; + trace!( + "loaded_fact = {:?} result_fact = {:?}", + loaded_fact, + result_fact + ); + if ctx.subsumes_fact_optionals(Some(&loaded_fact), result_fact) { + Ok(Some(loaded_fact.clone())) + } else { + Err(PccError::UnsupportedFact) + } + } + LoadOrStore::Store { stored_fact } => { + ctx.store(&addr, ty, stored_fact)?; + Ok(None) + } + } +} + +fn compute_addr( + ctx: &FactContext, + vcode: &VCode, + amode: &SyntheticAmode, + bits: u16, +) -> Option { + trace!("compute_addr: {:?}", amode); + match *amode { + SyntheticAmode::Real(Amode::ImmReg { simm32, base, .. }) => { + let base = get_fact_or_default(vcode, base, bits); + trace!("base = {:?}", base); + let simm32: i64 = simm32.into(); + let simm32: u64 = simm32 as u64; + let offset = Fact::constant(bits, simm32); + let sum = ctx.add(&base, &offset, bits)?; + trace!("sum = {:?}", sum); + Some(sum) + } + SyntheticAmode::Real(Amode::ImmRegRegShift { + simm32, + base, + index, + shift, + .. + }) => { + let base = get_fact_or_default(vcode, base.into(), bits); + let index = get_fact_or_default(vcode, index.into(), bits); + trace!("base = {:?} index = {:?}", base, index); + let shifted = ctx.shl(&index, bits, shift.into())?; + let sum = ctx.add(&base, &shifted, bits)?; + let simm32: i64 = simm32.into(); + let simm32: u64 = simm32 as u64; + let offset = Fact::constant(bits, simm32); + let sum = ctx.add(&sum, &offset, bits)?; + trace!("sum = {:?}", sum); + Some(sum) + } + SyntheticAmode::Real(Amode::RipRelative { .. }) + | SyntheticAmode::ConstantOffset(_) + | SyntheticAmode::NominalSPOffset { .. } => None, + } +} diff --git a/cranelift/codegen/src/legalizer/globalvalue.rs b/cranelift/codegen/src/legalizer/globalvalue.rs index 4ab5a4ad236d..9837dd59278e 100644 --- a/cranelift/codegen/src/legalizer/globalvalue.rs +++ b/cranelift/codegen/src/legalizer/globalvalue.rs @@ -4,7 +4,7 @@ //! instruction into code that depends on the kind of global value referenced. use crate::cursor::{Cursor, FuncCursor}; -use crate::ir::{self, InstBuilder}; +use crate::ir::{self, pcc::Fact, InstBuilder}; use crate::isa::TargetIsa; /// Expand a `global_value` instruction according to the definition of the global value. @@ -90,8 +90,17 @@ fn iadd_imm_addr( pos.func.dfg.facts[lhs] = Some(fact.clone()); } + // Generate the constant and attach a fact to the constant if + // there is a fact on the base. + let constant = pos.ins().iconst(global_type, offset); + if pos.func.global_value_facts[base].is_some() { + let bits = u16::try_from(global_type.bits()).unwrap(); + let unsigned_offset = offset as u64; // Safety: reinterpret i64 bits as u64. + pos.func.dfg.facts[constant] = Some(Fact::constant(bits, unsigned_offset)); + } + // Simply replace the `global_value` instruction with an `iadd_imm`, reusing the result value. - pos.func.dfg.replace(inst).iadd_imm(lhs, offset); + pos.func.dfg.replace(inst).iadd(lhs, constant); } /// Expand a `global_value` instruction for a load global. diff --git a/cranelift/codegen/src/machinst/compile.rs b/cranelift/codegen/src/machinst/compile.rs index 00b37821df8a..09f193dc525c 100644 --- a/cranelift/codegen/src/machinst/compile.rs +++ b/cranelift/codegen/src/machinst/compile.rs @@ -26,7 +26,8 @@ pub fn compile( let block_order = BlockLoweringOrder::new(f, domtree, ctrl_plane); // Build the lowering context. - let lower = crate::machinst::Lower::new(f, abi, emit_info, block_order, sigs)?; + let lower = + crate::machinst::Lower::new(f, abi, emit_info, block_order, sigs, b.flags().clone())?; // Lower the IR. let mut vcode = { diff --git a/cranelift/codegen/src/machinst/isle.rs b/cranelift/codegen/src/machinst/isle.rs index f14d1045185e..1fb41f9c7325 100644 --- a/cranelift/codegen/src/machinst/isle.rs +++ b/cranelift/codegen/src/machinst/isle.rs @@ -683,6 +683,11 @@ macro_rules! isle_lower_prelude_methods { fn jump_table_size(&mut self, targets: &BoxVecMachLabel) -> u32 { targets.len() as u32 } + + fn add_range_fact(&mut self, reg: Reg, bits: u16, min: u64, max: u64) -> Reg { + self.lower_ctx.add_range_fact(reg, bits, min, max); + reg + } }; } diff --git a/cranelift/codegen/src/machinst/lower.rs b/cranelift/codegen/src/machinst/lower.rs index e3f591787ee3..bed1a3989a5e 100644 --- a/cranelift/codegen/src/machinst/lower.rs +++ b/cranelift/codegen/src/machinst/lower.rs @@ -8,7 +8,7 @@ use crate::entity::SecondaryMap; use crate::fx::{FxHashMap, FxHashSet}; use crate::inst_predicates::{has_lowering_side_effect, is_constant_64bit}; -use crate::ir::pcc::{FactContext, PccError, PccResult}; +use crate::ir::pcc::{Fact, FactContext, PccError, PccResult}; use crate::ir::{ ArgumentPurpose, Block, Constant, ConstantData, DataFlowGraph, ExternalName, Function, GlobalValue, GlobalValueData, Immediate, Inst, InstructionData, MemFlags, RelSourceLoc, Type, @@ -19,6 +19,7 @@ use crate::machinst::{ MachLabel, Reg, SigSet, VCode, VCodeBuilder, VCodeConstant, VCodeConstantData, VCodeConstants, VCodeInst, ValueRegs, Writable, }; +use crate::settings::Flags; use crate::{trace, CodegenResult}; use alloc::vec::Vec; use cranelift_control::ControlPlane; @@ -219,6 +220,9 @@ pub struct Lower<'func, I: VCodeInst> { /// The register to use for GetPinnedReg, if any, on this architecture. pinned_reg: Option, + + /// Compilation flags. + flags: Flags, } /// How is a value used in the IR? @@ -342,6 +346,7 @@ impl<'func, I: VCodeInst> Lower<'func, I> { emit_info: I::Info, block_order: BlockLoweringOrder, sigs: SigSet, + flags: Flags, ) -> CodegenResult { let constants = VCodeConstants::with_capacity(f.dfg.constants.len()); let vcode = VCodeBuilder::new( @@ -456,6 +461,7 @@ impl<'func, I: VCodeInst> Lower<'func, I> { cur_inst: None, ir_insts: vec![], pinned_reg: None, + flags, }) } @@ -1416,4 +1422,18 @@ impl<'func, I: VCodeInst> Lower<'func, I> { trace!("set vreg alias: from {:?} to {:?}", from, to); self.vcode.set_vreg_alias(from, to); } + + /// Add a range fact to a register, if no other fact is present. + pub fn add_range_fact(&mut self, reg: Reg, bit_width: u16, min: u64, max: u64) { + if self.flags.enable_pcc() { + self.vregs.set_fact_if_missing( + reg.to_virtual_reg().unwrap(), + Fact::Range { + bit_width, + min, + max, + }, + ); + } + } } diff --git a/cranelift/codegen/src/machinst/mod.rs b/cranelift/codegen/src/machinst/mod.rs index c587d64adfb6..2d8acd2947c5 100644 --- a/cranelift/codegen/src/machinst/mod.rs +++ b/cranelift/codegen/src/machinst/mod.rs @@ -85,6 +85,7 @@ pub use inst_common::*; pub mod valueregs; pub use reg::*; pub use valueregs::*; +pub mod pcc; pub mod reg; /// A machine instruction. diff --git a/cranelift/codegen/src/machinst/pcc.rs b/cranelift/codegen/src/machinst/pcc.rs new file mode 100644 index 000000000000..db1bf3256533 --- /dev/null +++ b/cranelift/codegen/src/machinst/pcc.rs @@ -0,0 +1,160 @@ +//! Common helpers for ISA-specific proof-carrying-code implementations. + +use crate::ir::pcc::{Fact, FactContext, PccError, PccResult}; +use crate::machinst::{Reg, VCode, VCodeInst, Writable}; +use crate::trace; + +pub(crate) fn get_fact_or_default(vcode: &VCode, reg: Reg, width: u16) -> Fact { + trace!( + "get_fact_or_default: reg v{} -> {:?}", + reg.to_virtual_reg().unwrap().index(), + vcode.vreg_fact(reg.into()) + ); + vcode + .vreg_fact(reg.into()) + .map(|f| f.clone()) + .unwrap_or_else(|| Fact::max_range_for_width(width)) +} + +pub(crate) fn has_fact(vcode: &VCode, reg: Reg) -> bool { + vcode.vreg_fact(reg.into()).is_some() +} + +pub(crate) fn fail_if_missing(fact: Option) -> PccResult { + fact.ok_or(PccError::UnsupportedFact) +} + +pub(crate) fn clamp_range( + ctx: &FactContext, + to_bits: u16, + from_bits: u16, + fact: Option, +) -> PccResult { + let max = if from_bits == 64 { + u64::MAX + } else { + (1u64 << from_bits) - 1 + }; + trace!( + "clamp_range: fact {:?} from {} to {}", + fact, + from_bits, + to_bits + ); + Ok(fact + .and_then(|f| ctx.uextend(&f, from_bits, to_bits)) + .unwrap_or_else(|| { + let result = Fact::Range { + bit_width: to_bits, + min: 0, + max, + }; + trace!(" -> clamping to {:?}", result); + result + })) +} + +pub(crate) fn check_subsumes(ctx: &FactContext, subsumer: &Fact, subsumee: &Fact) -> PccResult<()> { + trace!( + "checking if derived fact {:?} subsumes stated fact {:?}", + subsumer, + subsumee + ); + + if ctx.subsumes(subsumer, subsumee) { + Ok(()) + } else { + Err(PccError::UnsupportedFact) + } +} + +pub(crate) fn check_output) -> PccResult>( + ctx: &FactContext, + vcode: &mut VCode, + out: Writable, + ins: &[Reg], + f: F, +) -> PccResult<()> { + if let Some(fact) = vcode.vreg_fact(out.to_reg().into()) { + let result = f(vcode)?; + check_subsumes(ctx, &result, fact) + } else if ins.iter().any(|r| { + vcode + .vreg_fact(r.into()) + .map(|fact| fact.propagates()) + .unwrap_or(false) + }) { + if let Ok(fact) = f(vcode) { + trace!("setting vreg {:?} to {:?}", out, fact); + vcode.set_vreg_fact(out.to_reg().into(), fact); + } + Ok(()) + } else { + Ok(()) + } +} + +pub(crate) fn check_unop PccResult>( + ctx: &FactContext, + vcode: &mut VCode, + reg_width: u16, + out: Writable, + ra: Reg, + f: F, +) -> PccResult<()> { + check_output(ctx, vcode, out, &[ra], |vcode| { + let ra = get_fact_or_default(vcode, ra, reg_width); + f(&ra) + }) +} + +pub(crate) fn check_binop PccResult>( + ctx: &FactContext, + vcode: &mut VCode, + reg_width: u16, + out: Writable, + ra: Reg, + rb: Reg, + f: F, +) -> PccResult<()> { + check_output(ctx, vcode, out, &[ra, rb], |vcode| { + let ra = get_fact_or_default(vcode, ra, reg_width); + let rb = get_fact_or_default(vcode, rb, reg_width); + f(&ra, &rb) + }) +} + +pub(crate) fn check_constant( + ctx: &FactContext, + vcode: &mut VCode, + out: Writable, + bit_width: u16, + value: u64, +) -> PccResult<()> { + let result = Fact::constant(bit_width, value); + if let Some(fact) = vcode.vreg_fact(out.to_reg().into()) { + check_subsumes(ctx, &result, fact) + } else { + trace!("setting vreg {:?} to {:?}", out, result); + vcode.set_vreg_fact(out.to_reg().into(), result); + Ok(()) + } +} + +/// The operation we're checking against an amode: either +/// +/// - a *load*, and we need to validate that the field's fact subsumes +/// the load result's fact, OR +/// +/// - a *store*, and we need to validate that the stored data's fact +/// subsumes the field's fact. +pub(crate) enum LoadOrStore<'a> { + Load { + result_fact: Option<&'a Fact>, + from_bits: u16, + to_bits: u16, + }, + Store { + stored_fact: Option<&'a Fact>, + }, +} diff --git a/cranelift/codegen/src/machinst/vcode.rs b/cranelift/codegen/src/machinst/vcode.rs index 3e76f394f116..c743fcd20fef 100644 --- a/cranelift/codegen/src/machinst/vcode.rs +++ b/cranelift/codegen/src/machinst/vcode.rs @@ -1301,6 +1301,7 @@ impl VCode { /// Set the fact for a given VReg. pub fn set_vreg_fact(&mut self, vreg: VReg, fact: Fact) { let vreg = self.resolve_vreg_alias(vreg); + trace!("set fact on {}: {:?}", vreg, fact); self.facts[vreg.vreg()] = Some(fact); } @@ -1625,6 +1626,13 @@ impl VRegAllocator { self.facts[vreg.index()].take() } + /// Set a fact only if one doesn't already exist. + pub fn set_fact_if_missing(&mut self, vreg: VirtualReg, fact: Fact) { + if self.facts[vreg.index()].is_none() { + self.set_fact(vreg, fact); + } + } + /// Allocate a fresh ValueRegs, with a given fact to apply if /// the value fits in one VReg. pub fn alloc_with_maybe_fact( diff --git a/cranelift/codegen/src/prelude_lower.isle b/cranelift/codegen/src/prelude_lower.isle index 99aab0a4ea6e..d90286e88a6f 100644 --- a/cranelift/codegen/src/prelude_lower.isle +++ b/cranelift/codegen/src/prelude_lower.isle @@ -178,6 +178,11 @@ (rule (multi_reg_to_single (MultiReg.One a)) (value_reg a)) +;; Add a range fact to a register, when compiling with +;; proof-carrying-code enabled. +(decl add_range_fact (Reg u16 u64 u64) Reg) +(extern constructor add_range_fact add_range_fact) + ;;;; Common Mach Types ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (type MachLabel (primitive MachLabel)) diff --git a/cranelift/filetests/filetests/pcc/fail/add.clif b/cranelift/filetests/filetests/pcc/fail/add.clif index a40879f3df72..afcf9765015a 100644 --- a/cranelift/filetests/filetests/pcc/fail/add.clif +++ b/cranelift/filetests/filetests/pcc/fail/add.clif @@ -1,6 +1,7 @@ test compile expect-fail set enable_pcc=true target aarch64 +target x86_64 function %f0(i32, i32) -> i32 { block0(v0 ! range(32, 0, 0x100): i32, v1 ! range(32, 0, 0x80): i32): diff --git a/cranelift/filetests/filetests/pcc/fail/blockparams.clif b/cranelift/filetests/filetests/pcc/fail/blockparams.clif index 0e8386f838c8..3840f9932322 100644 --- a/cranelift/filetests/filetests/pcc/fail/blockparams.clif +++ b/cranelift/filetests/filetests/pcc/fail/blockparams.clif @@ -1,6 +1,7 @@ test compile expect-fail set enable_pcc=true target aarch64 +target x86_64 function %f0(i64, i32) -> i64 { block0(v0 ! range(64, 0, 0x100): i64, v1: i32): diff --git a/cranelift/filetests/filetests/pcc/fail/extend.clif b/cranelift/filetests/filetests/pcc/fail/extend.clif index a1c31ee5fb5a..43b830d7a998 100644 --- a/cranelift/filetests/filetests/pcc/fail/extend.clif +++ b/cranelift/filetests/filetests/pcc/fail/extend.clif @@ -1,20 +1,10 @@ test compile expect-fail set enable_pcc=true target aarch64 +target x86_64 function %f0(i32) -> i64 { block0(v0 ! range(32, 0, 0xffff_ffff): i32): v1 ! range(64, 0, 0xffff_0000) = uextend.i64 v0 return v1 } - -;; This one is actually true, but we don't support the reasoning it -;; would need: one needs to assume the "don't-care" bits in the upper -;; half of the i32 are the worst case (all ones), so `0xffff_ffff` is -;; possible. If the `i32` were taken through another 32-bit operation -;; and we asserted its 32-bit range at that point, it would work. -function %f1(i32) -> i64 { -block0(v0 ! range(16, 0, 0xffff): i32): - v1 ! range(64, 0, 0xffff_ffff) = uextend.i64 v0 - return v1 -} diff --git a/cranelift/filetests/filetests/pcc/fail/load.clif b/cranelift/filetests/filetests/pcc/fail/load.clif index 5fa42311d2ce..c4c086b1f19b 100644 --- a/cranelift/filetests/filetests/pcc/fail/load.clif +++ b/cranelift/filetests/filetests/pcc/fail/load.clif @@ -1,6 +1,7 @@ test compile expect-fail set enable_pcc=true target aarch64 +target x86_64 function %f0(i64, i32) -> i64 { mt0 = memory 0x1000 diff --git a/cranelift/filetests/filetests/pcc/fail/memtypes.clif b/cranelift/filetests/filetests/pcc/fail/memtypes.clif index bf407779299a..6f5b12a38307 100644 --- a/cranelift/filetests/filetests/pcc/fail/memtypes.clif +++ b/cranelift/filetests/filetests/pcc/fail/memtypes.clif @@ -1,6 +1,7 @@ test verifier set enable_pcc=true target aarch64 +target x86_64 function %f0(i64) -> i32 { mt0 = struct 8 { 4: i32, 0: i32 } ; error: out-of-order diff --git a/cranelift/filetests/filetests/pcc/fail/shift.clif b/cranelift/filetests/filetests/pcc/fail/shift.clif index 4059c3eba35d..b4f4929b3bc2 100644 --- a/cranelift/filetests/filetests/pcc/fail/shift.clif +++ b/cranelift/filetests/filetests/pcc/fail/shift.clif @@ -1,6 +1,7 @@ test compile expect-fail set enable_pcc=true target aarch64 +target x86_64 function %f0(i32) -> i32 { block0(v0 ! range(32, 1, 0x100): i32): diff --git a/cranelift/filetests/filetests/pcc/fail/simple.clif b/cranelift/filetests/filetests/pcc/fail/simple.clif index 777451212d41..45b2e9ab665d 100644 --- a/cranelift/filetests/filetests/pcc/fail/simple.clif +++ b/cranelift/filetests/filetests/pcc/fail/simple.clif @@ -1,6 +1,7 @@ test compile expect-fail set enable_pcc=true target aarch64 +target x86_64 ;; The `memory` memtype is not large enough here -- the 4GiB-range ;; 32-bit offset could go out of range. PCC should catch this. diff --git a/cranelift/filetests/filetests/pcc/fail/struct.clif b/cranelift/filetests/filetests/pcc/fail/struct.clif index c8d79dcdb558..88d905e32e3a 100644 --- a/cranelift/filetests/filetests/pcc/fail/struct.clif +++ b/cranelift/filetests/filetests/pcc/fail/struct.clif @@ -1,6 +1,7 @@ test compile expect-fail set enable_pcc=true target aarch64 +target x86_64 function %f0(i64) -> i64 { mt0 = struct 8 { 0: i64 ! mem(mt1, 0, 0) } diff --git a/cranelift/filetests/filetests/pcc/fail/vmctx.clif b/cranelift/filetests/filetests/pcc/fail/vmctx.clif index 9f53602f7867..1dc947bf21fb 100644 --- a/cranelift/filetests/filetests/pcc/fail/vmctx.clif +++ b/cranelift/filetests/filetests/pcc/fail/vmctx.clif @@ -1,6 +1,7 @@ test compile expect-fail set enable_pcc=true target aarch64 +target x86_64 ;; Equivalent to a Wasm `i64.load` from a static memory. function %f0(i64, i32) -> i64 { diff --git a/cranelift/filetests/filetests/pcc/succeed/add.clif b/cranelift/filetests/filetests/pcc/succeed/add.clif index 6cfcb486f618..2e2416ce642d 100644 --- a/cranelift/filetests/filetests/pcc/succeed/add.clif +++ b/cranelift/filetests/filetests/pcc/succeed/add.clif @@ -1,6 +1,7 @@ test compile set enable_pcc=true target aarch64 +target x86_64 function %f0(i32, i32) -> i32 { block0(v0 ! range(32, 0, 0x100): i32, v1 ! range(32, 0, 0x80): i32): diff --git a/cranelift/filetests/filetests/pcc/succeed/blockparams.clif b/cranelift/filetests/filetests/pcc/succeed/blockparams.clif index 914edf12a17f..7a823e1bf27d 100644 --- a/cranelift/filetests/filetests/pcc/succeed/blockparams.clif +++ b/cranelift/filetests/filetests/pcc/succeed/blockparams.clif @@ -1,6 +1,7 @@ test compile set enable_pcc=true target aarch64 +target x86_64 function %f0(i64, i32) -> i64 { block0(v0 ! range(64, 0, 0x100): i64, v1: i32): diff --git a/cranelift/filetests/filetests/pcc/succeed/extend.clif b/cranelift/filetests/filetests/pcc/succeed/extend.clif index 690e3d0acd90..2926aa65bd7d 100644 --- a/cranelift/filetests/filetests/pcc/succeed/extend.clif +++ b/cranelift/filetests/filetests/pcc/succeed/extend.clif @@ -1,6 +1,7 @@ test compile set enable_pcc=true target aarch64 +target x86_64 function %f0(i32) -> i64 { block0(v0 ! range(32, 42, 0xffff_fffe): i32): @@ -8,3 +9,9 @@ block0(v0 ! range(32, 42, 0xffff_fffe): i32): v1 ! range(64, 1, 0xffff_ffff) = uextend.i64 v0 return v1 } + +function %f1(i32) -> i64 { +block0(v0 ! range(16, 0, 0xffff): i32): + v1 ! range(64, 0, 0xffff_ffff) = uextend.i64 v0 + return v1 +} diff --git a/cranelift/filetests/filetests/pcc/succeed/gv_fact.clif b/cranelift/filetests/filetests/pcc/succeed/gv_fact.clif index bc2f39c930b7..329b8d444edc 100644 --- a/cranelift/filetests/filetests/pcc/succeed/gv_fact.clif +++ b/cranelift/filetests/filetests/pcc/succeed/gv_fact.clif @@ -1,6 +1,7 @@ test compile set enable_pcc=true target aarch64 +target x86_64 function %f0(i64 vmctx) -> i64 { mt0 = struct 16 { 8: i64 ! mem(mt1, 0, 0) } @@ -26,7 +27,7 @@ block0(v0 ! mem(mt0, 0, 0): i64): return v1 } -function %f1(i64 vmctx) -> i64 { +function %f2(i64 vmctx) -> i64 { mt0 = struct 16 { 8: i64 ! mem(mt1, 0, 0) } mt1 = struct 8 { 0: i64 ! mem(mt2, 0, 0) } mt2 = memory 0x1_0000_0000 @@ -40,7 +41,7 @@ block0(v0 ! mem(mt0, 0, 0): i64): return v1 } -function %f2(i64 vmctx) -> i64 { +function %f3(i64 vmctx) -> i64 { mt0 = struct 16 { 8: i64 ! mem(mt1, 0, 0) } mt1 = struct 8 { 0: i64 ! mem(mt2, 0, 0) } mt2 = memory 0x1_0000_0000 diff --git a/cranelift/filetests/filetests/pcc/succeed/load.clif b/cranelift/filetests/filetests/pcc/succeed/load.clif index 90b8e030dd38..59996ef05fae 100644 --- a/cranelift/filetests/filetests/pcc/succeed/load.clif +++ b/cranelift/filetests/filetests/pcc/succeed/load.clif @@ -1,6 +1,7 @@ test compile set enable_pcc=true target aarch64 +target x86_64 function %f0(i64, i32) -> i64 { mt0 = memory 0x1_0000_0000 @@ -25,8 +26,8 @@ block0(v0 ! mem(mt0, 0, 0): i64, v1 ! range(32, 0, 0xffff_ffff): i32): function %f2(i64, i32) -> i8 { mt0 = memory 0x1000 block0(v0 ! mem(mt0, 0, 0): i64, v1 ! range(32, 0, 0xfff): i32): - v2 ! range(64, 0, 0x100) = uextend.i64 v1 - v3 ! mem(mt0, 0, 0x100) = iadd.i64 v0, v2 + v2 ! range(64, 0, 0xfff) = uextend.i64 v1 + v3 ! mem(mt0, 0, 0xfff) = iadd.i64 v0, v2 v4 = load.i8 checked v3 return v4 } diff --git a/cranelift/filetests/filetests/pcc/succeed/memtypes.clif b/cranelift/filetests/filetests/pcc/succeed/memtypes.clif index ea35dcdbf462..dc4805c90cc5 100644 --- a/cranelift/filetests/filetests/pcc/succeed/memtypes.clif +++ b/cranelift/filetests/filetests/pcc/succeed/memtypes.clif @@ -1,6 +1,7 @@ test compile set enable_pcc=true target aarch64 +target x86_64 function %f0(i64) -> i32 { mt0 = struct 8 { 0: i32, 4: i32 readonly } diff --git a/cranelift/filetests/filetests/pcc/succeed/opt.clif b/cranelift/filetests/filetests/pcc/succeed/opt.clif index 5e996aed2d04..8fda923223c4 100644 --- a/cranelift/filetests/filetests/pcc/succeed/opt.clif +++ b/cranelift/filetests/filetests/pcc/succeed/opt.clif @@ -2,6 +2,7 @@ test compile set enable_pcc=true set opt_level=speed target aarch64 +target x86_64 ;; Equivalent to a Wasm `i64.load` from a static memory, but with some ;; redundant stuff that should be optimized away (x+0 -> x). diff --git a/cranelift/filetests/filetests/pcc/succeed/shift.clif b/cranelift/filetests/filetests/pcc/succeed/shift.clif index 685d5749774a..0fcf627464ef 100644 --- a/cranelift/filetests/filetests/pcc/succeed/shift.clif +++ b/cranelift/filetests/filetests/pcc/succeed/shift.clif @@ -1,6 +1,7 @@ test compile set enable_pcc=true target aarch64 +target x86_64 function %f0(i32) -> i32 { block0(v0 ! range(32, 1, 0x100): i32): diff --git a/cranelift/filetests/filetests/pcc/succeed/simple.clif b/cranelift/filetests/filetests/pcc/succeed/simple.clif index 2baa3f9915e0..80dc769899ef 100644 --- a/cranelift/filetests/filetests/pcc/succeed/simple.clif +++ b/cranelift/filetests/filetests/pcc/succeed/simple.clif @@ -1,6 +1,7 @@ test compile set enable_pcc=true target aarch64 +target x86_64 function %simple1(i64 vmctx, i32) -> i8 { mt0 = memory 0x1_0000_0000 diff --git a/cranelift/filetests/filetests/pcc/succeed/struct.clif b/cranelift/filetests/filetests/pcc/succeed/struct.clif index 1d7c5688249a..f53bdaff54d4 100644 --- a/cranelift/filetests/filetests/pcc/succeed/struct.clif +++ b/cranelift/filetests/filetests/pcc/succeed/struct.clif @@ -1,6 +1,7 @@ test compile set enable_pcc=true target aarch64 +target x86_64 function %f0(i64) -> i64 { mt0 = struct 8 { 0: i64 ! mem(mt1, 0, 0) } diff --git a/cranelift/filetests/filetests/pcc/succeed/vmctx.clif b/cranelift/filetests/filetests/pcc/succeed/vmctx.clif index 643984fa79fc..ec05f22fa6c0 100644 --- a/cranelift/filetests/filetests/pcc/succeed/vmctx.clif +++ b/cranelift/filetests/filetests/pcc/succeed/vmctx.clif @@ -1,6 +1,7 @@ test compile set enable_pcc=true target aarch64 +target x86_64 ;; Equivalent to a Wasm `i64.load` from a static memory. function %f0(i64, i32) -> i64 { diff --git a/cranelift/wasm/src/code_translator/bounds_checks.rs b/cranelift/wasm/src/code_translator/bounds_checks.rs index 6e236165b031..ee0447cc6d83 100644 --- a/cranelift/wasm/src/code_translator/bounds_checks.rs +++ b/cranelift/wasm/src/code_translator/bounds_checks.rs @@ -53,6 +53,7 @@ where index, heap.index_type, env.pointer_type(), + heap.memory_type.is_some(), &mut builder.cursor(), ); let offset_and_size = offset_plus_size(offset, access_size); @@ -301,6 +302,7 @@ fn cast_index_to_pointer_ty( index: ir::Value, index_ty: ir::Type, pointer_ty: ir::Type, + pcc: bool, pos: &mut FuncCursor, ) -> ir::Value { if index_ty == pointer_ty { @@ -315,6 +317,14 @@ fn cast_index_to_pointer_ty( // Convert `index` to `addr_ty`. let extended_index = pos.ins().uextend(pointer_ty, index); + // Add a range fact on the extended value. + if pcc { + pos.func.dfg.facts[extended_index] = Some(Fact::max_range_for_width_extended( + u16::try_from(index_ty.bits()).unwrap(), + u16::try_from(pointer_ty.bits()).unwrap(), + )); + } + // Add debug value-label alias so that debuginfo can name the extended // value as the address let loc = pos.srcloc(); From c1ed20cf8ccdbcd08c07d70df0418d4033aec29a Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Tue, 24 Oct 2023 14:53:01 -0700 Subject: [PATCH 2/3] Don't run regalloc checker if not requested in addition to PCC; it's fairly expensive. --- cranelift/codegen/src/machinst/compile.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/cranelift/codegen/src/machinst/compile.rs b/cranelift/codegen/src/machinst/compile.rs index 09f193dc525c..4c42e61ca3c4 100644 --- a/cranelift/codegen/src/machinst/compile.rs +++ b/cranelift/codegen/src/machinst/compile.rs @@ -76,11 +76,8 @@ pub fn compile( .expect("register allocation") }; - // Run the regalloc checker, if requested. Also run the checker if - // PCC is enabled (PCC only validates up to pre-regalloc VCode, so - // for a full guarantee we need to ensure that regalloc did a - // faithful translation to allocated machine code.) - if b.flags().regalloc_checker() || b.flags().enable_pcc() { + // Run the regalloc checker, if requested. + if b.flags().regalloc_checker() { let _tt = timing::regalloc_checker(); let mut checker = regalloc2::checker::Checker::new(&vcode, vcode.machine_env()); checker.prepare(®alloc_result); From a0cfee21f39e88242f5a700b9b95921aa6476763 Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Wed, 25 Oct 2023 18:34:30 -0700 Subject: [PATCH 3/3] Refactor x64 PCC code to avoid deep pattern matches on Gpr/Xmm types; explicitly match every instruction kind. --- cranelift/codegen/src/ir/pcc.rs | 5 +- cranelift/codegen/src/isa/x64/inst/args.rs | 16 +- cranelift/codegen/src/isa/x64/pcc.rs | 836 +++++++++++++-------- 3 files changed, 543 insertions(+), 314 deletions(-) diff --git a/cranelift/codegen/src/ir/pcc.rs b/cranelift/codegen/src/ir/pcc.rs index 7a6cd840f5ab..fe7f08c96cdb 100644 --- a/cranelift/codegen/src/ir/pcc.rs +++ b/cranelift/codegen/src/ir/pcc.rs @@ -781,7 +781,10 @@ pub fn check_vcode_facts( let block = BlockIndex::new(block); for inst in vcode.block_insns(block).iter() { // Check any output facts on this inst. - backend.check_fact(&ctx, vcode, inst)?; + if let Err(e) = backend.check_fact(&ctx, vcode, inst) { + log::error!("Error checking instruction: {:?}", vcode[inst]); + return Err(e); + } // If this is a branch, check that all block arguments subsume // the assumed facts on the blockparams of successors. diff --git a/cranelift/codegen/src/isa/x64/inst/args.rs b/cranelift/codegen/src/isa/x64/inst/args.rs index da74f0ca15ca..d88f3a2ac16c 100644 --- a/cranelift/codegen/src/isa/x64/inst/args.rs +++ b/cranelift/codegen/src/isa/x64/inst/args.rs @@ -108,13 +108,18 @@ macro_rules! newtype_of_reg { $( /// A newtype wrapper around `RegMem` for general-purpose registers. #[derive(Clone, Debug)] - pub struct $newtype_reg_mem(pub RegMem); + pub struct $newtype_reg_mem(RegMem); impl From<$newtype_reg_mem> for RegMem { fn from(rm: $newtype_reg_mem) -> Self { rm.0 } } + impl<'a> From<&'a $newtype_reg_mem> for &'a RegMem { + fn from(rm: &'a $newtype_reg_mem) -> &'a RegMem { + &rm.0 + } + } impl From<$newtype_reg> for $newtype_reg_mem { fn from(r: $newtype_reg) -> Self { @@ -169,13 +174,18 @@ macro_rules! newtype_of_reg { $( /// A newtype wrapper around `RegMemImm`. #[derive(Clone, Debug)] - pub struct $newtype_reg_mem_imm(pub RegMemImm); + pub struct $newtype_reg_mem_imm(RegMemImm); impl From<$newtype_reg_mem_imm> for RegMemImm { fn from(rmi: $newtype_reg_mem_imm) -> RegMemImm { rmi.0 } } + impl<'a> From<&'a $newtype_reg_mem_imm> for &'a RegMemImm { + fn from(rmi: &'a $newtype_reg_mem_imm) -> &'a RegMemImm { + &rmi.0 + } + } impl From<$newtype_reg> for $newtype_reg_mem_imm { fn from(r: $newtype_reg) -> Self { @@ -233,7 +243,7 @@ macro_rules! newtype_of_reg { /// A newtype wrapper around `Imm8Reg`. #[derive(Clone, Debug)] #[allow(dead_code)] // Used by some newtypes and not others. - pub struct $newtype_imm8_reg(pub Imm8Reg); + pub struct $newtype_imm8_reg(Imm8Reg); impl From<$newtype_reg> for $newtype_imm8_reg { fn from(r: $newtype_reg) -> Self { diff --git a/cranelift/codegen/src/isa/x64/pcc.rs b/cranelift/codegen/src/isa/x64/pcc.rs index ca39788290c4..3991186c524a 100644 --- a/cranelift/codegen/src/isa/x64/pcc.rs +++ b/cranelift/codegen/src/isa/x64/pcc.rs @@ -4,8 +4,7 @@ use crate::ir::pcc::*; use crate::ir::types::*; use crate::ir::Type; use crate::isa::x64::inst::args::{ - AluRmiROpcode, Amode, GprMem, GprMemImm, Imm8Gpr, Imm8Reg, RegMem, RegMemImm, ShiftKind, - SyntheticAmode, ToWritableReg, XmmMem, XmmMemAligned, XmmMemAlignedImm, XmmMemImm, + AluRmiROpcode, Amode, Gpr, Imm8Reg, RegMem, RegMemImm, ShiftKind, SyntheticAmode, ToWritableReg, }; use crate::isa::x64::inst::Inst; use crate::machinst::pcc::*; @@ -13,6 +12,26 @@ use crate::machinst::{InsnIndex, VCode}; use crate::machinst::{Reg, Writable}; use crate::trace; +fn undefined_result( + ctx: &FactContext, + vcode: &mut VCode, + dst: Writable, + reg_bits: u16, + result_bits: u16, +) -> PccResult<()> { + check_output(ctx, vcode, dst.to_writable_reg(), &[], |_vcode| { + clamp_range(ctx, reg_bits, result_bits, None) + }) +} + +fn ensure_no_fact(vcode: &VCode, reg: Reg) -> PccResult<()> { + if vcode.vreg_fact(reg.into()).is_some() { + Err(PccError::UnsupportedFact) + } else { + Ok(()) + } +} + pub(crate) fn check( ctx: &FactContext, vcode: &mut VCode, @@ -21,6 +40,8 @@ pub(crate) fn check( trace!("Checking facts on inst: {:?}", vcode[inst_idx]); match vcode[inst_idx] { + Inst::Nop { .. } => Ok(()), + Inst::Args { .. } => { // Defs on the args have "axiomatic facts": we trust the // ABI code to pass through the values unharmed, so the @@ -32,159 +53,280 @@ pub(crate) fn check( size, op: AluRmiROpcode::Add, src1, - src2: GprMemImm(RegMemImm::Reg { reg: src2 }), + ref src2, dst, - } => { - let bits = size.to_bits().into(); - check_binop( - ctx, - vcode, - 64, - dst.to_writable_reg(), - src1.to_reg(), - src2, - |src1, src2| clamp_range(ctx, 64, bits, ctx.add(src1, src2, bits)), - ) - } + } => match *<&RegMemImm>::from(src2) { + RegMemImm::Reg { reg: src2 } => { + let bits = size.to_bits().into(); + check_binop( + ctx, + vcode, + 64, + dst.to_writable_reg(), + src1.to_reg(), + src2, + |src1, src2| clamp_range(ctx, 64, bits, ctx.add(src1, src2, bits)), + ) + } + RegMemImm::Imm { simm32 } => { + let bits = size.to_bits().into(); + check_unop( + ctx, + vcode, + 64, + dst.to_writable_reg(), + src1.to_reg(), + |src1| { + let simm32: i64 = simm32.into(); + clamp_range(ctx, 64, bits, ctx.offset(src1, bits, simm32)) + }, + ) + } + RegMemImm::Mem { ref addr } => { + let bits: u16 = size.to_bits().into(); + let loaded = check_load(ctx, None, addr, vcode, size.to_type(), bits)?; + check_unop(ctx, vcode, 64, dst.to_writable_reg(), src1.into(), |src1| { + let sum = loaded.and_then(|loaded| ctx.add(src1, &loaded, bits)); + clamp_range(ctx, 64, bits, sum) + }) + } + }, Inst::AluRmiR { size, - op: AluRmiROpcode::Add, + op: AluRmiROpcode::Sub, src1, - src2: GprMemImm(RegMemImm::Imm { simm32 }), + ref src2, dst, - } => { - let bits = size.to_bits().into(); - check_unop( - ctx, - vcode, - 64, - dst.to_writable_reg(), - src1.to_reg(), - |src1| { - let simm32: i64 = simm32.into(); - clamp_range(ctx, 64, bits, ctx.offset(src1, bits, simm32)) - }, - ) - } + } => match *<&RegMemImm>::from(src2) { + RegMemImm::Imm { simm32 } => { + let bits = size.to_bits().into(); + check_unop( + ctx, + vcode, + 64, + dst.to_writable_reg(), + src1.to_reg(), + |src1| { + let simm32: i64 = simm32.into(); + clamp_range(ctx, 64, bits, ctx.offset(src1, bits, -simm32)) + }, + ) + } + RegMemImm::Reg { .. } => { + let bits: u16 = size.to_bits().into(); + check_output(ctx, vcode, dst.to_writable_reg(), &[], |_vcode| { + clamp_range(ctx, 64, bits, None) + }) + } + RegMemImm::Mem { ref addr } => { + let loaded = check_load(ctx, None, addr, vcode, size.to_type(), 64)?; + check_output(ctx, vcode, dst.to_writable_reg(), &[], |_vcode| { + clamp_range(ctx, 64, size.to_bits().into(), loaded) + }) + } + }, + Inst::AluRmiR { size, - op: AluRmiROpcode::Add, - src1, - src2: GprMemImm(RegMemImm::Mem { ref addr }), + ref src2, dst, + .. + } => match <&RegMemImm>::from(src2) { + RegMemImm::Mem { ref addr } => { + let loaded = check_load(ctx, None, addr, vcode, size.to_type(), 64)?; + check_output(ctx, vcode, dst.to_writable_reg(), &[], |_vcode| { + clamp_range(ctx, 64, size.to_bits().into(), loaded) + }) + } + RegMemImm::Reg { .. } | RegMemImm::Imm { .. } => { + undefined_result(ctx, vcode, dst, 64, size.to_bits().into()) + } + }, + + Inst::AluRM { + size, + op: _, + ref src1_dst, + src2: _, } => { - let bits: u16 = size.to_bits().into(); - let loaded = check_load(ctx, None, addr, vcode, size.to_type(), bits)?; - check_unop(ctx, vcode, 64, dst.to_writable_reg(), src1.into(), |src1| { - let sum = loaded.and_then(|loaded| ctx.add(src1, &loaded, bits)); - clamp_range(ctx, 64, bits, sum) - }) + check_load(ctx, None, src1_dst, vcode, size.to_type(), 64)?; + check_store(ctx, None, src1_dst, vcode, size.to_type()) } - Inst::AluRmiR { + Inst::AluRmRVex { size, - op: AluRmiROpcode::Sub, - src1, - src2: GprMemImm(RegMemImm::Imm { simm32 }), + ref src2, dst, - } => { - let bits = size.to_bits().into(); - check_unop( - ctx, - vcode, - 64, - dst.to_writable_reg(), - src1.to_reg(), - |src1| { - let simm32: i64 = simm32.into(); - clamp_range(ctx, 64, bits, ctx.offset(src1, bits, -simm32)) - }, - ) + .. + } => match <&RegMem>::from(src2) { + RegMem::Mem { ref addr } => { + let loaded = check_load(ctx, None, addr, vcode, size.to_type(), 64)?; + check_output(ctx, vcode, dst.to_writable_reg(), &[], |_vcode| { + clamp_range(ctx, 64, size.to_bits().into(), loaded) + }) + } + RegMem::Reg { .. } => undefined_result(ctx, vcode, dst, 64, size.to_bits().into()), + }, + + Inst::AluConstOp { + op: AluRmiROpcode::Xor, + dst, + .. + } => check_output(ctx, vcode, dst.to_writable_reg(), &[], |_vcode| { + Ok(Fact::constant(64, 0)) + }), + + Inst::AluConstOp { dst, .. } => undefined_result(ctx, vcode, dst, 64, 64), + + Inst::UnaryRmR { + size, ref src, dst, .. } + | Inst::UnaryRmRVex { + size, ref src, dst, .. + } + | Inst::UnaryRmRImmVex { + size, ref src, dst, .. + } => match <&RegMem>::from(src) { + RegMem::Mem { ref addr } => { + check_load(ctx, None, addr, vcode, size.to_type(), 64)?; + check_output(ctx, vcode, dst.to_writable_reg(), &[], |_vcode| { + clamp_range(ctx, 64, size.to_bits().into(), None) + }) + } + RegMem::Reg { .. } => undefined_result(ctx, vcode, dst, 64, size.to_bits().into()), + }, - Inst::AluRmiR { + Inst::Not { size, dst, .. } | Inst::Neg { size, dst, .. } => { + undefined_result(ctx, vcode, dst, 64, size.to_bits().into()) + } + + Inst::Div { size, - src2: GprMemImm(RegMemImm::Mem { ref addr }), - dst, + ref divisor, + dst_quotient, + dst_remainder, .. + } => { + match <&RegMem>::from(divisor) { + RegMem::Mem { ref addr } => { + check_load(ctx, None, addr, vcode, size.to_type(), 64)?; + } + RegMem::Reg { .. } => {} + } + undefined_result(ctx, vcode, dst_quotient, 64, 64)?; + undefined_result(ctx, vcode, dst_remainder, 64, 64)?; + Ok(()) } - | Inst::AluRmRVex { + Inst::Div8 { + dst, ref divisor, .. + } => { + match <&RegMem>::from(divisor) { + RegMem::Mem { ref addr } => { + check_load(ctx, None, addr, vcode, I8, 64)?; + } + RegMem::Reg { .. } => {} + } + // 64-bit result width because result may be negative + // hence high bits set. + undefined_result(ctx, vcode, dst, 64, 64)?; + Ok(()) + } + Inst::MulHi { size, - src2: GprMem(RegMem::Mem { ref addr }), - dst, + dst_lo, + dst_hi, + ref src2, .. } => { - let loaded = check_load(ctx, None, addr, vcode, size.to_type(), 64)?; - check_output(ctx, vcode, dst.to_writable_reg(), &[], |_vcode| { - clamp_range(ctx, 64, size.to_bits().into(), loaded) - }) + match <&RegMem>::from(src2) { + RegMem::Mem { ref addr } => { + check_load(ctx, None, addr, vcode, size.to_type(), 64)?; + } + RegMem::Reg { .. } => {} + } + undefined_result(ctx, vcode, dst_lo, 64, 64)?; + undefined_result(ctx, vcode, dst_hi, 64, 64)?; + Ok(()) } - Inst::CmpRmiR { + Inst::UMulLo { size, - src: GprMemImm(RegMemImm::Mem { ref addr }), + dst, + ref src2, .. } => { - check_load(ctx, None, addr, vcode, size.to_type(), 64)?; + match <&RegMem>::from(src2) { + RegMem::Mem { ref addr } => { + check_load(ctx, None, addr, vcode, size.to_type(), 64)?; + } + RegMem::Reg { .. } => {} + } + undefined_result(ctx, vcode, dst, 64, 64)?; Ok(()) } - Inst::Cmove { - size, - consequent: GprMem(RegMem::Mem { ref addr, .. }), + Inst::CheckedSRemSeq { + dst_quotient, + dst_remainder, .. } => { - check_load(ctx, None, addr, vcode, size.to_type(), 64)?; + undefined_result(ctx, vcode, dst_quotient, 64, 64)?; + undefined_result(ctx, vcode, dst_remainder, 64, 64)?; Ok(()) } + Inst::CheckedSRemSeq8 { dst, .. } => undefined_result(ctx, vcode, dst, 64, 64), + + Inst::SignExtendData { dst, .. } => undefined_result(ctx, vcode, dst, 64, 64), + Inst::Imm { simm64, dst, .. } => { check_output(ctx, vcode, dst.to_writable_reg(), &[], |_vcode| { Ok(Fact::constant(64, simm64)) }) } - Inst::AluConstOp { - op: AluRmiROpcode::Xor, - dst, - .. - } => check_output(ctx, vcode, dst.to_writable_reg(), &[], |_vcode| { - Ok(Fact::constant(64, 0)) - }), - - Inst::AluRmiR { - size, - dst, - src2: GprMemImm(RegMemImm::Reg { .. } | RegMemImm::Imm { .. }), - .. - } - | Inst::AluConstOp { size, dst, .. } - | Inst::MovRR { size, dst, .. } - | Inst::AluRmRVex { - size, - src2: GprMem(RegMem::Reg { .. }), - dst, - .. - } => { - let bits: u16 = size.to_bits().into(); - trace!("generic case: bits = {}", bits); - check_output(ctx, vcode, dst.to_writable_reg(), &[], |_vcode| { - clamp_range(ctx, 64, bits, None) - }) + Inst::MovRR { size, dst, .. } => { + undefined_result(ctx, vcode, dst, 64, size.to_bits().into()) } + Inst::MovFromPReg { dst, .. } => undefined_result(ctx, vcode, dst, 64, 64), + Inst::MovToPReg { .. } => Ok(()), + Inst::MovzxRmR { ref ext_mode, - src: GprMem(RegMem::Reg { reg: src }), + ref src, dst, } => { let from_bytes: u16 = ext_mode.src_size().into(); let to_bytes: u16 = ext_mode.dst_size().into(); - check_unop(ctx, vcode, 64, dst.to_writable_reg(), src, |src| { - let extended = ctx.uextend(src, from_bytes * 8, to_bytes * 8); - trace!("src = {:?} extended = {:?}", src, extended); - clamp_range(ctx, 64, from_bytes * 8, extended) - }) + match <&RegMem>::from(src) { + RegMem::Reg { reg } => { + check_unop(ctx, vcode, 64, dst.to_writable_reg(), *reg, |src| { + let extended = ctx.uextend(src, from_bytes * 8, to_bytes * 8); + clamp_range(ctx, 64, from_bytes * 8, extended) + }) + } + RegMem::Mem { ref addr } => { + let loaded = check_load( + ctx, + Some(dst.to_writable_reg()), + addr, + vcode, + ext_mode.src_type(), + 64, + )?; + check_output(ctx, vcode, dst.to_writable_reg(), &[], |_vcode| { + let extended = loaded + .and_then(|loaded| ctx.uextend(&loaded, from_bytes * 8, to_bytes * 8)); + clamp_range(ctx, 64, from_bytes * 8, extended) + }) + } + } + } + + Inst::Mov64MR { ref src, dst } => { + check_load(ctx, Some(dst.to_writable_reg()), src, vcode, I64, 64)?; + Ok(()) } Inst::LoadEffectiveAddress { @@ -195,288 +337,362 @@ pub(crate) fn check( let addr = addr.clone(); let bits: u16 = size.to_bits().into(); check_output(ctx, vcode, dst.to_writable_reg(), &[], |vcode| { - trace!("checking output: addr = {:?}", addr); - let clamped = clamp_range(ctx, 64, bits, compute_addr(ctx, vcode, &addr, bits)); - trace!("clamped = {:?}", clamped); - clamped + clamp_range(ctx, 64, bits, compute_addr(ctx, vcode, &addr, bits)) }) } - Inst::MovRM { size, src, ref dst } => { - check_store(ctx, Some(src.to_reg()), dst, vcode, size.to_type()) - } - Inst::MovImmM { - size, - simm32: _, - ref dst, - } => check_store(ctx, None, dst, vcode, size.to_type()), - Inst::Mov64MR { ref src, dst } => { - check_load(ctx, Some(dst.to_writable_reg()), src, vcode, I64, 64)?; - Ok(()) - } - Inst::MovzxRmR { + Inst::MovsxRmR { ref ext_mode, - src: GprMem(RegMem::Mem { ref addr }), + ref src, dst, } => { - check_load( - ctx, - Some(dst.to_writable_reg()), - addr, - vcode, - ext_mode.src_type(), - 64, - )?; - Ok(()) - } - - Inst::AluRM { - size, - op: _, - ref src1_dst, - src2: _, - } => { - check_load(ctx, None, src1_dst, vcode, size.to_type(), 64)?; - check_store(ctx, None, src1_dst, vcode, size.to_type()) + match <&RegMem>::from(src) { + RegMem::Mem { ref addr } => { + check_load(ctx, None, addr, vcode, ext_mode.src_type(), 64)?; + } + RegMem::Reg { .. } => {} + } + undefined_result(ctx, vcode, dst, 64, 64) } - Inst::UnaryRmR { - size, - src: GprMem(RegMem::Mem { ref addr }), - dst, - .. - } - | Inst::UnaryRmRVex { - size, - src: GprMem(RegMem::Mem { ref addr }), - dst, - .. - } - | Inst::UnaryRmRImmVex { - size, - src: GprMem(RegMem::Mem { ref addr }), - dst, - .. - } => { - check_load(ctx, None, addr, vcode, size.to_type(), 64)?; - check_output(ctx, vcode, dst.to_writable_reg(), &[], |_vcode| { - clamp_range(ctx, 64, size.to_bits().into(), None) - }) - } + Inst::MovImmM { size, ref dst, .. } => check_store(ctx, None, dst, vcode, size.to_type()), - Inst::Div { - divisor: GprMem(RegMem::Mem { ref addr }), - .. - } - | Inst::Div8 { - divisor: GprMem(RegMem::Mem { ref addr }), - .. - } - | Inst::MulHi { - src2: GprMem(RegMem::Mem { ref addr }), - .. - } - | Inst::UMulLo { - src2: GprMem(RegMem::Mem { ref addr }), - .. - } - | Inst::MovsxRmR { - src: GprMem(RegMem::Mem { ref addr }), - .. - } => { - // Round up on size: some of the above will take 32- or - // 8-bit mem args, but if we can validate assuming a - // 64-bit load, we're still (conservatively) safe. - check_load(ctx, None, addr, vcode, I64, 64)?; - Ok(()) + Inst::MovRM { size, src, ref dst } => { + check_store(ctx, Some(src.to_reg()), dst, vcode, size.to_type()) } Inst::ShiftR { size, kind: ShiftKind::ShiftLeft, src, - num_bits: Imm8Gpr(Imm8Reg::Imm8 { imm }), + ref num_bits, dst, - } => check_unop(ctx, vcode, 64, dst.to_writable_reg(), src.to_reg(), |src| { - clamp_range( - ctx, - 64, - size.to_bits().into(), - ctx.shl(src, size.to_bits().into(), imm.into()), - ) - }), + } => match num_bits.clone().to_imm8_reg() { + Imm8Reg::Imm8 { imm } => { + check_unop(ctx, vcode, 64, dst.to_writable_reg(), src.to_reg(), |src| { + clamp_range( + ctx, + 64, + size.to_bits().into(), + ctx.shl(src, size.to_bits().into(), imm.into()), + ) + }) + } + Imm8Reg::Reg { .. } => undefined_result(ctx, vcode, dst, 64, 64), + }, Inst::ShiftR { size, dst, .. } => { - check_output(ctx, vcode, dst.to_writable_reg(), &[], |_vcode| { - clamp_range(ctx, 64, size.to_bits().into(), None) - }) + undefined_result(ctx, vcode, dst, 64, size.to_bits().into()) } - Inst::Push64 { - src: GprMemImm(RegMemImm::Mem { ref addr }), - } => { - check_load(ctx, None, addr, vcode, I64, 64)?; - Ok(()) + Inst::XmmRmiReg { dst, ref src2, .. } => { + match <&RegMemImm>::from(src2) { + RegMemImm::Mem { ref addr } => { + check_load(ctx, None, addr, vcode, I8X16, 128)?; + } + _ => {} + } + ensure_no_fact(vcode, dst.to_writable_reg().to_reg()) } - Inst::XmmMovRMVex { ref dst, .. } - | Inst::XmmMovRMImmVex { ref dst, .. } - | Inst::XmmMovRM { ref dst, .. } - | Inst::XmmMovRMImm { ref dst, .. } => { - check_store(ctx, None, dst, vcode, I8X16)?; - Ok(()) - } + Inst::CmpRmiR { size, ref src, .. } => match <&RegMemImm>::from(src) { + RegMemImm::Mem { ref addr } => { + check_load(ctx, None, addr, vcode, size.to_type(), 64)?; + Ok(()) + } + RegMemImm::Reg { .. } | RegMemImm::Imm { .. } => Ok(()), + }, + + Inst::Setcc { dst, .. } => undefined_result(ctx, vcode, dst, 64, 64), - Inst::XmmRmiReg { - src2: XmmMemAlignedImm(RegMemImm::Mem { ref addr }), + Inst::Bswap { dst, .. } => undefined_result(ctx, vcode, dst, 64, 64), + + Inst::Cmove { + size, + dst, + ref consequent, .. + } => { + match <&RegMem>::from(consequent) { + RegMem::Mem { ref addr } => { + check_load(ctx, None, addr, vcode, size.to_type(), 64)?; + } + RegMem::Reg { .. } => {} + } + undefined_result(ctx, vcode, dst, 64, 64) } - | Inst::XmmRmiRVex { - src2: XmmMemImm(RegMemImm::Mem { ref addr }), + + Inst::XmmCmove { + dst, + ref consequent, .. + } => { + match <&RegMem>::from(consequent) { + RegMem::Mem { ref addr } => { + check_load(ctx, None, addr, vcode, I8X16, 128)?; + } + RegMem::Reg { .. } => {} + } + ensure_no_fact(vcode, dst.to_writable_reg().to_reg()) } - | Inst::XmmRmRImmVex { - src2: XmmMem(RegMem::Mem { ref addr }), - .. + + Inst::Push64 { ref src } => match <&RegMemImm>::from(src) { + RegMemImm::Mem { ref addr } => { + check_load(ctx, None, addr, vcode, I64, 64)?; + Ok(()) + } + RegMemImm::Reg { .. } | RegMemImm::Imm { .. } => Ok(()), + }, + + Inst::Pop64 { dst } => undefined_result(ctx, vcode, dst, 64, 64), + + Inst::StackProbeLoop { tmp, .. } => ensure_no_fact(vcode, tmp.to_reg()), + + Inst::XmmRmR { dst, ref src2, .. } + | Inst::XmmRmRBlend { dst, ref src2, .. } + | Inst::XmmUnaryRmR { + dst, src: ref src2, .. } - | Inst::XmmRmR { - src2: XmmMemAligned(RegMem::Mem { ref addr }), - .. + | Inst::XmmUnaryRmRImm { + dst, src: ref src2, .. + } => { + match <&RegMem>::from(src2) { + RegMem::Mem { ref addr } => { + check_load(ctx, None, addr, vcode, I8X16, 128)?; + } + RegMem::Reg { .. } => {} + } + ensure_no_fact(vcode, dst.to_writable_reg().to_reg()) } - | Inst::XmmRmRUnaligned { - src2: XmmMem(RegMem::Mem { ref addr }), + + Inst::XmmRmRUnaligned { dst, ref src2, .. } + | Inst::XmmRmRImmVex { dst, ref src2, .. } + | Inst::XmmRmRVex3 { + dst, + src3: ref src2, .. } - | Inst::XmmRmRBlend { - src2: XmmMemAligned(RegMem::Mem { ref addr }), - .. + | Inst::XmmRmRBlendVex { dst, ref src2, .. } + | Inst::XmmUnaryRmRVex { + dst, src: ref src2, .. } - | Inst::XmmRmRVex3 { - src3: XmmMem(RegMem::Mem { ref addr }), - .. + | Inst::XmmUnaryRmRImmVex { + dst, src: ref src2, .. } - | Inst::XmmRmRBlendVex { - src2: XmmMem(RegMem::Mem { ref addr }), - .. + | Inst::XmmRmREvex { dst, ref src2, .. } + | Inst::XmmUnaryRmRImmEvex { + dst, src: ref src2, .. } - | Inst::XmmUnaryRmR { - src: XmmMemAligned(RegMem::Mem { ref addr }), + | Inst::XmmRmREvex3 { + dst, + src3: ref src2, .. } | Inst::XmmUnaryRmRUnaligned { - src: XmmMem(RegMem::Mem { ref addr }), - .. - } - | Inst::XmmUnaryRmRImm { - src: XmmMemAligned(RegMem::Mem { ref addr }), - .. + dst, src: ref src2, .. } | Inst::XmmUnaryRmREvex { - src: XmmMem(RegMem::Mem { ref addr }), - .. + dst, src: ref src2, .. + } => { + match <&RegMem>::from(src2) { + RegMem::Mem { ref addr } => { + check_load(ctx, None, addr, vcode, I8X16, 128)?; + } + RegMem::Reg { .. } => {} + } + ensure_no_fact(vcode, dst.to_writable_reg().to_reg()) } - | Inst::XmmUnaryRmRVex { - src: XmmMem(RegMem::Mem { ref addr }), - .. + + Inst::XmmRmiRVex { dst, ref src2, .. } => { + match <&RegMemImm>::from(src2) { + RegMemImm::Mem { ref addr } => { + check_load(ctx, None, addr, vcode, I8X16, 128)?; + } + RegMemImm::Reg { .. } | RegMemImm::Imm { .. } => {} + } + ensure_no_fact(vcode, dst.to_writable_reg().to_reg()) } - | Inst::XmmUnaryRmRImmVex { - src: XmmMem(RegMem::Mem { ref addr }), - .. + + Inst::XmmVexPinsr { dst, ref src2, .. } => { + match <&RegMem>::from(src2) { + RegMem::Mem { ref addr } => { + check_load(ctx, None, addr, vcode, I64, 64)?; + } + RegMem::Reg { .. } => {} + } + ensure_no_fact(vcode, dst.to_writable_reg().to_reg()) } - | Inst::XmmRmREvex { - src2: XmmMem(RegMem::Mem { ref addr }), - .. + + Inst::XmmMovRMVex { ref dst, .. } | Inst::XmmMovRMImmVex { ref dst, .. } => { + check_store(ctx, None, dst, vcode, I8X16) } - | Inst::XmmUnaryRmRImmEvex { - src: XmmMem(RegMem::Mem { ref addr }), - .. + + Inst::XmmToGprImmVex { dst, .. } => ensure_no_fact(vcode, dst.to_writable_reg().to_reg()), + + Inst::GprToXmmVex { dst, ref src, .. } | Inst::GprToXmm { dst, ref src, .. } => { + match <&RegMem>::from(src) { + RegMem::Mem { ref addr } => { + check_load(ctx, None, addr, vcode, I64, 64)?; + } + RegMem::Reg { .. } => {} + } + ensure_no_fact(vcode, dst.to_writable_reg().to_reg()) } - | Inst::XmmRmREvex3 { - src3: XmmMem(RegMem::Mem { ref addr }), - .. - } => { - check_load(ctx, None, addr, vcode, I8X16, 128)?; + + Inst::XmmToGprVex { dst, .. } => undefined_result(ctx, vcode, dst, 64, 64), + + Inst::XmmMovRM { ref dst, .. } | Inst::XmmMovRMImm { ref dst, .. } => { + check_store(ctx, None, dst, vcode, I8X16)?; Ok(()) } - Inst::XmmVexPinsr { - src2: GprMem(RegMem::Mem { ref addr }), - .. + Inst::XmmToGpr { dst, .. } | Inst::XmmToGprImm { dst, .. } => { + undefined_result(ctx, vcode, dst, 64, 64) } - | Inst::GprToXmm { - src: GprMem(RegMem::Mem { ref addr }), - .. + + Inst::CvtIntToFloat { dst, ref src2, .. } + | Inst::CvtIntToFloatVex { dst, ref src2, .. } => { + match <&RegMem>::from(src2) { + RegMem::Mem { ref addr } => { + check_load(ctx, None, addr, vcode, I64, 64)?; + } + RegMem::Reg { .. } => {} + } + ensure_no_fact(vcode, dst.to_writable_reg().to_reg()) } - | Inst::GprToXmmVex { - src: GprMem(RegMem::Mem { ref addr }), + + Inst::CvtUint64ToFloatSeq { + dst, + tmp_gpr1, + tmp_gpr2, .. } => { - check_load(ctx, None, addr, vcode, I64, 64)?; + ensure_no_fact(vcode, dst.to_writable_reg().to_reg())?; + ensure_no_fact(vcode, tmp_gpr1.to_writable_reg().to_reg())?; + ensure_no_fact(vcode, tmp_gpr2.to_writable_reg().to_reg())?; Ok(()) } - Inst::XmmCmove { - consequent: XmmMemAligned(RegMem::Mem { ref addr }), + Inst::CvtFloatToSintSeq { + dst, + tmp_gpr, + tmp_xmm, .. } => { - check_load(ctx, None, addr, vcode, I8X16, 128)?; + undefined_result(ctx, vcode, dst, 64, 64)?; + ensure_no_fact(vcode, tmp_gpr.to_writable_reg().to_reg())?; + ensure_no_fact(vcode, tmp_xmm.to_writable_reg().to_reg())?; Ok(()) } - Inst::XmmCmpRmR { - src: XmmMemAligned(RegMem::Mem { ref addr }), - .. - } - | Inst::XmmRmRImm { - src2: RegMem::Mem { ref addr }, + Inst::CvtFloatToUintSeq { + dst, + tmp_gpr, + tmp_xmm, + tmp_xmm2, .. } => { - check_load(ctx, None, addr, vcode, I8X16, 128)?; + undefined_result(ctx, vcode, dst, 64, 64)?; + ensure_no_fact(vcode, tmp_gpr.to_writable_reg().to_reg())?; + ensure_no_fact(vcode, tmp_xmm.to_writable_reg().to_reg())?; + ensure_no_fact(vcode, tmp_xmm2.to_writable_reg().to_reg())?; Ok(()) } - Inst::CvtIntToFloat { - src2: GprMem(RegMem::Mem { ref addr }), - .. + Inst::XmmMinMaxSeq { dst, .. } => ensure_no_fact(vcode, dst.to_writable_reg().to_reg()), + + Inst::XmmCmpRmR { ref src, .. } => match <&RegMem>::from(src) { + RegMem::Mem { ref addr } => { + check_load(ctx, None, addr, vcode, I8X16, 128)?; + Ok(()) + } + RegMem::Reg { .. } => Ok(()), + }, + + Inst::XmmRmRImm { dst, ref src2, .. } => { + match <&RegMem>::from(src2) { + RegMem::Mem { ref addr } => { + check_load(ctx, None, addr, vcode, I8X16, 128)?; + } + RegMem::Reg { .. } => {} + } + ensure_no_fact(vcode, dst.to_reg()) + } + + Inst::CallKnown { .. } + | Inst::ReturnCallKnown { .. } + | Inst::JmpKnown { .. } + | Inst::Ret { .. } + | Inst::JmpIf { .. } + | Inst::JmpCond { .. } + | Inst::TrapIf { .. } + | Inst::TrapIfAnd { .. } + | Inst::TrapIfOr { .. } + | Inst::Hlt {} + | Inst::Ud2 { .. } => Ok(()), + Inst::Rets { .. } => Ok(()), + + Inst::CallUnknown { ref dest, .. } + | Inst::ReturnCallUnknown { + callee: ref dest, .. } - | Inst::CvtIntToFloatVex { - src2: GprMem(RegMem::Mem { ref addr }), - .. - } => { - check_load(ctx, None, addr, vcode, I64, 64)?; + | Inst::JmpUnknown { + target: ref dest, .. + } => match <&RegMem>::from(dest) { + RegMem::Mem { ref addr } => { + check_load(ctx, None, addr, vcode, I64, 64)?; + Ok(()) + } + RegMem::Reg { .. } => Ok(()), + }, + + Inst::JmpTableSeq { tmp1, tmp2, .. } => { + ensure_no_fact(vcode, tmp1.to_reg())?; + ensure_no_fact(vcode, tmp2.to_reg())?; Ok(()) } - Inst::LockCmpxchg { mem: ref dst, .. } | Inst::AtomicRmwSeq { mem: ref dst, .. } => { - check_store(ctx, None, dst, vcode, I64)?; + Inst::LoadExtName { dst, .. } => { + ensure_no_fact(vcode, dst.to_reg())?; Ok(()) } - Inst::CallUnknown { - dest: RegMem::Mem { ref addr }, - .. - } - | Inst::ReturnCallUnknown { - callee: RegMem::Mem { ref addr }, - .. + Inst::LockCmpxchg { + ref mem, dst_old, .. + } => { + ensure_no_fact(vcode, dst_old.to_reg())?; + check_store(ctx, None, mem, vcode, I64)?; + Ok(()) } - | Inst::JmpUnknown { - target: RegMem::Mem { ref addr }, + + Inst::AtomicRmwSeq { + ref mem, + temp, + dst_old, .. } => { - check_load(ctx, None, addr, vcode, I64, 64)?; + ensure_no_fact(vcode, dst_old.to_reg())?; + ensure_no_fact(vcode, temp.to_reg())?; + check_store(ctx, None, mem, vcode, I64)?; Ok(()) } - _ if vcode.inst_defines_facts(inst_idx) => { - trace!( - "Unsupported inst during PCC validation: {:?}", - vcode[inst_idx] - ); - Err(PccError::UnsupportedFact) + Inst::Fence { .. } | Inst::VirtualSPOffsetAdj { .. } => Ok(()), + + Inst::XmmUninitializedValue { dst } => { + ensure_no_fact(vcode, dst.to_writable_reg().to_reg()) + } + + Inst::ElfTlsGetAddr { dst, .. } | Inst::MachOTlsGetAddr { dst, .. } => { + ensure_no_fact(vcode, dst.to_writable_reg().to_reg()) + } + Inst::CoffTlsGetAddr { dst, tmp, .. } => { + ensure_no_fact(vcode, dst.to_writable_reg().to_reg())?; + ensure_no_fact(vcode, tmp.to_writable_reg().to_reg())?; + Ok(()) } - _ => Ok(()), + Inst::Unwind { .. } | Inst::DummyUse { .. } => Ok(()), } }