Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use correct memory pages and size #363

Merged
merged 5 commits into from
Jul 14, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,9 @@ impl Decoder {
}

pub fn decode_raw<M: Memory>(&mut self, memory: &mut M, pc: u64) -> Result<Instruction, Error> {
// since we are using RISCV_MAX_MEMORY as the default key in the instruction cache, have to check out of bound error first
if pc as usize >= RISCV_MAX_MEMORY {
// since we are using RISCV_MAX_MEMORY as the default key in the instruction cache, have to check out of bound
// error first.
if pc as usize >= memory.memory_size() {
mohanson marked this conversation as resolved.
Show resolved Hide resolved
return Err(Error::MemOutOfBound);
}
let instruction_cache_key = {
Expand Down
24 changes: 14 additions & 10 deletions src/instructions/common.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use super::super::machine::Machine;
use super::super::memory::Memory;
use super::super::RISCV_MAX_MEMORY;
use super::register::Register;
use super::utils::update_register;
use super::{Error, RegisterIndex, SImmediate, UImmediate};
Expand Down Expand Up @@ -81,11 +80,16 @@ pub fn addiw<Mac: Machine>(
// =======================
// # LOAD instructions #
// =======================
fn check_load_boundary<R: Register>(version0: bool, address: &R, bytes: u64) -> Result<(), Error> {
fn check_load_boundary<R: Register>(
version0: bool,
address: &R,
bytes: u64,
memory_size: u64,
) -> Result<(), Error> {
if version0 {
let address = address.to_u64();
let end = address.checked_add(bytes).ok_or(Error::MemOutOfBound)?;
if end == RISCV_MAX_MEMORY as u64 {
if end == memory_size {
return Err(Error::MemOutOfBound);
}
}
Expand All @@ -100,7 +104,7 @@ pub fn lb<Mac: Machine>(
version0: bool,
) -> Result<(), Error> {
let address = machine.registers()[rs1 as usize].overflowing_add(&Mac::REG::from_i32(imm));
check_load_boundary(version0, &address, 1)?;
check_load_boundary(version0, &address, 1, machine.memory().memory_size() as u64)?;
let value = machine.memory_mut().load8(&address)?;
// sign-extened
update_register(machine, rd, value.sign_extend(&Mac::REG::from_u8(8)));
Expand All @@ -115,7 +119,7 @@ pub fn lh<Mac: Machine>(
version0: bool,
) -> Result<(), Error> {
let address = machine.registers()[rs1 as usize].overflowing_add(&Mac::REG::from_i32(imm));
check_load_boundary(version0, &address, 2)?;
check_load_boundary(version0, &address, 2, machine.memory().memory_size() as u64)?;
let value = machine.memory_mut().load16(&address)?;
// sign-extened
update_register(machine, rd, value.sign_extend(&Mac::REG::from_u8(16)));
Expand All @@ -130,7 +134,7 @@ pub fn lw<Mac: Machine>(
version0: bool,
) -> Result<(), Error> {
let address = machine.registers()[rs1 as usize].overflowing_add(&Mac::REG::from_i32(imm));
check_load_boundary(version0, &address, 4)?;
check_load_boundary(version0, &address, 4, machine.memory().memory_size() as u64)?;
let value = machine.memory_mut().load32(&address)?;
update_register(machine, rd, value.sign_extend(&Mac::REG::from_u8(32)));
Ok(())
Expand All @@ -144,7 +148,7 @@ pub fn ld<Mac: Machine>(
version0: bool,
) -> Result<(), Error> {
let address = machine.registers()[rs1 as usize].overflowing_add(&Mac::REG::from_i32(imm));
check_load_boundary(version0, &address, 8)?;
check_load_boundary(version0, &address, 8, machine.memory().memory_size() as u64)?;
let value = machine.memory_mut().load64(&address)?;
update_register(machine, rd, value.sign_extend(&Mac::REG::from_u8(64)));
Ok(())
Expand All @@ -158,7 +162,7 @@ pub fn lbu<Mac: Machine>(
version0: bool,
) -> Result<(), Error> {
let address = machine.registers()[rs1 as usize].overflowing_add(&Mac::REG::from_i32(imm));
check_load_boundary(version0, &address, 1)?;
check_load_boundary(version0, &address, 1, machine.memory().memory_size() as u64)?;
let value = machine.memory_mut().load8(&address)?;
update_register(machine, rd, value);
Ok(())
Expand All @@ -172,7 +176,7 @@ pub fn lhu<Mac: Machine>(
version0: bool,
) -> Result<(), Error> {
let address = machine.registers()[rs1 as usize].overflowing_add(&Mac::REG::from_i32(imm));
check_load_boundary(version0, &address, 2)?;
check_load_boundary(version0, &address, 2, machine.memory().memory_size() as u64)?;
let value = machine.memory_mut().load16(&address)?;
update_register(machine, rd, value);
Ok(())
Expand All @@ -186,7 +190,7 @@ pub fn lwu<Mac: Machine>(
version0: bool,
) -> Result<(), Error> {
let address = machine.registers()[rs1 as usize].overflowing_add(&Mac::REG::from_i32(imm));
check_load_boundary(version0, &address, 4)?;
check_load_boundary(version0, &address, 4, machine.memory().memory_size() as u64)?;
let value = machine.memory_mut().load32(&address)?;
update_register(machine, rd, value);
Ok(())
Expand Down
27 changes: 14 additions & 13 deletions src/machine/asm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use crate::{
FLAG_EXECUTABLE, FLAG_FREEZED, FLAG_WRITABLE, FLAG_WXORX_BIT,
},
CoreMachine, DefaultMachine, Error, Machine, Memory, SupportMachine, MEMORY_FRAME_SHIFTS,
RISCV_MAX_MEMORY, RISCV_PAGES, RISCV_PAGESIZE,
RISCV_PAGES, RISCV_PAGESIZE,
};

impl CoreMachine for Box<AsmCoreMachine> {
Expand Down Expand Up @@ -115,7 +115,7 @@ fn check_memory_writable(
) -> Result<(), Error> {
debug_assert!(size == 1 || size == 2 || size == 4 || size == 8);
let page = addr >> RISCV_PAGE_SHIFTS;
if page as usize >= RISCV_PAGES {
if page as usize >= machine.memory_pages() {
return Err(Error::MemOutOfBound);
}
check_permission(machine, page, FLAG_WRITABLE)?;
Expand All @@ -126,7 +126,7 @@ fn check_memory_writable(
let page_offset = addr as usize % RISCV_PAGESIZE;
if page_offset + size > RISCV_PAGESIZE {
let page = page + 1;
if page as usize >= RISCV_PAGES {
if page as usize >= machine.memory_pages() {
return Err(Error::MemOutOfBound);
} else {
check_permission(machine, page, FLAG_WRITABLE)?;
Expand All @@ -146,7 +146,7 @@ fn check_memory_executable(
debug_assert!(size == 2 || size == 4);

let page = addr >> RISCV_PAGE_SHIFTS;
if page as usize >= RISCV_PAGES {
if page as usize >= machine.memory_pages() {
return Err(Error::MemOutOfBound);
}
check_permission(machine, page, FLAG_EXECUTABLE)?;
Expand All @@ -156,7 +156,7 @@ fn check_memory_executable(
let page_offset = addr as usize % RISCV_PAGESIZE;
if page_offset + size > RISCV_PAGESIZE {
let page = page + 1;
if page as usize >= RISCV_PAGES {
if page as usize >= machine.memory_pages() {
return Err(Error::MemOutOfBound);
} else {
check_permission(machine, page, FLAG_EXECUTABLE)?;
Expand All @@ -174,7 +174,7 @@ fn check_memory_inited(
) -> Result<(), Error> {
debug_assert!(size == 1 || size == 2 || size == 4 || size == 8);
let page = addr >> RISCV_PAGE_SHIFTS;
if page as usize >= RISCV_PAGES {
if page as usize >= machine.memory_pages() {
return Err(Error::MemOutOfBound);
}
check_memory(machine, page);
Expand All @@ -183,7 +183,7 @@ fn check_memory_inited(
let page_offset = addr as usize % RISCV_PAGESIZE;
if page_offset + size > RISCV_PAGESIZE {
let page = page + 1;
if page as usize >= RISCV_PAGES {
if page as usize >= machine.memory_pages() {
return Err(Error::MemOutOfBound);
} else {
check_memory(machine, page);
Expand Down Expand Up @@ -349,9 +349,10 @@ impl Memory for Box<AsmCoreMachine> {
if round_page_down(addr) != addr || round_page_up(size) != size {
return Err(Error::MemPageUnalignedAccess);
}
if addr > RISCV_MAX_MEMORY as u64
|| size > RISCV_MAX_MEMORY as u64
|| addr + size > RISCV_MAX_MEMORY as u64
let memory_size = self.memory_size() as u64;
if addr > memory_size
|| size > memory_size as u64
|| addr + size > memory_size as u64
|| offset_from_addr > size
{
return Err(Error::MemOutOfBound);
Expand Down Expand Up @@ -381,15 +382,15 @@ impl Memory for Box<AsmCoreMachine> {
}

fn fetch_flag(&mut self, page: u64) -> Result<u8, Error> {
if page < RISCV_PAGES as u64 {
if page < self.memory_pages() as u64 {
Ok(self.flags[page as usize])
} else {
Err(Error::MemOutOfBound)
}
}

fn set_flag(&mut self, page: u64, flag: u8) -> Result<(), Error> {
if page < RISCV_PAGES as u64 {
if page < self.memory_pages() as u64 {
self.flags[page as usize] |= flag;
// Clear last write page cache
self.last_write_page = u64::max_value();
Expand All @@ -400,7 +401,7 @@ impl Memory for Box<AsmCoreMachine> {
}

fn clear_flag(&mut self, page: u64, flag: u8) -> Result<(), Error> {
if page < RISCV_PAGES as u64 {
if page < self.memory_pages() as u64 {
self.flags[page as usize] &= !flag;
// Clear last write page cache
self.last_write_page = u64::max_value();
Expand Down
3 changes: 3 additions & 0 deletions src/memory/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ pub trait Memory {
fn set_flag(&mut self, page: u64, flag: u8) -> Result<(), Error>;
fn clear_flag(&mut self, page: u64, flag: u8) -> Result<(), Error>;
fn memory_size(&self) -> usize;
fn memory_pages(&self) -> usize {
self.memory_size() >> RISCV_PAGE_SHIFTS
}

// This is in fact just memset
fn store_byte(&mut self, addr: u64, size: u64, value: u8) -> Result<(), Error>;
Expand Down
2 changes: 1 addition & 1 deletion src/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub fn make_snapshot<T: CoreMachine>(machine: &mut T) -> Result<Snapshot, Error>
snap.registers[i] = v.to_u64();
}

for i in 0..(machine.memory().memory_size() >> RISCV_PAGE_SHIFTS) {
for i in 0..machine.memory().memory_pages() {
let flag = machine.memory_mut().fetch_flag(i as u64)?;
if flag & FLAG_DIRTY != 0 {
let addr_from = i << RISCV_PAGE_SHIFTS;
Expand Down
4 changes: 2 additions & 2 deletions src/snapshot2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::{
elf::{LoadingAction, ProgramMetadata},
machine::SupportMachine,
memory::{Memory, FLAG_DIRTY},
Error, Register, RISCV_GENERAL_REGISTER_NUMBER, RISCV_PAGESIZE, RISCV_PAGE_SHIFTS,
Error, Register, RISCV_GENERAL_REGISTER_NUMBER, RISCV_PAGESIZE,
};
use bytes::Bytes;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -146,7 +146,7 @@ impl<I: Clone + PartialEq, D: DataSource<I>> Snapshot2Context<I, D> {
/// Create a snapshot for the passed machine.
pub fn make_snapshot<M: SupportMachine>(&self, machine: &mut M) -> Result<Snapshot2<I>, Error> {
let mut dirty_pages: Vec<(u64, u8, Vec<u8>)> = vec![];
for i in 0..(machine.memory().memory_size() as u64 >> RISCV_PAGE_SHIFTS) {
for i in 0..machine.memory().memory_pages() as u64 {
if self.pages.contains_key(&i) {
continue;
}
Expand Down
1 change: 1 addition & 0 deletions tests/programs/_build_all_native.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ riscv64-unknown-elf-as -o amo_compare.o amo_compare.S && riscv64-unknown-elf-ld
riscv64-unknown-elf-as -o amo_write_permission.o amo_write_permission.S && riscv64-unknown-elf-ld -o amo_write_permission amo_write_permission.o && rm amo_write_permission.o
# SKIP: andi
riscv64-unknown-elf-gcc -o argv_null_test argv_null_test.c
riscv64-unknown-elf-gcc -o big_binary big_binary.c
riscv64-unknown-elf-as -march=rv64imc -o cadd_hints.o cadd_hints.S && riscv64-unknown-elf-ld -o cadd_hints cadd_hints.o && rm cadd_hints.o
riscv64-unknown-elf-as -o ckbforks.o ckbforks.S && riscv64-unknown-elf-ld -o ckbforks ckbforks.o && rm ckbforks.o
# TODO: clzw_bug
Expand Down
Binary file added tests/programs/big_binary
Binary file not shown.
5 changes: 5 additions & 0 deletions tests/programs/big_binary.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const char array[1024*1024] = {1};

int main() {
return 0;
}
12 changes: 11 additions & 1 deletion tests/test_asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use ckb_vm::cost_model::constant_cycles;
use ckb_vm::decoder::build_decoder;
use ckb_vm::machine::asm::traces::{MemoizedDynamicTraceDecoder, MemoizedFixedTraceDecoder};
use ckb_vm::machine::asm::{AsmCoreMachine, AsmMachine};
use ckb_vm::machine::{CoreMachine, VERSION0, VERSION1};
use ckb_vm::machine::{CoreMachine, VERSION0, VERSION1, VERSION2};
use ckb_vm::memory::Memory;
use ckb_vm::registers::{A0, A1, A2, A3, A4, A5, A7};
use ckb_vm::{Debugger, DefaultMachineBuilder, Error, Register, SupportMachine, Syscalls, ISA_IMC};
Expand Down Expand Up @@ -476,3 +476,13 @@ fn test_memoized_dynamic_secp256k1() {
let result = machine.run_with_decoder(&mut decoder);
assert_eq!(result.unwrap(), 0);
}

#[test]
pub fn test_big_binary() {
let buffer = fs::read("tests/programs/big_binary").unwrap().into();
let asm_core = AsmCoreMachine::new_with_memory(ISA_IMC, VERSION2, u64::max_value(), 1024 * 512);
let core = DefaultMachineBuilder::new(asm_core).build();
let mut machine = AsmMachine::new(core);
let result = machine.load_program(&buffer, &vec!["simple".into()]);
assert_eq!(result, Err(Error::MemOutOfBound));
}