[Y2038] Replacement of struct timespec with struct __timespec64 in glibc internal code

[Y2038] Replacement of struct timespec with struct __timespec64 in glibc internal code

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 2286 bytes --]

Dear Community,

I would like to bring up one relevant Y2038 support issue  - to be more
specific - the conversion/replacement of struct timespec to struct
__timespec64 in glibc internal code (source tree).

For example - I would like to convert nptl and pthreads to be Y2038
safe. To do that the timespec_* helpers and some functions, which use
futex_time64 syscall, (from 5.1+) need to use struct __timespec64. 

The problem is with instant replacement of struct timespec with
struct __timespec64 in glibc internal code (and tests). 


To do it I could:
- Replace its occurences in relevant directories - like ./nptl or
  ./sysdeps/pthread - i.e. rename all occurrences in a single directory
 
- Replace them in functions (tests) and use explicit conversion
  functions - like valid_timespec_to_timespec64() before passing struct
  __timespec64 arguments (like ones for futex_time64 for nptl).

- Replace _all_ occurrences in glibc tree of struct timespec with struct
  __timespec64 at once with using sed on the glibc tree.

The last option seems to be the most appealing as we already use
__timespec64 (with its aliasing) for some core system syscalls (like
clock_gettime). 
However, such patch shall be applied just after the release of new
stable glibc version (August 2020?) to have time for potential fixes.


Which options shall we use?



A few more related questions:
- Shall tests in ./nptl and ./sysdeps/pthread [*] use struct
  __timespec64 or struct timespec? 
  From my understanding tests (like ./nptl/tst-*) use exported headers
  so struct timespec for them is struct __timespec64 anyway for archs
  with __WORDSIZE == 32 and __TIMESIZE !=64.

- Other time related structures needs to be converted as well - like
  struct itimerspec. 


[*] - from running scripts/build-many-glibcs.py it looks like only
i686-gnu port (HURD) is using code in ./sysdeps/pthread. Will
./sysdeps/pthread be replaced by nptl in some near time in the future
(and removed)? 



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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Y2038] Replacement of struct timespec with struct __timespec64 in glibc internal code

[Joseph Myers]

On Wed, 3 Jun 2020, Lukasz Majewski wrote:

> To do it I could:
> - Replace its occurences in relevant directories - like ./nptl or
>   ./sysdeps/pthread - i.e. rename all occurrences in a single directory
>  
> - Replace them in functions (tests) and use explicit conversion
>   functions - like valid_timespec_to_timespec64() before passing struct
>   __timespec64 arguments (like ones for futex_time64 for nptl).
> 
> - Replace _all_ occurrences in glibc tree of struct timespec with struct
>   __timespec64 at once with using sed on the glibc tree.

Using sed obviously won't work, since external interfaces need different 
handling from purely internal uses.  I think you need to change things bit 
by bit, in sufficiently small patches for convenient review.

Regarding tests, I expect many tests of time-related interfaces should end 
up being built and run twice on systems that currently use 32-bit time, 
once to test the interfaces with 32-bit time and once to test the 
interfaces with 64-bit time.  Also, tests can't generally use 64-bit time 
interfaces from libc until _TIME_BITS=64 support is actually implemented.  
So I think tests would be one of the last places to change (and similarly 
installed executables).  Whereas any internal use of time in a function in 
the libraries that does not involve time in its interface can be updated 
more or less independently of any other such use, provided the relevant 
internal interfaces using 64-bit time are now available.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [Y2038] Replacement of struct timespec with struct __timespec64 in glibc internal code

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 2027 bytes --]

Hi Joseph,

> On Wed, 3 Jun 2020, Lukasz Majewski wrote:
> 
> > To do it I could:
> > - Replace its occurences in relevant directories - like ./nptl or
> >   ./sysdeps/pthread - i.e. rename all occurrences in a single
> > directory 
> > - Replace them in functions (tests) and use explicit conversion
> >   functions - like valid_timespec_to_timespec64() before passing
> > struct __timespec64 arguments (like ones for futex_time64 for nptl).
> > 
> > - Replace _all_ occurrences in glibc tree of struct timespec with
> > struct __timespec64 at once with using sed on the glibc tree.  
> 
> Using sed obviously won't work, since external interfaces need
> different handling from purely internal uses.  I think you need to
> change things bit by bit, in sufficiently small patches for
> convenient review.

