arch: fix the issue of asynchronous signal processing

hujun260 · jerpelea · commit 0f243bde339b · 2023-05-08T09:11:14.000+02:00
in SMP, signal processing cannot be nested, we use xcp.sigdeliver to identify whether there is currently a signal being processed, but this state does not match the actual situation
One possible scenario is that signal processing has already been completed, but an interrupt occurs, resulting in xcp.sigdeliver not being correctly set to NULL,
At this point, a new signal arrives, which can only be placed in the queue and cannot be processed immediately
Our solution is that signal processing and signal complete status are set in the same critical section, which can ensure status synchronization

Signed-off-by: hujun5 &lt;hujun5@xiaomi.com&gt;
diff --git a/arch/arm/src/armv6-m/arm_sigdeliver.c b/arch/arm/src/armv6-m/arm_sigdeliver.c
@@ -71,14 +71,15 @@ void arm_sigdeliver(void)
         rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head);
   DEBUGASSERT(rtcb->xcp.sigdeliver != NULL);
 
+retry:
 #ifdef CONFIG_SMP
   /* In the SMP case, up_schedule_sigaction(0) will have incremented
    * 'irqcount' in order to force us into a critical section.  Save the
    * pre-incremented irqcount.
    */
 
-  saved_irqcount = rtcb->irqcount - 1;
-  DEBUGASSERT(saved_irqcount >= 0);
+  saved_irqcount = rtcb->irqcount;
+  DEBUGASSERT(saved_irqcount >= 1);
 
   /* Now we need call leave_critical_section() repeatedly to get the irqcount
    * to zero, freeing all global spinlocks that enforce the critical section.
@@ -129,6 +130,12 @@ void arm_sigdeliver(void)
   up_irq_save();
 #endif
 
+  if (!sq_empty(&rtcb->sigpendactionq) &&
+      (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0)
+    {
+      goto retry;
+    }
+
   /* Modify the saved return state with the actual saved values in the
    * TCB.  This depends on the fact that nested signal handling is
    * not supported.  Therefore, these values will persist throughout the
@@ -146,5 +153,8 @@ void arm_sigdeliver(void)
    */
 
   board_autoled_off(LED_SIGNAL);
+#ifdef CONFIG_SMP
+  rtcb->irqcount--;
+#endif
   arm_fullcontextrestore(regs);
 }
diff --git a/arch/arm/src/armv7-a/arm_sigdeliver.c b/arch/arm/src/armv7-a/arm_sigdeliver.c
@@ -71,14 +71,15 @@ void arm_sigdeliver(void)
         rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head);
   DEBUGASSERT(rtcb->xcp.sigdeliver != NULL);
 
+retry:
 #ifdef CONFIG_SMP
   /* In the SMP case, up_schedule_sigaction(0) will have incremented
    * 'irqcount' in order to force us into a critical section.  Save the
    * pre-incremented irqcount.
    */
 
-  saved_irqcount = rtcb->irqcount - 1;
-  DEBUGASSERT(saved_irqcount >= 0);
+  saved_irqcount = rtcb->irqcount;
+  DEBUGASSERT(saved_irqcount >= 1);
 
   /* Now we need call leave_critical_section() repeatedly to get the irqcount
    * to zero, freeing all global spinlocks that enforce the critical section.
@@ -129,6 +130,12 @@ void arm_sigdeliver(void)
   up_irq_save();
 #endif
 
+  if (!sq_empty(&rtcb->sigpendactionq) &&
+      (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0)
+    {
+      goto retry;
+    }
+
   /* Modify the saved return state with the actual saved values in the
    * TCB.  This depends on the fact that nested signal handling is
    * not supported.  Therefore, these values will persist throughout the
@@ -144,5 +151,8 @@ void arm_sigdeliver(void)
   /* Then restore the correct state for this thread of execution. */
 
   board_autoled_off(LED_SIGNAL);
+#ifdef CONFIG_SMP
+  rtcb->irqcount--;
+#endif
   arm_fullcontextrestore(regs);
 }
