Re: [PATCH v2 2/7] y2038: Introduce __ASSUME_64BIT_TIME define

[Joseph Myers <joseph@codesourcery.com>]

On Tue, 30 Apr 2019, Lukasz Majewski wrote:

>  - The need for explicit clearing padding when calling syscalls (as to
>    be better safe than sorry in the future - there was related
>    discussion started by Stepan).

This really isn't a difficult question.  What it comes down to is whether 
the Linux kernel, in the first release version with these syscalls (we 
don't care about old -rc versions; what matters is the actual 5.1 
release), ignores the padding.

If 5.1 *release* ignores the padding, that is part of the kernel/userspace 
ABI, in accordance with the kernel principle of not breaking userspace.  
Thus, it is something userspace can rely on, now and in the future.

If 5.1 release does not ignore the padding, syscall presence does not mean 
the padding is ignored by the kernel and so glibc needs to clear padding.  
Of course, it needs to clear padding in a *copy* of the value provided by 
the user unless the glibc API in question requires the timespec value in 
question to be in writable memory.

So, which is (or will be) the case in 5.1 release?  Padding ignored or 
not?  If more complicated (ignored for some architectures / ABIs but not 
for others, or depending on whether compat syscalls are in use), then say 
so - give a precise description of the exact circumstances under which the 
padding around a 32-bit tv_nsec will or will not be ignored by the kernel 
on input from userspace.

(x32 is a separate matter, as it already has 64-bit times, and a 
non-POSIX-conforming tv_nsec, so this patch series just needs to avoid 
breaking anything that currently works there.  Any fix for bug 16437 would 
need to involve clearing padding in userspace, unless not only the kernel 
changes to ignore that padding but all kernel versions without such a 
change cease to be supported by glibc for x32.)

> You are right here - the 
> 
> #if __TIMESIZE != 64
> # if __LINUX_KERNEL_VERSION >= 0x050100
> #  define __ASSUME_64BIT_TIME 1
> # endif
> #endif
> 
> would do the trick.

But that wouldn't be right for *future* configurations where the kernel 
"long" is 32-bit but only 64-bit time is supported in the kernel and glibc 
(so __TIMESIZE is 64, and only the new syscalls and not the old ones are 
supported).  That is, the right abstraction here is not really __TIMESIZE.

It's possible it's __SYSCALL_WORDSIZE, except that's only defined for 
x86_64, so would need to be made more generally available if it's to be 
used here.  And if made more generally available, it would need a careful 
comment explaining exactly what it means.

> 1. The __ASSUME_64BIT_TIME is _never_ defined for 64 bit native systems
> 
> 2. It is defined by default in:
> sysdeps/unix/sysv/linux/kernel-features.h for 32 bit systems (and the

It would be best to avoid descriptions such as "64 bit native systems" and 
"32 bit systems" when defining the relevant abstractions, because they are 
simply too ambiguous in this context, where one of the key thing to do is 
to make the semantics obvious in cases that have some attributes of 32-bit 
systems and other attributes of 64-bit systems.

We have configurations such as x32 and MIPS n32 which have 64-bit 
registers but 32-bit "long" and pointers.  Are those 64-bit or 32-bit?  As 
far as glibc's definition of __WORDSIZE is concerned, they are 32-bit; 
__WORDSIZE is the size in bits of long and pointers.  As far as optimized 
code working one word at a time is concerned (libm functions, string 
functions, etc.), they are best considered 64-bit, because of the 64-bit 
registers.  For the present issue, they are *different* from each other: 
x32 does not have the new syscalls (it already had 64-bit times), n32 does 
have the new syscalls (it previously had 32-bit times).

Again, I think the size of __syscall_slong_t is probably what's relevant.  
Note that "size of long for syscalls" (which is 64-bit for x32 but 32-bit 
for n32) is *not* the same thing as "size passed in a single register for 
syscalls" (n32 passes 64-bits values in a single register to syscalls, on 
the principle of keeping the ABI for those similar to that for normal 
function calls; but there have been more recent suggestions in the kernel 
context - I don't know the conclusion from them - of whether future such 
ILP32 ABIs with 64-bit registers should be more similar to the ABIs using 
32-bit registers, to allow compat syscall code to be used for them more 
consistently).

> As those syscalls are provided on almost every 32 bit system now
> (5.1-rc6):
> git grep -n "clock_settime64"
> 
> gives support for: arm, arm64 (compat mode), m68k, microblaze, mips,
> parisc, powerpc, s390, sh, sparc, x86, xtensa
> 
> So it would be reasonable to just add this __ASSUME definition code to
> sysdeps/unix/sysv/linux/kernel-features.h and #undef it for
> architectures not supporting it (i.e. c-sky and riscv).

No, that's not accurate.  Newer architectures (such as csky and riscv) use 
the asm-generic syscall table and so get these syscalls automatically if 
__BITS_PER_LONG == 32.  So it would be wrong to #undef in those cases.

When checking each glibc architecture / ABI combination, to see if the 
syscalls are present in the kernel, you need to allow for some 
architectures using asm-generic (which means that for such architectures 
you only need to check the generic logic, then look at any compat syscall 
tables, such as for arm on arm64).  For architectures not using 
asm-generic you need to check the per-architecture syscalls tables for 
each relevant ABI.

-- 
Joseph S. Myers
joseph@codesourcery.com