niXman / mingw-builds

Scripts for building the 32 and 64-bit MinGW-W64 compilers for Windows
Other
282 stars 107 forks source link

Add support for mcf thread model #659

Closed starg2 closed 11 months ago

starg2 commented 11 months ago

WIP. I think it's easier for us to discuss the issues when we have actual code.

Closes #658.

lhmouse commented 11 months ago

WIP. I think it's easier for us to discuss the issues when we have actual code.

  • [ ] I couldn't build 9004-crt-Copy-clock-and-nanosleep-from-winpthreads.patch so I commented it out.

What's the error?

While building libstdc++, its configure script checks for existence of nanosleep() and #define _GLIBCXX_USE_NANOSLEEP 1 in 'c++config.h', which is almost always the case.

However nanosleep() is implemented in winpthreads. This means that if a program calls std::this_thread::sleep_for() it will have to link against winpthreads. This patch provides nanosleep() in the main CRT library to get rid of such dependency.

  • [ ] The CRT patches only apply to trunk and not v11.

There may be conflicts. I think we can apply them on trunk, then cherry-pick them to the v11 branch, then re-format-patch them.

  • [ ] How do we bootstrap the first toolchain?

If we link GCC against the unpatched CRT, then no special action is necessary. GCC is single-threaded, so it doesn't matter what CRT itself is linked against; only GCC libraries (libgcc, libstdc++, libobjc, libgfortran, etc.) matter.

If we want to link GCC against the patched CRT which references __MCF_cxa_atexit() and __MCF_cxa_finalize(), then a conventional procedure for cross compilation shall apply:

  1. Build a GCC that links against the 'host' (unpatched) CRT and produces code linking against the 'target' (patched) CRT.
  2. Use this cross GCC to build the patched CRT.
  3. Install the patched CRT, so the 'host' is changed to patched CRT.
  4. Rebuild GCC, linking against the 'host' (patched ) CRT and produces code linking against the 'target' (patched) CRT. This is now a normal three-stage bootstrap.
  • [ ] Update README.

We may take the comments from https://www.mingw-w64.org/downloads/#gcc-with-the-mcf-thread-model but it's a bit out of date. I shall update it a little a few moments later.

starg2 commented 11 months ago

WIP. I think it's easier for us to discuss the issues when we have actual code.

  • [ ] I couldn't build 9004-crt-Copy-clock-and-nanosleep-from-winpthreads.patch so I commented it out.

What's the error?

I got a bunch of undeclared identifier errors like this:

../../src/mingw-w64/mingw-w64-crt/time/clock.c: In function 'clock_getres':
../../src/mingw-w64/mingw-w64-crt/time/clock.c:81:15: error: 'CLOCK_REALTIME' undeclared (first use in this function); did you mean 'NIF_REALTIME'?
   81 |     if (id == CLOCK_REALTIME && load_GetSystemTimeBestAsFileTime() == GetSystemTimeAsFileTime)
      |               ^~~~~~~~~~~~~~
      |               NIF_REALTIME

CLOCK_REALTIME is usually defined in pthread_time.h, but the one I found under $ROOT_DIR/runtime was dummy:

/* Dummy header, which gets overridden, if winpthread library gets installed.  */

I have no idea why a dummy header is installed.

While building libstdc++, its configure script checks for existence of nanosleep() and #define _GLIBCXX_USE_NANOSLEEP 1 in 'c++config.h', which is almost always the case.

However nanosleep() is implemented in winpthreads. This means that if a program calls std::this_thread::sleep_for() it will have to link against winpthreads. This patch provides nanosleep() in the main CRT library to get rid of such dependency.

Sounds like this patch is useful for the win32 thread model as well.

  • [ ] The CRT patches only apply to trunk and not v11.

There may be conflicts. I think we can apply them on trunk, then cherry-pick them to the v11 branch, then re-format-patch them.

I think we at least want to support the latest release branch (v11), so let's do that before merging this PR.

  • [ ] How do we bootstrap the first toolchain?

If we link GCC against the unpatched CRT, then no special action is necessary. GCC is single-threaded, so it doesn't matter what CRT itself is linked against; only GCC libraries (libgcc, libstdc++, libobjc, libgfortran, etc.) matter.

What about the prerequisites like zlib, libiconv, gmp, mpfr, etc? They are built before CRT.

lhmouse commented 11 months ago

I have no idea why a dummy header is installed.

Ah I see. The patch only move the implementation of nanosleep() into the main CRT, but the code still depends winpthreads headers. I have remembered why 'pthread_time.h' wasn't included in that patch: There are still libraries that require winpthreads (such as libgomp) so this header is still installed with winpthreads and not the main CRT.

Therefore the patch should be considered incomplete, as 'pthread_time.h' from winpthreads should be installed by hand before building the CRT.

Sounds like this patch is useful for the win32 thread model as well.

I suggest you check 'c++config.h' in the win32 thread model for sure. I have checked my Debian installation and it is not defined in '/usr/lib/gcc/x86_64-w64-mingw32/8.3-win32/include/c++/x86_64-w64-mingw32/bits/c++config.h'. It's either that they pass --disable-libstdcxx-time when configuring libstdc++, or they do not install winpthreads.

What about the prerequisites like zlib, libiconv, gmp, mpfr, etc? They are built before CRT.

Those can be rebuilt with the new GCC. however I don't rebuild them.

lhmouse commented 11 months ago

I'm considering to try --disable-libstdcxx-time and drop that nanosleep() patch. Not only because the patch is incomplete, it also looks like a hack and non-solution.

