From 4d474fa3297c0d5d632e2c0bff6ccb0edbedaa5d Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 19 Nov 2013 15:00:06 +0100 Subject: [PATCH] lib: fix backtraces broken by 837d16c... 837d16c ("*: use array_size() helper macro") accidentally changed one of the expressions in the backtrace code, which afterwards read: zlog_backtrace_sigsafe(): if (((size = backtrace(array,array_size(array)) <= 0) || which boils down to: (size = backtrace(...) <= 0). The braces were intended to go: (size = backtrace(...)) <= 0. All in all, this makes a nice textbook example of the original author being too clever (trying to save a single line by pulling the assignment into the condition) and the next person touching the code tripping over it... This code occurs another time in zlog_backtrace() where it is actually correct. Pulling out the assignment nonetheless. Also, new test program. Cc: Andrew J. Schorr Cc: Balaji.G Cc: Scott Feldman Signed-off-by: David Lamparter --- lib/log.c | 8 +++---- tests/.gitignore | 1 + tests/Makefile.am | 4 +++- tests/test-segv.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 5 deletions(-) create mode 100644 tests/test-segv.c diff --git a/lib/log.c b/lib/log.c index e4ec7c2283..55a3b052db 100644 --- a/lib/log.c +++ b/lib/log.c @@ -443,8 +443,8 @@ zlog_backtrace_sigsafe(int priority, void *program_counter) #define LOC s,buf+sizeof(buf)-s #ifdef HAVE_GLIBC_BACKTRACE - if (((size = backtrace(array,array_size(array)) <= 0) || - ((size_t)size > array_size(array)))) + size = backtrace(array, array_size(array)); + if (size <= 0 || (size_t)size > array_size(array)) return; #define DUMP(FD) { \ @@ -526,8 +526,8 @@ zlog_backtrace(int priority) int size, i; char **strings; - if (((size = backtrace(array,array_size(array))) <= 0) || - ((size_t)size > array_size(array))) + size = backtrace(array, array_size(array)); + if (size <= 0 || (size_t)size > array_size(array)) { zlog_err("Cannot get backtrace, returned invalid # of frames %d " "(valid range is between 1 and %lu)", diff --git a/tests/.gitignore b/tests/.gitignore index 31fb70a484..8d00a3df7c 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -29,6 +29,7 @@ testbuffer testchecksum testmemory testprivs +testsegv testsig teststream testnexthopiter diff --git a/tests/Makefile.am b/tests/Makefile.am index 9260a900bd..5e631d6a86 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -24,13 +24,14 @@ else TESTS_BGPD = endif -check_PROGRAMS = testsig testbuffer testmemory heavy heavywq heavythread \ +check_PROGRAMS = testsig testsegv testbuffer testmemory heavy heavywq heavythread \ testprivs teststream testchecksum tabletest testnexthopiter \ $(TESTS_BGPD) noinst_HEADERS = prng.h testsig_SOURCES = test-sig.c +testsegv_SOURCES = test-segv.c testbuffer_SOURCES = test-buffer.c testmemory_SOURCES = test-memory.c testprivs_SOURCES = test-privs.c @@ -48,6 +49,7 @@ tabletest_SOURCES = table_test.c testnexthopiter_SOURCES = test-nexthop-iter.c prng.c testsig_LDADD = ../lib/libzebra.la @LIBCAP@ +testsegv_LDADD = ../lib/libzebra.la @LIBCAP@ testbuffer_LDADD = ../lib/libzebra.la @LIBCAP@ testmemory_LDADD = ../lib/libzebra.la @LIBCAP@ testprivs_LDADD = ../lib/libzebra.la @LIBCAP@ diff --git a/tests/test-segv.c b/tests/test-segv.c new file mode 100644 index 0000000000..55bd25a59d --- /dev/null +++ b/tests/test-segv.c @@ -0,0 +1,61 @@ +/* + * SEGV / backtrace handling test. + * + * copied from test-sig.c + * + * Copyright (C) 2013 by David Lamparter, Open Source Routing. + * Copyright (C) 2013 by Internet Systems Consortium, Inc. ("ISC") + * + * This file is part of Quagga + * + * Quagga 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, or (at your option) any + * later version. + * + * Quagga 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 Quagga; see the file COPYING. If not, write to the Free + * Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA + * 02111-1307, USA. + */ + +#include +#include +#include "lib/log.h" +#include "lib/memory.h" + +struct quagga_signal_t sigs[] = +{ +}; + +struct thread_master *master; + +int +threadfunc (struct thread *thread) +{ + int *null = NULL; + *null += 1; + return 0; +} + +int +main (void) +{ + master = thread_master_create (); + signal_init (master, array_size(sigs), sigs); + + zlog_default = openzlog("testsegv", ZLOG_NONE, + LOG_CONS|LOG_NDELAY|LOG_PID, LOG_DAEMON); + zlog_set_level (NULL, ZLOG_DEST_SYSLOG, ZLOG_DISABLED); + zlog_set_level (NULL, ZLOG_DEST_STDOUT, LOG_DEBUG); + zlog_set_level (NULL, ZLOG_DEST_MONITOR, ZLOG_DISABLED); + + thread_execute (master, threadfunc, 0, 0); + + exit (0); +} -- 2.39.5