From 875b32e8389ac6e06bfb328e992c1865445d96c1 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Sun, 23 Feb 2014 08:25:49 -0600 Subject: ARMv6-M/ARMv7-M: Correct a register handling error in signal delivery (Kernel build mode only). Noted by Mike Smith. --- nuttx/arch/arm/include/armv6-m/irq.h | 5 +- nuttx/arch/arm/include/armv7-m/irq.h | 5 +- nuttx/arch/arm/src/armv6-m/up_schedulesigaction.c | 64 ++++++++++----------- nuttx/arch/arm/src/armv6-m/up_sigdeliver.c | 6 +- nuttx/arch/arm/src/armv7-m/up_schedulesigaction.c | 68 +++++++++++------------ nuttx/arch/arm/src/armv7-m/up_sigdeliver.c | 6 +- 6 files changed, 82 insertions(+), 72 deletions(-) (limited to 'nuttx') diff --git a/nuttx/arch/arm/include/armv6-m/irq.h b/nuttx/arch/arm/include/armv6-m/irq.h index 4538317e3..14f6cc1bb 100644 --- a/nuttx/arch/arm/include/armv6-m/irq.h +++ b/nuttx/arch/arm/include/armv6-m/irq.h @@ -1,7 +1,7 @@ /**************************************************************************** * arch/arm/include/armv6-m/irq.h * - * Copyright (C) 2013 Gregory Nutt. All rights reserved. + * Copyright (C) 2013-2014 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -178,6 +178,9 @@ struct xcptcontext uint32_t saved_pc; uint32_t saved_primask; uint32_t saved_xpsr; +#ifdef CONFIG_NUTTX_KERNEL + uint32_t saved_lr; +#endif # ifdef CONFIG_NUTTX_KERNEL /* This is the saved address to use when returning from a user-space diff --git a/nuttx/arch/arm/include/armv7-m/irq.h b/nuttx/arch/arm/include/armv7-m/irq.h index 30ce8993b..4da338f34 100644 --- a/nuttx/arch/arm/include/armv7-m/irq.h +++ b/nuttx/arch/arm/include/armv7-m/irq.h @@ -1,7 +1,7 @@ /**************************************************************************** * arch/arm/include/armv7-m/irq.h * - * Copyright (C) 2009, 2011-2012 Gregory Nutt. All rights reserved. + * Copyright (C) 2009, 2011-2012, 2014 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -140,6 +140,9 @@ struct xcptcontext uint32_t saved_primask; #endif uint32_t saved_xpsr; +#ifdef CONFIG_NUTTX_KERNEL + uint32_t saved_lr; +#endif # ifdef CONFIG_NUTTX_KERNEL /* This is the saved address to use when returning from a user-space diff --git a/nuttx/arch/arm/src/armv6-m/up_schedulesigaction.c b/nuttx/arch/arm/src/armv6-m/up_schedulesigaction.c index e41a1a33a..308f395b8 100644 --- a/nuttx/arch/arm/src/armv6-m/up_schedulesigaction.c +++ b/nuttx/arch/arm/src/armv6-m/up_schedulesigaction.c @@ -1,7 +1,7 @@ /**************************************************************************** * arch/arm/src/armv6-m/up_schedulesigaction.c * - * Copyright (C) 2013 Gregory Nutt. All rights reserved. + * Copyright (C) 2013-2014 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -116,16 +116,16 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver) flags = irqsave(); - /* First, handle some special cases when the signal is - * being delivered to the currently executing task. + /* First, handle some special cases when the signal is being delivered + * to the currently executing task. */ sdbg("rtcb=0x%p current_regs=0x%p\n", g_readytorun.head, current_regs); if (tcb == (struct tcb_s*)g_readytorun.head) { - /* CASE 1: We are not in an interrupt handler and - * a task is signalling itself for some reason. + /* CASE 1: We are not in an interrupt handler and a task is + * signalling itself for some reason. */ if (!current_regs) @@ -135,30 +135,26 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver) sigdeliver(tcb); } - /* CASE 2: We are in an interrupt handler AND the - * interrupted task is the same as the one that - * must receive the signal, then we will have to modify - * the return state as well as the state in the TCB. - * - * Hmmm... there looks like a latent bug here: The following - * logic would fail in the strange case where we are in an - * interrupt handler, the thread is signalling itself, but - * a context switch to another task has occurred so that - * current_regs does not refer to the thread at g_readytorun.head! + /* CASE 2: We are in an interrupt handler AND the interrupted + * task is the same as the one that must receive the signal, then + * we will have to modify the return state as well as the state in + * the TCB. */ else { - /* Save the return lr and cpsr and one scratch register - * These will be restored by the signal trampoline after - * the signals have been delivered. + /* Save the return PC, CPSR, and PRIMASK registers (and + * perhaps the LR). These will be restored by the signal + * trampoline after the signal has been delivered. */ tcb->xcp.sigdeliver = sigdeliver; tcb->xcp.saved_pc = current_regs[REG_PC]; tcb->xcp.saved_primask = current_regs[REG_PRIMASK]; tcb->xcp.saved_xpsr = current_regs[REG_XPSR]; - +#ifdef CONFIG_NUTTX_KERNEL + tcb->xcp.saved_lr = current_regs[REG_LR]; +#endif /* Then set up to vector to the trampoline with interrupts * disabled. The kernel-space trampoline must run in * privileged thread mode. @@ -168,43 +164,47 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver) current_regs[REG_PRIMASK] = 1; current_regs[REG_XPSR] = ARMV6M_XPSR_T; #ifdef CONFIG_NUTTX_KERNEL - current_regs[REG_XPSR] = EXC_RETURN_PRIVTHR; + current_regs[REG_LR] = EXC_RETURN_PRIVTHR; #endif - - /* And make sure that the saved context in the TCB - * is the same as the interrupt return context. + /* And make sure that the saved context in the TCB is the same + * as the interrupt return context. */ up_savestate(tcb->xcp.regs); } } - /* Otherwise, we are (1) signaling a task is not running - * from an interrupt handler or (2) we are not in an - * interrupt handler and the running task is signalling - * some non-running task. + /* Otherwise, we are (1) signalling a task is not running from an + * interrupt handler or (2) we are not in an interrupt handler and the + * running task is signalling some non-running task. */ else { - /* Save the return lr and cpsr and one scratch register - * These will be restored by the signal trampoline after - * the signals have been delivered. + /* Save the return PS, CPSR and PRIMASK register (a perhaps also + * the LR). These will be restored by the signal trampoline after + * the signal has been delivered. */ tcb->xcp.sigdeliver = sigdeliver; tcb->xcp.saved_pc = tcb->xcp.regs[REG_PC]; tcb->xcp.saved_primask = tcb->xcp.regs[REG_PRIMASK]; tcb->xcp.saved_xpsr = tcb->xcp.regs[REG_XPSR]; +#ifdef CONFIG_NUTTX_KERNEL + tcb->xcp.saved_lr = tcb->xcp.regs[REG_LR]; +#endif /* Then set up to vector to the trampoline with interrupts - * disabled. We must already be in privileged thread mode - * to be here. + * disabled. We must already be in privileged thread mode to be + * here. */ tcb->xcp.regs[REG_PC] = (uint32_t)up_sigdeliver; tcb->xcp.regs[REG_PRIMASK] = 1; tcb->xcp.regs[REG_XPSR] = ARMV6M_XPSR_T; +#ifdef CONFIG_NUTTX_KERNEL + tcb->xcp.regs[REG_LR] = EXC_RETURN_PRIVTHR; +#endif } irqrestore(flags); diff --git a/nuttx/arch/arm/src/armv6-m/up_sigdeliver.c b/nuttx/arch/arm/src/armv6-m/up_sigdeliver.c index 25c25aa08..b38ed228a 100644 --- a/nuttx/arch/arm/src/armv6-m/up_sigdeliver.c +++ b/nuttx/arch/arm/src/armv6-m/up_sigdeliver.c @@ -1,7 +1,7 @@ /**************************************************************************** * arch/arm/src/armv6-m/up_sigdeliver.c * - * Copyright (C) 2013 Gregory Nutt. All rights reserved. + * Copyright (C) 2013-2014 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -109,6 +109,9 @@ void up_sigdeliver(void) regs[REG_PC] = rtcb->xcp.saved_pc; regs[REG_PRIMASK] = rtcb->xcp.saved_primask; regs[REG_XPSR] = rtcb->xcp.saved_xpsr; +#ifdef CONFIG_NUTTX_KERNEL + regs[REG_LR] = rtcb->xcp.saved_lr; +#endif /* Get a local copy of the sigdeliver function pointer. We do this so that * we can nullify the sigdeliver function pointer in the TCB and accept @@ -144,4 +147,3 @@ void up_sigdeliver(void) } #endif /* !CONFIG_DISABLE_SIGNALS */ - diff --git a/nuttx/arch/arm/src/armv7-m/up_schedulesigaction.c b/nuttx/arch/arm/src/armv7-m/up_schedulesigaction.c index f4cde206a..f1885d631 100644 --- a/nuttx/arch/arm/src/armv7-m/up_schedulesigaction.c +++ b/nuttx/arch/arm/src/armv7-m/up_schedulesigaction.c @@ -1,7 +1,7 @@ /**************************************************************************** * arch/arm/src/armv7-m/up_schedulesigaction.c * - * Copyright (C) 2009-2013 Gregory Nutt. All rights reserved. + * Copyright (C) 2009-2014 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -76,7 +76,7 @@ * This function is called by the OS when one or more * signal handling actions have been queued for execution. * The architecture specific code must configure things so - * that the 'igdeliver' callback is executed on the thread + * that the 'sigdeliver' callback is executed on the thread * specified by 'tcb' as soon as possible. * * This function may be called from interrupt handling logic. @@ -116,16 +116,16 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver) flags = irqsave(); - /* First, handle some special cases when the signal is - * being delivered to the currently executing task. + /* First, handle some special cases when the signal is being delivered + * to the currently executing task. */ sdbg("rtcb=0x%p current_regs=0x%p\n", g_readytorun.head, current_regs); if (tcb == (struct tcb_s*)g_readytorun.head) { - /* CASE 1: We are not in an interrupt handler and - * a task is signalling itself for some reason. + /* CASE 1: We are not in an interrupt handler and a task is + * signalling itself for some reason. */ if (!current_regs) @@ -135,23 +135,18 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver) sigdeliver(tcb); } - /* CASE 2: We are in an interrupt handler AND the - * interrupted task is the same as the one that - * must receive the signal, then we will have to modify - * the return state as well as the state in the TCB. - * - * Hmmm... there looks like a latent bug here: The following - * logic would fail in the strange case where we are in an - * interrupt handler, the thread is signalling itself, but - * a context switch to another task has occurred so that - * current_regs does not refer to the thread at g_readytorun.head! + /* CASE 2: We are in an interrupt handler AND the interrupted + * task is the same as the one that must receive the signal, then + * we will have to modify the return state as well as the state in + * the TCB. */ else { - /* Save the return lr and cpsr and one scratch register - * These will be restored by the signal trampoline after - * the signals have been delivered. + /* Save the return PC, CPSR and either the BASEPRI or PRIMASK + * registers (and perhaps also the LR). These will be + * restored by the signal trampoline after the signal has been + * delivered. */ tcb->xcp.sigdeliver = sigdeliver; @@ -162,7 +157,9 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver) tcb->xcp.saved_primask = current_regs[REG_PRIMASK]; #endif tcb->xcp.saved_xpsr = current_regs[REG_XPSR]; - +#ifdef CONFIG_NUTTX_KERNEL + tcb->xcp.saved_lr = current_regs[REG_LR]; +#endif /* Then set up to vector to the trampoline with interrupts * disabled. The kernel-space trampoline must run in * privileged thread mode. @@ -176,28 +173,26 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver) #endif current_regs[REG_XPSR] = ARMV7M_XPSR_T; #ifdef CONFIG_NUTTX_KERNEL - current_regs[REG_XPSR] = EXC_RETURN_PRIVTHR; + current_regs[REG_LR] = EXC_RETURN_PRIVTHR; #endif - - /* And make sure that the saved context in the TCB - * is the same as the interrupt return context. + /* And make sure that the saved context in the TCB is the same + * as the interrupt return context. */ up_savestate(tcb->xcp.regs); } } - /* Otherwise, we are (1) signaling a task is not running - * from an interrupt handler or (2) we are not in an - * interrupt handler and the running task is signalling - * some non-running task. + /* Otherwise, we are (1) signalling a task is not running from an + * interrupt handler or (2) we are not in an interrupt handler and the + * running task is signalling* some non-running task. */ else { - /* Save the return lr and cpsr and one scratch register - * These will be restored by the signal trampoline after - * the signals have been delivered. + /* Save the return PC, CPSR and either the BASEPRI or PRIMASK + * registers (and perhaps also the LR). These will be restored + * by the signal trampoline after the signal has been delivered. */ tcb->xcp.sigdeliver = sigdeliver; @@ -208,10 +203,12 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver) tcb->xcp.saved_primask = tcb->xcp.regs[REG_PRIMASK]; #endif tcb->xcp.saved_xpsr = tcb->xcp.regs[REG_XPSR]; - +#ifdef CONFIG_NUTTX_KERNEL + tcb->xcp.saved_lr = tcb->xcp.regs[REG_LR]; +#endif /* Then set up to vector to the trampoline with interrupts - * disabled. We must already be in privileged thread mode - * to be here. + * disabled. We must already be in privileged thread mode to be + * here. */ tcb->xcp.regs[REG_PC] = (uint32_t)up_sigdeliver; @@ -221,6 +218,9 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver) tcb->xcp.regs[REG_PRIMASK] = 1; #endif tcb->xcp.regs[REG_XPSR] = ARMV7M_XPSR_T; +#ifdef CONFIG_NUTTX_KERNEL + tcb->xcp.regs[REG_LR] = EXC_RETURN_PRIVTHR; +#endif } irqrestore(flags); diff --git a/nuttx/arch/arm/src/armv7-m/up_sigdeliver.c b/nuttx/arch/arm/src/armv7-m/up_sigdeliver.c index b5b6c6cb6..135c44111 100644 --- a/nuttx/arch/arm/src/armv7-m/up_sigdeliver.c +++ b/nuttx/arch/arm/src/armv7-m/up_sigdeliver.c @@ -1,7 +1,7 @@ /**************************************************************************** * arch/arm/src/armv7-m/up_sigdeliver.c * - * Copyright (C) 2009-2010, 2013 Gregory Nutt. All rights reserved. + * Copyright (C) 2009-2010, 2013-2014 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -108,6 +108,9 @@ void up_sigdeliver(void) regs[REG_PRIMASK] = rtcb->xcp.saved_primask; #endif regs[REG_XPSR] = rtcb->xcp.saved_xpsr; +#ifdef CONFIG_NUTTX_KERNEL + regs[REG_LR] = rtcb->xcp.saved_lr; +#endif /* Get a local copy of the sigdeliver function pointer. We do this so that * we can nullify the sigdeliver function pointer in the TCB and accept @@ -147,4 +150,3 @@ void up_sigdeliver(void) } #endif /* !CONFIG_DISABLE_SIGNALS */ - -- cgit v1.2.3