From: Nick Bowler Date: Sat, 27 Jan 2024 04:50:19 +0000 (-0500) Subject: libcdecl: Fix TLS cleanup on Windows. X-Git-Tag: v1.3~16 X-Git-Url: https://git.draconx.ca/gitweb/cdecl99.git/commitdiff_plain/898aa30bfb038ffeca54d6dea95f0f80fbc08f7f libcdecl: Fix TLS cleanup on Windows. The DllMain entry point is pretty much designed for this purpose and works in all versions of Windows that support threads. This is the technique used by pthreads-win32. But it only works for DLLs. For static linking into executables, there is a special table of function pointers that are called by the runtime in pretty much the same situations as DllMain. This apparently works starting around Windows XP timeframe, and is the technique used by libwinpthread. We combine both techniques so at least the DLL cleanup works even on very old versions of Windows. The static library cleanup will work on contemporary versions. To test this, we need to restructure the thread test program to link against a real (uninstalled) DLL with the test entry points. --- diff --git a/Makefile.am b/Makefile.am index 6ff050a..452e811 100644 --- a/Makefile.am +++ b/Makefile.am @@ -124,22 +124,19 @@ 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) +EXTRA_LTLIBRARIES += t/liberrtest.la +t_liberrtest_la_SOURCES = src/error.c t/errmemwrap.c +t_liberrtest_la_CPPFLAGS = $(AM_CPPFLAGS) -DTEST_MALLOC_HOOK +t_liberrtest_la_LDFLAGS = -no-undefined -avoid-version \ + -bindir '$(bindir)' -rpath '$(libdir)' +EXTRA_t_liberrtest_la_DEPENDENCIES = $(shared_gl_objects) +t_liberrtest_la_LIBADD = src/output.lo $(EXTRA_t_liberrtest_la_DEPENDENCIES) \ + $(LTLIBINTL) $(LIBTHREAD) +t_liberrtest_la_SHORTNAME = t +$(t_liberrtest_la_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_LDADD = t/liberrtest.la libtest.a $(LIBMULTITHREAD) $(t_errthread_OBJECTS): $(gnulib_headers) src/errmsg.h src/error.lo: src/errmsg.h diff --git a/src/thread-w32.h b/src/thread-w32.h index 181ebfc..393f0de 100644 --- a/src/thread-w32.h +++ b/src/thread-w32.h @@ -33,7 +33,7 @@ #define WIN32_LEAN_AND_MEAN #include -static DWORD tls_key; +static DWORD tls_key = TLS_OUT_OF_INDEXES; #define tls_key_valid (tls_key != TLS_OUT_OF_INDEXES) static void init_once_cb(void); @@ -158,3 +158,37 @@ static int init_once(void) return 1; } + +#if !TEST_W32_NO_DLLMAIN + +/* + * On Windows, DLLs are notified of thread exit via the DllMain entry point. + * This works in all versions. + */ +#if !DLL_EXPORT +static +#endif +BOOL WINAPI DllMain(HINSTANCE hinst, DWORD reason, LPVOID p) +{ + if (reason == DLL_THREAD_DETACH && tls_key != TLS_OUT_OF_INDEXES) + free(TlsGetValue(tls_key)); + + return TRUE; +} + +/* + * We can achieve similar behaviour with static linking executables by + * putting a pointer to the entry point in a special section. + * + * I believe this is supported beginning around Windows XP. + */ +#if !DLL_EXPORT +#pragma data_seg(".CRT$XLF") +#if __GNUC__ +__attribute__((section(".CRT$XLF"))) +#endif +PIMAGE_TLS_CALLBACK cdecl__tls_hook = (PIMAGE_TLS_CALLBACK)DllMain; +#pragma data_seg() +#endif + +#endif diff --git a/t/memwrap.c b/t/errmemwrap.c similarity index 88% rename from t/memwrap.c rename to t/errmemwrap.c index 8b54433..85541e1 100644 --- a/t/memwrap.c +++ b/t/errmemwrap.c @@ -21,7 +21,12 @@ #include #include #include + +#ifndef TEST_MALLOC_HOOK +# define TEST_MALLOC_HOOK 1 +#endif #include "cdecl-internal.h" +#undef malloc static union test_alloc { union test_alloc *p; @@ -68,7 +73,7 @@ void *test_realloc_hook(void *p, size_t n) return NULL; n = (n + sizeof *alloc - 1) / sizeof *alloc; - alloc = (malloc)((n + ALLOC_PAYLOAD) * sizeof *alloc); + alloc = malloc((n + ALLOC_PAYLOAD) * sizeof *alloc); if (!alloc) return NULL; @@ -114,3 +119,13 @@ size_t test_live_allocations(void) return ret; } + +/* + * Function called from output.c but not needed for error messaging + * (only current user of this file) + */ +const char *cdecl__token_name(unsigned token) +{ + printf("Bail out! stub cdecl__token_name called\n"); + exit(99); +} diff --git a/t/errthread.c b/t/errthread.c index 0f1fdf1..3161cf4 100644 --- a/t/errthread.c +++ b/t/errthread.c @@ -23,14 +23,6 @@ #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. */ @@ -60,46 +52,20 @@ static void check_simple_err(const struct cdecl_error *err, unsigned t, } } -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)); -} +#define DEFINE_SIMPLE_THREAD_TEST(run_func, thread_entry_func) \ + static void *run_func##_(void *p) { thread_entry_func(); return 0; } \ + static void run_func(void) \ + { \ + pthread_t t; int err; \ + if (!(err = pthread_create(&t, 0, run_func##_, 0))) \ + if (!(err = pthread_join(t, 0))) \ + return; \ + tap_bail_out("%s failed: %s", #run_func, strerror(err)); \ + } static void check_init_once(void) { @@ -110,21 +76,16 @@ static void check_init_once(void) #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"); -} +#define DEFINE_SIMPLE_THREAD_TEST(run_func, thread_entry_func) \ + static int run_func##_(void *p) { thread_entry_func(); return 0; } \ + static void run_func(void) \ + { \ + thrd_t t; \ + if (thrd_create(&t, run_func##_, 0) == thrd_success) \ + if (thrd_join(t, 0) == thrd_success) \ + return; \ + tap_bail_out("%s failed", #run_func); \ + } static void check_init_once(void) { @@ -136,30 +97,23 @@ static void check_init_once(void) #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); +#define DEFINE_SIMPLE_THREAD_TEST(run_func, thread_entry_func) \ + static DWORD WINAPI run_func##_(LPVOID p) \ + { thread_entry_func(); return 0; } \ + static void run_func(void) \ + { \ + HANDLE h; DWORD rc; \ + if ((h = CreateThread(0, 0, run_func##_, 0, 0, &rc))) do { \ + if (GetExitCodeThread(h, &rc) && rc != STILL_ACTIVE) { \ + CloseHandle(h); \ + return; \ + } \ + } while (WaitForSingleObject(h, INFINITE) != WAIT_FAILED); \ + tap_bail_out("%s failed (%lu)", #run_func, GetLastError()); \ } - tap_bail_out("run_thread2 failed (%lu)", GetLastError()); -} - /* Also include init_once sanity test on Windows */ +#define TEST_W32_NO_DLLMAIN 1 #include "thread-w32.h" HANDLE init_semaphore; @@ -222,13 +176,43 @@ int main(void) #endif #ifdef THREAD_API + +static void thread_result(void) +{ + tap_result(1, "thread runs"); +} +DEFINE_SIMPLE_THREAD_TEST(check_thread_create, thread_result); + +static void err_in_thread(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"); +} +DEFINE_SIMPLE_THREAD_TEST(check_err_in_thread, err_in_thread); + int main(void) { size_t test_live_allocations(void); const struct cdecl_error *err; tap_diag("using thread API: " THREAD_API); - tap_plan(10); + tap_plan(11); + + /* Create/join a thread to tickle DllMain on Windows. */ + check_thread_create(); /* Simulate an error in the main thread. */ cdecl__errmsg(CDECL__ENOMEM); @@ -236,7 +220,7 @@ int main(void) tap_diag("thread[1] err: %p", (void *)thread1_err); check_simple_err(thread1_err, 1, CDECL_ENOMEM, CDECL__ENOMEM); - run_thread2(); + check_err_in_thread(); /* * Back in the main thread, the error previously returned by diff --git a/tests/internal.at b/tests/internal.at index 3430c1a..b35477a 100644 --- a/tests/internal.at +++ b/tests/internal.at @@ -74,13 +74,8 @@ 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 +TEST_TAP_SIMPLE([cdecl__err thread safety], [errthread], + [TEST_NEED_PROGRAM([errthread])], [libcdecl internal threads]) AT_SETUP([cdecl_declare truncation]) AT_KEYWORDS([libcdecl internal])