btrfs: enhance ASSERT() to take optional format string
authorDavid Sterba <dsterba@suse.com>
Thu, 17 Apr 2025 09:16:59 +0000 (11:16 +0200)
committerDavid Sterba <dsterba@suse.com>
Thu, 15 May 2025 12:30:47 +0000 (14:30 +0200)
Currently ASSERT() prints the stringified condition and without macro
expansions so simple constants like BTRFS_MAX_METADATA_BLOCKSIZE remain
readable in the output.

There are expressions where we'd like to see the exact values but all we
get is something like:

  assertion failed: em->start <= start && start < extent_map_end(em), in fs/btrfs/extent_map.c:613

It would be nice to be able to print any additional information to help
understand the problem. With some preprocessor magic and compile-time
optimizations we can enhance ASSERT to work like that as well:

  ASSERT(value > limit, "value=%llu limit=%llu", value, limit);

with free-form printk arguments that will be part of the assertion
message.

Pros:
- helps debugging and understanding reported problems
- the optional format is verified at compile-time

Cons:
- increases the .ko size
- writing the assertion code is repetitive (condition, format, values)
- format and variable type must match (extra lookup)
- needs gcc 8.x and newer, otherwise it's the short format

Recommended use is for non-trivial expressions, so basic ASSERT(value) can be
used for pointers or sometimes integers.

The format has been slightly updated to also print the result of the
evaluation of the condition, appended to the stringified condition as
"condition :: <value>".

Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
fs/btrfs/messages.h

index 08a9272399d26fccd11eabfcb0c2af47800eb250..d4e85485d824d91e925f3416645d88964b35b5ad 100644 (file)
@@ -3,6 +3,7 @@
 #ifndef BTRFS_MESSAGES_H
 #define BTRFS_MESSAGES_H
 
+#include <linux/types.h>
 #include <linux/types.h>
 #include <linux/printk.h>
 #include <linux/bug.h>
@@ -170,15 +171,76 @@ do {                                                              \
 
 #ifdef CONFIG_BTRFS_ASSERT
 
-#define btrfs_assertfail(expr, file, line)     ({                              \
-       pr_err("assertion failed: %s, in %s:%d\n", (expr), (file), (line));     \
-       BUG();                                                          \
-})
+__printf(1, 2)
+static inline void verify_assert_printk_format(const char *fmt, ...) {
+       /* Stub to verify the assertion format string. */
+}
+
+/* Take the first token if any. */
+#define __FIRST_ARG(_, ...) _
+/*
+ * Skip the first token and return the rest, if it's empty the comma is dropped.
+ * As ##__VA_ARGS__ cannot be at the beginning of the macro the __VA_OPT__ is needed
+ * and supported since GCC 8 and Clang 12.
+ */
+#define __REST_ARGS(_, ... ) __VA_OPT__(,) __VA_ARGS__
+
+#if defined(CONFIG_CC_IS_CLANG) || GCC_VERSION >= 80000
+/*
+ * Assertion with optional printk() format.
+ *
+ * Accepted syntax:
+ * ASSERT(condition);
+ * ASSERT(condition, "string");
+ * ASSERT(condition, "variable=%d", variable);
+ *
+ * How it works:
+ * - if there's no format string, ""[0] evaluates at compile time to 0 and the
+ *   true branch is executed
+ * - any non-empty format string with the "" prefix evaluates to != 0 at
+ *   compile time and the false branch is executed
+ * - stringified condition is printed as %s so we don't accidentally mix format
+ *   strings (the % operator)
+ * - there can be only one printk() call, so the format strings and arguments are
+ *   spliced together:
+ *   DEFAULT_FMT [USER_FMT], DEFAULT_ARGS [, USER_ARGS]
+ * - comma between DEFAULT_ARGS and USER_ARGS is handled by preprocessor
+ *   (requires __VA_OPT__ support)
+ * - otherwise we could use __VA_OPT(,) __VA_ARGS__ for the 2nd+ argument of args,
+ */
+#define ASSERT(cond, args...)                                                  \
+do {                                                                           \
+       verify_assert_printk_format("check the format string" args);            \
+       if (!likely(cond)) {                                                    \
+               if (("" __FIRST_ARG(args) [0]) == 0) {                          \
+                       pr_err("assertion failed: %s :: %ld, in %s:%d\n",       \
+                               #cond, (long)(cond), __FILE__, __LINE__);       \
+               } else {                                                        \
+                       pr_err("assertion failed: %s :: %ld, in %s:%d (" __FIRST_ARG(args) ")\n", \
+                               #cond, (long)(cond), __FILE__, __LINE__ __REST_ARGS(args)); \
+               }                                                               \
+               BUG();                                                          \
+       }                                                                       \
+} while(0)
+
+#else
+
+/* For GCC < 8.x only the simple output. */
+
+#define ASSERT(cond, args...)                                                  \
+do {                                                                           \
+       verify_assert_printk_format("check the format string" args);            \
+       if (!likely(cond)) {                                                    \
+               pr_err("assertion failed: %s :: %ld, in %s:%d\n",               \
+                       #cond, (long)(cond), __FILE__, __LINE__);               \
+               BUG();                                                          \
+       }                                                                       \
+} while(0)
+
+#endif
 
-#define ASSERT(expr)                                           \
-       (likely(expr) ? (void)0 : btrfs_assertfail(#expr, __FILE__, __LINE__))
 #else
-#define ASSERT(expr)   (void)(expr)
+#define ASSERT(cond, args...)                  (void)(cond)
 #endif
 
 __printf(5, 6)