diff --git a/cranelift/codegen/src/ir/memflags.rs b/cranelift/codegen/src/ir/memflags.rs index 72d09dae394b..8a5626d7457e 100644 --- a/cranelift/codegen/src/ir/memflags.rs +++ b/cranelift/codegen/src/ir/memflags.rs @@ -1,5 +1,6 @@ //! Memory operation flags. +use super::TrapCode; use core::fmt; #[cfg(feature = "enable-serde")] @@ -9,6 +10,9 @@ enum FlagBit { /// Guaranteed not to trap. This may enable additional /// optimizations to be performed. Notrap, + /// If this memory access traps, use [TrapCode::TableOutOfBounds] + /// instead of [TrapCode::HeapOutOfBounds]. + TableTrap, /// Guaranteed to use "natural alignment" for the given type. This /// may enable better instruction selection. Aligned, @@ -36,8 +40,17 @@ enum FlagBit { Checked, } -const NAMES: [&str; 9] = [ - "notrap", "aligned", "readonly", "little", "big", "heap", "table", "vmctx", "checked", +const NAMES: [&str; 10] = [ + "notrap", + "tabletrap", + "aligned", + "readonly", + "little", + "big", + "heap", + "table", + "vmctx", + "checked", ]; /// Endianness of a memory access. @@ -162,6 +175,25 @@ impl MemFlags { self.with(FlagBit::Notrap) } + /// Test if the `tabletrap` bit is set. + /// + /// By default, if this memory access traps, the associated trap + /// code will be `HeapOutOfBounds`. With this flag set, the trap + /// code will instead be `TableOutOfBounds`. + pub const fn tabletrap(self) -> bool { + self.read(FlagBit::TableTrap) + } + + /// Set the `tabletrap` bit. + pub fn set_tabletrap(&mut self) { + *self = self.with_tabletrap(); + } + + /// Set the `tabletrap` bit, returning new flags. + pub const fn with_tabletrap(self) -> Self { + self.with(FlagBit::TableTrap) + } + /// Test if the `aligned` flag is set. /// /// By default, Cranelift memory instructions work with any unaligned effective address. If the @@ -296,6 +328,21 @@ impl MemFlags { pub const fn with_checked(self) -> Self { self.with(FlagBit::Checked) } + + /// Get the trap code to report if this memory access traps. If the + /// `notrap` flag is set, then return `None`. Otherwise, the choice + /// of trap code depends on the `tabletrap` flag. The default trap + /// code is `HeapOutOfBounds`, but with `tabletrap` set, the trap + /// code is instead `TableOutOfBounds`. + pub const fn trap_code(self) -> Option { + if self.notrap() { + None + } else if self.tabletrap() { + Some(TrapCode::TableOutOfBounds) + } else { + Some(TrapCode::HeapOutOfBounds) + } + } } impl fmt::Display for MemFlags { diff --git a/cranelift/codegen/src/isa/aarch64/inst/emit.rs b/cranelift/codegen/src/isa/aarch64/inst/emit.rs index 880a804a537f..b02fe200e598 100644 --- a/cranelift/codegen/src/isa/aarch64/inst/emit.rs +++ b/cranelift/codegen/src/isa/aarch64/inst/emit.rs @@ -4,7 +4,7 @@ use cranelift_control::ControlPlane; use regalloc2::Allocation; use crate::binemit::StackMap; -use crate::ir::{self, types::*, TrapCode}; +use crate::ir::{self, types::*}; use crate::isa::aarch64::inst::*; use crate::trace; @@ -1005,9 +1005,9 @@ impl MachInstEmit for Inst { _ => unreachable!(), }; - if !flags.notrap() { + if let Some(trap_code) = flags.trap_code() { // Register the offset at which the actual load instruction starts. - sink.add_trap(TrapCode::HeapOutOfBounds); + sink.add_trap(trap_code); } match &mem { @@ -1140,9 +1140,9 @@ impl MachInstEmit for Inst { _ => unreachable!(), }; - if !flags.notrap() { + if let Some(trap_code) = flags.trap_code() { // Register the offset at which the actual store instruction starts. - sink.add_trap(TrapCode::HeapOutOfBounds); + sink.add_trap(trap_code); } match &mem { @@ -1219,9 +1219,9 @@ impl MachInstEmit for Inst { let rt = allocs.next(rt); let rt2 = allocs.next(rt2); let mem = mem.with_allocs(&mut allocs); - if !flags.notrap() { + if let Some(trap_code) = flags.trap_code() { // Register the offset at which the actual store instruction starts. - sink.add_trap(TrapCode::HeapOutOfBounds); + sink.add_trap(trap_code); } match &mem { &PairAMode::SignedOffset { reg, simm7 } => { @@ -1250,9 +1250,9 @@ impl MachInstEmit for Inst { let rt = allocs.next(rt.to_reg()); let rt2 = allocs.next(rt2.to_reg()); let mem = mem.with_allocs(&mut allocs); - if !flags.notrap() { + if let Some(trap_code) = flags.trap_code() { // Register the offset at which the actual load instruction starts. - sink.add_trap(TrapCode::HeapOutOfBounds); + sink.add_trap(trap_code); } match &mem { @@ -1289,9 +1289,9 @@ impl MachInstEmit for Inst { let rt2 = allocs.next(rt2.to_reg()); let mem = mem.with_allocs(&mut allocs); - if !flags.notrap() { + if let Some(trap_code) = flags.trap_code() { // Register the offset at which the actual load instruction starts. - sink.add_trap(TrapCode::HeapOutOfBounds); + sink.add_trap(trap_code); } let opc = match self { @@ -1334,9 +1334,9 @@ impl MachInstEmit for Inst { let rt2 = allocs.next(rt2); let mem = mem.with_allocs(&mut allocs); - if !flags.notrap() { + if let Some(trap_code) = flags.trap_code() { // Register the offset at which the actual store instruction starts. - sink.add_trap(TrapCode::HeapOutOfBounds); + sink.add_trap(trap_code); } let opc = match self { @@ -1494,8 +1494,8 @@ impl MachInstEmit for Inst { let rt = allocs.next_writable(rt); let rn = allocs.next(rn); - if !flags.notrap() { - sink.add_trap(TrapCode::HeapOutOfBounds); + if let Some(trap_code) = flags.trap_code() { + sink.add_trap(trap_code); } sink.put4(enc_acq_rel(ty, op, rs, rt, rn)); @@ -1534,8 +1534,8 @@ impl MachInstEmit for Inst { // again: sink.bind_label(again_label, &mut state.ctrl_plane); - if !flags.notrap() { - sink.add_trap(TrapCode::HeapOutOfBounds); + if let Some(trap_code) = flags.trap_code() { + sink.add_trap(trap_code); } sink.put4(enc_ldaxr(ty, x27wr, x25)); // ldaxr x27, [x25] @@ -1658,8 +1658,8 @@ impl MachInstEmit for Inst { } } - if !flags.notrap() { - sink.add_trap(TrapCode::HeapOutOfBounds); + if let Some(trap_code) = flags.trap_code() { + sink.add_trap(trap_code); } if op == AtomicRMWLoopOp::Xchg { sink.put4(enc_stlxr(ty, x24wr, x26, x25)); // stlxr w24, x26, [x25] @@ -1699,8 +1699,8 @@ impl MachInstEmit for Inst { _ => panic!("Unsupported type: {}", ty), }; - if !flags.notrap() { - sink.add_trap(TrapCode::HeapOutOfBounds); + if let Some(trap_code) = flags.trap_code() { + sink.add_trap(trap_code); } sink.put4(enc_cas(size, rd, rt, rn)); @@ -1733,8 +1733,8 @@ impl MachInstEmit for Inst { // again: sink.bind_label(again_label, &mut state.ctrl_plane); - if !flags.notrap() { - sink.add_trap(TrapCode::HeapOutOfBounds); + if let Some(trap_code) = flags.trap_code() { + sink.add_trap(trap_code); } // ldaxr x27, [x25] @@ -1760,8 +1760,8 @@ impl MachInstEmit for Inst { )); sink.use_label_at_offset(br_out_offset, out_label, LabelUse::Branch19); - if !flags.notrap() { - sink.add_trap(TrapCode::HeapOutOfBounds); + if let Some(trap_code) = flags.trap_code() { + sink.add_trap(trap_code); } sink.put4(enc_stlxr(ty, x24wr, x28, x25)); // stlxr w24, x28, [x25] @@ -1789,8 +1789,8 @@ impl MachInstEmit for Inst { let rn = allocs.next(rn); let rt = allocs.next_writable(rt); - if !flags.notrap() { - sink.add_trap(TrapCode::HeapOutOfBounds); + if let Some(trap_code) = flags.trap_code() { + sink.add_trap(trap_code); } sink.put4(enc_ldar(access_ty, rt, rn)); @@ -1804,8 +1804,8 @@ impl MachInstEmit for Inst { let rn = allocs.next(rn); let rt = allocs.next(rt); - if !flags.notrap() { - sink.add_trap(TrapCode::HeapOutOfBounds); + if let Some(trap_code) = flags.trap_code() { + sink.add_trap(trap_code); } sink.put4(enc_stlr(access_ty, rt, rn)); @@ -2977,9 +2977,9 @@ impl MachInstEmit for Inst { let rn = allocs.next(rn); let (q, size) = size.enc_size(); - if !flags.notrap() { + if let Some(trap_code) = flags.trap_code() { // Register the offset at which the actual load instruction starts. - sink.add_trap(TrapCode::HeapOutOfBounds); + sink.add_trap(trap_code); } sink.put4(enc_ldst_vec(q, size, rn, rd)); diff --git a/cranelift/codegen/src/isa/riscv64/inst/emit.rs b/cranelift/codegen/src/isa/riscv64/inst/emit.rs index aa86021c4a58..c4f32c58bb79 100644 --- a/cranelift/codegen/src/isa/riscv64/inst/emit.rs +++ b/cranelift/codegen/src/isa/riscv64/inst/emit.rs @@ -630,9 +630,9 @@ impl Inst { _ => return None, }; - if !flags.notrap() { + if let Some(trap_code) = flags.trap_code() { // Register the offset at which the actual load instruction starts. - sink.add_trap(TrapCode::HeapOutOfBounds); + sink.add_trap(trap_code); } sink.put2(encode_ci_sp_load(op, rd, imm6)); } @@ -701,9 +701,9 @@ impl Inst { encode_cl_type(op, rd, base, imm5) }; - if !flags.notrap() { + if let Some(trap_code) = flags.trap_code() { // Register the offset at which the actual load instruction starts. - sink.add_trap(TrapCode::HeapOutOfBounds); + sink.add_trap(trap_code); } sink.put2(encoded); } @@ -732,9 +732,9 @@ impl Inst { _ => return None, }; - if !flags.notrap() { + if let Some(trap_code) = flags.trap_code() { // Register the offset at which the actual load instruction starts. - sink.add_trap(TrapCode::HeapOutOfBounds); + sink.add_trap(trap_code); } sink.put2(encode_css_type(op, src, imm6)); } @@ -799,9 +799,9 @@ impl Inst { encode_cs_type(op, src, base, imm5) }; - if !flags.notrap() { + if let Some(trap_code) = flags.trap_code() { // Register the offset at which the actual load instruction starts. - sink.add_trap(TrapCode::HeapOutOfBounds); + sink.add_trap(trap_code); } sink.put2(encoded); } @@ -1077,9 +1077,9 @@ impl Inst { } }; - if !flags.notrap() { + if let Some(trap_code) = flags.trap_code() { // Register the offset at which the actual load instruction starts. - sink.add_trap(TrapCode::HeapOutOfBounds); + sink.add_trap(trap_code); } sink.put4(encode_i_type(op.op_code(), rd, op.funct3(), addr, imm12)); @@ -1100,9 +1100,9 @@ impl Inst { } }; - if !flags.notrap() { + if let Some(trap_code) = flags.trap_code() { // Register the offset at which the actual load instruction starts. - sink.add_trap(TrapCode::HeapOutOfBounds); + sink.add_trap(trap_code); } sink.put4(encode_s_type(op.op_code(), op.funct3(), addr, src, imm12)); @@ -1492,7 +1492,11 @@ impl Inst { src, amo, } => { - sink.add_trap(TrapCode::HeapOutOfBounds); + // TODO: get flags from original CLIF atomic instruction + let flags = MemFlags::new(); + if let Some(trap_code) = flags.trap_code() { + sink.add_trap(trap_code); + } let x = op.op_code() | reg_to_gpr_num(rd.to_reg()) << 7 | op.funct3() << 12 @@ -2718,9 +2722,9 @@ impl Inst { } }; - if !flags.notrap() { + if let Some(trap_code) = flags.trap_code() { // Register the offset at which the actual load instruction starts. - sink.add_trap(TrapCode::HeapOutOfBounds); + sink.add_trap(trap_code); } sink.put4(encode_vmem_load( @@ -2765,9 +2769,9 @@ impl Inst { } }; - if !flags.notrap() { + if let Some(trap_code) = flags.trap_code() { // Register the offset at which the actual load instruction starts. - sink.add_trap(TrapCode::HeapOutOfBounds); + sink.add_trap(trap_code); } sink.put4(encode_vmem_store( diff --git a/cranelift/codegen/src/isa/s390x/inst/args.rs b/cranelift/codegen/src/isa/s390x/inst/args.rs index 553ca8660811..0f1389c80ac0 100644 --- a/cranelift/codegen/src/isa/s390x/inst/args.rs +++ b/cranelift/codegen/src/isa/s390x/inst/args.rs @@ -102,10 +102,6 @@ impl MemArg { } } - pub(crate) fn can_trap(&self) -> bool { - !self.get_flags().notrap() - } - /// Edit registers with allocations. pub fn with_allocs(&self, allocs: &mut AllocationConsumer<'_>) -> Self { match self { @@ -187,10 +183,6 @@ impl MemArgPair { } } - pub(crate) fn can_trap(&self) -> bool { - !self.flags.notrap() - } - /// Edit registers with allocations. pub fn with_allocs(&self, allocs: &mut AllocationConsumer<'_>) -> Self { MemArgPair { diff --git a/cranelift/codegen/src/isa/s390x/inst/emit.rs b/cranelift/codegen/src/isa/s390x/inst/emit.rs index 6531d7df6114..b929957b501d 100644 --- a/cranelift/codegen/src/isa/s390x/inst/emit.rs +++ b/cranelift/codegen/src/isa/s390x/inst/emit.rs @@ -184,8 +184,10 @@ pub fn mem_emit( inst.emit(&[], sink, emit_info, state); } - if add_trap && mem.can_trap() { - sink.add_trap(TrapCode::HeapOutOfBounds); + if add_trap { + if let Some(trap_code) = mem.get_flags().trap_code() { + sink.add_trap(trap_code); + } } match &mem { @@ -245,8 +247,10 @@ pub fn mem_rs_emit( inst.emit(&[], sink, emit_info, state); } - if add_trap && mem.can_trap() { - sink.add_trap(TrapCode::HeapOutOfBounds); + if add_trap { + if let Some(trap_code) = mem.get_flags().trap_code() { + sink.add_trap(trap_code); + } } match &mem { @@ -294,8 +298,10 @@ pub fn mem_imm8_emit( inst.emit(&[], sink, emit_info, state); } - if add_trap && mem.can_trap() { - sink.add_trap(TrapCode::HeapOutOfBounds); + if add_trap { + if let Some(trap_code) = mem.get_flags().trap_code() { + sink.add_trap(trap_code); + } } match &mem { @@ -339,8 +345,10 @@ pub fn mem_imm16_emit( inst.emit(&[], sink, emit_info, state); } - if add_trap && mem.can_trap() { - sink.add_trap(TrapCode::HeapOutOfBounds); + if add_trap { + if let Some(trap_code) = mem.get_flags().trap_code() { + sink.add_trap(trap_code); + } } match &mem { @@ -363,8 +371,10 @@ pub fn mem_mem_emit( sink: &mut MachBuffer, _state: &mut EmitState, ) { - if add_trap && (dst.can_trap() || src.can_trap()) { - sink.add_trap(TrapCode::HeapOutOfBounds); + if add_trap { + if let Some(trap_code) = dst.flags.trap_code().or(src.flags.trap_code()) { + sink.add_trap(trap_code); + } } put( @@ -405,8 +415,10 @@ pub fn mem_vrx_emit( inst.emit(&[], sink, emit_info, state); } - if add_trap && mem.can_trap() { - sink.add_trap(TrapCode::HeapOutOfBounds); + if add_trap { + if let Some(trap_code) = mem.get_flags().trap_code() { + sink.add_trap(trap_code); + } } match &mem { diff --git a/cranelift/codegen/src/isa/x64/encoding/evex.rs b/cranelift/codegen/src/isa/x64/encoding/evex.rs index 03a3ac6c7b0e..3805e7215545 100644 --- a/cranelift/codegen/src/isa/x64/encoding/evex.rs +++ b/cranelift/codegen/src/isa/x64/encoding/evex.rs @@ -15,7 +15,6 @@ //! Software Development Manual, volume 2A). use super::rex::{self, LegacyPrefixes, OpcodeMap}; -use crate::ir::TrapCode; use crate::isa::x64::args::{Amode, Avx512TupleType}; use crate::isa::x64::inst::Inst; use crate::MachBuffer; @@ -202,8 +201,8 @@ impl EvexInstruction { /// - an optional immediate, if necessary (not currently implemented) pub fn encode(&self, sink: &mut MachBuffer) { if let RegisterOrAmode::Amode(amode) = &self.rm { - if amode.can_trap() { - sink.add_trap(TrapCode::HeapOutOfBounds); + if let Some(trap_code) = amode.get_flags().trap_code() { + sink.add_trap(trap_code); } } sink.put4(self.bits); diff --git a/cranelift/codegen/src/isa/x64/encoding/rex.rs b/cranelift/codegen/src/isa/x64/encoding/rex.rs index 88b6eca2d453..ba7b3b19d74c 100644 --- a/cranelift/codegen/src/isa/x64/encoding/rex.rs +++ b/cranelift/codegen/src/isa/x64/encoding/rex.rs @@ -10,7 +10,6 @@ use crate::machinst::{Reg, RegClass}; use crate::{ - ir::TrapCode, isa::x64::inst::{ args::{Amode, OperandSize}, regs, Inst, LabelUse, @@ -314,9 +313,8 @@ pub(crate) fn emit_std_enc_mem( // 64-bit integer registers, because they are part of an address // expression. But `enc_g` can be derived from a register of any class. - let can_trap = mem_e.can_trap(); - if can_trap { - sink.add_trap(TrapCode::HeapOutOfBounds); + if let Some(trap_code) = mem_e.get_flags().trap_code() { + sink.add_trap(trap_code); } prefixes.emit(sink); diff --git a/cranelift/codegen/src/isa/x64/encoding/vex.rs b/cranelift/codegen/src/isa/x64/encoding/vex.rs index 7e853c3361ce..1124a6b5b28c 100644 --- a/cranelift/codegen/src/isa/x64/encoding/vex.rs +++ b/cranelift/codegen/src/isa/x64/encoding/vex.rs @@ -4,7 +4,6 @@ use super::evex::{Register, RegisterOrAmode}; use super::rex::{LegacyPrefixes, OpcodeMap}; use super::ByteSink; -use crate::ir::TrapCode; use crate::isa::x64::args::Amode; use crate::isa::x64::encoding::rex; use crate::isa::x64::inst::Inst; @@ -251,8 +250,8 @@ impl VexInstruction { /// Emit the VEX-encoded instruction to the provided buffer. pub fn encode(&self, sink: &mut MachBuffer) { if let RegisterOrAmode::Amode(amode) = &self.rm { - if amode.can_trap() { - sink.add_trap(TrapCode::HeapOutOfBounds); + if let Some(trap_code) = amode.get_flags().trap_code() { + sink.add_trap(trap_code); } } diff --git a/cranelift/codegen/src/isa/x64/inst/args.rs b/cranelift/codegen/src/isa/x64/inst/args.rs index 85c70b6b31f0..45ab8e6e8a5b 100644 --- a/cranelift/codegen/src/isa/x64/inst/args.rs +++ b/cranelift/codegen/src/isa/x64/inst/args.rs @@ -405,10 +405,6 @@ impl Amode { } } - pub(crate) fn can_trap(&self) -> bool { - !self.get_flags().notrap() - } - pub(crate) fn with_allocs(&self, allocs: &mut AllocationConsumer<'_>) -> Self { // The order in which we consume allocs here must match the // order in which we produce operands in get_operands() above. diff --git a/cranelift/interpreter/src/interpreter.rs b/cranelift/interpreter/src/interpreter.rs index 7d0bbe3d017c..6737494e6d9d 100644 --- a/cranelift/interpreter/src/interpreter.rs +++ b/cranelift/interpreter/src/interpreter.rs @@ -331,7 +331,11 @@ impl<'a> State<'a> for InterpreterState<'a> { let src = match addr.region { AddressRegion::Stack => { if addr_end > self.stack.len() { - return Err(MemoryError::OutOfBoundsLoad { addr, load_size }); + return Err(MemoryError::OutOfBoundsLoad { + addr, + load_size, + mem_flags, + }); } &self.stack[addr_start..addr_end] @@ -363,7 +367,11 @@ impl<'a> State<'a> for InterpreterState<'a> { let dst = match addr.region { AddressRegion::Stack => { if addr_end > self.stack.len() { - return Err(MemoryError::OutOfBoundsStore { addr, store_size }); + return Err(MemoryError::OutOfBoundsStore { + addr, + store_size, + mem_flags, + }); } &mut self.stack[addr_start..addr_end] diff --git a/cranelift/interpreter/src/state.rs b/cranelift/interpreter/src/state.rs index 5c5d7a685be3..9f3bfdddd95e 100644 --- a/cranelift/interpreter/src/state.rs +++ b/cranelift/interpreter/src/state.rs @@ -125,9 +125,17 @@ pub enum MemoryError { #[error("Requested an offset of {offset} but max was {max}")] InvalidOffset { offset: u64, max: u64 }, #[error("Load of {load_size} bytes is larger than available size at address {addr:?}")] - OutOfBoundsLoad { addr: Address, load_size: usize }, + OutOfBoundsLoad { + addr: Address, + load_size: usize, + mem_flags: MemFlags, + }, #[error("Store of {store_size} bytes is larger than available size at address {addr:?}")] - OutOfBoundsStore { addr: Address, store_size: usize }, + OutOfBoundsStore { + addr: Address, + store_size: usize, + mem_flags: MemFlags, + }, #[error("Load of {load_size} bytes is misaligned at address {addr:?}")] MisalignedLoad { addr: Address, load_size: usize }, #[error("Store of {store_size} bytes is misaligned at address {addr:?}")] diff --git a/cranelift/interpreter/src/step.rs b/cranelift/interpreter/src/step.rs index 0ffff70e2cbe..975ae2f7c8d5 100644 --- a/cranelift/interpreter/src/step.rs +++ b/cranelift/interpreter/src/step.rs @@ -161,8 +161,12 @@ where MemoryError::InvalidAddressType(_) => TrapCode::HeapOutOfBounds, MemoryError::InvalidOffset { .. } => TrapCode::HeapOutOfBounds, MemoryError::InvalidEntry { .. } => TrapCode::HeapOutOfBounds, - MemoryError::OutOfBoundsStore { .. } => TrapCode::HeapOutOfBounds, - MemoryError::OutOfBoundsLoad { .. } => TrapCode::HeapOutOfBounds, + MemoryError::OutOfBoundsStore { mem_flags, .. } => mem_flags + .trap_code() + .expect("store with notrap flag should not trap"), + MemoryError::OutOfBoundsLoad { mem_flags, .. } => mem_flags + .trap_code() + .expect("load with notrap flag should not trap"), MemoryError::MisalignedLoad { .. } => TrapCode::HeapMisaligned, MemoryError::MisalignedStore { .. } => TrapCode::HeapMisaligned, };