From 5daa34f155c758a9f626bce88b527b92598c33b7 Mon Sep 17 00:00:00 2001 From: Aurelien Jarno Date: Sat, 3 Nov 2018 00:46:06 +0100 Subject: [PATCH 1/5] bn_mul.h: require at least ARMv6 to enable the ARM DSP code Commit 16b1bd89326e "bn_mul.h: add ARM DSP optimized MULADDC code" added some ARM DSP instructions that was assumed to always be available when __ARM_FEATURE_DSP is defined to 1. Unfortunately it appears that the ARMv5TE architecture (GCC flag -march=armv5te) supports the DSP instructions, but only in Thumb mode and not in ARM mode, despite defining __ARM_FEATURE_DSP in both cases. This patch fixes the build issue by requiring at least ARMv6 in addition to the DSP feature. --- include/mbedtls/bn_mul.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/bn_mul.h b/include/mbedtls/bn_mul.h index c33bd8d4ab..748975ea51 100644 --- a/include/mbedtls/bn_mul.h +++ b/include/mbedtls/bn_mul.h @@ -642,7 +642,8 @@ "r6", "r7", "r8", "r9", "cc" \ ); -#elif defined (__ARM_FEATURE_DSP) && (__ARM_FEATURE_DSP == 1) +#elif (__ARM_ARCH >= 6) && \ + defined (__ARM_FEATURE_DSP) && (__ARM_FEATURE_DSP == 1) #define MULADDC_INIT \ asm( From a5cb7d48f35aeb8e3a78242614080c920bb21f9c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 5 Aug 2019 11:34:11 +0200 Subject: [PATCH 2/5] Add changelog entry for ARM assembly fix --- ChangeLog | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ChangeLog b/ChangeLog index 37373a23ca..e1110a154a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -79,6 +79,9 @@ Bugfix This previously limited the maximum size of DER encoded certificates in mbedtls_x509write_crt_der() to 2Kb. Reported by soccerGB in #2631. * Fix partial zeroing in x509_get_other_name. Found and fixed by ekse, #2716. + * Fix the build on ARMv5TE in ARM mode to not use assembly instructions + that are only available in Thumb mode. Fix contributed by Aurelien Jarno + in #2169. API Changes * Extend the MBEDTLS_SSL_EXPORT_KEYS to export the handshake randbytes, From 93e4e03f9473306afe077c6631924fadf16b3f82 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 5 Aug 2019 11:34:25 +0200 Subject: [PATCH 3/5] Add a build on ARMv5TE in ARM mode Non-regression test for "bn_mul.h: require at least ARMv6 to enable the ARM DSP code" --- tests/scripts/all.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 22c81296c8..d9ef302ab3 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1088,6 +1088,12 @@ component_build_arm_none_eabi_gcc () { make CC=arm-none-eabi-gcc AR=arm-none-eabi-ar LD=arm-none-eabi-ld CFLAGS='-Werror -Wall -Wextra' lib } +component_build_arm_none_eabi_gcc_armel () { + msg "build: arm-none-eabi-gcc, make" # ~ 10s + scripts/config.pl baremetal + make CC=arm-none-eabi-gcc AR=arm-none-eabi-ar CFLAGS='-Werror -Wall -Wextra -march=armv5te' LDFLAGS='-march=armv5te' SHELL='sh -x' lib +} + component_build_arm_none_eabi_gcc_no_udbl_division () { msg "build: arm-none-eabi-gcc -DMBEDTLS_NO_UDBL_DIVISION, make" # ~ 10s scripts/config.pl baremetal From 8a52af9b7704b7c0e17eb7f56dc0fdfe94ec51b5 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 8 Aug 2019 16:09:02 +0200 Subject: [PATCH 4/5] Switch armel build to -Os Without any -O option, the default is -O0, and then the assembly code is not used, so this would not be a non-regression test for the assembly code that doesn't build. --- tests/scripts/all.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index d9ef302ab3..739f06143d 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1089,9 +1089,9 @@ component_build_arm_none_eabi_gcc () { } component_build_arm_none_eabi_gcc_armel () { - msg "build: arm-none-eabi-gcc, make" # ~ 10s + msg "build: arm-none-eabi-gcc -march=arm5vte, make" # ~ 10s scripts/config.pl baremetal - make CC=arm-none-eabi-gcc AR=arm-none-eabi-ar CFLAGS='-Werror -Wall -Wextra -march=armv5te' LDFLAGS='-march=armv5te' SHELL='sh -x' lib + make CC=arm-none-eabi-gcc AR=arm-none-eabi-ar CFLAGS='-Werror -Wall -Wextra -march=armv5te -O1' LDFLAGS='-march=armv5te' SHELL='sh -x' lib } component_build_arm_none_eabi_gcc_no_udbl_division () { From 2c897d76ff6f3d6891cbfe55703f3c1dfada0fc3 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 9 Aug 2019 16:05:05 +0200 Subject: [PATCH 5/5] Document the rationale for the armel build Call the component xxx_arm5vte, because that's what it does. Explain "armel", and more generally why this component exists, in a comment. --- tests/scripts/all.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 739f06143d..e49277a85c 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1088,9 +1088,14 @@ component_build_arm_none_eabi_gcc () { make CC=arm-none-eabi-gcc AR=arm-none-eabi-ar LD=arm-none-eabi-ld CFLAGS='-Werror -Wall -Wextra' lib } -component_build_arm_none_eabi_gcc_armel () { +component_build_arm_none_eabi_gcc_arm5vte () { msg "build: arm-none-eabi-gcc -march=arm5vte, make" # ~ 10s scripts/config.pl baremetal + # Build for a target platform that's close to what Debian uses + # for its "armel" distribution (https://wiki.debian.org/ArmEabiPort). + # See https://github.com/ARMmbed/mbedtls/pull/2169 and comments. + # It would be better to build with arm-linux-gnueabi-gcc but + # we don't have that on our CI at this time. make CC=arm-none-eabi-gcc AR=arm-none-eabi-ar CFLAGS='-Werror -Wall -Wextra -march=armv5te -O1' LDFLAGS='-march=armv5te' SHELL='sh -x' lib }