On 07/02/2024 07:59, Tejas Belagod wrote:
> This patch fixes a bug that causes indirect calls in PAC-enabled functions
> to be tailcalled incorrectly when all argument registers R0-R3 are used.
>
> Tested on arm-none-eabi for armv8.1-m.main. OK for trunk?
>
> 2024-02-07 Tejas Belagod <tejas.bela...@arm.com>
>
> PR target/113780
> * gcc/config/arm.cc (arm_function_ok_for_sibcall): Don't allow tailcalls
> for indirect calls with 4 or more arguments in pac-enabled functions.
>
> * gcc.target/arm/pac-sibcall.c: New.
> ---
> gcc/config/arm/arm.cc | 12 ++++++++----
> gcc/testsuite/gcc.target/arm/pac-sibcall.c | 11 +++++++++++
> 2 files changed, 19 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/arm/pac-sibcall.c
>
> diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc
> index c44047c377a..c1f8286a4d4 100644
> --- a/gcc/config/arm/arm.cc
> +++ b/gcc/config/arm/arm.cc
> @@ -7980,10 +7980,14 @@ arm_function_ok_for_sibcall (tree decl, tree exp)
> && DECL_WEAK (decl))
> return false;
>
> - /* We cannot do a tailcall for an indirect call by descriptor if all the
> - argument registers are used because the only register left to load the
> - address is IP and it will already contain the static chain. */
> - if (!decl && CALL_EXPR_BY_DESCRIPTOR (exp) && !flag_trampolines)
> + /* We cannot do a tailcall for an indirect call by descriptor or for an
> + indirect call in a pac-enabled function if all the argument registers
> + are used because the only register left to load the address is IP and
> + it will already contain the static chain or the PAC signature in the
> + case of PAC-enabled functions. */
This comment is becoming a bit unwieldy. I suggest restructuring it as:
We cannot tailcall an indirect call by descriptor if all the call-clobbered
general registers are live (r0-r3 and ip). This can happen when:
- IP contains the static chain, or
- IP is needed for validating the PAC signature.
> + if (!decl
> + && ((CALL_EXPR_BY_DESCRIPTOR (exp) && !flag_trampolines)
> + || arm_current_function_pac_enabled_p()))
> {
> tree fntype = TREE_TYPE (TREE_TYPE (CALL_EXPR_FN (exp)));
> CUMULATIVE_ARGS cum;
> diff --git a/gcc/testsuite/gcc.target/arm/pac-sibcall.c
> b/gcc/testsuite/gcc.target/arm/pac-sibcall.c
> new file mode 100644
> index 00000000000..c57bf7a952c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/pac-sibcall.c
> @@ -0,0 +1,11 @@
> +/* Testing return address signing. */
> +/* { dg-do compile } */
> +/* { dg-require-effective-target mbranch_protection_ok } */
> +/* { dg-options " -mcpu=cortex-m85 -mbranch-protection=pac-ret+leaf -O2" } */
No, you can't just add options like this, you need to first check that they
won't result in conflicts with other options on the command line. See
https://gcc.gnu.org/pipermail/gcc-patches/2024-January/644077.html for an
example of how to handle this.
> +
> +void fail(void (*f)(int, int, int, int))
> +{
> + f(1, 2, 3, 4);
> +}
> +
> +/* { dg-final { scan-assembler-not "bx\tip\t@ indirect register sibling
> call" } } */
R.
