[REACTOS] Add support for compiling with GCC 13 by tkreuzer · Pull Request #6824 · reactos/reactos

tkreuzer · 2024-05-01T09:16:58Z

Purpose

This PR makes it possible to compile ros with GCC 13. It will still create warnings, so for x86 it requires to remove -Werror

Proposed changes

Add some compiler flags for GCC 13
Add imported library winpthread
Add support library for GCC's libsupc++ and libstdc++

HBelusca · 2024-05-01T09:36:28Z

sdk/cmake/gcc.cmake

 set_target_properties(libgcc PROPERTIES IMPORTED_LOCATION ${LIBGCC_LOCATION})
 # libgcc needs kernel32 imports, a CRT and msvcrtex
-target_link_libraries(libgcc INTERFACE libkernel32 libmsvcrt msvcrtex)
+target_link_libraries(libgcc INTERFACE libwinpthread libkernel32 libmsvcrt msvcrtex)


geez GCC... why the hell do they need winpthread?

For threading support in their runtime library I guess. The better alternative on Windows would be MCF Gthread, but that requires Windows 7.

I do hope all this gcc nastiness is static, and our binaries do not actually depend on some shit like winpthread.dll or so?

That's what I was wondering too.

Luckily, libwinpthread and libgcc_dw2 / libgcc_seh2 can be statically linked to the produced executable (it's actually a compiler option of the configure step of gcc buildscript that ends up generating libwinpthread.dll.a or libwinpthread.a).

sdk/lib/gcc-compat/__throw_out_of_range_fmt.cpp

sdk/lib/gcc-compat/StringCchCopyNA.c

HBelusca

SergeGautherie · 2024-05-03T16:07:42Z

sdk/cmake/gcc.cmake

+        add_compile_options(-fno-builtin-sin)
+        add_compile_options(-fno-builtin-cos)
+        add_compile_options(-fno-builtin-sincos)
+        add_compile_options(-fno-builtin-pow)
+        add_compile_options(-fno-builtin-sqrt)
+        add_compile_options(-fno-builtin-sqrtf)
+        add_compile_options(-fno-builtin-floor)
+        add_compile_options(-fno-builtin-floorf)
+        add_compile_options(-fno-builtin-ceilf)
+        add_compile_options(-fno-builtin-ceil)


Sort them. Want to merge them into "1" line?

cbialorucki

Just fix the copyright dates

wjk · 2024-05-18T00:25:26Z

If I understand correctly, this new library is required for any code compiled with GCC to run. winpthread dependency aside, does this not mean that everything now depends on kernel32.dll and msvcrt.dll, including kernel components and DLLs lower than kernel32 and msvcrt? Won’t this cause problems?

SergeGautherie · 2024-05-18T15:49:43Z

sdk/cmake/gcc.cmake

 set_target_properties(libsupc++ PROPERTIES IMPORTED_LOCATION ${LIBSUPCXX_LOCATION})
 # libsupc++ requires libgcc
-target_link_libraries(libsupc++ INTERFACE libgcc)
+target_link_libraries(libsupc++ INTERFACE libgcc stdc++compat)


Update comment too?

SergeGautherie · 2024-05-18T15:50:29Z

sdk/cmake/gcc.cmake

 # libgcc needs kernel32 imports, a CRT and msvcrtex
-target_link_libraries(libgcc INTERFACE libkernel32 libmsvcrt msvcrtex)
+target_link_libraries(libgcc INTERFACE libwinpthread libkernel32 libmsvcrt msvcrtex)


Update comment too?

HBelusca · 2024-05-24T13:46:58Z

I wonder whether it is possible to add a temporary github action that makes use of GCC13, to see how things go.

oleg-dubinskiy

Some nit-picks, LGTM otherwise.

sdk/lib/gcc-compat/StringCchCopyNA.c

sdk/lib/gcc-compat/__mingw_vsprintf.c

sdk/lib/gcc-compat/__throw_out_of_range_fmt.cpp

oleg-dubinskiy · 2024-05-24T16:19:40Z

sdk/cmake/gcc.cmake

 execute_process(COMMAND ${GXX_EXECUTABLE} -print-file-name=libsupc++.a OUTPUT_VARIABLE LIBSUPCXX_LOCATION)
 string(STRIP ${LIBSUPCXX_LOCATION} LIBSUPCXX_LOCATION)
 set_target_properties(libsupc++ PROPERTIES IMPORTED_LOCATION ${LIBSUPCXX_LOCATION})
 # libsupc++ requires libgcc


And update this comment as follows:
# libsupc++ requires libgcc and stdc++compat

oleg-dubinskiy · 2024-05-24T16:21:23Z

sdk/cmake/gcc.cmake

 execute_process(COMMAND ${GXX_EXECUTABLE} -print-file-name=libgcc.a OUTPUT_VARIABLE LIBGCC_LOCATION)
 string(STRIP ${LIBGCC_LOCATION} LIBGCC_LOCATION)
 set_target_properties(libgcc PROPERTIES IMPORTED_LOCATION ${LIBGCC_LOCATION})
 # libgcc needs kernel32 imports, a CRT and msvcrtex


... and this one as follows:
# libgcc needs winpthread, kernel32, a CRT and msvcrtex imports

sdk/cmake/gcc.cmake

__throw_out_of_range_fmt is implemented in libstdc++, but it is needed by libsupc++ and that would mean we would have to link all C++ code to libstdc++.

SergeGautherie · 2024-06-16T13:01:15Z

sdk/cmake/gcc.cmake

+        add_compile_options(-fno-builtin-sqrt)
+        add_compile_options(-fno-builtin-sqrtf)
+    endif()
+    if(CMAKE_C_COMPILER_VERSION VERSION_GREATER_EQUAL 13)


Shouldn't the other changes (winpthread, gcc-compat, ...) too depend on GCC version? As in unneeded for RosBE's GCC 8.4.0.

tkreuzer self-assigned this May 1, 2024

tkreuzer requested review from DarkFire01, GeoB99, HBelusca, binarymaster, cbialorucki, gonzoMD, hpoussin, katahiromz, learn-more, oleg-dubinskiy and whindsaks May 1, 2024 09:17

tkreuzer added the GCC-13 Fixes to get ROS build with GCC 13 label May 1, 2024

tkreuzer force-pushed the RosBE/GCC13-build branch from 561b950 to 1e0829c Compare May 1, 2024 09:20

HBelusca reviewed May 1, 2024

View reviewed changes

HBelusca self-requested a review May 1, 2024 09:39

binarymaster added the enhancement For PRs with an enhancement/new feature. label May 1, 2024

ghost mentioned this pull request May 1, 2024

[CONTENT] Newsletter 104 - April/May news reactos/web-content#126

Merged

learn-more approved these changes May 2, 2024

View reviewed changes

HBelusca approved these changes May 3, 2024

View reviewed changes

SergeGautherie reviewed May 3, 2024

View reviewed changes

cbialorucki approved these changes May 3, 2024

View reviewed changes

SergeGautherie reviewed May 18, 2024

View reviewed changes

DarkFire01 approved these changes May 23, 2024

View reviewed changes

oleg-dubinskiy approved these changes May 24, 2024

View reviewed changes

tkreuzer added 3 commits June 15, 2024 14:08

[CMAKE] Some compiler flags for GCC 13

c9df35e

[CMAKE] Add imported library winpthread

5b00713

[GCC-COMPAT] Add support library for GCC's libsupc++ and libstdc++

91cf63c

__throw_out_of_range_fmt is implemented in libstdc++, but it is needed by libsupc++ and that would mean we would have to link all C++ code to libstdc++.

tkreuzer force-pushed the RosBE/GCC13-build branch from 1e0829c to 91cf63c Compare June 15, 2024 11:12

SergeGautherie reviewed Jun 16, 2024

View reviewed changes

learn-more approved these changes Jun 17, 2024

View reviewed changes

tkreuzer merged commit fcbccaa into reactos:master Jul 9, 2024

tkreuzer deleted the RosBE/GCC13-build branch July 13, 2024 11:53

Uh oh!

Conversation

tkreuzer commented May 1, 2024

Purpose

Proposed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

HBelusca left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cbialorucki left a comment

Choose a reason for hiding this comment

Uh oh!

wjk commented May 18, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HBelusca commented May 24, 2024

Uh oh!

oleg-dubinskiy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants