[RFC,0/6] y2038: Prepare glibc to be Y2038 safe for 32 bit ports

Message ID 20201204233604.7430-1-lukma@denx.de
Headers
Series y2038: Prepare glibc to be Y2038 safe for 32 bit ports |

Message

Lukasz Majewski Dec. 4, 2020, 11:35 p.m. UTC
  This patch series is the RFC for preparing glibc to be Y2038 safe on
ports with __WORDSIZE=32 && __TIMESIZE!=64 (e.g. 32 bit ARM).

First patch provides necessary definitions and redirections to
achieve the above task.

Following patches are adjustments to glibc to provide proper handling
of 64 bit time in some exported and local structures. Those patches
shall be applied before the first one is pulled.

This patch set causes following Y2038 tests being all passed:
https://github.com/lmajewski/y2038-tests/

Those tests are run as part of meta-y2038:
https://github.com/lmajewski/meta-y2038/

Lukasz Majewski (6):
  y2038: Introduce _TIME_BITS flag to support 64 bit time on 32 bit
    systems
  y2038: stat: {f}stat{at}64_time64 redirection to be used on Y2038
    systems
  y2038: Export struct_stat_time64_helper.h with Y2038 safe stat{64}
    content
  y2038: Enhance struct msqid_ds to support 64 bit time
  msqid: Provide internal copy of struct __msqid64_ds
  msg: provide glibc local copy of struct msqid_ds

 include/bits/types/struct_msqid64_ds.h        |  36 +++++
 include/features.h                            |  19 +++
 io/Versions                                   |   6 +
 io/sys/poll.h                                 |  11 ++
 io/sys/stat.h                                 | 100 +++++++++++-
 io/utime.h                                    |  15 ++
 manual/creature.texi                          |  28 ++++
 misc/Versions                                 |   5 +
 misc/sys/select.h                             |  27 ++++
 nptl/Versions                                 |  15 ++
 posix/Versions                                |   2 +
 posix/sched.h                                 |   9 ++
 posix/sys/wait.h                              |  20 +++
 resolv/Versions                               |   3 +
 resolv/netdb.h                                |  11 ++
 resource/Versions                             |   1 +
 resource/bits/types/struct_rusage.h           |   5 +
 resource/sys/resource.h                       |  10 ++
 rt/Versions                                   |   7 +
 rt/aio.h                                      |  25 ++-
 rt/mqueue.h                                   |  25 +++
 signal/Versions                               |   3 +
 signal/signal.h                               |  12 ++
 socket/sys/socket.h                           |  11 ++
 sysdeps/nptl/pthread.h                        | 117 ++++++++++++++
 sysdeps/pthread/semaphore.h                   |  23 +++
 sysdeps/pthread/threads.h                     |  33 ++++
 sysdeps/unix/sysv/linux/Makefile              |   3 +-
 sysdeps/unix/sysv/linux/Versions              |   7 +
 sysdeps/unix/sysv/linux/bits/struct_stat.h    |  17 +-
 .../linux/bits/struct_stat_time64_helper.h    |  70 +++++++++
 sysdeps/unix/sysv/linux/bits/time.h           |  10 ++
 sysdeps/unix/sysv/linux/bits/timex.h          |  31 ++++
 .../sysv/linux/bits/types/struct_msqid64_ds.h |   4 -
 .../sysv/linux/bits/types/struct_msqid_ds.h   |   8 +
 .../linux/hppa/bits/types/struct_msqid_ds.h   |   8 +
 sysdeps/unix/sysv/linux/include/sys/msg.h     |  66 +++++++-
 .../unix/sysv/linux/m68k/bits/struct_stat.h   |  16 ++
 .../sysv/linux/microblaze/bits/struct_stat.h  |  16 ++
 .../unix/sysv/linux/mips/bits/struct_stat.h   |  16 ++
 .../linux/mips/bits/types/struct_msqid_ds.h   |  12 +-
 .../sysv/linux/powerpc/bits/struct_stat.h     |  48 ++++--
 .../powerpc/bits/types/struct_msqid_ds.h      |   8 +
 .../linux/sparc/bits/types/struct_msqid_ds.h  |   8 +
 sysdeps/unix/sysv/linux/struct_stat_time64.h  |  60 ++-----
 sysdeps/unix/sysv/linux/sys/timerfd.h         |  22 +++
 sysdeps/unix/sysv/linux/sys/timex.h           |  37 ++++-
 .../unix/sysv/linux/x86/bits/struct_stat.h    |  16 ++
 sysvipc/Versions                              |   2 +
 sysvipc/sys/msg.h                             |  10 ++
 sysvipc/sys/sem.h                             |  21 +++
 sysvipc/sys/shm.h                             |  10 ++
 time/Versions                                 |  24 +++
 time/bits/types/struct_timespec.h             |  17 +-
 time/bits/types/struct_timeval.h              |   5 +
 time/bits/types/time_t.h                      |   4 +
 time/sys/time.h                               |  91 +++++++++++
 time/time.h                                   | 146 ++++++++++++++++++
 58 files changed, 1294 insertions(+), 98 deletions(-)
 create mode 100644 include/bits/types/struct_msqid64_ds.h
 create mode 100644 sysdeps/unix/sysv/linux/bits/struct_stat_time64_helper.h
  

