64 bit time_t on riscv32

64 bit time_t on riscv32

[Antonios Salios]

Hi all!

I'm having trouble with timestamps that are sent by the Linux evdev
interface to userspace applications (such as evtest).
I am using a virtualized riscv32 environment built by Buildroot with
glibc 2.37.

The kernel and userspace do not agree on how to define the timestamps.
The values should be 32-bit, so the kernel uses custom 32-bit __sec &
__usec variables instead of a timeval struct.
Userspace, on the other hand, wants to use a timeval struct, that uses
64-bit time_t on rv32.
The struct in question (input_event) can be found here [1].

According to a kernel maintainer, the __USE_TIME_BITS64 macro should be
set on architectures that use 64-bit time [2], otherwise the kernel
headers will not be able to pick the correct definition. 

When I define __USE_TIME_BITS64 manually, the compiler (gcc 13.2) does
not recognize the __time64_t type: `unknown type name '__time64_t'`.
This is fixed by also defining __LIBC [3], although I'm not sure why
this is necessary.
However, the preprocessor will then pick the correct definition of the
input_event struct.

But now the linker cannot find the reference to __ioctl_time64 (and
__select64) because it is only defined (as an alias) when __TIMESIZE !=
64 [4], which is not the case on rv32.

What am I missing?

Kind regards

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/include/uapi/linux/input.h?h=v6.5.6#n28
[2]
https://lore.kernel.org/lkml/1a528414-f193-4ac0-a911-af426bb48d64@app.fastmail.com/
[3]
https://sourceware.org/git/?p=glibc.git;a=blob;f=posix/bits/types.h;hb=glibc-2.37#l220
[4]
https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/ioctl.c;hb=glibc-2.37#l48
-- 
Antonios Salios
Software Engineer

MachineWare GmbH | www.machineware.de
Hühnermarkt 19, 52062 Aachen, Germany
Amtsgericht Aachen HRB25734

Geschäftsführung
Lukas Jünger
Dr.-Ing. Jan Henrik Weinstock

Re: 64 bit time_t on riscv32

[Florian Weimer]

* Antonios Salios via Libc-help:

> According to a kernel maintainer, the __USE_TIME_BITS64 macro should be
> set on architectures that use 64-bit time [2], otherwise the kernel
> headers will not be able to pick the correct definition.

__USE_TIME_BITS64 is an internal glibc macro.  It is not used on
architectures which have a 64-bit time_t by default.

Surely the UAPI headers know which time size the architecture uses in
the kernel interface, and can be written arcordingly?

Thanks,
Florian

Re: 64 bit time_t on riscv32

[Adhemerval Zanella Netto]

On 15/01/24 10:40, Florian Weimer via Libc-help wrote:
> * Antonios Salios via Libc-help:
> 
>> According to a kernel maintainer, the __USE_TIME_BITS64 macro should be
>> set on architectures that use 64-bit time [2], otherwise the kernel
>> headers will not be able to pick the correct definition.
> 
> __USE_TIME_BITS64 is an internal glibc macro.  It is not used on
> architectures which have a 64-bit time_t by default.
> 
> Surely the UAPI headers know which time size the architecture uses in
> the kernel interface, and can be written arcordingly?

The use of a glibc internal definition on a kABI header is not really
a good design.  This seems to be only user so far, so I suggest to fix
on the kernel to not tie to a glibc internal definition. 

CCing Rich from musl that most likely would want to see this fixed. The
Android developers might be interested.

Re: 64 bit time_t on riscv32

[Adhemerval Zanella Netto]

