Re: [PATCH] linux: Change syscall return value to long int

[Xi Ruoyao via Libc-alpha <libc-alpha@sourceware.org>]

Xuerui: can you help to rewrite the commit message?  The code change
seems obvious but I'm not sure how to describe the rationale precisely
either.

On Tue, 2022-11-29 at 08:55 +0000, Szabolcs Nagy wrote:
> The 11/29/2022 11:16, XingLi wrote:
> > From: Xing Li <lixing@loongson.cn>
> > 
> > The kernel syscall return is long value.
> > The generic syscall interface return value
> > is int, which may lead to incorrect return value.
> 
> it's not clear what you mean here, the generic syscall
> function returns long (according to unistd.h).
> 
> > 
> > The following test is syscall with mmap executed on LoongArch,
> > only 32bits and sign extension value returned leading to mmap
> > failure,
> > which should be with 47bits address returned.
> > 
> > Testcase:
> > 
> >  #include <sys/syscall.h>
> >  #include <sys/mman.h>
> >  #include <stdio.h>
> > 
> > void main()
> > {
> >         long int ret;
> >         ret = syscall(SYS_mmap, NULL, 0x801000, PROT_READ |
> > PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0);
> 
> Note: there are many reasons why direct calls to syscall
> may not work.
> 
> syscall is a variadic argument function that takes long
> arguments, but you pass ints that may *not* be sign/zero
> extended on the caller site so e.g. the top 32bits of
> size and offset can be arbitrary on a 64bit system.
> (you have to cast args to long to make the example valid).
> 
> and some systems use SYS_mmap2.
> 
> >         printf("map address is %lx\n",ret);
> > }
> > 
> > Result:
> > [lixing@Sunhaiyong test]$ ./mmap
> > map address is fffffffff008c000
> > ---
> >  sysdeps/unix/sysv/linux/syscall.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/sysdeps/unix/sysv/linux/syscall.c
> > b/sysdeps/unix/sysv/linux/syscall.c
> > index 7303ba7188..8cb0b66b1c 100644
> > --- a/sysdeps/unix/sysv/linux/syscall.c
> > +++ b/sysdeps/unix/sysv/linux/syscall.c
> > @@ -33,7 +33,7 @@ syscall (long int number, ...)
> >    long int a5 = va_arg (args, long int);
> >    va_end (args);
> >  
> > -  int r = INTERNAL_SYSCALL_NCS_CALL (number, a0, a1, a2, a3, a4,
> > a5);
> > +  long int r = INTERNAL_SYSCALL_NCS_CALL (number, a0, a1, a2, a3,
> > a4, a5);
> >    if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (r)))
> 
> this change looks reasonable to me.
> 
> >      {
> >        __set_errno (-r);
> > -- 
> > 2.31.1
> > 

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University