cmake: add LATE_LINK to fix llext weak syscall definitions #79023
+36
−9
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This patch adds a new property to Zephyr libraries called LATE_LINK. When set to TRUE, the library will be linked after all other libraries that are not marked "late". Ordering between late libs is not guaranteed. Signed-off-by: Luca Burelli <l.burelli@arduino.cc>
The llext_export.c file defines weak symbols for any syscall; these are intended to be overridden by the application if the implementation is compiled in. However, including this among all other libraries caused the weak symbols to override the actual symbols defined in the following libraries. This commit moves the file to a separate library that is linked late to prevent the weak symbols from overriding the actual symbols. Signed-off-by: Luca Burelli <l.burelli@arduino.cc>
zephyrbot
requested review from
57300,
jeremybettis,
lyakh,
nashif,
nordicjm,
teburd and
tejlmand
tejlmand
requested changes
Sep 27, 2024
Comment on lines
+981
to
+991
| # Move the libraries that are supposed to be linked late in the link | ||
| # process to the end of their respective list. | ||
| foreach(zephyr_lib ${ZEPHYR_LATE_LIBS_PROPERTY}) | ||
| if("${zephyr_lib}" IN_LIST WHOLE_ARCHIVE_LIBS) | ||
| list(REMOVE_ITEM WHOLE_ARCHIVE_LIBS ${zephyr_lib}) | ||
| list(APPEND WHOLE_ARCHIVE_LIBS ${zephyr_lib}) | ||
| elseif("${zephyr_lib}" IN_LIST NO_WHOLE_ARCHIVE_LIBS) | ||
| list(REMOVE_ITEM NO_WHOLE_ARCHIVE_LIBS ${zephyr_lib}) | ||
| list(APPEND NO_WHOLE_ARCHIVE_LIBS ${zephyr_lib}) | ||
| endif() | ||
| endforeach() |
There was a problem hiding this comment.
let's not re-invent something already well handled by CMake.
Also this will become whac-a-mole in the future when order starts to matter and we then need to guarantee an order, ref:
Ordering between late libs is not guaranteed.
From the PR description, then I would like to ask, why is the llext-exports library a Zephyr library in the first place ?
From the looks of this PR then it seems to me that the llext-exports lib should be outside the --whole-archive and thus not be a Zephyr library.
Keeping the lib which needs this LATE LINKING control outside whole-archive allows to user regular target_link_libraries() to ensure correct link order.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since 62b19ef LLEXT has gained the ability to export all syscalls automatically. The export is done for any syscall declared in the build, including those that are never actually defined - most commonly due to not enabling some Kconfig.
These undefined functions would normally cause a link error; the workaround has been to define, in an autogenerated file included by
llext_export.c, weak implementations for all syscall symbols to the stockno_handlerfunction, with the intention of providing a sensible default to link with.However, during development I discovered that some of these symbols were actually defined in the final ELF as
NULL. This has been traced to thellext_export.cfile being included in the middle of the--whole-archivelist, instead of at the end.This PR adds a
LATE_LINKproperty to Zephyr libraries (by default disabled). Libraries that have this feature enabled will be placed after all the ones that do not enable it in the linker command line; no other ordering is enforced between these two groups.This feature is then used to split build the
llext_exports.cfile in its own separate library and move it to the end of the link list, fixing the symbol definition issue.