From 93bb67090c1cf8779a2609bd7a0bcaa5ead08f3d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 16 Nov 2023 10:48:37 +0900 Subject: [PATCH 1/3] log: introduce a knob to make assert_return() critical These can be used to check if we trigger assert_return() unexpectedly. Co-authored-by: Frantisek Sumsal --- src/basic/log.c | 13 +++++++++++++ src/basic/log.h | 3 +++ 2 files changed, 16 insertions(+) diff --git a/src/basic/log.c b/src/basic/log.c index 34e0ccd956..8bcdfa03e5 100644 --- a/src/basic/log.c +++ b/src/basic/log.c @@ -69,6 +69,7 @@ static bool upgrade_syslog_to_journal = false; static bool always_reopen_console = false; static bool open_when_needed = false; static bool prohibit_ipc = false; +static bool assert_return_is_critical = false; /* Akin to glibc's __abort_msg; which is private and we hence cannot * use here. */ @@ -981,6 +982,10 @@ void log_assert_failed_return( const char *file, int line, const char *func) { + + if (assert_return_is_critical) + log_assert_failed(text, file, line, func); + PROTECT_ERRNO; log_assert(LOG_DEBUG, text, file, line, func, "Assertion '%s' failed at %s:%u, function %s(). Ignoring."); @@ -1232,6 +1237,14 @@ static int log_set_ratelimit_kmsg_from_string(const char *e) { return 0; } +void log_set_assert_return_is_critical(bool b) { + assert_return_is_critical = b; +} + +bool log_get_assert_return_is_critical(void) { + return assert_return_is_critical; +} + static int parse_proc_cmdline_item(const char *key, const char *value, void *data) { /* diff --git a/src/basic/log.h b/src/basic/log.h index 9008d47390..140e501eb1 100644 --- a/src/basic/log.h +++ b/src/basic/log.h @@ -331,6 +331,9 @@ void log_set_open_when_needed(bool b); * stderr, the console or kmsg */ void log_set_prohibit_ipc(bool b); +void log_set_assert_return_is_critical(bool b); +bool log_get_assert_return_is_critical(void) _pure_; + int log_dup_console(void); int log_syntax_internal( From 8161f6086e56dc494a976a6eba14d3d4286ef6a0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 24 Dec 2023 01:49:57 +0900 Subject: [PATCH 2/3] test: make assert_return() critical by default on fuzzer and unit tests Several test cases intentionally trigger assert_return(). So, to avoid the entire test fails, this introduces several macros that tentatively make assert_return() not critical. --- src/fuzz/fuzz.h | 1 + src/libsystemd-network/test-dhcp-client.c | 12 ++-- src/libsystemd-network/test-dhcp-server.c | 12 ++-- src/libsystemd-network/test-ipv4ll.c | 35 ++++----- src/libsystemd-network/test-ndisc-ra.c | 72 +++++++++---------- src/libsystemd/sd-bus/test-bus-cleanup.c | 2 +- src/libsystemd/sd-bus/test-bus-error.c | 4 +- src/libsystemd/sd-bus/test-bus-marshal.c | 4 +- src/libsystemd/sd-journal/test-journal-init.c | 7 +- src/libsystemd/sd-login/test-login.c | 10 +-- src/libudev/test-libudev.c | 3 + src/shared/tests.c | 1 + src/shared/tests.h | 21 ++++++ src/test/test-id128.c | 4 +- src/test/test-log.c | 21 ++++++ src/test/test-sd-hwdb.c | 8 +-- 16 files changed, 132 insertions(+), 85 deletions(-) diff --git a/src/fuzz/fuzz.h b/src/fuzz/fuzz.h index 698ba42d2f..7afe38ed1f 100644 --- a/src/fuzz/fuzz.h +++ b/src/fuzz/fuzz.h @@ -31,6 +31,7 @@ static inline bool outside_size_range(size_t size, size_t lower, size_t upper) { static inline void fuzz_setup_logging(void) { /* We don't want to fill the logs and slow down stuff when running * in a fuzzing mode, so disable most of the logging. */ + log_set_assert_return_is_critical(true); log_set_max_level(LOG_CRIT); log_parse_environment(); log_open(); diff --git a/src/libsystemd-network/test-dhcp-client.c b/src/libsystemd-network/test-dhcp-client.c index e3f148daf5..7c5b282fe2 100644 --- a/src/libsystemd-network/test-dhcp-client.c +++ b/src/libsystemd-network/test-dhcp-client.c @@ -57,14 +57,14 @@ static void test_request_basic(sd_event *e) { r = sd_dhcp_client_attach_event(client, e, 0); assert_se(r >= 0); - assert_se(sd_dhcp_client_set_request_option(NULL, 0) == -EINVAL); - assert_se(sd_dhcp_client_set_request_address(NULL, NULL) == -EINVAL); - assert_se(sd_dhcp_client_set_ifindex(NULL, 0) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_dhcp_client_set_request_option(NULL, 0) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_dhcp_client_set_request_address(NULL, NULL) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_dhcp_client_set_ifindex(NULL, 0) == -EINVAL); assert_se(sd_dhcp_client_set_ifindex(client, 15) == 0); - assert_se(sd_dhcp_client_set_ifindex(client, -42) == -EINVAL); - assert_se(sd_dhcp_client_set_ifindex(client, -1) == -EINVAL); - assert_se(sd_dhcp_client_set_ifindex(client, 0) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_dhcp_client_set_ifindex(client, -42) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_dhcp_client_set_ifindex(client, -1) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_dhcp_client_set_ifindex(client, 0) == -EINVAL); assert_se(sd_dhcp_client_set_ifindex(client, 1) == 0); assert_se(sd_dhcp_client_set_hostname(client, "host") == 1); diff --git a/src/libsystemd-network/test-dhcp-server.c b/src/libsystemd-network/test-dhcp-server.c index b2e6034b42..e04a819eea 100644 --- a/src/libsystemd-network/test-dhcp-server.c +++ b/src/libsystemd-network/test-dhcp-server.c @@ -17,7 +17,7 @@ static void test_pool(struct in_addr *address, unsigned size, int ret) { assert_se(sd_dhcp_server_new(&server, 1) >= 0); - assert_se(sd_dhcp_server_configure_pool(server, address, 8, 0, size) == ret); + ASSERT_RETURN_IS_CRITICAL(ret >= 0, assert_se(sd_dhcp_server_configure_pool(server, address, 8, 0, size) == ret)); } static int test_basic(bool bind_to_interface) { @@ -41,20 +41,20 @@ static int test_basic(bool bind_to_interface) { server->bind_to_interface = bind_to_interface; assert_se(sd_dhcp_server_attach_event(server, event, 0) >= 0); - assert_se(sd_dhcp_server_attach_event(server, event, 0) == -EBUSY); + ASSERT_RETURN_EXPECTED_SE(sd_dhcp_server_attach_event(server, event, 0) == -EBUSY); assert_se(sd_dhcp_server_get_event(server) == event); assert_se(sd_dhcp_server_detach_event(server) >= 0); assert_se(!sd_dhcp_server_get_event(server)); assert_se(sd_dhcp_server_attach_event(server, NULL, 0) >= 0); - assert_se(sd_dhcp_server_attach_event(server, NULL, 0) == -EBUSY); + ASSERT_RETURN_EXPECTED_SE(sd_dhcp_server_attach_event(server, NULL, 0) == -EBUSY); assert_se(sd_dhcp_server_ref(server) == server); assert_se(!sd_dhcp_server_unref(server)); - assert_se(sd_dhcp_server_start(server) == -EUNATCH); + ASSERT_RETURN_EXPECTED_SE(sd_dhcp_server_start(server) == -EUNATCH); - assert_se(sd_dhcp_server_configure_pool(server, &address_any, 28, 0, 0) == -EINVAL); - assert_se(sd_dhcp_server_configure_pool(server, &address_lo, 38, 0, 0) == -ERANGE); + ASSERT_RETURN_EXPECTED_SE(sd_dhcp_server_configure_pool(server, &address_any, 28, 0, 0) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_dhcp_server_configure_pool(server, &address_lo, 38, 0, 0) == -ERANGE); assert_se(sd_dhcp_server_configure_pool(server, &address_lo, 8, 0, 0) >= 0); assert_se(sd_dhcp_server_configure_pool(server, &address_lo, 8, 0, 0) >= 0); diff --git a/src/libsystemd-network/test-ipv4ll.c b/src/libsystemd-network/test-ipv4ll.c index bb42930cf5..e7dbd7f36f 100644 --- a/src/libsystemd-network/test-ipv4ll.c +++ b/src/libsystemd-network/test-ipv4ll.c @@ -85,33 +85,34 @@ static void test_public_api_setters(sd_event *e) { assert_se(sd_ipv4ll_new(&ll) == 0); assert_se(ll); - assert_se(sd_ipv4ll_attach_event(NULL, NULL, 0) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_ipv4ll_attach_event(NULL, NULL, 0) == -EINVAL); assert_se(sd_ipv4ll_attach_event(ll, e, 0) == 0); - assert_se(sd_ipv4ll_attach_event(ll, e, 0) == -EBUSY); + ASSERT_RETURN_EXPECTED_SE(sd_ipv4ll_attach_event(ll, e, 0) == -EBUSY); - assert_se(sd_ipv4ll_set_callback(NULL, NULL, NULL) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_ipv4ll_set_callback(NULL, NULL, NULL) == -EINVAL); assert_se(sd_ipv4ll_set_callback(ll, NULL, NULL) == 0); - assert_se(sd_ipv4ll_set_address(ll, &address) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_ipv4ll_set_address(ll, &address) == -EINVAL); address.s_addr |= htobe32(169U << 24 | 254U << 16); - assert_se(sd_ipv4ll_set_address(ll, &address) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_ipv4ll_set_address(ll, &address) == -EINVAL); address.s_addr |= htobe32(0x00FF); - assert_se(sd_ipv4ll_set_address(ll, &address) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_ipv4ll_set_address(ll, &address) == -EINVAL); address.s_addr |= htobe32(0xF000); assert_se(sd_ipv4ll_set_address(ll, &address) == 0); address.s_addr |= htobe32(0x0F00); - assert_se(sd_ipv4ll_set_address(ll, &address) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_ipv4ll_set_address(ll, &address) == -EINVAL); - assert_se(sd_ipv4ll_set_address_seed(NULL, seed) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_ipv4ll_set_address_seed(NULL, seed) == -EINVAL); assert_se(sd_ipv4ll_set_address_seed(ll, seed) == 0); - assert_se(sd_ipv4ll_set_mac(NULL, NULL) == -EINVAL); - assert_se(sd_ipv4ll_set_mac(ll, NULL) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_ipv4ll_set_mac(NULL, NULL) == -EINVAL); + + ASSERT_RETURN_EXPECTED_SE(sd_ipv4ll_set_mac(ll, NULL) == -EINVAL); assert_se(sd_ipv4ll_set_mac(ll, &mac_addr) == 0); - assert_se(sd_ipv4ll_set_ifindex(NULL, -1) == -EINVAL); - assert_se(sd_ipv4ll_set_ifindex(ll, -1) == -EINVAL); - assert_se(sd_ipv4ll_set_ifindex(ll, -99) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_ipv4ll_set_ifindex(NULL, -1) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_ipv4ll_set_ifindex(ll, -1) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_ipv4ll_set_ifindex(ll, -99) == -EINVAL); assert_se(sd_ipv4ll_set_ifindex(ll, 1) == 0); assert_se(sd_ipv4ll_ref(ll) == ll); @@ -134,17 +135,17 @@ static void test_basic_request(sd_event *e, const struct in_addr *start_address) assert_se(sd_ipv4ll_new(&ll) == 0); if (in4_addr_is_set(start_address)) assert_se(sd_ipv4ll_set_address(ll, start_address) >= 0); - assert_se(sd_ipv4ll_start(ll) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_ipv4ll_start(ll) == -EINVAL); assert_se(sd_ipv4ll_attach_event(ll, e, 0) == 0); - assert_se(sd_ipv4ll_start(ll) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_ipv4ll_start(ll) == -EINVAL); assert_se(sd_ipv4ll_set_mac(ll, &mac_addr) == 0); - assert_se(sd_ipv4ll_start(ll) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_ipv4ll_start(ll) == -EINVAL); assert_se(sd_ipv4ll_set_callback(ll, basic_request_handler, basic_request_handler_userdata) == 0); - assert_se(sd_ipv4ll_start(ll) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_ipv4ll_start(ll) == -EINVAL); assert_se(sd_ipv4ll_set_ifindex(ll, 1) == 0); assert_se(sd_ipv4ll_start(ll) == 1); diff --git a/src/libsystemd-network/test-ndisc-ra.c b/src/libsystemd-network/test-ndisc-ra.c index 23abe780d6..442b139f9b 100644 --- a/src/libsystemd-network/test-ndisc-ra.c +++ b/src/libsystemd-network/test-ndisc-ra.c @@ -103,26 +103,26 @@ TEST(radv_prefix) { assert_se(sd_radv_prefix_new(&p) >= 0); - assert_se(sd_radv_prefix_set_onlink(NULL, true) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_prefix_set_onlink(NULL, true) < 0); assert_se(sd_radv_prefix_set_onlink(p, true) >= 0); assert_se(sd_radv_prefix_set_onlink(p, false) >= 0); - assert_se(sd_radv_prefix_set_address_autoconfiguration(NULL, true) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_prefix_set_address_autoconfiguration(NULL, true) < 0); assert_se(sd_radv_prefix_set_address_autoconfiguration(p, true) >= 0); assert_se(sd_radv_prefix_set_address_autoconfiguration(p, false) >= 0); - assert_se(sd_radv_prefix_set_valid_lifetime(NULL, 1, 1) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_prefix_set_valid_lifetime(NULL, 1, 1) < 0); assert_se(sd_radv_prefix_set_valid_lifetime(p, 0, 0) >= 0); assert_se(sd_radv_prefix_set_valid_lifetime(p, 300 * USEC_PER_SEC, USEC_INFINITY) >= 0); assert_se(sd_radv_prefix_set_valid_lifetime(p, 300 * USEC_PER_SEC, USEC_PER_YEAR) >= 0); - assert_se(sd_radv_prefix_set_preferred_lifetime(NULL, 1, 1) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_prefix_set_preferred_lifetime(NULL, 1, 1) < 0); assert_se(sd_radv_prefix_set_preferred_lifetime(p, 0, 0) >= 0); assert_se(sd_radv_prefix_set_preferred_lifetime(p, 300 * USEC_PER_SEC, USEC_INFINITY) >= 0); assert_se(sd_radv_prefix_set_preferred_lifetime(p, 300 * USEC_PER_SEC, USEC_PER_YEAR) >= 0); - assert_se(sd_radv_prefix_set_prefix(NULL, NULL, 0) < 0); - assert_se(sd_radv_prefix_set_prefix(p, NULL, 0) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_prefix_set_prefix(NULL, NULL, 0) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_prefix_set_prefix(p, NULL, 0) < 0); assert_se(sd_radv_prefix_set_prefix(p, &prefix[0].address, 64) >= 0); assert_se(sd_radv_prefix_set_prefix(p, &prefix[0].address, 0) < 0); @@ -131,8 +131,8 @@ TEST(radv_prefix) { assert_se(sd_radv_prefix_set_prefix(p, &prefix[0].address, 3) >= 0); assert_se(sd_radv_prefix_set_prefix(p, &prefix[0].address, 125) >= 0); assert_se(sd_radv_prefix_set_prefix(p, &prefix[0].address, 128) >= 0); - assert_se(sd_radv_prefix_set_prefix(p, &prefix[0].address, 129) < 0); - assert_se(sd_radv_prefix_set_prefix(p, &prefix[0].address, 255) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_prefix_set_prefix(p, &prefix[0].address, 129) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_prefix_set_prefix(p, &prefix[0].address, 255) < 0); assert_se(!sd_radv_prefix_unref(p)); } @@ -142,13 +142,13 @@ TEST(radv_route_prefix) { assert_se(sd_radv_route_prefix_new(&p) >= 0); - assert_se(sd_radv_route_prefix_set_lifetime(NULL, 1, 1) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_route_prefix_set_lifetime(NULL, 1, 1) < 0); assert_se(sd_radv_route_prefix_set_lifetime(p, 0, 0) >= 0); assert_se(sd_radv_route_prefix_set_lifetime(p, 300 * USEC_PER_SEC, USEC_INFINITY) >= 0); assert_se(sd_radv_route_prefix_set_lifetime(p, 300 * USEC_PER_SEC, USEC_PER_YEAR) >= 0); - assert_se(sd_radv_route_prefix_set_prefix(NULL, NULL, 0) < 0); - assert_se(sd_radv_route_prefix_set_prefix(p, NULL, 0) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_route_prefix_set_prefix(NULL, NULL, 0) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_route_prefix_set_prefix(p, NULL, 0) < 0); assert_se(sd_radv_route_prefix_set_prefix(p, &prefix[0].address, 64) >= 0); assert_se(sd_radv_route_prefix_set_prefix(p, &prefix[0].address, 0) >= 0); @@ -157,8 +157,8 @@ TEST(radv_route_prefix) { assert_se(sd_radv_route_prefix_set_prefix(p, &prefix[0].address, 3) >= 0); assert_se(sd_radv_route_prefix_set_prefix(p, &prefix[0].address, 125) >= 0); assert_se(sd_radv_route_prefix_set_prefix(p, &prefix[0].address, 128) >= 0); - assert_se(sd_radv_route_prefix_set_prefix(p, &prefix[0].address, 129) < 0); - assert_se(sd_radv_route_prefix_set_prefix(p, &prefix[0].address, 255) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_route_prefix_set_prefix(p, &prefix[0].address, 129) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_route_prefix_set_prefix(p, &prefix[0].address, 255) < 0); assert_se(!sd_radv_route_prefix_unref(p)); } @@ -168,8 +168,8 @@ TEST(radv_pref64_prefix) { assert_se(sd_radv_pref64_prefix_new(&p) >= 0); - assert_se(sd_radv_pref64_prefix_set_prefix(NULL, NULL, 0, 0) < 0); - assert_se(sd_radv_pref64_prefix_set_prefix(p, NULL, 0, 0) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_pref64_prefix_set_prefix(NULL, NULL, 0, 0) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_pref64_prefix_set_prefix(p, NULL, 0, 0) < 0); assert_se(sd_radv_pref64_prefix_set_prefix(p, &prefix[0].address, 32, 300 * USEC_PER_SEC) >= 0); assert_se(sd_radv_pref64_prefix_set_prefix(p, &prefix[0].address, 40, 300 * USEC_PER_SEC) >= 0); @@ -190,37 +190,37 @@ TEST(radv) { assert_se(sd_radv_new(&ra) >= 0); assert_se(ra); - assert_se(sd_radv_set_ifindex(NULL, 0) < 0); - assert_se(sd_radv_set_ifindex(ra, 0) < 0); - assert_se(sd_radv_set_ifindex(ra, -1) < 0); - assert_se(sd_radv_set_ifindex(ra, -2) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_set_ifindex(NULL, 0) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_set_ifindex(ra, 0) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_set_ifindex(ra, -1) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_set_ifindex(ra, -2) < 0); assert_se(sd_radv_set_ifindex(ra, 42) >= 0); - assert_se(sd_radv_set_mac(NULL, NULL) < 0); - assert_se(sd_radv_set_mac(ra, NULL) >= 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_set_mac(NULL, NULL) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_set_mac(ra, NULL) >= 0); assert_se(sd_radv_set_mac(ra, &mac_addr) >= 0); - assert_se(sd_radv_set_mtu(NULL, 0) < 0); - assert_se(sd_radv_set_mtu(ra, 0) < 0); - assert_se(sd_radv_set_mtu(ra, 1279) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_set_mtu(NULL, 0) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_set_mtu(ra, 0) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_set_mtu(ra, 1279) < 0); assert_se(sd_radv_set_mtu(ra, 1280) >= 0); assert_se(sd_radv_set_mtu(ra, ~0) >= 0); - assert_se(sd_radv_set_hop_limit(NULL, 0) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_set_hop_limit(NULL, 0) < 0); assert_se(sd_radv_set_hop_limit(ra, 0) >= 0); assert_se(sd_radv_set_hop_limit(ra, ~0) >= 0); - assert_se(sd_radv_set_router_lifetime(NULL, 0) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_set_router_lifetime(NULL, 0) < 0); assert_se(sd_radv_set_router_lifetime(ra, 0) >= 0); assert_se(sd_radv_set_router_lifetime(ra, USEC_INFINITY) < 0); assert_se(sd_radv_set_router_lifetime(ra, USEC_PER_YEAR) < 0); assert_se(sd_radv_set_router_lifetime(ra, 300 * USEC_PER_SEC) >= 0); - assert_se(sd_radv_set_preference(NULL, 0) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_set_preference(NULL, 0) < 0); assert_se(sd_radv_set_preference(ra, SD_NDISC_PREFERENCE_LOW) >= 0); assert_se(sd_radv_set_preference(ra, SD_NDISC_PREFERENCE_MEDIUM) >= 0); assert_se(sd_radv_set_preference(ra, SD_NDISC_PREFERENCE_HIGH) >= 0); - assert_se(sd_radv_set_preference(ra, ~0) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_set_preference(ra, ~0) < 0); assert_se(sd_radv_set_preference(ra, SD_NDISC_PREFERENCE_HIGH) >= 0); assert_se(sd_radv_set_router_lifetime(ra, 300 * USEC_PER_SEC) >= 0); @@ -228,22 +228,22 @@ TEST(radv) { assert_se(sd_radv_set_preference(ra, SD_NDISC_PREFERENCE_MEDIUM) >= 0); assert_se(sd_radv_set_router_lifetime(ra, 0) >= 0); - assert_se(sd_radv_set_managed_information(NULL, true) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_set_managed_information(NULL, true) < 0); assert_se(sd_radv_set_managed_information(ra, true) >= 0); assert_se(sd_radv_set_managed_information(ra, false) >= 0); - assert_se(sd_radv_set_other_information(NULL, true) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_set_other_information(NULL, true) < 0); assert_se(sd_radv_set_other_information(ra, true) >= 0); assert_se(sd_radv_set_other_information(ra, false) >= 0); - assert_se(sd_radv_set_retransmit(NULL, 10 * USEC_PER_MSEC) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_set_retransmit(NULL, 10 * USEC_PER_MSEC) < 0); assert_se(sd_radv_set_retransmit(ra, 10 * USEC_PER_MSEC) >= 0); assert_se(sd_radv_set_retransmit(ra, 0) >= 0); assert_se(sd_radv_set_retransmit(ra, usec_add(UINT32_MAX * USEC_PER_MSEC, USEC_PER_MSEC)) < 0); - assert_se(sd_radv_set_rdnss(NULL, 0, NULL, 0) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_set_rdnss(NULL, 0, NULL, 0) < 0); assert_se(sd_radv_set_rdnss(ra, 0, NULL, 0) >= 0); - assert_se(sd_radv_set_rdnss(ra, 0, NULL, 128) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_set_rdnss(ra, 0, NULL, 128) < 0); assert_se(sd_radv_set_rdnss(ra, 600 * USEC_PER_SEC, &test_rdnss, 0) >= 0); assert_se(sd_radv_set_rdnss(ra, 600 * USEC_PER_SEC, &test_rdnss, 1) >= 0); assert_se(sd_radv_set_rdnss(ra, 0, &test_rdnss, 1) >= 0); @@ -254,15 +254,15 @@ TEST(radv) { assert_se(sd_radv_set_dnssl(ra, 0, (char **)test_dnssl) >= 0); assert_se(sd_radv_set_dnssl(ra, 600 * USEC_PER_SEC, (char **)test_dnssl) >= 0); - assert_se(sd_radv_set_home_agent_information(NULL, true) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_set_home_agent_information(NULL, true) < 0); assert_se(sd_radv_set_home_agent_information(ra, true) >= 0); assert_se(sd_radv_set_home_agent_information(ra, false) >= 0); - assert_se(sd_radv_set_home_agent_preference(NULL, 10) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_set_home_agent_preference(NULL, 10) < 0); assert_se(sd_radv_set_home_agent_preference(ra, 10) >= 0); assert_se(sd_radv_set_home_agent_preference(ra, 0) >= 0); - assert_se(sd_radv_set_home_agent_lifetime(NULL, 300 * USEC_PER_SEC) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_radv_set_home_agent_lifetime(NULL, 300 * USEC_PER_SEC) < 0); assert_se(sd_radv_set_home_agent_lifetime(ra, 300 * USEC_PER_SEC) >= 0); assert_se(sd_radv_set_home_agent_lifetime(ra, 0) >= 0); assert_se(sd_radv_set_home_agent_lifetime(ra, USEC_PER_DAY) < 0); diff --git a/src/libsystemd/sd-bus/test-bus-cleanup.c b/src/libsystemd/sd-bus/test-bus-cleanup.c index 3e14627c75..b569986a94 100644 --- a/src/libsystemd/sd-bus/test-bus-cleanup.c +++ b/src/libsystemd/sd-bus/test-bus-cleanup.c @@ -30,7 +30,7 @@ static void test_bus_fork(void) { r = safe_fork("(bus-fork-test)", FORK_WAIT|FORK_LOG, NULL); if (r == 0) { assert_se(bus); - assert_se(sd_bus_is_ready(bus) == -ECHILD); + ASSERT_RETURN_EXPECTED_SE(sd_bus_is_ready(bus) == -ECHILD); assert_se(sd_bus_flush_close_unref(bus) == NULL); assert_se(sd_bus_close_unref(bus) == NULL); assert_se(sd_bus_unref(bus) == NULL); diff --git a/src/libsystemd/sd-bus/test-bus-error.c b/src/libsystemd/sd-bus/test-bus-error.c index a55f3f9856..91045c06c2 100644 --- a/src/libsystemd/sd-bus/test-bus-error.c +++ b/src/libsystemd/sd-bus/test-bus-error.c @@ -213,8 +213,8 @@ TEST(errno_mapping_custom) { assert_se(sd_bus_error_set(NULL, BUS_ERROR_NO_SUCH_UNIT, NULL) == -ENOENT); - assert_se(sd_bus_error_add_map(test_errors_bad1) == -EINVAL); - assert_se(sd_bus_error_add_map(test_errors_bad2) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_bus_error_add_map(test_errors_bad1) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_bus_error_add_map(test_errors_bad2) == -EINVAL); } TEST(sd_bus_error_set_errnof) { diff --git a/src/libsystemd/sd-bus/test-bus-marshal.c b/src/libsystemd/sd-bus/test-bus-marshal.c index 0044d33a59..92da6272b7 100644 --- a/src/libsystemd/sd-bus/test-bus-marshal.c +++ b/src/libsystemd/sd-bus/test-bus-marshal.c @@ -42,8 +42,8 @@ static void test_bus_path_encode(void) { assert_se(sd_bus_path_decode(a, "/waldo", &b) == 0 && b == NULL); assert_se(sd_bus_path_decode(a, "/foo/bar", &b) > 0 && streq(b, "waldo")); - assert_se(sd_bus_path_encode("xxxx", "waldo", &c) < 0); - assert_se(sd_bus_path_encode("/foo/", "waldo", &c) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_bus_path_encode("xxxx", "waldo", &c) < 0); + ASSERT_RETURN_EXPECTED_SE(sd_bus_path_encode("/foo/", "waldo", &c) < 0); assert_se(sd_bus_path_encode("/foo/bar", "", &c) >= 0 && streq(c, "/foo/bar/_")); assert_se(sd_bus_path_decode(c, "/foo/bar", &d) > 0 && streq(d, "")); diff --git a/src/libsystemd/sd-journal/test-journal-init.c b/src/libsystemd/sd-journal/test-journal-init.c index c8a19779f5..5fe4b7a2d6 100644 --- a/src/libsystemd/sd-journal/test-journal-init.c +++ b/src/libsystemd/sd-journal/test-journal-init.c @@ -45,8 +45,8 @@ int main(int argc, char *argv[]) { r = safe_fork("(journal-fork-test)", FORK_WAIT|FORK_LOG, NULL); if (r == 0) { assert_se(j); - assert_se(sd_journal_get_realtime_usec(j, NULL) == -ECHILD); - assert_se(sd_journal_seek_tail(j) == -ECHILD); + ASSERT_RETURN_EXPECTED_SE(sd_journal_get_realtime_usec(j, NULL) == -ECHILD); + ASSERT_RETURN_EXPECTED_SE(sd_journal_seek_tail(j) == -ECHILD); assert_se(j->current_location.type == LOCATION_HEAD); sd_journal_close(j); _exit(EXIT_SUCCESS); @@ -57,8 +57,7 @@ int main(int argc, char *argv[]) { sd_journal_close(j); j = NULL; - r = sd_journal_open_directory(&j, t, SD_JOURNAL_LOCAL_ONLY); - assert_se(r == -EINVAL); + ASSERT_RETURN_EXPECTED(assert_se(sd_journal_open_directory(&j, t, SD_JOURNAL_LOCAL_ONLY) == -EINVAL)); assert_se(j == NULL); } diff --git a/src/libsystemd/sd-login/test-login.c b/src/libsystemd/sd-login/test-login.c index 313e5e339d..c83c953dd9 100644 --- a/src/libsystemd/sd-login/test-login.c +++ b/src/libsystemd/sd-login/test-login.c @@ -104,7 +104,7 @@ TEST(login) { assert_se(IN_SET(r, 0, -ENOMEDIUM)); } - r = sd_uid_get_display(u2, &display_session); + r = ASSERT_RETURN_IS_CRITICAL(uid_is_valid(u2), sd_uid_get_display(u2, &display_session)); log_info("sd_uid_get_display("UID_FMT", …) → %s / \"%s\"", u2, e(r), strnull(display_session)); if (u2 == UID_INVALID) assert_se(r == -EINVAL); @@ -116,7 +116,7 @@ TEST(login) { sd_peer_get_session(pair[1], &qq); assert_se(streq_ptr(pp, qq)); - r = sd_uid_get_sessions(u2, false, &sessions); + r = ASSERT_RETURN_IS_CRITICAL(uid_is_valid(u2), sd_uid_get_sessions(u2, false, &sessions)); assert_se(t = strv_join(sessions, " ")); log_info("sd_uid_get_sessions("UID_FMT", …) → %s \"%s\"", u2, e(r), t); if (u2 == UID_INVALID) @@ -128,9 +128,9 @@ TEST(login) { sessions = strv_free(sessions); free(t); - assert_se(r == sd_uid_get_sessions(u2, false, NULL)); + assert_se(r == ASSERT_RETURN_IS_CRITICAL(uid_is_valid(u2), sd_uid_get_sessions(u2, false, NULL))); - r = sd_uid_get_seats(u2, false, &seats); + r = ASSERT_RETURN_IS_CRITICAL(uid_is_valid(u2), sd_uid_get_seats(u2, false, &seats)); assert_se(t = strv_join(seats, " ")); log_info("sd_uid_get_seats("UID_FMT", …) → %s \"%s\"", u2, e(r), t); if (u2 == UID_INVALID) @@ -142,7 +142,7 @@ TEST(login) { seats = strv_free(seats); free(t); - assert_se(r == sd_uid_get_seats(u2, false, NULL)); + assert_se(r == ASSERT_RETURN_IS_CRITICAL(uid_is_valid(u2), sd_uid_get_seats(u2, false, NULL))); if (session) { r = sd_session_is_active(session); diff --git a/src/libudev/test-libudev.c b/src/libudev/test-libudev.c index e05a062c8e..99934c6f03 100644 --- a/src/libudev/test-libudev.c +++ b/src/libudev/test-libudev.c @@ -339,6 +339,9 @@ static void test_hwdb(struct udev *udev, const char *modalias) { if (!hwdb) log_warning_errno(errno, "Failed to open hwdb: %m"); + SAVE_ASSERT_RETURN_IS_CRITICAL; + log_set_assert_return_is_critical(hwdb); + udev_list_entry_foreach(entry, udev_hwdb_get_properties_list_entry(hwdb, modalias, 0)) log_info("'%s'='%s'", udev_list_entry_get_name(entry), udev_list_entry_get_value(entry)); diff --git a/src/shared/tests.c b/src/shared/tests.c index 195be70b03..41177f821e 100644 --- a/src/shared/tests.c +++ b/src/shared/tests.c @@ -114,6 +114,7 @@ bool slow_tests_enabled(void) { } void test_setup_logging(int level) { + log_set_assert_return_is_critical(true); log_set_max_level(level); log_parse_environment(); log_open(); diff --git a/src/shared/tests.h b/src/shared/tests.h index d76cf2edbe..d5dfb78678 100644 --- a/src/shared/tests.h +++ b/src/shared/tests.h @@ -10,6 +10,27 @@ #include "static-destruct.h" #include "strv.h" +static inline void log_set_assert_return_is_criticalp(bool *p) { + log_set_assert_return_is_critical(*p); +} + +#define _SAVE_ASSERT_RETURN_IS_CRITICAL(b) \ + _unused_ _cleanup_(log_set_assert_return_is_criticalp) bool b = \ + log_get_assert_return_is_critical() + +#define SAVE_ASSERT_RETURN_IS_CRITICAL \ + _SAVE_ASSERT_RETURN_IS_CRITICAL(UNIQ_T(saved, UNIQ)) + +#define ASSERT_RETURN_IS_CRITICAL(b, expr) \ + ({ \ + SAVE_ASSERT_RETURN_IS_CRITICAL; \ + log_set_assert_return_is_critical(b); \ + expr; \ + }) + +#define ASSERT_RETURN_EXPECTED(expr) ASSERT_RETURN_IS_CRITICAL(false, expr) +#define ASSERT_RETURN_EXPECTED_SE(expr) ASSERT_RETURN_EXPECTED(assert_se(expr)); + static inline bool manager_errno_skip_test(int r) { return IN_SET(abs(r), EPERM, diff --git a/src/test/test-id128.c b/src/test/test-id128.c index ae7df27d8f..f1159ecb15 100644 --- a/src/test/test-id128.c +++ b/src/test/test-id128.c @@ -194,8 +194,8 @@ TEST(id128) { } /* Check return values */ - assert_se(sd_id128_get_app_specific(SD_ID128_ALLF, SD_ID128_NULL, &id) == -ENXIO); - assert_se(sd_id128_get_app_specific(SD_ID128_NULL, SD_ID128_ALLF, &id) == 0); + ASSERT_RETURN_EXPECTED_SE(sd_id128_get_app_specific(SD_ID128_ALLF, SD_ID128_NULL, &id) == -ENXIO); + ASSERT_RETURN_EXPECTED_SE(sd_id128_get_app_specific(SD_ID128_NULL, SD_ID128_ALLF, &id) == 0); } TEST(sd_id128_get_invocation) { diff --git a/src/test/test-log.c b/src/test/test-log.c index b5ba67b74b..97eb5e0199 100644 --- a/src/test/test-log.c +++ b/src/test/test-log.c @@ -22,6 +22,26 @@ assert_cc(!IS_SYNTHETIC_ERRNO(0)); #define X100(x) X10(X10(x)) #define X1000(x) X100(X10(x)) +static int fail_with_EINVAL(void) { + assert_return(false, -EINVAL); + return 0; +} + +static void test_assert_return_is_critical(void) { + SAVE_ASSERT_RETURN_IS_CRITICAL; + + log_set_assert_return_is_critical(false); + assert_se(fail_with_EINVAL() == -EINVAL); + + log_set_assert_return_is_critical(true); + ASSERT_RETURN_IS_CRITICAL(false, assert_se(fail_with_EINVAL() == -EINVAL)); + assert_se(log_get_assert_return_is_critical() == true); + ASSERT_RETURN_EXPECTED(assert_se(fail_with_EINVAL() == -EINVAL)); + assert_se(log_get_assert_return_is_critical() == true); + ASSERT_RETURN_EXPECTED_SE(fail_with_EINVAL() == -EINVAL); + assert_se(log_get_assert_return_is_critical() == true); +} + static void test_file(void) { log_info("__FILE__: %s", __FILE__); log_info("RELATIVE_SOURCE_PATH: %s", RELATIVE_SOURCE_PATH); @@ -207,6 +227,7 @@ static void test_log_prefix(void) { int main(int argc, char* argv[]) { test_setup_logging(LOG_DEBUG); + test_assert_return_is_critical(); test_file(); assert_se(log_info_errno(SYNTHETIC_ERRNO(EUCLEAN), "foo") == -EUCLEAN); diff --git a/src/test/test-sd-hwdb.c b/src/test/test-sd-hwdb.c index ecb6118bca..307609ce32 100644 --- a/src/test/test-sd-hwdb.c +++ b/src/test/test-sd-hwdb.c @@ -18,8 +18,8 @@ TEST(failed_enumerate) { assert_se(sd_hwdb_seek(hwdb, "no-such-modalias-should-exist") == 0); assert_se(sd_hwdb_enumerate(hwdb, &key, &value) == 0); - assert_se(sd_hwdb_enumerate(hwdb, &key, NULL) == -EINVAL); - assert_se(sd_hwdb_enumerate(hwdb, NULL, &value) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_hwdb_enumerate(hwdb, &key, NULL) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_hwdb_enumerate(hwdb, NULL, &value) == -EINVAL); } #define DELL_MODALIAS \ @@ -58,8 +58,8 @@ TEST(sd_hwdb_new_from_path) { _cleanup_(sd_hwdb_unrefp) sd_hwdb *hwdb = NULL; int r; - assert_se(sd_hwdb_new_from_path(NULL, &hwdb) == -EINVAL); - assert_se(sd_hwdb_new_from_path("", &hwdb) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_hwdb_new_from_path(NULL, &hwdb) == -EINVAL); + ASSERT_RETURN_EXPECTED_SE(sd_hwdb_new_from_path("", &hwdb) == -EINVAL); assert_se(sd_hwdb_new_from_path("/path/that/should/not/exist", &hwdb) < 0); NULSTR_FOREACH(hwdb_bin_path, hwdb_bin_paths) { From fce9e8a168ce526614193ea858edccee79650104 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 18 Nov 2023 04:18:44 +0900 Subject: [PATCH 3/3] log: make assert_return() critical when -Dmode=developer Triggering assert_return() should be a bug in general, and we should really fix that. But, previously, it is hard to notice such bug, as it was not critical. This is for making CI or our testing environment fail if we unexpectedly trigger assert_return(). So, hopefully we can easily find such bugs. --- src/basic/log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/log.c b/src/basic/log.c index 8bcdfa03e5..7ddca92afa 100644 --- a/src/basic/log.c +++ b/src/basic/log.c @@ -69,7 +69,7 @@ static bool upgrade_syslog_to_journal = false; static bool always_reopen_console = false; static bool open_when_needed = false; static bool prohibit_ipc = false; -static bool assert_return_is_critical = false; +static bool assert_return_is_critical = BUILD_MODE_DEVELOPER; /* Akin to glibc's __abort_msg; which is private and we hence cannot * use here. */