investigate the problem on MinGW #217
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Currently, I cannot fix the problem on my own. I have traced into the deallocation path. During the process, |
|
Do you have a debug stack trace of when the flag is updated? The easiest way to debug this kind of crash is with time travel debugging with "WinDBG Preview", when you get the If you can post the stack traces, I can probably work it out what might be happening. |
|
stack traces of |
|
more detailed version: |
|
But Also, are the stack traces being printed interleaved? The following looks strange. |
|
already checked let me check whether the output is not interrupting each other. |
|
Here is the result generated by suspending the program on each hit: |
|
It is not calling static thread_local OnDestruct<ThreadAllocCommon::inner_release> tidier;This works on other implementations of the CRT. I guess it could be a mingw bug? The only call to Has this been failing long? Or has it just started? Can you bisect when it started? |
|
test failed since 74657d9, which is really bad because it is the point to add the test. and debug failed to compile since |
|
so, according to the current result, it seems that MinGW GCC support is never correct since added? |
|
I get no output with : #include <iostream>
#include <mutex>
#include <thread>
#include <vector>
static std::mutex lock;
struct DtorTest {
~DtorTest(){
lock.lock();
std::cout << "dtor called " << std::this_thread::get_id() << std::endl;
lock.unlock();
}
};
void _register() {
static thread_local DtorTest test {};
}
const size_t TEST = 12;
int main() {
std::vector<std::thread> threads;
for (size_t i = 0; i < TEST; ++i) {
threads.emplace_back(_register);
}
for (auto& i : threads) {
i.join();
}
} |
|
I don't really want to support MinGW as I don't see much demand, and this makes me even less keen. It will lead to memory leaks on MinGW. Your mini examples works on my machine in WSL with g++/clang++ and Visual studio cl.exe. |
I assume this is with MinGW? |
yes. MinGW's compilers. |
|
I am reporting this bug. |
|
Three year old issues. I am not particularly hopeful it will be fixed. |
|
@SchrodingerZhu I believe it's one of many MinGW libgcc issues. |
|
Hello, I am the guy who reported the issue in mingw. |
|
I got some time tracing the TLS destruction procedure. It seems that those FYI, |
|
I'm working on fixing MSYS2 libc++. In the meantime you can try using this MinGW toolchain: https://github.com/mstorsjo/llvm-mingw |
|
@mjp41 I think @mati865 shows that with @mati865 I have tried the mcfgthread library by @lhmouse. I think it is also not working here (but maybe I was not using it correctly)? As what I have said in #217 (comment), the problem seems to occur at the crt library (https://github.com/mirror/mingw-w64/blob/944854bca875739814cc466736f8cda5fc9b7f7d/mingw-w64-crt/crt/tls_atexit.c#L38), and this explains why llvm |
|
I think further discussions are not closely related to |
|
@SchrodingerZhu with llvm-mingw toolchain: There is no error.
Sure, thought this info would be useful. |
CMakeLists.txt
Outdated
| @@ -169,7 +169,7 @@ if(NOT DEFINED SNMALLOC_ONLY_HEADER_LIBRARY) | |||
| set(CMAKE_EXE_LINKER_FLAGS_RELEASE "${CMAKE_EXE_LINKER_FLAGS_RELEASE} /DEBUG") | |||
| else() | |||
| add_compile_options(-fno-exceptions -fno-rtti -g -ftls-model=initial-exec -fomit-frame-pointer) | |||
| if(SNMALLOC_CI_BUILD OR (${CMAKE_BUILD_TYPE} MATCHES "Debug")) | |||
| if((SNMALLOC_CI_BUILD OR (${CMAKE_BUILD_TYPE} MATCHES "Debug")) AND NOT MINGW) | |||
There was a problem hiding this comment.
Some of the options in the line above (e.g. the TLS model) also don't look like they make sense on Windows. It looks as if we're conflating Windows and MSVC here a bit and should probably disentangle that a bit.
There was a problem hiding this comment.
check_cxx_compiler_flag(-rdynamic SNMALLOC_RDYNAMIC)
check_cxx_compiler_flag(-ftls-model=initial-exec SNMALLOC_STATIC_TLS)is this a better solution?
src/ds/bits.h
Outdated
| @@ -76,7 +76,7 @@ namespace snmalloc | |||
|
|
|||
| return BITS - index - 1; | |||
| # endif | |||
| #elif defined(USE_CLZLL) | |||
| #elif defined(USE_BIT_LL) | |||
There was a problem hiding this comment.
It would be nice to use something like this here:
if constexpr (std::is_same_v<unsigned long, std::size_t>)
{
return __builtin_clzl(x);
}
else if constexpr (std::is_same_v<unsigned long long, std::size_t>)
{
return __builtin_clzll(x);
}
src/ds/bits.h
Outdated
| @@ -142,7 +142,9 @@ namespace snmalloc | |||
|
|
|||
| inline size_t ctz(size_t x) | |||
| { | |||
| #if __has_builtin(__builtin_ctzl) | |||
| #if defined(USE_BIT_LL) | |||
|
The |
|
MinGW-w64 seems to have no effective respond to this problem. As other crt ( |
|
Well |
isn't it using universal runtime on default?
|
|
@SchrodingerZhu any toolchain can use any CRT library (it's mingw-w64 configuration option) but it does not change anything with regard to TLS bug. |
Is there a combination we should recommend for building |
|
@mjp41 I think the safest bet would be saying something like "our testing revealed issues with different mingw-w64 based toolchains, currently https://github.com/mstorsjo/llvm-mingw is the only toolchain known to work properly". |
SchrodingerZhu
force-pushed
the
master
branch
from
92866a3
to
ecfcc31
Compare
SchrodingerZhu
force-pushed
the
master
branch
from
ecfcc31
to
8359523
Compare
|
@mati865 would you help test whether the current PR is working (passing all the tests) under |
|
@SchrodingerZhu I had to make this change to avoid error on MinGW: diff --git a/src/test/func/malloc/malloc.cc b/src/test/func/malloc/malloc.cc
index 1d54525..9fe591a 100644
--- a/src/test/func/malloc/malloc.cc
+++ b/src/test/func/malloc/malloc.cc
@@ -4,7 +4,7 @@
#define SNMALLOC_NAME_MANGLE(a) our_##a
#include "../../../override/malloc.cc"
-#if defined(_WIN32) && !defined(_MSC_VER)
+#if defined(_WIN32) && defined(_MSC_VER)
# define ST_FMT "I"
#else
# define ST_FMT "z"After that with GCC based MinGW those tests fail: With llvm-mingw everything passes. |
|
FWIW MSYS2 now ships more environments: https://www.msys2.org/docs/environments/ |
|
@SchrodingerZhu the new setting here: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In debug mode, I see some other errors:
ctzfailed to return the correct result.mingw gcchas no-rdynamicfunc-first-operationfailed, showingERROR: allocator in use