Re: [PATCH v5 8/8] newlib: libc: setjmp M-profile PACBTI-enablement

On 21/12/2022 11:42, 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/setjmp.S | 39 ++++++++++++++++++++++++++++++++
>   1 file changed, 39 insertions(+)
> 
> diff --git a/newlib/libc/machine/arm/setjmp.S b/newlib/libc/machine/arm/setjmp.S
> index d814afea8..3e4d7cb70 100644
> --- a/newlib/libc/machine/arm/setjmp.S
> +++ b/newlib/libc/machine/arm/setjmp.S
> @@ -155,6 +155,8 @@ SYM (.arm_start_of.\name):
>   	.align 2
>   	MODE
>   	.globl SYM (\name)
> +	.fnstart
> +	.cfi_startproc
>   	TYPE (\name)
>   SYM (\name):
>   	PROLOGUE \name
> @@ -162,6 +164,8 @@ SYM (\name):
>   
>   .macro FUNC_END name
>   	RET
> +	.cfi_endproc
> +	.fnend
>   	SIZE (\name)
>   .endm
>   
> @@ -171,6 +175,21 @@ SYM (\name):
>   
>   	FUNC_START setjmp
>   
> +#if __ARM_FEATURE_PAC_DEFAULT
> +# if __ARM_FEATURE_BTI_DEFAULT
> +	pacbti	ip, lr, sp
> +# else
> +	pac	ip, lr, sp
> +# endif /* __ARM_FEATURE_BTI_DEFAULT */
> +	mov r3, ip
> +	str r3, [r0, #104]

#104 here is a bit obscure.  I think it would be clearer to write 
something like

	str r3, [r0, #(CORE_REGS_SAVE_SIZE + FP_REGS_SAVE_SIZE)]

and then define these as appropriate.

> +	.cfi_register 143, 12
> +#else
> +# if __ARM_FEATURE_BTI_DEFAULT
> +	bti
> +# endif /* __ARM_FEATURE_BTI_DEFAULT */
> +#endif /* __ARM_FEATURE_PAC_DEFAULT */
> +
>   	/* Save all the callee-preserved registers into the jump buffer.  */
>   #ifdef __thumb2__
>   	mov		ip, sp
> @@ -184,6 +203,10 @@ SYM (\name):
>   
>   	/* When setting up the jump buffer return 0.  */
>   	mov		r0, #0
> +#if __ARM_FEATURE_PAC_DEFAULT
> +	mov ip, r3
> +	aut ip, lr, sp
> +#endif /* __ARM_FEATURE_PAC_DEFAULT */
>   
>   	FUNC_END setjmp
>   
> @@ -193,6 +216,16 @@ SYM (\name):
>   
>   	FUNC_START longjmp
>   
> +#if __ARM_FEATURE_BTI_DEFAULT
> +	bti
> +#endif /* __ARM_FEATURE_BTI_DEFAULT */
> +
> +#if __ARM_FEATURE_PAC_DEFAULT
> +	/* Keep original jmpbuf address for retrieving pac-code
> +	   for authentication.  */
> +	mov	r2, r0
> +#endif /* __ARM_FEATURE_PAC_DEFAULT */
> +
>   	/* If we have stack extension code it ought to be handled here.  */
>   
>   	/* Restore the registers, retrieving the state when setjmp() was called.  */
> @@ -212,5 +245,11 @@ SYM (\name):
>   	it		eq
>   	moveq		r0, #1
>   
> +#if __ARM_FEATURE_PAC_DEFAULT
> +	ldr r3, [r2, #104]
> +	mov ip, r3

See above.  Also, you don't need to load into r3 and then move to IP, 
just load ip directly.

> +	aut ip, lr, sp
> +#endif /* __ARM_FEATURE_PAC_DEFAULT */
> +
>   	FUNC_END longjmp
>   #endif

R.