[Bug target/95646] New: arm-none-eabi function attribute 'cmse_nonsecure_entry' wipes register values with -Os

[Bug target/95646] New: arm-none-eabi function attribute 'cmse_nonsecure_entry' wipes register values with -Os

[c.woodward at cascoda dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95646

            Bug ID: 95646
           Summary: arm-none-eabi function attribute
                    'cmse_nonsecure_entry' wipes register values with -Os
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: c.woodward at cascoda dot com
  Target Milestone: ---

Created attachment 48721
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48721&action=edit
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)

[Bug target/95646] arm-none-eabi function attribute 'cmse_nonsecure_entry' wipes register values with -Os

[avieira at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95646

avieira at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2020-06-15
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from avieira at gcc dot gnu.org ---
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.

[Bug target/95646] arm-none-eabi function attribute 'cmse_nonsecure_entry' wipes register values with -Os

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95646

--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
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.

[Bug target/95646] arm-none-eabi function attribute 'cmse_nonsecure_entry' wipes register values with -Os

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95646

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
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.

[Bug target/95646] [GCC 9/10] arm-none-eabi function attribute 'cmse_nonsecure_entry' wipes register values with -Os

[avieira at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95646

avieira at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|arm-none-eabi function      |[GCC 9/10] arm-none-eabi
                   |attribute                   |function attribute
                   |'cmse_nonsecure_entry'      |'cmse_nonsecure_entry'
                   |wipes register values with  |wipes register values with
                   |-Os                         |-Os

--- Comment #4 from avieira at gcc dot gnu.org ---
Changed title to reflect that  this still needs backports to GCC 9 and 10.

[Bug target/95646] [GCC 9/10] arm-none-eabi function attribute 'cmse_nonsecure_entry' wipes register values with -Os

[xinyu.zhang at arm dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95646

xinyu.zhang at arm dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |xinyu.zhang at arm dot com

--- Comment #5 from xinyu.zhang at arm dot com ---
Could this bug be fixed in next version? If so, when could the new version be
released?

[Bug target/95646] [GCC 9/10] arm-none-eabi function attribute 'cmse_nonsecure_entry' wipes register values with -Os

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95646

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

[Bug target/95646] [GCC 9/10] arm-none-eabi function attribute 'cmse_nonsecure_entry' wipes register values with -Os

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95646

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
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)

[Bug target/95646] [GCC 9/10] arm-none-eabi function attribute 'cmse_nonsecure_entry' wipes register values with -Os

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95646

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
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)