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

[bug] Conan setting CMAKE_FIND_ROOT_PATH_MODE_*=BOTH fundamentally breaks cross-compilation #16324

Open
tttapa opened this issue May 22, 2024 · 4 comments
Assignees

Comments

@tttapa
Copy link

tttapa commented May 22, 2024

Describe the bug

When cross-compiling, it is essential that the build system and the host system are strictly separated. In CMake, this can be ensured by setting CMAKE_FIND_ROOT_PATH_MODE_{LIBRARY,INCLUDE,PACKAGE}=ONLY, which tells the corresponding find_* command to only look for libraries, headers and packages in the host system's sysroot or in the CMAKE_FIND_ROOT_PATH.

However, when tools.build.cross_building:cross_build=true, Conan silently (!) sets these variables to BOTH, and in doing so it allows CMake to look for libraries, headers and packages in the build system's root filesystem as well. As a result, packages cross-compiled using Conan can silently link to wrong versions of libraries it found in /usr/lib, headers of different versions than intended, etc.
In the best case, this results in linker errors because of incompatible architectures, in the worst case, the resulting binaries are subtly broken because they depend on libraries (or versioned symbols in those libraries) that are unavailable on the host system.

If a user explicitly sets CMAKE_FIND_ROOT_PATH_MODE_{LIBRARY,INCLUDE,PACKAGE}=ONLY in their toolchain file, it is often for good reasons, and IMHO silently changing those values inside of Conan's toolchain file is not an acceptable solution.
I can help with a PR, but wanted to discuss here first. In the meantime, I'd suggest adding a very clear warning to Conan's toolchain file if it detects that the user set these variables to ONLY. (It will still be broken, but at least then the people know that it's broken.)

Here are some previous discussions on the same topic:

Addressing some of the comments there:

#10513
I see the problems with using CMAKE_FIND_ROOT_PATH, though - conan doesn't really want to re-root everybody else's path's either.

I don't believe that this is a problem. Re-rooting all paths is exactly what you want when cross-compiling. If a user wants to search the build system's paths as well, they should not set CMAKE_FIND_ROOT_PATH_MODE_*=ONLY in the first place. But that's for the user to decide, Conan should not silently change this.

#9427 (comment)
Here is the problem, let's take CMAKE_FIND_ROOT_PATH_MODE_INCLUDE set to ONLY as an example: find_file() doesn't look into CMAKE_INCLUDE_PATH (or CMAKE_PREFIX_PATH) in this case, it iterates in paths of CMAKE_FIND_ROOT_PATH, but not the paths themselves, only <path>/include.

Exactly, that's what you want when cross-compiling.

What does it mean? It means that you must add the root package folder to CMAKE_FIND_ROOT_PATH. But by doing this you may also make tools (in <path>/bin) of host context discoverable, and they may be found by find_program() before executables of build context.

Not if you set CMAKE_FIND_ROOT_PATH_MODE_PROGRAM=NEVER, which is the common use case when cross-compiling.

If you think a little bit about this, you'll see that you have no other choice than populating CMAKE_FIND_ROOT_PATH with root package folders of requirements AND build requirements.

No, this is not the correct conclusion. They should stay separate, and CMAKE_FIND_ROOT_PATH and CMAKE_FIND_ROOT_PATH_MODE_* are exactly the tools that CMake provides to allow you to keep them separate.

Actually CMAKE_FIND_ROOT_PATH_MODE_PROGRAM set to ONLY even breaks the discovery of Ninja.

Which is why you usually don't see people setting CMAKE_FIND_ROOT_PATH_MODE_PROGRAM=ONLY when cross-compiling :)


How to reproduce it

Tested on Linux, you can find a reproducible version with Docker here:
Script: https://github.com/tttapa/conan-find-mode-bug/blob/main/.github/workflows/test.yml
Output: https://github.com/tttapa/conan-find-mode-bug/actions/runs/9197591352/job/25298402017

conanfile.txt

[generators]
CMakeDeps
CMakeToolchain
[layout]
cmake_layout

toolchain.cmake

# System information
set(CMAKE_SYSTEM_NAME "Linux")
set(CMAKE_SYSTEM_PROCESSOR "x86_64")

# Search path configuration
set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM NEVER)
set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY)
set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY)
set(CMAKE_FIND_ROOT_PATH_MODE_PACKAGE ONLY)

