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

chore: Add useful information to certain Error variants #393

Merged
merged 2 commits into from
Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions definitions/src/asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,6 @@ pub struct InvokeData {
pub fixed_trace_mask: u64,
}

// Although the memory here is an array, but when it is created,
// its size is allocated through memory_size, and its maximum length RISCV_MAX_MEMORY
// is used in the structure declaration.
#[repr(C)]
pub struct AsmCoreMachine {
pub registers: [u64; RISCV_GENERAL_REGISTER_NUMBER],
Expand All @@ -94,6 +91,8 @@ pub struct AsmCoreMachine {
pub isa: u8,
pub version: u32,

pub error_arg0: u64,

pub memory_size: u64,
pub frames_size: u64,
pub flags_size: u64,
Expand Down Expand Up @@ -149,6 +148,7 @@ impl AsmCoreMachine {
}
machine.chaos_seed = 0;
machine.reset_signal = 0;
machine.error_arg0 = 0;
machine.version = version;
machine.isa = isa;

Expand Down
4 changes: 4 additions & 0 deletions definitions/src/generate_asm_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@ fn main() {
"#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_LOAD_RESERVATION_ADDRESS {}",
(&m.load_reservation_address as *const u64 as usize) - m_address
);
println!(
"#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_ERROR_ARG0 {}",
(&m.error_arg0 as *const u64 as usize) - m_address
);
println!(
"#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_MEMORY_SIZE {}",
(&m.memory_size as *const u64 as usize) - m_address
Expand Down
3 changes: 2 additions & 1 deletion src/decoder.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use ckb_vm_definitions::instructions::{self as insts};
use ckb_vm_definitions::registers::{RA, ZERO};

use crate::error::OutOfBoundKind;
use crate::instructions::{
a, b, extract_opcode, i, instruction_length, m, rvc, set_instruction_length_n, Instruction,
InstructionFactory, Itype, R4type, R5type, Register, Rtype, Utype,
Expand Down Expand Up @@ -99,7 +100,7 @@ impl Decoder {
// 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);
return Err(Error::MemOutOfBound(pc, OutOfBoundKind::Memory));
}
let instruction_cache_key = {
// according to RISC-V instruction encoding, the lowest bit in PC will always be zero
Expand Down
14 changes: 9 additions & 5 deletions src/elf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ pub use goblin_v023::elf::program_header::{PF_R, PF_W, PF_X, PT_LOAD};
pub use goblin_v023::elf::section_header::SHF_EXECINSTR;

/// Converts goblin's ELF flags into RISC-V flags
pub fn convert_flags(p_flags: u32, allow_freeze_writable: bool) -> Result<u8, Error> {
pub fn convert_flags(p_flags: u32, allow_freeze_writable: bool, vaddr: u64) -> Result<u8, Error> {
let readable = p_flags & PF_R != 0;
let writable = p_flags & PF_W != 0;
let executable = p_flags & PF_X != 0;
if !readable {
return Err(Error::ElfSegmentUnreadable);
return Err(Error::ElfSegmentUnreadable(vaddr));
}
if writable && executable {
return Err(Error::ElfSegmentWritableAndExecutable);
return Err(Error::ElfSegmentWritableAndExecutable(vaddr));
}
if executable {
Ok(FLAG_EXECUTABLE | FLAG_FREEZED)
Expand Down Expand Up @@ -188,12 +188,16 @@ pub fn parse_elf<R: Register>(program: &Bytes, version: u32) -> Result<ProgramMe
.p_offset
.wrapping_add(program_header.p_filesz);
if slice_start > slice_end || slice_end > program.len() as u64 {
return Err(Error::ElfSegmentAddrOrSizeError);
return Err(Error::ElfSegmentAddrOrSizeError(program_header.p_vaddr));
}
actions.push(LoadingAction {
addr: aligned_start,
size,
flags: convert_flags(program_header.p_flags, version < VERSION1)?,
flags: convert_flags(
program_header.p_flags,
version < VERSION1,
program_header.p_vaddr,
)?,
source: slice_start..slice_end,
offset_from_addr: padding_start,
});
Expand Down
39 changes: 23 additions & 16 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,15 @@ pub enum Error {
ElfBits,
#[display(fmt = "elf error: {}", "_0")]
ElfParseError(String),
#[display(fmt = "elf error: segment is unreadable")]
ElfSegmentUnreadable,
#[display(fmt = "elf error: segment is writable and executable")]
ElfSegmentWritableAndExecutable,
#[display(fmt = "elf error: segment addr or size is wrong")]
ElfSegmentAddrOrSizeError,
#[display(fmt = "elf error: segment is unreadable vaddr=0x{:x}", "_0")]
ElfSegmentUnreadable(u64),
#[display(
fmt = "elf error: segment is writable and executable vaddr=0x{:x}",
"_0"
)]
ElfSegmentWritableAndExecutable(u64),
#[display(fmt = "elf error: segment addr or size is wrong vaddr=0x{:x}", "_0")]
ElfSegmentAddrOrSizeError(u64),
// External error type is for the debugging tool of CKB-VM, it should not be
// used in this project.
#[display(fmt = "external error: {}", "_0")]
Expand All @@ -37,22 +40,26 @@ pub enum Error {
kind: std::io::ErrorKind,
data: String,
},
#[display(fmt = "memory error: out of bound")]
MemOutOfBound,
#[display(fmt = "memory error: out of bound addr=0x{:x}, kind={:?}", "_0", "_1")]
MemOutOfBound(u64, OutOfBoundKind),
#[display(fmt = "memory error: out of stack")]
MemOutOfStack,
#[display(fmt = "memory error: unaligned page access")]
MemPageUnalignedAccess,
#[display(fmt = "memory error: write on executable page")]
MemWriteOnExecutablePage,
#[display(fmt = "memory error: write on freezed page")]
MemWriteOnFreezedPage,
#[display(fmt = "memory error: unaligned page access addr=0x{:x}", "_0")]
MemPageUnalignedAccess(u64),
#[display(fmt = "memory error: write on executable page page_index={}", "_0")]
MemWriteOnExecutablePage(u64),
#[display(fmt = "memory error: write on freezed page page_index={}", "_0")]
MemWriteOnFreezedPage(u64),
#[display(fmt = "pause")]
Pause,
#[display(fmt = "unexpected error")]
Unexpected(String),
#[display(fmt = "unimplemented")]
Unimplemented,
}

#[derive(Debug, PartialEq, Clone, Eq, Display)]
pub enum OutOfBoundKind {
Memory,
ExternalData,
}

impl std::error::Error for Error {}
Expand Down
7 changes: 4 additions & 3 deletions src/instructions/common.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use super::super::error::OutOfBoundKind;
use super::super::machine::Machine;
use super::super::memory::Memory;
use super::register::Register;
Expand Down Expand Up @@ -88,9 +89,9 @@ fn check_load_boundary<R: Register>(
) -> Result<(), Error> {
if version0 {
let address = address.to_u64();
let end = address.checked_add(bytes).ok_or(Error::MemOutOfBound)?;
if end == memory_size {
return Err(Error::MemOutOfBound);
let (end, overflow) = address.overflowing_add(bytes);
if overflow || end == memory_size {
return Err(Error::MemOutOfBound(end, OutOfBoundKind::Memory));
}
}
Ok(())
Expand Down
17 changes: 9 additions & 8 deletions src/machine/asm/cdefinitions_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,15 @@
#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_CHAOS_MODE 296
#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_CHAOS_SEED 300
#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_LOAD_RESERVATION_ADDRESS 304
#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_MEMORY_SIZE 320
#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_FRAMES_SIZE 328
#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_FLAGS_SIZE 336
#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_LAST_READ_FRAME 344
#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_LAST_WRITE_PAGE 352
#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_MEMORY_PTR 360
#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_FLAGS_PTR 368
#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_FRAMES_PTR 376
#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_ERROR_ARG0 320
#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_MEMORY_SIZE 328
#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_FRAMES_SIZE 336
#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_FLAGS_SIZE 344
#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_LAST_READ_FRAME 352
#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_LAST_WRITE_PAGE 360
#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_MEMORY_PTR 368
#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_FLAGS_PTR 376
#define CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_FRAMES_PTR 384

#define CKB_VM_ASM_OP_UNLOADED 16
#define CKB_VM_ASM_OP_ADD 17
Expand Down
34 changes: 17 additions & 17 deletions src/machine/asm/execute_aarch64.S
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,12 @@
cmp TEMP1, TEMP2 SEP \
beq 2f SEP \
3: \
mov TEMP1, address_reg SEP \
mov TEMP3, address_reg SEP \
ldr TEMP2, [MACHINE, CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_MEMORY_SIZE] SEP \
cmp TEMP1, TEMP2 SEP \
cmp TEMP3, TEMP2 SEP \
bhs .exit_out_of_bound SEP \
add TEMP1, TEMP1, length SEP \
cmp TEMP1, TEMP2 SEP \
add TEMP3, TEMP3, length SEP \
cmp TEMP3, TEMP2 SEP \
bhs .exit_out_of_bound SEP \
_CHECK_READ_FRAMES(address_reg, length)

Expand All @@ -200,23 +200,22 @@
cmp TEMP1, TEMP2 SEP \
beq 2f SEP \
3: \
mov TEMP1, address_reg SEP \
mov TEMP3, address_reg SEP \
ldr TEMP2, [MACHINE, CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_MEMORY_SIZE] SEP \
cmp TEMP1, TEMP2 SEP \
cmp TEMP3, TEMP2 SEP \
bhs .exit_out_of_bound SEP \
add TEMP1, TEMP1, length SEP \
cmp TEMP1, TEMP2 SEP \
add TEMP3, TEMP3, length SEP \
cmp TEMP3, TEMP2 SEP \
bhi .exit_out_of_bound SEP \
_CHECK_READ_FRAMES(address_reg, length)

#define CHECK_WRITE(address_reg, length) \
mov TEMP1, address_reg SEP \
lsr TEMP1, TEMP1, CKB_VM_ASM_RISCV_PAGE_SHIFTS SEP \
mov TEMP3, address_reg SEP \
lsr TEMP1, TEMP3, CKB_VM_ASM_RISCV_PAGE_SHIFTS SEP \
ldr TEMP2, [MACHINE, CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_LAST_WRITE_PAGE] SEP \
cmp TEMP1, TEMP2 SEP \
bne 3f SEP \
mov TEMP2, address_reg SEP \
add TEMP2, TEMP2, length SEP \
add TEMP2, TEMP3, length SEP \
lsr TEMP2, TEMP2, CKB_VM_ASM_RISCV_PAGE_SHIFTS SEP \
cmp TEMP1, TEMP2 SEP \
beq 2f SEP \
Expand Down Expand Up @@ -248,12 +247,11 @@
CALL_INITED_MEMORY SEP \
POSTCALL SEP \
1: \
mov TEMP1, TEMP2 SEP \
add TEMP1, TEMP1, 1 SEP \
add TEMP1, TEMP2, 1 SEP \
lsl TEMP1, TEMP1, CKB_VM_ASM_RISCV_PAGE_SHIFTS SEP \
mov TEMP2, address_reg SEP \
add TEMP2, TEMP2, length SEP \
cmp TEMP2, TEMP1 SEP \
mov TEMP3, address_reg SEP \
add TEMP3, TEMP3, length SEP \
cmp TEMP3, TEMP1 SEP \
bls 2f SEP \
lsr TEMP1, TEMP1, CKB_VM_ASM_RISCV_PAGE_SHIFTS SEP \
ldr TEMP2, [MACHINE, CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_FLAGS_SIZE] SEP \
Expand Down Expand Up @@ -1935,9 +1933,11 @@ ckb_vm_x64_execute:
mov x0, CKB_VM_ASM_RET_CYCLES_OVERFLOW
b .exit
.exit_out_of_bound:
str TEMP3, [MACHINE, CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_ERROR_ARG0]
mov x0, CKB_VM_ASM_RET_OUT_OF_BOUND
b .exit
.exit_invalid_permission:
str TEMP1, [MACHINE, CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_ERROR_ARG0]
mov x0, CKB_VM_ASM_RET_INVALID_PERMISSION
b .exit
.exit_pause:
Expand Down
25 changes: 14 additions & 11 deletions src/machine/asm/execute_x64.S
Original file line number Diff line number Diff line change
Expand Up @@ -231,11 +231,11 @@
cmp TEMP2, TEMP1; \
je 2f; \
3: ; \
movq address_reg, TEMP1; \
cmp MEMORY_SIZE, TEMP1; \
movq address_reg, TEMP3; \
cmp MEMORY_SIZE, TEMP3; \
jae .exit_out_of_bound; \
addq $length, TEMP1; \
cmp MEMORY_SIZE, TEMP1; \
addq $length, TEMP3; \
cmp MEMORY_SIZE, TEMP3; \
jae .exit_out_of_bound; \
_CHECK_READ_FRAMES(address_reg, length)

Expand All @@ -251,11 +251,11 @@
cmp TEMP2, TEMP1; \
je 2f; \
3: ;\
movq address_reg, TEMP1; \
cmp MEMORY_SIZE, TEMP1; \
movq address_reg, TEMP3; \
cmp MEMORY_SIZE, TEMP3; \
jae .exit_out_of_bound; \
addq $length, TEMP1; \
cmp MEMORY_SIZE, TEMP1; \
addq $length, TEMP3; \
cmp MEMORY_SIZE, TEMP3; \
ja .exit_out_of_bound; \
_CHECK_READ_FRAMES(address_reg, length)

Expand All @@ -271,6 +271,7 @@
cmp TEMP2, TEMP1; \
je 2f;\
3:; \
movq address_reg, TEMP3; \
movq TEMP1, CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_LAST_WRITE_PAGE(MACHINE); \
movq CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_FLAGS_SIZE(MACHINE), TEMP2; \
cmp TEMP2, TEMP1; \
Expand Down Expand Up @@ -299,9 +300,9 @@
movq TEMP2, TEMP1; \
addq $1, TEMP1; \
shl $CKB_VM_ASM_RISCV_PAGE_SHIFTS, TEMP1; \
movq address_reg, TEMP2; \
addq $length, TEMP2; \
cmp TEMP1, TEMP2; \
movq address_reg, TEMP3; \
addq $length, TEMP3; \
cmp TEMP1, TEMP3; \
jbe 2f; \
shr $CKB_VM_ASM_RISCV_PAGE_SHIFTS, TEMP1; \
movq CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_FLAGS_SIZE(MACHINE), TEMP2; \
Expand Down Expand Up @@ -2396,6 +2397,7 @@ ckb_vm_x64_execute:
NEXT_INST
.p2align 3
.exit_out_of_bound:
mov TEMP3, CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_ERROR_ARG0(MACHINE)
mov $CKB_VM_ASM_RET_OUT_OF_BOUND, ARG_RETd
jmp .exit
.p2align 3
Expand All @@ -2408,6 +2410,7 @@ ckb_vm_x64_execute:
jmp .exit
.p2align 3
.exit_invalid_permission:
mov TEMP1, CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_ERROR_ARG0(MACHINE)
mov $CKB_VM_ASM_RET_INVALID_PERMISSION, ARG_RETd
jmp .exit
/*
Expand Down
Loading