[GitHub] [nuttx] pussuw commented on a diff in pull request #8227: arch/ARCH_KERNEL_STACK: Fix signal handling with kernel stack

["pussuw (via GitHub)" <gi...@apache.org>]

pussuw commented on code in PR #8227:
URL: https://github.com/apache/nuttx/pull/8227#discussion_r1085666671


##########
arch/risc-v/src/common/riscv_swint.c:
##########
@@ -497,8 +515,16 @@ int riscv_swint(int irq, void *context, void *arg)
           if (index == 0 && rtcb->xcp.kstack != NULL)
             {
               rtcb->xcp.ustkptr = (uintptr_t *)regs[REG_SP];
-              regs[REG_SP]      = (uintptr_t)rtcb->xcp.kstack +
+              if (rtcb->xcp.kstkptr == NULL)

Review Comment:
   The original check (== NULL) is actually correct. However, the original implementation is very difficult to follow and understand. I can't know how it was supposed to work, only that it was broken.
   
   Without this check, i.e. unconditionally setting 
   
   `regs[REG_SP]  = (uintptr_t)rtcb->xcp.kstack + ARCH_KERNEL_STACKSIZE;`
   
   Destroys the entire kernel stack frame, because it resets the stack pointer to stack top. If rtcb->xcp.kstkptr == NULL we know that no one is using the kernel stack atm and we can set the stack to stack top. Otherwise we should not touch regs[REG_SP].



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org