Considering the above comment - it seems like it would be best to
replace struct timespec with struct __timespec64 in directories - like
./nptl and omit tests from converison.

> 
> Regarding tests, I expect many tests of time-related interfaces
> should end up being built and run twice on systems that currently use
> 32-bit time, once to test the interfaces with 32-bit time and once to
> test the interfaces with 64-bit time.  Also, tests can't generally
> use 64-bit time interfaces from libc until _TIME_BITS=64 support is
> actually implemented. So I think tests would be one of the last
> places to change (and similarly installed executables).  Whereas any
> internal use of time in a function in the libraries that does not
> involve time in its interface can be updated more or less
> independently of any other such use, provided the relevant internal
> interfaces using 64-bit time are now available.
> 




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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Y2038] Replacement of struct timespec with struct __timespec64 in glibc internal code

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 3656 bytes --]

Hi Joseph,

> On Wed, 3 Jun 2020, Lukasz Majewski wrote:
> 
> > To do it I could:
> > - Replace its occurences in relevant directories - like ./nptl or
> >   ./sysdeps/pthread - i.e. rename all occurrences in a single
> > directory 
> > - Replace them in functions (tests) and use explicit conversion
> >   functions - like valid_timespec_to_timespec64() before passing
> > struct __timespec64 arguments (like ones for futex_time64 for nptl).
> > 
> > - Replace _all_ occurrences in glibc tree of struct timespec with
> > struct __timespec64 at once with using sed on the glibc tree.  
> 
> Using sed obviously won't work, since external interfaces need
> different handling from purely internal uses.  I think you need to
> change things bit by bit, in sufficiently small patches for
> convenient review.
> 
> Regarding tests, I expect many tests of time-related interfaces
> should end up being built and run twice on systems that currently use
> 32-bit time, once to test the interfaces with 32-bit time and once to
> test the interfaces with 64-bit time.  

Does it mean that I shall NOT make the struct timespec to struct
__timespec64 conversion in glibc tests (e.g.
./sysdeps/pthread/tst-mutex5.c). Would those tests build infrastructure
be changed to build them w/ _TIME_BITS=64 support?


One thing puzzles me though - it seems like xclock_gettime, xclock_now,
timespec_add, timespec_sub are used by glibc tests (#include
<support/timespec.h>).

Am I correct that those functions are not exported and used solely
inside glibc?

The problem is that I do need to convert them to support 64 bit time as
pthreads use them (sysdeps/pthread/timer_settime.c). This also means
that I would need to update tests (otherwise I would experience
build/tests errors), which you don't recommend before the _TIME_BITS=64
support is added.

How to proceed in this case? Two big parts of Y2038 support are still
missing:

- One is stat and friends (which may be simplified when we move forward
  with minimal kernel version supported)

- And second are pthreads (nptl).

Pthreads are important for user space applications - so they shall be
converted sooner than latter IMHO.

> Also, tests can't generally
> use 64-bit time interfaces from libc until _TIME_BITS=64 support is
> actually implemented. 

Does it mean that tests - like sysdeps/pthread/tst-mutex5.c - will
always use exported struct timespec? (aliased to 64 bit struct 
__timespec64 when needed)?

> So I think tests would be one of the last
> places to change (and similarly installed executables). 

> Whereas any
> internal use of time in a function in the libraries that does not
> involve time in its interface can be updated more or less
> independently of any other such use, provided the relevant internal
> interfaces using 64-bit time are now available.
> 

As I stated above - e.g. timespec_sub() helper function (from /support)
is used by both pthreats and tests.

Despite it is internal function - shall I follow the pattern and
rewrite it as:

__timespec_add64(... struct __timespec64)
{

..

}
#if __TIMESIZE != 64
libc_hidden_def (__timespec_add64)