# These are the only places where CMake is allowed to look for libraries etc.
set(CMAKE_FIND_ROOT_PATH ${CMAKE_CURRENT_LIST_DIR}/find_root)
set(CMAKE_SYSROOT ${CMAKE_CURRENT_LIST_DIR}/sysroot)

CMakeLists.txt

cmake_minimum_required(VERSION 3.20)
project(conan_find_mode)

set(CMAKE_FIND_DEBUG_MODE On)
# libreadline.so is in /usr/lib/x86_64-linux-gnu on the build system (not in the sysroot)
find_library(LIBREADLINE "libreadline.so")
if (LIBREADLINE)
    message(SEND_ERROR "Should not have found this library: ${LIBREADLINE}")
endif()

Let's first build this project using only CMake:

cmake -B build-cmake -S . --toolchain toolchain.cmake

This works as expected, libreadline.so is not found, because it does not exist in the (empty) sysroot.
In the output, you'll see that CMake only searched the allowed locations: the host system's sysroot, and the directories explicitly allowed by CMAKE_FIND_ROOT_PATH.

Now using Conan:

conan install . -c tools.cmake.cmaketoolchain:user_toolchain="[\"$PWD/toolchain.cmake\"]" -c tools.build.cross_building:cross_build=true
cmake --preset conan-release

This fails. In the output, you can see that CMake is looking in folders like /usr/lib on the build system. Eventually, it finds the file in /usr/lib/x86_64-linux-gnu/libreadline.so. If you're cross-compiling, this file is not likely to be compatible with your host system (e.g. different architecture, different glibc version, different ABI, different compilation flags, etc.), and the resulting binaries are now broken.

@memsharded memsharded self-assigned this May 22, 2024
@memsharded
Copy link
Member

Hi @tttapa

Thanks for your report.

Can you please clarify what Conan version are you using?

Conan 2 sets only:

        {% if build_paths %}
        # The explicitly defined "builddirs" of "host" context dependencies must be in PREFIX_PATH
        list(PREPEND CMAKE_PREFIX_PATH {{ build_paths }})
        {% endif %}
        {% if cmake_program_path %}
        list(PREPEND CMAKE_PROGRAM_PATH {{ cmake_program_path }})
        {% endif %}

from the build context, should this 2 variables CMAKE_PREFIX_PATH or CMAKE_PROGRAM_PATH make the find_library() find a library in the build context?

@tttapa
Copy link
Author

tttapa commented May 22, 2024

Hi, I'm using version 2.3.1.

At first sight, the code you posted looks fine, the problematic lines are further down:

{% if cross_building %}
if(NOT DEFINED CMAKE_FIND_ROOT_PATH_MODE_PACKAGE OR CMAKE_FIND_ROOT_PATH_MODE_PACKAGE STREQUAL "ONLY")
set(CMAKE_FIND_ROOT_PATH_MODE_PACKAGE "BOTH")
endif()
if(NOT DEFINED CMAKE_FIND_ROOT_PATH_MODE_PROGRAM OR CMAKE_FIND_ROOT_PATH_MODE_PROGRAM STREQUAL "ONLY")
set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM "BOTH")
endif()
if(NOT DEFINED CMAKE_FIND_ROOT_PATH_MODE_LIBRARY OR CMAKE_FIND_ROOT_PATH_MODE_LIBRARY STREQUAL "ONLY")
set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY "BOTH")
endif()
{% if is_apple %}
if(NOT DEFINED CMAKE_FIND_ROOT_PATH_MODE_FRAMEWORK OR CMAKE_FIND_ROOT_PATH_MODE_FRAMEWORK STREQUAL "ONLY")
set(CMAKE_FIND_ROOT_PATH_MODE_FRAMEWORK "BOTH")
endif()
{% endif %}
if(NOT DEFINED CMAKE_FIND_ROOT_PATH_MODE_INCLUDE OR CMAKE_FIND_ROOT_PATH_MODE_INCLUDE STREQUAL "ONLY")
set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE "BOTH")
endif()
{% endif %}

