[PATCH v5 1/8] newlib: libc: define M-profile PACBTI-enablement macros

Christophe Lyon christophe.lyon@arm.com
Fri Jan 6 10:42:02 GMT 2023


Hi Victor,

Thanks for the patch series, a few comments/questions below.

Christophe


On 12/21/22 12:19, Victor L. Do Nascimento wrote:
> Augment the arm_asm.h header file to simplify function prologues and
> epilogues whilst adding support for PACBTI enablement via macros for
> hand-written assembly functions.  For PACBTI, both prologues/epilogues
> as well as cfi-related directives are automatically amended
> accordingly, depending on the compile-time mbranch-protection argument
> values.
> 
> It defines the following preprocessor macros:
>     * HAVE_PAC_LEAF: Indicates whether pac-signing has been requested for
>     leaf functions.
>     * PAC_LEAF_PUSH_IP: Whether leaf functions should push the pac code
>     to the stack irrespective of whether the ip register is clobbered in
>     the function or not.
>     * STACK_ALIGN_ENFORCE: Whether a dummy register should be added to
>     the push list as necessary in the prologue to ensure stack
>     alignment preservation at the start of assembly function.  The
>     epilogue behavior is likewise affected by this flag, ensuring any
>     pushed dummy registers also get popped on function return.
IIUC, these new macros are meant for general usage outside of newlib, do 
they need proper documentation? Or maybe an entry in the "News" section? 
I don't know. Otherwise, I think they should not appear in the user 
naming space.


> It also defines the following assembler macros:
>     * prologue: In addition to pushing any callee-saved registers onto
>     the stack, it generates any requested pacbti instructions.
>     Pushed registers are specified via the optional `first', `last',
>     `push_ip' and `push_lr' macro argument parameters.
Maybe you should quote 'first' and 'last' differently from 'push_ip' and 
'push_lr', since the example below shows that 'first' and 'last' are in 
fact register numbers (IIUC)

>     when a single register number is provided, it pushes that
Typo: "When" (with a capital)

>     register.  When two register numbers are provided, they specify a
>     rage to save.  If push_ip and/or push_lr are non-zero, the
Typo: "range"

>     respective registers are also saved.  Stack alignment is requested
>     via the `align` argument, which defaults to the value of
>     STACK_ALIGN_ENFORCE, unless manually overridden.
> 
>     For example:
> 
>         prologue push_ip=1 -> push {ip}
>         prologue push_ip=1, align8=1 -> push {r2, ip}
>         prologue push_ip=1, push_lr=1 -> push {ip, lr}
>         prologue 1 -> push {r1}
>         prologue 1, align8=1 -> push {r0, r1}
>         prologue 1 push_ip=1 -> push {r1, ip}
>         prologue 1 4 -> push {r1-r4}
>         prologue 1 4 push_ip=1 -> push {r1-r4, ip}
can you include an example with pacbti?


>     * epilogue: pops registers off the stack and emits pac key signing
>     instruction, if requested. The `first', `last', `push_ip',
>     `push_lr' and `align' function as per the prologue macro,
>     generating pop instead of push instructions.
> 
>     Stack alignment is enforced via the following helper macro
>     call-chain:
> 
> 	{prologue|epilogue} ->_align8 -> _preprocess_reglist ->
> 	  _preprocess_reglist1 -> {_prologue|_epilogue}
> 
>     Finally, the necessary cfi directives for adding debug information
>     to prologue and epilogue are generated via the following macros:
> 
>     * cfisavelist - prologue macro helper function, generating
>     necessary .cfi_offset directives associated with push instruction.
>     Therefore, the net effect of calling `prologue 1 2 push_ip=1' is
>     to generate the following:
> 
>         push {r1-r2, ip}
>         .cfi_adjust_cfa_offset 12
>         .cfi_offset 143, -4
>         .cfi_offset 2, -8
>         .cfi_offset 1, -12
> 
>     * cfirestorelist - epilogue macro helper function, emitting
>     .cfi_restore instructions prior to resetting the cfa offset.  As
>     such, calling `epilogue 1 2 push_ip=1' will produce:
> 
>          pop {r1-r2, ip}
> 	.cfi_register 143, 12
> 	.cfi_restore 2
> 	.cfi_restore 1
> 	.cfi_def_cfa_offset 0
> ---
>   newlib/libc/machine/arm/arm_asm.h | 441 ++++++++++++++++++++++++++++++
>   1 file changed, 441 insertions(+)
> 
> diff --git a/newlib/libc/machine/arm/arm_asm.h b/newlib/libc/machine/arm/arm_asm.h
> index 2708057de..94fa77b4d 100644
> --- a/newlib/libc/machine/arm/arm_asm.h
> +++ b/newlib/libc/machine/arm/arm_asm.h
> @@ -60,4 +60,445 @@
>   # define _ISA_THUMB_1
>   #endif
>   
> +/* Check whether leaf function PAC signing has been requested in the
> +   -mbranch-protect compile-time option.  */
> +#define LEAF_PROTECT_BIT 2
Shouldn't this start with '__' or be #undefed at the end of this file to 
avoid polluting user naming space? (I noticed it's not used outside this 
file in this patch series)

