Hi, Willy > On Mon, Jul 03, 2023 at 04:36:51PM +0800, Zhangjin Wu wrote: > > > Syscalls that return pointer use that -MAX_ERRNO range to encode errors > > > (such as mmap()). I just do not know if there is a convention saying that > > > other ones also restrict themselves to that range or not. If you find > > > some info which guarantees that it's the case for all of them, then by > > > all means let's proceed like this, but in this case it should be mentioned > > > in the comment why we think it's valid to do this. For now it's presented > > > as an opportunity only. > > > > Currently, I only found a prove-in-use case in musl: > > > > https://elixir.bootlin.com/musl/latest/source/src/internal/syscall_ret.c: > > > > #include <errno.h> > > #include "syscall.h" > > > > long __syscall_ret(unsigned long r) > > { > > if (r > -4096UL) { > > errno = -r; > > return -1; > > } > > return r; > > } > > > > Our new implementation (based on the one used by mmap()) is almostly the same > > as musl. Not sure if this is enough. I have tried to 'git blame' on > > __syscall_ret() of musl to find some clue, but failed, because the function has > > been added before importing into its git repo. > > OK, we already used the glibc-saved registers in the past to determine > the official list of clobbered registers (and the ABI spec was even > updated based on this). Here, musl is sufficiently deployed to consider > this as valid. You can simply go that route and mention in the commit > message that while you found no official reference stating that this is > valid for int/long returns, you found at least one other implementation > relying on this (i.e. if the kernel ever changes it will cause breakage). > ok. > > > Also, the rest of the commit message regarding uintptr_t (which we don't > > > use), bit values and modular arithmetics is extremely confusing and not > > > needed at all. What matters is only to know if we need to consider only > > > values -MAX_ERRNO..-1 as error or all negative ones. If so, then it's > > > obvious that ret >= (unsigned long)-MAX_ERRNO catches them all, as the > > > current mmap() function already does with -4095UL. > > > > > > > Yes, will clean up the commit message, but at first, let's continue get > > more information about which one is ok: > > > > - -MAX_ERRNO..-1 as error, for sys_mmap (we know in nolibc) currently > > > > - all negative ones, for others currently > > You can double-check in glibc for example, but I'm starting to guess > you'll find the same test as above, i.e. errors are exclusively >-4096, > regardless of the expected return type. > Your guest is definitely true ;-) Glibc has the same logic in its INLINE_SYSCALL() macro: https://elixir.bootlin.com/glibc/latest/source/sysdeps/unix/sysv/linux/sysdep.h #undef INTERNAL_SYSCALL_ERROR_P #define INTERNAL_SYSCALL_ERROR_P(val) \ ((unsigned long int) (val) > -4096UL) #ifndef SYSCALL_ERROR_LABEL # define SYSCALL_ERROR_LABEL(sc_err) \ ({ \ __set_errno (sc_err); \ -1L; \ }) #endif /* Define a macro which expands into the inline wrapper code for a system call. It sets the errno and returns -1 on a failure, or the syscall return value otherwise. */ #undef INLINE_SYSCALL #define INLINE_SYSCALL(name, nr, args...) \ ({ \ long int sc_ret = INTERNAL_SYSCALL (name, nr, args); \ __glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (sc_ret)) \ ? SYSCALL_ERROR_LABEL (INTERNAL_SYSCALL_ERRNO (sc_ret)) \ : sc_ret; \ }) Nothing differs. But 'git blame' has no clue to any 'spec' or 'standard' either. - fcb78a55058fd, linux: Consolidate INLINE_SYSCALL Moved all of the arch specific INTERNAL_SYSCALL_ERROR_P() to common header - 369b849f1a382, sysdeps/unix/sysv/linux/s390/s390-32/sysdep.h (INTERNAL_SYSCALL,... Firstly defined this macro: INTERNAL_SYSCALL_ERROR_P() $ git show 369b849f1a3 | grep "define.*INTERNAL_SYSCALL_ERROR_P" +#define INTERNAL_SYSCALL_ERROR_P(val) ((unsigned int) (val) >= 0xfffff001u) +#define INTERNAL_SYSCALL_ERROR_P(val) ((unsigned int) (val) >= 0xfffff001u) +#define INTERNAL_SYSCALL_ERROR_P(val) ((unsigned long) (val) >= -515L) +#define INTERNAL_SYSCALL_ERROR_P(val) ((unsigned long) (val) >= -4095L) Willy, I plan to further use something like, is it ok for you? tools/include/nolibc/errno.h: -#define MAX_ERRNO 4095 +#define MAX_ERRNO 4095UL tools/include/nolibc/sys.h: /* Syscall return helper for library routines * set errno as -ret when ret in [-MAX_ERRNO, -1] * * Note, No official reference states the errno range * here aligns with musl (src/internal/syscall_ret.c) * and glibc (sysdeps/unix/sysv/linux/sysdep.h) */ static __inline__ __attribute__((unused, always_inline)) long __sysret(unsigned long ret) { if (ret >= -MAX_ERRNO) { SET_ERRNO(-(long)ret); return -1; } return ret; } Or we also directly use 4096UL here. static __inline__ __attribute__((unused, always_inline)) long __sysret(unsigned long ret) { if (ret > -4096UL) { SET_ERRNO(-(long)ret); return -1; } return ret; } Best regards, Zhangjin > Thanks! > Willy