starg2 commented 11 months ago

CI finally ran successfully, so I checked c++config.h:

Thread model _GLIBCXX_USE_NANOSLEEP defined?
posix yes
win32 no
mcf no

We always pass --enable-libstdcxx-time=yes to configure though.

lhmouse commented 11 months ago

The result looks correct. Was it the case that winpthreads was not installed before building GCC? Failure to reference nanosleep() is likely to cause _GLIBCXX_USE_NANOSLEEP to be not defined.

Anyway I have filed a PR to MSYS2: https://github.com/msys2/MINGW-packages/pull/18667

starg2 commented 11 months ago

We always install winpthreads before building gcc. Also, regardless of the target thread model, we use gcc with the posix thread model as host toolchains by default.

lhmouse commented 11 months ago

I see. Here is a test program: test.cc.txt

The C++ standard requires that all thread_local objects be destroyed before all static objects. With the posix or win32 thread model, this program outputs:

static_obj_type constructor: `this` = 00007ff633e6e030
thread_local constructor: `this` = 000002ab619d1d68, `00007ff633e6e030` is alive
main
static_obj_type destructor: `this` = 00007ff633e6e030
thread_local destructor: `this` = 000002ab619d1d68, `00007ff633e6e030` is dead

which violates the standard (the destructor of tls references the destroyed st).

With the mcf thread model and the patched CRT this outputs:

static_obj_type constructor: `this` = 00007ff70c0dd030
thread_local constructor: `this` = 0000023149ff1518, `00007ff70c0dd030` is alive
main
thread_local destructor: `this` = 0000023149ff1518, `00007ff70c0dd030` is alive
static_obj_type destructor: `this` = 00007ff70c0dd030
lhmouse commented 11 months ago

Found the original issue for the nanosleep() patch: https://github.com/lhmouse/MINGW-packages/issues/11#issuecomment-1517917078 Just checked, this test build of winlibs also didn't define _GLIBCXX_USE_NANOSLEEP.

This report says 'several projects fail to build because clock_gettime seems to be missing.' GCC with the posix thread model and a mingw target seems to add -lpthread as a default library, which isn't the case on Linux (this is the outout of the MSYS2 gcc):

$ gcc test.c -v 2>&1 |  grep --color pthread
(... ...) -lmingw32 -lgcc -lgcc_eh -lmoldname -lmingwex -lmsvcrt -lkernel32 -lpthread -ladvapi32 -lshell32 -luser32 -lkernel32 -lmingw32 -lgcc -lgcc_eh -lmoldname -lmingwex -lmsvcrt -lkernel32 (... ...)
                                                                            ^^^^^^^^^

With winpthreads, clock_gettime() and nanosleep() require -lpthread when linking, but on Linux they don't (they do not require -lrt any more). The patch was created to match the Linux behavior.

starg2 commented 11 months ago

I just tried building with sjlj exception handling and gcc fails to build during stage 2:

echo timestamp > stmp-int-hdrs
/home/user/ucrt-sjlj-13/x86_64-1320-mcf-sjlj-ucrt-rt_v12/build/gcc-13.2.0/./gcc/xgcc -B/home/user/ucrt-sjlj-13/x86_64-1320-mcf-sjlj-ucrt-rt_v12/build/gcc-13.2.0/./gcc/ -fno-checking -xc -nostdinc nul -S -o nul -fself-test=../../../../src/gcc-13.2.0/gcc/testsuite/selftests
/home/user/ucrt-sjlj-13/x86_64-1320-mcf-sjlj-ucrt-rt_v12/build/gcc-13.2.0/./gcc/xgcc -B/home/user/ucrt-sjlj-13/x86_64-1320-mcf-sjlj-ucrt-rt_v12/build/gcc-13.2.0/./gcc/ -fno-checking -xc++ -nostdinc nul -S -o nul -fself-test=../../../../src/gcc-13.2.0/gcc/testsuite/selftests
make[3]: *** [../../../../src/gcc-13.2.0/gcc/c/Make-lang.in:128: s-selftest-c] Error 1
make[3]: *** Waiting for unfinished jobs....
make[3]: *** [../../../../src/gcc-13.2.0/gcc/cp/Make-lang.in:230: s-selftest-c++] Error 1
rm gcc.pod
make[3]: Leaving directory '/home/user/ucrt-sjlj-13/x86_64-1320-mcf-sjlj-ucrt-rt_v12/build/gcc-13.2.0/gcc'
make[2]: *** [Makefile:5022: all-stage2-gcc] Error 2
make[2]: Leaving directory '/home/user/ucrt-sjlj-13/x86_64-1320-mcf-sjlj-ucrt-rt_v12/build/gcc-13.2.0'
make[1]: *** [Makefile:23922: stage2-bubble] Error 2
make[1]: Leaving directory '/home/user/ucrt-sjlj-13/x86_64-1320-mcf-sjlj-ucrt-rt_v12/build/gcc-13.2.0'
make: *** [Makefile:1091: all] Error 2

@lhmouse Are mcf and sjlj incompatible? If so, I'll add a test in the build script to reject that configuration.

lhmouse commented 11 months ago

@lhmouse Are mcf and sjlj incompatible? If so, I'll add a test in the build script to reject that configuration.

In theory, no. I used to build GCC with SJLJ exception model many years ago and there was no build issue.

starg2 commented 11 months ago

I see. Since sjlj is rarely used these days, I'll leave it as is until someone complains.

niXman commented 11 months ago

merged.

thank you guys!