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

Pause ckb-vm by signal #348

Merged
merged 12 commits into from
Jun 7, 2023
1 change: 1 addition & 0 deletions definitions/src/asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ pub const RET_CYCLES_OVERFLOW: u8 = 6;
pub const RET_OUT_OF_BOUND: u8 = 7;
pub const RET_INVALID_PERMISSION: u8 = 8;
pub const RET_SLOWPATH: u8 = 9;
pub const RET_PAUSE: u8 = 10;

#[inline(always)]
pub fn calculate_slot(addr: u64) -> usize {
Expand Down
5 changes: 3 additions & 2 deletions definitions/src/generate_asm_constants.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use ckb_vm_definitions::{
asm::{
AsmCoreMachine, Trace, RET_CYCLES_OVERFLOW, RET_DECODE_TRACE, RET_DYNAMIC_JUMP, RET_EBREAK,
RET_ECALL, RET_INVALID_PERMISSION, RET_MAX_CYCLES_EXCEEDED, RET_OUT_OF_BOUND, RET_SLOWPATH,
TRACE_ITEM_LENGTH,
RET_ECALL, RET_INVALID_PERMISSION, RET_MAX_CYCLES_EXCEEDED, RET_OUT_OF_BOUND, RET_PAUSE,
RET_SLOWPATH, TRACE_ITEM_LENGTH,
},
instructions::{
instruction_opcode_name, Instruction, INSTRUCTION_OPCODE_NAMES, MAXIMUM_OPCODE,
Expand Down Expand Up @@ -64,6 +64,7 @@ fn main() {
RET_INVALID_PERMISSION
);
println!("#define CKB_VM_ASM_RET_SLOWPATH {}", RET_SLOWPATH);
println!("#define CKB_VM_ASM_RET_PAUSE {}", RET_PAUSE);
println!();

println!("#define CKB_VM_ASM_REGISTER_RA {}", RA);
Expand Down
2 changes: 2 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ pub enum Error {
MemWriteOnExecutablePage,
#[display(fmt = "memory error: write on freezed page")]
MemWriteOnFreezedPage,
#[display(fmt = "pause")]
Pause,
#[display(fmt = "unexpected error")]
Unexpected(String),
#[display(fmt = "unimplemented")]
Expand Down
1 change: 1 addition & 0 deletions src/machine/asm/cdefinitions_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#define CKB_VM_ASM_RET_OUT_OF_BOUND 7
#define CKB_VM_ASM_RET_INVALID_PERMISSION 8
#define CKB_VM_ASM_RET_SLOWPATH 9
#define CKB_VM_ASM_RET_PAUSE 10

#define CKB_VM_ASM_REGISTER_RA 1
#define CKB_VM_ASM_REGISTER_SP 2
Expand Down
21 changes: 15 additions & 6 deletions src/machine/asm/execute_aarch64.S
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#define UINT64_MAX 0xffffffffffffffff

#define MACHINE x0
#define PAUSE x1

#define TEMP1 x9
#define TEMP1w w9
Expand Down Expand Up @@ -52,14 +53,16 @@
#endif

#define PREPCALL \
stp x0, x9, [sp, -48]! SEP \
xxuejie marked this conversation as resolved.
Show resolved Hide resolved
stp x10, x11, [sp, 16] SEP \
stp x12, x13, [sp, 32]
stp x0, x1, [sp, -64]! SEP \
stp x9, x10, [sp, 16] SEP \
stp x11, x12, [sp, 32] SEP \
stp x13, x14, [sp, 48] SEP \

#define POSTCALL \
ldp x12, x13, [sp, 32] SEP \
ldp x10, x11, [sp, 16] SEP \
ldp x0, x9, [sp], 48
ldp x13, x14, [sp, 48] SEP \
ldp x11, x12, [sp, 32] SEP \
ldp x9, x10, [sp, 16] SEP \
ldp x0, x1, [sp], 64

#define REGISTER_ADDRESS(r) [REGISTER_BASE, r, lsl 3]
#define ZERO_ADDRESS [REGISTER_BASE]
Expand Down Expand Up @@ -336,6 +339,9 @@ ckb_vm_x64_execute:
add INST_PC, TRACE, CKB_VM_ASM_TRACE_OFFSET_THREAD
NEXT_INST
.prepare_trace:
ldarb TEMP2w, [PAUSE]
cmp TEMP2, ZERO_VALUE
bne .exit_pause
ldr TEMP2, [MACHINE, CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_PC]
mov TEMP3, TEMP2
lsr TEMP2, TEMP2, 2
Expand Down Expand Up @@ -1951,6 +1957,9 @@ ckb_vm_x64_execute:
.exit_invalid_permission:
mov x0, CKB_VM_ASM_RET_INVALID_PERMISSION
b .exit
.exit_pause:
mov x0, CKB_VM_ASM_RET_PAUSE
b .exit
.exit_trace:
.CKB_VM_ASM_LABEL_OP_UNLOADED:
DECODE_U
Expand Down
11 changes: 11 additions & 0 deletions src/machine/asm/execute_x64.S
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
#define MACHINE %rsi
#define TRACE %rbx
#define MEMORY_SIZE %r13
#define PAUSE %r14

/*
* INST_PC contains the current address of decoded Instruction in
Expand Down Expand Up @@ -405,6 +406,8 @@ ckb_vm_x64_execute:
push %rbx
push %r12
push %r13
push %r14
mov ARG2, PAUSE
mov ARG1, MACHINE
movq CKB_VM_ASM_ASM_CORE_MACHINE_OFFSET_MEMORY_SIZE(MACHINE), MEMORY_SIZE
.p2align 3
Expand Down Expand Up @@ -440,6 +443,9 @@ ckb_vm_x64_execute:
NEXT_INST
.p2align 3
.prepare_trace:
movzbl 0(PAUSE), %eax
cmp $0, %rax
jnz .exit_pause
movq PC_ADDRESS, %rax
mov %eax, %ecx
shr $2, %eax
Expand Down Expand Up @@ -2349,12 +2355,17 @@ ckb_vm_x64_execute:
mov $CKB_VM_ASM_RET_SLOWPATH, ARG_RETd
jmp .exit
.p2align 3
.exit_pause:
mov $CKB_VM_ASM_RET_PAUSE, ARG_RETd
jmp .exit
.p2align 3
.exit_trace:
.CKB_VM_ASM_LABEL_OP_UNLOADED:
DECODE_U
mov $CKB_VM_ASM_RET_DECODE_TRACE, ARG_RETd
jmp .exit
.exit:
pop %r14
pop %r13
pop %r12
pop %rbx
Expand Down
24 changes: 19 additions & 5 deletions src/machine/asm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ pub use ckb_vm_definitions::asm::AsmCoreMachine;
use ckb_vm_definitions::{
asm::{
calculate_slot, Trace, RET_CYCLES_OVERFLOW, RET_DECODE_TRACE, RET_DYNAMIC_JUMP, RET_EBREAK,
RET_ECALL, RET_INVALID_PERMISSION, RET_MAX_CYCLES_EXCEEDED, RET_OUT_OF_BOUND, RET_SLOWPATH,
TRACE_ITEM_LENGTH, TRACE_SIZE,
RET_ECALL, RET_INVALID_PERMISSION, RET_MAX_CYCLES_EXCEEDED, RET_OUT_OF_BOUND, RET_PAUSE,
RET_SLOWPATH, TRACE_ITEM_LENGTH, TRACE_SIZE,
},
instructions::OP_CUSTOM_TRACE_END,
ISA_MOP, MEMORY_FRAMES, MEMORY_FRAME_PAGE_SHIFTS, RISCV_GENERAL_REGISTER_NUMBER,
Expand Down Expand Up @@ -456,7 +456,7 @@ impl SupportMachine for Box<AsmCoreMachine> {
}

extern "C" {
pub fn ckb_vm_x64_execute(m: *mut AsmCoreMachine) -> c_uchar;
pub fn ckb_vm_x64_execute(m: *mut AsmCoreMachine, s: *mut u8) -> c_uchar;
// We are keeping this as a function here, but at the bottom level this really
// just points to an array of assembly label offsets for each opcode.
pub fn ckb_vm_asm_labels();
Expand Down Expand Up @@ -489,7 +489,12 @@ impl AsmMachine {
if self.machine.reset_signal() {
decoder.reset_instructions_cache();
}
let result = unsafe { ckb_vm_x64_execute(&mut **self.machine.inner_mut()) };
let result = unsafe {
ckb_vm_x64_execute(
&mut **self.machine.inner_mut(),
self.machine.pause.get_raw_ptr(),
)
};
match result {
RET_DECODE_TRACE => {
let pc = *self.machine.pc();
Expand Down Expand Up @@ -538,6 +543,10 @@ impl AsmMachine {
let instruction = decoder.decode(self.machine.memory_mut(), pc)?;
execute_instruction(instruction, &mut self.machine)?;
}
RET_PAUSE => {
self.machine.pause.free();
return Err(Error::Pause);
}
_ => return Err(Error::Asm(result)),
}
}
Expand Down Expand Up @@ -567,7 +576,12 @@ impl AsmMachine {
trace.length = len;
self.machine.inner_mut().traces[slot] = trace;

let result = unsafe { ckb_vm_x64_execute(&mut (**self.machine.inner_mut())) };
let result = unsafe {
ckb_vm_x64_execute(
&mut (**self.machine.inner_mut()),
self.machine.pause.get_raw_ptr(),
)
};
match result {
RET_DECODE_TRACE => (),
RET_ECALL => self.machine.ecall()?,
Expand Down
53 changes: 53 additions & 0 deletions src/machine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ pub mod elf_adaptor;
pub mod trace;

use std::fmt::{self, Display};
use std::sync::atomic::{AtomicU8, Ordering};
use std::sync::Arc;

use bytes::Bytes;
use scroll::Pread;
Expand Down Expand Up @@ -425,6 +427,7 @@ pub type InstructionCycleFunc = dyn Fn(Instruction) -> u64 + Send + Sync;

pub struct DefaultMachine<Inner> {
inner: Inner,
pause: Pause,

// We have run benchmarks on secp256k1 verification, the performance
// cost of the Box wrapper here is neglectable, hence we are sticking
Expand Down Expand Up @@ -592,6 +595,10 @@ impl<Inner: SupportMachine> DefaultMachine<Inner> {
self.inner
}

pub fn pause(&self) -> Pause {
self.pause.clone()
}

pub fn exit_code(&self) -> i8 {
self.exit_code
}
Expand All @@ -615,6 +622,10 @@ impl<Inner: SupportMachine> DefaultMachine<Inner> {
let mut decoder = build_decoder::<Inner::REG>(self.isa(), self.version());
self.set_running(true);
while self.running() {
if self.pause.has_interrupted() {
self.pause.free();
return Err(Error::Pause);
}
if self.reset_signal() {
decoder.reset_instructions_cache();
}
Expand Down Expand Up @@ -673,10 +684,52 @@ impl<Inner> DefaultMachineBuilder<Inner> {
pub fn build(self) -> DefaultMachine<Inner> {
DefaultMachine {
inner: self.inner,
pause: Pause::new(),
instruction_cycle_func: self.instruction_cycle_func,
debugger: self.debugger,
syscalls: self.syscalls,
exit_code: 0,
}
}
}

#[derive(Clone, Default)]
pub struct Pause {
s: Arc<AtomicU8>,
}

impl Pause {
pub fn new() -> Self {
Self {
s: Arc::new(AtomicU8::new(0)),
}
}

pub fn interrupt(&self) {
self.s.store(1, Ordering::SeqCst);
}

pub fn has_interrupted(&self) -> bool {
self.s.load(Ordering::SeqCst) != 0
}

pub fn get_raw_ptr(&self) -> *mut u8 {
&*self.s as *const _ as *mut u8
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need get_mut here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot borrow data in an Arc as mutable.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that sense you will need Mutex to wrap on AtomicU8.

What I'm concerned here, is that we are casting a *AtomicU8 directly to a *u8, which means we are relying on the underlying layout that the u8 value lies at the first of the AtomicU8 structure. This is something that worries me.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can write a test case to guarantee the relationship between AtomicU8 and u8.

}

pub fn free(&mut self) {
self.s.store(0, Ordering::SeqCst);
}
}

#[cfg(test)]
mod tests {
use std::sync::atomic::AtomicU8;

#[test]
fn test_atomicu8() {
// Assert AtomicU8 type has the same in-memory representation as u8.
// This ensures that Pause::get_raw_ptr() works properly.
assert_eq!(std::mem::size_of::<AtomicU8>(), 1);
}
}
4 changes: 4 additions & 0 deletions src/machine/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ impl<Inner: SupportMachine> TraceMachine<Inner> {
// larger trace item length.
self.traces.resize_with(TRACE_SIZE, Trace::default);
while self.machine.running() {
if self.machine.pause.has_interrupted() {
self.machine.pause.free();
return Err(Error::Pause);
}
if self.machine.reset_signal() {
decoder.reset_instructions_cache();
for i in self.traces.iter_mut() {
Expand Down
2 changes: 2 additions & 0 deletions tests/programs/_build_all_native.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ riscv64-unknown-elf-as -o ckbforks.o ckbforks.S && riscv64-unknown-elf-ld -o ckb
# TODO: clzw_bug
# SKIP: decoder_instructions_cache_pc_out_of_bound_timeout
riscv64-unknown-elf-as -o ebreak.o ebreak.S && riscv64-unknown-elf-ld -o ebreak64 ebreak.o && rm ebreak.o
riscv64-unknown-elf-gcc -o fib30 fib30.c
riscv64-unknown-elf-gcc -o fib35 fib35.c
# SKIP: flat_crash_64
# SKIP: goblin_overflow_elf
# SKIP: invalid_file_offset64*
Expand Down
Binary file added tests/programs/fib30
Binary file not shown.
15 changes: 15 additions & 0 deletions tests/programs/fib30.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
int fib(int n) {
if (n <= 1) {
return n;
} else {
return fib(n - 1) + fib(n - 2);
}
}

int main() {
// A calculation problem, it takes about 2 seconds in interpreter.
if (fib(30) == 832040) {
return 0;
};
return 1;
}
Binary file added tests/programs/fib35
Binary file not shown.
15 changes: 15 additions & 0 deletions tests/programs/fib35.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
int fib(int n) {
if (n <= 1) {
return n;
} else {
return fib(n - 1) + fib(n - 2);
}
}

int main() {
// A calculation problem, it takes about 2 seconds in asm.
if (fib(35) == 9227465) {
return 0;
};
return 1;
}