fix(task): task should always call do_exit to become zombie state

Parent can call wait4 to clean zombie children, however, in previous
design, task could be zombie state without calling do_exit function,
which will cause bugs, e.g. children not reparented.

crates/range-map/Cargo.toml

@@ -0,0 +1,9 @@
+[package]
+name = "range-map"
+version = "0.1.0"
+edition = "2021"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+log = "0.4"

kernel/src/mm/memory_space/range_map.rs → crates/range-map/src/lib.rs

@@ -1,6 +1,12 @@
+#![no_std]
+#![no_main]
+#![feature(map_try_insert)]
+
 use alloc::collections::BTreeMap;
 use core::ops::{Add, Range};
 
+extern crate alloc;
+
 #[derive(Clone, Debug)]
 struct Node<U, V> {
     pub end: U,

kernel/Cargo.toml

@@ -23,6 +23,7 @@ net = { path = "../modules/net/" }
 recycle-allocator = { path = "../crates/recycle-allocator/" }
 async-utils = { path = "../crates/async-utils/" }
 sbi-print = { path = "../crates/sbi-print/" }
+range-map = { path = "../crates/range-map/" }
 backtrace = { path = "../crates/backtrace/" }
 
 cfg-if = "1.0"

kernel/src/mm/memory_space/mod.rs

@@ -12,6 +12,7 @@ use core::{
     ops::{self, Range},
 };
 
+use ::range_map::RangeMap;
 use arch::memory::{sfence_vma_all, sfence_vma_vaddr};
 use async_utils::block_on;
 use config::{
@@ -33,7 +34,7 @@ use systype::{SysError, SysResult};
 use vfs_core::{Dentry, File};
 use xmas_elf::ElfFile;
 
-use self::{range_map::RangeMap, vm_area::VmArea};
+use self::vm_area::VmArea;
 use super::{kernel_page_table, PageFaultAccessType};
 use crate::{
     mm::{
@@ -48,7 +49,6 @@ use crate::{
     },
 };
 
-mod range_map;
 pub mod vm_area;
 
 /// Virtual memory space for kernel and user.

kernel/src/syscall/process.rs

@@ -88,7 +88,7 @@ impl Syscall<'_> {
     /// group.
     pub fn sys_exit(&self, exit_code: i32) -> SyscallResult {
         let task = self.task;
-        task.set_zombie();
+        task.set_terminated();
         // non-leader thread are detached (see CLONE_THREAD flag in manual page clone.2)
         if task.is_leader() {
             task.set_exit_code((exit_code & 0xFF) << 8);
@@ -102,8 +102,8 @@ impl Syscall<'_> {
         let task = self.task;
         task.with_thread_group(|tg| {
             for t in tg.iter() {
-                t.set_zombie();
-                log::info!("tid {} set zombie", t.tid());
+                t.set_terminated();
+                log::info!("tid {} set terminated", t.tid());
             }
         });
         task.set_exit_code((exit_code & 0xFF) << 8);

kernel/src/task/schedule.rs

@@ -80,25 +80,25 @@ pub async fn task_loop(task: Arc<Task>) {
     *task.waker() = Some(get_waker().await);
     loop {
         match task.state() {
-            Zombie => break,
+            Terminated => break,
             Stopped => suspend_now().await,
             _ => {}
         }
 
         trap::user_trap::trap_return(&task);
 
-        // task may be set to zombie by other task, e.g. execve will kill other tasks in
-        // the same thread group
+        // task may be set to terminated by other task, e.g. execve will kill other
+        // tasks in the same thread group
         match task.state() {
-            Zombie => break,
+            Terminated => break,
             Stopped => suspend_now().await,
             _ => {}
         }
 
         let intr = trap::user_trap::trap_handler(&task).await;
 
         match task.state() {
-            Zombie => break,
+            Terminated => break,
             Stopped => suspend_now().await,
             _ => {}
         }

kernel/src/task/signal.rs

@@ -256,12 +256,13 @@ fn terminate(task: &Arc<Task>, sig: Sig) {
     // exit all the memers of a thread group
     task.with_thread_group(|tg| {
         for t in tg.iter() {
-            t.set_zombie();
+            t.set_terminated();
         }
     });
     // 将信号放入低7位 (第8位是core dump标志,在gdb调试崩溃程序中用到)
     task.set_exit_code(sig.raw() as i32 & 0x7F);
 }
+
 fn stop(task: &Arc<Task>, sig: Sig) {
     log::warn!("[do_signal] task stopped!");
     task.with_mut_thread_group(|tg| {

kernel/src/task/task.rs

@@ -146,8 +146,13 @@ pub enum TaskState {
     /// The task is currently running or ready to run, occupying the CPU and
     /// executing its code.
     Running,
-    /// The task has terminated, but its process control block (PCB) still
-    /// exists for the parent process to read its exit status.
+    /// The task has been terminated for user mode, which means it will
+    /// never return back to user anymore. All it left to do is to call
+    /// `do_exit` to end its life in kernel mode.
+    Terminated,
+    /// The task has has called `do_exit` function, but its process
+    /// control block (PCB) still exists for the parent process to read its
+    /// exit status.
     Zombie,
     /// The task has been stopped, usually due to receiving a stop signal (e.g.,
     /// SIGSTOP). It can be resumed with a continue signal (e.g., SIGCONT).
@@ -169,7 +174,14 @@ pub enum TaskState {
 
 impl Task {
     // you can use is_running() / set_running()、 is_zombie() / set_zombie()
-    generate_state_methods!(Running, Zombie, Stopped, Interruptable, UnInterruptable);
+    generate_state_methods!(
+        Running,
+        Zombie,
+        Stopped,
+        Terminated,
+        Interruptable,
+        UnInterruptable
+    );
     generate_accessors!(
         waker: Option<Waker>,
         tid_address: TidAddress,
@@ -469,7 +481,7 @@ impl Task {
             let mut pid = 0;
             for t in tg.iter() {
                 if !t.is_leader() {
-                    t.set_zombie();
+                    t.set_terminated();
                 } else {
                     pid = t.tid();
                 }
@@ -523,7 +535,7 @@ impl Task {
     // NOTE: After all of the threads in a thread group is terminated, the parent
     // process of the thread group is sent a SIGCHLD (or other termination) signal.
     // WARN: do not call this function directly if a task should be terminated,
-    // instead, call `set_zombie`
+    // instead, call `set_terminated`
     pub fn do_exit(self: &Arc<Self>) {
         log::info!("thread {} do exit", self.tid());
         assert_ne!(
@@ -551,22 +563,22 @@ impl Task {
 
         let mut tg = self.thread_group.lock();
 
-        if (!self.leader().is_zombie())
+        if (!self.leader().is_terminated())
             || (self.is_leader && tg.len() > 1)
             || (!self.is_leader && tg.len() > 2)
         {
-            if !self.is_leader {
+            if !self.is_leader() {
                 // NOTE: leader will be removed by parent calling `sys_wait4`
                 tg.remove(self);
                 TASK_MANAGER.remove(self.tid());
             }
             return;
         }
 
-        if self.is_leader {
-            debug_assert!(tg.len() == 1);
+        if self.is_leader() {
+            assert!(tg.len() == 1);
         } else {
-            debug_assert!(tg.len() == 2);
+            assert!(tg.len() == 2);
             // NOTE: leader will be removed by parent calling `sys_wait4`
             tg.remove(self);
             TASK_MANAGER.remove(self.tid());
@@ -628,6 +640,14 @@ impl Task {
 
         // TODO: drop most resources here instead of wait4 function parent
         // called
+
+        if self.is_leader() {
+            self.set_zombie();
+        } else {
+            self.leader().set_zombie();
+        }
+        // When the task is not leader, which means its is not a process, it
+        // will get dropped when hart leaves this task.
     }
 
     /// The dirfd argument is used in conjunction with the pathname argument as

kernel/src/trap/user_trap.rs

@@ -98,14 +98,13 @@ pub async fn trap_handler(task: &Arc<Task>) -> bool {
                             },
                             false,
                         );
-                        // task.set_zombie();
                     }
                 }
                 Exception::IllegalInstruction => {
                     log::warn!(
                         "[trap_handler] detected illegal instruction, stval {stval:#x}, sepc {sepc:#x}",
                     );
-                    task.set_zombie();
+                    task.set_terminated();
                 }
                 e => {
                     log::warn!("Unknown user exception: {:?}", e);
@@ -163,7 +162,7 @@ pub fn trap_return(task: &Arc<Task>) {
     task.trap_context_mut().user_fx.restore();
     task.trap_context_mut().sstatus.set_fs(FS::Clean);
     assert!(!task.trap_context_mut().sstatus.sie());
-    assert!(!task.is_zombie());
+    assert!(!task.is_terminated() && !task.is_zombie());
     unsafe {
         __return_to_user(task.trap_context_mut());
         // NOTE: next time when user traps into kernel, it will come back here

modules/memory/src/page_table.rs

@@ -73,8 +73,8 @@ impl PageTable {
             frames: Vec::new(),
         };
         let leaf_pte = page_table.find_pte(vaddr.floor()).unwrap();
-        let paddr = (leaf_pte.ppn().bits() << PAGE_SIZE_BITS) + (vaddr.bits() & PAGE_MASK);
-        paddr.into()
+        let paddr = leaf_pte.ppn().to_pa() + vaddr.page_offset();
+        paddr
     }
 
     /// Switch to this pagetable

user/src/bin/final_tests.rs

@@ -10,17 +10,17 @@ use user_lib::{execve, fork, wait, waitpid};
 #[macro_use]
 extern crate user_lib;
 
-const TESTCASES: [&str; 9] = [
+const TESTCASES: [&str; 10] = [
     "time-test",
     "busybox_testcode.sh",
     "lua_testcode.sh",
     "netperf_testcode.sh",
-    // "cyclictest_testcode.sh",
     "libc-bench",
     "libctest_testcode.sh",
     "iozone_testcode.sh",
     "unixbench_testcode.sh",
     "lmbench_testcode.sh",
+    "cyclictest_testcode.sh",
 ];
 
 fn run_cmd(cmd: &str) {