Re: [PATCH v4 5/8] newlib: libc: memcpy M-profile PACBTI-enablement

[Richard Earnshaw <Richard.Earnshaw@foss.arm.com>]

On 26/10/2022 12:50, Victor L. Do Nascimento wrote:
> Add function prologue/epilogue to conditionally add BTI landing pads
> and/or PAC code generation & authentication instructions depending on
> compilation flags.
> ---
>   newlib/libc/machine/arm/memcpy-armv7m.S | 37 +++++++++++++++++--------
>   1 file changed, 26 insertions(+), 11 deletions(-)
> 
> diff --git a/newlib/libc/machine/arm/memcpy-armv7m.S b/newlib/libc/machine/arm/memcpy-armv7m.S
> index c8bff36f6..a74bacc97 100644
> --- a/newlib/libc/machine/arm/memcpy-armv7m.S
> +++ b/newlib/libc/machine/arm/memcpy-armv7m.S
> @@ -46,6 +46,8 @@
>        __OPT_BIG_BLOCK_SIZE: Size of big block in words.  Default to 64.
>        __OPT_MID_BLOCK_SIZE: Size of big block in words.  Default to 16.
>    */
> +#include "arm_asm.h"
> +
>   #ifndef __OPT_BIG_BLOCK_SIZE
>   #define __OPT_BIG_BLOCK_SIZE (4 * 16)
>   #endif
> @@ -85,6 +87,8 @@
>   	.global	memcpy
>   	.thumb
>   	.thumb_func
> +	.fnstart
> +	.cfi_startproc
>   	.type	memcpy, %function
>   memcpy:
>   	@ r0: dst
> @@ -93,10 +97,11 @@ memcpy:
>   #ifdef __ARM_FEATURE_UNALIGNED
>   	/* In case of UNALIGNED access supported, ip is not used in
>   	   function body.  */
> +	prologue push_ip=HAVE_PAC_LEAF
>   	mov	ip, r0
>   #else
> -	push	{r0}
> -#endif
> +	prologue 0 push_ip=HAVE_PAC_LEAF
> +#endif /* __ARM_FEATURE_UNALIGNED */
>   	orr	r3, r1, r0
>   	ands	r3, r3, #3
>   	bne	.Lmisaligned_copy
> @@ -135,13 +140,13 @@ memcpy:
>   	ldr	r3, [r1], #4
>   	str	r3, [r0], #4
>   	END_UNROLL
> -#else /* __ARM_ARCH_7M__ */
> +#else

Why this change?

>   	ldr	r3, [r1, \offset]
>   	str	r3, [r0, \offset]
>   	END_UNROLL
>   	adds    r0, __OPT_MID_BLOCK_SIZE
>   	adds    r1, __OPT_MID_BLOCK_SIZE
> -#endif
> +#endif /* __ARM_ARCH_7M__ */

And this?

Please put them back as they were.

[Just for the record, this test is using a deprecated GCC extension and 
needs rewriting using the appropriate feature tests from ACLE; but 
that's not something you need to fix.]

Otherwise OK.

R.

>   	subs	r2, __OPT_MID_BLOCK_SIZE
>   	bhs	.Lmid_block_loop
>   
> @@ -178,15 +183,17 @@ memcpy:
>   #endif /* __ARM_FEATURE_UNALIGNED */
>   
>   .Ldone:
> +	.cfi_remember_state
>   #ifdef __ARM_FEATURE_UNALIGNED
>   	mov	r0, ip
> +	epilogue push_ip=HAVE_PAC_LEAF
>   #else
> -	pop	{r0}
> -#endif
> -	bx	lr
> +	epilogue 0 push_ip=HAVE_PAC_LEAF
> +#endif /*  __ARM_FEATURE_UNALIGNED */
>   
>   	.align 2
>   .Lmisaligned_copy:
> +	.cfi_restore_state
>   #ifdef __ARM_FEATURE_UNALIGNED
>   	/* Define label DST_ALIGNED to BIG_BLOCK.  It will go to aligned copy
>   	   once destination is adjusted to aligned.  */
> @@ -247,6 +254,9 @@ memcpy:
>   	/* dst is aligned, but src isn't.  Misaligned copy.  */
>   
>   	push	{r4, r5}
> +	.cfi_adjust_cfa_offset 8
> +	.cfi_rel_offset 4, 0
> +	.cfi_rel_offset 5, 4
>   	subs	r2, #4
>   
>   	/* Backward r1 by misaligned bytes, to make r1 aligned.
> @@ -299,6 +309,9 @@ memcpy:
>   	adds	r2, #4
>   	subs	r1, ip
>   	pop	{r4, r5}
> +	.cfi_restore 4
> +	.cfi_restore 5
> +	.cfi_adjust_cfa_offset -8
>   
>   #endif /* __ARM_FEATURE_UNALIGNED */
>   
> @@ -321,9 +334,11 @@ memcpy:
>   
>   #ifdef __ARM_FEATURE_UNALIGNED
>   	mov	r0, ip
> +	epilogue push_ip=HAVE_PAC_LEAF
>   #else
> -	pop	{r0}
> -#endif
> -	bx	lr
> -
> +	epilogue 0 push_ip=HAVE_PAC_LEAF
> +#endif /* __ARM_FEATURE_UNALIGNED */
> +	.cfi_endproc
> +	.cantunwind
> +	.fnend
>   	.size	memcpy, .-memcpy