From faeeed4fb94e03b5108940dd71f2b360f97d3983 Mon Sep 17 00:00:00 2001 From: Afonso Bordado Date: Mon, 31 Oct 2022 22:07:14 +0000 Subject: [PATCH] cranelift: Correctly calculate heap addresses in interpreter (#5155) We were accidentally including the size as part of the offset when computing heap addresses. --- .../filetests/filetests/runtests/heap.clif | 17 +++++++++++++++++ cranelift/interpreter/src/step.rs | 13 ++++++++++--- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/cranelift/filetests/filetests/runtests/heap.clif b/cranelift/filetests/filetests/runtests/heap.clif index 9b283db702..e956dcff38 100644 --- a/cranelift/filetests/filetests/runtests/heap.clif +++ b/cranelift/filetests/filetests/runtests/heap.clif @@ -204,3 +204,20 @@ block0(v0: i64, v1: i32): ; heap: static, size=0x1000, ptr=vmctx+0, bound=vmctx+8 ; run: %iadd_imm(1) == 1 ; run: %iadd_imm(-1) == -1 + +function %heap_limit_i64(i64 vmctx, i64, i32) -> i32 { + gv0 = vmctx + gv1 = load.i64 notrap aligned gv0+0 + heap0 = static gv1, min 0, bound 0x8, offset_guard 0, index_type i64 + +block0(v0: i64, v1: i64, v2: i32): + v3 = heap_addr.i64 heap0, v1, 4 + store.i32 v2, v3 + v4 = load.i32 v3 + return v4 +} +; heap: static, size=0x8, ptr=vmctx+0, bound=vmctx+8 +; run: %heap_limit_i64(0, 1) == 1 +; run: %heap_limit_i64(0, -1) == -1 +; run: %heap_limit_i64(4, 1) == 1 +; run: %heap_limit_i64(4, -1) == -1 diff --git a/cranelift/interpreter/src/step.rs b/cranelift/interpreter/src/step.rs index 6da172d6d8..18ecc22a76 100644 --- a/cranelift/interpreter/src/step.rs +++ b/cranelift/interpreter/src/step.rs @@ -498,10 +498,17 @@ where Opcode::TlsValue => unimplemented!("TlsValue"), Opcode::HeapAddr => { if let InstructionData::HeapAddr { heap, .. } = inst { - let load_ty = inst_context.controlling_type().unwrap(); - let offset = calculate_addr(ctrl_ty, imm(), args()?)? as u64; + let addr_ty = inst_context.controlling_type().unwrap(); + let offset = arg(0)?.into_int()? as u64; + let load_size = imm().into_int()? as u64; assign_or_memtrap({ - AddressSize::try_from(load_ty).and_then(|addr_size| { + AddressSize::try_from(addr_ty).and_then(|addr_size| { + // Attempt to build an address at the maximum possible offset + // for this load. If address generation fails we know it's out of bounds. + let bound_offset = (offset + load_size).saturating_sub(1); + state.heap_address(addr_size, heap, bound_offset)?; + + // Build the actual address let addr = state.heap_address(addr_size, heap, offset)?; let dv = DataValue::try_from(addr)?; Ok(dv.into())