diff --git a/arch/arm/src/armv7-m/arm_sigdeliver.c b/arch/arm/src/armv7-m/arm_sigdeliver.c
@@ -71,14 +71,15 @@ void arm_sigdeliver(void)
         rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head);
   DEBUGASSERT(rtcb->xcp.sigdeliver != NULL);
 
+retry:
 #ifdef CONFIG_SMP
   /* In the SMP case, up_schedule_sigaction(0) will have incremented
    * 'irqcount' in order to force us into a critical section.  Save the
    * pre-incremented irqcount.
    */
 
-  saved_irqcount = rtcb->irqcount - 1;
-  DEBUGASSERT(saved_irqcount >= 0);
+  saved_irqcount = rtcb->irqcount;
+  DEBUGASSERT(saved_irqcount >= 1);
 
   /* Now we need call leave_critical_section() repeatedly to get the irqcount
    * to zero, freeing all global spinlocks that enforce the critical section.
@@ -133,6 +134,12 @@ void arm_sigdeliver(void)
   up_irq_save();
 #endif
 
+  if (!sq_empty(&rtcb->sigpendactionq) &&
+      (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0)
+    {
+      goto retry;
+    }
+
   /* Modify the saved return state with the actual saved values in the
    * TCB.  This depends on the fact that nested signal handling is
    * not supported.  Therefore, these values will persist throughout the
@@ -150,5 +157,8 @@ void arm_sigdeliver(void)
    */
 
   board_autoled_off(LED_SIGNAL);
+#ifdef CONFIG_SMP
+  rtcb->irqcount--;
+#endif
   arm_fullcontextrestore(regs);
 }
diff --git a/arch/arm/src/armv7-r/arm_sigdeliver.c b/arch/arm/src/armv7-r/arm_sigdeliver.c
@@ -71,14 +71,15 @@ void arm_sigdeliver(void)
         rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head);
   DEBUGASSERT(rtcb->xcp.sigdeliver != NULL);
 
+retry:
 #ifdef CONFIG_SMP
   /* In the SMP case, up_schedule_sigaction(0) will have incremented
    * 'irqcount' in order to force us into a critical section.  Save the
    * pre-incremented irqcount.
    */
 
-  saved_irqcount = rtcb->irqcount - 1;
-  DEBUGASSERT(saved_irqcount >= 0);
+  saved_irqcount = rtcb->irqcount;
+  DEBUGASSERT(saved_irqcount >= 1);
 
   /* Now we need call leave_critical_section() repeatedly to get the irqcount
    * to zero, freeing all global spinlocks that enforce the critical section.
@@ -126,6 +127,12 @@ void arm_sigdeliver(void)
   up_irq_save();
 #endif
 
+  if (!sq_empty(&rtcb->sigpendactionq) &&
+      (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0)
+    {
+      goto retry;
+    }
+
   /* Modify the saved return state with the actual saved values in the
    * TCB.  This depends on the fact that nested signal handling is
    * not supported.  Therefore, these values will persist throughout the
@@ -141,5 +148,8 @@ void arm_sigdeliver(void)
   /* Then restore the correct state for this thread of execution. */
 
   board_autoled_off(LED_SIGNAL);
+#ifdef CONFIG_SMP
+  rtcb->irqcount--;
+#endif
   arm_fullcontextrestore(regs);
 }
diff --git a/arch/arm/src/armv8-m/arm_sigdeliver.c b/arch/arm/src/armv8-m/arm_sigdeliver.c
@@ -71,14 +71,15 @@ void arm_sigdeliver(void)
         rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head);
   DEBUGASSERT(rtcb->xcp.sigdeliver != NULL);
 
+retry:
 #ifdef CONFIG_SMP
   /* In the SMP case, up_schedule_sigaction(0) will have incremented
    * 'irqcount' in order to force us into a critical section.  Save the
    * pre-incremented irqcount.
    */
 