Looking at the git blame, the last change to this bit of code was two years ago in 861170c.

@jcar87
Copy link
Contributor

jcar87 commented May 23, 2024

If I remember correctly, if CMAKE_FIND_ROOT_PATH_MODE_{LIBRARY,INCLUDE,PACKAGE} is set to ONLY, and there is a CMAKE_SYSROOT defined, none of the calls to find_package/find_library/find_program will work to locate anything provided by Conan - the entire search path, incl. CMAKE_PREFIX_PATH and so on, will be re-rooted to the sysroot), and none of the relevant locations will actually be considered.

If a user explicitly sets CMAKE_FIND_ROOT_PATH_MODE_{LIBRARY,INCLUDE,PACKAGE}=ONLY in their toolchain file, it is often for good reasons, and IMHO silently changing those values inside of Conan's toolchain file is not an acceptable solution.

I could agree that if the variable is already defined with an ONLY, we could warn - either honour it and expect CMake to not find anything provided by conan (the user can provide their own toolchain to work that out if they wish), or warn the user to set it to both.

The only case where I could see this working properly is when the only package provided by conan is the sysroot itself, and the sysroot contains everything and no individual libraries are provided as conan packages.

The provided example is fine and valid, but in the situation described, both are an error for find_library(LIBREADLINE "libreadline.so")

  • if it is not in the sysroot, error out early (the desired outcome)
  • if it found in the build machine, it will error out late (and attempt a link against the wrong one)

Obviously I can see how erroring out early is much preferable, as it would point you to arrive at the solution sooner - but once the underlying problem is addressed (libreadline.so is present in the sysroot), it will work correctly even with the current Conan behaviour.

@tttapa
Copy link
Author

tttapa commented May 23, 2024

If I remember correctly, if CMAKE_FIND_ROOT_PATH_MODE_{LIBRARY,INCLUDE,PACKAGE} is set to ONLY, and there is a CMAKE_SYSROOT defined, none of the calls to find_package/find_library/find_program will work to locate anything provided by Conan - the entire search path, incl. CMAKE_PREFIX_PATH and so on, will be re-rooted to the sysroot), and none of the relevant locations will actually be considered.

This is true, but only because Conan does not currently set CMAKE_FIND_ROOT_PATH.

The provided example is fine and valid, but in the situation described, both are an error for find_library(LIBREADLINE "libreadline.so")

  • if it is not in the sysroot, error out early (the desired outcome)
  • if it found in the build machine, it will error out late (and attempt a link against the wrong one)

No, this is not a given. For example, I'm in a situation where I'm cross-compiling with build=x86_64-linux-gnu and host=x86_64-linux-gnu, but they're different systems with different Linux distributions, different toolchains, and different versions of glibc etc. Even though it may happen to link without errors, linking to binaries from the build system will still result in binaries that do not run once deployed on the host system. Or even worse: it may appear to run correctly in most cases, but ABI incompatibilities or ODR violations cause it to crash sporadically or make it exploitable to bad actors.

Second, when cross-compiling, one often prepares a sysroot with specific versions of packages. If for some reason, CMake is unable to find the intended package in the sysroot (e.g. because of a typo in the paths provided by the user), or it is unable to find the Conan-provided package, CMake should should fail, not silently link to a random version of the package it picked up in a /opt/side-projects/thirdparty/libfoo-master folder that happened to be in the build system's PATH.

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

No branches or pull requests

3 participants