Laszlo Ersek
2023-Mar-15 11:01 UTC
[Libguestfs] [libnbd PATCH v4 3/3] lib/utils: add unit test for async-signal-safe assert()
Don't try to test async-signal-safety, only that
NBD_INTERNAL_FORK_SAFE_ASSERT() works similarly to assert():
- it prints diagnostics to stderr,
- it calls abort().
Some unfortunate gymnastics are necessary to avoid littering the system
with unwanted core dumps.
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
Notes:
v4:
- update Red Hat Copyright Notices in the new files, in accordance with
commit bbf47ffd4ac4 ("Update Red Hat Copyright Notices",
2023-03-04)
- explain the TRUE and FALSE macro definitions in code comments [Eric]
- test that the pattern "`TRUE'" is not present in the output
file; that
is, the passing assertion does not generate unexpected output [Eric]
- do not pick up R-b tags due to the logic update in the last bullet
above
configure.ac | 5 ++
lib/test-fork-safe-assert.c | 66 ++++++++++++++++++++
lib/Makefile.am | 38 +++++++++--
lib/test-fork-safe-assert.sh | 32 ++++++++++
.gitignore | 2 +
5 files changed, 139 insertions(+), 4 deletions(-)
diff --git a/configure.ac b/configure.ac
index b6d60c3df6a1..62fe470b6cd5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -132,11 +132,16 @@ dnl Check for various libc functions, all optional.
dnl
dnl posix_fadvise helps to optimise linear reads and writes.
dnl
+dnl When /proc/sys/kernel/core_pattern starts with a pipe (|) symbol, Linux
+dnl ignores "ulimit -c" and (equivalent) setrlimit(RLIMIT_CORE)
actions, for
+dnl disabling core dumping. Only prctl() can be used then, for that purpose.
+dnl
dnl strerrordesc_np (glibc only) is preferred over sys_errlist:
dnl https://lists.fedoraproject.org/archives/list/glibc at
lists.fedoraproject.org/thread/WJHGG2OO7ABNAYICGA5WQZ2Q34Q2FEHU/
AC_CHECK_FUNCS([\
posix_fadvise \
posix_memalign \
+ prctl \
strerrordesc_np \
valloc])
diff --git a/lib/test-fork-safe-assert.c b/lib/test-fork-safe-assert.c
new file mode 100644
index 000000000000..4a4f6e88ce65
--- /dev/null
+++ b/lib/test-fork-safe-assert.c
@@ -0,0 +1,66 @@
+/* nbd client library in userspace
+ * Copyright Red Hat
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#ifdef HAVE_PRCTL
+#include <sys/prctl.h>
+#endif
+#include <sys/resource.h>
+
+#undef NDEBUG
+
+#include "internal.h"
+
+/* Define these to verify that NBD_INTERNAL_FORK_SAFE_ASSERT() properly
+ * stringifies the expression passed to it.
+ */
+#define TRUE 1
+#define FALSE 0
+
+int
+main (void)
+{
+ struct rlimit rlimit;
+
+ /* The standard approach for disabling core dumping. Has no effect on Linux
if
+ * /proc/sys/kernel/core_pattern starts with a pipe (|) symbol.
+ */
+ if (getrlimit (RLIMIT_CORE, &rlimit) == -1) {
+ perror ("getrlimit");
+ return EXIT_FAILURE;
+ }
+ rlimit.rlim_cur = 0;
+ if (setrlimit (RLIMIT_CORE, &rlimit) == -1) {
+ perror ("setrlimit");
+ return EXIT_FAILURE;
+ }
+
+#ifdef HAVE_PRCTL
+ if (prctl (PR_SET_DUMPABLE, 0, 0, 0, 0) == -1) {
+ perror ("prctl");
+ return EXIT_FAILURE;
+ }
+#endif
+
+ NBD_INTERNAL_FORK_SAFE_ASSERT (TRUE);
+ NBD_INTERNAL_FORK_SAFE_ASSERT (FALSE);
+ return 0;
+}
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 52b525819bde..326b550f90bd 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -25,15 +25,30 @@ generator_built = \
unlocked.h \
$(NULL)
+CLEANFILES += \
+ test-fork-safe-assert.err \
+ $(NULL)
+
EXTRA_DIST = \
$(generator_built) \
libnbd.syms \
+ test-fork-safe-assert.sh \
$(NULL)
lib_LTLIBRARIES = libnbd.la
BUILT_SOURCES = $(generator_built)
+AM_CPPFLAGS = \
+ -I$(top_srcdir)/include \
+ -I$(top_srcdir)/common/include \
+ -I$(top_srcdir)/common/utils \
+ $(NULL)
+
+AM_CFLAGS = \
+ $(WARNINGS_CFLAGS) \
+ $(NULL)
+
libnbd_la_SOURCES = \
aio.c \
api.c \
@@ -60,13 +75,11 @@ libnbd_la_SOURCES = \
utils.c \
$(NULL)
libnbd_la_CPPFLAGS = \
- -I$(top_srcdir)/include \
- -I$(top_srcdir)/common/include \
- -I$(top_srcdir)/common/utils \
+ $(AM_CPPFLAGS) \
-Dsysconfdir=\"$(sysconfdir)\" \
$(NULL)
libnbd_la_CFLAGS = \
- $(WARNINGS_CFLAGS) \
+ $(AM_CFLAGS) \
$(PTHREAD_CFLAGS) \
$(GNUTLS_CFLAGS) \
$(LIBXML2_CFLAGS) \
@@ -86,3 +99,20 @@ libnbd_la_LDFLAGS = \
pkgconfigdir = $(libdir)/pkgconfig
pkgconfig_DATA = libnbd.pc
+
+# Unit tests.
+
+TESTS = \
+ test-fork-safe-assert.sh \
+ $(NULL)
+
+check_PROGRAMS = \
+ test-fork-safe-assert \
+ $(NULL)
+
+test_fork_safe_assert_SOURCES = \
+ $(top_srcdir)/common/utils/vector.c \
+ errors.c \
+ test-fork-safe-assert.c \
+ utils.c \
+ $(NULL)
diff --git a/lib/test-fork-safe-assert.sh b/lib/test-fork-safe-assert.sh
new file mode 100755
index 000000000000..2cf0a29a4c6d
--- /dev/null
+++ b/lib/test-fork-safe-assert.sh
@@ -0,0 +1,32 @@
+#!/usr/bin/env bash
+# nbd client library in userspace
+# Copyright Red Hat
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2 of the License, or (at your option) any later version.
+#
+# This library 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
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+set +e
+
+./test-fork-safe-assert 2>test-fork-safe-assert.err
+exit_status=$?
+
+set -e
+
+test $exit_status -gt 128
+signal_name=$(kill -l $exit_status)
+test "x$signal_name" = xABRT || test "x$signal_name" =
xSIGABRT
+
+ptrn="^test-fork-safe-assert\\.c:[0-9]+: main: Assertion \`FALSE'
failed\\.\$"
+grep -E -q -- "$ptrn" test-fork-safe-assert.err
+grep -v -q "\`TRUE'" test-fork-safe-assert.err
diff --git a/.gitignore b/.gitignore
index 4ebd96519c29..54bc20173f95 100644
--- a/.gitignore
+++ b/.gitignore
@@ -128,6 +128,8 @@ Makefile.in
/lib/states-run.c
/lib/states.c
/lib/states.h
+/lib/test-fork-safe-assert
+/lib/test-fork-safe-assert.err
/lib/unlocked.h
/libtool
/ltmain.sh
Eric Blake
2023-Mar-15 17:25 UTC
[Libguestfs] [libnbd PATCH v4 3/3] lib/utils: add unit test for async-signal-safe assert()
On Wed, Mar 15, 2023 at 12:01:57PM +0100, Laszlo Ersek wrote:> Don't try to test async-signal-safety, only that > NBD_INTERNAL_FORK_SAFE_ASSERT() works similarly to assert(): > > - it prints diagnostics to stderr, > > - it calls abort(). > > Some unfortunate gymnastics are necessary to avoid littering the system > with unwanted core dumps. > > Signed-off-by: Laszlo Ersek <lersek at redhat.com> > ---> diff --git a/configure.ac b/configure.ac > index b6d60c3df6a1..62fe470b6cd5 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -132,11 +132,16 @@ dnl Check for various libc functions, all optional. > dnl > dnl posix_fadvise helps to optimise linear reads and writes. > dnl > +dnl When /proc/sys/kernel/core_pattern starts with a pipe (|) symbol, Linux > +dnl ignores "ulimit -c" and (equivalent) setrlimit(RLIMIT_CORE) actions, for > +dnl disabling core dumping. Only prctl() can be used then, for that purpose. > +dnl > dnl strerrordesc_np (glibc only) is preferred over sys_errlist: > dnl https://lists.fedoraproject.org/archives/list/glibc at lists.fedoraproject.org/thread/WJHGG2OO7ABNAYICGA5WQZ2Q34Q2FEHU/ > AC_CHECK_FUNCS([\ > posix_fadvise \ > posix_memalign \ > + prctl \ > strerrordesc_np \ > valloc])AC_CHECK_FUNCS looks for whether the given entry point can be linked with, which is okay for functions in the common headers (<stdlib.h>, <unistd.h>, ...) that autoconf includes in its test programs by default. But...> > diff --git a/lib/test-fork-safe-assert.c b/lib/test-fork-safe-assert.c > new file mode 100644 > index 000000000000..4a4f6e88ce65 > --- /dev/null > +++ b/lib/test-fork-safe-assert.c > @@ -0,0 +1,66 @@> + > +#include <stdio.h> > +#include <stdlib.h> > +#ifdef HAVE_PRCTL > +#include <sys/prctl.h> > +#endif...the fact that prctl() is in a non-standard header makes me wonder if we might fail to detect the function merely because we didn't include the right header, rather than because its symbol was not exported. On the other hand, prctl() is definitely Linux-specific, so I think you are quite safe in assuming that <sys/prctl.h> exists if and only if prctl() is a linkable entry point. If it does turn out to break someone, we can fix it in a followup patch, so no change needed in your usage at this time.> +++ b/lib/test-fork-safe-assert.sh > @@ -0,0 +1,32 @@ > +#!/usr/bin/env bashReviewed-by: Eric Blake <eblake at redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org