-  saved_irqcount = rtcb->irqcount - 1;
-  DEBUGASSERT(saved_irqcount >= 0);
+  saved_irqcount = rtcb->irqcount;
+  DEBUGASSERT(saved_irqcount >= 1);
 
   /* Now we need call leave_critical_section() repeatedly to get the irqcount
    * to zero, freeing all global spinlocks that enforce the critical section.
@@ -133,6 +134,12 @@ void arm_sigdeliver(void)
   up_irq_save();
 #endif
 
+  if (!sq_empty(&rtcb->sigpendactionq) &&
+      (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0)
+    {
+      goto retry;
+    }
+
   /* Modify the saved return state with the actual saved values in the
    * TCB.  This depends on the fact that nested signal handling is
    * not supported.  Therefore, these values will persist throughout the
@@ -150,5 +157,8 @@ void arm_sigdeliver(void)
    */
 
   board_autoled_off(LED_SIGNAL);
+#ifdef CONFIG_SMP
+  rtcb->irqcount--;
+#endif
   arm_fullcontextrestore(regs);
 }
diff --git a/arch/arm64/src/common/arm64_sigdeliver.c b/arch/arm64/src/common/arm64_sigdeliver.c
@@ -77,14 +77,15 @@ void arm64_sigdeliver(void)
         rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head);
   DEBUGASSERT(rtcb->xcp.sigdeliver != NULL);
 
+retry:
 #ifdef CONFIG_SMP
   /* In the SMP case, up_schedule_sigaction(0) will have incremented
    * 'irqcount' in order to force us into a critical section.  Save the
    * pre-incremented irqcount.
    */
 
-  saved_irqcount = rtcb->irqcount - 1;
-  DEBUGASSERT(saved_irqcount >= 0);
+  saved_irqcount = rtcb->irqcount;
+  DEBUGASSERT(saved_irqcount >= 1);
 
   /* Now we need call leave_critical_section() repeatedly to get the irqcount
    * to zero, freeing all global spinlocks that enforce the critical section.
@@ -135,6 +136,12 @@ void arm64_sigdeliver(void)
   up_irq_save();
 #endif
 
+  if (!sq_empty(&rtcb->sigpendactionq) &&
+      (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0)
+    {
+      goto retry;
+    }
+
   /* Modify the saved return state with the actual saved values in the
    * TCB.  This depends on the fact that nested signal handling is
    * not supported.  Therefore, these values will persist throughout the
@@ -155,5 +162,8 @@ void arm64_sigdeliver(void)
 
   /* Then restore the correct state for this thread of execution. */
 
+#ifdef CONFIG_SMP
+  rtcb->irqcount--;
+#endif
   arm64_fullcontextrestore(rtcb->xcp.regs);
 }
diff --git a/arch/risc-v/src/common/riscv_sigdeliver.c b/arch/risc-v/src/common/riscv_sigdeliver.c
@@ -72,14 +72,15 @@ void riscv_sigdeliver(void)
         rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head);
   DEBUGASSERT(rtcb->xcp.sigdeliver != NULL);
 
+retry:
 #ifdef CONFIG_SMP
   /* In the SMP case, up_schedule_sigaction(0) will have incremented
    * 'irqcount' in order to force us into a critical section.  Save the
    * pre-incremented irqcount.
    */
 
-  saved_irqcount = rtcb->irqcount - 1;
-  DEBUGASSERT(saved_irqcount >= 0);
+  saved_irqcount = rtcb->irqcount;
+  DEBUGASSERT(saved_irqcount >= 1);
 
   /* Now we need call leave_critical_section() repeatedly to get the irqcount
    * to zero, freeing all global spinlocks that enforce the critical section.
@@ -128,6 +129,12 @@ void riscv_sigdeliver(void)
   up_irq_save();
 #endif
 
+  if (!sq_empty(&rtcb->sigpendactionq) &&
+      (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0)
+    {
+      goto retry;
+    }
+
   /* Modify the saved return state with the actual saved values in the
    * TCB.  This depends on the fact that nested signal handling is
    * not supported.  Therefore, these values will persist throughout the
@@ -145,5 +152,8 @@ void riscv_sigdeliver(void)
    */
 
   board_autoled_off(LED_SIGNAL);
+#ifdef CONFIG_SMP
+  rtcb->irqcount--;
+#endif
   riscv_fullcontextrestore(regs);
 }
