tests: ASSERT_SIGNAL: Stop exit codes from masquerading as signals

When a child process exits normally (si_code == CLD_EXITED),
siginfo.si_status contains the exit code. When it is killed by a signal
(si_code == CLD_KILLED or CLD_DUMPED), si_status contains the signal
number.  However, assert_signal_internal() returns si_status blindly.
This causes exit codes to be misinterpreted as signal numbers.

This allows failing tests to silently pass if their exit code
numerically coincides with the expected signal. For example, a test
expecting SIGABRT (6) would incorrectly pass if the child simply exited
with status 6 instead of being killed by a signal.

Fix this by checking si_code. Only return si_status as a signal number
if the child was actually killed by a signal (CLD_KILLED or CLD_DUMPED).
If the child exited normally (CLD_EXITED), return 0 to indicate that no
signal occurred.

diff --git a/src/shared/tests.c b/src/shared/tests.c
index 53dda342a467f47ed9d692ee61aa43633ac260aa..9dc0980025e0d2dc211a882d8b0b8b931ea34063 100644 (file)
--- a/src/shared/tests.c
+++ b/src/shared/tests.c
@@ -446,7 +446,15 @@ int assert_signal_internal(void) {
         if (r < 0)
                 return r;
 
-        return siginfo.si_status;
+        /* si_status means different things depending on si_code:
+         * - CLD_EXITED: si_status is the exit code
+         * - CLD_KILLED/CLD_DUMPED: si_status is the signal number that killed the process
+         * We need to return the signal number only if the child was killed by a signal. */
+        if (IN_SET(siginfo.si_code, CLD_KILLED, CLD_DUMPED))
+                return siginfo.si_status;
+
+        /* Child exited normally, return 0 to indicate no signal was received, regardless of actual exit */
+        return 0;
 }
 
 

diff --git a/src/test/test-tests.c b/src/test/test-tests.c
index 40112a6762496e302b2ef2a6fdf0b685c2804372..15e36444d3b27177f0918f711ba0566442cfa73c 100644 (file)
--- a/src/test/test-tests.c
+++ b/src/test/test-tests.c
@@ -137,4 +137,33 @@ TEST(ASSERT_OK_OR) {
         ASSERT_SIGNAL(ASSERT_OK_OR(-1, -2), SIGABRT);
 }
 
+/* Regression test for issue where assert_signal_internal() wasn't checking si_code before returning
+ * si_status.
+ *
+ * In the bug case, siginfo.si_status has different meanings depending on siginfo.si_code:
+ *
+ *   - If si_code == CLD_EXITED: si_status is the exit code (0-255)
+ *   - If si_code == CLD_KILLED/CLD_DUMPED: si_status is the signal number
+ *
+ * In the bug case where st_code is not checked, exit codes would be confused with signal numbers. For
+ * example, if a child exits with code 6, it would incorrectly look like SIGABRT.
+ *
+ * This test verifies that exit codes are NOT confused with signal numbers, even when the exit code
+ * numerically matches a signal number.
+ */
+TEST(ASSERT_SIGNAL_exit_code_vs_signal) {
+        /* These exit codes numerically match common signal numbers, but ASSERT_SIGNAL should correctly
+         * identify them as exit codes (si_code==CLD_EXITED), not signals. The inner ASSERT_SIGNAL expects a
+         * signal but gets an exit code, so it should fail (aborting with SIGABRT), which the outer
+         * ASSERT_SIGNAL then catches. */
+
+        ASSERT_SIGNAL(ASSERT_SIGNAL(_exit(6), SIGABRT), SIGABRT);  /* 6 = SIGABRT */
+        ASSERT_SIGNAL(ASSERT_SIGNAL(_exit(9), SIGKILL), SIGABRT);  /* 9 = SIGKILL */
+        ASSERT_SIGNAL(ASSERT_SIGNAL(_exit(11), SIGSEGV), SIGABRT); /* 11 = SIGSEGV */
+        ASSERT_SIGNAL(ASSERT_SIGNAL(_exit(15), SIGTERM), SIGABRT); /* 15 = SIGTERM */
+
+        /* _exit(0) should not be confused with any signal */
+        ASSERT_SIGNAL(ASSERT_SIGNAL(_exit(0), SIGABRT), SIGABRT);
+}
+
 DEFINE_TEST_MAIN(LOG_INFO);