From 39adecfcd84250f7e382c220ec7bcb2b0faa5193 Mon Sep 17 00:00:00 2001 From: Chris Down Date: Wed, 19 Nov 2025 16:50:38 +0800 Subject: [PATCH] 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. --- src/shared/tests.c | 10 +++++++++- src/test/test-tests.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/shared/tests.c b/src/shared/tests.c index 53dda342a46..9dc0980025e 100644 --- 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 40112a67624..15e36444d3b 100644 --- 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); -- 2.47.3