This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [RFC v6 08/23] RISC-V: Define __NR_* as __NR_*_time64/64 for 32-bit
On Mon, Jan 27, 2020 at 11:02 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Arnd Bergmann:
>
> > On Sat, Jan 25, 2020 at 9:33 PM Khem Raj <raj.khem@gmail.com> wrote:
> >> On 1/14/20 11:03 PM, Alistair Francis wrote:
> >>
> >> There is userspace code like [1] which expects SYS_futex
> >> These overrides do not end up in usr/include/bits/syscall-32.h
> >> so it fails to compile
> >>
> >> src/xshmfence_futex.h:58:17: error: use of undeclared identifier
> >> 'SYS_futex'; did you mean 'sys_futex'?
> >> return syscall(SYS_futex, addr1, op, val1, timeout, addr2, val3);
> >> ^~~~~~~~~
> >>
> >> I wonder if __NR_futex and other aliases here should be exposed to
> >> userspace?
> >
> > Please don't, that just makes it harder for the other ports to find
> > these instances and fix them in a portable way.
> >
> > Any source code reference to a low-level system call that passes
> > a time32 type needs to be handled roughly like this:
> >
> > typedef unsigned int __u32;
> > #if defined(__x86_64__) && defined(__ILP32__)
> > typedef long long __kernel_long_t;
> > #else
> > typedef long __kernel_long_t;
> > #endif
>
> Is this the type that glibc and some others call register_t? I think
> some MIPS targets also need a special definition for it.
No, this is the type that is only different on x32, mips n32
also uses the regular syscall ABI.
> > typedef __kernel_long_t __kernel_old_time_t;
> > struct __kernel_old_timespec {
> > __kernel_long_t tv_sec;
> > __kernel_long_t tv_nsec;
> > };
> > typedef long long __kernel_time64_t;
> > struct __kernel_timespec {
> > __kernel_time64_t tv_sec;
> > long long tv_nsec;
> > };
>
> Who is going to provide these types?
>
> If we put it into glibc headers, we should not use the __kernel_*
> prefix.
I used the types from the existing kernel headers here to not have
to come up with a new naming scheme. If we end up providing the
wrappers from the kernel, then these are the types that should be
used. If glibc ends up providing a futex implementation, that
would clearly use the glibc provided types on the interface and
hide the implementation details. If we make the inline version a
standalone header that can be included in applications under
a permissive license, then all the identifiers should probably be
renamed to something that doesn't conflict with either kernel or
glibc.
> > static inline long __kernel_futex_time64(__u32 * uaddr, int op, __u32 val,
> > struct __kernel_timespec *utime,
> > __u32 * uaddr2, __u32 val3)
> > {
> > #ifdef __NR_futex_time64
> > return syscall(__NR_futex_time64, uaddr, op, val, utime, uaddr2, val3);
> > #else
> > errno = -ENOSYS;
> > return -1;
> > #endif
> > }
> >
> > static inline long __kernel_futex_old(__u32 * uaddr, int op, __u32 val,
> > struct __kernel_old_timespec *utime,
> > __u32 * uaddr2, __u32 val3)
> > {
> > #ifdef __NR_futex
> > return syscall(__NR_futex, uaddr, op, val, utime, uaddr2, val3);
> > #else
> > errno = -ENOSYS;
> > return -1;
> > #endif
> >
> > }
>
> Ah better design would split the operations that need timeouts from
> those which do not, so that only code that actually uses timeouts needs
> porting. This avoids the type-polymorphic val2/utime argument.
I just realized that my version above is completely broken for the
'op' values that pass an integer instead of a pointer, so I agree
this has to be changed in some form that understands the difference
between timeout and val2 passing. :(
My hope was that we can stay close to the in-kernel types for the
prototypes. If we actually autogenerate a header file from the kernel
for all system calls that would help ensure that the prototype that user
space sees is exactly the same one that the kernel sees.
It would almost work by just defining a futex function locally as either
__kernel_futex_time64() or __kernel_futex_old() above based on
sizeof(time_t), but it breaks if anyone ever tries to support the resulting
binary with time64 support on an old kernel.
Having either individual wrappers per futex-op, or a wrapper that
does the conversion based on a switch(op) {} would solve that, but
both have the downside of not working with additional values of
'op' that might be added later.
> > The code above uses the identifiers from the kernel namespace,
> > and this is how we might decide to distribute it along with the kernel
> > headers. If applications want to ship their own copy, they might
> > need to define their own types or require a very recent version of the
> > kernel headers, as __kernel_old_timespec was only added in linux-5.5
> > (the older headers just define 'timespec' which conflicts with the libc
> > type of the same name and cannot be included here at all).
>
> Even for the kernel, __kernel_* prefixes are awkward because the old
> type variants are almost exclusively userspace constructs.
The __kernel_ prefix was always just intended to provide a namespace
for things that may be defined differently from user space and is unlikely
to cause conflicts. It was first added in linux-1.3.79 for basic types
[https://lkml.org/lkml/1996/3/26/68].
> > An alternative would be for all C libraries to start providing a futex()
> > wrapper and a way to identify whether it exists, then the applications
> > can largely start using that one on all architectures, and fall back to
> > the time32 version when building against an older libc.
>
> Yes, I think this is the right direction. In theory, it is possible to
> use GCC extensions to implement overloads. But I tried to do that for
> fcntl once, and the macros are rather unwieldy. Furthermore, C++ neds a
> different implementation.
What is the problem with using a plain uintptr_t or struct timespec* and
requiring callers to add a cast? Is this just a matter of ugly calling
conventions, or are there cases where it causes bigger problems?
Would it help to have the libc futex wrapper take seven arguments with
separate timeout and val2 arguments, and requiring at least one of them
to be 0 or NULL? I think the only reason for the awkward interface in the
kernel is that a syscall with seven arguments would require passing
a structure on most architectures.
Arnd