DEBUG: reduce the footprint of BUG_ON() calls

Many inline functions involve some BUG_ON() calls and because of the
partial complexity of the functions, they're not inlined anymore (e.g.
co_data()). The reason is that the expression instantiates the message,
its size, sometimes a counter, then the atomic OR to taint the process,
and the back trace. That can be a lot for an inline function and most
of it is always the same.

This commit modifies this by delegating the common parts to a dedicated
function "complain()" that takes care of updating the counter if needed,
writing the message and measuring its length, and tainting the process.
This way the caller only has to check a condition, pass a pointer to the
preset message, and the info about the type (bug or warn) for the tainting,
then decide whether to dump or crash. Note that this part could also be
moved to the function but resulted in complain() always being at the top
of the stack, which didn't seem like an improvement.

Thanks to these changes, the BUG_ON() calls do not result in uninlining
functions anymore and the overall code size was reduced by 60 to 120 kB
depending on the build options.

diff --git a/include/haproxy/bug.h b/include/haproxy/bug.h
index 2ed27c72d037b6cdd93c78bac980ea4d6ea97f9e..71b37eda6974c5cf08e34ce1c84ae6f7d777aa1a 100644 (file)
--- a/include/haproxy/bug.h
+++ b/include/haproxy/bug.h
@@ -61,22 +61,14 @@
        __BUG_ON(cond, file, line, crash, pfx, sfx)
 
 #define __BUG_ON(cond, file, line, crash, pfx, sfx)                     \
-       ({                                                              \
-               int __bug_cond = !!(cond);                              \
-               if (unlikely(__bug_cond)) {                             \
-                       const char msg[] = "\n" pfx "condition \"" #cond "\" matched at " file ":" #line "" sfx "\n"; \
-                       DISGUISE(write(2, msg, __builtin_strlen(msg))); \
-                       if (crash & 2)                                  \
-                               mark_tainted(TAINTED_BUG);              \
-                       else                                            \
-                               mark_tainted(TAINTED_WARN);             \
-                       if (crash & 1)                                  \
-                               ABORT_NOW();                            \
-                       else                                            \
-                               DUMP_TRACE();                           \
-               }                                                       \
-               __bug_cond; /* let's return the condition */            \
-       })
+       (unlikely(cond) ? ({                                            \
+               complain(NULL, "\n" pfx "condition \"" #cond "\" matched at " file ":" #line "" sfx "\n", crash); \
+               if (crash & 1)                                          \
+                       ABORT_NOW();                                    \
+               else                                                    \
+                       DUMP_TRACE();                                   \
+               1; /* let's return the true condition */                \
+       }) : 0)
 
 /* This one is equivalent except that it only emits the message once by
  * maintaining a static counter. This may be used with warnings to detect
@@ -87,24 +79,15 @@
        __BUG_ON_ONCE(cond, file, line, crash, pfx, sfx)
 
 #define __BUG_ON_ONCE(cond, file, line, crash, pfx, sfx)                \
-       ({                                                              \
+       (unlikely(cond) ? ({                                            \
                static int __match_count_##line;                        \
-               int __bug_cond = !!(cond);                              \
-               if (unlikely(__bug_cond) &&                             \
-                   !_HA_ATOMIC_FETCH_ADD(&__match_count_##line, 1)) {  \
-                       const char msg[] = "\n" pfx "condition \"" #cond "\" matched at " file ":" #line "" sfx "\n"; \
-                       DISGUISE(write(2, msg, __builtin_strlen(msg))); \
-                       if (crash & 2)                                  \
-                               mark_tainted(TAINTED_BUG);              \
-                       else                                            \
-                               mark_tainted(TAINTED_WARN);             \
-                       if (crash & 1)                                  \
-                               ABORT_NOW();                            \
-                       else                                            \
-                               DUMP_TRACE();                           \
-               }                                                       \
-               __bug_cond; /* let's return the condition */            \
-       })
+               complain(&__match_count_##line, "\n" pfx "condition \"" #cond "\" matched at " file ":" #line "" sfx "\n", crash); \
+               if (crash & 1)                                          \
+                       ABORT_NOW();                                    \
+               else                                                    \
+                       DUMP_TRACE();                                   \
+               1; /* let's return the true condition */                \
+       }) : 0)
 
 /* DEBUG_STRICT enables/disables runtime checks on condition <cond>
  * DEBUG_STRICT_ACTION indicates the level of verification on the rules when
@@ -208,6 +191,8 @@ enum tainted_flags {
 /* this is a bit field made of TAINTED_*, and is declared in haproxy.c */
 extern unsigned int tainted;
 
+void complain(int *counter, const char *msg, int taint);
+
 static inline void mark_tainted(const enum tainted_flags flag)
 {
        HA_ATOMIC_OR(&tainted, flag);

diff --git a/src/debug.c b/src/debug.c
index 4bc70a0d34d501f64ee36aef20ebf4d7ed2fbfde..ebdbb885ef57940c27889e90abedf7c15f54861a 100644 (file)
--- a/src/debug.c
+++ b/src/debug.c
@@ -337,6 +337,22 @@ void ha_panic()
                abort();
 }
 
+/* Complain with message <msg> on stderr. If <counter> is not NULL, it is
+ * atomically incremented, and the message is only printed when the counter
+ * was zero, so that the message is only printed once. <taint> is only checked
+ * on bit 1, and will taint the process either for a bug (2) or warn (0).
+ */
+void complain(int *counter, const char *msg, int taint)
+{
+       if (counter && _HA_ATOMIC_FETCH_ADD(counter, 1))
+               return;
+       DISGUISE(write(2, msg, strlen(msg)));
+       if (taint & 2)
+               mark_tainted(TAINTED_BUG);
+       else
+               mark_tainted(TAINTED_WARN);
+}
+
 /* parse a "debug dev exit" command. It always returns 1, though it should never return. */
 static int debug_parse_cli_exit(char **args, char *payload, struct appctx *appctx, void *private)
 {