CMake: Rework assertions and add dependency on NDEBUG

Separate enabling assertions from WS_DEBUG. Use NDEBUG if ENABLE_ASSERT
is not defined, to conform to CMake conventions for the build type.

Misc assertion header enhancements.

This partially reverts 25d4a099f7.
This commit is contained in:
João Valverde 2023-09-16 11:59:18 +01:00
parent b7aa23bb3f
commit 4af7a8071c
5 changed files with 34 additions and 32 deletions

View File

@ -517,6 +517,7 @@ set_property(DIRECTORY
"G_DISABLE_SINGLE_INCLUDES"
$<$<OR:$<BOOL:${ENABLE_DEBUG}>,$<CONFIG:Debug>>:WS_DEBUG>
$<$<OR:$<AND:$<BOOL:${ENABLE_DEBUG}>,$<BOOL:${ENABLE_DEBUG_UTF_8}>>,$<CONFIG:Debug>>:WS_DEBUG_UTF_8>
$<$<BOOL:${ENABLE_ASSERT}>:ENABLE_ASSERT>
)
if(WIN32)

View File

@ -50,11 +50,12 @@ option(BUILD_mmdbresolve "Build MaxMind DB resolver" ON)
option(BUILD_fuzzshark "Build fuzzshark" OFF)
option(ENABLE_WERROR "Treat warnings as errors" ON)
# Debugging is enabled for "Debug" and "RelWithDebInfo" build types.
# Debugging is enabled for "Debug" build type.
option(ENABLE_DEBUG "Enable debugging for all build configurations" OFF)
# UTF-8 debugging is enabled for "Debug" and "RelWithDebInfo" build types.
option(ENABLE_DEBUG_UTF_8 "Enable UTF-8 sanity checks (requires ENABLE_DEBUG)" ON)
option(ENABLE_DEBUG_A2W "Enable line directive from .cnf file" OFF)
# Assertions are enabled for "Debug" and "RelWithDebInfo" build types.
option(ENABLE_ASSERT "Enable assertions for all build configurations" OFF)
option(ENABLE_CCACHE "Speed up compiling and linking using ccache if possible" OFF)
option(DISABLE_FRAME_LARGER_THAN_WARNING "Disable warning if the size of a function frame is large" OFF)

View File

@ -235,29 +235,32 @@ CMake can compile Wireshark for several different build types:
|`RelWithDebInfo`
|`-O2 -g`
|Default, build with default optimizations and generate debug symbols.
Does not enable extra debugging code or debug level logs
Enables assertions and disables debug level logs
|`Debug`
|`-O0 -g -DWS_DEBUG -DWS_DEBUG_UTF_8`
|For debugging, no optimization. Enables extra debugging code
|For development, no optimization. Enables assertions
and debug level logs
|`Release`
|`-O3 -DNDEBUG`
|Optimized for speed, no debug symbols or debug level logs
|Optimized for speed, no debug symbols or debug level logs or assertions
|`MinSizeRel`
|`-Os -DNDEBUG`
|Optimized for size, no debug symbols or debug level logs
|Optimized for size, no debug symbols or debug level logs or assertions
|===
The default is `RelWithDebInfo`, which provides a good compromise of
some optimization (`-O2`) along with including debug symbols (`-g`)
for release builds. For normal development coding you probably want to be
using `Debug` build type or set -DENABLE_DEBUG=On, to enable full
<<ChSrcLogging,logging capabilities>> and all assertions.
<<ChSrcLogging,logging capabilities>> and debug code.
CMake will automatically add the -DNDEBUG option to certain build
types but this macro is not used very much in Wireshark code, if at all.
types. This macro is used to disable assertions but it can be overruled
using ENABLE_ASSERT, which can be used to unconditionally enable assertions
if defined.
To change the build type, set the CMake variable `CMAKE_BUILD_TYPE`, e.g.:

View File