Comments

Joseph Myers Dec. 5, 2020, 12:01 a.m. UTC | #1
On Sat, 5 Dec 2020, Lukasz Majewski wrote:

> This patch series is the RFC for preparing glibc to be Y2038 safe on
> ports with __WORDSIZE=32 && __TIMESIZE!=64 (e.g. 32 bit ARM).

What exactly does "preparing" mean here?

I'd expect support for _TIME_BITS to go along with public symbol versions 
for all the new symbols that functions get redirected to, because 
otherwise people building with _TIME_BITS=64 will get either link failures 
or broken binaries depending on GLIBC_PRIVATE symbols.  But as far as I 
can see, this patch series only adds GLIBC_PRIVATE symbols, not any public 
symbols or corresponding abilist updates.

I'd also expect support for _TIME_BITS to go along with tests covering all 
the new interfaces (that is, for every time-affected interface, there 
should be a test that it works properly with _TIME_BITS=64, typically a 
test that defines _TIME_BITS to 64 then includes an existing test for that 
interface, possibly with other tests to verify that large time values work 
properly where possible).

You don't say how this series was tested, but this may be around the point 
where build-many-glibcs.py testing is needed.  Certainly any patch in such 
a series that adds a new public symbol should be tested with 
build-many-glibcs.py to make sure that all the ABI test baseline updates 
are correct.
  
Lukasz Majewski Dec. 7, 2020, 10:22 a.m. UTC | #2
Hi Joseph,

> On Sat, 5 Dec 2020, Lukasz Majewski wrote:
> 
> > This patch series is the RFC for preparing glibc to be Y2038 safe on
> > ports with __WORDSIZE=32 && __TIMESIZE!=64 (e.g. 32 bit ARM).  
> 
> What exactly does "preparing" mean here?

In this particular case - I meant that all the patches in this series
are necessary to provide Y2038 support. Please also be aware that this
is only the RFC.

To be more specific - only patch 1/6 will be the "transitional" patch
in a sense that after applying it the _TIME_BITS flag would be
introduced and ready to be use by users.

Patches following it - i.e. from 2 to 6 are added to show what is
needed to tune glibc to have 64 bit time support. Those are mostly
related to earlier 'stat' rework done by Adhemerval and shall be
applied (or the problem shall be resolved in the other way) before patch
1/6 is applied.

I just wanted to wrap all the necessary patches in a single patch
series to show how much work is still necessary.

> 
> I'd expect support for _TIME_BITS to go along with public symbol
> versions for all the new symbols that functions get redirected to,
> because otherwise people building with _TIME_BITS=64 will get either
> link failures or broken binaries depending on GLIBC_PRIVATE symbols.
> But as far as I can see, this patch series only adds GLIBC_PRIVATE
> symbols, not any public symbols or corresponding abilist updates.

