From 464ebc311d460b29f681703aea0aa00eef9e6475 Mon Sep 17 00:00:00 2001 From: Philippe Proulx Date: Wed, 28 Feb 2018 18:37:08 -0500 Subject: [PATCH] Add internal BT_ASSERT() and BT_ASSERT_PRE() helpers This patch adds new internal assertion macros to control what is asserted. The ultimate purpose of this is to disable logic/postcondition assertions and run-time precondition checks in a production build to optimize the project. `configure.ac` is modified so that it handles two new environment variables: `BABELTRACE_DEBUG_MODE`: When set to `1`: enables the Babeltrace debugging mode. This enables internal assertions which validate logic and postconditions with the BT_ASSERT() macro. This defines `BT_DEBUG_MODE` in sources. As of this patch, when in debugging mode, BT_ASSERT() is the equivalent of assert(). However, assert() is enabled by not passing `-DNDEBUG` to the compiler, while BT_ASSERT() is enabled by passing `-DBT_DEBUG_MODE`. Because the build is not in debugging mode by default, even if `-DNDEBUG` is not passed to the compiler (thus not enabling assert()), neither is `-DBT_DEBUG_MODE`, so all logic/postcondition assertions are disabled in production. You can use BT_ASSERT() anywhere in the whole project by including . If a function is to be used only within a BT_ASSERT() context, then the compiler would complain that it is unused in non-debugging mode. You can tag the function with `BT_ASSERT_FUNC` for this. `BABELTRACE_DEV_MODE`: When set to `1`: enables the Babeltrace developer mode. This enables precondition assertions in the library. This defines `BT_DEV_MODE` in sources. A lot of run-time checks can be avoided in production once you know that the code is correct, i.e. that it won't perform anything illegal, whatever the end user does. Some examples are NULL checks, frozen checks, and bound checks. The developer mode allows run-time precondition checks to be performed with BT_ASSERT_PRE(). This macro expects the condition to check as well as a message. In developer mode, when the condition is false, the message is printed with BT_LIB_LOGF(), indicating that a library precondition was not satisfied at this point, and then abort() is called. The developer mode is a trade-off between strictness and performance: you can get a very strict library with a developer mode build, and a more efficient library with a non-developer mode build. If a function is to be used only within a BT_ASSERT_PRE() context, then the compiler would complain that it is unused in non-developer mode. You can tag the function with `BT_ASSERT_PRE_FUNC` for this. In such a function, use BT_ASSERT_PRE_MSG() to log any additional information without aborting (since you know the process will abort thanks to BT_ASSERT_PRE()). BT_ASSERT_PRE_NON_NULL() is a helper which does a non-NULL precondition check. BT_ASSERT_PRE_HOT() is a helper which does a non-frozen precondition check. `lib/logging.c` is modified so that, in developer mode, the library's default log level is FATAL and you cannot set it to NONE. This is important because BT_ASSERT_PRE() and BT_ASSERT_PRE_MSG() use BT_LIB_LOGF() to print messages. Signed-off-by: Philippe Proulx --- configure.ac | 20 ++++ include/Makefile.am | 2 + include/babeltrace/assert-internal.h | 46 +++++++++ include/babeltrace/assert-pre-internal.h | 118 +++++++++++++++++++++++ lib/logging.c | 30 +++++- 5 files changed, 215 insertions(+), 1 deletion(-) create mode 100644 include/babeltrace/assert-internal.h create mode 100644 include/babeltrace/assert-pre-internal.h diff --git a/configure.ac b/configure.ac index 1ffe6962..09092bef 100644 --- a/configure.ac +++ b/configure.ac @@ -394,6 +394,21 @@ AS_IF([test "$BABELTRACE_MINIMAL_LOG_LEVEL" != "VERBOSE" && \ ) AC_DEFINE_UNQUOTED([BT_LOG_LEVEL], [BT_LOG_$BABELTRACE_MINIMAL_LOG_LEVEL], [Minimal log level]) +# BABELTRACE_DEV_MODE: +AC_ARG_VAR([BABELTRACE_DEV_MODE], [Set to 1 to enable the Babeltrace developer mode (enables run-time checks for plugin developers)]) +AS_IF([test "x$BABELTRACE_DEV_MODE" = x1], [ + AS_IF([test "x$BABELTRACE_MINIMAL_LOG_LEVEL" = "xNONE"], [ + AC_MSG_ERROR([Babeltrace developer mode (\$BABELTRACE_DEV_MODE) needs \$BABELTRACE_MINIMAL_LOG_LEVEL to be at least FATAL.]) + ]) + AC_DEFINE([BT_DEV_MODE], 1, [Babeltrace developer mode]) +], [BABELTRACE_DEV_MODE=0]) + +# BABELTRACE_DEBUG_MODE: +AC_ARG_VAR([BABELTRACE_DEBUG_MODE], [Set to 1 to enable the Babeltrace debug mode (enables internal assertions for Babeltrace maintainers)]) +AS_IF([test "x$BABELTRACE_DEBUG_MODE" = x1], [ + AC_DEFINE([BT_DEBUG_MODE], 1, [Babeltrace debug mode]) +], [BABELTRACE_DEBUG_MODE=0]) + ## ## ## Optionnal features selection ## @@ -892,6 +907,11 @@ AS_ECHO PPRINT_SUBTITLE([Logging]) PPRINT_PROP_STRING([Minimal log level], $BABELTRACE_MINIMAL_LOG_LEVEL) +AS_ECHO +PPRINT_SUBTITLE([Special build modes]) +PPRINT_PROP_BOOL([Debug mode], $BABELTRACE_DEBUG_MODE) +PPRINT_PROP_BOOL([Developer mode], $BABELTRACE_DEV_MODE) + report_bindir="`eval eval echo $bindir`" report_libdir="`eval eval echo $libdir`" report_sysconfdif="`eval eval echo $sysconfdir`" diff --git a/include/Makefile.am b/include/Makefile.am index 80cd4dda..a310b37b 100644 --- a/include/Makefile.am +++ b/include/Makefile.am @@ -156,6 +156,8 @@ babeltracegraphinclude_HEADERS = \ noinst_HEADERS = \ babeltrace/align-internal.h \ + babeltrace/assert-internal.h \ + babeltrace/assert-pre-internal.h \ babeltrace/babeltrace-internal.h \ babeltrace/bitfield-internal.h \ babeltrace/common-internal.h \ diff --git a/include/babeltrace/assert-internal.h b/include/babeltrace/assert-internal.h new file mode 100644 index 00000000..c8a9f837 --- /dev/null +++ b/include/babeltrace/assert-internal.h @@ -0,0 +1,46 @@ +#ifndef BABELTRACE_ASSERT_INTERNAL_H +#define BABELTRACE_ASSERT_INTERNAL_H + +/* + * Copyright (c) 2018 EfficiOS Inc. and Linux Foundation + * Copyright (c) 2018 Philippe Proulx + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +#include + +#ifdef BT_DEBUG_MODE +/* + * Internal assertion (to detect logic errors on which the library user + * has no influence). Use BT_ASSERT_PRE() to check a precondition which + * must be directly or indirectly satisfied by the library user. + */ +# define BT_ASSERT(_cond) do { assert(_cond); } while (0) + +/* + * Marks a function as being only used within a BT_ASSERT() context. + */ +# define BT_ASSERT_FUNC +#else +# define BT_ASSERT(_cond) +# define BT_ASSERT_FUNC __attribute__((unused)) +#endif /* BT_DEBUG_MODE */ + +#endif /* BABELTRACE_ASSERT_INTERNAL_H */ diff --git a/include/babeltrace/assert-pre-internal.h b/include/babeltrace/assert-pre-internal.h new file mode 100644 index 00000000..cc412340 --- /dev/null +++ b/include/babeltrace/assert-pre-internal.h @@ -0,0 +1,118 @@ +#ifndef BABELTRACE_ASSERT_PRE_INTERNAL_H +#define BABELTRACE_ASSERT_PRE_INTERNAL_H + +/* + * Copyright (c) 2018 EfficiOS Inc. and Linux Foundation + * Copyright (c) 2018 Philippe Proulx + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +/* + * The macros in this header use macros defined in + * . We don't want this header to + * automatically include because you + * need to manually define BT_LOG_TAG before including + * and it is unexpected that you + * also need to define it before including this header. + * + * This is a reminder that in order to use + * , you also need to use logging + * explicitly. + */ +#ifndef BABELTRACE_LIB_LOGGING_INTERNAL_H +# error Include before this header. +#endif + +#include + +#ifdef BT_DEV_MODE +/* + * Asserts that the library precondition _cond is satisfied. + * + * If _cond is false, log a fatal statement using _fmt and the optional + * arguments using BT_LIB_LOGF(), and abort. + * + * To assert that a postcondition is satisfied or that some internal + * object/context/value is in the expected state, use BT_ASSERT(). + */ +# define BT_ASSERT_PRE(_cond, _fmt, ...) \ + do { \ + if (!(_cond)) { \ + BT_LOGF_STR("Library precondition not satisfied:"); \ + BT_LIB_LOGF((_fmt), ##__VA_ARGS__); \ + BT_LOGF_STR("Aborting..."); \ + abort(); \ + } \ + } while (0) + +/* + * Marks a function as being only used within a BT_ASSERT_PRE() context. + */ +# define BT_ASSERT_PRE_FUNC + +/* + * Prints the details of an unsatisfied precondition without immediately + * aborting. You should use this within a function which checks + * preconditions, but which is called from a BT_ASSERT_PRE() context, so + * that the function can still return its result for BT_ASSERT_PRE() to + * evaluate it. + * + * Example: + * + * BT_ASSERT_PRE_FUNC + * static inline bool check_complex_precond(...) + * { + * ... + * + * if (...) { + * BT_ASSERT_PRE_MSG("Invalid object: ...", ...); + * return false; + * } + * + * ... + * } + * + * ... + * + * BT_ASSERT_PRE(check_complex_precond(...), + * "Precondition is not satisfied: ...", ...); + */ +# define BT_ASSERT_PRE_MSG BT_LIB_LOGF +#else +# define BT_ASSERT_PRE(_cond, _fmt, ...) +# define BT_ASSERT_PRE_FUNC __attribute__((unused)) +# define BT_ASSERT_PRE_MSG(_fmt, ...) +#endif /* BT_DEV_MODE */ + +/* + * Developer mode: asserts that a given variable is not NULL. + */ +#define BT_ASSERT_PRE_NON_NULL(_obj, _obj_name) \ + BT_ASSERT_PRE((_obj) != NULL, "%s is NULL: ", _obj_name) + +/* + * Developer mode: asserts that a given object is NOT frozen. This macro + * checks the `frozen` field of _obj. + */ +#define BT_ASSERT_PRE_HOT(_obj, _obj_name, _fmt, ...) \ + BT_ASSERT_PRE(!(_obj)->frozen, "%s is frozen" _fmt, _obj_name, \ + ##__VA_ARGS__) + +#endif /* BABELTRACE_ASSERT_PRE_INTERNAL_H */ diff --git a/lib/logging.c b/lib/logging.c index 371bc596..3d9a1bcd 100644 --- a/lib/logging.c +++ b/lib/logging.c @@ -27,8 +27,25 @@ #define BT_LOG_TAG "LIB" #include +#ifdef BT_DEV_MODE +/* + * Default log level is FATAL in developer mode because fatal logging is + * our way to communicate an unsatisfied precondition and the details. + */ +# define DEFAULT_LOG_LEVEL BT_LOG_FATAL +#else +/* + * In non-developer mode, use NONE by default: we don't to print logging + * statements for any executable which links with the library. The + * executable should call bt_logging_set_global_level() or the + * executable's user should set the BABELTRACE_LOGGING_GLOBAL_LEVEL + * environment variable. + */ +# define DEFAULT_LOG_LEVEL BT_LOG_NONE +#endif /* BT_DEV_MODE */ + BT_HIDDEN -int bt_lib_log_level = BT_LOG_NONE; +int bt_lib_log_level = DEFAULT_LOG_LEVEL; enum bt_logging_level bt_logging_get_minimal_level(void) { @@ -42,6 +59,17 @@ enum bt_logging_level bt_logging_get_global_level(void) void bt_logging_set_global_level(enum bt_logging_level log_level) { +#ifdef BT_DEV_MODE + /* + * Do not allow the library's log level to fall to NONE when in + * developer mode because fatal logging is our way to + * communicate an unsatisfied precondition and the details. + */ + if (log_level == BT_LOG_NONE) { + log_level = BT_LOG_FATAL; + } +#endif + bt_lib_log_level = log_level; } -- 2.34.1