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

Building v3.2.1 on ARM fails #6089

Closed
Aethedor opened this issue Jul 14, 2022 · 12 comments · Fixed by #6090 · May be fixed by hannestschofenig/mbedtls#399
Closed

Building v3.2.1 on ARM fails #6089

Aethedor opened this issue Jul 14, 2022 · 12 comments · Fixed by #6090 · May be fixed by hannestschofenig/mbedtls#399
Assignees
Labels
bug component-crypto Crypto primitives and low-level interfaces priority-medium Medium priority - this can be reviewed as time permits size-s Estimated task size: small (~2d)

Comments

@Aethedor
Copy link

Aethedor commented Jul 14, 2022

Summary

I tried to build the Hiawatha webserver, which uses mbed TLS, on a Raspberry Pi v4. It fails with errors like these:

/tmp/ccWJfJUb.s:3868: Error: unexpected character w' in type specifier /tmp/ccWJfJUb.s:3868: Error: bad instruction ldr.w r5,[r3]'
/tmp/ccWJfJUb.s:3870: Error: unexpected character w' in type specifier /tmp/ccWJfJUb.s:3870: Error: bad instruction str.w r5,[r3],#4'

System information

Mbed TLS version (number or commit id): 3.2.1
Operating system and version: Raspberry Pi v4
Configuration (if not default, please attach mbedtls_config.h):
Compiler and options (if you used a pre-built binary, please indicate how you obtained it):
Additional environment information:

Expected behavior

Compliation successful

Actual behavior

Compilation failed

Steps to reproduce

Compile Hiawatha v11.2 on a Raspberry Pi.
https://www.hiawatha-webserver.org/files/hiawatha-11.2.tar.gz

Additional information

In bn_mul.h, I changed ldr.w and str.w to ldr and str at line 752 and further. That solved my issue.

@tom-cosgrove-arm
Copy link
Contributor

tom-cosgrove-arm commented Jul 15, 2022

Could you please give us the output of cat /etc/os-release, uname -a and cc --version?

And which version of Mbed TLS were you using previously, or is this the first time you have tried?

Update: on my own RPi 4:

pi64 $ curl -L -o mbedtls-3.2.1.tgz https://github.com/Mbed-TLS/mbedtls/archive/refs/tags/v3.2.1.tar.gz
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100 4001k    0 4001k    0     0  3601k      0 --:--:--  0:00:01 --:--:-- 6903k
pi64 $ tar xfz mbedtls-3.2.1.tgz
pi64 $ cd mbedtls-3.2.1
pi64 $ make CFLAGS="-O3"
  CC    aes.c
  CC    aesni.c
  CC    aria.c
  CC    asn1parse.c
:
:
  Gen   test_suite_cipher.chachapoly.c
  CC    test_suite_cipher.chachapoly.c
  Gen   test_suite_mpi.c
  CC    test_suite_mpi.c