You are right - this was my omission. I've added those new symbols to
GLIBC_PRIVATE as it was easier for maintenance.

For the "final" version of this patch set I will add all those symbols
to GLIBC_2.3X {}.

> 
> I'd also expect support for _TIME_BITS to go along with tests
> covering all the new interfaces (that is, for every time-affected
> interface, there should be a test that it works properly with
> _TIME_BITS=64, typically a test that defines _TIME_BITS to 64 then
> includes an existing test for that interface, possibly with other
> tests to verify that large time values work properly where possible).

This is a bit problematic as was discussed earlier [1] as we don't
have the easy way to set host's (on which tests are running) time.

The time namespaces don't support CLOCK_REALTIME, and the only feasible
way to do this is with re-using (and optimizing):
test-wrapper='/opt/glibc/src/scripts/cross-test-ssh.sh

I'm also concerned how this work shall be divided - the patch which
introduces _TIME_BITS flag would be very large and difficult to review.
Would it be allowed to add tests (which would use _TIME_BITS) to glibc
tree earlier?

> 
> You don't say how this series was tested,

This patch set causes following Y2038 tests being all passed:
https://github.com/lmajewski/y2038-tests/

Those tests are run as part of meta-y2038:
https://github.com/lmajewski/meta-y2038/

Those are "simple" programs to test single syscall (e.g. clock_settime
or stat) built with _TIME_BITS=64 and _FILE_OFFSET=64 and run on QEMU
(arm32 bit vexpress-c15 board).

This is the only way to be able to set future time after time_t
overflow.

> but this may be around the
> point where build-many-glibcs.py testing is needed. 

In the thread [2] - I've suggested such approach, but this would imply
setting/changing time on the build host.

To overcome this problem we would need time namespaces, but for now
there is no plan to provide such functionality ([1]).

> Certainly any
> patch in such a series that adds a new public symbol should be tested
> with build-many-glibcs.py to make sure that all the ABI test baseline
> updates are correct.
> 

Would it be possible to test e.g. clock_settime64 if it sets correct
time after 32 bit time_t overflow? Any other syscall would require time
adjustment of host to properly test execution paths.

Would you recommend extending build-many-glibc to also download, build
qemu and run tests inside?

In the discussion [1] it was pointed out that this would be a bit
cumbersome. 

Last but not least, just the "build" testing shall be provided with
current setup as for e.g. __WORDSIZE=32 and __TIMESIZE!=64 (e.g. arm32
bit) we do use 64 bit interfaces anyway.


Links:
[1] - https://lore.kernel.org/lkml/20201114102503.GB1000@bug/T/
[2] -
http://sourceware-org.1504.n7.nabble.com/Y2038-Question-about-porting-y2038-tests-to-glibc-td652557.html

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
  
Joseph Myers Dec. 7, 2020, 6:01 p.m. UTC | #3
On Mon, 7 Dec 2020, Lukasz Majewski wrote:

> > I'd also expect support for _TIME_BITS to go along with tests
> > covering all the new interfaces (that is, for every time-affected
> > interface, there should be a test that it works properly with
> > _TIME_BITS=64, typically a test that defines _TIME_BITS to 64 then
> > includes an existing test for that interface, possibly with other
> > tests to verify that large time values work properly where possible).
> 
> This is a bit problematic as was discussed earlier [1] as we don't
> have the easy way to set host's (on which tests are running) time.

I'd like the _TIME_BITS=64 interfaces to be covered as well as the 
existing interfaces.  That is, test what can readily be covered with the 
existing testsuite infrastructure, but not things that require setting the 
system clock.  (You might be able to e.g. test sleeping for 2^32 seconds 
and interrupting that sleep after a second or two.)

E.g. pthread_mutex_timedlock / pthread_mutex_clocklock / mtx_timedlock are 
functions with a complicated implementation and several tests (in nptl/ 
and sysdeps/pthread/).  Whether or not there are further things to test 
specific to 64-bit time, it's definitely straightforward to add tests that 
define _TIME_BITS=64 then include one of the existing tests for one of 
those functions - so ensuring similar test coverage for both versions of 
each such function.

