lib: rework how we "override" assert()

The previous method, using zassert.h and hoping nothing includes
assert.h (which, on glibc at least, just does "#undef assert" and puts
its own definition in...) was fragile - and actually broke undetected.

Just provide our own assert.h and control overriding by putting it in a
separate directory to add to the include path (or not.)

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

6 files changed, 132 insertions, 21 deletions

diff --git a/lib/assert/assert.h b/lib/assert/assert.h
new file mode 100644
index 0000000000..fbdbd52ce8
--- /dev/null
+++ b/lib/assert/assert.h
@@ -0,0 +1,98 @@
+/*
+ * Copyright (c) 2021  David Lamparter, for NetDEF, Inc.
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+/* WARNING: this file is "special" in that it overrides the system-provided
+ * assert.h by being on the include path before it.  That means it should
+ * provide the functional equivalent.
+ *
+ * This is intentional because FRR extends assert() to write to the log and
+ * add backtraces.  Overriding the entire file is the simplest and most
+ * reliable way to get this to work;  there were problems previously with the
+ * system assert.h getting included afterwards and redefining assert() back to
+ * the system variant.
+ */
+
+#ifndef _FRR_ASSERT_H
+#define _FRR_ASSERT_H
+
+#include "xref.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#ifndef __cplusplus
+/* C++ has this built-in, but C provides it in assert.h for >=C11.  Since we
+ * replace assert.h entirely, we need to provide it here too.
+ */
+#define static_assert _Static_assert
+#endif
+
+struct xref_assert {
+	struct xref xref;
+
+	const char *expr;
+	const char *extra, *args;
+};
+
+extern void _zlog_assert_failed(const struct xref_assert *xref,
+				const char *extra, ...) PRINTFRR(2, 3)
+	__attribute__((noreturn));
+
+/* the "do { } while (expr_)" is there to get a warning for assignments inside
+ * the assert expression aka "assert(x = 1)".  The (necessary) braces around
+ * expr_ in the if () statement would suppress these warnings.  Since
+ * _zlog_assert_failed() is noreturn, the while condition will never be
+ * checked.
+ */
+#define assert(expr_)                                                          \
+	({                                                                     \
+		static const struct xref_assert _xref __attribute__(           \
+			(used)) = {                                            \
+			.xref = XREF_INIT(XREFT_ASSERT, NULL, __func__),       \
+			.expr = #expr_,                                        \
+		};                                                             \
+		XREF_LINK(_xref.xref);                                         \
+		if (__builtin_expect((expr_) ? 0 : 1, 0))                      \
+			do {                                                   \
+				_zlog_assert_failed(&_xref, NULL);             \
+			} while (expr_);                                       \
+	})
+
+#define assertf(expr_, extra_, ...)                                            \
+	({                                                                     \
+		static const struct xref_assert _xref __attribute__(           \
+			(used)) = {                                            \
+			.xref = XREF_INIT(XREFT_ASSERT, NULL, __func__),       \
+			.expr = #expr_,                                        \
+			.extra = extra_,                                       \
+			.args = #__VA_ARGS__,                                  \
+		};                                                             \
+		XREF_LINK(_xref.xref);                                         \
+		if (__builtin_expect((expr_) ? 0 : 1, 0))                      \
+			do {                                                   \
+				_zlog_assert_failed(&_xref, extra_,            \
+						    ##__VA_ARGS__);            \
+			} while (expr_);                                       \
+	})
+
+#define zassert assert
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _FRR_ASSERT_H */
diff --git a/lib/clippy.c b/lib/clippy.c
index f1923d2a56..7ca99c9a94 100644
--- a/lib/clippy.c
+++ b/lib/clippy.c
@@ -115,15 +115,6 @@ void vzlogx(const struct xref_logmsg *xref, int prio,
 	fputs("\n", stderr);
 }
 
-void _zlog_assert_failed(const char *assertion, const char *file,
-			 unsigned int line, const char *function)
-{
-	fprintf(stderr,
-		"Assertion `%s' failed in file %s, line %u, function %s",
-		assertion, file, line, (function ? function : "?"));
-	abort();
-}
-
 void memory_oom(size_t size, const char *name)
 {
 	abort();
diff --git a/lib/log.c b/lib/log.c
index ca2f501686..15fdfd4b0a 100644
--- a/lib/log.c
+++ b/lib/log.c
@@ -311,17 +311,6 @@ void zlog_thread_info(int log_level)
 		zlog(log_level, "Current thread not known/applicable");
 }
 
-void _zlog_assert_failed(const char *assertion, const char *file,
-			 unsigned int line, const char *function)
-{
-	zlog(LOG_CRIT, "Assertion `%s' failed in file %s, line %u, function %s",
-	     assertion, file, line, (function ? function : "?"));
-	zlog_backtrace(LOG_CRIT);
-	zlog_thread_info(LOG_CRIT);
-	log_memstats(stderr, "log");
-	abort();
-}
-
 void memory_oom(size_t size, const char *name)
 {
 	zlog(LOG_CRIT,
diff --git a/lib/subdir.am b/lib/subdir.am
index fcaae9628a..480c2938d0 100644
--- a/lib/subdir.am
+++ b/lib/subdir.am
@@ -284,6 +284,8 @@ pkginclude_HEADERS += \
 	lib/zlog_targets.h \
 	lib/pbr.h \
 	lib/routing_nb.h \
+	\
+	lib/assert/assert.h \
 	# end
 
 
@@ -412,7 +414,7 @@ lib_grammar_sandbox_SOURCES = \
 lib_grammar_sandbox_LDADD = \
 	lib/libfrr.la
 
-lib_clippy_CPPFLAGS = $(AM_CPPFLAGS) -D_GNU_SOURCE -DBUILDING_CLIPPY
+lib_clippy_CPPFLAGS = $(CPPFLAGS_BASE) -D_GNU_SOURCE -DBUILDING_CLIPPY
 lib_clippy_CFLAGS = $(AC_CFLAGS) $(PYTHON_CFLAGS)
 lib_clippy_LDADD = $(PYTHON_LIBS) $(UST_LIBS) -lelf
 lib_clippy_LDFLAGS = -export-dynamic
diff --git a/lib/xref.h b/lib/xref.h
index 63166b069a..949458b313 100644
--- a/lib/xref.h
+++ b/lib/xref.h
@@ -33,6 +33,7 @@ enum xref_type {
 	XREFT_THREADSCHED = 0x100,
 
 	XREFT_LOGMSG = 0x200,
+	XREFT_ASSERT = 0x280,
 
 	XREFT_DEFUN = 0x300,
 	XREFT_INSTALL_ELEMENT = 0x301,
diff --git a/lib/zlog.c b/lib/zlog.c
index 24800c6e64..d2851c6028 100644
--- a/lib/zlog.c
+++ b/lib/zlog.c
@@ -521,6 +521,36 @@ void zlog_sigsafe(const char *text, size_t len)
 	}
 }
 
+void _zlog_assert_failed(const struct xref_assert *xref, const char *extra, ...)
+{
+	va_list ap;
+	static bool assert_in_assert; /* "global-ish" variable, init to 0 */
+
+	if (assert_in_assert)
+		abort();
+	assert_in_assert = true;
+
+	if (extra) {
+		struct va_format vaf;
+
+		va_start(ap, extra);
+		vaf.fmt = extra;
+		vaf.va = ≈
+
+		zlog(LOG_CRIT,
+		     "%s:%d: %s(): assertion (%s) failed, extra info: %pVA",
+		     xref->xref.file, xref->xref.line, xref->xref.func,
+		     xref->expr, &vaf);
+
+		va_end(ap);
+	} else
+		zlog(LOG_CRIT, "%s:%d: %s(): assertion (%s) failed",
+		     xref->xref.file, xref->xref.line, xref->xref.func,
+		     xref->expr);
+
+	/* abort() prints backtrace & memstats in SIGABRT handler */
+	abort();
+}
 
 int zlog_msg_prio(struct zlog_msg *msg)
 {