On 15/01/24 16:55, Adhemerval Zanella Netto wrote:
> 
> 
> On 15/01/24 10:40, Florian Weimer via Libc-help wrote:
>> * Antonios Salios via Libc-help:
>>
>>> According to a kernel maintainer, the __USE_TIME_BITS64 macro should be
>>> set on architectures that use 64-bit time [2], otherwise the kernel
>>> headers will not be able to pick the correct definition.
>>
>> __USE_TIME_BITS64 is an internal glibc macro.  It is not used on
>> architectures which have a 64-bit time_t by default.
>>
>> Surely the UAPI headers know which time size the architecture uses in
>> the kernel interface, and can be written arcordingly?
> 
> The use of a glibc internal definition on a kABI header is not really
> a good design.  This seems to be only user so far, so I suggest to fix
> on the kernel to not tie to a glibc internal definition. 
> 
> CCing Rich from musl that most likely would want to see this fixed. The
> Android developers might be interested.

So Arnd raised it was the agreement when it was added 2018 between glibc and 
kernel headers, and we changed the deal three years later. And not sure if 
it was on y2038 draft documentation, or on the initial patchset. Nor I
recall the discussion where it was accorded (Arnd could help me here).  

And I am not sure this is a good design, it ties glibc internal definition
to kABI meaning that glibc won't be able to refactor this code because it
might eventually break the ABI.  I think we will need at least to proper
document this somewhere to avoid future breakages.

For glibc side, I think we can always define the macro so the check would
be '__USE_TIME_BITS64 == 1' for 64 time_t support.  It would require some
internal refactoring, but it should be doable.

Re: 64 bit time_t on riscv32

[Arnd Bergmann]

On Mon, Jan 15, 2024, at 21:15, Adhemerval Zanella Netto wrote:
> On 15/01/24 16:55, Adhemerval Zanella Netto wrote:
>> 
>> 
>> On 15/01/24 10:40, Florian Weimer via Libc-help wrote:
>>> * Antonios Salios via Libc-help:
>>>
>>>> According to a kernel maintainer, the __USE_TIME_BITS64 macro should be
>>>> set on architectures that use 64-bit time [2], otherwise the kernel
>>>> headers will not be able to pick the correct definition.
>>>
>>> __USE_TIME_BITS64 is an internal glibc macro.  It is not used on
>>> architectures which have a 64-bit time_t by default.
>>>
>>> Surely the UAPI headers know which time size the architecture uses in
>>> the kernel interface, and can be written arcordingly?
>> 
>> The use of a glibc internal definition on a kABI header is not really
>> a good design.  This seems to be only user so far, so I suggest to fix
>> on the kernel to not tie to a glibc internal definition. 
>> 
>> CCing Rich from musl that most likely would want to see this fixed. The
>> Android developers might be interested.
>
> So Arnd raised it was the agreement when it was added 2018 between glibc and 
> kernel headers, and we changed the deal three years later. And not sure if 
> it was on y2038 draft documentation, or on the initial patchset. Nor I
> recall the discussion where it was accorded (Arnd could help me here).  
>
> And I am not sure this is a good design, it ties glibc internal definition
> to kABI meaning that glibc won't be able to refactor this code because it
> might eventually break the ABI.  I think we will need at least to proper
> document this somewhere to avoid future breakages.

It's in the design document and was added in

https://sourceware.org/glibc/wiki/Y2038ProofnessDesign?action=diff&rev1=86&rev2=87

which was all we had to work with at the time. I understand
that some aspects of that design changed over time, but I didn't
know this one did.

> For glibc side, I think we can always define the macro so the check would
> be '__USE_TIME_BITS64 == 1' for 64 time_t support.  It would require some
> internal refactoring, but it should be doable.

I took another look at the input_event definition now, and I think
we can actually change it to remove the __USE_TIME_BITS64 check in
this one, dropping that 'timeval' reference from it, since it was
only added here to prevent compile-time errors on architectures
that use the traditional types (all 64-bit ones and 32-bit
architectures with time32):

struct input_event {              
#if (__BITS_PER_LONG != 32 || !defined(__USE_TIME_BITS64)) && !defined(__KERNEL__)
        struct timeval time;
#define input_event_sec time.tv_sec 
#define input_event_usec time.tv_usec
#else
        __kernel_ulong_t __sec;
        __kernel_ulong_t __sec;
#define input_event_sec  __sec
#define input_event_usec __usec
...
};