If any of the functions being changed aren't covered in the testsuite at 
all, but can be tested meaningfully as an unprivileged user, it would be a 
good idea to add basic tests of those functions that get run both with and 
without _TIME_BITS=64.

> Would it be allowed to add tests (which would use _TIME_BITS) to glibc
> tree earlier?

I think that's fine - certainly for tests which just define _TIME_BITS 
before including another test and will work for both 32-bit and 64-bit 
time_t.

The commit that causes installed headers to support redirecting interfaces 
when _TIME_BITS is defined to 64 should come last - there shouldn't be an 
intermediate state where a user defining _TIME_BITS=64 gets some 
interfaces redirected by installed headers and others not redirected.  
But before that final change to installed headers, there can be any number 
of preparatory commits (possibly including ones that add redirection 
support to individual headers, if the features.h patch that handles 
_TIME_BITS comes last).

I'm not sure if there is going to be any support for _TIME_BITS=64 for 
Hurd - the minimum requirement there is simply not to provide an 
inconsistent ABI (so either no header redirections, or all header 
redirections operate and go to working functions) and not to break the 
build of glibc or the testsuite.

> Would you recommend extending build-many-glibc to also download, build
> qemu and run tests inside?

My recommendation is simply to add those tests of the new interfaces that 
are reasonably straightforward within the existing testsuite and don't 
require running QEMU or changing the system clock or anything needing 
privileged access or unsuitable for running on a shared system.

Adding extra support for building and running QEMU should be considered a 
separate matter, quite possibly useful to help people test glibc patches 
more thoroughly where appropriate but not expected to be done as part of 
the Y2038 work.
  
Lukasz Majewski Dec. 8, 2020, 10:07 a.m. UTC | #4
Hi Joseph,

> On Mon, 7 Dec 2020, Lukasz Majewski wrote:
> 
> > > I'd also expect support for _TIME_BITS to go along with tests
> > > covering all the new interfaces (that is, for every time-affected
> > > interface, there should be a test that it works properly with
> > > _TIME_BITS=64, typically a test that defines _TIME_BITS to 64 then
> > > includes an existing test for that interface, possibly with other
> > > tests to verify that large time values work properly where
> > > possible).  
> > 
> > This is a bit problematic as was discussed earlier [1] as we don't
> > have the easy way to set host's (on which tests are running) time.  
> 
> I'd like the _TIME_BITS=64 interfaces to be covered as well as the 
> existing interfaces.  That is, test what can readily be covered with
> the existing testsuite infrastructure, but not things that require
> setting the system clock.  (You might be able to e.g. test sleeping
> for 2^32 seconds and interrupting that sleep after a second or two.)
> 
> E.g. pthread_mutex_timedlock / pthread_mutex_clocklock /
> mtx_timedlock are functions with a complicated implementation and
> several tests (in nptl/ and sysdeps/pthread/).  Whether or not there
> are further things to test specific to 64-bit time, it's definitely
> straightforward to add tests that define _TIME_BITS=64 then include
> one of the existing tests for one of those functions - so ensuring
> similar test coverage for both versions of each such function.
> 
> If any of the functions being changed aren't covered in the testsuite
> at all, but can be tested meaningfully as an unprivileged user, it
> would be a good idea to add basic tests of those functions that get
> run both with and without _TIME_BITS=64.

Ok, so if I understood you correctly - I shall devise tests, which can
be run as unprivileged user and if possible reuse existing tests with
_TIME_BITS=64 defined.

> 
> > Would it be allowed to add tests (which would use _TIME_BITS) to
> > glibc tree earlier?  
> 
> I think that's fine - certainly for tests which just define
> _TIME_BITS before including another test and will work for both
> 32-bit and 64-bit time_t.
> 
> The commit that causes installed headers to support redirecting
> interfaces when _TIME_BITS is defined to 64 should come last - there
> shouldn't be an intermediate state where a user defining
> _TIME_BITS=64 gets some interfaces redirected by installed headers
> and others not redirected. But before that final change to installed
> headers, there can be any number of preparatory commits (possibly
> including ones that add redirection support to individual headers, if
> the features.h patch that handles _TIME_BITS comes last).

