From d335dc8d5ac23d97c2707aa838420fddb8bd7538 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 15 Nov 2022 11:54:52 -0800 Subject: [PATCH] Cranelift: Do not optimize heap bounds checking comparison in legalization (#5272) That optimization is only for 12-bit immediates in Aarch64, which is now handled in backend lowering, so we can simplify this code a bit now. --- cranelift/codegen/src/legalizer/heap.rs | 31 +++++++------------------ 1 file changed, 8 insertions(+), 23 deletions(-) diff --git a/cranelift/codegen/src/legalizer/heap.rs b/cranelift/codegen/src/legalizer/heap.rs index 96e905b3f1..46284e973e 100644 --- a/cranelift/codegen/src/legalizer/heap.rs +++ b/cranelift/codegen/src/legalizer/heap.rs @@ -189,37 +189,22 @@ fn static_addr( let mut spectre_oob_comparison = None; let index = cast_index_to_pointer_ty(index, index_ty, addr_ty, &mut pos); if index_ty != ir::types::I32 || limit < 0xffff_ffff { - // Here we want to test the condition `index > limit` and if that's - // true then this is an out-of-bounds access and needs to trap. For ARM - // and other RISC architectures it's easier to test against an immediate - // that's even instead of odd, so if `limit` is odd then we instead test - // for `index >= limit + 1`. - // - // The thinking behind this is that: - // - // A >= B + 1 => A - 1 >= B => A > B - // - // where the last step here is true because A/B are integers, which - // should mean that `A >= B + 1` is an equivalent check for `A > B` - let (cc, lhs, limit_imm) = if limit & 1 == 1 { - let limit = limit as i64 + 1; - (IntCC::UnsignedGreaterThanOrEqual, index, limit) - } else { - let limit = limit as i64; - (IntCC::UnsignedGreaterThan, index, limit) - }; - let oob = pos.ins().icmp_imm(cc, lhs, limit_imm); + // Here we want to test the condition `index > limit` and if that's true + // then this is an out-of-bounds access and needs to trap. + let oob = pos + .ins() + .icmp_imm(IntCC::UnsignedGreaterThan, index, limit as i64); trace!(" inserting: {}", pos.func.dfg.display_value_inst(oob)); let trapnz = pos.ins().trapnz(oob, ir::TrapCode::HeapOutOfBounds); trace!(" inserting: {}", pos.func.dfg.display_inst(trapnz)); if isa.flags().enable_heap_access_spectre_mitigation() { - let limit = pos.ins().iconst(addr_ty, limit_imm); + let limit = pos.ins().iconst(addr_ty, limit as i64); trace!(" inserting: {}", pos.func.dfg.display_value_inst(limit)); spectre_oob_comparison = Some(SpectreOobComparison { - cc, - lhs, + cc: IntCC::UnsignedGreaterThan, + lhs: index, rhs: limit, }); }