[smart] Fix bugs on lwp kill (#7892)

Signed-off-by: Shell <smokewood@qq.com>

components/lwp/arch/aarch64/cortex-a/lwp_arch.c

@@ -130,7 +130,6 @@ void *arch_signal_ucontext_restore(rt_base_t user_sp)
 
 void *arch_signal_ucontext_save(rt_base_t user_sp, siginfo_t *psiginfo,
                                 struct rt_hw_exp_stack *exp_frame,
-                                rt_base_t elr, rt_base_t spsr,
                                 lwp_sigset_t *save_sig_mask)
 {
     struct signal_ucontext *new_sp;
@@ -147,11 +146,6 @@ void *arch_signal_ucontext_save(rt_base_t user_sp, siginfo_t *psiginfo,
         /* exp frame is already aligned as AAPCS64 required */
         memcpy(&new_sp->frame, exp_frame, sizeof(*exp_frame));
 
-        /* fix the 3 fields in exception frame, so that memcpy will be fine */
-        new_sp->frame.pc = elr;
-        new_sp->frame.cpsr = spsr;
-        new_sp->frame.sp_el0 = user_sp;
-
         /* copy the save_sig_mask */
         memcpy(&new_sp->save_sigmask, save_sig_mask, sizeof(lwp_sigset_t));
 

components/lwp/arch/aarch64/cortex-a/lwp_arch.h

@@ -48,7 +48,6 @@ rt_inline void icache_invalid_all(void)
  */
 void *arch_signal_ucontext_save(rt_base_t user_sp, siginfo_t *psiginfo,
                                 struct rt_hw_exp_stack *exp_frame,
-                                rt_base_t elr, rt_base_t spsr,
                                 lwp_sigset_t *save_sig_mask);
 
 /**

components/lwp/arch/aarch64/cortex-a/lwp_gcc.S

@@ -193,7 +193,7 @@ arch_syscall_exit:
     add sp, sp, #0x40
     RESTORE_FPU sp
 
-/* the sp is reset to the outer most level */
+/* the sp is reset to the outer most level, irq and fiq are disabled */
 START_POINT(arch_ret_to_user)
     /* save exception frame */
     SAVE_FPU sp
@@ -245,11 +245,18 @@ START_POINT(arch_ret_to_user)
 
     /**
      * push 2 dummy words to simulate a exception frame of interrupt
+     * @note in kernel state, the context switch dont saved the context
      */
-    add sp, sp, #-0x10
+    mrs x0, spsr_el1
+    mrs x1, elr_el1
+    stp x1, x0, [sp, #-0x10]!
     mov x0, sp
+    msr daifclr, #3
     bl lwp_thread_signal_catch
-    add sp, sp, #0x10
+    msr daifset, #3
+    ldp x1, x0, [sp], #0x10
+    msr spsr_el1, x0
+    msr elr_el1, x1
 
     /* check debug */
     /* restore exception frame */
@@ -405,7 +412,6 @@ arch_signal_quit:
     msr spsr_el1, x3
 
     ldp x29, x30, [sp], #0x10
-    // msr sp_el0, x29
 
     ldp x28, x29, [sp], #0x10
     msr fpcr, x28
@@ -452,12 +458,9 @@ arch_thread_signal_enter:
      * move exception frame to user stack
      */
     mrs x0, sp_el0
-    mrs x3, elr_el1
+    mov x3, x4
-    mov x5, x4
-    /** FIXME: spsr must restore from exception frame */
-    mrs x4, spsr_el1
 
-    /* arch_signal_ucontext_save(user_sp, psiginfo, exp_frame, elr, spsr, save_sig_mask); */
+    /* arch_signal_ucontext_save(user_sp, psiginfo, exp_frame, save_sig_mask); */
     bl arch_signal_ucontext_save
 
     dc cvau, x0
@@ -469,7 +472,16 @@ arch_thread_signal_enter:
      * @brief Prepare the environment for signal handler
      */
 
-    /** drop exp frame on kernel stack, reset kernel sp */
+    /** 
+     * reset the cpsr
+     * and drop exp frame on kernel stack, reset kernel sp
+     *
+     * @note Since we will reset spsr, but the reschedule will
+     * corrupt the spsr, we diable irq for a short period here
+     */
+    msr daifset, #3
+    ldr x1, [x20, #CONTEXT_OFFSET_SPSR_EL1]
+    msr spsr_el1, x1
     add sp, x20, #CONTEXT_SIZE
 
     /** reset user sp */

components/lwp/lwp_pid.c

@@ -983,26 +983,31 @@ void lwp_terminate(struct rt_lwp *lwp)
         return;
     }
 
+    LOG_D("%s(lwp=%p \"%s\")", __func__, lwp, lwp->cmd);
+
     level = rt_hw_interrupt_disable();
 
     /* stop the receiving of signals */
-    lwp->terminated = RT_TRUE;
+    if (!lwp->terminated)
-
-    /* broadcast exit request for sibling threads */
-    for (list = lwp->t_grp.next; list != &lwp->t_grp; list = list->next)
     {
-        rt_thread_t thread;
+        lwp->terminated = RT_TRUE;
 
-        thread = rt_list_entry(list, struct rt_thread, sibling);
+        /* broadcast exit request for sibling threads */
-        if (thread->exit_request == LWP_EXIT_REQUEST_NONE)
+        for (list = lwp->t_grp.next; list != &lwp->t_grp; list = list->next)
         {
-            thread->exit_request = LWP_EXIT_REQUEST_TRIGGERED;
+            rt_thread_t thread;
-        }
+
-        if ((thread->stat & RT_THREAD_SUSPEND_MASK) == RT_THREAD_SUSPEND_MASK)
+            thread = rt_list_entry(list, struct rt_thread, sibling);
-        {
+            if (thread->exit_request == LWP_EXIT_REQUEST_NONE)
-            thread->error = RT_EINTR;
+            {
-            rt_hw_dsb();
+                thread->exit_request = LWP_EXIT_REQUEST_TRIGGERED;
-            rt_thread_wakeup(thread);
+            }
+            if ((thread->stat & RT_THREAD_SUSPEND_MASK) == RT_THREAD_SUSPEND_MASK)
+            {
+                thread->error = RT_EINTR;
+                rt_hw_dsb();
+                rt_thread_wakeup(thread);
+            }
         }
     }
     rt_hw_interrupt_enable(level);
@@ -1031,6 +1036,7 @@ void lwp_wait_subthread_exit(void)
     while (1)
     {
         int subthread_is_terminated;
+        LOG_D("%s: wait for subthread exiting", __func__);
 
         level = rt_hw_interrupt_disable();
         subthread_is_terminated = (int)(thread->sibling.prev == &lwp->t_grp);

components/lwp/lwp_signal.c

@@ -546,32 +546,36 @@ void lwp_thread_signal_catch(void *exp_frame)
          * @note that the p_usi is release before entering signal action by
          * reseting the kernel sp.
          */
+        LOG_D("%s: enter signal handler(signo=%d) at %p", __func__, signo, handler);
         arch_thread_signal_enter(signo, p_usi, exp_frame, handler, &save_sig_mask);
         /* the arch_thread_signal_enter() never return */
         RT_ASSERT(0);
     }
 }
-
 static int _do_signal_wakeup(rt_thread_t thread, int sig)
 {
     int need_schedule;
-    if ((thread->stat & RT_THREAD_SUSPEND_MASK) == RT_THREAD_SUSPEND_MASK)
+    if (!_sigismember(&thread->signal.sigset_mask, sig))
     {
-
+        if ((thread->stat & RT_THREAD_SUSPEND_MASK) == RT_THREAD_SUSPEND_MASK)
-        if ((thread->stat & RT_SIGNAL_COMMON_WAKEUP_MASK) != RT_SIGNAL_COMMON_WAKEUP_MASK)
         {
-            rt_thread_wakeup(thread);
+            if ((thread->stat & RT_SIGNAL_COMMON_WAKEUP_MASK) != RT_SIGNAL_COMMON_WAKEUP_MASK)
-            need_schedule = 1;
+            {
-        }
+                rt_thread_wakeup(thread);
-        else if ((sig == SIGKILL) && ((thread->stat & RT_SIGNAL_KILL_WAKEUP_MASK) != RT_SIGNAL_KILL_WAKEUP_MASK))
+                need_schedule = 1;
-        {
+            }
-            rt_thread_wakeup(thread);
+            else if ((sig == SIGKILL) && ((thread->stat & RT_SIGNAL_KILL_WAKEUP_MASK) != RT_SIGNAL_KILL_WAKEUP_MASK))
-            need_schedule = 1;
+            {
+                rt_thread_wakeup(thread);
+                need_schedule = 1;
+            }
+            else
+            {
+                need_schedule = 0;
+            }
         }
         else
-        {
             need_schedule = 0;
-        }
     }
     else
         need_schedule = 0;
@@ -633,7 +637,7 @@ static int _siginfo_deliver_to_lwp(struct rt_lwp *lwp, lwp_siginfo_t siginfo)
     return _do_signal_wakeup(catcher, siginfo->ksiginfo.signo);
 }
 
-static int _siginfo_deliver_to_thread(struct rt_lwp *lwp, rt_thread_t thread, lwp_siginfo_t siginfo)
+static int _siginfo_deliver_to_thread(rt_thread_t thread, lwp_siginfo_t siginfo)
 {
     sigqueue_enqueue(_SIGQ(thread), siginfo);
     return _do_signal_wakeup(thread, siginfo->ksiginfo.signo);
@@ -679,6 +683,9 @@ rt_err_t lwp_signal_kill(struct rt_lwp *lwp, long signo, long code, long value)
     }
     else
     {
+        LOG_D("%s(lwp=%p \"%s\",signo=%ld,code=%ld,value=%ld)",
+              __func__, lwp, lwp->cmd, signo, code, value);
+
         need_schedule = RT_FALSE;
 
         /* FIXME: acquire READ lock to lwp */
@@ -848,7 +855,7 @@ rt_err_t lwp_thread_signal_kill(rt_thread_t thread, long signo, long code, long
 
             if (siginfo)
             {
-                need_schedule = _siginfo_deliver_to_thread(lwp, thread, siginfo);
+                need_schedule = _siginfo_deliver_to_thread(thread, siginfo);
                 ret = 0;
             }
             else
@@ -943,10 +950,10 @@ static int _dequeue_signal(rt_thread_t thread, lwp_sigset_t *mask, siginfo_t *us
 rt_err_t lwp_thread_signal_timedwait(rt_thread_t thread, lwp_sigset_t *sigset,
                                      siginfo_t *usi, struct timespec *timeout)
 {
-    LOG_D("%s", __func__);
-
     rt_base_t level;
     rt_err_t ret;
+    lwp_sigset_t saved_sigset;
+    lwp_sigset_t blocked_sigset;
     int sig;
 
     /**
@@ -960,6 +967,7 @@ rt_err_t lwp_thread_signal_timedwait(rt_thread_t thread, lwp_sigset_t *sigset,
     _sigdelset(sigset, SIGSTOP);
     _signotsets(sigset, sigset);
 
+
     /* FIXME: acquire READ lock to lwp */
     level = rt_hw_interrupt_disable();
     sig = _dequeue_signal(thread, sigset, usi);
@@ -967,17 +975,19 @@ rt_err_t lwp_thread_signal_timedwait(rt_thread_t thread, lwp_sigset_t *sigset,
     if (sig)
         return sig;
 
-    /* WARNING atomic problem, what if pending signal arrives before we sleep */
-
     /**
      * @brief POSIX
      * if none of the signals specified by set are pending, sigtimedwait() shall
      * wait for the time interval specified in the timespec structure referenced
      * by timeout.
+     *
+     * @note If the pending signal arrives before thread suspend, the suspend
+     * operation will return a failure
      */
+    _sigandsets(&blocked_sigset, &thread->signal.sigset_mask, sigset);
+    _thread_signal_mask(thread, LWP_SIG_MASK_CMD_SET_MASK, &blocked_sigset, &saved_sigset);
     if (timeout)
     {
-        /* TODO: verify timeout valid ? not overflow 32bits, nanosec valid, ... */
         rt_uint32_t time;
         time = rt_timespec_to_tick(timeout);
 
@@ -1006,9 +1016,14 @@ rt_err_t lwp_thread_signal_timedwait(rt_thread_t thread, lwp_sigset_t *sigset,
     if (ret == RT_EOK)
     {
         rt_schedule();
-        ret = -EAGAIN;
+        /* If thread->error reliable? */
+        if (thread->error == -RT_EINTR)
+            ret = -EINTR;
+        else
+            ret = -EAGAIN;
     }
     /* else ret == -EINTR */
+    _thread_signal_mask(thread, LWP_SIG_MASK_CMD_SET_MASK, &saved_sigset, RT_NULL);
 
     /* FIXME: acquire READ lock to lwp */
     level = rt_hw_interrupt_disable();

components/lwp/lwp_syscall.c

@@ -347,10 +347,11 @@ void sys_exit(int value)
         lwp_put_to_user(clear_child_tid, &t, sizeof t);
         sys_futex(clear_child_tid, FUTEX_WAKE, 1, RT_NULL, RT_NULL, 0);
     }
+    lwp_terminate(lwp);
+
     main_thread = rt_list_entry(lwp->t_grp.prev, struct rt_thread, sibling);
     if (main_thread == tid)
     {
-        lwp_terminate(lwp);
         lwp_wait_subthread_exit();
         lwp->lwp_ret = value;
     }
@@ -3495,6 +3496,7 @@ sysret_t sys_sigtimedwait(const sigset_t *sigset, siginfo_t *info, const struct
     }
     else
     {
+        /* if sigset of user is smaller, clear extra space */
         memset(&lwpset, 0, sizeof(lwpset));
     }
 
@@ -3527,7 +3529,7 @@ sysret_t sys_sigtimedwait(const sigset_t *sigset, siginfo_t *info, const struct
 
     sig = lwp_thread_signal_timedwait(rt_thread_self(), &lwpset, &kinfo, ptimeout);
 
-    if (info)
+    if (sig > 0 && info)
     {
         if (!lwp_user_accessable((void *)info, sizeof(*info)))
             return -EFAULT;