diff --git a/executor/programs/asm/misalign_ld.s b/executor/programs/asm/misalign_ld.s new file mode 100644 index 000000000..9572db15e --- /dev/null +++ b/executor/programs/asm/misalign_ld.s @@ -0,0 +1,21 @@ + .attribute 5, "rv64i2p1_m2p0" + .globl main +main: + # Misaligned LD: 8-byte load at address 33. + li t0, 32 + li t1, 0x80FF1234 + sw t1, 0(t0) + li t1, 0xDEADBEEF + sw t1, 4(t0) + li t1, 0x000000AA + sw t1, 8(t0) + ld a0, 1(t0) + li t2, 0xAADEADBEEF80FF12 + bne a0, t2, .Lfail + li a0, 0 + li a7, 93 + ecall +.Lfail: + li a0, 1 + li a7, 93 + ecall diff --git a/executor/programs/asm/misalign_lh.s b/executor/programs/asm/misalign_lh.s new file mode 100644 index 000000000..08cff504e --- /dev/null +++ b/executor/programs/asm/misalign_lh.s @@ -0,0 +1,19 @@ + .attribute 5, "rv64i2p1_m2p0" + .globl main +main: + # Misaligned LH: 2-byte load at address 35. + li t0, 32 + li t1, 0x3412FF80 + sw t1, 0(t0) + li t1, 0x000000AA + sw t1, 4(t0) + lh a0, 3(t0) + li t2, 0xFFFFFFFFFFFFAA34 + bne a0, t2, .Lfail + li a0, 0 + li a7, 93 + ecall +.Lfail: + li a0, 1 + li a7, 93 + ecall diff --git a/executor/programs/asm/misalign_lhu.s b/executor/programs/asm/misalign_lhu.s new file mode 100644 index 000000000..9a9aed7a1 --- /dev/null +++ b/executor/programs/asm/misalign_lhu.s @@ -0,0 +1,19 @@ + .attribute 5, "rv64i2p1_m2p0" + .globl main +main: + # Misaligned LHU: 2-byte load at address 35. + li t0, 32 + li t1, 0x3412FF80 + sw t1, 0(t0) + li t1, 0x000000AA + sw t1, 4(t0) + lhu a0, 3(t0) + li t2, 0xAA34 + bne a0, t2, .Lfail + li a0, 0 + li a7, 93 + ecall +.Lfail: + li a0, 1 + li a7, 93 + ecall diff --git a/executor/programs/asm/misalign_lw.s b/executor/programs/asm/misalign_lw.s new file mode 100644 index 000000000..246c955b3 --- /dev/null +++ b/executor/programs/asm/misalign_lw.s @@ -0,0 +1,19 @@ + .attribute 5, "rv64i2p1_m2p0" + .globl main +main: + # Misaligned LW: 4-byte load at address 33. + li t0, 32 + li t1, 0x80FF1234 + sw t1, 0(t0) + li t1, 0x000000AA + sw t1, 4(t0) + lw a0, 1(t0) + li t2, 0xFFFFFFFFAA80FF12 + bne a0, t2, .Lfail + li a0, 0 + li a7, 93 + ecall +.Lfail: + li a0, 1 + li a7, 93 + ecall diff --git a/executor/programs/asm/misalign_lwu.s b/executor/programs/asm/misalign_lwu.s new file mode 100644 index 000000000..f990d8cdb --- /dev/null +++ b/executor/programs/asm/misalign_lwu.s @@ -0,0 +1,19 @@ + .attribute 5, "rv64i2p1_m2p0" + .globl main +main: + # Misaligned LWU: 4-byte load at address 33. + li t0, 32 + li t1, 0x80FF1234 + sw t1, 0(t0) + li t1, 0x000000AA + sw t1, 4(t0) + lwu a0, 1(t0) + li t2, 0xAA80FF12 + bne a0, t2, .Lfail + li a0, 0 + li a7, 93 + ecall +.Lfail: + li a0, 1 + li a7, 93 + ecall diff --git a/executor/programs/asm/misalign_sd.s b/executor/programs/asm/misalign_sd.s new file mode 100644 index 000000000..159af7032 --- /dev/null +++ b/executor/programs/asm/misalign_sd.s @@ -0,0 +1,23 @@ + .attribute 5, "rv64i2p1_m2p0" + .globl main +main: + # Misaligned SD: 8-byte store at address 65. + li t0, 64 + li t1, 0x123456789ABCDEF0 + sd t1, 1(t0) + lwu a1, 0(t0) + li t2, 0xBCDEF000 + bne a1, t2, .Lfail + lwu a1, 4(t0) + li t2, 0x3456789A + bne a1, t2, .Lfail + lwu a1, 8(t0) + li t2, 0x00000012 + bne a1, t2, .Lfail + li a0, 0 + li a7, 93 + ecall +.Lfail: + li a0, 1 + li a7, 93 + ecall diff --git a/executor/programs/asm/misalign_sh.s b/executor/programs/asm/misalign_sh.s new file mode 100644 index 000000000..4769869ac --- /dev/null +++ b/executor/programs/asm/misalign_sh.s @@ -0,0 +1,20 @@ + .attribute 5, "rv64i2p1_m2p0" + .globl main +main: + # Misaligned SH: 2-byte store at address 67. + li t0, 64 + li t1, 0xAA12 + sh t1, 3(t0) + lwu a1, 0(t0) + li t2, 0x12000000 + bne a1, t2, .Lfail + lwu a1, 4(t0) + li t2, 0x000000AA + bne a1, t2, .Lfail + li a0, 0 + li a7, 93 + ecall +.Lfail: + li a0, 1 + li a7, 93 + ecall diff --git a/executor/programs/asm/misalign_sw.s b/executor/programs/asm/misalign_sw.s new file mode 100644 index 000000000..124a8cee8 --- /dev/null +++ b/executor/programs/asm/misalign_sw.s @@ -0,0 +1,20 @@ + .attribute 5, "rv64i2p1_m2p0" + .globl main +main: + # Misaligned SW: 4-byte store at address 65. + li t0, 64 + li t1, 0xDEADBEEF + sw t1, 1(t0) + lwu a1, 0(t0) + li t2, 0xADBEEF00 + bne a1, t2, .Lfail + lwu a1, 4(t0) + li t2, 0x000000DE + bne a1, t2, .Lfail + li a0, 0 + li a7, 93 + ecall +.Lfail: + li a0, 1 + li a7, 93 + ecall diff --git a/executor/programs/asm/misaligned_pc.s b/executor/programs/asm/misaligned_pc.s new file mode 100644 index 000000000..832f27678 --- /dev/null +++ b/executor/programs/asm/misaligned_pc.s @@ -0,0 +1,8 @@ + .attribute 5, "rv64i2p1_m2p0" + .globl main +main: + # Jump to PC = 2 (not 4-aligned). + jalr zero, zero, 2 + li a0, 0 + li a7, 93 + ecall diff --git a/executor/src/tests/memory_tests.rs b/executor/src/tests/memory_tests.rs index fcc0e07b9..6c3b3f2ed 100644 --- a/executor/src/tests/memory_tests.rs +++ b/executor/src/tests/memory_tests.rs @@ -109,3 +109,33 @@ fn test_commit_public_output_total_cap() { let err = memory.commit_public_output(0x1_0000, 1).unwrap_err(); assert!(matches!(err, MemoryError::CommitSizeExceeded)); } + +#[test] +fn test_misaligned_load_store_overflow_errors() { + let mut memory = Memory::default(); + + assert!(matches!( + memory.load_half(u64::MAX).unwrap_err(), + MemoryError::AddressOverflow + )); + assert!(matches!( + memory.store_half(u64::MAX, 0).unwrap_err(), + MemoryError::AddressOverflow + )); + assert!(matches!( + memory.load_word(u64::MAX - 1).unwrap_err(), + MemoryError::AddressOverflow + )); + assert!(matches!( + memory.store_word(u64::MAX - 1, 0).unwrap_err(), + MemoryError::AddressOverflow + )); + assert!(matches!( + memory.load_doubleword(u64::MAX - 6).unwrap_err(), + MemoryError::AddressOverflow + )); + assert!(matches!( + memory.store_doubleword(u64::MAX - 6, 0).unwrap_err(), + MemoryError::AddressOverflow + )); +} diff --git a/executor/src/vm/execution.rs b/executor/src/vm/execution.rs index 37dcf0198..614aad649 100644 --- a/executor/src/vm/execution.rs +++ b/executor/src/vm/execution.rs @@ -64,6 +64,9 @@ impl Executor { self.logs.clear(); while self.pc != 0 && self.logs.len() < CHUNK_SIZE { + if !self.pc.is_multiple_of(4) { + return Err(ExecutorError::InstructionAddressMisaligned(self.pc)); + } let instruction = match self.instructions.get(self.pc) { Some(&instr) => instr, None => { @@ -252,4 +255,6 @@ pub enum ExecutorError { ExecutionError(#[from] ExecutionError), #[error("Memory error: {0}")] MemoryError(#[from] MemoryError), + #[error("Instruction address misaligned: {0:#018x}")] + InstructionAddressMisaligned(u64), } diff --git a/executor/src/vm/memory.rs b/executor/src/vm/memory.rs index c94376e76..ea84e2620 100644 --- a/executor/src/vm/memory.rs +++ b/executor/src/vm/memory.rs @@ -81,77 +81,105 @@ impl Memory { } pub fn load_word(&self, address: u64) -> Result { - if !address.is_multiple_of(4) { - return Err(MemoryError::UnalignedAccess); + if address.is_multiple_of(4) { + let bytes = self.cells.get(&address).cloned().unwrap_or_default(); + Ok(u32::from_le_bytes(bytes)) + } else { + address.checked_add(3).ok_or(MemoryError::AddressOverflow)?; + Ok(u32::from_le_bytes([ + self.load_byte(address), + self.load_byte(address + 1), + self.load_byte(address + 2), + self.load_byte(address + 3), + ])) } - let bytes = self.cells.get(&address).cloned().unwrap_or_default(); - Ok(u32::from_le_bytes(bytes)) } pub fn store_word(&mut self, address: u64, value: u32) -> Result<(), MemoryError> { - if !address.is_multiple_of(4) { - return Err(MemoryError::UnalignedAccess); - } let bytes = value.to_le_bytes(); - self.cells.insert(address, bytes); + if address.is_multiple_of(4) { + self.cells.insert(address, bytes); + } else { + address.checked_add(3).ok_or(MemoryError::AddressOverflow)?; + for (i, b) in bytes.iter().enumerate() { + self.store_byte(address + i as u64, *b); + } + } Ok(()) } /// Load a doubleword (64-bit) from memory - for LD instruction pub fn load_doubleword(&self, address: u64) -> Result { - if !address.is_multiple_of(8) { - return Err(MemoryError::UnalignedAccess); + if address.is_multiple_of(8) { + // 8-alignment bounds `address` to `u64::MAX - 7`, so `address + 4` can't overflow. + let low_bytes = self.cells.get(&address).cloned().unwrap_or_default(); + let high_bytes = self.cells.get(&(address + 4)).cloned().unwrap_or_default(); + let low = u32::from_le_bytes(low_bytes) as u64; + let high = u32::from_le_bytes(high_bytes) as u64; + Ok(low | (high << 32)) + } else { + address.checked_add(7).ok_or(MemoryError::AddressOverflow)?; + let mut bytes = [0u8; 8]; + for (i, b) in bytes.iter_mut().enumerate() { + *b = self.load_byte(address + i as u64); + } + Ok(u64::from_le_bytes(bytes)) } - let low_bytes = self.cells.get(&address).cloned().unwrap_or_default(); - let high_bytes = self.cells.get(&(address + 4)).cloned().unwrap_or_default(); - let low = u32::from_le_bytes(low_bytes) as u64; - let high = u32::from_le_bytes(high_bytes) as u64; - Ok(low | (high << 32)) } /// Store a doubleword (64-bit) to memory - for SD instruction pub fn store_doubleword(&mut self, address: u64, value: u64) -> Result<(), MemoryError> { - if !address.is_multiple_of(8) { - return Err(MemoryError::UnalignedAccess); + if address.is_multiple_of(8) { + let low = (value & 0xFFFFFFFF) as u32; + let high = (value >> 32) as u32; + // 8-alignment bounds `address` to `u64::MAX - 7`, so `address + 4` can't overflow. + self.cells.insert(address, low.to_le_bytes()); + self.cells.insert(address + 4, high.to_le_bytes()); + } else { + address.checked_add(7).ok_or(MemoryError::AddressOverflow)?; + let bytes = value.to_le_bytes(); + for (i, b) in bytes.iter().enumerate() { + self.store_byte(address + i as u64, *b); + } } - let low = (value & 0xFFFFFFFF) as u32; - let high = (value >> 32) as u32; - self.cells.insert(address, low.to_le_bytes()); - self.cells.insert(address + 4, high.to_le_bytes()); Ok(()) } pub fn load_half(&self, address: u64) -> Result { - if !address.is_multiple_of(2) { - unimplemented!( - "Unaligned load half memory access at address 0x{:016x}", - address - ); + if address.is_multiple_of(2) { + let aligned_address = address - address % 4; + let bytes = self + .cells + .get(&aligned_address) + .cloned() + .unwrap_or_default(); + let offset = (address % 4) as usize; + Ok(u16::from_le_bytes([bytes[offset], bytes[offset + 1]])) + } else { + address.checked_add(1).ok_or(MemoryError::AddressOverflow)?; + Ok(u16::from_le_bytes([ + self.load_byte(address), + self.load_byte(address + 1), + ])) } - let aligned_address = address - address % 4; - let bytes = self - .cells - .get(&aligned_address) - .cloned() - .unwrap_or_default(); - let value = &bytes[(address % 4) as usize..(address % 4) as usize + 2]; - Ok(u16::from_le_bytes( - value.try_into().map_err(|_| MemoryError::LoadHalf)?, - )) } pub fn store_half(&mut self, address: u64, value: u16) -> Result<(), MemoryError> { - if !address.is_multiple_of(2) { - return Err(MemoryError::UnalignedAccess); - } - let aligned_address = address - address % 4; - let entry = self - .cells - .entry(aligned_address) - .or_insert_with(|| [0, 0, 0, 0]); let bytes = value.to_le_bytes(); - entry[(address % 4) as usize] = bytes[0]; - entry[(address % 4) as usize + 1] = bytes[1]; + if address.is_multiple_of(2) { + let aligned_address = address - address % 4; + let entry = self + .cells + .entry(aligned_address) + .or_insert_with(|| [0, 0, 0, 0]); + let offset = (address % 4) as usize; + entry[offset] = bytes[0]; + entry[offset + 1] = bytes[1]; + } else { + address.checked_add(1).ok_or(MemoryError::AddressOverflow)?; + self.store_byte(address, bytes[0]); + self.store_byte(address + 1, bytes[1]); + } Ok(()) } @@ -233,8 +261,6 @@ impl Memory { #[derive(thiserror::Error, Debug)] pub enum MemoryError { - #[error("Failed to convert bytes to u16")] - LoadHalf, #[error("Unaligned memory access")] UnalignedAccess, #[error("Public output commit size exceeded")] diff --git a/executor/tests/asm.rs b/executor/tests/asm.rs index 58d4ef0ad..e9c9c08dd 100644 --- a/executor/tests/asm.rs +++ b/executor/tests/asm.rs @@ -1,4 +1,7 @@ -use executor::{elf::Elf, vm::execution::Executor}; +use executor::{ + elf::Elf, + vm::execution::{Executor, ExecutorError}, +}; /// Run a program and verify it exits successfully (exit code 0). /// @@ -424,12 +427,70 @@ fn test_lw_sw_offset() { run_program("./program_artifacts/asm/lw_sw_offset.elf"); } -#[ignore = "Unaligned memory access not properly implemented yet"] #[test] fn test_lw_sw_offset_odd() { run_program("./program_artifacts/asm/lw_sw_offset_odd.elf"); } +#[test] +fn test_misalign_lh() { + run_program("./program_artifacts/asm/misalign_lh.elf"); +} + +#[test] +fn test_misalign_lhu() { + run_program("./program_artifacts/asm/misalign_lhu.elf"); +} + +#[test] +fn test_misalign_lw() { + run_program("./program_artifacts/asm/misalign_lw.elf"); +} + +#[test] +fn test_misalign_lwu() { + run_program("./program_artifacts/asm/misalign_lwu.elf"); +} + +#[test] +fn test_misalign_ld() { + run_program("./program_artifacts/asm/misalign_ld.elf"); +} + +#[test] +fn test_misalign_sh() { + run_program("./program_artifacts/asm/misalign_sh.elf"); +} + +#[test] +fn test_misalign_sw() { + run_program("./program_artifacts/asm/misalign_sw.elf"); +} + +#[test] +fn test_misalign_sd() { + run_program("./program_artifacts/asm/misalign_sd.elf"); +} + +#[test] +fn test_misaligned_pc_traps() { + let elf_data = std::fs::read("./program_artifacts/asm/misaligned_pc.elf").unwrap(); + let program = Elf::load(&elf_data).unwrap(); + let mut executor = Executor::new(&program, vec![]).expect("Failed to create executor"); + let err = loop { + match executor.resume() { + Ok(Some(_)) => continue, + Ok(None) => panic!("expected misaligned PC trap, program halted normally"), + Err(e) => break e, + } + }; + assert!( + matches!(err, ExecutorError::InstructionAddressMisaligned(2)), + "expected InstructionAddressMisaligned(2), got {:?}", + err + ); +} + #[test] fn test_auipc() { run_program("./program_artifacts/asm/auipc.elf"); diff --git a/prover/src/tests/prove_elfs_tests.rs b/prover/src/tests/prove_elfs_tests.rs index 1cc1de8af..95127b5e6 100644 --- a/prover/src/tests/prove_elfs_tests.rs +++ b/prover/src/tests/prove_elfs_tests.rs @@ -498,6 +498,97 @@ fn test_prove_elfs_sign_ext_edge_cases_8() { ); } +// Misaligned load/store regression tests. Each program issues one load or +// store whose effective address is not naturally aligned to the access width, +// crossing one or more 4-byte cell boundaries in the executor's memory map. +#[test] +fn test_prove_elfs_misalign_lh() { + let (elf, logs, _instructions) = run_asm_elf("misalign_lh"); + let mut traces = + Traces::from_elf_and_logs_minimal(&elf, &logs, &Default::default(), &[]).unwrap(); + assert!( + prove_and_verify_vm_minimal(&elf, &mut traces), + "misalign_lh failed" + ); +} + +#[test] +fn test_prove_elfs_misalign_lhu() { + let (elf, logs, _instructions) = run_asm_elf("misalign_lhu"); + let mut traces = + Traces::from_elf_and_logs_minimal(&elf, &logs, &Default::default(), &[]).unwrap(); + assert!( + prove_and_verify_vm_minimal(&elf, &mut traces), + "misalign_lhu failed" + ); +} + +#[test] +fn test_prove_elfs_misalign_lw() { + let (elf, logs, _instructions) = run_asm_elf("misalign_lw"); + let mut traces = + Traces::from_elf_and_logs_minimal(&elf, &logs, &Default::default(), &[]).unwrap(); + assert!( + prove_and_verify_vm_minimal(&elf, &mut traces), + "misalign_lw failed" + ); +} + +#[test] +fn test_prove_elfs_misalign_lwu() { + let (elf, logs, _instructions) = run_asm_elf("misalign_lwu"); + let mut traces = + Traces::from_elf_and_logs_minimal(&elf, &logs, &Default::default(), &[]).unwrap(); + assert!( + prove_and_verify_vm_minimal(&elf, &mut traces), + "misalign_lwu failed" + ); +} + +#[test] +fn test_prove_elfs_misalign_ld() { + let (elf, logs, _instructions) = run_asm_elf("misalign_ld"); + let mut traces = + Traces::from_elf_and_logs_minimal(&elf, &logs, &Default::default(), &[]).unwrap(); + assert!( + prove_and_verify_vm_minimal(&elf, &mut traces), + "misalign_ld failed" + ); +} + +#[test] +fn test_prove_elfs_misalign_sh() { + let (elf, logs, _instructions) = run_asm_elf("misalign_sh"); + let mut traces = + Traces::from_elf_and_logs_minimal(&elf, &logs, &Default::default(), &[]).unwrap(); + assert!( + prove_and_verify_vm_minimal(&elf, &mut traces), + "misalign_sh failed" + ); +} + +#[test] +fn test_prove_elfs_misalign_sw() { + let (elf, logs, _instructions) = run_asm_elf("misalign_sw"); + let mut traces = + Traces::from_elf_and_logs_minimal(&elf, &logs, &Default::default(), &[]).unwrap(); + assert!( + prove_and_verify_vm_minimal(&elf, &mut traces), + "misalign_sw failed" + ); +} + +#[test] +fn test_prove_elfs_misalign_sd() { + let (elf, logs, _instructions) = run_asm_elf("misalign_sd"); + let mut traces = + Traces::from_elf_and_logs_minimal(&elf, &logs, &Default::default(), &[]).unwrap(); + assert!( + prove_and_verify_vm_minimal(&elf, &mut traces), + "misalign_sd failed" + ); +} + // MULW where the 32-bit product overflows past bit 31. #[test] fn test_prove_elfs_mulw_overflow() {