From 7483dfd7c4a4fe49868dee30c19f285d07606ea9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 30 Nov 2022 09:47:01 +0100 Subject: [PATCH 1/5] std-aux: fix _NM_ASSERT_FAIL_ENABLED for plain assert() and NDEBUG Fixes: 8e3299498dd1 ('std-aux,glib-aux: rework nm_assert() implementations') --- src/libnm-std-aux/nm-std-aux.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libnm-std-aux/nm-std-aux.h b/src/libnm-std-aux/nm-std-aux.h index a8b18ef1b..bef11c86e 100644 --- a/src/libnm-std-aux/nm-std-aux.h +++ b/src/libnm-std-aux/nm-std-aux.h @@ -226,7 +226,7 @@ _nm_assert_fail_internal(const char *assertion, #define _NM_ASSERT_FAIL_ENABLED 1 #define _nm_assert_fail(msg) __assert_fail((msg), __FILE__, __LINE__, __func__) #else -#define _NM_ASSERT_FAIL_ENABLED 1 +#define _NM_ASSERT_FAIL_ENABLED 0 #define _nm_assert_fail(msg) \ do { \ _nm_unused const char *_msg = (msg); \ From fffdb14887629dcaeb72b6d4d8ed30d3d7cc530b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 30 Nov 2022 09:52:10 +0100 Subject: [PATCH 2/5] std-aux: make NM_BOOLEAN_EXPR() a constant expression for constant arguments This allows the compiler to see that nm_assert(0) is unreachable code. That is because nm_assert(0) calls NM_LIKELY(0), which calls NM_BOOLEAN_EXPR(0). The latter was a statement expression, which to the compiler was not a constant expression. Hence, this may trigger compiler warnings about uninitialized variables. Let NM_BOOLEAN_EXPR() to be constant, if the arguments are. This can avoid compiler warnings in some cases. Note that __builtin_choose_expr(__builtin_constant_p(...), ...) does not properly work with gcc 4.8 ([1]). Hence only do macro shenanigans with a newer gcc. Then entire point of NM_BOOLEAN_EXPR() is anyway to preserve the "-Wparentheses" warning (while only evaluating the argument once, being safe with nested invocations, propagate constness). If we don't care about "-Wparentheses", it should be the same as (!!(expr)). We can ignore that on non-recent gcc. [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=19449 --- src/libnm-std-aux/nm-std-aux.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/libnm-std-aux/nm-std-aux.h b/src/libnm-std-aux/nm-std-aux.h index bef11c86e..fa70fde64 100644 --- a/src/libnm-std-aux/nm-std-aux.h +++ b/src/libnm-std-aux/nm-std-aux.h @@ -175,7 +175,15 @@ typedef uint64_t _nm_bitwise nm_be64_t; NM_UNIQ_T(V, v) = 0; \ NM_UNIQ_T(V, v); \ }) -#define NM_BOOLEAN_EXPR(expr) _NM_BOOLEAN_EXPR_IMPL(NM_UNIQ, expr) + +#if defined(__GNUC__) && (__GNUC__ > 4) +#define NM_BOOLEAN_EXPR(expr) \ + __builtin_choose_expr(__builtin_constant_p(expr), \ + (!!(expr)), \ + _NM_BOOLEAN_EXPR_IMPL(NM_UNIQ, expr)) +#else +#define NM_BOOLEAN_EXPR(expr) (!!(expr)) +#endif #if defined(__GNUC__) && (__GNUC__ > 2) && defined(__OPTIMIZE__) #define NM_LIKELY(expr) __builtin_expect(NM_BOOLEAN_EXPR(expr), 1) From 5ac5d7f8c3a5b42191f9b335e5994336c87c1178 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 30 Nov 2022 09:48:04 +0100 Subject: [PATCH 3/5] std-aux: add _nm_unreachable_code() macro to wrap __builtin_unreachable() This is a GCC-ism, but clang and all our current compiler support it. --- src/libnm-std-aux/nm-std-aux.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libnm-std-aux/nm-std-aux.h b/src/libnm-std-aux/nm-std-aux.h index fa70fde64..1d1079456 100644 --- a/src/libnm-std-aux/nm-std-aux.h +++ b/src/libnm-std-aux/nm-std-aux.h @@ -75,6 +75,14 @@ /*****************************************************************************/ +/* This is mainly used in case of failed assertions. Usually assert() itself + * already ensures that the code path is marked as unreachable, however with + * NDEBUG that might not be the case. We want to mark the code as unreachable + * even with NDEBUG/G_DISABLE_ASSERT. */ +#define _nm_unreachable_code() __builtin_unreachable() + +/*****************************************************************************/ + #ifndef _NM_CC_SUPPORT_AUTO_TYPE #if (defined(__GNUC__) && (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 9))) #define _NM_CC_SUPPORT_AUTO_TYPE 1 From 06931221b56e3100f8819c696428d507d0140f30 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 30 Nov 2022 09:49:32 +0100 Subject: [PATCH 4/5] std-aux: mark _nm_assert_fail() as _nm_unreachable_code() with NDEBUG/G_DISABLE_ASSERT This is useful, because it can avoid compiler warnings that are emitted if the compiler things that the code can be reached. _nm_assert_fail() can clearly never be reached (unless a bug happens). When compiling we can disable assertion checks with NDEBUG/G_DISABLE_ASSERT, but if we know that an assertion must not be hit (for example with nm_assert_not_reached()) then we still want to mark the path as unreachable, even if assert() does not abort the process. --- src/libnm-glib-aux/nm-macros-internal.h | 9 +++++++-- src/libnm-std-aux/nm-std-aux.h | 2 ++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/libnm-glib-aux/nm-macros-internal.h b/src/libnm-glib-aux/nm-macros-internal.h index 085a27ff5..b8c754a41 100644 --- a/src/libnm-glib-aux/nm-macros-internal.h +++ b/src/libnm-glib-aux/nm-macros-internal.h @@ -529,8 +529,13 @@ nm_str_realloc(char *str) /* redefine assertions to use g_assert*() */ #undef _nm_assert_fail -#define _nm_assert_fail(msg) \ - g_assertion_message_expr(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, msg) +#define _nm_assert_fail(msg) \ + G_STMT_START \ + { \ + g_assertion_message_expr(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, msg); \ + _nm_unreachable_code(); \ + } \ + G_STMT_END #undef _NM_ASSERT_FAIL_ENABLED #ifndef G_DISABLE_ASSERT diff --git a/src/libnm-std-aux/nm-std-aux.h b/src/libnm-std-aux/nm-std-aux.h index 1d1079456..148cbe619 100644 --- a/src/libnm-std-aux/nm-std-aux.h +++ b/src/libnm-std-aux/nm-std-aux.h @@ -246,6 +246,8 @@ _nm_assert_fail_internal(const char *assertion, #define _nm_assert_fail(msg) \ do { \ _nm_unused const char *_msg = (msg); \ + \ + _nm_unreachable_code(); \ } while (0) #endif From 4753358dd5b4ae94336c6e9b2f170e3c9172a829 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 30 Nov 2022 10:01:17 +0100 Subject: [PATCH 5/5] std-aux: mark failures of nm_assert() as unreachable code - with nm_assert(), if the argument is a compile time constant always check it (regardless of NDEBUG, G_DISABLE_ASSERT) and mark the failure as _nm_unreachable_code(). We do this, even if we usually would not evaluate run time checks with NDEBUG/G_DISABLE_ASSERT. - with nm_assert_se(), if assertions are disabled with NDEBUG and G_DISABLE_ASSERT, still mark the path as _nm_unreachable_code(). --- src/libnm-std-aux/nm-std-aux.h | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/libnm-std-aux/nm-std-aux.h b/src/libnm-std-aux/nm-std-aux.h index 148cbe619..1aeaccbce 100644 --- a/src/libnm-std-aux/nm-std-aux.h +++ b/src/libnm-std-aux/nm-std-aux.h @@ -253,21 +253,26 @@ _nm_assert_fail_internal(const char *assertion, #define NM_MORE_ASSERTS_EFFECTIVE (_NM_ASSERT_FAIL_ENABLED ? NM_MORE_ASSERTS : 0) -#define nm_assert(cond) \ - ({ \ +#define nm_assert(cond) \ + ({ \ /* nm_assert() must do *nothing* of effect, except evaluating * @cond (0 or 1 times). * * As such, nm_assert() is async-signal-safe (provided @cond is, and - * the assertion does not fail). */ \ - if (NM_MORE_ASSERTS_EFFECTIVE == 0) { \ - /* pass */ \ - } else if (NM_LIKELY(cond)) { \ - /* pass */ \ - } else { \ - _nm_assert_fail(#cond); \ - } \ - 1; \ + * the assertion does not fail). */ \ + if (NM_MORE_ASSERTS_EFFECTIVE == 0) { \ + if (__builtin_constant_p(cond) && !(cond)) { \ + /* Constant expressions are still evaluated and result + * in unreachable code. This handles nm_assert(FALSE). */ \ + _nm_unreachable_code(); \ + } \ + /* pass */ \ + } else if (NM_LIKELY(cond)) { \ + /* pass */ \ + } else { \ + _nm_assert_fail(#cond); \ + } \ + 1; \ }) #define nm_assert_se(cond) \ @@ -279,10 +284,11 @@ _nm_assert_fail_internal(const char *assertion, * the assertion does not fail). */ \ if (NM_LIKELY(cond)) { \ /* pass */ \ - } else if (NM_MORE_ASSERTS_EFFECTIVE == 0) { \ - /* pass */ \ } else { \ - _nm_assert_fail(#cond); \ + if (NM_MORE_ASSERTS_EFFECTIVE != 0) { \ + _nm_assert_fail(#cond); \ + } \ + _nm_unreachable_code(); \ } \ 1; \ })