Static apps build with MinGW-w64 exit with segfault (Issue #26195)
Maarten
MaartenBent created an issue (wxWidgets/wxWidgets#26195)Bug description:
Every time I close an application, it ends with a segmentation fault:
$ ./lib/clang_x64_lib/minimal.exe
Segmentation fault ./lib/clang_x64_lib/minimal.exe
This is caused by the changes from #25928 / #25897, cc @ardovm
Maybe this should only apply to old MinGW versions? The linked issue uses the old GCC 7.2.0 from 2017.
There is mentioning of a possible fix in msys2/MINGW-packages#2519 with mingw-w64/mingw-w64@8e06daa, so I propose to only use this workaround for MinGW-w64 versions before 14.0.
diff --git "a/src/common/translation.cpp" "b/src/common/translation.cpp" index c5b38899496..d99d088ba04 100644 --- "a/src/common/translation.cpp" +++ "b/src/common/translation.cpp" @@ -1473,7 +1473,8 @@ using UntranslatedStrings = std::unordered_set<wxString>; be executed when any thread (not necessarily created by wxWidgets) exits to ensure that we always perform the required cleanup. */ -#ifdef __MINGW32__ +#if defined(__MINGW32__) && \ + (!defined(__MINGW64_VERSION_MAJOR) || __MINGW64_VERSION_MAJOR < 14) class UntranslatedStringHolder {
Stack trace:
Thread 1 received signal SIGSEGV, Segmentation fault.
0x00007ffcbf954a16 in ntdll!RtlWaitOnAddress () from C:\Windows\SYSTEM32\ntdll.dll
(gdb) bt
#0 0x00007ffcbf954a16 in ntdll!RtlWaitOnAddress () from C:\Windows\SYSTEM32\ntdll.dll
#1 0x00007ffcbf91fcb4 in ntdll!RtlEnterCriticalSection () from C:\Windows\SYSTEM32\ntdll.dll
#2 0x00007ffcbf91fae2 in ntdll!RtlEnterCriticalSection () from C:\Windows\SYSTEM32\ntdll.dll
#3 0x00007ff66f88a854 in wxCriticalSection::Enter (this=0x7ff66fde48c0 <(anonymous namespace)::UntranslatedStringHolder::ms_criticalSection>)
at D:\dev\wxWidgets\src\msw/thread.cpp:143
#4 0x00007ff66f9f52b3 in wxCriticalSectionLocker::wxCriticalSectionLocker (this=0x5ffd68, cs=...) at D:/dev/wxWidgets/include/wx/thread.h:299
#5 0x00007ff66f85527a in (anonymous namespace)::UntranslatedStringHolder::~UntranslatedStringHolder (this=0x630c78)
at D:\dev\wxWidgets\src\common/translation.cpp:1505
#6 0x00007ff66f969a8d in run_dtor_list (ptr=<optimized out>) at D:/W/B/src/mingw-w64/mingw-w64-crt/crt/tls_atexit.c:62
#7 run_dtor_list (ptr=0x630cb0) at D:/W/B/src/mingw-w64/mingw-w64-crt/crt/tls_atexit.c:56
#8 tls_atexit_callback (hDllHandle=<optimized out>, dwReason=0, lpReserved=<optimized out>) at D:/W/B/src/mingw-w64/mingw-w64-crt/crt/tls_atexit.c:92
#9 tls_atexit_callback (hDllHandle=<optimized out>, dwReason=<optimized out>, lpReserved=<optimized out>)
at D:/W/B/src/mingw-w64/mingw-w64-crt/crt/tls_atexit.c:89
#10 0x00007ff66f980eb9 in run_callback () at D:/W/B/src/mingw-w64/mingw-w64-crt/misc/register_tls_atexit.c:17
#11 0x00007ffcbeeca73b in msvcrt!_initterm_e () from C:\Windows\System32\msvcrt.dll
#12 0x00007ff66f5d140e in __tmainCRTStartup () at D:/W/B/src/mingw-w64/mingw-w64-crt/crt/crtexe.c:263
#13 0x00007ff66f5d1436 in WinMainCRTStartup () at D:/W/B/src/mingw-w64/mingw-w64-crt/crt/crtexe.c:99
To Reproduce:
Built the static minimal sample with MSYS2/MinGW x64 or x32, either with Clang or GCC. Open and close the app.
Platform and version information
- wxWidgets version you use: master
- wxWidgets port you use: wxMSW
- OS and its version: Windows 10
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26195)It looks like the local wxPerThreadStrings is destroyed after the global UntranslatedStringHolder::ms_criticalSection (this could be checked by wrapping the latter into some class printing something in its dtor)?
I wonder if we should just change wxGetTranslation() to return wxString and get rid of all this code entirely. Would returning wxString really break anything?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
MaartenBent left a comment (wxWidgets/wxWidgets#26195)Looks like it. But even without using wxCriticalSectionLocker it segfaults, this time on the ms_setsMap access:
0x00007ff60a542237 in std::less<unsigned long>::operator() (
this=0x7ff60a8b48c0 <(anonymous namespace)::UntranslatedStringHolder::ms_setsMap>,
__x=<error reading variable: Cannot access memory at address 0xfeeefeeefeeeff0e>, __y=@0x5ffd6c: 6424)
at D:/msys64/mingw64/include/c++/15.2.0/bits/stl_function.h:405
405 { return __x < __y; }
I don't use wxGetTranslation myself. Is it returning a reference just to prevent copying unnecessarily?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26195)Well, if the order of destruction is wrong, then it would apply to ms_setsMap too. I wonder if making wxPerThreadStrings itself global could be enough to fix this? Globals in the same translation unit should really be initialized/destroyed in (reverse) order of declaration.
Is it returning a reference just to prevent copying unnecessarily?
I think it was for compatibility with some code but by now I can't even think of any reasonable code that would be broken by this (of course, you can always find something, e.g. if the function is overloaded for wxString and const wxString&, then changing its return type would change which overload is being called — but nobody has overloaded functions with different behaviour anyhow (hopefully...)).
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
MaartenBent left a comment (wxWidgets/wxWidgets#26195)Well, if the order of destruction is wrong, then it would apply to ms_setsMap too. I wonder if making wxPerThreadStrings itself global could be enough to fix this? Globals in the same translation unit should really be initialized/destroyed in (reverse) order of declaration.
This doesn't seem to work. If it is just static it works fine, but with thread_local it always segfaults.
In #25928 you already questioned the need for thread_local when a static map with critical section is used. Can we change it to:
/* static */ const wxString& wxTranslations::GetUntranslatedString(const wxString& str) { #if defined(__MINGW32__) static UntranslatedStringHolder wxPerThreadStrings; return wxPerThreadStrings.get(str); #else thread_local UntranslatedStrings wxPerThreadStrings; return *wxPerThreadStrings.insert(str).first; #endif }
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26195)I think we wanted thread_local to free memory whenever a thread exits. Without it, the dtor will only be executed on the application exit, i.e. possibly much later, so this could, theoretically, result in a problem (something looking very much like a memory leak, even if it formally isn't), but it would only happen for a program creating many threads that would each use _() with unique strings and this seems highly unlikely.
So if no other solution can be found, then we could indeed apply the patch above.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
MaartenBent left a comment (wxWidgets/wxWidgets#26195)I'll go with your initial idea of returning wxString from wxGetTranslation() and remove that code entirely.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Arrigo Marchiori
ardovm left a comment (wxWidgets/wxWidgets#26195)Hello and thank you for calling me into this conversation.
I am not sure I get the full picture, but to me, it looks like another wrong ordering of destructors. Destructors of a class should be called before the class data members (static or not) are destroyed, should they not?
If so, I am sorry that my proposed workaround for the old, cross-compiled MingW version I am using introduced problems into another situation.
I agree with @vadz's explanation of the use of thread_local to free memory when threads exit.
I think that wxGetTranslation() is returning wxString & in conformity with the const char *gettext(const char *) function. If we pass it a "static" object, such as a literal string, we may expect it to return something "static" as well, i.e. not destroyed when leaving the current scope (think of its c_str() return value).
@MaartenBent's link to commit mingw-w64/mingw-w64@8e06daa seems to indicate tag 13.0.0. Does that solve this issue? If so, maybe the initially proposed #ifdef guard should rather check for version < 13 rather than < 14?
If we want to keep the current structure, and make it a bit more resistant to "unordered" calls to destructors, we could try making UntranslatedStringHolder::ms_criticalSection and UntranslatedStringHolder::ms_setsMap not crash even after their destructor is called, for example using a helper class with a pointer that is NULL-ified by their destructor... I am just speculating. But we would add layers and layers of complexity...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Arrigo Marchiori
ardovm left a comment (wxWidgets/wxWidgets#26195)Replying to my own comment:
I think that
wxGetTranslation()is returningwxString &in conformity with theconst char *gettext(const char *)function. If we pass it a "static" object, such as a literal string, we may expect it to return something "static" as well, i.e. not destroyed when leaving the current scope (think of itsc_str()return value).
Looking at what other popular toolkits do: Qt have QString QObject::tr(...) and QString QCoreApplication::translate(...) so we have one more reason to feel entitled to return wxString.
You can discard my observation above.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
MaartenBent left a comment (wxWidgets/wxWidgets#26195)I tried your first example from #25928 (comment) with the 'default' std::unordered_set<wxString> implementation. Only a few UntranslatedStringHolder destructors get called and I get a non-zero exit code. So even with the latest MinGW-w64 thread_local does not seem to work correctly.
I noticed tag 13.0 as well, but after checking the version defines in the history of _mingw_mac.h, it was updated to 13.0 in May 2024 whilst this fix is from December 2024. Thats why I suggested 14.0. But with the mentioned test results, I think we can't use this solution.
I changed the return type to wxString in #26196 . This seems like the simplest solution to me.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.