> +
> +#ifdef __ARM_FEATURE_PAC_DEFAULT
> +# define HAVE_PAC_LEAF \
> +	((__ARM_FEATURE_PAC_DEFAULT & (1 << LEAF_PROTECT_BIT)) && 1)
> +#else
> +# define HAVE_PAC_LEAF 0
> +#endif
> +
> +/* Provide default parameters for PAC-code handling in leaf-functions.  */
> +#if HAVE_PAC_LEAF
> +# ifndef PAC_LEAF_PUSH_IP
> +#  define PAC_LEAF_PUSH_IP 1
> +# endif
> +#else /* !HAVE_PAC_LEAF */
> +# undef PAC_LEAF_PUSH_IP
> +# define PAC_LEAF_PUSH_IP 0
> +#endif /* HAVE_PAC_LEAF */
> +
> +#define STACK_ALIGN_ENFORCE 0
> +
> +#ifdef __ASSEMBLER__
> +
> +/******************************************************************************
> +* Implementation of the prologue and epilogue assembler macros and their
> +* associated helper functions.
> +*
> +* These functions add support for the following:
> +*
> +* - M-profile branch target identification (BTI) landing-pads when compiled
> +*   with `-mbranch-protection=bti'.
> +* - PAC-signing and verification instructions, depending on hardware support
> +*   and whether the PAC-signing of leaf functions has been requested via the
> +*   `-mbranch-protection=pac-ret+leaf' compiler argument.
> +* - 8-byte stack alignment preservation at function entry, defaulting to the
> +*   value of STACK_ALIGN_ENFORCE.
> +*
> +* Notes:
> +* - Prologue stack alignment is implemented by detecting a push with an odd
> +*   number of registers and prepending a dummy register to the list.
> +* - If alignment is attempted on a list containing r0, compilation will result
> +*   in an error.
> +* - If alignment is attempted in a list containing r1, r0 will be prepended to
> +*   the register list and r0 will be restored prior to function return.  for
> +*   functions with non-void return types, this will result in the corruption of
> +*   the result register.
> +* - Stack alignment is enforced via the following helper macro call-chain:
> +*
> +*	{prologue|epilogue} ->_align8 -> _preprocess_reglist ->
> +*		_preprocess_reglist1 -> {_prologue|_epilogue}
> +*
> +* - Debug CFI directives are automatically added to prologues and epilogues,
> +*   assisted by `cfisavelist' and `cfirestorelist', respectively.
> +*
> +* Arguments:
> +* prologue
> +* --------
> +* - first	- If `last' specified, this serves as start of general-purpose
> +*		  register (GPR) range to push onto stack, otherwise represents
> +*		  single GPR to push onto stack.  If omitted, no GPRs pushed
> +*		  onto stack at prologue.
> +* - last	- If given, specifies inclusive upper-bound of GPR range.
> +* - push_ip	- Determines whether IP register is to be pushed to stack at
> +*		  prologue.  When pac-signing is requested, this holds the
> +*		  the pac-key.  Either 1 or 0 to push or not push, respectively.
> +*		  Default behavior: Set to value of PAC_LEAF_PUSH_IP macro.
> +* - push_lr	- Determines whether to push lr to the stack on function entry.
> +*		  Either 1 or 0  to push or not push, respectively.
> +* - align8	- Whether to enforce alignment. Either 1 or 0, with 1 requesting
> +*		  alignment.
> +*
> +* epilogue
> +* --------
> +*   The epilogue should be called passing the same arguments as those passed to
> +*   the prologue to ensure the stack is not corrupted on function return.
> +*
> +* Usage examples:
> +*
> +*   prologue push_ip=1 -> push {ip}
> +*   epilogue push_ip=1, align8=1 -> pop {r2, ip}
> +*   prologue push_ip=1, push_lr=1 -> push {ip, lr}
> +*   epilogue 1 -> pop {r1}
> +*   prologue 1, align8=1 -> push {r0, r1}
> +*   epilogue 1, push_ip=1 -> pop {r1, ip}
> +*   prologue 1, 4 -> push {r1-r4}
> +*   epilogue 1, 4 push_ip=1 -> pop {r1-r4, ip}
> +*
> +******************************************************************************/
> +
> +/* Emit .cfi_restore directives for a consecutive sequence of registers.  */
> +	.macro cfirestorelist first, last
> +	.cfi_restore \last
> +	.if \last-\first
> +	 cfirestorelist \first, \last-1
> +	.endif
> +	.endm
> +
> +/* Emit .cfi_offset directives for a consecutive sequence of registers.  */
> +	.macro cfisavelist first, last, index=1
> +	.cfi_offset \last, -4*(\index)
> +	.if \last-\first
> +	 cfisavelist \first, \last-1, \index+1
> +	.endif
> +	.endm
> +
> +.macro _prologue first=-1, last=-1, push_ip=PAC_LEAF_PUSH_IP, push_lr=0
> +	.if \push_ip & 1 != \push_ip
> +	 .error "push_ip may be either 0 or 1"
> +	.endif
> +	.if \push_lr & 1 != \push_lr
> +	 .error "push_lr may be either 0 or 1"
> +	.endif
> +	.if \first != -1
> +	 .if \last == -1
> +	  /* Upper-bound not provided: Set upper = lower.  */
> +	  _prologue \first, \first, \push_ip, \push_lr
> +	  .exitm
> +	 .endif
> +	.endif
> +#if HAVE_PAC_LEAF
> +#if __ARM_FEATURE_BTI_DEFAULT
> +	pacbti	ip, lr, sp
> +#else
> +	pac	ip, lr, sp
> +#endif /* __ARM_FEATURE_BTI_DEFAULT */
> +	.cfi_register 143, 12
> +#else
> +#if __ARM_FEATURE_BTI_DEFAULT
> +	bti
> +#endif /* __ARM_FEATURE_BTI_DEFAULT */
> +#endif /* HAVE_PAC_LEAF */
> +	.if \first != -1
> +	 .if \last != \first
> +	  .if \last >= 13
> +	.error "SP cannot be in the save list"
> +	  .endif
I think you should also check that IP (r12) is not in the range, 
otherwise I think nothing prevents from doing
prologue 12, push_ip=1 which will result in emitting push {r12, ip}
(I suppose gas would complain?)
.... scratch that, I saw later that this sanity checking is performed in 
_preprocess_reglist1 :-)


