Skip to content

Commit b0edae2

Browse files
committed
fix: fix errors in intrinsics::mem::replace_element,
`intrinsics::mem::store_felt_unchecked`, `intrinsics::mem::load_sw` #230 - assert in `intrinsics::mem::replace_element`, - storing not in the word address in `intrinsics::mem::store_felt_unchecked`, -loading from not from a pointer, but a value(partial) in `intrinsics::mem::load_sw` causing "out of bounds memory access". Add test for `mem::intrinsics::store_sw/load_sw`
1 parent 79d1c24 commit b0edae2

File tree

2 files changed

+72
-5
lines changed

2 files changed

+72
-5
lines changed

codegen/masm/intrinsics/mem.masm

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ export.load_sw # [waddr, index, offset]
126126
dup.0 # [waddr, waddr, offset]
127127
u32overflowing_add.1 assertz # [waddr + 1, waddr, offset]
128128
# load the word and drop the unused elements
129-
padw movup.4 mem_loadw movdn.4 drop drop drop # [w0, waddr, offset]
129+
padw movup.4 mem_loadw movdn.3 drop drop drop # [w0, waddr, offset]
130130
# shift the low bits
131131
push.32 dup.3 # [offset, 32, w0, waddr, offset]
132132
u32overflowing_sub assertz # [32 - offset, w0, waddr, offset]
@@ -364,12 +364,11 @@ proc.store_felt_unchecked # [waddr, index, value]
364364
mem_loadw # [w0, w1, w2, w3, waddr, index, value]
365365

366366
# rewrite the desired element
367-
movup.6
368-
movup.5
369-
exec.replace_element
367+
movup.6 # [value, w0, w1, w2, w3, waddr, index]
368+
movup.6 # [index, value, w0, w1, w2, w3, waddr]
369+
exec.replace_element # [w0', w1', w2', w3', waddr]
370370

371371
# store the updated word
372-
movup.4
373372
mem_storew
374373
dropw
375374
end

codegen/masm/src/tests.rs

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use midenc_hir::{
66
AbiParam, CallConv, Felt, FieldElement, FunctionIdent, Immediate, InstBuilder, Linkage,
77
OperandStack, ProgramBuilder, Signature, SourceSpan, Stack, Type,
88
};
9+
use prop::test_runner::{Config, TestRunner};
910
use proptest::prelude::*;
1011
use smallvec::{smallvec, SmallVec};
1112

@@ -208,6 +209,10 @@ impl TestByEmulationHarness {
208209
pub fn step_over(&mut self) -> Result<EmulatorEvent, EmulationError> {
209210
self.emulator.step_over()
210211
}
212+
213+
fn reset(&mut self) {
214+
self.emulator.reset();
215+
}
211216
}
212217

213218
#[test]
@@ -453,6 +458,69 @@ fn i32_checked_neg() {
453458
harness.invoke(neg, &[min]).expect("execution failed");
454459
}
455460

461+
#[test]
462+
fn codegen_mem_store_sw_load_sw() {
463+
const MEMORY_SIZE_BYTES: u32 = 1048576 * 2; // Twice the size of the default Rust shadow stack size
464+
const MEMORY_SIZE_VM_WORDS: u32 = MEMORY_SIZE_BYTES / 16;
465+
let context = TestContext::default();
466+
let mut builder = ProgramBuilder::new(&context.session.diagnostics);
467+
let mut mb = builder.module("test");
468+
let id = {
469+
let mut fb = mb
470+
.function(
471+
"store_load_sw",
472+
Signature::new(
473+
[AbiParam::new(Type::U32), AbiParam::new(Type::U32)],
474+
[AbiParam::new(Type::U32)],
475+
),
476+
)
477+
.expect("unexpected symbol conflict");
478+
let entry = fb.current_block();
479+
let (ptr_u32, value) = {
480+
let args = fb.block_params(entry);
481+
(args[0], args[1])
482+
};
483+
let ptr = fb.ins().inttoptr(ptr_u32, Type::Ptr(Type::U32.into()), SourceSpan::UNKNOWN);
484+
fb.ins().store(ptr, value, SourceSpan::UNKNOWN);
485+
let loaded_value = fb.ins().load(ptr, SourceSpan::UNKNOWN);
486+
fb.ins().ret(Some(loaded_value), SourceSpan::UNKNOWN);
487+
fb.build().expect("unexpected error building function")
488+
};
489+
490+
mb.build().expect("unexpected error constructing test module");
491+
492+
let program = builder.with_entrypoint(id).link().expect("failed to link program");
493+
494+
let mut compiler = MasmCompiler::new(&context.session);
495+
let program = compiler.compile(program).expect("compilation failed").freeze();
496+
497+
// eprintln!("{}", program);
498+
499+
fn test(program: Arc<Program>, ptr: u32, value: u32) -> u32 {
500+
eprintln!("---------------------------------");
501+
eprintln!("testing store_sw/load_sw ptr: {ptr}, value: {value}");
502+
eprintln!("---------------------------------");
503+
let mut harness = TestByEmulationHarness::with_emulator_config(
504+
MEMORY_SIZE_VM_WORDS as usize,
505+
Emulator::DEFAULT_HEAP_START as usize,
506+
Emulator::DEFAULT_LOCALS_START as usize,
507+
true,
508+
);
509+
let mut stack = harness
510+
.execute_program(program.clone(), &[Felt::new(ptr as u64), Felt::new(value as u64)])
511+
.expect("execution failed");
512+
stack.pop().unwrap().as_int() as u32
513+
}
514+
515+
TestRunner::new(Config::with_cases(1024))
516+
.run(&(0u32..MEMORY_SIZE_BYTES - 4, any::<u32>()), move |(ptr, value)| {
517+
let out = test(program.clone(), ptr, value);
518+
prop_assert_eq!(out, value);
519+
Ok(())
520+
})
521+
.unwrap();
522+
}
523+
456524
macro_rules! proptest_unary_numeric_op {
457525
($ty_name:ident :: $op:ident, $ty:ty => $ret:ty, $rust_op:ident) => {
458526
proptest_unary_numeric_op_impl!($ty_name :: $op, $ty => $ret, $rust_op, 0..$ty_name::MAX);

0 commit comments

Comments
 (0)