@ -212,12 +212,6 @@ get_compiled_version_info(gather_feature_func gather_compile)
#ifdef WS_DEBUG
g_string_append(str, ", debug build");
#else
g_string_append(str, ", release build");
#endif
#ifdef WS_DEBUG_UTF_8
g_string_append(str, " (+utf8)");
#endif
g_string_append(str, ".");

View File

@ -17,10 +17,12 @@
#include <wsutil/wslog.h>
#include <wsutil/wmem/wmem.h>
#ifdef WS_DEBUG
#define _ASSERT_ENABLED true
#if defined(ENABLE_ASSERT)
#define WS_ASSERT_ENABLED 1
#elif defined(NDEBUG)
#define WS_ASSERT_ENABLED 0
#else
#define _ASSERT_ENABLED false
#define WS_ASSERT_ENABLED 1
#endif
#ifdef __cplusplus
@ -28,32 +30,32 @@ extern "C" {
#endif /* __cplusplus */
/*
* We don't want to execute the expression with !WS_DEBUG because
* We don't want to execute the expression without assertions because
* it might be time and space costly and the goal here is to optimize for
* !WS_DEBUG. However removing it completely is not good enough
* that case. However removing it completely is not good enough
* because it might generate many unused variable warnings. So we use
* if (false) and let the compiler optimize away the dead execution branch.
*/
#define _ASSERT_IF_ACTIVE(active, expr) \
#define ws_assert_if_active(active, expr) \
do { \
if ((active) && !(expr)) \
ws_error("assertion failed: %s", #expr); \
} while (0)
/*
* ws_abort_if_fail() is not conditional on WS_DEBUG.
* ws_abort_if_fail() is not conditional on having assertions enabled.
* Usually used to appease a static analyzer.
*/
#define ws_abort_if_fail(expr) \
_ASSERT_IF_ACTIVE(true, expr)
ws_assert_if_active(true, expr)
/*
* ws_assert() cannot produce side effects, otherwise code will
* behave differently because of WS_DEBUG, and probably introduce
* some difficult to track bugs.
* behave differently because of having assertions enabled/disabled, and
* probably introduce some difficult to track bugs.
*/
#define ws_assert(expr) \
_ASSERT_IF_ACTIVE(_ASSERT_ENABLED, expr)
ws_assert_if_active(WS_ASSERT_ENABLED, expr)
#define ws_assert_streq(s1, s2) \
@ -62,7 +64,7 @@ extern "C" {
#define ws_assert_utf8(str, len) \
do { \
const char *__assert_endptr; \
if (_ASSERT_ENABLED && \
if (WS_ASSERT_ENABLED && \
!g_utf8_validate(str, len, &__assert_endptr)) { \
ws_log_utf8_full(LOG_DOMAIN_UTF_8, LOG_LEVEL_ERROR, \
__FILE__, __LINE__, __func__, \
@ -71,7 +73,8 @@ extern "C" {
} while (0)
/*
* We don't want to disable ws_assert_not_reached() with WS_DEBUG.
* We don't want to disable ws_assert_not_reached() with (optional) assertions
* disabled.
* That would blast compiler warnings everywhere for no benefit, not
* even a miniscule performance gain. Reaching this function is always
* a programming error and will unconditionally abort execution.
@ -99,21 +102,21 @@ extern "C" {
*/
#define ws_warn_badarg(str) \
ws_log_full(LOG_DOMAIN_EINVAL, LOG_LEVEL_INFO, \
ws_log_full(LOG_DOMAIN_EINVAL, LOG_LEVEL_WARNING, \
__FILE__, __LINE__, __func__, \
"bad argument: %s", str)
"invalid argument: %s", str)
#define ws_return_str_if(expr, scope) \
do { \
if (expr) { \
if (WS_ASSERT_ENABLED && (expr)) { \
ws_warn_badarg(#expr); \
return wmem_strdup(scope, "(invalid argument)"); \
return wmem_strdup_printf(scope, "(invalid argument: %s)", #expr); \
} \
} while (0)
#define ws_return_val_if(expr, val) \
do { \
if (expr) { \
if (WS_ASSERT_ENABLED && (expr)) { \
ws_warn_badarg(#expr); \
return (val); \
} \