diff --git a/Cargo.lock b/Cargo.lock index 2d8455475b..d9e5a5f118 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1106,6 +1106,7 @@ dependencies = [ "signal_handler", "stack_trace", "task", + "task_cancel", "tlb_shootdown", "tss", "unwind", @@ -3179,6 +3180,7 @@ dependencies = [ "scheduler_round_robin", "sleep", "task", + "task_cancel", "x86_64", ] @@ -3719,6 +3721,16 @@ dependencies = [ "waker_generic", ] +[[package]] +name = "task_cancel" +version = "0.1.0" +dependencies = [ + "log", + "task", + "unwind", + "x86_64", +] + [[package]] name = "task_fs" version = "0.1.0" @@ -3948,7 +3960,7 @@ dependencies = [ "log", "spawn", "spin 0.9.4", - "task", + "task_cancel", ] [[package]] diff --git a/applications/test_task_cancel/Cargo.toml b/applications/test_task_cancel/Cargo.toml index 604b86d564..6ffcc19faa 100644 --- a/applications/test_task_cancel/Cargo.toml +++ b/applications/test_task_cancel/Cargo.toml @@ -9,4 +9,4 @@ edition = "2021" log = "0.4" spawn = { path = "../../kernel/spawn" } spin = "0.9" -task = { path = "../../kernel/task" } +task_cancel = { path = "../../kernel/task_cancel" } diff --git a/applications/test_task_cancel/src/lib.rs b/applications/test_task_cancel/src/lib.rs index 64b6fce4ce..a50818d171 100644 --- a/applications/test_task_cancel/src/lib.rs +++ b/applications/test_task_cancel/src/lib.rs @@ -1,11 +1,21 @@ -// TODO: Properly implement Task::kill so the test passes. +// TODO: Test that the other thread is succesfully cancelled in the following +// scenarios: +// +// 1. In lsda_generator, in which case it should trigger the first criteria of +// unwind::can_unwind. +// +// 2. At the call lsda_generator instruction, in which case it should trigger +// the second criteria of unwind::can_unwind. +// +// 3. At the jmp (loop) instruction, in which case it should continue to the +// next (call) instruction and then unwind. #![no_std] extern crate alloc; use alloc::{string::String, sync::Arc, vec::Vec}; -use core::sync::atomic::{AtomicBool, Ordering::Relaxed}; +use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering::Relaxed}; use spin::Mutex; pub fn main(_: Vec) -> isize { @@ -16,8 +26,7 @@ pub fn main(_: Vec) -> isize { while !lock.is_locked() {} - task.kill(task::KillReason::Requested) - .expect("failed to abort task"); + task.cancel(); log::debug!("waiting for lock to be unlocked"); @@ -51,4 +60,8 @@ fn lsda_generator() { if FALSE.load(Relaxed) { panic!(); } + + // Spend more time in lsda_generator to increase likelihood of scenario 1. + static __COUNTER: AtomicUsize = AtomicUsize::new(0); + __COUNTER.fetch_add(1, Relaxed); } diff --git a/kernel/exceptions_full/Cargo.toml b/kernel/exceptions_full/Cargo.toml index dfa5114f4a..2d895b6c1f 100644 --- a/kernel/exceptions_full/Cargo.toml +++ b/kernel/exceptions_full/Cargo.toml @@ -52,5 +52,8 @@ path = "../debug_info" [dependencies.signal_handler] path = "../signal_handler" +[dependencies.task_cancel] +path = "../task_cancel" + [lib] crate-type = ["rlib"] diff --git a/kernel/exceptions_full/src/lib.rs b/kernel/exceptions_full/src/lib.rs index c456e6a3d2..d11a971f9b 100644 --- a/kernel/exceptions_full/src/lib.rs +++ b/kernel/exceptions_full/src/lib.rs @@ -29,7 +29,7 @@ pub fn init(idt_ref: &'static LockedIdt) { // SET UP FIXED EXCEPTION HANDLERS idt.divide_error.set_handler_fn(divide_error_handler); - idt.debug.set_handler_fn(debug_handler); + idt.debug.set_handler_fn(task_cancel::interrupt_handler); idt.non_maskable_interrupt.set_handler_fn(nmi_handler); idt.breakpoint.set_handler_fn(breakpoint_handler); idt.overflow.set_handler_fn(overflow_handler); @@ -257,15 +257,7 @@ extern "x86-interrupt" fn divide_error_handler(stack_frame: InterruptStackFrame) kill_and_halt(0x0, &stack_frame, None, true) } -/// exception 0x01 -extern "x86-interrupt" fn debug_handler(stack_frame: InterruptStackFrame) { - println_both!("\nEXCEPTION: DEBUG EXCEPTION\n{:#X?}", stack_frame); - // don't halt here, this isn't a fatal/permanent failure, just a brief pause. -} - -/// Exception 0x02 is a Non-Maskable Interrupt (NMI). -/// -/// Theseus uses this for TLB Shootdown IPIs and sampling interrupts. +/// exception 0x02, also used for TLB Shootdown IPIs and sampling interrupts. /// /// # Important Note /// Acquiring ANY locks in this function, even irq-safe ones, could cause a deadlock diff --git a/kernel/interrupts/src/aarch64/mod.rs b/kernel/interrupts/src/aarch64/mod.rs index 716dd42506..f3bc0bbe31 100644 --- a/kernel/interrupts/src/aarch64/mod.rs +++ b/kernel/interrupts/src/aarch64/mod.rs @@ -64,7 +64,10 @@ struct EsrEL1(InMemoryRegister); macro_rules! interrupt_handler { ($name:ident, $x86_64_eoi_param:expr, $stack_frame:ident, $code:block) => { extern "C" fn $name($stack_frame: &$crate::InterruptStackFrame) -> $crate::EoiBehaviour $code - } + }; + ($name:ident, $x86_64_eoi_param:expr, mut $stack_frame:ident, $code:block) => { + extern "C" fn $name(mut $stack_frame: &$crate::InterruptStackFrame) -> $crate::EoiBehaviour $code + }; } /// The exception context as it is stored on the stack on exception entry. diff --git a/kernel/interrupts/src/x86_64/mod.rs b/kernel/interrupts/src/x86_64/mod.rs index b052f775d8..8f06062ec5 100644 --- a/kernel/interrupts/src/x86_64/mod.rs +++ b/kernel/interrupts/src/x86_64/mod.rs @@ -44,7 +44,15 @@ macro_rules! interrupt_handler { $crate::eoi($x86_64_eoi_param); } } - } + }; + ($name:ident, $x86_64_eoi_param:expr, mut $stack_frame:ident, $code:block) => { + extern "x86-interrupt" fn $name(mut sf: $crate::InterruptStackFrame) { + let mut $stack_frame = &mut sf; + if let $crate::EoiBehaviour::HandlerDidNotSendEoi = $code { + $crate::eoi($x86_64_eoi_param); + } + } + }; } diff --git a/kernel/scheduler/Cargo.toml b/kernel/scheduler/Cargo.toml index 14f61e37fa..b553e49f4c 100644 --- a/kernel/scheduler/Cargo.toml +++ b/kernel/scheduler/Cargo.toml @@ -9,6 +9,7 @@ edition = "2018" log = "0.4.8" cfg-if = "1.0.0" +task_cancel = { path = "../task_cancel" } cpu = { path = "../cpu" } interrupts = { path = "../interrupts" } sleep = { path = "../sleep" } diff --git a/kernel/scheduler/src/lib.rs b/kernel/scheduler/src/lib.rs index af0d3bb9cd..32e712c7c7 100644 --- a/kernel/scheduler/src/lib.rs +++ b/kernel/scheduler/src/lib.rs @@ -61,7 +61,7 @@ pub fn init() -> Result<(), &'static str> { } // Architecture-independent timer interrupt handler for preemptive scheduling. -interrupt_handler!(timer_tick_handler, None, _stack_frame, { +interrupt_handler!(timer_tick_handler, None, mut stack_frame, { #[cfg(target_arch = "aarch64")] interrupts::schedule_next_timer_tick(); @@ -89,6 +89,13 @@ interrupt_handler!(timer_tick_handler, None, _stack_frame, { schedule(); + if let Some(current_task) = task::get_my_current_task() { + if current_task.is_cancelled() { + // Trigger a debug interrupt on the next instruction which will invoke task_cancel::interrupt_handler. + task_cancel::set_trap_flag(stack_frame); + } + } + EoiBehaviour::HandlerSentEoi }); diff --git a/kernel/task/src/lib.rs b/kernel/task/src/lib.rs index b64af49d3a..559cf29166 100755 --- a/kernel/task/src/lib.rs +++ b/kernel/task/src/lib.rs @@ -133,6 +133,7 @@ struct TaskRefInner { /// /// This is not public because it permits interior mutability. joinable: AtomicBool, + pub cancel_requested: AtomicBool, } impl TaskRef { @@ -160,6 +161,7 @@ impl TaskRef { exit_value_mailbox, // A new task is joinable until its `JoinableTaskRef` is dropped. joinable: AtomicBool::new(true), + cancel_requested: AtomicBool::new(false), })); // Add the new TaskRef to the global task list. @@ -215,6 +217,14 @@ impl TaskRef { self.internal_exit(ExitValue::Killed(reason)) } + pub fn cancel(&self) { + self.0.cancel_requested.store(true, Ordering::Relaxed); + } + + pub fn is_cancelled(&self) -> bool { + self.0.cancel_requested.load(Ordering::Relaxed) + } + /// The internal routine that actually exits or kills a Task. fn internal_exit(&self, val: ExitValue) -> Result<(), &'static str> { if self.has_exited() { @@ -687,7 +697,6 @@ pub fn task_switch( cpu_id: CpuId, preemption_guard: PreemptionGuard, ) -> (bool, PreemptionGuard) { - // We use the `with_current_task_and_value()` closure here in order to ensure that // the borrowed reference to the current task is guaranteed to be dropped // *before* the actual context switch operation occurs. diff --git a/kernel/task_cancel/Cargo.toml b/kernel/task_cancel/Cargo.toml new file mode 100644 index 0000000000..ce33472fe3 --- /dev/null +++ b/kernel/task_cancel/Cargo.toml @@ -0,0 +1,12 @@ +[package] +name = "task_cancel" +version = "0.1.0" +authors = ["Klim Tsoutsman "] +description = "Task cancellation" +edition = "2021" + +[dependencies] +task = { path = "../task" } +unwind = { path = "../unwind" } +x86_64 = "0.14" +log = "0.4" diff --git a/kernel/task_cancel/src/lib.rs b/kernel/task_cancel/src/lib.rs new file mode 100644 index 0000000000..1b3b2faf56 --- /dev/null +++ b/kernel/task_cancel/src/lib.rs @@ -0,0 +1,34 @@ +// TODO: Move into task crate. Hasn't yet been done because it creates a +// circular dependency as unwind depends on task. + +#![feature(abi_x86_interrupt, try_blocks)] +#![no_std] + +use task::KillReason; +use x86_64::structures::idt::InterruptStackFrame; + +pub fn set_trap_flag(stack_frame: &mut InterruptStackFrame) { + unsafe { stack_frame.as_mut() }.update(|stack_frame| stack_frame.cpu_flags |= 0x100); +} + +pub extern "x86-interrupt" fn interrupt_handler(mut stack_frame: InterruptStackFrame) { + let instruction_pointer = stack_frame.instruction_pointer.as_u64(); + let stack_pointer = stack_frame.stack_pointer.as_u64(); + + if unwind::can_unwind(instruction_pointer) { + log::info!("unwinding a cancelled task"); + unwind::start_remote_unwinding( + KillReason::Requested, + 0, + stack_pointer, + instruction_pointer, + ) + .expect("failed to unwind"); + } else { + log::debug!("couldn't unwind at {instruction_pointer:0x?}; resetting trap flag"); + // The trap flag is reset after every debug interrupt. Since we can't unwind at + // this instruction, we reset the flag to check again at the next instruction. + set_trap_flag(&mut stack_frame); + // FIXME: What happens if a LAPIC timer interrupt triggers here? + } +} diff --git a/kernel/unwind/src/lib.rs b/kernel/unwind/src/lib.rs index 8db365bdea..413bb0a0b5 100644 --- a/kernel/unwind/src/lib.rs +++ b/kernel/unwind/src/lib.rs @@ -403,7 +403,7 @@ impl FallibleIterator for StackFrameIter { // If there is an error code pushed, then we need to account for that additionally beyond the exception stack frame being pushed. let size_of_exception_stack_frame: i64 = 5 * 8; trace!("StackFrameIter: next stack frame is an exception handler: adding {:#X} to cfa, new cfa: {:#X}", size_of_exception_stack_frame, cfa); - + this_frame_is_exception_handler = true; Some(size_of_error_code + size_of_exception_stack_frame) } else { @@ -446,6 +446,88 @@ unsafe fn deref_ptr(ptr: Pointer) -> u64 { pub trait FuncWithRegisters = FnMut(Registers) -> Result<(), &'static str>; type FuncWithRegistersRefMut<'a> = &'a mut dyn FuncWithRegisters; +/// Returns whether unwinding can begin from the given instruction pointer. +/// +/// This happens when one of the following two criteria is met: +/// +/// 1. The pointer is covered by the unwind tables, but does not have an LSDA. +/// This occurs when the function does not have any associated drop handlers, +/// either because it has nothing to drop or Rust has determined that the +/// function will never be unwound. +/// +/// 2. The pointer is covered by the unwind tables, and has an associated +/// landing pad. This occurs when the function has associated drop handlers and +/// the instruction pointer is at an instruction that could cause an unwinding +/// (usually a call site to a function that can cause an unwinding). +pub fn can_unwind(instruction_pointer: u64) -> bool { + can_unwind_inner(instruction_pointer).is_some() +} + +fn can_unwind_inner(instruction_pointer: u64) -> Option<()> { + let task = task::get_my_current_task()?; + let instruction_address = VirtualAddress::new(instruction_pointer as usize)?; + let krate = task + .namespace + .get_crate_containing_address(instruction_address, false)?; + + let (eh_frame, base_addresses) = get_eh_frame_info(&krate) + .map(|(eh_frame_section, base_addresses)| { + (EhFrameReference::Section(eh_frame_section), base_addresses) + }) + .or_else(|| { + external_unwind_info::get_unwind_info(instruction_address).map(|unwind_info| { + let base_addresses = BaseAddresses::default() + .set_eh_frame(unwind_info.unwind_info.start.value() as u64) + .set_text(unwind_info.text_section.start.value() as u64); + (EhFrameReference::External(unwind_info), base_addresses) + }) + })?; + + let unwind_row = UnwindRowReference { + caller: instruction_pointer, + eh_frame_sec: eh_frame, + base_addrs: base_addresses, + }; + let StackFrame { + lsda: lsda_address, + initial_address, + call_site_address, + .. + } = unwind_row + .with_unwind_info(|fde, _| { + Ok(StackFrame { + personality: None, + lsda: fde.lsda().map(|address| unsafe { deref_ptr(address) }), + initial_address: fde.initial_address(), + call_site_address: instruction_pointer, + }) + }) + .ok()?; + + let lsda_address = match lsda_address { + Some(address) => VirtualAddress::new_canonical(address as usize), + // Criteria 1: Covered by unwind tables, but no LSDA. + None => return Some(()), + }; + + let (lsda_section, _) = task + .namespace + .get_section_containing_address(lsda_address, true)?; + let start = + lsda_section.mapped_pages_offset + (lsda_address.value() - lsda_section.virt_addr.value()); + let length = lsda_section.virt_addr.value() + lsda_section.size - lsda_address.value(); + + let lsda_pages = lsda_section.mapped_pages.lock(); + let lsda_slice = lsda_pages.as_slice::(start, length).ok()?; + let table = lsda::GccExceptTableArea::new(lsda_slice, NativeEndian, initial_address); + + let entry = table + .call_site_table_entry_for_address(call_site_address) + .ok()?; + + // Criteria 2: Covered by unwind tables, and has an associated landing pad. + entry.landing_pad_address().map(|_| ()) +} /// This function saves the current CPU register values onto the stack (to preserve them) /// and then invokes the given closure with those registers as the argument. @@ -699,7 +781,6 @@ fn get_eh_frame_info(crate_ref: &StrongCrateRef) -> Option<(StrongSectionRef, Ba Some((eh_frame_sec.clone(), base_addrs)) } - /// Starts the unwinding procedure for the current task /// by working backwards up the call stack starting from the current stack frame. /// @@ -712,9 +793,19 @@ fn get_eh_frame_info(crate_ref: &StrongCrateRef) -> Option<(StrongSectionRef, Ba /// ## Note: Skipping frames /// If you are unsure how many frames you could possibly skip, then it's always safe to pass `0` /// such that all function frames on the stack are unwound. -/// -#[doc(hidden)] pub fn start_unwinding(reason: KillReason, stack_frames_to_skip: usize) -> Result<(), &'static str> { + start_unwinding_inner(reason, stack_frames_to_skip, None, None) +} + +/// Starts the unwinding procedure for a specific instruction pointer, and stack pointer. +/// +/// See [`start_unwinding`] for more information. +// TODO: Rename. This isn't technically remote unwinding. +pub fn start_remote_unwinding(reason: KillReason, stack_frames_to_skip: usize, rsp: u64, ip: u64) -> Result<(), &'static str> { + start_unwinding_inner(reason, stack_frames_to_skip, Some(rsp), Some(ip)) +} + +fn start_unwinding_inner(reason: KillReason, stack_frames_to_skip: usize, rsp: Option, ip: Option) -> Result<(), &'static str> { // Here we have to be careful to have no resources waiting to be dropped/freed/released on the stack. let unwinding_context_ptr = { let current_task = task::get_my_current_task().ok_or("couldn't get current task")?; @@ -738,11 +829,20 @@ pub fn start_unwinding(reason: KillReason, stack_frames_to_skip: usize) -> Resul // We pass a pointer to the unwinding context to this closure. - let res = invoke_with_current_registers(&mut |registers| { + let res = invoke_with_current_registers(&mut |mut registers: Registers| { // set the proper register values before start the actual unwinding procedure. { // SAFE: we just created this pointer above let unwinding_context = unsafe { &mut *unwinding_context_ptr }; + if let Some(rsp) = rsp { + registers[X86_64::RSP] = Some(rsp); + } + if let Some(ip) = ip { + // StackFrameIter::next subtracts 1 from the return address to get the call site. + // Since we're already providing the call site instruction pointer, we add 1 here + // to negate the subtraction later. + registers[X86_64::RA] = Some(ip + 1); + } unwinding_context.stack_frame_iter.registers = registers; // Skip the first several frames, e.g., to skip unwinding the panic entry point functions themselves. @@ -770,7 +870,6 @@ pub fn start_unwinding(reason: KillReason, stack_frames_to_skip: usize) -> Resul cleanup_unwinding_context(unwinding_context_ptr); } - /// Continues the unwinding process from the point it left off at, /// which is defined by the given unwinding context. ///