This is probably ok because by now most users of this structure will
have been fixed to use input_event_sec/input_event_usec instead
of time.tv_sec/time.tv_usec. Anything that has not yet been changed
will now also break on 64-bit targets.

The sound/asound.h header is much harder to change: Removing
the __USE_TIME_BITS64 check here would require using the
new-style ioctl commands everywhere, but anything built with
that new header would break when running on linux-5.5 or
earlier.

      Arnd

Re: 64 bit time_t on riscv32

[Rich Felker]

On Mon, Jan 15, 2024 at 04:55:30PM -0300, Adhemerval Zanella Netto wrote:
> 
> 
> On 15/01/24 10:40, Florian Weimer via Libc-help wrote:
> > * Antonios Salios via Libc-help:
> > 
> >> According to a kernel maintainer, the __USE_TIME_BITS64 macro should be
> >> set on architectures that use 64-bit time [2], otherwise the kernel
> >> headers will not be able to pick the correct definition.
> > 
> > __USE_TIME_BITS64 is an internal glibc macro.  It is not used on
> > architectures which have a 64-bit time_t by default.
> > 
> > Surely the UAPI headers know which time size the architecture uses in
> > the kernel interface, and can be written arcordingly?
> 
> The use of a glibc internal definition on a kABI header is not really
> a good design.  This seems to be only user so far, so I suggest to fix
> on the kernel to not tie to a glibc internal definition. 
> 
> CCing Rich from musl that most likely would want to see this fixed. The
> Android developers might be interested.

At the time this was done, I understood __USE_TIME_BITS64 as the
contract between the userspace implementation and the kernel uapi
headers to say "we use 64-bit time_t and want the 64-bit version of
the types/ioctls/etc." That's what musl does -- we always define
__USE_TIME_BITS64, unconditionally. I haven't read this whole thread,
but it sounds like glibc's rv32 port is breaking their own contract
and not defining __USE_TIME_BITS64...

Rich

Re: 64 bit time_t on riscv32

[Adhemerval Zanella Netto]

On 15/01/24 19:26, Rich Felker wrote:
> On Mon, Jan 15, 2024 at 04:55:30PM -0300, Adhemerval Zanella Netto wrote:
>>
>>
>> On 15/01/24 10:40, Florian Weimer via Libc-help wrote:
>>> * Antonios Salios via Libc-help:
>>>
>>>> According to a kernel maintainer, the __USE_TIME_BITS64 macro should be
>>>> set on architectures that use 64-bit time [2], otherwise the kernel
>>>> headers will not be able to pick the correct definition.
>>>
>>> __USE_TIME_BITS64 is an internal glibc macro.  It is not used on
>>> architectures which have a 64-bit time_t by default.
>>>
>>> Surely the UAPI headers know which time size the architecture uses in
>>> the kernel interface, and can be written arcordingly?
>>
>> The use of a glibc internal definition on a kABI header is not really
>> a good design.  This seems to be only user so far, so I suggest to fix
>> on the kernel to not tie to a glibc internal definition. 
>>
>> CCing Rich from musl that most likely would want to see this fixed. The
>> Android developers might be interested.
> 
> At the time this was done, I understood __USE_TIME_BITS64 as the
> contract between the userspace implementation and the kernel uapi
> headers to say "we use 64-bit time_t and want the 64-bit version of
> the types/ioctls/etc." That's what musl does -- we always define
> __USE_TIME_BITS64, unconditionally. I haven't read this whole thread,
> but it sounds like glibc's rv32 port is breaking their own contract
> and not defining __USE_TIME_BITS64...

