Skip to content

Fix bug that caused build to break with -DUSE_SHARED_TF_PSA_CRYPTO_LIBRARY=ON #277

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.

Sign up for GitHub

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

Closed
felixc-arm wants to merge 7 commits into from
Closed

Fix bug that caused build to break with -DUSE_SHARED_TF_PSA_CRYPTO_LIBRARY=ON #277

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
aafe9c6
Fix bug that cause build to break with -DUSE_SHARED_TF_PSA_CRYPTO_LIB…
felixc-arm May 2, 2025
63f4915
Fix linker error
felixc-arm May 2, 2025
8d619e6
Remove changes to test new CI
felixc-arm May 6, 2025
d28522a
Add CI job to catch issue in future
felixc-arm May 6, 2025
ac140e5
Make CI job use GCC
felixc-arm May 6, 2025
77f4e51
Improve comments
felixc-arm May 6, 2025
b920a5b
Re-add bug fix to validate CI job
felixc-arm May 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion programs/test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ target_link_libraries(benchmark PRIVATE ${tfpsacrypto_target} ${CMAKE_THREAD_LIB

# which_aes
tfpsacrypto_build_program_common(which_aes)
target_link_libraries(which_aes PRIVATE ${tfpsacrypto_target})
target_sources(which_aes PRIVATE $<TARGET_OBJECTS:tf_psa_crypto_test>)
Copy link
Contributor

@ronald-cron-arm ronald-cron-arm May 22, 2025

Choose a reason for hiding this comment

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

I think it would be better to not do that as this rather hides than fixes the issue. The core issue seems that we have two separate libraries, in the C compiler/linker sense, for the PSA core and the driver builtin that reference each other and it is linker dependent how this is handled. We should probably have only one library and in the CMake scripts to define the builtin library as an object library. I will create an issue for that.

Copy link
Contributor

@ronald-cron-arm ronald-cron-arm May 26, 2025

Choose a reason for hiding this comment

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

Here is the issue: #300

target_link_libraries(which_aes PRIVATE ${tfpsacrypto_target} ${CMAKE_THREAD_LIBS_INIT})

# dlopen
if(USE_SHARED_TF_PSA_CRYPTO_LIBRARY AND NOT ${CMAKE_SYSTEM_NAME} MATCHES "[Ww][Ii][Nn]")
Expand Down
34 changes: 34 additions & 0 deletions tests/scripts/components-build-system.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,40 @@ component_test_tf_psa_crypto_shared () {
$FRAMEWORK/tests/programs/dlopen_demo.sh
}

support_build_tf_psa_crypto_shared_newer_ld_gcc () {
ld_version=""
ld_version_major=""
ld_version_minor=""
distrib_id=""

# Attempt to parse ld -v to find out ld’s version. If not found this should fail safe.
if command ld -v >/dev/null 2>&1; then
ld_version=$(ld -v | awk '{print $NF}')
# split into major and minor
ld_version_major=${ld_version%%.*}
tmp=${ld_version#*.}
ld_version_minor=${tmp%%.*}
fi

if [[ -f /etc/lsb-release ]]; then
while read -r lsb_line; do
case "$lsb_line" in
"DISTRIB_ID"*) distrib_id=${lsb_line/#DISTRIB_ID=};;
esac
done < /etc/lsb-release
fi

# Newer lds with gcc cause extra errors that wouldn't be caught with older versions
[ "$distrib_id" != "Ubuntu" ] || [ "$ld_version_minor" -gt 37 ]
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm Jun 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Why does the exact Linux distribution matter? As far as I understand, there are three possibilities:

  • Recent GNU binutils — good.
  • Old GNU binutils — bad.
  • Non-GNU ld — good? Or bad, since we can't be sure they would catch the problem? It's a bit of a moot point anyway since we run every component on Ubuntu (and even if we create non-Ubuntu components one day, we'd still keep Ubuntu as a possibility).

If we just want a recent enough GNU binutils, given that we can assume Ubuntu, we can just check the version of binutils.

support_build_tf_psa_crypto_shared_newer_ld_gcc () {
    # Require a recent enough GNU binutils, because older ones are more
    # permissive and would not detect e.g.
    # https://github.com/Mbed-TLS/TF-PSA-Crypto/issues/300
    dpkg --compare-versions "$(dpkg-query --show --showformat '${Version}' binutils)" '>=' 2.38 2>/dev/null
}

}

component_build_tf_psa_crypto_shared_newer_ld_gcc () {
msg "build/test: shared libraries"
# Test building with this option with newer lds and gcc for a more thorough check
CC=gcc cmake -DUSE_SHARED_TF_PSA_CRYPTO_LIBRARY=ON "$TF_PSA_CRYPTO_ROOT_DIR"
make
}

component_test_tf_psa_crypto_out_of_source () {
msg "build: cmake tf-psa-crypto 'out-of-source' build"
cd $OUT_OF_SOURCE_DIR
Expand Down