Created attachment 36119 [details] proposed patch to GCC Since operator< is not available for thread types when using the pthreads-win32 library, gcc build fails when configured with POSIX threads on MinGW64. The attached patch implements a field-by-field comparison.
Created attachment 36120 [details] complementary patch to the w32-pthreads library
The problem is real, but obviously this patch is not acceptable because you would need to add pthread_less to every implementation of pthreads, and to the POSIX standard. GCC doesn't only support mingw.
Yes, I understand. The purpose of the patch is to point the problem. In particular I don't know why operator< is needed and what are the required semantics (i.e. comparing pointers vs. comparing thread sequence numbers; what is the x field in the struct?). I am also concerned if the current implementation of pthread_equal in the w32-pthreads library is correct in this respect.
The C++ standard requires thread::id objects to be comparable with operator< and for it to impose a total order. The current libstdc++ code assumes that a total order can be obtained simply by comparing the pthread_t objects, which is not portable, but some form of total order over pthread_t objects is needed. For w32-pthreads comparing thread sequence numbers should be sufficient.
Created attachment 36124 [details] Add _GLIBCXX_THREAD_ID_LESS hook for thread ID comparisons This patch preserves the existing behaviour for most targets but adds a new macro hook that can be used to define a target-specific alternative in os_defines.h This does not require any changes to pthreads-win32. Could you test it?
Created attachment 36125 [details] Patch to add _GLIBCXX_THREAD_ID_LESS hook. Oops, that wasn't the final version of the patch. This one should actually compile!
1. After applying the patch to GCC 5.2.0, the build fails (I do not see PTW32_VERSION defined): Making all in c++11 make[5]: Entering directory `/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/src/c++11' /bin/sh ../../libtool --tag CXX --tag disable-shared --mode=compile /home/czarek/src/nowe/build-pthreads/objdir-gcc/./gcc/xgcc -shared-libgcc -B/home/czarek/src/nowe/build-pthreads/objdir-gcc/./gcc -nostdinc++ -L/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/src -L/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/src/.libs -L/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/libsupc++/.libs -L/opt/mingw64/x86_64-w64-mingw32/lib -L/opt/mingw64/mingw/lib -isystem /opt/mingw64/x86_64-w64-mingw32/include -isystem /opt/mingw64/mingw/include -B/opt/mingw64/x86_64-w64-mingw32/bin/ -B/opt/mingw64/x86_64-w64-mingw32/lib/ -isystem /opt/mingw64/x86_64-w64-mingw32/include -isystem /opt/mingw64/x86_64-w64-mingw32/sys-include -I/home/czarek/src/nowe/build-pthreads/gcc-5.2.0/libstdc++-v3/../libgcc -I/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/include/x86_64-w64-mingw32 -I/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/include -I/home/czarek/src/nowe/build-pthreads/gcc-5.2.0/libstdc++-v3/libsupc++ -std=gnu++11 -prefer-pic -D_GLIBCXX_SHARED -fno-implicit-templates -Wall -Wextra -Wwrite-strings -Wcast-qual -Wabi -fdiagnostics-show-location=once -ffunction-sections -fdata-sections -frandom-seed=functexcept.lo -g -O2 -c -o functexcept.lo ../../../../../gcc-5.2.0/libstdc++-v3/src/c++11/functexcept.cc libtool: compile: /home/czarek/src/nowe/build-pthreads/objdir-gcc/./gcc/xgcc -shared-libgcc -B/home/czarek/src/nowe/build-pthreads/objdir-gcc/./gcc -nostdinc++ -L/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/src -L/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/src/.libs -L/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/libsupc++/.libs -L/opt/mingw64/x86_64-w64-mingw32/lib -L/opt/mingw64/mingw/lib -isystem /opt/mingw64/x86_64-w64-mingw32/include -isystem /opt/mingw64/mingw/include -B/opt/mingw64/x86_64-w64-mingw32/bin/ -B/opt/mingw64/x86_64-w64-mingw32/lib/ -isystem /opt/mingw64/x86_64-w64-mingw32/include -isystem /opt/mingw64/x86_64-w64-mingw32/sys-include -I/home/czarek/src/nowe/build-pthreads/gcc-5.2.0/libstdc++-v3/../libgcc -I/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/include/x86_64-w64-mingw32 -I/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/include -I/home/czarek/src/nowe/build-pthreads/gcc-5.2.0/libstdc++-v3/libsupc++ -std=gnu++11 -D_GLIBCXX_SHARED -fno-implicit-templates -Wall -Wextra -Wwrite-strings -Wcast-qual -Wabi -fdiagnostics-show-location=once -ffunction-sections -fdata-sections -frandom-seed=functexcept.lo -g -O2 -c ../../../../../gcc-5.2.0/libstdc++-v3/src/c++11/functexcept.cc -o functexcept.o In file included from /home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/include/future:40:0, from ../../../../../gcc-5.2.0/libstdc++-v3/src/c++11/functexcept.cc:34: /home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/include/thread: In instantiation of ‘static bool std::thread::id::__less<_Tp, <template-parameter-1-2> >::_S_less(const _Tp&, const _Tp&) [with _Tp = ptw32_handle_t; <template-parameter-1-2> = void]’: /home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/include/thread:106:37: required from here /home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/include/thread:91:36: error: ‘_GLIBCXX_THREAD_ID_LESS’ was not declared in this scope { return _GLIBCXX_THREAD_ID_LESS(__x, __y); } ^ make[5]: *** [functexcept.lo] Error 1 make[5]: Leaving directory `/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/src/c++11' make[5]: Entering directory `/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/src' /bin/sh ../libtool --tag CXX --mode=compile /home/czarek/src/nowe/build-pthreads/objdir-gcc/./gcc/xgcc -shared-libgcc -B/home/czarek/src/nowe/build-pthreads/objdir-gcc/./gcc -nostdinc++ -L/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/src -L/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/src/.libs -L/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/libsupc++/.libs -L/opt/mingw64/x86_64-w64-mingw32/lib -L/opt/mingw64/mingw/lib -isystem /opt/mingw64/x86_64-w64-mingw32/include -isystem /opt/mingw64/mingw/include -B/opt/mingw64/x86_64-w64-mingw32/bin/ -B/opt/mingw64/x86_64-w64-mingw32/lib/ -isystem /opt/mingw64/x86_64-w64-mingw32/include -isystem /opt/mingw64/x86_64-w64-mingw32/sys-include -I/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/include/x86_64-w64-mingw32 -I/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/include -I/home/czarek/src/nowe/build-pthreads/gcc-5.2.0/libstdc++-v3/libsupc++ -DDLL_EXPORT -DPIC -fno-implicit-templates -Wall -Wextra -Wwrite-strings -Wcast-qual -Wabi -fdiagnostics-show-location=once -ffunction-sections -fdata-sections -frandom-seed=compatibility-thread-c++0x.lo -g -O2 -std=gnu++11 -c ../../../../gcc-5.2.0/libstdc++-v3/src/c++11/compatibility-thread-c++0x.cc libtool: compile: /home/czarek/src/nowe/build-pthreads/objdir-gcc/./gcc/xgcc -shared-libgcc -B/home/czarek/src/nowe/build-pthreads/objdir-gcc/./gcc -nostdinc++ -L/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/src -L/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/src/.libs -L/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/libsupc++/.libs -L/opt/mingw64/x86_64-w64-mingw32/lib -L/opt/mingw64/mingw/lib -isystem /opt/mingw64/x86_64-w64-mingw32/include -isystem /opt/mingw64/mingw/include -B/opt/mingw64/x86_64-w64-mingw32/bin/ -B/opt/mingw64/x86_64-w64-mingw32/lib/ -isystem /opt/mingw64/x86_64-w64-mingw32/include -isystem /opt/mingw64/x86_64-w64-mingw32/sys-include -I/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/include/x86_64-w64-mingw32 -I/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/include -I/home/czarek/src/nowe/build-pthreads/gcc-5.2.0/libstdc++-v3/libsupc++ -DDLL_EXPORT -DPIC -fno-implicit-templates -Wall -Wextra -Wwrite-strings -Wcast-qual -Wabi -fdiagnostics-show-location=once -ffunction-sections -fdata-sections -frandom-seed=compatibility-thread-c++0x.lo -g -O2 -std=gnu++11 -c ../../../../gcc-5.2.0/libstdc++-v3/src/c++11/compatibility-thread-c++0x.cc -DDLL_EXPORT -DPIC -D_GLIBCXX_SHARED -o .libs/compatibility-thread-c++0x.o In file included from /home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/include/future:40:0, from ../../../../gcc-5.2.0/libstdc++-v3/src/c++11/compatibility-thread-c++0x.cc:30: /home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/include/thread: In instantiation of ‘static bool std::thread::id::__less<_Tp, <template-parameter-1-2> >::_S_less(const _Tp&, const _Tp&) [with _Tp = ptw32_handle_t; <template-parameter-1-2> = void]’: /home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/include/thread:106:37: required from here /home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/include/thread:91:36: error: ‘_GLIBCXX_THREAD_ID_LESS’ was not declared in this scope { return _GLIBCXX_THREAD_ID_LESS(__x, __y); } ^ make[5]: *** [compatibility-thread-c++0x.lo] Error 1 make[5]: Leaving directory `/home/czarek/src/nowe/build-pthreads/objdir-gcc/x86_64-w64-mingw32/libstdc++-v3/src' 2. What is the appropriate test to verify the correctness of the order imposed? 3. My procedure is the following: I build GCC with Win32 threads, use it to build the w32-pthreads library, and only then I build the compiler which uses POSIX threads. Is there a better or simpler way? Building libgcc requires pthreads, w32-pthreads compiled with gcc require libgcc. This looks like a circular dependence.
(In reply to Cezary Śliwa from comment #7) > 1. After applying the patch to GCC 5.2.0, the build fails (I do not see > PTW32_VERSION defined): Doh, of course not, because gthr.h isn't included until after os_defines.h, sorry.
One think I missed is that MinGW64 uses the winpthreads library. If using winpthreads, there is no failure. However, as far as I understand, pthreads-win32 is in use in MinGW.
Author: jason Date: Sat Aug 8 22:01:12 2015 New Revision: 226736 URL: https://gcc.gnu.org/viewcvs?rev=226736&root=gcc&view=rev Log: PR c++/67114 * call.c (joust): Only call more_constrained on decls. Added: trunk/gcc/testsuite/g++.dg/cpp1z/regress1.C Modified: trunk/gcc/cp/ChangeLog trunk/gcc/cp/call.c
(In reply to Jason Merrill from comment #10) Oops, wrong bug number.
Created attachment 38009 [details] pthreads-w32 compatibility patch Patch to allow building gcc-5.3.0 with pthreads-w32, no bugs so far but im still running the testsuite so something might turn up, feel free to test.
Also needs a small change in pthread.h to guard against including windows.h #if defined(PTW32_CONFIG_MINGW) && defined(__cplusplus) change to #if defined(PTW32_CONFIG_MINGW) && defined(PTW32_BUILD) && defined(__cplusplus)
Created attachment 38010 [details] Whopsie Uploaded an older version by mistake, here is the correct one.
--- gcc-5.3.0.old/libstdc++-v3/include/std/thread 2015-03-26 20:59:08.000000000 +0100 +++ gcc-5.3.0.old/libstdc++-v3/include/std/thread 2016-03-17 06:32:41.124378000 +0100 @@ -83,10 +83,19 @@ operator==(thread::id __x, thread::id __y) noexcept { return __gthread_equal(__x._M_thread, __y._M_thread); } + // implement operator< explicitly in terms of the internals of + // pthreads-win32's ptw32_handle_t so that std::thread can use pthreads +#ifdef PTW32_VERSION + friend bool + operator<(thread::id __x, thread::id __y) + { return ( (__x._M_thread.p < __y._M_thread.p) || + ((__x._M_thread.p == __y._M_thread.p) && (__x._M_thread.x < __y._M_thread.x) ) ); } I suggest: return std::tie(__x._M_thread.p, __x._M_thread.x) < std::tie(__y._M_thread.p, __y._M_thread.x); +#else friend bool operator<(thread::id __x, thread::id __y) noexcept - { return __x._M_thread < __y._M_thread; } - + { return __less<native_handle_type>::_S_less(__x._M_thread, __y._M_thread); } +#endif + What's this?
(In reply to Jonathan Wakely from comment #15) > friend bool > operator<(thread::id __x, thread::id __y) noexcept > - { return __x._M_thread < __y._M_thread; } > - > + { return __less<native_handle_type>::_S_less(__x._M_thread, > __y._M_thread); } > +#endif > + > > What's this? Oh, from the earlier patch attached here, which I'd forgotten about! Sorry :)
The point of my patch in comment 6 was to allow the customization to be done in os_defines.h, not to litter the <thread> header. If you're going to add #ifdef checks for PTW32_VERSION in <thread> then there's no point using my patch in comment 6.
So I suggest something like: friend bool operator<(thread::id __x, thread::id __y) noexcept - { return __x._M_thread < __y._M_thread; } + { +#ifdef PTW32_VERSION + // implement operator< explicitly in terms of the internals of + // pthreads-win32's ptw32_handle_t. + return std::tie(__x._M_thread.p, __x._M_thread.x) + < std::tie(__y._M_thread.p, __y._M_thread.x); +#else + // Pthreads doesn't define any way to do this, so we just have to + // assume native_handle_type is LessThanComparable. + return __x._M_thread < __y._M_thread; +#endif + } On trunk operator== is different to the gcc-5-branch, so would require a similar change.
Had problems with the define not being pulled in from os_defines.h (i think a better place would have been gthr_posix.h since that gets pulled in at build time) hence the change, also if using another posix thread implementation it would break using that instead since your define would then be undefined leading to a build error. Besides that better implementations are welcome :) this was just an example of a possible fix.
Clarification. If not using pthreads-w32 #ifdef PTW32_VERSION // See libstdc++/67114 # define _GLIBCXX_THREAD_ID_LESS(X, Y) ((X).x < (Y).x) <- this #endif would end up undefined if say we build with winpthreads. As you noticed yourself os_defines.h gets pulled in after gthr.h so it fails, my suggestion if using a macro would be to put it in gthr_posix.h instead. My reason for this suggestion is that gthr_posix.h gets linked to gthr.h when building with posix threads and is visible to all parts of the code. My patch was made for the mingw.org compiler, since they still use pthreads-w32. Hope this clears it up, i was a little tired at the time of writing my last reply.
(In reply to ralphengels@gmail.com from comment #13) > Also needs a small change in pthread.h to guard against including windows.h > > #if defined(PTW32_CONFIG_MINGW) && defined(__cplusplus) > > change to > > #if defined(PTW32_CONFIG_MINGW) && defined(PTW32_BUILD) && > defined(__cplusplus) Agreed, pthread.h from pthreads-win32 needs to change, (but that's hardly an issue of particular interest here). Personally, I would patch it much more aggressively: it should NEVER include windows.h, but it also incorporates a load of additional cruft which simply doesn't belong there. However, this isn't the place to discuss such issues; let's take them up on a MinGW.org, or a pthreads-win32, focussed mailing list.
Aye the pthreads specific fixes needs to be taken care of upstream. The comment was just to let people know that the gcc changes are not enough. Ill try and get a hold on the current maintainer for pthreads-w32, the repository is sadly read only so i cannot submit patches via git.