Backport #363 (#364)

nervosnetwork · Jul 14, 2023 · b2dd21e · b2dd21e
1 parent 438c059
commit b2dd21e
Show file tree

Hide file tree

Showing 9 changed files with 55 additions and 30 deletions.
diff --git a/src/decoder.rs b/src/decoder.rs
@@ -7,7 +7,7 @@ use crate::instructions::{
 };
 use crate::machine::VERSION2;
 use crate::memory::Memory;
-use crate::{Error, ISA_A, ISA_B, ISA_MOP, RISCV_MAX_MEMORY, RISCV_PAGESIZE};
+use crate::{Error, ISA_A, ISA_B, ISA_MOP, RISCV_PAGESIZE};
 
 const RISCV_PAGESIZE_MASK: u64 = RISCV_PAGESIZE as u64 - 1;
 const INSTRUCTION_CACHE_SIZE: usize = 4096;
@@ -30,7 +30,7 @@ impl Decoder {
             factories: vec![],
             mop,
             version,
-            instructions_cache: vec![(RISCV_MAX_MEMORY as u64, 0); INSTRUCTION_CACHE_SIZE],
+            instructions_cache: vec![(u64::MAX as u64, 0); INSTRUCTION_CACHE_SIZE],
         }
     }
 
@@ -91,8 +91,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 u64::MAX as the default key in the instruction cache, have to check out of bound
+        // error first.
+        if pc as usize >= memory.memory_size() {
             return Err(Error::MemOutOfBound);
         }
         let instruction_cache_key = {
@@ -861,7 +862,7 @@ impl Decoder {
     }
 
     pub fn reset_instructions_cache(&mut self) {
-        self.instructions_cache = vec![(RISCV_MAX_MEMORY as u64, 0); INSTRUCTION_CACHE_SIZE];
+        self.instructions_cache = vec![(u64::MAX, 0); INSTRUCTION_CACHE_SIZE];
     }
 }
 

diff --git a/src/instructions/common.rs b/src/instructions/common.rs
@@ -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};
@@ -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);
         }
     }
@@ -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)));
@@ -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)));
@@ -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(())
@@ -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(())
@@ -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(())
@@ -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(())
@@ -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(())

diff --git a/src/machine/asm/mod.rs b/src/machine/asm/mod.rs
@@ -26,7 +26,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> {
@@ -113,7 +113,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)?;
@@ -124,7 +124,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)?;
@@ -144,7 +144,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)?;
@@ -154,7 +154,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)?;
@@ -172,7 +172,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);
@@ -181,7 +181,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);
@@ -212,9 +212,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);
@@ -244,15 +245,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();
@@ -263,7 +264,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();

diff --git a/src/memory/mod.rs b/src/memory/mod.rs
@@ -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>;

diff --git a/src/snapshot.rs b/src/snapshot.rs
@@ -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;

diff --git a/tests/programs/_build_all_native.sh b/tests/programs/_build_all_native.sh
@@ -5,6 +5,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

diff --git a/tests/programs/big_binary b/tests/programs/big_binary
diff --git a/tests/programs/big_binary.c b/tests/programs/big_binary.c
@@ -0,0 +1,5 @@
+const char array[1024*1024] = {1};
+
+int main() {
+    return 0;
+}
diff --git a/tests/test_asm.rs b/tests/test_asm.rs
@@ -2,7 +2,7 @@
 use ckb_vm::cost_model::constant_cycles;
 use ckb_vm::decoder::build_decoder;
 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};
@@ -435,3 +435,13 @@ fn test_zero_address() {
     assert!(result.is_ok());
     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));
+}