Bug 95646 - [GCC 9/10] arm-none-eabi function attribute 'cmse_nonsecure_entry' wipes register values with -Os
Summary: [GCC 9/10] arm-none-eabi function attribute 'cmse_nonsecure_entry' wipes regi...
Status: ASSIGNED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 9.3.1
: P3 normal
Target Milestone: ---
Assignee: avieira
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2020-06-11 17:42 UTC by Ciaran Woodward
Modified: 2021-05-06 09:51 UTC (History)
5 users (show)

See Also:
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 (79 bytes, text/plain)
2020-06-11 17:42 UTC, Ciaran Woodward
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ciaran Woodward 2020-06-11 17:42:48 UTC
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)
Comment 1 avieira 2020-06-15 12:57:57 UTC
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.
Comment 2 CVS Commits 2020-06-23 14:24:53 UTC
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.
Comment 3 CVS Commits 2020-09-15 10:25:31 UTC
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.
Comment 4 avieira 2021-01-25 11:05:53 UTC
Changed title to reflect that  this still needs backports to GCC 9 and 10.
Comment 5 xinyu.zhang 2021-01-26 07:53:38 UTC
Could this bug be fixed in next version? If so, when could the new version be released?
Comment 6 CVS Commits 2021-05-06 09:49:53 UTC
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)
Comment 7 CVS Commits 2021-05-06 09:51:49 UTC
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)