From 3fbe42c3a219731869a6ece9621d47b55eed0259 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Tue, 2 Jan 2024 14:57:36 +0100 Subject: [PATCH] Add unittest to check thread_local destructor brokenness --- src/unittest/test_threading.cpp | 86 +++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/src/unittest/test_threading.cpp b/src/unittest/test_threading.cpp index 65ef7c02d..9d7b7c808 100644 --- a/src/unittest/test_threading.cpp +++ b/src/unittest/test_threading.cpp @@ -20,6 +20,7 @@ with this program; if not, write to the Free Software Foundation, Inc., #include "test.h" #include +#include #include "threading/semaphore.h" #include "threading/thread.h" @@ -32,6 +33,7 @@ public: void testStartStopWait(); void testAtomicSemaphoreThread(); + void testTLS(); }; static TestThreading g_test_instance; @@ -40,6 +42,7 @@ void TestThreading::runTests(IGameDef *gamedef) { TEST(testStartStopWait); TEST(testAtomicSemaphoreThread); + TEST(testTLS); } class SimpleTestThread : public Thread { @@ -156,3 +159,86 @@ void TestThreading::testAtomicSemaphoreThread() UASSERT(val == num_threads * 0x10000); } + + +static volatile bool g_tls_broken; + +class TLSTestThread : public Thread { +public: + TLSTestThread() : Thread("TLSTest") + { + } + +private: + struct TestObject { + TestObject() { + for (u32 i = 0; i < buffer_size; i++) + buffer[i] = (i % 2) ? 0xa1 : 0x1a; + } + ~TestObject() { + for (u32 i = 0; i < buffer_size; i++) { + const u8 expect = (i % 2) ? 0xa1 : 0x1a; + if (buffer[i] != expect) { + std::cout << "At offset " << i << " expected " << (int)expect + << " but found " << (int)buffer[i] << std::endl; + g_tls_broken = true; + break; + } + // If we're unlucky the loop might actually just crash. + // probably the user will realize the test failure :) + } + } + // larger objects seem to surface corruption more easily + static constexpr u32 buffer_size = 576; + u8 buffer[buffer_size]; + }; + + void *run() { + thread_local TestObject foo; + while (!stopRequested()) + sleep_ms(1); + return nullptr; + } +}; + +/* + What are we actually testing here? + + MinGW with gcc has a long-standing unsolved bug where memory belonging to + thread-local variables is freed *before* the destructors are called. + Needless to say this leads to unreliable crashes whenever a thread exits. + It does not affect MSVC or MinGW+clang and as far as we know no other platforms + are affected either. + + Related reports and information: + * (2018) + * (2017) + * maybe + + Note that this is different from , + which affected only 32-bit MinGW. It was caused by incorrect calling convention + and fixed in GCC in 2020. + + Occurrences in Minetest: + * (2020) + * (2022) + * (2023) +*/ +void TestThreading::testTLS() +{ + static const int num_threads = 10; + + for (int j = 0; j < num_threads; j++) { + g_tls_broken = false; + + TLSTestThread thread; + thread.start(); + thread.stop(); + thread.wait(); + + if (g_tls_broken) { + std::cout << "While running test thread " << j << std::endl; + UASSERT(!g_tls_broken); + } + } +}