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

[Lukasz Majewski <lukma at denx dot de>]

Hi Joseph,

> 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?

As confirmed in the other mail - the padding is ignored in Linux kernel
(and the fix patch for x32 is up its way to be pulled).

>  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.

Cannot we just use __WORDSIZE != 64  as proposed by Stepan?
(for x32 we would have it defined by default)

#if __WORDSIZE != 64
# if __LINUX_KERNEL_VERSION >= 0x050100
#  define __ASSUME_TIME64_SYSCALLS 1
# endif
#endif  

Such approach would allow us to avoid introducing new abstraction
(__SYSCALL_WORDSIZE).

As of now only x32 has __WORDSIZE=32 and __TIMESIZE=64 and would be
treated as a special case with solution proposed by Stepan in the
other mail:

---->8-----
One way would be by defining __ASSUME_TIME64_SYSCALLS unconditionally
on x32 and then defining __NR_clock_settime64 to __NR_clock_settime
when __ASSUME_TIME64_SYSCALLS is defined while __NR_clock_settime64
isn't.
----8<-----

Or even simpler:
#undef __ASSUME_TIME64_SYSCALLS for x32 (with proper comment)

x32 is special here - if (unlikely) some other arch with __WORDSIZE=32
and __TIMESIZE=64 emerge - it shall follow the same pattern




For __WORDSIZE/__TIMESIZE=32 and __WORDSIZE/__TIMESIZE=64 archs we
would have a clear situation.


> 
> > 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.
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

Attachment: pgp1wC6qAtOyb.pgp
Description: OpenPGP digital signature