Skip to content

Commit

Permalink
Move Safety comments inside the unsafe blocks.
Browse files Browse the repository at this point in the history
It's funny, I could've sworn there was already some tool for this, but
couldn't find it when I went to look for it. *shrug*
  • Loading branch information
kitlith committed Nov 2, 2019
1 parent 0936063 commit 0d026af
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 23 deletions.
14 changes: 9 additions & 5 deletions kernel/src/i386/interrupt_service_routines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -576,8 +576,10 @@ macro_rules! generate_trap_gate_handler {
if $interrupt_context {
let _ = INSIDE_INTERRUPT_COUNT.fetch_add(1, Ordering::SeqCst);
// Lets SpinLockIrq know that we are an interrupt; interrupts should not be re-enabled.
// Safety: Paired with decrement_lock_count, which is called before exiting the interrupt.
unsafe { disable_interrupts(); }
unsafe {
// Safety: Paired with decrement_lock_count, which is called before exiting the interrupt.
disable_interrupts();
}
}

if let PrivilegeLevel::Ring0 = SegmentSelector(userspace_context.cs as u16).rpl() {
Expand All @@ -599,9 +601,11 @@ macro_rules! generate_trap_gate_handler {

if $interrupt_context {
let _ = INSIDE_INTERRUPT_COUNT.fetch_sub(1, Ordering::SeqCst);
// Safety: Paired with disable_interrupts, which was called earlier in this wrapper function.
// Additionally, this is called shortly before an iret occurs, inside the asm wrapper.
unsafe { decrement_lock_count(); }
unsafe {
// Safety: Paired with disable_interrupts, which was called earlier in this wrapper function.
// Additionally, this is called shortly before an iret occurs, inside the asm wrapper.
decrement_lock_count();
}
}

// if we're returning to userspace, check we haven't been killed
Expand Down
15 changes: 9 additions & 6 deletions kernel/src/i386/process_switch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,12 +349,15 @@ fn jump_to_entrypoint(ep: usize, userspace_stack_ptr: usize, arg1: usize, arg2:
const_assert_eq!((GdtIndex::UStack as u16) << 3 | 0b11, 0x4B);


// Safety: This is paired with an undropped SpinLockIrq (interrupt_manager) in scheduler::internal_schedule.
// (Normally, this SpinLockIrq evens out with an identical one in the same function in the new process,
// however, when a new process starts this object is not present, therefore we must manually decrement
// the counter.)
// Additionally, an iret occurs later in this function, enabling interrupts.
unsafe { crate::sync::spin_lock_irq::decrement_lock_count(); }
unsafe {
// Safety: This is paired with an undropped SpinLockIrq (interrupt_manager) in scheduler::internal_schedule.
// (Normally, this SpinLockIrq evens out with an identical one in the same function in the new process,
// however, when a new process starts this object is not present, therefore we must manually decrement
// the counter.)
// Additionally, an iret occurs later in this function, enabling interrupts.
crate::sync::spin_lock_irq::decrement_lock_count();
}

unsafe {
asm!("
mov ax,0x33 // ds, es <- UData, Ring 3
Expand Down
6 changes: 4 additions & 2 deletions kernel/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,10 @@ pub extern "C" fn common_start(multiboot_info_addr: usize) -> ! {
init_cpu_locals(1);
// Now that cpu_locals are present, ensure that SpinLockIRQs
// are aware that interrupts are disabled.
// Safety: paired with enable_interrupts in interrupt_service_routines::init
unsafe { sync::spin_lock_irq::disable_interrupts(); }
unsafe {
// Safety: paired with enable_interrupts in interrupt_service_routines::init
sync::spin_lock_irq::disable_interrupts();
}

info!("Enabling interrupts");
unsafe { i386::interrupt_service_routines::init(); }
Expand Down
28 changes: 18 additions & 10 deletions kernel/src/sync/spin_lock_irq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,10 @@ impl<T> SpinLockIRQ<T> {
impl<T: ?Sized> SpinLockIRQ<T> {
/// Disables interrupts and locks the mutex.
pub fn lock(&self) -> SpinLockIRQGuard<'_, T> {
// Safety: Paired with enable_interrupts in the impl of Drop for SpinLockIrqGuard.
unsafe { disable_interrupts(); }
unsafe {
// Safety: Paired with enable_interrupts in the impl of Drop for SpinLockIrqGuard.
disable_interrupts();
}

// TODO: Disable preemption.
// TODO: Spin acquire
Expand All @@ -133,9 +135,11 @@ impl<T: ?Sized> SpinLockIRQ<T> {

/// Disables interrupts and locks the mutex.
pub fn try_lock(&self) -> Option<SpinLockIRQGuard<'_, T>> {
// Safety: Paired with enable_interrupts in the impl of Drop for SpinLockIrq,
// or in case a guard is not created, later in this function.
unsafe { disable_interrupts(); }
unsafe {
// Safety: Paired with enable_interrupts in the impl of Drop for SpinLockIrq,
// or in case a guard is not created, later in this function.
disable_interrupts();
}

// TODO: Disable preemption.
// TODO: Spin acquire
Expand All @@ -145,8 +149,10 @@ impl<T: ?Sized> SpinLockIRQ<T> {
Some(internalguard) => Some(SpinLockIRQGuard(ManuallyDrop::new(internalguard))),
None => {
// We couldn't lock. Restore irqs and return None
// Safety: Paired with disable_interrupts above in the case that a guard is not created.
unsafe { enable_interrupts(); }
unsafe {
// Safety: Paired with disable_interrupts above in the case that a guard is not created.
enable_interrupts();
}
None
}
}
Expand Down Expand Up @@ -181,9 +187,11 @@ impl<'a, T: ?Sized + 'a> Drop for SpinLockIRQGuard<'a, T> {
// unlock
unsafe { ManuallyDrop::drop(&mut self.0); }

// Safety: paired with disable_interrupts in SpinLockIRQ::{lock, try_lock}, which returns
// this guard to re-enable interrupts when it is dropped.
unsafe { enable_interrupts(); }
unsafe {
// Safety: paired with disable_interrupts in SpinLockIRQ::{lock, try_lock}, which returns
// this guard to re-enable interrupts when it is dropped.
enable_interrupts();
}

// TODO: Enable preempt
}
Expand Down

0 comments on commit 0d026af

Please sign in to comment.