Skip to content

Commit

Permalink
Fix #37 reported by @pierrec
Browse files Browse the repository at this point in the history
  • Loading branch information
maximecb committed Nov 26, 2023
1 parent 8ab6e6e commit 32be919
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 29 deletions.
10 changes: 5 additions & 5 deletions vm/src/sys/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,10 +245,10 @@ fn memcpy(vm: &mut VM, dst_ptr: Value, src_ptr: Value, num_bytes: Value)

// TODO: panic if slices are overlapping

let dst_ptr: *mut u8 = vm.get_heap_ptr(dst_ptr);
let src_ptr: *mut u8 = vm.get_heap_ptr(src_ptr);

unsafe {
let dst_ptr: *mut u8 = vm.get_heap_ptr(dst_ptr, num_bytes);
let src_ptr: *mut u8 = vm.get_heap_ptr(src_ptr, num_bytes);

std::ptr::copy_nonoverlapping(src_ptr, dst_ptr, num_bytes);
}
}
Expand All @@ -258,8 +258,8 @@ fn memcmp(vm: &mut VM, ptr_a: Value, ptr_b: Value, num_bytes: Value) -> Value
let num_bytes = num_bytes.as_usize();

unsafe {
let ptr_a: *const libc::c_void = vm.get_heap_ptr(ptr_a.as_usize());
let ptr_b: *const libc::c_void = vm.get_heap_ptr(ptr_b.as_usize());
let ptr_a: *const libc::c_void = vm.get_heap_ptr(ptr_a.as_usize(), num_bytes);
let ptr_b: *const libc::c_void = vm.get_heap_ptr(ptr_b.as_usize(), num_bytes);

let result = libc::memcmp(ptr_a, ptr_b, num_bytes);

Expand Down
6 changes: 3 additions & 3 deletions vm/src/sys/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,8 @@ pub fn net_accept(
{
let socket_id = socket_id.as_u64();
let client_addr_buf = client_addr_buf.as_usize();
let addr_buf_ptr: *mut u8 = vm.get_heap_ptr(client_addr_buf);
let addr_buf_len = addr_buf_len.as_usize();
let addr_buf_ptr: *mut u8 = vm.get_heap_ptr(client_addr_buf, addr_buf_len);
let on_incoming_data = on_incoming_data.as_u64();

let mut net_state = &mut vm.sys_state.net_state;
Expand Down Expand Up @@ -283,7 +283,7 @@ pub fn net_read(
let socket_id = socket_id.as_u64();
let buf_len = buf_len.as_usize();
let buf_ptr = buf_ptr.as_usize();
let buf_ptr: *mut u8 = vm.get_heap_ptr(buf_ptr);
let buf_ptr: *mut u8 = vm.get_heap_ptr(buf_ptr, buf_len);

let mut net_state = &mut vm.sys_state.net_state;
match net_state.sockets.get_mut(&socket_id) {
Expand Down Expand Up @@ -315,7 +315,7 @@ pub fn net_write(
let socket_id = socket_id.as_u64();
let buf_len = buf_len.as_usize();
let buf_ptr = buf_ptr.as_usize();
let buf_ptr: *mut u8 = vm.get_heap_ptr(buf_ptr);
let buf_ptr: *mut u8 = vm.get_heap_ptr(buf_ptr, buf_len);

let mut net_state = &mut vm.sys_state.net_state;
match net_state.sockets.get_mut(&socket_id) {
Expand Down
8 changes: 4 additions & 4 deletions vm/src/sys/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,12 @@ pub fn window_create(vm: &mut VM, width: Value, height: Value, title: Value, fla

pub fn window_draw_frame(vm: &mut VM, window_id: Value, src_addr: Value)
{
// Get the address to copy pixel data from
let data_ptr = vm.get_heap_ptr(src_addr.as_usize());

let window = get_window(window_id.as_u32());

// Get the address to copy pixel data from
let data_len = (4 * window.width * window.height) as usize;
let data_ptr = vm.get_heap_ptr(src_addr.as_usize(), data_len);

// If no frame has been drawn yet
if window.texture.is_none() {
// Creat the texture to render into
Expand All @@ -150,7 +151,6 @@ pub fn window_draw_frame(vm: &mut VM, window_id: Value, src_addr: Value)

// Update the texture
let pitch = 4 * window.width as usize;
let data_len = (4 * window.width * window.height) as usize;
let pixel_slice = unsafe { std::slice::from_raw_parts(data_ptr, data_len) };
window.texture.as_mut().unwrap().update(None, pixel_slice, pitch).unwrap();

Expand Down
39 changes: 22 additions & 17 deletions vm/src/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -600,14 +600,13 @@ impl VM
self.heap.resize(num_bytes)
}

// FIXME: this function should be marked unsafe
//
/// Get a pointer to an address/offset in the heap
pub fn get_heap_ptr<T>(&mut self, addr: usize) -> *mut T
pub fn get_heap_ptr<T>(&mut self, addr: usize, num_elems: usize) -> *mut T
{
if addr + size_of::<T>() > self.heap.len() {
panic!(
"attempting to access data of type {} past end of heap",
std::any::type_name::<T>()
);
if addr + std::mem::size_of::<T>() * num_elems > self.heap.len() {
panic!("attempting to access memory slice past end of heap");
}

if addr & (size_of::<T>() - 1) != 0 {
Expand Down Expand Up @@ -639,9 +638,7 @@ impl VM

unsafe {
let heap_ptr: *mut u8 = self.heap.data.as_mut_ptr().add(addr);

let start_ptr = transmute::<*mut u8 , *mut T>(heap_ptr);

std::slice::from_raw_parts_mut(start_ptr, num_elems)
}
}
Expand All @@ -667,7 +664,7 @@ impl VM
}

// Convert the string to a Rust string
let char_ptr = self.get_heap_ptr(str_ptr);
let char_ptr = self.get_heap_ptr(str_ptr, str_len);
let c_str = unsafe { CStr::from_ptr(char_ptr as *const i8) };
let rust_str = c_str.to_str().unwrap();
rust_str
Expand Down Expand Up @@ -1346,57 +1343,57 @@ impl VM

Op::load_u8 => {
let addr = self.pop().as_usize();
let heap_ptr = self.get_heap_ptr(addr);
let heap_ptr = self.get_heap_ptr(addr, 1);
let val: u8 = unsafe { *heap_ptr };
self.push(val);
}

Op::load_u16 => {
let addr = self.pop().as_usize();
let heap_ptr = self.get_heap_ptr(addr);
let heap_ptr = self.get_heap_ptr(addr, 1);
let val: u16 = unsafe { *heap_ptr };
self.push(val);
}

Op::load_u32 => {
let addr = self.pop().as_usize();
let heap_ptr = self.get_heap_ptr(addr);
let heap_ptr = self.get_heap_ptr(addr, 1);
let val: u32 = unsafe { *heap_ptr };
self.push(val);
}

Op::load_u64 => {
let addr = self.pop().as_usize();
let heap_ptr = self.get_heap_ptr(addr);
let heap_ptr = self.get_heap_ptr(addr, 1);
let val: u64 = unsafe { *heap_ptr };
self.push(val);
}

Op::store_u8 => {
let val = self.pop().as_u8();
let addr = self.pop().as_usize();
let heap_ptr = self.get_heap_ptr(addr);
let heap_ptr = self.get_heap_ptr(addr, 1);
unsafe { *heap_ptr = val; }
}

Op::store_u16 => {
let val = self.pop().as_u16();
let addr = self.pop().as_usize();
let heap_ptr = self.get_heap_ptr(addr);
let heap_ptr = self.get_heap_ptr(addr, 1);
unsafe { *heap_ptr = val; }
}

Op::store_u32 => {
let val = self.pop().as_u32();
let addr = self.pop().as_usize();
let heap_ptr = self.get_heap_ptr(addr);
let heap_ptr = self.get_heap_ptr(addr, 1);
unsafe { *heap_ptr = val; }
}

Op::store_u64 => {
let val = self.pop().as_u64();
let addr = self.pop().as_usize();
let heap_ptr = self.get_heap_ptr(addr);
let heap_ptr = self.get_heap_ptr(addr, 1);
unsafe { *heap_ptr = val; }
}

Expand Down Expand Up @@ -1724,4 +1721,12 @@ mod tests
{
eval_src(".data; LABEL: .zero 1; .code; push LABEL; push 255; push 100_000_000; syscall memset; push 0; exit;");
}

// Regression: this used to segfault
#[test]
#[should_panic]
fn test_memcmp_n1()
{
eval_src(".data; A: .zero 10; B: .zero 10; .code; push A; push B; push -1; syscall memcpy;");
}
}

0 comments on commit 32be919

Please sign in to comment.