From 24be35547e5c227ea764e8c68c265639fa9dbbce Mon Sep 17 00:00:00 2001 From: Quentin Legot Date: Wed, 5 Apr 2023 12:01:31 +0200 Subject: [PATCH 1/6] Add Exit Exception --- src/kernel/exception.rs | 34 +++++++++++++++++++++++++--------- src/kernel/thread_manager.rs | 18 +++++++++--------- src/main.rs | 4 ++-- src/simulator/machine.rs | 19 ++++++++++--------- 4 files changed, 46 insertions(+), 29 deletions(-) diff --git a/src/kernel/exception.rs b/src/kernel/exception.rs index 5a96b44..0cf5921 100644 --- a/src/kernel/exception.rs +++ b/src/kernel/exception.rs @@ -1,5 +1,9 @@ +use std::rc::Rc; + use crate::simulator::{machine::{ExceptionType, Machine}, error::{MachineOk, MachineError}}; +use super::system::System; + pub const SC_SHUTDOWN: u8 = 0; pub const SC_EXIT: u8 = 1; @@ -40,11 +44,11 @@ pub const SC_DEBUG: u8 = 34; pub const CONSOLE_OUTPUT: u8 = 1; // todo : returns new types, not just machine errors and machine ok -pub fn call(exception: ExceptionType, machine: &Machine) -> Result { +pub fn call(exception: ExceptionType, machine: &mut Machine, system: &mut System) -> Result { match exception { ExceptionType::NoException => todo!(), - ExceptionType::SyscallException => syscall(machine), + ExceptionType::SyscallException => syscall(machine, system), ExceptionType::PagefaultException => todo!(), ExceptionType::ReadOnlyException => todo!(), ExceptionType::BusErrorException => todo!(), @@ -55,12 +59,23 @@ pub fn call(exception: ExceptionType, machine: &Machine) -> Result Result { +fn syscall(machine: &mut Machine, system: &mut System) -> Result { let call_type = machine.read_int_register(17) as u8; match call_type { SC_SHUTDOWN => Ok(MachineOk::Shutdown), - SC_EXIT => todo!(), + SC_EXIT => { + match &system.get_thread_manager().g_current_thread { + Some(th) => { + let th = Rc::clone(th); + system.get_thread_manager().thread_finish(machine, th); + Ok(MachineOk::Ok) + }, + None => { + Err("Current thread is None".into()) + } + } + }, SC_EXEC => todo!(), SC_JOIN => todo!(), SC_CREATE => todo!(), @@ -121,6 +136,7 @@ fn syscall(machine: &Machine) -> Result { #[cfg(test)] mod test { use crate::kernel::exception::{SC_SHUTDOWN, SC_WRITE}; + use crate::kernel::system::System; use crate::simulator::instruction::Instruction; use crate::simulator::machine::Machine; @@ -132,8 +148,8 @@ mod test { machine.write_memory(4, 0, 0b000000000000_00000_000_00000_1110011); // ecall machine.write_memory(4, 4, 0b000000001010_00000_000_00001_0010011); // r1 <- 10 - - machine.run(); + let mut system = System::default(); + machine.run(&mut system); // If the machine was stopped with no error, the shutdown worked assert_ne!(machine.read_int_register(1), 10); // Check if the next instruction was executed } @@ -162,9 +178,9 @@ mod test { machine.write_memory(4, 4, 0b000000000000_00000_000_10001_0010011); // r17 <- SC_SHUTDOWN machine.write_memory(4, 8, 0b000000000000_00000_000_00000_1110011); // ecall - - - machine.run(); + + let mut system = System::default(); + machine.run(&mut system); } } \ No newline at end of file diff --git a/src/kernel/thread_manager.rs b/src/kernel/thread_manager.rs index ad123f3..3b558bc 100644 --- a/src/kernel/thread_manager.rs +++ b/src/kernel/thread_manager.rs @@ -123,8 +123,8 @@ impl ThreadManager { /// Put the thread to sleep and relinquish the processor pub fn thread_sleep(&mut self, machine: &mut Machine, thread: Rc>) { - assert_eq!(Option::Some(Rc::clone(&thread)), self.g_current_thread); - assert_eq!(machine.interrupt.get_status(), InterruptStatus::InterruptOff); + debug_assert_eq!(Option::Some(Rc::clone(&thread)), self.g_current_thread); + debug_assert_eq!(machine.interrupt.get_status(), InterruptStatus::InterruptOff); let mut next_thread = self.find_next_to_run(); while next_thread.is_none() { @@ -220,22 +220,22 @@ mod test { let thread2 = Rc::new(RefCell::new(thread2)); system.get_thread_manager().get_g_alive().push(Rc::clone(&thread1)); - let owner2 = Process { num_thread: 0 }; - system.get_thread_manager().start_thread(Rc::clone(&thread2), owner2, ptr1 + loader2.elf_header.entrypoint, ptr2 , -1); - let owner1 = Process { num_thread: 0 }; system.get_thread_manager().start_thread(Rc::clone(&thread1), owner1, loader.elf_header.entrypoint, ptr1, -1); debug_assert_eq!(thread1.borrow_mut().thread_context.pc, start_pc); debug_assert!(system.get_thread_manager().get_g_alive().contains(&Rc::clone(&thread1))); + let owner2 = Process { num_thread: 0 }; + system.get_thread_manager().start_thread(Rc::clone(&thread2), owner2, ptr1 + loader2.elf_header.entrypoint, ptr2 , -1); + let to_run = system.get_thread_manager().find_next_to_run().unwrap(); - debug_assert_eq!(to_run, Rc::clone(&thread2)); + debug_assert_eq!(to_run, Rc::clone(&thread1)); system.get_thread_manager().switch_to(&mut machine, Rc::clone(&to_run)); - debug_assert_eq!(system.get_thread_manager().g_current_thread, Option::Some(Rc::clone(&thread2))); - debug_assert_eq!(machine.pc, ptr1 + loader2.elf_header.entrypoint); + debug_assert_eq!(system.get_thread_manager().g_current_thread, Option::Some(Rc::clone(&thread1))); + debug_assert_eq!(machine.pc, loader.elf_header.entrypoint); - machine.run(); + machine.run(system); } } \ No newline at end of file diff --git a/src/main.rs b/src/main.rs index 9fe5976..73a439f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -18,6 +18,6 @@ use simulator::machine::Machine; fn main() { let mut machine = Machine::init_machine(); - let system = System::default(); - machine.run() + let mut system = System::default(); + machine.run(&mut system); } diff --git a/src/simulator/machine.rs b/src/simulator/machine.rs index cf320a5..123b43b 100644 --- a/src/simulator/machine.rs +++ b/src/simulator/machine.rs @@ -15,13 +15,13 @@ use std::{ io::Write, fs::File }; -use crate::simulator::{ +use crate::{simulator::{ error::MachineError, instruction::{*, self}, interrupt::Interrupt, global::*, register::* -}; +}, kernel::system::System}; use crate::kernel::{ exception @@ -226,11 +226,11 @@ impl Machine { s } - pub fn raise_exception(&mut self, exception: ExceptionType, address : u64) -> Result{ + pub fn raise_exception(&mut self, exception: ExceptionType, address : u64, system: &mut System) -> Result{ self.set_status(MachineStatus::SystemMode); // Handle the interruption - match exception::call(exception, self) { + match exception::call(exception, self, system) { Ok(MachineOk::Shutdown) => { self.set_status(MachineStatus::UserMode); return Ok(MachineOk::Shutdown); @@ -246,9 +246,9 @@ impl Machine { /// ### Parameters /// /// - **machine** which contains a table of instructions - pub fn run(&mut self) { + pub fn run(&mut self, system: &mut System) { loop { - match self.one_instruction() { + match self.one_instruction(system) { Ok(MachineOk::Ok) => println!("hello"), Ok(MachineOk::Shutdown) => break, Err(e) => { if e.to_string().contains("System") { break; } panic!("FATAL at pc {} -> {}", self.pc, e) } @@ -262,7 +262,7 @@ impl Machine { /// ### Parameters /// /// - **machine** which contains a table of instructions and a pc to the actual instruction - pub fn one_instruction(&mut self) -> Result { + pub fn one_instruction(&mut self, system: &mut System) -> Result { if self.main_memory.len() <= self.pc as usize { panic!("ERROR : number max of instructions rushed"); @@ -335,7 +335,7 @@ impl Machine { RISCV_FP => self.fp_instruction(inst), // Treatment for: SYSTEM CALLS - RISCV_SYSTEM => self.raise_exception(ExceptionType::SyscallException, self.pc), + RISCV_SYSTEM => self.raise_exception(ExceptionType::SyscallException, self.pc, system), // Default case _ => Err(format!("{:x}: Unknown opcode\npc: {:x}", inst.opcode, self.pc))? @@ -710,7 +710,8 @@ mod test { let memory_before = mem_cmp::MemChecker::from(get_full_path!("memory", $a)).unwrap(); let memory_after = mem_cmp::MemChecker::from(get_full_path!("memory", &end_file_name)).unwrap(); mem_cmp::MemChecker::fill_memory_from_mem_checker(&memory_before, &mut m); - m.run(); + let mut system = crate::kernel::system::System::default(); + m.run(&mut system); let expected_trace = fs::read_to_string(get_full_path!("reg_trace", $a)).unwrap(); assert!(mem_cmp::MemChecker::compare_machine_memory(&memory_after, &m)); assert!(expected_trace.contains(m.registers_trace.as_str())); From 8b3a3bebe79d9bdff2e97af755014350706d1fb4 Mon Sep 17 00:00:00 2001 From: Quentin Legot Date: Wed, 5 Apr 2023 13:07:10 +0200 Subject: [PATCH 2/6] Fix list::remove when trying to remove first element of the list (SIGSEGV) --- src/kernel/thread_manager.rs | 2 ++ src/utility/list.rs | 24 ++++++++++++++++++++++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/kernel/thread_manager.rs b/src/kernel/thread_manager.rs index 3b558bc..e7b1288 100644 --- a/src/kernel/thread_manager.rs +++ b/src/kernel/thread_manager.rs @@ -141,6 +141,8 @@ impl ThreadManager { let old_status = machine.interrupt.set_status(InterruptStatus::InterruptOff); self.g_thread_to_be_destroyed = Option::Some(Rc::clone(&thread)); self.g_alive.remove(Rc::clone(&thread)); + #[cfg(debug_assertions)] + println!("Sleeping thread {}", thread.borrow().get_name()); // g_objets_addrs->removeObject(self.thread) // a ajouté plus tard self.thread_sleep(machine, Rc::clone(&thread)); machine.interrupt.set_status(old_status); diff --git a/src/utility/list.rs b/src/utility/list.rs index 801a74e..db514de 100644 --- a/src/utility/list.rs +++ b/src/utility/list.rs @@ -113,8 +113,12 @@ impl List { let mut current: *mut Node = self.head; let mut previous: *mut Node = ptr::null_mut(); while !current.is_null() { - if (*current).elem == item { - (*previous).next = (*current).next; + if (&*current).elem == item { + if !previous.is_null() { + (*previous).next = (*current).next; + } else { + self.head = (*current).next; + } drop(Box::from_raw(current).elem); return true; } else { @@ -320,6 +324,22 @@ mod test { assert_eq!(list.peek(), Option::None); } + #[test] + fn remove_test2() { + let mut list = List::default(); + assert_eq!(list.peek(), None); + list.push(1); + list.push(2); + list.push(3); + + assert_eq!(list.contains(&1), true); + list.remove(1); + assert_eq!(list.contains(&1), false); + assert_eq!(list.pop(), Option::Some(2)); + assert_eq!(list.pop(), Option::Some(3)); + assert_eq!(list.peek(), Option::None); + } + #[test] fn miri_test() { let mut list = List::default(); From 69f91170f6a216afa438bc292460295d2af0cae3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Autin?= Date: Wed, 5 Apr 2023 13:34:06 +0200 Subject: [PATCH 3/6] Optimized to avoid cloning heap values --- src/kernel/exception.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/kernel/exception.rs b/src/kernel/exception.rs index 0cf5921..de4defe 100644 --- a/src/kernel/exception.rs +++ b/src/kernel/exception.rs @@ -65,16 +65,12 @@ fn syscall(machine: &mut Machine, system: &mut System) -> Result Ok(MachineOk::Shutdown), SC_EXIT => { - match &system.get_thread_manager().g_current_thread { - Some(th) => { - let th = Rc::clone(th); - system.get_thread_manager().thread_finish(machine, th); - Ok(MachineOk::Ok) - }, - None => { - Err("Current thread is None".into()) - } - } + let th = match &system.get_thread_manager().g_current_thread { + Some(th) => th, + None => Err("Current thread is None".into()) + }?; + system.get_thread_manager().thread_finish(machine, th); + Ok(MachineOk::Ok) }, SC_EXEC => todo!(), SC_JOIN => todo!(), From 3685bb6590ea084a8b2467ef26772a5451bcf5ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Autin?= Date: Wed, 5 Apr 2023 13:41:56 +0200 Subject: [PATCH 4/6] Added general failure case in raise exception and removed break in main loop for system exceptions --- src/simulator/machine.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/simulator/machine.rs b/src/simulator/machine.rs index 123b43b..9baf651 100644 --- a/src/simulator/machine.rs +++ b/src/simulator/machine.rs @@ -227,7 +227,6 @@ impl Machine { } pub fn raise_exception(&mut self, exception: ExceptionType, address : u64, system: &mut System) -> Result{ - self.set_status(MachineStatus::SystemMode); // Handle the interruption match exception::call(exception, self, system) { @@ -235,7 +234,7 @@ impl Machine { self.set_status(MachineStatus::UserMode); return Ok(MachineOk::Shutdown); } - _ => () + _ => Err(format!("Syscall {:?} invalid or not implemented", exception))? } // todo: return error if the syscall code is invalid self.set_status(MachineStatus::UserMode); Ok(MachineOk::Ok) @@ -251,7 +250,7 @@ impl Machine { match self.one_instruction(system) { Ok(MachineOk::Ok) => println!("hello"), Ok(MachineOk::Shutdown) => break, - Err(e) => { if e.to_string().contains("System") { break; } panic!("FATAL at pc {} -> {}", self.pc, e) } + Err(e) => panic!("FATAL at pc {} -> {}", self.pc, e) } self.write_int_register(0, 0); // In case an instruction write on register 0 } From b96808b55f51145086e36a723769d54f87ccba16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Autin?= Date: Wed, 5 Apr 2023 13:44:43 +0200 Subject: [PATCH 5/6] Fixed incorrect flow control --- src/kernel/exception.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/kernel/exception.rs b/src/kernel/exception.rs index de4defe..17599f2 100644 --- a/src/kernel/exception.rs +++ b/src/kernel/exception.rs @@ -67,9 +67,9 @@ fn syscall(machine: &mut Machine, system: &mut System) -> Result { let th = match &system.get_thread_manager().g_current_thread { Some(th) => th, - None => Err("Current thread is None".into()) - }?; - system.get_thread_manager().thread_finish(machine, th); + None => Err("Current thread is None")? + }; + system.get_thread_manager().thread_finish(machine, *th); Ok(MachineOk::Ok) }, SC_EXEC => todo!(), From 2756477e67208aa8d69f46839380845399ac29f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Autin?= Date: Wed, 5 Apr 2023 13:49:32 +0200 Subject: [PATCH 6/6] Project now builds --- src/kernel/exception.rs | 6 +++--- src/simulator/machine.rs | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/kernel/exception.rs b/src/kernel/exception.rs index 17599f2..b020f6e 100644 --- a/src/kernel/exception.rs +++ b/src/kernel/exception.rs @@ -44,7 +44,7 @@ pub const SC_DEBUG: u8 = 34; pub const CONSOLE_OUTPUT: u8 = 1; // todo : returns new types, not just machine errors and machine ok -pub fn call(exception: ExceptionType, machine: &mut Machine, system: &mut System) -> Result { +pub fn call(exception: &ExceptionType, machine: &mut Machine, system: &mut System) -> Result { match exception { ExceptionType::NoException => todo!(), @@ -66,10 +66,10 @@ fn syscall(machine: &mut Machine, system: &mut System) -> Result Ok(MachineOk::Shutdown), SC_EXIT => { let th = match &system.get_thread_manager().g_current_thread { - Some(th) => th, + Some(th) => th.clone(), None => Err("Current thread is None")? }; - system.get_thread_manager().thread_finish(machine, *th); + system.get_thread_manager().thread_finish(machine, th); Ok(MachineOk::Ok) }, SC_EXEC => todo!(), diff --git a/src/simulator/machine.rs b/src/simulator/machine.rs index 9baf651..166fbce 100644 --- a/src/simulator/machine.rs +++ b/src/simulator/machine.rs @@ -34,6 +34,7 @@ use super::error::MachineOk; /// Textual names of the exceptions that can be generated by user program /// execution, for debugging purpose. /// todo: is this really supposed to stand in machine.rs? +#[derive(Debug)] pub enum ExceptionType { /// Everything ok NoException, @@ -229,7 +230,7 @@ impl Machine { pub fn raise_exception(&mut self, exception: ExceptionType, address : u64, system: &mut System) -> Result{ self.set_status(MachineStatus::SystemMode); // Handle the interruption - match exception::call(exception, self, system) { + match exception::call(&exception, self, system) { Ok(MachineOk::Shutdown) => { self.set_status(MachineStatus::UserMode); return Ok(MachineOk::Shutdown);