pi64 $ echo $?
0
pi64 $ cat /etc/os-release
NAME="Ubuntu"
VERSION="20.04.3 LTS (Focal Fossa)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 20.04.3 LTS"
VERSION_ID="20.04"
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
VERSION_CODENAME=focal
UBUNTU_CODENAME=focal
pi64 $ uname -a
Linux pi64 5.4.0-1044-raspi #48-Ubuntu SMP PREEMPT Thu Sep 9 15:24:01 UTC 2021 aarch64 aarch64 aarch64 GNU/Linux
pi64 $ cc --version
cc (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

@tom-cosgrove-arm tom-cosgrove-arm self-assigned this Jul 15, 2022
@Aethedor
Copy link
Author

Aethedor commented Jul 15, 2022

/etc/os-release:
PRETTY_NAME="Raspbian GNU/Linux 11 (bullseye)"
NAME="Raspbian GNU/Linux"
VERSION_ID="11"
VERSION="11 (bullseye)"
VERSION_CODENAME=bullseye
ID=raspbian
ID_LIKE=debian
HOME_URL="http://www.raspbian.org/"
SUPPORT_URL="http://www.raspbian.org/RaspbianForums"
BUG_REPORT_URL="http://www.raspbian.org/RaspbianBugs"

uname -a:
Linux raspberrypi 5.15.32-v7l+ #1538 SMP Thu Mar 31 19:39:41 BST 2022 armv7l GNU/Linux

cc --version:
cc (Raspbian 10.2.1-6+rpi1) 10.2.1 20210110
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Before 3.2.1, I used 3.1.0.

@Aethedor
Copy link
Author

Aethedor commented Jul 15, 2022

Never mind this issue. It seems the mbed TLS building has changed. Need to address that too in my building scripts.

@tom-cosgrove-arm
Copy link
Contributor

tom-cosgrove-arm commented Jul 15, 2022

Hmm, actually this does seem to be an issue when compiling with optimisation (i.e. anything > -O0) on Armv7 with older gcc

@tom-cosgrove-arm tom-cosgrove-arm added bug component-crypto Crypto primitives and low-level interfaces size-s Estimated task size: small (~2d) priority-medium Medium priority - this can be reviewed as time permits labels Jul 15, 2022
@Aethedor
Copy link
Author

Yes, for some reason, this used to work, but now on a Pi no more...

option(USE_STATIC_MBEDTLS_LIBRARY "Build mbed TLS static library." off)
option(USE_SHARED_MBEDTLS_LIBRARY "Build mbed TLS shared library." on)
option(ENABLE_PROGRAMS "Build mbed TLS programs." off)
add_subdirectory(mbedtls)

@tom-cosgrove-arm
Copy link
Contributor

Yes, these changes are optimisations introduced in April 2022 in #5706

@Aethedor
Copy link
Author

I don't understand what's said in #5706, I just wanna be able to use a TLS library. ;)

@Aethedor Aethedor reopened this Jul 15, 2022
@hanno-becker
Copy link

Thanks for the report @Aethedor.

A change was made to a piece of assembly under the assumption that this assembly would be used on M-profile systems only, but strangely (I don't know if intentionally, initially), it was also used and valid on Armv7-A systems. The change, however, was strictly M-profile only, and that's what's leading to the issue you're seeing.

I guess the solution will be to reintroduce an A-class version of the code under correct compile time guards.

@Aethedor
Copy link
Author

If you have a patch that needs testing, let me know.

hanno-becker pushed a commit to hanno-becker/mbedtls that referenced this issue Jul 15, 2022
Within the M-profile of the Arm architecture, some instructions
admit both a 16-bit and a 32-bit encoding. For those instructions,
some assemblers support the use of the .n (narrow) and .w (wide)
suffixes to force a choice of instruction encoding width.
Forcing the size of encodings may be useful to ensure alignment
of code, which can have a significant performance impact on some
microarchitectures.

It is for this reason that a previous commit introduced explicit
.w suffixes into what was believed to be M-profile only assembly
in library/bn_mul.h.

This change, however, introduced two issues:
- First, the assembly block in question is used also for Armv7-A
  systems, on which the .n/.w distinction is not meaningful
  (all instructions are 32-bit).
- Second, compiler support for .n/.w suffixes appears patchy,
  leading to compilation failures even when building for M-profile
  targets.

This commit removes the .w annotations in order to restore working
code, deferring controlled re-introduction for the sake of performance.

Fixes Mbed-TLS#6089.

Signed-off-by: Hanno Becker <[email protected]>
@hanno-becker
Copy link

@Aethedor Can you check #6090 please?

@Aethedor
Copy link
Author

@hanno-arm The changes to bn_mul.h are as I mentioned in the first post. That works for me. Good to hear that my manual patch is indeed the right one.

@hanno-becker
Copy link

@Aethedor This is rather a workaround than the final solution -- the suffixes serve performance optimization purposes for M-class code, but compiler support seem brittle so we're removing them for now.

lhuang04 pushed a commit to lhuang04/mbedtls that referenced this issue Aug 22, 2022
Within the M-profile of the Arm architecture, some instructions
admit both a 16-bit and a 32-bit encoding. For those instructions,
some assemblers support the use of the .n (narrow) and .w (wide)
suffixes to force a choice of instruction encoding width.
Forcing the size of encodings may be useful to ensure alignment
of code, which can have a significant performance impact on some
microarchitectures.

It is for this reason that a previous commit introduced explicit
.w suffixes into what was believed to be M-profile only assembly
in library/bn_mul.h.

This change, however, introduced two issues:
- First, the assembly block in question is used also for Armv7-A
  systems, on which the .n/.w distinction is not meaningful
  (all instructions are 32-bit).
- Second, compiler support for .n/.w suffixes appears patchy,
  leading to compilation failures even when building for M-profile
  targets.

This commit removes the .w annotations in order to restore working
code, deferring controlled re-introduction for the sake of performance.

Fixes Mbed-TLS#6089.

Signed-off-by: Hanno Becker <[email protected]>
kurt-cb pushed a commit to kurt-cb/mbedtls that referenced this issue Aug 1, 2023
Within the M-profile of the Arm architecture, some instructions
admit both a 16-bit and a 32-bit encoding. For those instructions,
some assemblers support the use of the .n (narrow) and .w (wide)
suffixes to force a choice of instruction encoding width.
Forcing the size of encodings may be useful to ensure alignment
of code, which can have a significant performance impact on some
microarchitectures.

It is for this reason that a previous commit introduced explicit
.w suffixes into what was believed to be M-profile only assembly
in library/bn_mul.h.

This change, however, introduced two issues:
- First, the assembly block in question is used also for Armv7-A
  systems, on which the .n/.w distinction is not meaningful
  (all instructions are 32-bit).
- Second, compiler support for .n/.w suffixes appears patchy,
  leading to compilation failures even when building for M-profile
  targets.

This commit removes the .w annotations in order to restore working
code, deferring controlled re-introduction for the sake of performance.

Fixes Mbed-TLS#6089.

Signed-off-by: Hanno Becker <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component-crypto Crypto primitives and low-level interfaces priority-medium Medium priority - this can be reviewed as time permits size-s Estimated task size: small (~2d)
Projects
None yet
3 participants