__timespec_add(... struct timespec64)
{
	call __timespec_add64();

}

However, the above approach seems to me like an overkill to do this for
internally used support function.



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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Y2038] Replacement of struct timespec with struct __timespec64 in glibc internal code

[Andreas Schwab]

On Jun 24 2020, Lukasz Majewski wrote:

> As I stated above - e.g. timespec_sub() helper function (from /support)
> is used by both pthreats and tests.

There are two definitions of timespec_sub, in support/timespec-sub.c and
in sysdeps/pthread/posix-timer.h.  The former is for use in tests, the
latter is used by libpthread.

The functions in support/timespec-*.c should probably be renamed to
xtimespec_*.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [Y2038] Replacement of struct timespec with struct __timespec64 in glibc internal code

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 931 bytes --]

Hi Andreas,

> On Jun 24 2020, Lukasz Majewski wrote:
> 
> > As I stated above - e.g. timespec_sub() helper function (from
> > /support) is used by both pthreats and tests.  
> 
> There are two definitions of timespec_sub, in support/timespec-sub.c
> and in sysdeps/pthread/posix-timer.h.  The former is for use in
> tests, the latter is used by libpthread.

So if I understood it correctly - the support/* functions are to
facilitate writing tests?

> 
> The functions in support/timespec-*.c should probably be renamed to
> xtimespec_*.

And the 'x' prefix means that those functions are supposed to be used
for testing?

> 
> Andreas.
> 




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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Y2038] Replacement of struct timespec with struct __timespec64 in glibc internal code

[Joseph Myers]

On Wed, 24 Jun 2020, Lukasz Majewski wrote:

> > The functions in support/timespec-*.c should probably be renamed to
> > xtimespec_*.
> 
> And the 'x' prefix means that those functions are supposed to be used
> for testing?

The 'x' prefix is in the style of xmalloc as used in many GNU packages - 
checking for errors in a safer way than the default libc API.

Most of the 'x' functions are thereby unsuitable for use in library code, 
but may be used in tests and in code that goes into the various 
executables shipped with glibc, because they exit the program on error and 
library code should generally not do that.  (The 'x' names are also not in 
the implementation namespace; non-static internal functions used within 
library code need to have reserved names such as __*.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [Y2038] Replacement of struct timespec with struct __timespec64 in glibc internal code

[Joseph Myers]

On Wed, 24 Jun 2020, Lukasz Majewski wrote:

> > Also, tests can't generally
> > use 64-bit time interfaces from libc until _TIME_BITS=64 support is
> > actually implemented. 
> 
> Does it mean that tests - like sysdeps/pthread/tst-mutex5.c - will
> always use exported struct timespec? (aliased to 64 bit struct 
> __timespec64 when needed)?

Yes.  Tests normally use public interfaces, not internal ones.  Where a 
test needs to use internal interfaces related to time, such interfaces may 
end up needing variants for different sizes of time_t (but unless they 
actually result in functions defined in installed shared libraries and 
used both from those shared libraries and from other installed libraries 
or executables, the hidden_def mechanism is not relevant for such 
interfaces).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [Y2038] Replacement of struct timespec with struct __timespec64 in glibc internal code

[Samuel Thibault]

Hello,

Lukasz Majewski, le mer. 03 juin 2020 14:53:47 +0200, a ecrit:
> [*] - from running scripts/build-many-glibcs.py it looks like only
> i686-gnu port (HURD) is using code in ./sysdeps/pthread.

? No, nptl also uses e.g. the C11 threads implementation that is there.

> Will ./sysdeps/pthread be replaced by nptl in some near time in the
> future (and removed)?

Ideally nptl and htl (and fbtl of the freebsd port) would be merged.
Apparently nptl is not as linux-specific as I could fear, and now
that gnumach has a futex-like interface (gsync) possibly it could
just be used, but it won't be just a snap, htl implements at least
some cancel support that we need in userland filesystem support
(pthread_hurd_cond_timedwait_np), and also the pthread cancel, signal,
TLS supports need to be plugged. I tend to make htl interface like nptl
with the libc, so that should be feasibly long-term.

Samuel