-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Zephyr toolchain / linking improvements #78320
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
Changes from all commits
6817339
aadb9d1
d07d52e
669d625
0cadd94
361b553
b21510c
3b8e621
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2233,3 +2233,5 @@ add_subdirectory_ifdef( | |
CONFIG_MAKEFILE_EXPORTS | ||
cmake/makefile_exports | ||
) | ||
|
||
toolchain_linker_finalize() |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -139,3 +139,7 @@ set_compiler_property(PROPERTY warning_shadow_variables) | |
# Compiler flags to avoid recognizing built-in functions | ||
set_compiler_property(PROPERTY no_builtin) | ||
set_compiler_property(PROPERTY no_builtin_malloc) | ||
|
||
# Compiler flag for defining specs. Used only by gcc, other compilers may keep | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we eventually plan on supporting clang's There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking of that, and some clang/LLVM releases, like LLVM for Arm, does provide dedicated configs for this, but i'm unsure how well established / standardized this principle is, so for a start it's kept a bit independent in |
||
# this undefined. | ||
set_compiler_property(PROPERTY specs) |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
# Copyright (c) 2024 Nordic Semiconductor | ||
# | ||
# SPDX-License-Identifier: Apache-2.0 | ||
|
||
# Per default armclang (Arm Compiler 6) doesn't need explicit C library linking | ||
# so we only need to set link order linking in case a custom C library is linked | ||
# in, such as picolibc. | ||
set_property(TARGET linker APPEND PROPERTY link_order_library "c;rt") |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -108,5 +108,23 @@ function(toolchain_ld_link_elf) | |
) | ||
endfunction(toolchain_ld_link_elf) | ||
|
||
# This function will generate the correct CMAKE_C_LINK_EXECUTABLE / CMAKE_CXX_LINK_EXECUTABLE | ||
# rule to ensure that standard c and runtime libraries are correctly placed | ||
# and the end of link invocation and doesn't appear in the middle of the link | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this explicit ordering working around some gaps in the dependency graph? Would cmake automatically move those libraries to the correct location if the graph was complete? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
yes, what we are ensuring here is that we link the std libs at the exact location where they are required and not being relocated by CMake's internal logic. This is what you usually do in a CMake toolchain file using At the same time the use of properties on the linker target follows the design of compiler and linker flags and allow toolchains a uniform way of describing how standard libraries are selected and in which order (even allowing multiple times without de-duplication). The toolchain can describe a default set, while still allowing Zephyr and Zephyr module to adjust this set (and link order) if there is a need, for example based on Kconfig settings or static libs.
Followup to this work will be some proper documentation regarding Zephyr's toolchain infrastructure so that you should be able to create oot toolchain configurations, and even upstream those.
The problem is that this graph will never be complete. but that is not scalable and actually makes it harder to create static libs. From design perspective, then such library link information is not something a library should ever care about. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I managed to construct a complete graph in #65434, but the results were not terribly robust. In any case, I think this change is a good step forward. |
||
# command invocation. | ||
macro(toolchain_linker_finalize) | ||
set(zephyr_std_libs) | ||
get_property(link_order TARGET linker PROPERTY link_order_library) | ||
foreach(lib ${link_order}) | ||
get_property(link_flag TARGET linker PROPERTY ${lib}_library) | ||
set(zephyr_std_libs "${zephyr_std_libs} ${link_flag}") | ||
endforeach() | ||
|
||
set(common_link "<LINK_FLAGS> <LINK_LIBRARIES> <OBJECTS> ${zephyr_std_libs} -o <TARGET>") | ||
set(CMAKE_C_LINK_EXECUTABLE "<CMAKE_LINKER> <CMAKE_C_LINK_FLAGS> ${common_link}") | ||
set(CMAKE_CXX_LINK_EXECUTABLE "<CMAKE_LINKER> <CMAKE_CXX_LINK_FLAGS> ${common_link}") | ||
set(CMAKE_ASM_LINK_EXECUTABLE "<CMAKE_LINKER> <CMAKE_ASM_LINK_FLAGS> ${common_link}") | ||
endmacro() | ||
|
||
include(${ZEPHYR_BASE}/cmake/linker/ld/target_relocation.cmake) | ||
include(${ZEPHYR_BASE}/cmake/linker/ld/target_configure.cmake) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
# Copyright (c) 2024 Nordic Semiconductor | ||
# | ||
# SPDX-License-Identifier: Apache-2.0 | ||
|
||
# Do not specify default link libraries when targeting host (native build). | ||
if(NOT CONFIG_NATIVE_BUILD) | ||
set_linker_property(NO_CREATE PROPERTY c_library "-lc") | ||
set_linker_property(NO_CREATE PROPERTY rt_library "-lgcc") | ||
set_linker_property(NO_CREATE PROPERTY c++_library "-lstdc++") | ||
set_linker_property(NO_CREATE PROPERTY math_library "-lm") | ||
# Keeping default include dir empty. The linker will then select libraries | ||
# from its default search path. The toolchain may adjust the value to a | ||
# specific location, for example gcc infrastructure will set the value based | ||
# on output from --print-libgcc-file-name. | ||
set_linker_property(NO_CREATE PROPERTY lib_include_dir "") | ||
endif() | ||
|
||
if(CONFIG_CPP | ||
AND NOT CONFIG_MINIMAL_LIBCPP | ||
AND NOT CONFIG_NATIVE_LIBRARY | ||
# When new link principle is fully introduced, then the below condition can | ||
# be removed, and instead the external module c++ should use: | ||
# set_property(TARGET linker PROPERTY c++_library "<external_c++_lib>") | ||
AND NOT CONFIG_EXTERNAL_MODULE_LIBCPP | ||
) | ||
set_property(TARGET linker PROPERTY link_order_library "c++") | ||
endif() | ||
|
||
|
||
if(CONFIG_NEWLIB_LIBC AND CMAKE_C_COMPILER_ID STREQUAL "GNU") | ||
# We are using c;rt;c (expands to '-lc -lgcc -lc') in code below. | ||
# This is needed because when linking with newlib on aarch64, then libgcc has a | ||
# link dependency to libc (strchr), but libc also has dependencies to libgcc. | ||
# Lib C depends on libgcc. e.g. libc.a(lib_a-fvwrite.o) references __aeabi_idiv | ||
set_property(TARGET linker APPEND PROPERTY link_order_library "math;c;rt;c") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be reasonable to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personally I think that it's good to make it visible that there are indeed circular dependencies, and that we can see the number of loops. As side-note, from the ld manual:
|
||
else() | ||
set_property(TARGET linker APPEND PROPERTY link_order_library "c;rt") | ||
endif() |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,16 +6,12 @@ set(CMAKE_LINKER ${GNULD_LINKER}) | |
|
||
set_ifndef(LINKERFLAGPREFIX -Wl) | ||
|
||
if(NOT "${ZEPHYR_TOOLCHAIN_VARIANT}" STREQUAL "host") | ||
if(CONFIG_CPP_EXCEPTIONS AND LIBGCC_DIR) | ||
# When building with C++ Exceptions, it is important that crtbegin and crtend | ||
# are linked at specific locations. | ||
# The location is so important that we cannot let this be controlled by normal | ||
# link libraries, instead we must control the link command specifically as | ||
# part of toolchain. | ||
set(CMAKE_CXX_LINK_EXECUTABLE | ||
"<CMAKE_CXX_COMPILER> <FLAGS> <CMAKE_CXX_LINK_FLAGS> <LINK_FLAGS> ${LIBGCC_DIR}/crtbegin.o <OBJECTS> -o <TARGET> <LINK_LIBRARIES> ${LIBGCC_DIR}/crtend.o") | ||
endif() | ||
if((${CMAKE_LINKER} STREQUAL "${CROSS_COMPILE}ld.bfd") OR | ||
${GNULD_LINKER_IS_BFD}) | ||
# ld.bfd was found so let's explicitly use that for linking, see #32237 | ||
list(APPEND TOOLCHAIN_LD_FLAGS -fuse-ld=bfd) | ||
list(APPEND CMAKE_REQUIRED_FLAGS -fuse-ld=bfd) | ||
string(REPLACE ";" " " CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS}") | ||
endif() | ||
|
||
# Run $LINKER_SCRIPT file through the C preprocessor, producing ${linker_script_gen} | ||
|
@@ -117,16 +113,9 @@ function(toolchain_ld_link_elf) | |
${ARGN} # input args to parse | ||
) | ||
|
||
if((${CMAKE_LINKER} STREQUAL "${CROSS_COMPILE}ld.bfd") OR | ||
${GNULD_LINKER_IS_BFD}) | ||
# ld.bfd was found so let's explicitly use that for linking, see #32237 | ||
set(use_linker "-fuse-ld=bfd") | ||
endif() | ||
|
||
target_link_libraries( | ||
${TOOLCHAIN_LD_LINK_ELF_TARGET_ELF} | ||
${TOOLCHAIN_LD_LINK_ELF_LIBRARIES_PRE_SCRIPT} | ||
${use_linker} | ||
${TOPT} | ||
${TOOLCHAIN_LD_LINK_ELF_LINKER_SCRIPT} | ||
${TOOLCHAIN_LD_LINK_ELF_LIBRARIES_POST_SCRIPT} | ||
|
@@ -137,14 +126,44 @@ function(toolchain_ld_link_elf) | |
${LINKERFLAGPREFIX},--no-whole-archive | ||
${NO_WHOLE_ARCHIVE_LIBS} | ||
$<TARGET_OBJECTS:${OFFSETS_LIB}> | ||
${LIB_INCLUDE_DIR} | ||
-L${PROJECT_BINARY_DIR} | ||
${TOOLCHAIN_LIBS} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now If I restore this specific line then I wonder why was this
Indeed, if I remove cc: @kv2019i , @andyross, @cujomalainey, @dcpleung FYI:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. an oversight. I initially wanted to still allow for the old use of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @tejlmand !
While @dcpleung is removing the obsolete TOOLCHAIN_LIBS for Xtensa in #79731, as of today I see it still present in the main branch for In case these toolchains have no owner left, then it is a great opportunity to notice that and delete them entirely. There is also a mysterious |
||
|
||
${TOOLCHAIN_LD_LINK_ELF_DEPENDENCIES} | ||
) | ||
endfunction(toolchain_ld_link_elf) | ||
|
||
# Function for finalizing link setup after Zephyr configuration has completed. | ||
# | ||
# This function will generate the correct CMAKE_C_LINK_EXECUTABLE / CMAKE_CXX_LINK_EXECUTABLE | ||
# rule to ensure that standard c and runtime libraries are correctly placed | ||
# and the end of link invocation and doesn't appear in the middle of the link | ||
# command invocation. | ||
macro(toolchain_linker_finalize) | ||
get_property(zephyr_std_libs TARGET linker PROPERTY lib_include_dir) | ||
get_property(link_order TARGET linker PROPERTY link_order_library) | ||
foreach(lib ${link_order}) | ||
get_property(link_flag TARGET linker PROPERTY ${lib}_library) | ||
list(APPEND zephyr_std_libs "${link_flag}") | ||
endforeach() | ||
string(REPLACE ";" " " zephyr_std_libs "${zephyr_std_libs}") | ||
|
||
set(link_libraries "<LINK_FLAGS> <OBJECTS> -o <TARGET> <LINK_LIBRARIES> ${zephyr_std_libs}") | ||
set(common_link "<LINK_FLAGS> ${link_libraries}") | ||
|
||
set(CMAKE_ASM_LINK_EXECUTABLE "<CMAKE_ASM_COMPILER> <FLAGS> <CMAKE_ASM_LINK_FLAGS> ${common_link}") | ||
set(CMAKE_C_LINK_EXECUTABLE "<CMAKE_C_COMPILER> <FLAGS> <CMAKE_C_LINK_FLAGS> ${common_link}") | ||
|
||
set(cpp_link "${common_link}") | ||
if(NOT "${ZEPHYR_TOOLCHAIN_VARIANT}" STREQUAL "host") | ||
if(CONFIG_CPP_EXCEPTIONS AND LIBGCC_DIR) | ||
# When building with C++ Exceptions, it is important that crtbegin and crtend | ||
# are linked at specific locations. | ||
set(cpp_link "<LINK_FLAGS> ${LIBGCC_DIR}/crtbegin.o ${link_libraries} ${LIBGCC_DIR}/crtend.o") | ||
endif() | ||
endif() | ||
set(CMAKE_CXX_LINK_EXECUTABLE "<CMAKE_CXX_COMPILER> <FLAGS> <CMAKE_CXX_LINK_FLAGS> ${cpp_link}") | ||
endmacro() | ||
|
||
# Load toolchain_ld-family macros | ||
include(${ZEPHYR_BASE}/cmake/linker/${LINKER}/target_relocation.cmake) | ||
include(${ZEPHYR_BASE}/cmake/linker/${LINKER}/target_configure.cmake) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
# Copyright (c) 2024 Nordic Semiconductor | ||
# | ||
# SPDX-License-Identifier: Apache-2.0 | ||
|
||
# Linker flags for fixed linking with standard libraries, such as the C and runtime libraries. | ||
# It is the responsibility of the linker infrastructure to use those properties to specify the | ||
# correct placement of those libraries for correct link order. | ||
# For example, GCC usually has the order: -lc -lgcc | ||
# It is also possible to define extra libraries of the form `<name>_library`, and then include | ||
# Fixed library search path can be defined in the `lib_include_dir` property if needed. | ||
# <name> in the link_order_property. | ||
# Usage example: | ||
# set_linker_property(PROPERTY lib_include_dir "-L/path/to/libs") | ||
# set_linker_property(PROPERTY c_library "-lc") | ||
# set_linker_property(PROPERTY rt_library "-lgcc") | ||
# set_linker_property(PROPERTY link_order_library "c;rt") |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,21 @@ | ||||||
# Copyright (c) 2024 Nordic Semiconductor | ||||||
# | ||||||
# SPDX-License-Identifier: Apache-2.0 | ||||||
|
||||||
set_linker_property(NO_CREATE TARGET linker PROPERTY c_library "-lc") | ||||||
# Default per standard, will be populated by clang/target.cmake based on clang output. | ||||||
set_linker_property(NO_CREATE TARGET linker PROPERTY rt_library "") | ||||||
set_linker_property(TARGET linker PROPERTY c++_library "-lc++;-lc++abi") | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this mention There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. II don't think so. zephyr/cmake/linker/linker_libraries_template.cmake Lines 9 to 10 in 3b8e621
I will be working on a docset to describe the whole toolchain infrastructure and how to implement support for a custom one. |
||||||
|
||||||
if(CONFIG_CPP | ||||||
AND NOT CONFIG_MINIMAL_LIBCPP | ||||||
AND NOT CONFIG_NATIVE_LIBRARY | ||||||
# When new link principle is fully introduced, then the below condition can | ||||||
# be removed, and instead the external module c++ should use: | ||||||
# set_property(TARGET linker PROPERTY c++_library "<external_c++_lib>") | ||||||
AND NOT CONFIG_EXTERNAL_MODULE_LIBCPP | ||||||
) | ||||||
set_property(TARGET linker PROPERTY link_order_library "c++") | ||||||
endif() | ||||||
|
||||||
set_property(TARGET linker APPEND PROPERTY link_order_library "c;rt") |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
# SPDX-License-Identifier: Apache-2.0 | ||
# | ||
# Copyright (c) 2024, Nordic Semiconductor ASA | ||
|
||
# Template file for optional Zephyr linker macros. | ||
# | ||
# This file will define optional linker macros for toolchains that are not | ||
# defining these macros themselves. | ||
|
||
if(NOT COMMAND toolchain_linker_finalize) | ||
macro(toolchain_linker_finalize) | ||
endmacro() | ||
endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe share the --specs mechanism somehow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see other answer related to this. But good to have in mind for further improvements 👍