From: Nick Bowler Date: Tue, 23 Jan 2024 02:02:04 +0000 (-0500) Subject: libcdecl: Actually test threading support. X-Git-Tag: v1.3~26 X-Git-Url: https://git.draconx.ca/gitweb/cdecl99.git/commitdiff_plain/beac10d6593fee948759c05d956b83db6803f2ec libcdecl: Actually test threading support. Nothing actually runs libcdecl's error messaging in multiple threads to check if it actually works. Let's fix that with a new test application. It does mostly work, but it turns out that we leak memory on Windows because gnulib doesn't actually implement TLS destructors in a useful way (on the other hand, using --enable-threads=posix should work even on Windows with a suitable Windows pthreads library implementation). --- diff --git a/.gitignore b/.gitignore index b99d82d..0c4212e 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ *.a +*.exe *.la *.lo *.o diff --git a/Makefile.am b/Makefile.am index cb50bfc..4b3ea88 100644 --- a/Makefile.am +++ b/Makefile.am @@ -123,6 +123,24 @@ check_PROGRAMS += t/scantest t_scantest_LDADD = src/scan.lo src/parse.lo src/keywords.lo $(TEST_LIBS) $(t_scantest_OBJECTS): $(gnulib_headers) src/scan.h src/parse.h +EXTRA_LIBRARIES += t/libmemwrap.a +t_libmemwrap_a_SOURCES = t/memwrap.c +$(t_libmemwrap_a_OBJECTS): $(gnulib_headers) + +EXTRA_LIBRARIES += t/liberrtest.a +t_liberrtest_a_SOURCES = src/error.c +t_liberrtest_a_CPPFLAGS = $(AM_CPPFLAGS) -DTEST_MALLOC_HOOK +t_liberrtest_a_SHORTNAME = t +$(t_liberrtest_a_OBJECTS): $(gnulib_headers) + +check_PROGRAMS += t/errthread +EXTRA_t_errthread_DEPENDENCIES = $(t_liberrtest_a_OBJECTS) \ + $(t_libmemwrap_a_OBJECTS) \ + src/output.lo $(shared_gl_objects) +t_errthread_LDADD = $(EXTRA_t_errthread_DEPENDENCIES) \ + libtest.a $(LTLIBINTL) $(LIBMULTITHREAD) +$(t_errthread_OBJECTS): $(gnulib_headers) src/errmsg.h + src/error.lo: src/errmsg.h src/keywords.lo: src/parse.h src/output.lo: src/parse.h src/specstr.h @@ -133,9 +151,10 @@ t/declgen.$(OBJEXT): t/typegen.h t/cdeclerr.$(OBJEXT): src/errmsg.h check_PROGRAMS += t/cdeclerr -t_cdeclerr_SOURCES = common/src/tap.c t/cdeclerr.c +t_cdeclerr_SOURCES = t/cdeclerr.c EXTRA_t_cdeclerr_DEPENDENCIES = src/error.lo src/output.lo $(shared_gl_objects) -t_cdeclerr_LDADD = $(EXTRA_t_cdeclerr_DEPENDENCIES) $(LTLIBINTL) $(LIBTHREAD) +t_cdeclerr_LDADD = $(EXTRA_t_cdeclerr_DEPENDENCIES) \ + libtest.a $(LTLIBINTL) $(LIBTHREAD) $(t_cdeclerr_OBJECTS): $(gnulib_headers) # Supporting rules for gettext. diff --git a/configure.ac b/configure.ac index 62715e8..d8a413b 100644 --- a/configure.ac +++ b/configure.ac @@ -40,6 +40,10 @@ gl_INIT # no need for multithreaded mbrtowc (all programs are single threaded). AC_DEFINE([GNULIB_MBRTOWC_SINGLE_THREAD], [1], [Define to 1.]) +# As the wcwidth replacement module is not used by the library, there is +# no need to support varying locales (programs set locale once at startup). +AC_DEFINE([GNULIB_WCHAR_SINGLE_LOCALE], [1], [Define to 1.]) + AS_IF([test x"$gl_cv_lib_readline" = x"no"], [AS_IF([test x"$with_readline" = x"yes"], [AC_MSG_FAILURE([--with-readline requested but readline was not found])], diff --git a/src/cdecl-internal.h b/src/cdecl-internal.h index dc8d1f0..335094c 100644 --- a/src/cdecl-internal.h +++ b/src/cdecl-internal.h @@ -134,4 +134,28 @@ struct parse_item *cdecl__alloc_item(size_t s_sz); # endif #endif +/* + * Build-time hook for white-box testing of memory allocation behaviour. + */ +#if TEST_MALLOC_HOOK +void *test_realloc_hook(void *, size_t); + +static inline void *test_wrap_malloc(size_t n) +{ + return test_realloc_hook(0, n); +} + +static inline void test_wrap_free(void *p) +{ + test_realloc_hook(p, 0); +} + +#undef realloc +#define realloc test_realloc_hook +#undef malloc +#define malloc test_wrap_malloc +#undef free +#define free test_wrap_free +#endif + #endif diff --git a/t/.gitignore b/t/.gitignore index 895ab96..c596b97 100644 --- a/t/.gitignore +++ b/t/.gitignore @@ -7,3 +7,4 @@ /rng-test /scantest /typegen.h +/errthread diff --git a/t/cdeclerr.c b/t/cdeclerr.c index 3555d68..2120e24 100644 --- a/t/cdeclerr.c +++ b/t/cdeclerr.c @@ -27,9 +27,12 @@ #include "errmsg.h" #include "tap.h" +/* + * Function called from output.c but not needed for error messaging. + */ const char *cdecl__token_name(unsigned token) { - assert(0); + tap_bail_out("stub cdecl__token_name called"); } static char *fmt_char(int c, char *buf) diff --git a/t/errthread.c b/t/errthread.c new file mode 100644 index 0000000..445fc83 --- /dev/null +++ b/t/errthread.c @@ -0,0 +1,200 @@ +/* + * Sanity check of libcdecl multithread safety. + * + * Copyright © 2024 Nick Bowler + * + * 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 3 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. If not, see . + */ + +#include +#include +#include "cdecl-internal.h" +#include "errmsg.h" +#include "tap.h" + +/* + * Function called from output.c but not needed for error messaging. + */ +const char *cdecl__token_name(unsigned token) +{ + tap_bail_out("stub cdecl__token_name called"); +} + +/* + * Prior returned value from cdecl_get_error in the main thread. + */ +static const struct cdecl_error *thread1_err; + +/* + * Check that the error code and message matches expectations (noting that + * this application does not call setlocale to enable translations). + */ +static void check_simple_err(const struct cdecl_error *err, unsigned t, + unsigned exp_code, unsigned msg_id) +{ + static const char errmsgs[] = STRTAB_INITIALIZER; + const char *exp_msg; + + if (!tap_result(err->code == exp_code, "thread[%u] err->code", t)) { + tap_diag("Failed, unexpected result"); + tap_diag(" Received: %u", err->code); + tap_diag(" Expected: %u", exp_code); + } + + exp_msg = &errmsgs[msg_id]; + if (!tap_result(!strcmp(err->str, exp_msg), "thread[%u] err->str", t)) { + tap_diag("Failed, unexpected result"); + tap_diag(" Received: %.*s", (int)strlen(exp_msg), err->str); + tap_diag(" Expected: %s", exp_msg); + } +} + +static void thread2_func(void) +{ + const struct cdecl_error *err; + + cdecl__errmsg(CDECL__ENOTYPE); + err = cdecl_get_error(); + + /* + * Ensure that the error returned in this new thread is distinct from + * the error returned in the main thread. + */ + tap_diag("thread[2] err: %p", (void *)err); + tap_result(thread1_err != err, "thread[2] new state"); + + check_simple_err(err, 2, CDECL_ENOPARSE, CDECL__ENOTYPE); + + tap_diag("thread[2] exit"); +} + +#if USE_POSIX_THREADS || USE_ISOC_AND_POSIX_THREADS +#define THREAD_API "posix" +#include + +static void *thread2(void *p) +{ + thread2_func(); + return 0; +} + +static void run_thread2(void) +{ + pthread_t t; + int err; + + if (!(err = pthread_create(&t, 0, thread2, 0))) + if (!(err = pthread_join(t, 0))) + return; + + tap_bail_out("run_thread2 failed: %s", strerror(err)); +} + +#elif USE_ISOC_THREADS +#define THREAD_API "isoc" +#include + +static int thread2(void *p) +{ + thread2_func(); + return 0; +} + +static void run_thread2(void) +{ + thrd_t t; + if (thrd_create(&t, thread2, 0) == thrd_success) + if (thrd_join(t, 0) == thrd_success) + return; + + tap_bail_out("run_thread2 failed"); +} + +#elif USE_WINDOWS_THREADS +#define THREAD_API "windows" +#define WIN32_LEAN_AND_MEAN +#include + +static DWORD WINAPI thread2(LPVOID p) +{ + thread2_func(); + return 0; +} + +static void run_thread2(void) +{ + HANDLE h; + DWORD rc; + + if ((h = CreateThread(NULL, 0, thread2, NULL, 0, &rc))) { + do { + if (GetExitCodeThread(h, &rc) && rc != STILL_ACTIVE) { + CloseHandle(h); + return; + } + } while (WaitForSingleObject(h, INFINITE) != WAIT_FAILED); + } + + tap_bail_out("run_thread2 failed (%lu)", GetLastError()); +} +#else +#undef THREAD_API +int main(void) +{ + tap_skip_all("multithreading support disabled"); +} +#endif + +#ifdef THREAD_API +int main(void) +{ + size_t test_live_allocations(void); + const struct cdecl_error *err; + + tap_diag("using thread API: " THREAD_API); + tap_plan(9); + + /* Simulate an error in the main thread. */ + cdecl__errmsg(CDECL__ENOMEM); + thread1_err = cdecl_get_error(); + tap_diag("thread[1] err: %p", (void *)thread1_err); + check_simple_err(thread1_err, 1, CDECL_ENOMEM, CDECL__ENOMEM); + + run_thread2(); + + /* + * Back in the main thread, the error previously returned by + * cdecl_get_error() should still be valid. + */ + check_simple_err(thread1_err, 1, CDECL_ENOMEM, CDECL__ENOMEM); + + /* + * Moreover, cdecl_get_error should return the same pointer it did + * last time (undocumented implementation detail). + */ + if (!tap_result((err = cdecl_get_error()) == thread1_err, + "thread[1] unchanged state")) + { + tap_diag("Failed, unexpected result"); + tap_diag(" Received: %p", (void *)err); + tap_diag(" Expected: %p", (void *)thread1_err); + } + + /* + * Main thread allocation should be the only one left. + */ + tap_result(test_live_allocations() == 1, "thread cleanup"); + tap_done(); +} +#endif diff --git a/t/memwrap.c b/t/memwrap.c new file mode 100644 index 0000000..8b54433 --- /dev/null +++ b/t/memwrap.c @@ -0,0 +1,116 @@ +/* + * Allocation wrapper for test purposes. + * + * Copyright © 2024 Nick Bowler + * + * 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 3 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. If not, see . + */ + +#include +#include +#include +#include +#include "cdecl-internal.h" + +static union test_alloc { + union test_alloc *p; + max_align_t a; + size_t v; +} *alloc_head; + +enum { + ALLOC_NEXT_PTR, + ALLOC_STATE, + ALLOC_SIZE, + ALLOC_PAYLOAD +}; + +#define ALLOC_STATE_ALLOCATED 0x7143u +#define ALLOC_STATE_FREED 0xdeadu + +/* + * Hook for testing allocation behaviour of the library, enabling a basic + * verification in test cases that the library does not leak memory. + * + * The library sources must be recompiled with -DTEST_MALLOC_HOOK to + * make use of this functionality. + * + * In this implementation, allocated memory is never freed, instead just + * marked with an indication that the memory is no longer live. + */ +void *test_realloc_hook(void *p, size_t n) +{ + union test_alloc *old_alloc = p, *alloc; + + if (old_alloc) { + old_alloc = &old_alloc[-ALLOC_PAYLOAD]; + if (old_alloc[ALLOC_STATE].v != ALLOC_STATE_ALLOCATED) { + printf("Bail out! %p is not a live allocation!\n", p); + exit(99); + } + + old_alloc[ALLOC_STATE].v = ALLOC_STATE_FREED; + printf("# %p freed\n", p); + } + + if (!n) + return NULL; + + n = (n + sizeof *alloc - 1) / sizeof *alloc; + alloc = (malloc)((n + ALLOC_PAYLOAD) * sizeof *alloc); + if (!alloc) + return NULL; + + alloc[ALLOC_NEXT_PTR].p = alloc_head; + alloc_head = alloc; + + alloc[ALLOC_STATE].v = ALLOC_STATE_ALLOCATED; + alloc[ALLOC_SIZE].v = n * sizeof *alloc; + + if (old_alloc) { + n = old_alloc[ALLOC_SIZE].v; + memcpy(&alloc[ALLOC_PAYLOAD], &old_alloc[ALLOC_PAYLOAD], n); + } + + p = &alloc[ALLOC_PAYLOAD]; + printf("# %p allocated\n", p); + return p; +} + +/* + * Returns the total number of allocations that have not yet been freed. + */ +size_t test_live_allocations(void) +{ + union test_alloc *a; + size_t ret = 0; + + for (a = alloc_head; a; a = a[ALLOC_NEXT_PTR].p) { + void *p = &a[ALLOC_PAYLOAD]; + + switch (a[ALLOC_STATE].v) { + case ALLOC_STATE_ALLOCATED: + printf("# %p still live\n", p); + ret++; + break; + case ALLOC_STATE_FREED: + break; + default: + printf("Bail out! detected %p state corruption\n", p); + exit(99); + } + } + + return ret; +} diff --git a/tests/internal.at b/tests/internal.at index ee87919..3430c1a 100644 --- a/tests/internal.at +++ b/tests/internal.at @@ -1,4 +1,4 @@ -# Copyright © 2021, 2023 Nick Bowler +# Copyright © 2021, 2023-2024 Nick Bowler # # 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 @@ -74,6 +74,14 @@ AT_CLEANUP TEST_TAP_SIMPLE([cdecl__err sanity], [cdeclerr], [TEST_NEED_PROGRAM([cdeclerr])], [libcdecl internal]) +AT_SETUP([cdecl_err thread safety]) +AT_KEYWORDS([libcdecl internal threads])dnl +TEST_NEED_PROGRAM([errthread]) +AT_XFAIL_IF( + [grep '^#define USE_WINDOWS_THREADS 1' "$builddir/config.h" >/dev/null 2>&1]) +TEST_TAP([errthread]) +AT_CLEANUP + AT_SETUP([cdecl_declare truncation]) AT_KEYWORDS([libcdecl internal])