cshung commented on code in PR #3536:
URL: https://github.com/apache/nuttx-apps/pull/3536#discussion_r3430206604
##########
testing/sig_sp_test/sig_sp_test_main.c:
##########
@@ -0,0 +1,198 @@
+/****************************************************************************
+ * apps/testing/sig_sp_test/sig_sp_test_main.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Test: Verify that modifying SP (REG_R13) in the saved register context
+ * is honored on context restore (exception return path).
+ *
+ * Scenario (simulates runtime unwinding past a trampoline):
+ * 1. Push values 1 and 2 onto the stack (simulating a trampoline push)
+ * 2. Busy-wait for a timer signal (SIGALRM)
+ * 3. In the handler, "emulate a pop" by advancing SP by 4
+ * (simulating a runtime deciding the top frame was a stub)
+ * 4. Redirect PC to resume_after_signal
+ * 5. After signal return, pop a value — it should be 1
+ *
+ * This exercises:
+ * - The SP relocation fix in arm_exception.S
+ * - The backward sliding copy (new SP > old SP, stack shrinks)
+ * - A realistic use case: unwinding past a trampoline frame
+ *
+ * Requirements:
+ * - Flat build only (accesses nxsched_self() for saved_regs)
+ * - ARM architecture (uses ARMv7-M register layout)
+ *
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <signal.h>
+#include <string.h>
+#include <stdint.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <nuttx/arch.h>
+#include <nuttx/sched.h>
+#include <arch/irq.h>
+
+static volatile int g_result = -1;
+static volatile int g_ready = 0;
+
+/* Forward declarations */
+
+void verify_result(uint32_t value);
+
+/****************************************************************************
+ * Name: verify_result
+ *
+ * Description:
+ * Called with the popped value in r0. Verifies it equals 1.
+ ****************************************************************************/
+
+void __attribute__((noinline, used)) verify_result(uint32_t value)
+{
+ printf("sig_sp_test: popped value = %lu (expected 1)\n",
+ (unsigned long)value);
+
+ if (value == 1)
+ {
+ printf("sig_sp_test: PASS\n");
+ g_result = 0;
+ }
+ else
+ {
+ printf("sig_sp_test: FAIL - expected 1, got %lu\n",
+ (unsigned long)value);
+ g_result = 1;
+ }
+
+ exit(g_result);
+}
+
+/****************************************************************************
+ * Name: resume_after_signal
+ *
+ * Description:
+ * We land here after the signal handler adjusts SP and PC.
+ * Stack now has [SP] = 1 (the '2' was skipped by SP += 4).
+ * Pop one value and verify it equals 1.
+ ****************************************************************************/
+
+static void __attribute__((naked, used)) resume_after_signal(void)
+{
+ __asm__ __volatile__(
+ "pop {r0}\n\t"
+ "b verify_result\n\t"
+ );
+}
+
+/****************************************************************************
+ * Name: sigalrm_handler
+ *
+ * Description:
+ * Called from the timer interrupt path (async signal delivery).
+ * Accesses saved_regs via nxsched_self() to modify the context
+ * that will be restored after signal return.
+ ****************************************************************************/
+
+static void sigalrm_handler(int signo, siginfo_t *info, void *ucontext)
+{
+ struct tcb_s *rtcb = nxsched_self();
+ uint32_t *regs;
+
+ (void)signo;
+ (void)info;
+ (void)ucontext;
+
+ regs = rtcb->xcp.saved_regs;
Review Comment:
Thanks for the suggestion. I looked into passing the register context
through ucontext (the 3rd argument to the signal handler).
Currently, `sig_deliver.c` passes NULL for ucontext on all three dispatch
paths (lines 142, 164, 173). To pass the saved register context there, we'd
need to address a few concerns:
1. **Protected/kernel builds**: `saved_regs` resides in kernel memory and
cannot be directly exposed to userspace. Supporting ucontext in protected mode
would require copying the context to the user stack before dispatch and copying
it back after — adding unconditional overhead to every signal delivery, even
when the handler doesn't use ucontext.
2. **NULL on synchronous signals**: `saved_regs` is only populated on the
async path (interrupt → `arm_schedulesigaction`). For synchronous signals
(e.g., `raise()`), it is NULL, so the handler cannot assume ucontext is always
valid.
3. **Cross-architecture scope**: `sig_deliver.c` is arch-independent.
Changing the ucontext passing behavior affects all architectures, which seems
like a larger change than this test PR should drive.
Given these constraints, I think keeping the `BUILD_FLAT` dependency is
appropriate for now. The test validates the SP restoration fix using
`nxsched_self()->xcp.saved_regs`, which is the simplest correct approach for
flat builds. A proper ucontext implementation could be a separate effort if
there's broader interest.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
