Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Link with "msvcr[n]" available on the system for MINGW compilation #307

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

OanaStanoi
Copy link

Replace the hardcoded dependency on "msvcr90" with the available library on the system (msvcr140, msvcr120, msvcr110, msvcr100, msvcr90) when compiling with MINGW

@@ -216,7 +216,7 @@ if(WIN32)
set(CMAKE_EXE_LINKER_FLAGS "-Wl,--pdb= ")
add_compile_options(-gcodeview)
else()
set(MINGW_MSVCR_LIBRARY "msvcr90$<$<CONFIG:DEBUG>:d>" CACHE STRING "Mingw MSVC runtime import library")
find_library(MINGW_MSVCR_LIBRARY ucrtbase msvcr140 msvcr120 msvcr110 msvcr100 msvcr90 REQUIRED DOC "Mingw MSVC runtime import library")
Copy link
Contributor

@madebr madebr Sep 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of using find_library to find system libraries.
It requires a non-minimal cmake toolchain file that includes the paths of the system libraries.

The following fails:

CC=x86_64-w64-mingw32-gcc CXX=x86_64-w64-mingw32-g++ cmake -S . -B build -GNinja -DCMAKE_SYSTEM_NAME=Windows
(snip)
CMake Error at BackwardConfig.cmake:219 (find_library):
  Could not find MINGW_MSVCR_LIBRARY using the following names: ucrtbase,
  msvcr140, msvcr120, msvcr110, msvcr100, msvcr90
Call Stack (most recent call first):
  CMakeLists.txt:32 (include)

When I add the library paths of the mingw64 system libraries to CMAKE_FIND_ROOT_PATH, configuration succeeds.

Also, this does not fix __imp__set_abort_behavior (reported by me in #291 (comment)).
I think it's better to fix the missing symbol error first before addressing the choice of c runtime library.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand you're cross-compiling with mingw so it is normal that cmake is not able to resolve the libraries. It is expected that you supply a toolchain file.

As reported in #291 the current master does not have any missing symbol.

Instead of find_library what would you recommend?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't use find_library because 1. this depends on a completely correctly configured cmake toolchain and 2. we don't really care about the location of msvcr90. We only care about whether the toolchain links correctly, providing _set_abort_behavior.

The following cmake script does that:

set(CMAKE_SYSTEM_NAME Windows)
set(CMAKE_C_COMPILER x86_64-w64-mingw32-gcc)

cmake_minimum_required(VERSION 3.20)
project(ct C)
include(CheckCSourceCompiles)
include(CMakePushCheckState)

set(src_set_abort_behavior "#include <stdlib.h>\nint main() { _set_abort_behavior(0, 0); return 0;}\n")

cmake_push_check_state(RESET)
check_c_source_compiles("${src_set_abort_behavior}" BUILTIN_SET_ABORT_BEHAVIOR)
if(NOT BUILTIN_SET_ABORT_BEHAVIOR)
  set(CMAKE_REQUIRED_LIBRARIES msvcr90)
  check_c_source_compiles("${src_set_abort_behavior}" MSVCR90_SET_ABORT_BEHAVIOR)
  # target_link_libraries(... PRIVATE msvcr90)
endif()
cmake_pop_check_state()

message("BUILTIN_SET_ABORT_BEHAVIOR:  ${BUILTIN_SET_ABORT_BEHAVIOR}")
message("MSVCR90_SET_ABORT_BEHAVIOR: ${MSVCR90_SET_ABORT_BEHAVIOR}")

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is true we are not interested in the msvcr90 location, we're interested finding out what C library is present on the system instead of fixing msvcr90 (visual studio 2008, uncommon nowadays).

Would you care to explain somehow why find_library and check_c_source_compiles are different in this case? I understand that both require compiler and linker.

Copy link
Contributor

@madebr madebr Jan 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

find_library is a cmake function that will look for a msvcr90.dll.a/msvcr90.a file (on msvc) or libmsvcr90.dll.a/libmsvcr90.a (with gcc). So it's important for CMake to know it what locations to search: this needs to be configured by the cmake toolchain.
With check_c_source_compiles, cmake just says to the compiler: try to compile+link this file and link msvcr90.lib/-lmsvcr90. The compiler itself will look in its preconfigured locations (cmake does not need to know about them).

oana.stanoi and others added 2 commits January 26, 2024 15:38
Use check_cxx_source_compiles() to find the most recent linkable ucrtbase/msvcr.
Linking of ucrtbase/msvcr is enabled regardless of the -gcodeview support, as the two concepts do not overlap.
@cowo78
Copy link

cowo78 commented Jan 26, 2024

Applied @madebr suggestions. Currently with MINGW gcc 12.2/msys2 there are no unresolved symbols (in shared mode). Tested also in cross-compilation environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants