Skip to content

Commit

Permalink
fix(kernel_function): propagate unsafety
Browse files Browse the repository at this point in the history
Signed-off-by: Martin Kröning <[email protected]>
  • Loading branch information
mkroening committed Mar 23, 2024
1 parent 83b2be0 commit abcf8c4
Show file tree
Hide file tree
Showing 12 changed files with 120 additions and 47 deletions.
2 changes: 1 addition & 1 deletion src/arch/aarch64/kernel/scheduler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ extern "C" fn task_start(_f: extern "C" fn(usize), _arg: usize) -> ! {
}

impl TaskFrame for Task {
fn create_stack_frame(&mut self, func: extern "C" fn(usize), arg: usize) {
fn create_stack_frame(&mut self, func: unsafe extern "C" fn(usize), arg: usize) {
// Check if TLS is allocated already and if the task uses thread-local storage.
if self.tls.is_none() {
self.tls = TaskTLS::from_environment();
Expand Down
2 changes: 1 addition & 1 deletion src/arch/aarch64/kernel/switch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ macro_rules! kernel_function_impl {
($kernel_function:ident($($arg:ident: $A:ident),*) { $($operands:tt)* }) => {
/// Executes `f` on the kernel stack.
#[allow(dead_code)]
pub fn $kernel_function<R, $($A),*>(f: unsafe extern "C" fn($($A),*) -> R, $($arg: $A),*) -> R {
pub unsafe fn $kernel_function<R, $($A),*>(f: unsafe extern "C" fn($($A),*) -> R, $($arg: $A),*) -> R {
unsafe {
assert!(mem::size_of::<R>() <= mem::size_of::<usize>());

Expand Down
2 changes: 1 addition & 1 deletion src/arch/riscv64/kernel/scheduler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ extern "C" fn task_entry(func: extern "C" fn(usize), arg: usize) {
}

impl TaskFrame for Task {
fn create_stack_frame(&mut self, func: extern "C" fn(usize), arg: usize) {
fn create_stack_frame(&mut self, func: unsafe extern "C" fn(usize), arg: usize) {
// Check if the task (process or thread) uses Thread-Local-Storage.
// check is TLS is already allocated
if self.tls.is_none() {
Expand Down
2 changes: 1 addition & 1 deletion src/arch/x86_64/kernel/scheduler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ extern "C" fn task_entry(func: extern "C" fn(usize), arg: usize) -> ! {
}

impl TaskFrame for Task {
fn create_stack_frame(&mut self, func: extern "C" fn(usize), arg: usize) {
fn create_stack_frame(&mut self, func: unsafe extern "C" fn(usize), arg: usize) {
// Check if TLS is allocated already and if the task uses thread-local storage.
#[cfg(not(feature = "common-os"))]
if self.tls.is_none() {
Expand Down
2 changes: 1 addition & 1 deletion src/arch/x86_64/kernel/switch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ macro_rules! kernel_function_impl {
($kernel_function:ident($($arg:ident: $A:ident),*) { $($operands:tt)* }) => {
/// Executes `f` on the kernel stack.
#[allow(dead_code)]
pub fn $kernel_function<R, $($A),*>(f: unsafe extern "C" fn($($A),*) -> R, $($arg: $A),*) -> R {
pub unsafe fn $kernel_function<R, $($A),*>(f: unsafe extern "C" fn($($A),*) -> R, $($arg: $A),*) -> R {
unsafe {
assert!(mem::size_of::<R>() <= mem::size_of::<usize>());

Expand Down
10 changes: 9 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,15 @@ fn boot_processor_main() -> ! {
}

// Start the initd task.
scheduler::PerCoreScheduler::spawn(initd, 0, scheduler::task::NORMAL_PRIO, 0, USER_STACK_SIZE);
unsafe {
scheduler::PerCoreScheduler::spawn(
initd,
0,
scheduler::task::NORMAL_PRIO,
0,
USER_STACK_SIZE,
)
};

// Run the scheduler loop.
PerCoreScheduler::run();
Expand Down
80 changes: 72 additions & 8 deletions src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,45 +80,109 @@ macro_rules! dbg {
macro_rules! kernel_function {
($f:ident()) => {{
use $crate::errno::ToErrno;

// This propagates any unsafety requirements of `f` to the caller.
if false {
$f();
}

#[allow(unreachable_code)]
$crate::arch::switch::kernel_function0($f).set_errno()
#[allow(unused_unsafe)]
unsafe {
$crate::arch::switch::kernel_function0($f).set_errno()
}
}};

($f:ident($arg1:expr)) => {{
use $crate::errno::ToErrno;

// This propagates any unsafety requirements of `f` to the caller.
if false {
$f($arg1);
}

#[allow(unreachable_code)]
$crate::arch::switch::kernel_function1($f, $arg1).set_errno()
#[allow(unused_unsafe)]
unsafe {
$crate::arch::switch::kernel_function1($f, $arg1).set_errno()
}
}};

($f:ident($arg1:expr, $arg2:expr)) => {{
use $crate::errno::ToErrno;

// This propagates any unsafety requirements of `f` to the caller.
if false {
$f($arg1, $arg2);
}

#[allow(unreachable_code)]
$crate::arch::switch::kernel_function2($f, $arg1, $arg2).set_errno()
#[allow(unused_unsafe)]
unsafe {
$crate::arch::switch::kernel_function2($f, $arg1, $arg2).set_errno()
}
}};

($f:ident($arg1:expr, $arg2:expr, $arg3:expr)) => {{
use $crate::errno::ToErrno;

// This propagates any unsafety requirements of `f` to the caller.
if false {
$f($arg1, $arg2, $arg3);
}

#[allow(unreachable_code)]
$crate::arch::switch::kernel_function3($f, $arg1, $arg2, $arg3).set_errno()
#[allow(unused_unsafe)]
unsafe {
$crate::arch::switch::kernel_function3($f, $arg1, $arg2, $arg3).set_errno()
}
}};

($f:ident($arg1:expr, $arg2:expr, $arg3:expr, $arg4:expr)) => {{
use $crate::errno::ToErrno;

// This propagates any unsafety requirements of `f` to the caller.
if false {
$f($arg1, $arg2, $arg3, $arg4);
}

#[allow(unreachable_code)]
$crate::arch::switch::kernel_function4($f, $arg1, $arg2, $arg3, $arg4).set_errno()
#[allow(unused_unsafe)]
unsafe {
$crate::arch::switch::kernel_function4($f, $arg1, $arg2, $arg3, $arg4).set_errno()
}
}};

($f:ident($arg1:expr, $arg2:expr, $arg3:expr, $arg4:expr, $arg5:expr)) => {{
use $crate::errno::ToErrno;

// This propagates any unsafety requirements of `f` to the caller.
if false {
$f($arg1, $arg2, $arg3, $arg4, $arg5);
}

#[allow(unreachable_code)]
$crate::arch::switch::kernel_function5($f, $arg1, $arg2, $arg3, $arg4, $arg5).set_errno()
#[allow(unused_unsafe)]
unsafe {
$crate::arch::switch::kernel_function5($f, $arg1, $arg2, $arg3, $arg4, $arg5)
.set_errno()
}
}};

($f:ident($arg1:expr, $arg2:expr, $arg3:expr, $arg4:expr, $arg5:expr, $arg6:expr)) => {{
use $crate::errno::ToErrno;

// This propagates any unsafety requirements of `f` to the caller.
if false {
$f($arg1, $arg2, $arg3, $arg4, $arg5, $arg6);
}

#[allow(unreachable_code)]
$crate::arch::switch::kernel_function6($f, $arg1, $arg2, $arg3, $arg4, $arg5, $arg6)
.set_errno()
#[allow(unused_unsafe)]
unsafe {
$crate::arch::switch::kernel_function6($f, $arg1, $arg2, $arg3, $arg4, $arg5, $arg6)
.set_errno()
}
}};
}

Expand Down
12 changes: 6 additions & 6 deletions src/scheduler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ impl PerCoreSchedulerExt for &mut PerCoreScheduler {

struct NewTask {
tid: TaskId,
func: extern "C" fn(usize),
func: unsafe extern "C" fn(usize),
arg: usize,
prio: Priority,
core_id: CoreId,
Expand Down Expand Up @@ -231,8 +231,8 @@ impl From<NewTask> for Task {

impl PerCoreScheduler {
/// Spawn a new task.
pub fn spawn(
func: extern "C" fn(usize),
pub unsafe fn spawn(
func: unsafe extern "C" fn(usize),
arg: usize,
prio: Priority,
core_id: CoreId,
Expand Down Expand Up @@ -941,8 +941,8 @@ fn get_scheduler_input(core_id: CoreId) -> &'static InterruptTicketMutex<Schedul
SCHEDULER_INPUTS.lock()[usize::try_from(core_id).unwrap()]
}

pub fn spawn(
func: extern "C" fn(usize),
pub unsafe fn spawn(
func: unsafe extern "C" fn(usize),
arg: usize,
prio: Priority,
stack_size: usize,
Expand All @@ -957,7 +957,7 @@ pub fn spawn(
selector as u32
};

PerCoreScheduler::spawn(func, arg, prio, core_id, stack_size)
unsafe { PerCoreScheduler::spawn(func, arg, prio, core_id, stack_size) }
}

pub fn getpid() -> TaskId {
Expand Down
2 changes: 1 addition & 1 deletion src/scheduler/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ pub(crate) struct Task {

pub(crate) trait TaskFrame {
/// Create the initial stack frame for a new task
fn create_stack_frame(&mut self, func: extern "C" fn(usize), arg: usize);
fn create_stack_frame(&mut self, func: unsafe extern "C" fn(usize), arg: usize);
}

impl Task {
Expand Down
4 changes: 1 addition & 3 deletions src/syscalls/entropy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,9 @@ unsafe extern "C" fn __sys_read_entropy(buf: *mut u8, len: usize, flags: u32) ->
/// Returns either the number of bytes written to buf (a positive value) or
/// * `-EINVAL` if `flags` contains unknown flags.
/// * `-ENOSYS` if the system does not support random data generation.
#[allow(unsafe_op_in_unsafe_fn)]
#[no_mangle]
#[cfg_attr(target_arch = "riscv64", allow(unsafe_op_in_unsafe_fn))] // FIXME
pub unsafe extern "C" fn sys_read_entropy(buf: *mut u8, len: usize, flags: u32) -> isize {
kernel_function!(__sys_read_entropy(buf, len, flags))
unsafe { kernel_function!(__sys_read_entropy(buf, len, flags)) }
}

#[cfg(not(feature = "newlib"))]
Expand Down
21 changes: 11 additions & 10 deletions src/syscalls/tasks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,36 +236,37 @@ pub extern "C" fn sys_signal(handler: SignalHandler) -> i32 {
kernel_function!(__sys_signal(handler))
}

extern "C" fn __sys_spawn2(
func: extern "C" fn(usize),
unsafe extern "C" fn __sys_spawn2(
func: unsafe extern "C" fn(usize),
arg: usize,
prio: u8,
stack_size: usize,
selector: isize,
) -> Tid {
scheduler::spawn(func, arg, Priority::from(prio), stack_size, selector).into()
unsafe { scheduler::spawn(func, arg, Priority::from(prio), stack_size, selector).into() }
}

#[no_mangle]
pub extern "C" fn sys_spawn2(
func: extern "C" fn(usize),
pub unsafe extern "C" fn sys_spawn2(
func: unsafe extern "C" fn(usize),
arg: usize,
prio: u8,
stack_size: usize,
selector: isize,
) -> Tid {
kernel_function!(__sys_spawn2(func, arg, prio, stack_size, selector))
unsafe { kernel_function!(__sys_spawn2(func, arg, prio, stack_size, selector)) }
}

unsafe extern "C" fn __sys_spawn(
id: *mut Tid,
func: extern "C" fn(usize),
func: unsafe extern "C" fn(usize),
arg: usize,
prio: u8,
selector: isize,
) -> i32 {
let new_id =
scheduler::spawn(func, arg, Priority::from(prio), USER_STACK_SIZE, selector).into();
let new_id = unsafe {
scheduler::spawn(func, arg, Priority::from(prio), USER_STACK_SIZE, selector).into()
};

if !id.is_null() {
unsafe {
Expand All @@ -279,7 +280,7 @@ unsafe extern "C" fn __sys_spawn(
#[no_mangle]
pub unsafe extern "C" fn sys_spawn(
id: *mut Tid,
func: extern "C" fn(usize),
func: unsafe extern "C" fn(usize),
arg: usize,
prio: u8,
selector: isize,
Expand Down
28 changes: 15 additions & 13 deletions tests/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub fn thread_test() {
let threadnum = 5;
for i in 0..threadnum {
println!("SPAWNING THREAD {}", i);
let id = sys_spawn2(thread_func, i, NORMAL_PRIO, USER_STACK_SIZE, -1);
let id = unsafe { sys_spawn2(thread_func, i, NORMAL_PRIO, USER_STACK_SIZE, -1) };
children.push(id);
}
println!("SPAWNED THREADS");
Expand All @@ -47,13 +47,13 @@ pub fn thread_test() {
}
}

extern "C" fn waker_func(futex: usize) {
unsafe extern "C" fn waker_func(futex: usize) {
let futex = unsafe { &*(futex as *const AtomicU32) };

sys_usleep(100_000);

futex.store(1, Relaxed);
let ret = sys_futex_wake(futex as *const AtomicU32 as *mut u32, i32::MAX);
let ret = unsafe { sys_futex_wake(futex as *const AtomicU32 as *mut u32, i32::MAX) };
assert_eq!(ret, 1);
}

Expand All @@ -62,26 +62,28 @@ pub fn test_futex() {
let futex = AtomicU32::new(0);
let futex_ptr = &futex as *const AtomicU32 as *mut u32;

let ret = sys_futex_wait(futex_ptr, 1, ptr::null(), 0);
let ret = unsafe { sys_futex_wait(futex_ptr, 1, ptr::null(), 0) };
assert_eq!(ret, -EAGAIN);

let timeout = timespec {
tv_sec: 0,
tv_nsec: 100_000_000,
};
let ret = sys_futex_wait(futex_ptr, 0, &timeout, 1);
let ret = unsafe { sys_futex_wait(futex_ptr, 0, &timeout, 1) };
assert_eq!(ret, -ETIMEDOUT);

let waker = sys_spawn2(
waker_func,
futex_ptr as usize,
NORMAL_PRIO,
USER_STACK_SIZE,
-1,
);
let waker = unsafe {
sys_spawn2(
waker_func,
futex_ptr as usize,
NORMAL_PRIO,
USER_STACK_SIZE,
-1,
)
};
assert!(waker >= 0);

let ret = sys_futex_wait(futex_ptr, 0, ptr::null(), 0);
let ret = unsafe { sys_futex_wait(futex_ptr, 0, ptr::null(), 0) };
assert_eq!(ret, 0);
assert_eq!(futex.load(Relaxed), 1);

Expand Down

0 comments on commit abcf8c4

Please sign in to comment.