On 2025-02-16 10:49:38+0100, Willy Tarreau wrote: > On Wed, Feb 12, 2025 at 07:49:53PM +0100, Thomas Weißschuh wrote: > > +#if defined(_ABIO32) > > + > > #define _NOLIBC_SYSCALL_CLOBBERLIST \ > > "memory", "cc", "at", "v1", "hi", "lo", \ > > "t0", "t1", "t2", "t3", "t4", "t5", "t6", "t7", "t8", "t9" > > +#define _NOLIBC_SYSCALL_STACK_RESERVE "addiu $sp, $sp, -32\n" > > +#define _NOLIBC_SYSCALL_STACK_UNRESERVE "addiu $sp, $sp, 32\n" > > + > > +#elif defined(_ABIN32) || defined(_ABI64) > > + > > +/* binutils, GCC and clang disagree about register aliases, use numbers instead. */ > > Is this often encountered despite this ? I guess it can cause portability > issues :-/ No idea. It's the first time I saw something like this. > > +#if defined(_ABIO32) > > + > > #define my_syscall5(num, arg1, arg2, arg3, arg4, arg5) \ > > ({ \ > > register long _num __asm__ ("v0") = (num); \ > (...) > > @@ -178,6 +201,50 @@ > > _arg4 ? -_num : _num; \ > > }) > > > > +#else > > + > > Here you should indicate which ABI is covered by this #else, because one > has to go up to previous definitions to figure it's _ABIN32 and _ABI64. Ack. > > +#define my_syscall5(num, arg1, arg2, arg3, arg4, arg5) \ > > +({ \ > > + register long _num __asm__ ("v0") = (num); \ > > + register long _arg1 __asm__ ("$4") = (long)(arg1); \ > > + register long _arg2 __asm__ ("$5") = (long)(arg2); \ > > + register long _arg3 __asm__ ("$6") = (long)(arg3); \ > > + register long _arg4 __asm__ ("$7") = (long)(arg4); \ > > + register long _arg5 __asm__ ("$8") = (long)(arg5); \ > (...) > > @@ -190,13 +257,33 @@ void __attribute__((weak, noreturn)) __nolibc_entrypoint __no_stack_protector __ > > "1:\n" > > ".cpload $ra\n" > > "move $a0, $sp\n" /* save stack pointer to $a0, as arg1 of _start_c */ > > + > > +#if defined(_ABIO32) > > "addiu $sp, $sp, -4\n" /* space for .cprestore to store $gp */ > > ".cprestore 0\n" > > "li $t0, -8\n" > > "and $sp, $sp, $t0\n" /* $sp must be 8-byte aligned */ > > "addiu $sp, $sp, -16\n" /* the callee expects to save a0..a3 there */ > > - "lui $t9, %hi(_start_c)\n" /* ABI requires current function address in $t9 */ > > +#else > > So same here. I think you should do it for all #else since you're generally > grouping 2 ABIs vs one between a #if and a #else and it's not trivial to > figure what a #else covers, like below. Ack. > > + "daddiu $sp, $sp, -8\n" /* space for .cprestore to store $gp */ > > + ".cpsetup $ra, 0, 1b\n" > > + "li $t0, -16\n" > > + "and $sp, $sp, $t0\n" /* $sp must be 16-byte aligned */ > > +#endif > > + > > + /* ABI requires current function address in $t9 */ > > +#if defined(_ABIO32) || defined(_ABIN32) > > + "lui $t9, %hi(_start_c)\n" > > "ori $t9, %lo(_start_c)\n" > > +#else > > This one indeed covers only _ABI64 (That is intentional) > > + "lui $t9, %highest(_start_c)\n" > > + "ori $t9, %higher(_start_c)\n" > > + "dsll $t9, 0x10\n" > > + "ori $t9, %hi(_start_c)\n" > > + "dsll $t9, 0x10\n" > > + "ori $t9, %lo(_start_c)\n" > > +#endif > > With the tiny details above, this looks fine. It's great that syscall > numbers didn't change so that you can cover an extra arch with only a > few ifdefs. I have not tested but I guess you did :-) So that's OK for > me: The syscall numbers are different, but the UAPI headers also detect the ABI in use and select the correct numbers. > Acked-by: Willy Tarreau <w@xxxxxx> Thanks!