Ok, I will prepare proper patch - as I do already have all the
redirections for already converted syscalls.

In that way we would avoid one bit patch, which would add _TIME_BITS
support.

> 
> I'm not sure if there is going to be any support for _TIME_BITS=64
> for Hurd

I've focused on the glibc ports, which use Linux as the kernel (mostly
ARM 32 bit). 

I do NOT have any plans to also support HURD.

> - the minimum requirement there is simply not to provide an 
> inconsistent ABI (so either no header redirections,

I would opt for the above option - no redirections for HURD.

> or all header 
> redirections operate and go to working functions) and not to break
> the build of glibc or the testsuite.

All exported headers' redirections are "protected" by:
"#ifdef __USE_TIME_BITS64"

And __USE_TIME_BITS64 will be only defined when _TIME_BITS=64 &&
_FILE_OFFSET_BITS=64 && __TIMESIZE != 64

I think that the most pragmatic approach would be for HURD to:

1. Introduce new header - ./sysdeps/hurd/include/features.h
2. In this file:

	#include_next <features.h>
	/*
	 * HURD is not (yet) supporting 64 bit time on archs with
	 * __TIMESIZE!=64 && __WORDSIZE=32
	 * For that reason we explicitly undefine the __USE_TIME_BITS64
	 */
	#undef __USE_TIME_BITS64

or even better:

	#ifdef __USE_TIME_BITS64
	# error "On i686-gnu HURD the _TIME_BITS flag is not supported!"
	#endif


In that way we would either silently disable Y2038 suport on HURD or
(preferable) prevent compilation with the error information.

> 
> > Would you recommend extending build-many-glibc to also download,
> > build qemu and run tests inside?  
> 
> My recommendation is simply to add those tests of the new interfaces
> that are reasonably straightforward within the existing testsuite and
> don't require running QEMU or changing the system clock or anything
> needing privileged access or unsuitable for running on a shared
> system.

Ok. Thanks for the clarification.

> 
> Adding extra support for building and running QEMU should be
> considered a separate matter, quite possibly useful to help people
> test glibc patches more thoroughly where appropriate but not expected
> to be done as part of the Y2038 work.
> 

I could add / port / maintain the meta-y2038 on the sourceware.org or
Yocto, so people could use Y2038 safe glibc on embedded systems. For
now I do maintain it on GitHub:
https://github.com/lmajewski/meta-y2038/blob/master/README


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
  
Joseph Myers Dec. 8, 2020, 3:25 p.m. UTC | #5
On Tue, 8 Dec 2020, Lukasz Majewski wrote:

> 1. Introduce new header - ./sysdeps/hurd/include/features.h
> 2. In this file:
> 
> 	#include_next <features.h>

<features.h> is an *installed* header, and sysdeps/*/include/ is only for 
*non-installed* wrappers used when building glibc itself.  So this 
approach can't work.  (But you could e.g. have two different 
bits/features-time64.h headers, one in the toplevel bits/ and one under 
sysdeps, so that the appropriate one is installed, and include 
<bits/features-time64.h> from <features.h>.)
  
Lukasz Majewski Dec. 13, 2020, 11:49 a.m. UTC | #6
Hi Joseph,

> On Tue, 8 Dec 2020, Lukasz Majewski wrote:
> 
> > 1. Introduce new header - ./sysdeps/hurd/include/features.h
> > 2. In this file:
> > 
> > 	#include_next <features.h>  
> 
> <features.h> is an *installed* header, and sysdeps/*/include/ is only
> for *non-installed* wrappers used when building glibc itself.  So
> this approach can't work.  (But you could e.g. have two different 
> bits/features-time64.h headers, one in the toplevel bits/ and one
> under sysdeps, so that the appropriate one is installed, and include 
> <bits/features-time64.h> from <features.h>.)
> 

Ok, I will do this in that way. Thanks for sharing the idea.


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