Re: Final (?) time64 proposal

[Rich Felker <dalias@libc.org>]

On Thu, Jul 25, 2019 at 04:01:07PM -0400, Rich Felker wrote:
> On Thu, Jul 25, 2019 at 01:18:32AM -0400, Rich Felker wrote:
> > On Wed, Jul 24, 2019 at 01:31:42AM -0400, Rich Felker wrote:
> > > With that said, the following are the functions I've identified as
> > > still interfacing with the kernel in ways that involve time_t:
> > > 
> > > - clock_gettime **
> > > [...]
> > 
> > I think a good next-step here would be modifying all of the above to
> > go through an intermediate kernel-type struct when using the existing
> > syscalls. This should not change behavior anywhere except for a slight
> > increase in size/time-spent, but will set things up so that, when the
> > time_t definition is switched over, everything should just start
> > working automatically with 64-bit time. It will also let us drop the
> > __fixup hacks in arch/x32/syscall_arch.h and
> > src/thread/x32/syscall_cp_fixup.c, analogous to how commit
> > 01ae3fc6d48f4a45535189b7a6db286535af08ca let us drop the corresponding
> > mips hacks.
> 
> I'm not so sure about this approach now. Conversions will never be
> needed for 64-bit archs, nor for new code paths using the time64
> syscalls on 32-bit archs. Aside from x32 awfulness, the condition for
> the new version of the code to be needed is SYS_*time64 being defined
> and time_t being 64-bit. This suggests we might let x32/syscall_arch.h
> define the SYS_*time64 macros, and undef the legacy names, so that it
> gets treated like a 32-bit arch with no legacy time32 syscalls (i.e.
> like riscv32 will be). The logic will roughly be:
> 
> #ifdef SYS_foo_time64
> 	if (sizeof(time_t)>4) {
> 		[if there's an input timespec, copy it and zero padding]
> 		SYS_foo_time64
> 		[if success, return]
> 	}
> #ifdef SYS_foo [legacy time32]
> 	[if there's input, explicitly convert to legacy time32 form]
> 	SYS_foo
> 	[if success and there's output, convert back to libc time form]
> #endif
> #else
> 	SYS_foo [native 64-bit]
> #endif

I'm working on this for the "timeout" style functions, and it's mostly
working out (in terms of the details making sense, not actually trying
to run code). I've added logic to only try the time64 syscall if a
timeout was provided (in practice often it's null), and then only if
tv_sec doesn't fit in 32 bits, so that performance won't suffer on
existing kernels without time64 syscalls.

> The "if (sizeof(time_t)>4)" condition will become always-false once
> 32-bit archs are converted over, and can be removed after that. But
> for now, it would allow the code to be compile-tested on all archs and
> to take effect on x32, with the above-described renaming of syscalls
> made in x32/syscall_arch.h.
> 
> Granted this is somewhat ugly, but I don't see any other better way;
> it seems minimal to meet the (hard) requirements of running on pre-5.1
> kernels and using correct time representations.

This seems to be working for x32, but one ugly detail is that
SYS_futex is used everywhere and isn't really a "time32" syscall
unless a WAIT/LOCK_PI operation is being used with a timeout. So,
rather than undefining the non-time64 syscall names after mapping
them, I'm just going to define both, and then instead of:

#ifdef SYS_foo_time64
...
#ifdef SYS_foo
...
#endif
...
#endif

we can do:

#ifdef SYS_foo_time64
...
#if defined(SYS_foo) && SYS_foo != SYS_foo_time64
...
#endif
...
#endif

> The easiest are timeouts. Whether the overflowing timeout is absolute
> or relative, it can't happen before the non-Y2038-safe system dies in
> 2038, so the syscall can just be made with no timeout. That covers:
> 
> - clock_nanosleep
> - mq_timedsend
> - mq_timedreceive
> - select
> - pselect
> - ppoll
> - recvmmsg
> - sigtimedwait
> - semtimedop
> - __timedwait

Also add pthread_mutex_timedlock. I thought __timedwait covered it,
but it performs FUTEX_LOCK_PI for PI mutexes, which also has a timeout
argument.

> In the case of clock_nanosleep (and select, if we want to continue
> providing the updated remaining time for select), we can't just use an
> unlimited timeout, but instead need to do something like INT32_MAX so
> that we can add back the remaining time if the syscall does return.

There doesn't seem to be any advantage to using an unlimited timeout
instead of just INT32_MAX timeout when the tv_sec doesn't fit in 32
bits. There's also (at least for absolute timeouts; it's not clear
what's supposed to happen for relative ones with negative tv_sec) the
possibility of times in the distant past, which should ETIMEDOUT
immediately rather than getting truncated or sleeping indefinitely. So
the approach I'm trying now is using a CLAMP() macro on both tv_sec
and tv_nsec when converting to 32-bit. This ensures that large
positive or negative tv_sec will do the right thing, and that large or
negative tv_nsec will map to values that error out rather than
possibly truncating to an in-range value.

Rich