Re: [PATCH 3/8] newlib: libc: strlen M-profile PACBTI-enablement

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

On 05/07/2022 14:58, Victor Do Nascimento wrote:
> Add function prologue/epilogue to conditionally add BTI landing pads
> and/or PAC code generation & authentication instructions depending on
> compilation flags.
> 
> This patch enables PACBTI for all relevant variants of strlen:
>       * Newlib for armv8.1-m.main+pacbti
>       * Newlib for armv8.1-m.main+pacbti+mve
>       * Newlib-nano
> ---
>   newlib/libc/machine/arm/strlen-armv7.S     | 52 ++++++++++++++++++++--
>   newlib/libc/machine/arm/strlen-stub.c      |  9 ++++
>   newlib/libc/machine/arm/strlen-thumb2-Os.S | 13 ++++--
>   3 files changed, 67 insertions(+), 7 deletions(-)
> 
> diff --git a/newlib/libc/machine/arm/strlen-armv7.S b/newlib/libc/machine/arm/strlen-armv7.S
> index f3dda0d60..18c8226d0 100644
> --- a/newlib/libc/machine/arm/strlen-armv7.S
> +++ b/newlib/libc/machine/arm/strlen-armv7.S
> @@ -59,6 +59,7 @@
>      OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.  */
>   
>   #include "acle-compat.h"
> +#include "pacbti.h"
>   
>   	.macro def_fn f p2align=0
>   	.text
> @@ -77,7 +78,9 @@
>   #endif
>   
>   	/* This code requires Thumb.  */
> -#if __ARM_ARCH_PROFILE == 'M'
> +#if __ARM_ARCH_8M_MAIN__

These GCC architecture macros (those that end with '__') aren't portable 
and are essentially deprecated.  What exactly are you trying to achieve 
here?

> +    /* keep config inherited from -march= */
> +#elif __ARM_ARCH_PROFILE == 'M'
>   	.arch   armv7e-m
>   #else
>   	.arch	armv6t2
> @@ -100,8 +103,34 @@
>   #define tmp2		r5
>   
>   def_fn	strlen p2align=6
> +	.fnstart
> +	.cfi_startproc
> +	/* common pacbti_prologue macro from pacbti.h not used.
> +	   handwritten prologue saves one push instruction. */
> +#if __ARM_FEATURE_PAC_DEFAULT
> +#if __ARM_FEATURE_BTI_DEFAULT
> +	pacbti ip, lr, sp
> +#else
> +	pac ip, lr, sp
> +#endif /* __ARM_FEATURE_BTI_DEFAULT */
> +	push    {r4, r5, ip}
> +	.save   {r4, r5, ra_auth_code}
> +	.cfi_def_cfa_offset 12
> +	.cfi_offset 143, -4
> +	.cfi_offset 5, -8
> +	.cfi_offset 4, -12
> +
> +#else
> +#if __ARM_FEATURE_BTI_DEFAULT
> +	bti
> +#endif /* __ARM_FEATURE_BTI_DEFAULT */
> +	push    {r4, r5}
> +	.save   {r4, r5}
> +	.cfi_def_cfa_offset 8
> +	.cfi_offset 5, -4
> +	.cfi_offset 4, -8
> +#endif /* __ARM_FEATURE_PAC_DEFAULT */
>   	pld	[srcin, #0]
> -	strd	r4, r5, [sp, #-8]!
>   	bic	src, srcin, #7
>   	mvn	const_m1, #0
>   	ands	tmp1, srcin, #7		/* (8 - bytes) to alignment.  */
> @@ -159,9 +188,22 @@ def_fn	strlen p2align=6
>   	rev	data1a, data1a
>   #endif
>   	clz	data1a, data1a
> -	ldrd	r4, r5, [sp], #8
>   	add	result, result, data1a, lsr #3	/* Bits -> Bytes.  */
> -	bx	lr
> +#if __ARM_FEATURE_PAC_DEFAULT
> +	pop	{r4, r5, ip}
> +	.cfi_restore 4
> +	.cfi_restore 5
> +	.cfi_restore 143
> +	.cfi_def_cfa_offset 0
> +	aut ip, lr, sp
> +#else
> +	ldrd	r4, r5, [sp], #8
> +	.cfi_restore 4
> +	.cfi_restore 5
> +	.cfi_def_cfa_offset 0
> +#endif /* __ARM_FEATURE_PAC_DEFAULT */
> +	bx lr
> +
>   
>   .Lmisaligned8:
>   	ldrd	data1a, data1b, [src]
> @@ -177,4 +219,6 @@ def_fn	strlen p2align=6
>   	movne	data1a, const_m1
>   	mov	const_0, #0
>   	b	.Lstart_realigned
> +	.cfi_endproc
> +	.fnend
>   	.size	strlen, . - strlen
> diff --git a/newlib/libc/machine/arm/strlen-stub.c b/newlib/libc/machine/arm/strlen-stub.c
> index fc2daf16f..4a0bb8cbb 100644
> --- a/newlib/libc/machine/arm/strlen-stub.c
> +++ b/newlib/libc/machine/arm/strlen-stub.c
> @@ -58,6 +58,11 @@ strlen (const char* str)
>          "data .req r3\n\t"
>          "addr .req r1\n\t"
>   
> +#ifdef __ARM_FEATURE_PAC_DEFAULT
> +       "pac ip, lr, sp\n\t"
> +       "str ip, [sp, #-4]!\n\t"
> +#endif
> +
>   #ifdef _ISA_ARM_7
>          "pld [r0]\n\t"
>   #endif
> @@ -167,6 +172,10 @@ strlen (const char* str)
>          "it	ne\n\t"
>          "addne	len, len, #1\n\t"
>   # endif
> +#endif
> +#ifdef __ARM_FEATURE_PAC_DEFAULT
> +       "ldr ip, [sp], #4\n\t"
> +       "aut ip, lr, sp\n\t"
>   #endif
>          "bx	lr\n\t");
>   }
> diff --git a/newlib/libc/machine/arm/strlen-thumb2-Os.S b/newlib/libc/machine/arm/strlen-thumb2-Os.S
> index 961f41a0a..823b0310e 100644
> --- a/newlib/libc/machine/arm/strlen-thumb2-Os.S
> +++ b/newlib/libc/machine/arm/strlen-thumb2-Os.S
> @@ -25,6 +25,7 @@
>      OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.  */
>   
>   #include "acle-compat.h"
> +#include "pacbti.h"
>   
>   	.macro def_fn f p2align=0
>   	.text
> @@ -33,8 +34,9 @@
>   	.type \f, %function
>   \f:
>   	.endm
> -
> -#if __ARM_ARCH_ISA_THUMB >= 2 && __ARM_ARCH >= 7
> +#if __ARM_ARCH_8M_MAIN__
> +	/* keep config inherited from -march= */
> +#elif __ARM_ARCH_ISA_THUMB >= 2 && __ARM_ARCH >= 7
>   	.arch   armv7
>   #else
>   	.arch	armv6t2
> @@ -44,11 +46,16 @@
>   	.syntax unified
>   
>   def_fn	strlen p2align=1
> +	.fnstart
> +	.cfi_startproc
> +	pacbti_prologue
>   	mov     r3, r0
>   1:	ldrb.w  r2, [r3], #1
>   	cmp     r2, #0
>   	bne	1b
>   	subs    r0, r3, r0
>   	subs    r0, #1
> -	bx      lr
> +	pacbti_epilogue
> +	.cfi_endproc
> +	.fnend
>   	.size	strlen, . - strlen