Ede_Vau
/
frr
mirror of https://github.com/FRRouting/frr.git
1
1
Fork 0
Code Issues Releases Wiki Activity

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>
This commit is contained in:
David Lamparter 2021-03-16 11:03:44 +01:00 committed by David Lamparter
parent 642ac49da4
commit 64dd77361f
12 changed files with 271 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
Makefile.am
View File

@ -11,11 +11,19 @@ AM_CFLAGS = \
$(SAN_FLAGS) \
$(WERROR) \
# end
AM_CPPFLAGS = \
# CPPFLAGS_BASE does not contain the include path for overriding assert.h,
# therefore should be used in tools that do *not* link libfrr or do not want
# assert() overridden
CPPFLAGS_BASE = \
-I$(top_srcdir) -I$(top_srcdir)/include -I$(top_srcdir)/lib \
-I$(top_builddir) \
$(LUA_INCLUDE) \
# end
AM_CPPFLAGS = \
-I$(top_srcdir)/lib/assert \
$(CPPFLAGS_BASE) \
# end
AM_LDFLAGS = \
-export-dynamic \
$(AC_LDFLAGS) \

98
lib/assert/assert.h Normal file
View File

@ -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 */

9
lib/clippy.c
View File

@ -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();

11
lib/log.c
View File

@ -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,

4
lib/subdir.am
View File

@ -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

1
lib/xref.h
View File

@ -33,6 +33,7 @@ enum xref_type {
XREFT_THREADSCHED = 0x100,
XREFT_LOGMSG = 0x200,
XREFT_ASSERT = 0x280,
XREFT_DEFUN = 0x300,
XREFT_INSTALL_ELEMENT = 0x301,

30
lib/zlog.c
View File

@ -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 = &ap;
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)
{

3
tests/.gitignore vendored
View File

@ -21,6 +21,7 @@
/lib/cli/test_commands_defun.c
/lib/northbound/test_oper_data
/lib/cxxcompat
/lib/test_assert
/lib/test_atomlist
/lib/test_buffer
/lib/test_checksum
@ -52,4 +53,4 @@
/lib/test_zmq
/ospf6d/test_lsdb
/ospf6d/test_lsdb_clippy.c
/zebra/test_lm_plugin
/zebra/test_lm_plugin

64
tests/lib/test_assert.c Normal file
View File

@ -0,0 +1,64 @@
/*
* Quick test for assert()
* Copyright (C) 2021 David Lamparter for NetDEF, Inc.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the Free
* Software Foundation; either version 2 of the License, or (at your option)
* any later version.
*
* This program is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
* more details.
*
* You should have received a copy of the GNU General Public License along
* with this program; see the file COPYING; if not, write to the Free Software
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*/
#ifdef HAVE_CONFIG_H
#include "config.h"
#endif
/* make sure this works with assert.h & nothing else. also check the include
* shadowing, we don't want to pick up system assert.h
*/
#include <assert.h>
__attribute__((noinline))
void func_for_bt(int number)
{
assert(number > 2);
assertf(number > 3, "(A) the number was %d", number);
}
#include <zebra.h>
#include "lib/zlog.h"
#include "lib/thread.h"
#include "lib/sigevent.h"
int main(int argc, char **argv)
{
int number = 10;
struct thread_master *master;
zlog_aux_init("NONE: ", LOG_DEBUG);
if (argc > 1)
number = atoi(argv[1]);
assert(number > 0);
assertf(number > 1, "(B) the number was %d", number);
/* set up SIGABRT handler */
master = thread_master_create("test");
signal_init(master, 0, NULL);
func_for_bt(number);
assert(number > 4);
assertf(number > 5, "(C) the number was %d", number);
assertf(number > 10, "(D) the number was %d", number);
return 0;
}

56
tests/lib/test_assert.py Normal file
View File

@ -0,0 +1,56 @@
import frrtest
import os
import re
import subprocess
import inspect
basedir = os.path.dirname(__file__)
program = os.path.join(basedir, "test_assert")
def check(number, rex=None):
proc = subprocess.Popen(
[frrtest.binpath(program), str(number)],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
)
out, err = proc.communicate()
exitcode = proc.wait()
if rex is None:
assert exitcode == 0
else:
assert exitcode != 0
text = out.decode("US-ASCII") + err.decode("US-ASCII")
rex = re.compile(rex, re.M | re.S)
m = rex.search(text)
assert m is not None, "non-matching output: %s" % text
def test_assert_0():
check(0, r"test_assert\.c:\d+.*number > 0")
def test_assert_1():
check(1, r"test_assert\.c:\d+.*number > 1.*\(B\) the number was 1")
def test_assert_2():
check(2, r"test_assert\.c:\d+.*number > 2")
def test_assert_3():
check(3, r"test_assert\.c:\d+.*number > 3.*\(A\) the number was 3")
def test_assert_4():
check(4, r"test_assert\.c:\d+.*number > 4")
def test_assert_10():
check(10, r"test_assert\.c:\d+.*number > 10.*\(D\) the number was 10")
def test_assert_11():
check(11)

6
tests/subdir.am
View File

@ -66,6 +66,7 @@ clippy_scan += \
check_PROGRAMS = \
tests/lib/cxxcompat \
tests/lib/test_assert \
tests/lib/test_atomlist \
tests/lib/test_buffer \
tests/lib/test_checksum \
@ -249,6 +250,10 @@ tests_lib_northbound_test_oper_data_CPPFLAGS = $(TESTS_CPPFLAGS)
tests_lib_northbound_test_oper_data_LDADD = $(ALL_TESTS_LDADD)
tests_lib_northbound_test_oper_data_SOURCES = tests/lib/northbound/test_oper_data.c
nodist_tests_lib_northbound_test_oper_data_SOURCES = yang/frr-test-module.yang.c
tests_lib_test_assert_CFLAGS = $(TESTS_CFLAGS)
tests_lib_test_assert_CPPFLAGS = $(TESTS_CPPFLAGS)
tests_lib_test_assert_LDADD = $(ALL_TESTS_LDADD)
tests_lib_test_assert_SOURCES = tests/lib/test_assert.c
tests_lib_test_atomlist_CFLAGS = $(TESTS_CFLAGS)
tests_lib_test_atomlist_CPPFLAGS = $(TESTS_CPPFLAGS)
tests_lib_test_atomlist_LDADD = $(ALL_TESTS_LDADD)
@ -404,6 +409,7 @@ EXTRA_DIST += \
tests/lib/northbound/test_oper_data.in \
tests/lib/northbound/test_oper_data.py \
tests/lib/northbound/test_oper_data.refout \
tests/lib/test_assert.py \
tests/lib/test_atomlist.py \
tests/lib/test_nexthop_iter.py \
tests/lib/test_ntop.py \

2
tools/subdir.am
View File

@ -34,9 +34,11 @@ tools_gen_yang_deviations_SOURCES = tools/gen_yang_deviations.c
tools_gen_yang_deviations_LDADD = lib/libfrr.la $(LIBYANG_LIBS)
tools_ssd_SOURCES = tools/start-stop-daemon.c
tools_ssd_CPPFLAGS =
# don't bother autoconf'ing these for a simple optional tool
llvm_version = $(shell echo __clang_major__ | $(CC) -xc -P -E -)
tools_frr_llvm_cg_CPPFLAGS = $(CPPFLAGS_BASE)
tools_frr_llvm_cg_CFLAGS = $(AM_CFLAGS) `llvm-config-$(llvm_version) --cflags`
tools_frr_llvm_cg_LDFLAGS = `llvm-config-$(llvm_version) --ldflags --libs`
tools_frr_llvm_cg_SOURCES = \