> +	  .if \push_ip
> +	   .if \push_lr
> +	/* Case 1: push register range, ip and lr registers.  */
> +	push {r\first-r\last, ip, lr}
> +	.cfi_adjust_cfa_offset ((\last-\first)+3)*4
> +	.cfi_offset 14, -4
> +	.cfi_offset 143, -8
> +	cfisavelist \first, \last, 3
> +	   .else // !\push_lr
> +	/* Case 2: push register range and ip register.  */
> +	push {r\first-r\last, ip}
> +	.cfi_adjust_cfa_offset ((\last-\first)+2)*4
> +	.cfi_offset 143, -4
> +	cfisavelist \first, \last, 2
> +	   .endif
> +	  .else // !\push_ip
> +	   .if \push_lr
> +	/* Case 3: push register range and lr register.  */
> +	push {r\first-r\last, lr}
> +	.cfi_adjust_cfa_offset ((\last-\first)+2)*4
> +	.cfi_offset 14, -4
> +	cfisavelist \first, \last, 2
> +	   .else // !\push_lr
> +	/* Case 4: push register range.  */
> +	push {r\first-r\last}
> +	.cfi_adjust_cfa_offset ((\last-\first)+1)*4
> +	cfisavelist \first, \last, 1
> +	   .endif
> +	  .endif
> +	 .else // \last == \first
> +	  .if \push_ip
> +	   .if \push_lr
> +	/* Case 5: push single GP register plus ip and lr registers.  */
> +	push {r\first, ip, lr}
> +	.cfi_adjust_cfa_offset 12
> +	.cfi_offset 14, -4
> +	.cfi_offset 143, -8
> +        cfisavelist \first, \first, 3
> +	   .else // !\push_lr
> +	/* Case 6: push single GP register plus ip register.  */
> +	push {r\first, ip}
> +	.cfi_adjust_cfa_offset 8
> +	.cfi_offset 143, -4
> +        cfisavelist \first, \first, 2
> +	   .endif
> +	  .else // !\push_ip
> +	   .if \push_lr
> +	/* Case 7: push single GP register plus lr register.  */
> +	push {r\first, lr}
> +	.cfi_adjust_cfa_offset 8
> +	.cfi_offset 14, -4
> +	cfisavelist \first, \first, 2
> +	   .else // !\push_lr
> +	/* Case 8: push single GP register.  */
> +	push {r\first}
> +	.cfi_adjust_cfa_offset 4
> +	cfisavelist \first, \first, 1
> +	   .endif
> +	  .endif
> +	 .endif
> +	.else // \first == -1
> +	 .if \push_ip
> +	  .if \push_lr
> +	/* Case 9: push ip and lr registers.  */
> +	push {ip, lr}
> +	.cfi_adjust_cfa_offset 8
> +	.cfi_offset 14, -4
> +	.cfi_offset 143, -8
> +	  .else // !\push_lr
> +	/* Case 10: push ip register.  */
> +	push {ip}
> +	.cfi_adjust_cfa_offset 4
> +	.cfi_offset 143, -4
> +	  .endif
> +	 .else // !\push_ip
> +          .if \push_lr
> +	/* Case 11: push lr register.  */
> +	push {lr}
> +	.cfi_adjust_cfa_offset 4
> +	.cfi_offset 14, -4
> +          .endif
> +	 .endif
> +	.endif
> +.endm
> +
> +.macro _epilogue first=-1, last=-1, push_ip=PAC_LEAF_PUSH_IP, push_lr=0
> +	.if \push_ip & 1 != \push_ip
> +	 .error "push_ip may be either 0 or 1"
> +	.endif
> +	.if \push_lr & 1 != \push_lr
> +	 .error "push_lr may be either 0 or 1"
> +	.endif
> +	.if \first != -1
> +	 .if \last == -1
> +	  /* Upper-bound not provided: Set upper = lower.  */
> +	  _epilogue \first, \first, \push_ip, \push_lr
> +	  .exitm
> +	 .endif
> +	 .if \last != \first
> +	  .if \last >= 13
> +	.error "SP cannot be in the save list"
> +	  .endif
> +	  .if \push_ip
> +	   .if \push_lr
> +	/* Case 1: pop register range, ip and lr registers.  */
> +	pop {r\first-r\last, ip, lr}
> +	.cfi_restore 14
> +	.cfi_register 143, 12
> +	cfirestorelist \first, \last
> +	   .else // !\push_lr
> +	/* Case 2: pop register range and ip register.  */
> +	pop {r\first-r\last, ip}
> +	.cfi_register 143, 12
> +	cfirestorelist \first, \last
> +	   .endif
> +	  .else // !\push_ip
> +	   .if \push_lr
> +	/* Case 3: pop register range and lr register.  */
> +	pop {r\first-r\last, lr}
> +	.cfi_restore 14
> +	cfirestorelist \first, \last
> +	   .else // !\push_lr
> +	/* Case 4: pop register range.  */
> +	pop {r\first-r\last}
> +	cfirestorelist \first, \last
> +	   .endif
> +	  .endif
> +	 .else // \last == \first
> +	  .if \push_ip
> +	   .if \push_lr
> +	/* Case 5: pop single GP register plus ip and lr registers.  */
> +	pop {r\first, ip, lr}
> +	.cfi_restore 14
> +	.cfi_register 143, 12
> +	cfirestorelist \first, \first
> +	   .else // !\push_lr
> +	/* Case 6: pop single GP register plus ip register.  */
> +	pop {r\first, ip}
> +	.cfi_register 143, 12
> +	cfirestorelist \first, \first
> +	   .endif
> +	  .else // !\push_ip
> +	   .if \push_lr
> +	/* Case 7: pop single GP register plus lr register.  */
> +	pop {r\first, lr}
> +	.cfi_restore 14
> +	cfirestorelist \first, \first
> +	   .else // !\push_lr
> +	/* Case 8: pop single GP register.  */
> +	pop {r\first}
> +	cfirestorelist \first, \first
> +	   .endif
> +	  .endif
> +	 .endif
> +	.else // \first == -1
> +	 .if \push_ip
> +	  .if \push_lr
> +	/* Case 9: pop ip and lr registers.  */
> +	pop {ip, lr}
> +	.cfi_restore 14
> +	.cfi_register 143, 12
> +	  .else // !\push_lr
> +	/* Case 10: pop ip register.  */
> +	pop {ip}
> +	.cfi_register 143, 12
> +	  .endif
> +	 .else // !\push_ip
> +          .if \push_lr
> +	/* Case 11: pop lr register.  */
> +	pop {lr}
> +	.cfi_restore 14
> +          .endif
> +	 .endif
> +	.endif
> +#if HAVE_PAC_LEAF
> +	aut	ip, lr, sp
> +#endif /* HAVE_PAC_LEAF */
> +	bx	lr
> +.endm
> +
> +# clean up expressions in 'last'
> +.macro _preprocess_reglist1 first:req, last:req, push_ip:req, push_lr:req, reglist_op:req
> +	.if \last == 0
> +	 \reglist_op \first, 0, \push_ip, \push_lr
> +	.elseif \last == 1
> +	 \reglist_op \first, 1, \push_ip, \push_lr
> +	.elseif \last == 2
> +	 \reglist_op \first, 2, \push_ip, \push_lr
> +	.elseif \last == 3
> +	 \reglist_op \first, 3, \push_ip, \push_lr
> +	.elseif \last == 4
> +	 \reglist_op \first, 4, \push_ip, \push_lr
> +	.elseif \last == 5
> +	 \reglist_op \first, 5, \push_ip, \push_lr
> +	.elseif \last == 6
> +	 \reglist_op \first, 6, \push_ip, \push_lr
> +	.elseif \last == 7
> +	 \reglist_op \first, 7, \push_ip, \push_lr
> +	.elseif \last == 8
> +	 \reglist_op \first, 8, \push_ip, \push_lr
> +	.elseif \last == 9
> +	 \reglist_op \first, 9, \push_ip, \push_lr
> +	.elseif \last == 10
> +	 \reglist_op \first, 10, \push_ip, \push_lr
> +	.elseif \last == 11
> +	 \reglist_op \first, 11, \push_ip, \push_lr
> +	.else
> +	 .error "last (\last) out of range"
> +	.endif
> +.endm
> +
> +# clean up expressions in 'first'
> +.macro _preprocess_reglist first:req, last, push_ip=0, push_lr=0, reglist_op:req
> +	.ifb \last
> +	 _preprocess_reglist \first \first \push_ip \push_lr
> +	.else
> +	 .if \first > \last
> +	  .error "last (\last) must be at least as great as first (\first)"
> +	 .endif
> +	 .if \first == 0
> +	  _preprocess_reglist1 0, \last, \push_ip, \push_lr, \reglist_op
> +	 .elseif \first == 1
> +	  _preprocess_reglist1 1, \last, \push_ip, \push_lr, \reglist_op
> +	 .elseif \first == 2
> +	  _preprocess_reglist1 2, \last, \push_ip, \push_lr, \reglist_op
> +	 .elseif \first == 3
> +	  _preprocess_reglist1 3, \last, \push_ip, \push_lr, \reglist_op
> +	 .elseif \first == 4
> +	  _preprocess_reglist1 4, \last, \push_ip, \push_lr, \reglist_op
> +	 .elseif \first == 5
> +	  _preprocess_reglist1 5, \last, \push_ip, \push_lr, \reglist_op
> +	 .elseif \first == 6
> +	  _preprocess_reglist1 6, \last, \push_ip, \push_lr, \reglist_op
> +	 .elseif \first == 7
> +	  _preprocess_reglist1 7, \last, \push_ip, \push_lr, \reglist_op
> +	 .elseif \first == 8
> +	  _preprocess_reglist1 8, \last, \push_ip, \push_lr, \reglist_op
> +	 .elseif \first == 9
> +	  _preprocess_reglist1 9, \last, \push_ip, \push_lr, \reglist_op
> +	 .elseif \first == 10
> +	  _preprocess_reglist1 10, \last, \push_ip, \push_lr, \reglist_op
> +	 .elseif \first == 11
> +	  _preprocess_reglist1 11, \last, \push_ip, \push_lr, \reglist_op
> +	 .else
> +	  .error "first (\first) out of range"
> +	 .endif
> +	.endif
> +.endm
> +
> +.macro _align8 first, last, push_ip=0, push_lr=0, reglist_op=_prologue
> +	.ifb \first
> +	 .ifnb \last
> +	  .error "can't have last (\last) without specifying first"
> +	 .else // \last not blank
> +	  .if ((\push_ip + \push_lr) % 2) == 0
> +	   \reglist_op first=-1, last=-1, push_ip=\push_ip, push_lr=\push_lr
> +	   .exitm
> +	  .else // ((\push_ip + \push_lr) % 2) odd
> +	   _align8 2, 2, \push_ip, \push_lr, \reglist_op
> +	   .exitm
> +	  .endif // ((\push_ip + \push_lr) % 2) == 0
> +	 .endif // .ifnb \last
> +	.endif // .ifb \first
> +
> +	.ifb \last
> +	 _align8 \first, \first, \push_ip, \push_lr, \reglist_op
> +	.else
> +	 .if \push_ip & 1 <> \push_ip
> +	  .error "push_ip may be 0 or 1"
> +	 .endif
> +	 .if \push_lr & 1 <> \push_lr
> +	  .error "push_lr may be 0 or 1"
> +	 .endif
> +	 .ifeq (\last - \first + \push_ip + \push_lr) % 2
> +	  .if \first == 0
> +	   .error "Alignment required and first register is r0"
> +	   .exitm
> +	  .endif
> +	  _preprocess_reglist \first-1, \last, \push_ip, \push_lr, \reglist_op
> +	 .else
> +	  _preprocess_reglist \first \last, \push_ip, \push_lr, \reglist_op
> +	 .endif
> +	.endif
> +.endm
> +
> +.macro prologue first, last, push_ip=PAC_LEAF_PUSH_IP, push_lr=0, align8=STACK_ALIGN_ENFORCE
> +	.if \align8
> +	 _align8 \first, \last, \push_ip, \push_lr, _prologue
> +	.else
> +	 _prologue first=\first, last=\last, push_ip=\push_ip, push_lr=\push_lr
> +	.endif
> +.endm
> +
> +.macro epilogue first, last, push_ip=PAC_LEAF_PUSH_IP, push_lr=0, align8=STACK_ALIGN_ENFORCE
> +	.if \align8
> +	 _align8 \first, \last, \push_ip, \push_lr, reglist_op=_epilogue
> +	.else
> +	 _epilogue first=\first, last=\last, push_ip=\push_ip, push_lr=\push_lr
> +	.endif
> +.endm
> +
> +#endif /* __ASSEMBLER__ */
> +
>   #endif /* ARM_ASM__H */


More information about the Newlib mailing list