| Summary: | [GCC 9/10] arm-none-eabi function attribute 'cmse_nonsecure_entry' wipes register values with -Os | ||
|---|---|---|---|
| Product: | gcc | Reporter: | Ciaran Woodward <c.woodward> |
| Component: | target | Assignee: | avieira |
| Status: | ASSIGNED --- | ||
| Severity: | normal | CC: | acoplan, avieira, c.woodward, clyon, xinyu.zhang |
| Priority: | P3 | Keywords: | wrong-code |
| Version: | 9.3.1 | ||
| Target Milestone: | --- | ||
| Host: | Target: | arm-none-eabi | |
| Build: | Known to work: | ||
| Known to fail: | 7.2.1, 7.3.1, 8.3.1, 9.2.1, 9.3.1 | Last reconfirmed: | 2020-06-15 00:00:00 |
| Attachments: | test file exhibiting the issue with cmse_nonsecure_entry and Os | ||
Reproduced and confirmed. This is because we special treat HI_REGS in Thumb-1 when optimizing for size. I have a fix ready, just doing some testing. The master branch has been updated by Andre Simoes Dias Vieira <avieira@gcc.gnu.org>: https://gcc.gnu.org/g:5f426554fd804d65509875d706d8b8bc3a48393b commit r11-1601-g5f426554fd804d65509875d706d8b8bc3a48393b Author: Andre Simoes Dias Vieira <andre.simoesdiasvieira@arm.com> Date: Tue Jun 23 15:20:17 2020 +0100 arm: PR target/95646: Do not clobber callee saved registers with CMSE As reported in bugzilla when the -mcmse option is used while compiling for size (-Os) with a thumb-1 target the generated code will clear the registers r7-r10. These however are callee saved and should be preserved accross ABI boundaries. The reason this happens is because these registers are made "fixed" when optimising for size with Thumb-1 in a way to make sure they are not used, as pushing and popping hi-registers requires extra moves to and from LO_REGS. To fix this, this patch uses 'callee_saved_reg_p', which accounts for this optimisation, instead of 'call_used_or_fixed_reg_p'. Be aware of 'callee_saved_reg_p''s definition, as it does still take call used registers into account, which aren't callee_saved in my opinion, so it is a rather misnoemer, works in our advantage here though as it does exactly what we need. Regression tested on arm-none-eabi. Is this OK for trunk? (Will eventually backport to previous versions if stable.) gcc/ChangeLog: 2020-06-19 Andre Vieira <andre.simoesdiasvieira@arm.com> PR target/95646 * config/arm/arm.c: (cmse_nonsecure_entry_clear_before_return): Use 'callee_saved_reg_p' instead of 'calL_used_or_fixed_reg_p'. gcc/testsuite/ChangeLog: 2020-06-19 Andre Vieira <andre.simoesdiasvieira@arm.com> PR target/95646 * gcc.target/arm/pr95646.c: New test. The master branch has been updated by Andre Simoes Dias Vieira <avieira@gcc.gnu.org>: https://gcc.gnu.org/g:80297f897758f59071968ddff2a04a8d11481117 commit r11-3203-g80297f897758f59071968ddff2a04a8d11481117 Author: Andre Vieira <andre.simoesdiasvieira@arm.com> Date: Mon Sep 14 09:03:08 2020 +0100 arm: Fix testisms introduced with fix for pr target/95646 This patch changes the test to use the effective-target machinery disables the error message "ARMv8-M Security Extensions incompatible with selected FPU" when -mfloat-abi=soft. Further changes 'asm' to '__asm__' to avoid failures with '-std=' options. gcc/ChangeLog: 2020-07-06 Andre Vieira <andre.simoesdiasvieira@arm.com> * config/arm/arm.c (arm_options_perform_arch_sanity_checks): Do not check +D32 for CMSE if -mfloat-abi=soft gcc/testsuite/ChangeLog: 2020-07-06 Andre Vieira <andre.simoesdiasvieira@arm.com> * gcc.target/arm/pr95646.c: Fix testism. Changed title to reflect that this still needs backports to GCC 9 and 10. Could this bug be fixed in next version? If so, when could the new version be released? The releases/gcc-10 branch has been updated by SRINATH PARVATHANENI <sripar01@gcc.gnu.org>: https://gcc.gnu.org/g:f1370bf2aa6cac4ab6235d8480b0a5d4f85ca54e commit r10-9806-gf1370bf2aa6cac4ab6235d8480b0a5d4f85ca54e Author: Srinath Parvathaneni <srinath.parvathaneni@arm.com> Date: Thu May 6 10:46:45 2021 +0100 arm: Do not clobber callee saved registers with CMSE. As reported in bugzilla when the -mcmse option is used while compiling for size (-Os) with a thumb-1 target the generated code will clear the registers r7-r10. These however are callee saved and should be preserved accross ABI boundaries. The reason this happens is because these registers are made "fixed" when optimising for size with Thumb-1 in a way to make sure they are not used, as pushing and popping hi-registers requires extra moves to and from LO_REGS. To fix this, this patch uses 'callee_saved_reg_p', which accounts for this optimisation, instead of 'call_used_or_fixed_reg_p'. Be aware of 'callee_saved_reg_p''s definition, as it does still take call used registers into account, which aren't callee_saved in my opinion, so it is a rather misnoemer, works in our advantage here though as it does exactly what we need. gcc/ChangeLog: 2020-06-19 Andre Vieira <andre.simoesdiasvieira@arm.com> PR target/95646 * config/arm/arm.c: (cmse_nonsecure_entry_clear_before_return): Use 'callee_saved_reg_p' instead of 'calL_used_or_fixed_reg_p'. gcc/testsuite/ChangeLog: 2020-06-19 Andre Vieira <andre.simoesdiasvieira@arm.com> PR target/95646 * gcc.target/arm/pr95646.c: New test. (cherry picked from commit 5f426554fd804d65509875d706d8b8bc3a48393b) The releases/gcc-10 branch has been updated by SRINATH PARVATHANENI <sripar01@gcc.gnu.org>: https://gcc.gnu.org/g:d218fed53d811212b015a08cd2e1214000813fbf commit r10-9807-gd218fed53d811212b015a08cd2e1214000813fbf Author: Andre Vieira <andre.simoesdiasvieira@arm.com> Date: Mon Sep 14 09:03:08 2020 +0100 arm: Fix testisms introduced with fix for pr target/95646 This patch changes the test to use the effective-target machinery disables the error message "ARMv8-M Security Extensions incompatible with selected FPU" when -mfloat-abi=soft. Further changes 'asm' to '__asm__' to avoid failures with '-std=' options. gcc/ChangeLog: 2020-07-06 Andre Vieira <andre.simoesdiasvieira@arm.com> * config/arm/arm.c (arm_options_perform_arch_sanity_checks): Do not check +D32 for CMSE if -mfloat-abi=soft gcc/testsuite/ChangeLog: 2020-07-06 Andre Vieira <andre.simoesdiasvieira@arm.com> * gcc.target/arm/pr95646.c: Fix testism. (cherry picked from commit 80297f897758f59071968ddff2a04a8d11481117) |
Created attachment 48721 [details] test file exhibiting the issue with cmse_nonsecure_entry and Os This issue is specific to arm-none-eabi target, when building with arm CMSE and -Os. The issue is that the calling convention is violated in functions marked with __attribute__((cmse_nonsecure_entry)), and registers are wiped when they should be preserved. The issue only occurs when using -Os on the command line. O1, O2, O3 do not produce the issue. Using function attributes to affect the optimisation level does not seem to either cause nor prevent the error - only command line option. The issue is that upon returning from the entry function, r8, r9, r10, r11 and r12 are wiped without being restored. As per the 'Procedure Call Standard for ARM Architecture' document, these should be preserved by the subroutine. The 'ARM v8-M Security Extensions: Requirements on development tools' (section 5.4) specification specifies that all registers must be cleared before returning from a secure entry function, which I imagine is where this issue originates. However it also states that the registers should be restored, which can be observed in O0. In O1, O2, O3, these are optimised out, which is fine. However, in Os, the registers are cleared, but never restored, which causes issues that are quite difficult to debug. I have attached a simple (single function, no includes) c file that can be used to recreate the issue. I also have a runtime test that can demonstrate the broken behaviour if that would be useful? For anyone finding this bug and looking for a temporary workaround, do not use '-Os' when compiling secure code for trustzone. gcc args to recreate issue: -mcmse -mcpu=cortex-m23 -Os -c test.c gcc version: 9.3.1 20200408 (release) (GNU Arm Embedded Toolchain 9-2020-q2-update) Target: arm-none-eabi Configured with: /mnt/workspace/workspace/GCC-9-pipeline/jenkins-GCC-9-pipeline-200_20200521_1590053374/src/gcc/configure --target=arm-none-eabi --prefix=/mnt/workspace/workspace/GCC-9-pipeline/jenkins-GCC-9-pipeline-200_20200521_1590053374/install-native --libexecdir=/mnt/workspace/workspace/GCC-9-pipeline/jenkins-GCC-9-pipeline-200_20200521_1590053374/install-native/lib --infodir=/mnt/workspace/workspace/GCC-9-pipeline/jenkins-GCC-9-pipeline-200_20200521_1590053374/install-native/share/doc/gcc-arm-none-eabi/info --mandir=/mnt/workspace/workspace/GCC-9-pipeline/jenkins-GCC-9-pipeline-200_20200521_1590053374/install-native/share/doc/gcc-arm-none-eabi/man --htmldir=/mnt/workspace/workspace/GCC-9-pipeline/jenkins-GCC-9-pipeline-200_20200521_1590053374/install-native/share/doc/gcc-arm-none-eabi/html --pdfdir=/mnt/workspace/workspace/GCC-9-pipeline/jenkins-GCC-9-pipeline-200_20200521_1590053374/install-native/share/doc/gcc-arm-none-eabi/pdf --enable-languages=c,c++ --enable-plugins --disable-decimal-float --disable-libffi --disable-libgomp --disable-libmudflap --disable-libquadmath --disable-libssp --disable-libstdcxx-pch --disable-nls --disable-shared --disable-threads --disable-tls --with-gnu-as --with-gnu-ld --with-newlib --with-headers=yes --with-python-dir=share/gcc-arm-none-eabi --with-sysroot=/mnt/workspace/workspace/GCC-9-pipeline/jenkins-GCC-9-pipeline-200_20200521_1590053374/install-native/arm-none-eabi --build=x86_64-linux-gnu --host=x86_64-linux-gnu --with-gmp=/mnt/workspace/workspace/GCC-9-pipeline/jenkins-GCC-9-pipeline-200_20200521_1590053374/build-native/host-libs/usr --with-mpfr=/mnt/workspace/workspace/GCC-9-pipeline/jenkins-GCC-9-pipeline-200_20200521_1590053374/build-native/host-libs/usr --with-mpc=/mnt/workspace/workspace/GCC-9-pipeline/jenkins-GCC-9-pipeline-200_20200521_1590053374/build-native/host-libs/usr --with-isl=/mnt/workspace/workspace/GCC-9-pipeline/jenkins-GCC-9-pipeline-200_20200521_1590053374/build-native/host-libs/usr --with-libelf=/mnt/workspace/workspace/GCC-9-pipeline/jenkins-GCC-9-pipeline-200_20200521_1590053374/build-native/host-libs/usr --with-host-libstdcxx='-static-libgcc -Wl,-Bstatic,-lstdc++,-Bdynamic -lm' --with-pkgversion='GNU Arm Embedded Toolchain 9-2020-q2-update' --with-multilib-list=rmprofile,aprofile (also tested and issue still exists with gcc version 7.3.1, 7.2.1, 8.3.1, 9.2.1)