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...@1wt.eu>

Thanks!

Reply via email to