diff --git a/arch/sim/src/sim/sim_sigdeliver.c b/arch/sim/src/sim/sim_sigdeliver.c
@@ -52,7 +52,15 @@
 void sim_sigdeliver(void)
 {
   struct tcb_s *rtcb = current_task(this_cpu());
+#ifdef CONFIG_SMP
+  /* In the SMP case, we must terminate the critical section while the signal
+   * handler executes, but we also need to restore the irqcount when the
+   * we resume the main thread of the task.
+   */
 
+  int16_t saved_irqcount;
+  irqstate_t flags;
+#endif
   if (NULL == (rtcb->xcp.sigdeliver))
     {
       return;
@@ -63,16 +71,7 @@ void sim_sigdeliver(void)
    */
 
 #ifdef CONFIG_SMP
-  irqstate_t flags = enter_critical_section();
-#endif
-
-#ifdef CONFIG_SMP
-  /* In the SMP case, we must terminate the critical section while the signal
-   * handler executes, but we also need to restore the irqcount when the
-   * we resume the main thread of the task.
-   */
-
-  int16_t saved_irqcount;
+  flags = enter_critical_section();
 #endif
 
   sinfo("rtcb=%p sigdeliver=%p sigpendactionq.head=%p\n",
@@ -81,14 +80,15 @@ void sim_sigdeliver(void)
 
   /* NOTE: we do not save the return state for sim */
 
+retry:
 #ifdef CONFIG_SMP
   /* In the SMP case, up_schedule_sigaction(0) will have incremented
    * 'irqcount' in order to force us into a critical section.  Save the
    * pre-incremented irqcount.
    */
 
   saved_irqcount = rtcb->irqcount;
-  DEBUGASSERT(saved_irqcount >= 0);
+  DEBUGASSERT(saved_irqcount >= 1);
 
   /* Now we need call leave_critical_section() repeatedly to get the irqcount
    * to zero, freeing all global spinlocks that enforce the critical section.
@@ -127,6 +127,12 @@ void sim_sigdeliver(void)
     }
 #endif
 
+  if (!sq_empty(&rtcb->sigpendactionq) &&
+      (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0)
+    {
+      goto retry;
+    }
+
   /* Allows next handler to be scheduled */
 
   rtcb->xcp.sigdeliver = NULL;
diff --git a/arch/sparc/src/sparc_v8/sparc_v8_sigdeliver.c b/arch/sparc/src/sparc_v8/sparc_v8_sigdeliver.c
@@ -84,14 +84,15 @@ void sparc_sigdeliver(void)
 
   sparc_copystate(regs, rtcb->xcp.regs);
 
+retry:
 #ifdef CONFIG_SMP
   /* In the SMP case, up_schedule_sigaction(0) will have incremented
    * 'irqcount' in order to force us into a critical section.  Save the
    * pre-incremented irqcount.
    */
 
-  saved_irqcount = rtcb->irqcount - 1;
-  DEBUGASSERT(saved_irqcount >= 0);
+  saved_irqcount = rtcb->irqcount;
+  DEBUGASSERT(saved_irqcount >= 1);
 
   /* Now we need call leave_critical_section() repeatedly to get the irqcount
    * to zero, freeing all global spinlocks that enforce the critical section.
@@ -144,6 +145,12 @@ void sparc_sigdeliver(void)
 
   set_errno(saved_errno);
 
+  if (!sq_empty(&rtcb->sigpendactionq) &&
+      (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0)
+    {
+      goto retry;
+    }
+
   /* Modify the saved return state with the actual saved values in the
    * TCB.  This depends on the fact that nested signal handling is
    * not supported.  Therefore, these values will persist throughout the
@@ -195,5 +202,8 @@ void sparc_sigdeliver(void)
    */
 
   board_autoled_off(LED_SIGNAL);
+#ifdef CONFIG_SMP
+  rtcb->irqcount--;
+#endif
   sparc_fullcontextrestore(regs);
 }
diff --git a/arch/xtensa/src/common/xtensa_sigdeliver.c b/arch/xtensa/src/common/xtensa_sigdeliver.c