My understanding was __USE_TIME_BITS64 would be required only for ABIs
that actually would multiple time_t sizes and the glibc 64 bit time_t 
supported was added on this assumption.  However, the design document 
does state __USE_TIME_BITS64 would be defined even for architectures 
where is does not require it.  I it was not really caught in development
because the design creator was not really much involved at the time.

I think it should be feasible to fix it, although backporting it would
generate a quite large patch.

Re: 64 bit time_t on riscv32

[Florian Weimer]

* Adhemerval Zanella Netto:

> My understanding was __USE_TIME_BITS64 would be required only for ABIs
> that actually would multiple time_t sizes and the glibc 64 bit time_t 
> supported was added on this assumption.  However, the design document 
> does state __USE_TIME_BITS64 would be defined even for architectures 
> where is does not require it.  I it was not really caught in development
> because the design creator was not really much involved at the time.
>
> I think it should be feasible to fix it, although backporting it would
> generate a quite large patch.

It will break applications that use __USE_TIME_BITS64 with glibc's
current semantics centered on time64 redirects (incorrectly, of
course—it's an internal macro).  I think the use in the kernel is
probably easier to address, and it has to be changed for compatibility
with other libcs anyway.

Thanks,
Florian

Re: 64 bit time_t on riscv32

[Rich Felker]

On Tue, Jan 16, 2024 at 04:46:17PM +0100, Florian Weimer wrote:
> * Adhemerval Zanella Netto:
> 
> > My understanding was __USE_TIME_BITS64 would be required only for ABIs
> > that actually would multiple time_t sizes and the glibc 64 bit time_t 
> > supported was added on this assumption.  However, the design document 
> > does state __USE_TIME_BITS64 would be defined even for architectures 
> > where is does not require it.  I it was not really caught in development
> > because the design creator was not really much involved at the time.
> >
> > I think it should be feasible to fix it, although backporting it would
> > generate a quite large patch.
> 
> It will break applications that use __USE_TIME_BITS64 with glibc's
> current semantics centered on time64 redirects (incorrectly, of
> course—it's an internal macro).  I think the use in the kernel is
> probably easier to address, and it has to be changed for compatibility
> with other libcs anyway.

If there are applications assuming __USE_TIME_BITS64 implies redirects
and doing some hack around that, they really need to be fixed not to
do that. It will break on musl with (not yet merged) rv32 port as well
as any future 32-bit ports, which won't have any redirects because
there is no legacy 32-bit time_t ABI to support.

Rich

Re: 64 bit time_t on riscv32

[Florian Weimer]

* Rich Felker:

> On Tue, Jan 16, 2024 at 04:46:17PM +0100, Florian Weimer wrote:
>> * Adhemerval Zanella Netto:
>> 
>> > My understanding was __USE_TIME_BITS64 would be required only for ABIs
>> > that actually would multiple time_t sizes and the glibc 64 bit time_t 
>> > supported was added on this assumption.  However, the design document 
>> > does state __USE_TIME_BITS64 would be defined even for architectures 
>> > where is does not require it.  I it was not really caught in development
>> > because the design creator was not really much involved at the time.
>> >
>> > I think it should be feasible to fix it, although backporting it would
>> > generate a quite large patch.
>> 
>> It will break applications that use __USE_TIME_BITS64 with glibc's
>> current semantics centered on time64 redirects (incorrectly, of
>> course—it's an internal macro).  I think the use in the kernel is
>> probably easier to address, and it has to be changed for compatibility
>> with other libcs anyway.
>
> If there are applications assuming __USE_TIME_BITS64 implies redirects
> and doing some hack around that, they really need to be fixed not to
> do that. It will break on musl with (not yet merged) rv32 port as well
> as any future 32-bit ports, which won't have any redirects because
> there is no legacy 32-bit time_t ABI to support.

That's actually not a problem because __USE_TIME_BITS64 is not defined
on rv32 et al. (hence the complaint about what the UAPI headers are
doing).

Thanks,
Florian

Re: 64 bit time_t on riscv32

[Adhemerval Zanella Netto]

On 16/01/24 13:22, Florian Weimer wrote:
> * Rich Felker:
> 
>> On Tue, Jan 16, 2024 at 04:46:17PM +0100, Florian Weimer wrote:
>>> * Adhemerval Zanella Netto:
>>>
>>>> My understanding was __USE_TIME_BITS64 would be required only for ABIs
>>>> that actually would multiple time_t sizes and the glibc 64 bit time_t 
>>>> supported was added on this assumption.  However, the design document 
>>>> does state __USE_TIME_BITS64 would be defined even for architectures 
>>>> where is does not require it.  I it was not really caught in development
>>>> because the design creator was not really much involved at the time.
>>>>
>>>> I think it should be feasible to fix it, although backporting it would
>>>> generate a quite large patch.
>>>
>>> It will break applications that use __USE_TIME_BITS64 with glibc's
>>> current semantics centered on time64 redirects (incorrectly, of
>>> course—it's an internal macro).  I think the use in the kernel is
>>> probably easier to address, and it has to be changed for compatibility
>>> with other libcs anyway.
>>
>> If there are applications assuming __USE_TIME_BITS64 implies redirects
>> and doing some hack around that, they really need to be fixed not to
>> do that. It will break on musl with (not yet merged) rv32 port as well
>> as any future 32-bit ports, which won't have any redirects because
>> there is no legacy 32-bit time_t ABI to support.
> 
> That's actually not a problem because __USE_TIME_BITS64 is not defined
> on rv32 et al. (hence the complaint about what the UAPI headers are
> doing).

A possible fix would just check the redirection with 

  if __USE_TIME_BITS64 && __TIMESIZE == 32

The code is already poking an implementation detail, so it can just
continue to mimic what the installed header would do.

Re: 64 bit time_t on riscv32

[Rich Felker]

On Tue, Jan 16, 2024 at 05:22:55PM +0100, Florian Weimer wrote:
> * Rich Felker:
> 
> > On Tue, Jan 16, 2024 at 04:46:17PM +0100, Florian Weimer wrote:
> >> * Adhemerval Zanella Netto:
> >> 
> >> > My understanding was __USE_TIME_BITS64 would be required only for ABIs
> >> > that actually would multiple time_t sizes and the glibc 64 bit time_t 
> >> > supported was added on this assumption.  However, the design document 
> >> > does state __USE_TIME_BITS64 would be defined even for architectures 
> >> > where is does not require it.  I it was not really caught in development
> >> > because the design creator was not really much involved at the time.
> >> >
> >> > I think it should be feasible to fix it, although backporting it would
> >> > generate a quite large patch.
> >> 
> >> It will break applications that use __USE_TIME_BITS64 with glibc's
> >> current semantics centered on time64 redirects (incorrectly, of
> >> course—it's an internal macro).  I think the use in the kernel is
> >> probably easier to address, and it has to be changed for compatibility
> >> with other libcs anyway.
> >
> > If there are applications assuming __USE_TIME_BITS64 implies redirects
> > and doing some hack around that, they really need to be fixed not to
> > do that. It will break on musl with (not yet merged) rv32 port as well
> > as any future 32-bit ports, which won't have any redirects because
> > there is no legacy 32-bit time_t ABI to support.
> 
> That's actually not a problem because __USE_TIME_BITS64 is not defined
> on rv32 et al. (hence the complaint about what the UAPI headers are
> doing).

It is a problem for musl users because we *do* define
__USE_TIME_BITS64 and do so unconditionally, on all archs, including
future rv32. Even if we could opt not to define it on rv32, that would
not help with future 32-bit ports like sparc where there will be no
redirection but __USE_TIME_BITS64 is needed to get the kernel headers
to do the right thing. So if any applications are really misusing
__USE_TIME_BITS64 as an indicator of having symbol redirections, they
need to stop/get fixed.

Rich