Re: [PATCH] newlib: libc: make AArch64 assembly implementations portable

On 12/06/2025 08:01, Radek Barton wrote:
> Hello.
> 
> To allow AArch64 build of Cygwin, the following usages of `.hidden`, `.size`, and `.type` assembly directives needs to be wrapped in macros.
> 
> Thank you for your feedback.
> 
> Radek
> 
> ---
> From 16ff5ba2548f23501f82644b3c0d819d6b70b096 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Radek=20Barto=C5=88?= <[email protected]>
> Date: Thu, 5 Jun 2025 11:32:08 +0200
> Subject: [PATCH] newlib: libc: make AArch64 assembly implementations portable
> 
> .hidden, .size, and .type name directives are relevant only for ELF, they are not supported for COFF.
> ---
>  newlib/libc/machine/aarch64/asmdefs.h   | 18 ++++++++++++++----
>  newlib/libc/machine/aarch64/rawmemchr.S |  6 ++++--
>  newlib/libc/machine/aarch64/setjmp.S    | 10 ++++++----
>  3 files changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/newlib/libc/machine/aarch64/asmdefs.h b/newlib/libc/machine/aarch64/asmdefs.h
> index 131b95e1f..da1df3da6 100644
> --- a/newlib/libc/machine/aarch64/asmdefs.h
> +++ b/newlib/libc/machine/aarch64/asmdefs.h
> @@ -59,9 +59,19 @@
>  GNU_PROPERTY (FEATURE_1_AND, FEATURE_1_BTI|FEATURE_1_PAC)
>  #endif
>  
> +#ifdef __ELF__
> +#define HIDDEN(name) .hidden name
> +#define SYMBOL_SIZE(name) .size name, .-name
> +#define SYMBOL_TYPE(name, _type) .type name, _type
> +#else
> +#define HIDDEN(name)
> +#define SYMBOL_SIZE(name)
> +#define SYMBOL_TYPE(name, _type)
> +#endif
> +
>  #define ENTRY_ALIGN(name, alignment)	\
>    .global name;		\
> -  .type name,%function;	\
> +  SYMBOL_TYPE(name, %function);	\
>    .align alignment;		\
>    name:			\
>    .cfi_startproc;	\
> @@ -70,13 +80,13 @@ GNU_PROPERTY (FEATURE_1_AND, FEATURE_1_BTI|FEATURE_1_PAC)
>  #define ENTRY(name)	ENTRY_ALIGN(name, 6)
>  
>  #define ENTRY_ALIAS(name)	\
> -  .global name;		\
> -  .type name,%function;	\
> +  .global name;			\
> +  SYMBOL_TYPE(name, %function);	\
>    name:
>  
>  #define END(name)	\
>    .cfi_endproc;		\
> -  .size name, .-name;
> +  SYMBOL_SIZE(name);
>  
>  #define L(l) .L ## l

asmdefs.h should just be a copy of the one from the Arm optimized routines (AOR) library.  I'd prefer any changes to it be copied from there.  Would you be prepared to send your changes there first?  (https://github.com/ARM-software/optimized-routines)


>  
> diff --git a/newlib/libc/machine/aarch64/rawmemchr.S b/newlib/libc/machine/aarch64/rawmemchr.S
> index 26da81005..97374282e 100644
> --- a/newlib/libc/machine/aarch64/rawmemchr.S
> +++ b/newlib/libc/machine/aarch64/rawmemchr.S
> @@ -34,13 +34,15 @@
>  /* See rawmemchr-stub.c.  */
>  #else
>  
> +#include "asmdefs.h"
> +
>  #define L(l) .L ## l
>  
>  	.macro def_fn f p2align=0
>  	.text
>  	.p2align \p2align
>  	.global \f
> -	.type \f, %function
> +	SYMBOL_TYPE(\f, %function)
>  \f:
>  	.endm

Since you're including asmdefs.h we should just update this file to use it properly: get rid of def_fn and use ENTRY/ENTRY_ALIGN and END as appropriate.  There's no need for this function to deviate other than it predates the introduction of asmdefs.h. 

>  
> @@ -63,6 +65,6 @@ L(do_strlen):
>  	ret	x15
>  	.cfi_endproc
>  
> -	.size   rawmemchr, . - rawmemchr
> +	SYMBOL_SIZE(rawmemchr)
>  #endif
>  
> diff --git a/newlib/libc/machine/aarch64/setjmp.S b/newlib/libc/machine/aarch64/setjmp.S
> index 0856145bf..bba8d668a 100644
> --- a/newlib/libc/machine/aarch64/setjmp.S
> +++ b/newlib/libc/machine/aarch64/setjmp.S
> @@ -26,6 +26,8 @@
>     SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>   */
>  
> +#include "asmdefs.h"
> +
>  #define GPR_LAYOUT			\
>  	REG_PAIR (x19, x20,  0);	\
>  	REG_PAIR (x21, x22, 16);	\
> @@ -43,7 +45,7 @@
>  
>  // int setjmp (jmp_buf)
>  	.global	setjmp
> -	.type	setjmp, %function
> +	SYMBOL_TYPE(setjmp, %function)
>  setjmp:
>  	mov	x16, sp
>  #define REG_PAIR(REG1, REG2, OFFS)	stp REG1, REG2, [x0, OFFS]
> @@ -54,11 +56,11 @@ setjmp:
>  #undef REG_ONE
>  	mov	w0, #0
>  	ret
> -	.size	setjmp, .-setjmp
> +	SYMBOL_SIZE(setjmp)
>  
>  // void longjmp (jmp_buf, int) __attribute__ ((noreturn))
>  	.global	longjmp
> -	.type	longjmp, %function
> +	SYMBOL_TYPE(longjmp, %function)
>  longjmp:
>  #define REG_PAIR(REG1, REG2, OFFS)	ldp REG1, REG2, [x0, OFFS]
>  #define REG_ONE(REG1, OFFS)		ldr REG1, [x0, OFFS]
> @@ -71,4 +73,4 @@ longjmp:
>  	cinc	w0, w1, eq
>  	// use br not ret, as ret is guaranteed to mispredict
>  	br	x30
> -	.size	longjmp, .-longjmp
> +	SYMBOL_SIZE(longjmp)

And similarly here.

Neither of these two .S files exist in AOR, so modifying them here is OK in principle, but lets use the style that AOR uses.

R.