Message ID | CAKmqyKMdsVZg_ZS7fLTt23dy2vnZc1z85b_+8heiK-8UiqMx+g@mail.gmail.com |
---|---|
State | Superseded |
Headers |
Received: (qmail 89801 invoked by alias); 6 Sep 2019 16:57:35 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 89792 invoked by uid 89); 6 Sep 2019 16:57:34 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-17.4 required=5.0 tests=AWL, BAYES_00, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=rv32 X-HELO: mail-lj1-f194.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=PT17V4psdd/Li1am3fe3xOCe7tGb7wQiWekalaIUyiU=; b=NXMoEz9bZpA84lyrhbOr1qc7vhejfjlirNi5/6Ux7kzjOaN+6rhvY6xTTLloQwjC4T 86yQ21zZG6uzhwTQJ91+tHSEdbmww3LToGlVpquU7GP5e+0mH6tV1gAOagC/nQ6ROOLg ITq1n70aru+RWmlVAkMyggR/hfvQ6qel6d85d6WSGZwX/dPhutZP21FEzTcZG4PAA4Nz PoyT2VdCLpJ1jg+jhYijdRYB1UwQpFvtyXPDWEaSUuGiJxRU3qFxC1NazHokCTJ5+Bm2 LngvTqBxTI7S5WDW2zW5jz4M9jJjhcxfAEg1XriL+q5S6G1qDinykdchFinoL/PBb+GO Vrng== MIME-Version: 1.0 References: <20190906145911.30207-1-lukma@denx.de> In-Reply-To: <20190906145911.30207-1-lukma@denx.de> From: Alistair Francis <alistair23@gmail.com> Date: Fri, 6 Sep 2019 09:52:58 -0700 Message-ID: <CAKmqyKMdsVZg_ZS7fLTt23dy2vnZc1z85b_+8heiK-8UiqMx+g@mail.gmail.com> Subject: Re: [PATCH v7 0/3] y2038: Linux: Introduce __clock_settime64 function To: Lukasz Majewski <lukma@denx.de> Cc: Joseph Myers <joseph@codesourcery.com>, Zack Weinberg <zackw@panix.com>, Arnd Bergmann <arnd@arndb.de>, Alistair Francis <alistair.francis@wdc.com>, GNU C Library <libc-alpha@sourceware.org>, Adhemerval Zanella <adhemerval.zanella@linaro.org>, Florian Weimer <fweimer@redhat.com>, "Carlos O'Donell" <carlos@redhat.com>, Stepan Golosunov <stepan@golosunov.pp.ru> Content-Type: text/plain; charset="UTF-8" |
Commit Message
Alistair Francis
Sept. 6, 2019, 4:52 p.m. UTC
On Fri, Sep 6, 2019 at 7:59 AM Lukasz Majewski <lukma@denx.de> wrote: > > This patch set introduces the conversion of clock_settime to explicit > 64 bit struct __timespec64 arguments. As a result this function is now > Y2038 safe. > > This work is (loosely) based on a previous development/patches: > https://libc-alpha.sourceware.narkive.com/zniMOWui/rfc-patch-00-52-make-glibc-y2038-proof#post68 > > Github branch (including the y2038 conversion example): > https://github.com/lmajewski/y2038_glibc/commits/glibc__clock_settime-conversion-v7 > > Those patches have been applied on top of master branch: > SHA1: a26918cfda4bc4b9dad8aae1496e3ef7cbb63d96 > > Shall be used with provided meta-y2038 for development and testing: > https://github.com/lmajewski/meta-y2038 > > I've used guidelines from: > https://www.gnu.org/software/libc/manual/html_mono/libc.html > "D.2.1 64-bit time symbol handling in the GNU C Library" > to convert *clock_settime*. > > and most notably from: > https://sourceware.org/glibc/wiki/Y2038ProofnessDesign#clock_gettime.28.29 Thanks for the patches Lukassz! With my RV32 port I see this failure when compiling: In file included from ../sysdeps/generic/hp-timing.h:23, from ../nptl/descr.h:27, from ../sysdeps/riscv/nptl/tls.h:41, from ../include/errno.h:25, from ../sysdeps/unix/sysv/linux/clock_settime.c:18: ../include/time.h:129:28: error: conflicting types for '__clock_settime' # define __clock_settime64 __clock_settime ^~~~~~~~~~~~~~~ ../sysdeps/unix/sysv/linux/clock_settime.c:25:1: note: in expansion of macro '__clock_settime64' __clock_settime64 (clockid_t clock_id, const struct __timespec64 *tp) ^~~~~~~~~~~~~~~~~ In file included from <command-line>: ../include/time.h:25:20: note: previous declaration of '__clock_settime' was here libc_hidden_proto (__clock_settime) ^~~~~~~~~~~~~~~ ./../include/libc-symbols.h:598:33: note: in definition of macro '__hidden_proto' extern thread __typeof (name) name __hidden_proto_hiddenattr (attrs); ^~~~ ./../include/libc-symbols.h:617:44: note: in expansion of macro 'hidden_proto' # define libc_hidden_proto(name, attrs...) hidden_proto (name, ##attrs) ^~~~~~~~~~~~ ../include/time.h:25:1: note: in expansion of macro 'libc_hidden_proto' libc_hidden_proto (__clock_settime) ^~~~~~~~~~~~~~~~~ ../sysdeps/unix/sysv/linux/clock_settime.c:70:42: error: conflicting types for 'clock_settime' versioned_symbol (libc, __clock_settime, clock_settime, GLIBC_2_17); ^~~~~~~~~~~~~ ./../include/libc-symbols.h:152:26: note: in definition of macro '_weak_alias' extern __typeof (name) aliasname __attribute__ ((weak, alias (#name))) \ ^~~~~~~~~ ../include/shlib-compat.h:74:3: note: in expansion of macro 'weak_alias' weak_alias (local, symbol) ^~~~~~~~~~ ../sysdeps/unix/sysv/linux/clock_settime.c:70:1: note: in expansion of macro 'versioned_symbol' versioned_symbol (libc, __clock_settime, clock_settime, GLIBC_2_17); ^~~~~~~~~~~~~~~~ In file included from ../include/time.h:2, from ../sysdeps/generic/hp-timing.h:23, from ../nptl/descr.h:27, from ../sysdeps/riscv/nptl/tls.h:41, from ../include/errno.h:25, from ../sysdeps/unix/sysv/linux/clock_settime.c:18: ../time/time.h:222:12: note: previous declaration of 'clock_settime' was here extern int clock_settime (clockid_t __clock_id, const struct timespec *__tp) Which I can fix with this diff: { @@ -63,7 +62,6 @@ __clock_settime (clockid_t clock_id, const struct timespec *tp) valid_timespec_to_timespec64 (tp, &ts64); return __clock_settime64 (clock_id, &ts64); } -#endif libc_hidden_def (__clock_settime) Alistair > > > Lukasz Majewski (3): > y2038: Introduce internal for glibc struct __timespec64 > y2038: Provide conversion helpers for struct __timespec64 > y2038: linux: Provide __clock_settime64 implementation > > include/time.h | 116 ++++++++++++++++++++++++ > sysdeps/unix/sysv/linux/clock_settime.c | 38 +++++++- > 2 files changed, 150 insertions(+), 4 deletions(-) > > -- > 2.20.1 >
Comments
On Fri, 6 Sep 2019, Alistair Francis wrote:
> Which I can fix with this diff:
This diff is not the right way to fix this build failure.
One of the design principles in the Y2038 support is that is __TIMESIZE ==
64, the time functions *aren't* trivial wrappers of time64 functions;
rather, the time64 function definitions are remapped (via macros) so they
define the function name with no "64". For each case where there is a
pair of functions (for different time_t types) in the __TIMESIZE == 32
case, there should just be the one function when __TIMESIZE == 64.
This ensures that the Y2038 changes don't add any overhead at all in the
glibc binaries on existing platforms with __TIMESIZE == 64.
You should look at exactly what the types in question are, that are being
reported as conflicting in your build (probably by looking at preprocessed
source). __timespec64 and timespec are supposed to be the same type (via
#define) when __TIMESIZE == 64, to avoid such incompatibilities.
Hi Alistair, > On Fri, Sep 6, 2019 at 7:59 AM Lukasz Majewski <lukma@denx.de> wrote: > > > > This patch set introduces the conversion of clock_settime to > > explicit 64 bit struct __timespec64 arguments. As a result this > > function is now Y2038 safe. > > > > This work is (loosely) based on a previous development/patches: > > https://libc-alpha.sourceware.narkive.com/zniMOWui/rfc-patch-00-52-make-glibc-y2038-proof#post68 > > > > Github branch (including the y2038 conversion example): > > https://github.com/lmajewski/y2038_glibc/commits/glibc__clock_settime-conversion-v7 > > > > Those patches have been applied on top of master branch: > > SHA1: a26918cfda4bc4b9dad8aae1496e3ef7cbb63d96 > > > > Shall be used with provided meta-y2038 for development and testing: > > https://github.com/lmajewski/meta-y2038 > > > > I've used guidelines from: > > https://www.gnu.org/software/libc/manual/html_mono/libc.html > > "D.2.1 64-bit time symbol handling in the GNU C Library" > > to convert *clock_settime*. > > > > and most notably from: > > https://sourceware.org/glibc/wiki/Y2038ProofnessDesign#clock_gettime.28.29 > > > > Thanks for the patches Lukassz! > > With my RV32 port I see this failure when compiling: > But I did not change the core code between v5 (to which you refer here): https://patchwork.ozlabs.org/cover/1155397/ and then: https://sourceware.org/ml/libc-alpha/2019-05/msg00661.html and v7: https://patchwork.ozlabs.org/patch/1159056/ (The only change in v6 was to use 32 bit call to clock_settime syscall as requested by Arnd to facilitate validation on existing systems. However, Joseph was against this change). The notable change (but not in this patch set) was moving clock_settime symbol solely to glibc (and it has been removed from librt). > In file included from ../sysdeps/generic/hp-timing.h:23, > from ../nptl/descr.h:27, > from ../sysdeps/riscv/nptl/tls.h:41, > from ../include/errno.h:25, > from ../sysdeps/unix/sysv/linux/clock_settime.c:18: > ../include/time.h:129:28: error: conflicting types for > '__clock_settime' # define __clock_settime64 __clock_settime > ^~~~~~~~~~~~~~~ > ../sysdeps/unix/sysv/linux/clock_settime.c:25:1: note: in expansion of > macro '__clock_settime64' > __clock_settime64 (clockid_t clock_id, const struct __timespec64 *tp) > ^~~~~~~~~~~~~~~~~ > In file included from <command-line>: > ../include/time.h:25:20: note: previous declaration of > '__clock_settime' was here > libc_hidden_proto (__clock_settime) > ^~~~~~~~~~~~~~~ > ./../include/libc-symbols.h:598:33: note: in definition of macro > '__hidden_proto' > extern thread __typeof (name) name __hidden_proto_hiddenattr > (attrs); ^~~~ > ./../include/libc-symbols.h:617:44: note: in expansion of macro > 'hidden_proto' # define libc_hidden_proto(name, attrs...) > hidden_proto (name, ##attrs) ^~~~~~~~~~~~ > ../include/time.h:25:1: note: in expansion of macro > 'libc_hidden_proto' libc_hidden_proto (__clock_settime) > ^~~~~~~~~~~~~~~~~ > ../sysdeps/unix/sysv/linux/clock_settime.c:70:42: error: conflicting > types for 'clock_settime' > versioned_symbol (libc, __clock_settime, clock_settime, GLIBC_2_17); > ^~~~~~~~~~~~~ > ./../include/libc-symbols.h:152:26: note: in definition of macro > '_weak_alias' extern __typeof (name) aliasname __attribute__ ((weak, > alias (#name))) \ ^~~~~~~~~ > ../include/shlib-compat.h:74:3: note: in expansion of macro > 'weak_alias' weak_alias (local, symbol) > ^~~~~~~~~~ > ../sysdeps/unix/sysv/linux/clock_settime.c:70:1: note: in expansion of > macro 'versioned_symbol' > versioned_symbol (libc, __clock_settime, clock_settime, GLIBC_2_17); > ^~~~~~~~~~~~~~~~ > In file included from ../include/time.h:2, > from ../sysdeps/generic/hp-timing.h:23, > from ../nptl/descr.h:27, > from ../sysdeps/riscv/nptl/tls.h:41, > from ../include/errno.h:25, > from ../sysdeps/unix/sysv/linux/clock_settime.c:18: > ../time/time.h:222:12: note: previous declaration of 'clock_settime' > was here extern int clock_settime (clockid_t __clock_id, const struct > timespec *__tp) > > Which I can fix with this diff: > > diff --git a/include/time.h b/include/time.h > index 7ed3aa61d1d..28e2722de21 100644 > --- a/include/time.h > +++ b/include/time.h > @@ -125,13 +125,9 @@ extern __time64_t __timegm64 (struct tm *__tp) > __THROW; libc_hidden_proto (__timegm64) > #endif > > -#if __TIMESIZE == 64 What is the value of __TIMESIZE on your port? Is it 32 or 64 ? Do you also #define __USE_TIME_BITS64 1 ? > -# define __clock_settime64 __clock_settime > -#else > extern int __clock_settime64 (clockid_t clock_id, > const struct __timespec64 *tp); > libc_hidden_proto (__clock_settime64) > -#endif > > /* Compute the `struct tm' representation of T, > offset OFFSET seconds east of UTC, > diff --git a/sysdeps/unix/sysv/linux/clock_settime.c > b/sysdeps/unix/sysv/linux/clock_settime.c > index f5e084238a5..ab033c56ea9 100644 > --- a/sysdeps/unix/sysv/linux/clock_settime.c > +++ b/sysdeps/unix/sysv/linux/clock_settime.c > @@ -54,7 +54,6 @@ __clock_settime64 (clockid_t clock_id, const struct > __timespec64 *tp) > #endif > } > > -#if __TIMESIZE != 64 When I look on RISC-V patchset: https://patchwork.ozlabs.org/patch/1155391/ for the __clock_nanosleep for example you follow the same convention. And you don't need to remove #if __TIMESIZE != 64 for example. One difference is that the clock_settime64 code will be Y2038 on e.g. 32 bit ARM only when I apply on top of it following patch: https://github.com/lmajewski/y2038_glibc/commit/c6740c05ea9b224a552847d10402f98da8376994 (it enables redirect for clock_settime and support for -D_TIME_BITS=64 compilation flag). > int > __clock_settime (clockid_t clock_id, const struct timespec *tp) > { > @@ -63,7 +62,6 @@ __clock_settime (clockid_t clock_id, const struct > timespec *tp) > valid_timespec_to_timespec64 (tp, &ts64); > return __clock_settime64 (clock_id, &ts64); > } > -#endif > > libc_hidden_def (__clock_settime) > > Alistair > > > > > > > Lukasz Majewski (3): > > y2038: Introduce internal for glibc struct __timespec64 > > y2038: Provide conversion helpers for struct __timespec64 > > y2038: linux: Provide __clock_settime64 implementation > > > > include/time.h | 116 > > ++++++++++++++++++++++++ sysdeps/unix/sysv/linux/clock_settime.c | > > 38 +++++++- 2 files changed, 150 insertions(+), 4 deletions(-) > > > > -- > > 2.20.1 > > 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
On Fri, Sep 6, 2019 at 2:55 PM Lukasz Majewski <lukma@denx.de> wrote: > > Hi Alistair, > > > On Fri, Sep 6, 2019 at 7:59 AM Lukasz Majewski <lukma@denx.de> wrote: > > > > > > This patch set introduces the conversion of clock_settime to > > > explicit 64 bit struct __timespec64 arguments. As a result this > > > function is now Y2038 safe. > > > > > > This work is (loosely) based on a previous development/patches: > > > https://libc-alpha.sourceware.narkive.com/zniMOWui/rfc-patch-00-52-make-glibc-y2038-proof#post68 > > > > > > Github branch (including the y2038 conversion example): > > > https://github.com/lmajewski/y2038_glibc/commits/glibc__clock_settime-conversion-v7 > > > > > > Those patches have been applied on top of master branch: > > > SHA1: a26918cfda4bc4b9dad8aae1496e3ef7cbb63d96 > > > > > > Shall be used with provided meta-y2038 for development and testing: > > > https://github.com/lmajewski/meta-y2038 > > > > > > I've used guidelines from: > > > https://www.gnu.org/software/libc/manual/html_mono/libc.html > > > "D.2.1 64-bit time symbol handling in the GNU C Library" > > > to convert *clock_settime*. > > > > > > and most notably from: > > > https://sourceware.org/glibc/wiki/Y2038ProofnessDesign#clock_gettime.28.29 > > > > > > > Thanks for the patches Lukassz! > > > > With my RV32 port I see this failure when compiling: > > > > But I did not change the core code between v5 (to which you refer here): > https://patchwork.ozlabs.org/cover/1155397/ > and then: > https://sourceware.org/ml/libc-alpha/2019-05/msg00661.html > > and v7: > https://patchwork.ozlabs.org/patch/1159056/ > > (The only change in v6 was to use 32 bit call to clock_settime syscall > as requested by Arnd to facilitate validation on existing systems. > However, Joseph was against this change). > > The notable change (but not in this patch set) was moving clock_settime > symbol solely to glibc (and it has been removed from librt). > Yes, I have had this issue for awhile. > > In file included from ../sysdeps/generic/hp-timing.h:23, > > from ../nptl/descr.h:27, > > from ../sysdeps/riscv/nptl/tls.h:41, > > from ../include/errno.h:25, > > from ../sysdeps/unix/sysv/linux/clock_settime.c:18: > > ../include/time.h:129:28: error: conflicting types for > > '__clock_settime' # define __clock_settime64 __clock_settime > > ^~~~~~~~~~~~~~~ > > ../sysdeps/unix/sysv/linux/clock_settime.c:25:1: note: in expansion of > > macro '__clock_settime64' > > __clock_settime64 (clockid_t clock_id, const struct __timespec64 *tp) > > ^~~~~~~~~~~~~~~~~ > > In file included from <command-line>: > > ../include/time.h:25:20: note: previous declaration of > > '__clock_settime' was here > > libc_hidden_proto (__clock_settime) > > ^~~~~~~~~~~~~~~ > > ./../include/libc-symbols.h:598:33: note: in definition of macro > > '__hidden_proto' > > extern thread __typeof (name) name __hidden_proto_hiddenattr > > (attrs); ^~~~ > > ./../include/libc-symbols.h:617:44: note: in expansion of macro > > 'hidden_proto' # define libc_hidden_proto(name, attrs...) > > hidden_proto (name, ##attrs) ^~~~~~~~~~~~ > > ../include/time.h:25:1: note: in expansion of macro > > 'libc_hidden_proto' libc_hidden_proto (__clock_settime) > > ^~~~~~~~~~~~~~~~~ > > ../sysdeps/unix/sysv/linux/clock_settime.c:70:42: error: conflicting > > types for 'clock_settime' > > versioned_symbol (libc, __clock_settime, clock_settime, GLIBC_2_17); > > ^~~~~~~~~~~~~ > > ./../include/libc-symbols.h:152:26: note: in definition of macro > > '_weak_alias' extern __typeof (name) aliasname __attribute__ ((weak, > > alias (#name))) \ ^~~~~~~~~ > > ../include/shlib-compat.h:74:3: note: in expansion of macro > > 'weak_alias' weak_alias (local, symbol) > > ^~~~~~~~~~ > > ../sysdeps/unix/sysv/linux/clock_settime.c:70:1: note: in expansion of > > macro 'versioned_symbol' > > versioned_symbol (libc, __clock_settime, clock_settime, GLIBC_2_17); > > ^~~~~~~~~~~~~~~~ > > In file included from ../include/time.h:2, > > from ../sysdeps/generic/hp-timing.h:23, > > from ../nptl/descr.h:27, > > from ../sysdeps/riscv/nptl/tls.h:41, > > from ../include/errno.h:25, > > from ../sysdeps/unix/sysv/linux/clock_settime.c:18: > > ../time/time.h:222:12: note: previous declaration of 'clock_settime' > > was here extern int clock_settime (clockid_t __clock_id, const struct > > timespec *__tp) > > > > Which I can fix with this diff: > > > > diff --git a/include/time.h b/include/time.h > > index 7ed3aa61d1d..28e2722de21 100644 > > --- a/include/time.h > > +++ b/include/time.h > > @@ -125,13 +125,9 @@ extern __time64_t __timegm64 (struct tm *__tp) > > __THROW; libc_hidden_proto (__timegm64) > > #endif > > > > -#if __TIMESIZE == 64 > > What is the value of __TIMESIZE on your port? Is it 32 or 64 ? It's 64. > > Do you also #define __USE_TIME_BITS64 1 ? I don't. I can try with that, but I don't see if defined in the source. Alistair > > > -# define __clock_settime64 __clock_settime > > -#else > > extern int __clock_settime64 (clockid_t clock_id, > > const struct __timespec64 *tp); > > libc_hidden_proto (__clock_settime64) > > -#endif > > > > /* Compute the `struct tm' representation of T, > > offset OFFSET seconds east of UTC, > > diff --git a/sysdeps/unix/sysv/linux/clock_settime.c > > b/sysdeps/unix/sysv/linux/clock_settime.c > > index f5e084238a5..ab033c56ea9 100644 > > --- a/sysdeps/unix/sysv/linux/clock_settime.c > > +++ b/sysdeps/unix/sysv/linux/clock_settime.c > > @@ -54,7 +54,6 @@ __clock_settime64 (clockid_t clock_id, const struct > > __timespec64 *tp) > > #endif > > } > > > > -#if __TIMESIZE != 64 > > When I look on RISC-V patchset: > https://patchwork.ozlabs.org/patch/1155391/ > > for the __clock_nanosleep for example you follow the same convention. > And you don't need to remove #if __TIMESIZE != 64 for example. > > One difference is that the clock_settime64 code will be Y2038 on e.g. > 32 bit ARM only when I apply on top of it following patch: > https://github.com/lmajewski/y2038_glibc/commit/c6740c05ea9b224a552847d10402f98da8376994 > > (it enables redirect for clock_settime and support for -D_TIME_BITS=64 > compilation flag). > > > > > int > > __clock_settime (clockid_t clock_id, const struct timespec *tp) > > { > > @@ -63,7 +62,6 @@ __clock_settime (clockid_t clock_id, const struct > > timespec *tp) > > valid_timespec_to_timespec64 (tp, &ts64); > > return __clock_settime64 (clock_id, &ts64); > > } > > -#endif > > > > libc_hidden_def (__clock_settime) > > > > Alistair > > > > > > > > > > > Lukasz Majewski (3): > > > y2038: Introduce internal for glibc struct __timespec64 > > > y2038: Provide conversion helpers for struct __timespec64 > > > y2038: linux: Provide __clock_settime64 implementation > > > > > > include/time.h | 116 > > > ++++++++++++++++++++++++ sysdeps/unix/sysv/linux/clock_settime.c | > > > 38 +++++++- 2 files changed, 150 insertions(+), 4 deletions(-) > > > > > > -- > > > 2.20.1 > > > > > > > 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
Hi Alistair, > On Fri, Sep 6, 2019 at 2:55 PM Lukasz Majewski <lukma@denx.de> wrote: > > > > Hi Alistair, > > > > > On Fri, Sep 6, 2019 at 7:59 AM Lukasz Majewski <lukma@denx.de> > > > wrote: > > > > > > > > This patch set introduces the conversion of clock_settime to > > > > explicit 64 bit struct __timespec64 arguments. As a result this > > > > function is now Y2038 safe. > > > > > > > > This work is (loosely) based on a previous development/patches: > > > > https://libc-alpha.sourceware.narkive.com/zniMOWui/rfc-patch-00-52-make-glibc-y2038-proof#post68 > > > > > > > > Github branch (including the y2038 conversion example): > > > > https://github.com/lmajewski/y2038_glibc/commits/glibc__clock_settime-conversion-v7 > > > > > > > > Those patches have been applied on top of master branch: > > > > SHA1: a26918cfda4bc4b9dad8aae1496e3ef7cbb63d96 > > > > > > > > Shall be used with provided meta-y2038 for development and > > > > testing: https://github.com/lmajewski/meta-y2038 > > > > > > > > I've used guidelines from: > > > > https://www.gnu.org/software/libc/manual/html_mono/libc.html > > > > "D.2.1 64-bit time symbol handling in the GNU C Library" > > > > to convert *clock_settime*. > > > > > > > > and most notably from: > > > > https://sourceware.org/glibc/wiki/Y2038ProofnessDesign#clock_gettime.28.29 > > > > > > > > > > Thanks for the patches Lukassz! > > > > > > With my RV32 port I see this failure when compiling: > > > > > > > But I did not change the core code between v5 (to which you refer > > here): https://patchwork.ozlabs.org/cover/1155397/ > > and then: > > https://sourceware.org/ml/libc-alpha/2019-05/msg00661.html > > > > and v7: > > https://patchwork.ozlabs.org/patch/1159056/ > > > > (The only change in v6 was to use 32 bit call to clock_settime > > syscall as requested by Arnd to facilitate validation on existing > > systems. However, Joseph was against this change). > > > > The notable change (but not in this patch set) was moving > > clock_settime symbol solely to glibc (and it has been removed from > > librt). > > Yes, I have had this issue for awhile. > > > > In file included from ../sysdeps/generic/hp-timing.h:23, > > > from ../nptl/descr.h:27, > > > from ../sysdeps/riscv/nptl/tls.h:41, > > > from ../include/errno.h:25, > > > from > > > ../sysdeps/unix/sysv/linux/clock_settime.c:18: > > > ../include/time.h:129:28: error: conflicting types for > > > '__clock_settime' # define __clock_settime64 __clock_settime > > > ^~~~~~~~~~~~~~~ ../sysdeps/unix/sysv/linux/clock_settime.c:25:1: > > > note: in expansion of macro '__clock_settime64' > > > __clock_settime64 (clockid_t clock_id, const struct __timespec64 > > > *tp) ^~~~~~~~~~~~~~~~~ > > > In file included from <command-line>: > > > ../include/time.h:25:20: note: previous declaration of > > > '__clock_settime' was here > > > libc_hidden_proto (__clock_settime) > > > ^~~~~~~~~~~~~~~ > > > ./../include/libc-symbols.h:598:33: note: in definition of macro > > > '__hidden_proto' > > > extern thread __typeof (name) name __hidden_proto_hiddenattr > > > (attrs); ^~~~ > > > ./../include/libc-symbols.h:617:44: note: in expansion of macro > > > 'hidden_proto' # define libc_hidden_proto(name, attrs...) > > > hidden_proto (name, ##attrs) ^~~~~~~~~~~~ > > > ../include/time.h:25:1: note: in expansion of macro > > > 'libc_hidden_proto' libc_hidden_proto (__clock_settime) > > > ^~~~~~~~~~~~~~~~~ > > > ../sysdeps/unix/sysv/linux/clock_settime.c:70:42: error: > > > conflicting types for 'clock_settime' > > > versioned_symbol (libc, __clock_settime, clock_settime, > > > GLIBC_2_17); ^~~~~~~~~~~~~ > > > ./../include/libc-symbols.h:152:26: note: in definition of macro > > > '_weak_alias' extern __typeof (name) aliasname __attribute__ > > > ((weak, alias (#name))) \ ^~~~~~~~~ > > > ../include/shlib-compat.h:74:3: note: in expansion of macro > > > 'weak_alias' weak_alias (local, symbol) > > > ^~~~~~~~~~ > > > ../sysdeps/unix/sysv/linux/clock_settime.c:70:1: note: in > > > expansion of macro 'versioned_symbol' > > > versioned_symbol (libc, __clock_settime, clock_settime, > > > GLIBC_2_17); ^~~~~~~~~~~~~~~~ > > > In file included from ../include/time.h:2, > > > from ../sysdeps/generic/hp-timing.h:23, > > > from ../nptl/descr.h:27, > > > from ../sysdeps/riscv/nptl/tls.h:41, > > > from ../include/errno.h:25, > > > from > > > ../sysdeps/unix/sysv/linux/clock_settime.c:18: > > > ../time/time.h:222:12: note: previous declaration of > > > 'clock_settime' was here extern int clock_settime (clockid_t > > > __clock_id, const struct timespec *__tp) > > > > > > Which I can fix with this diff: > > > > > > diff --git a/include/time.h b/include/time.h > > > index 7ed3aa61d1d..28e2722de21 100644 > > > --- a/include/time.h > > > +++ b/include/time.h > > > @@ -125,13 +125,9 @@ extern __time64_t __timegm64 (struct tm > > > *__tp) __THROW; libc_hidden_proto (__timegm64) > > > #endif > > > > > > -#if __TIMESIZE == 64 > > > > What is the value of __TIMESIZE on your port? Is it 32 or 64 ? > > It's 64. > > > > > Do you also #define __USE_TIME_BITS64 1 ? > > I don't. I can try with that, but I don't see if defined in the > source. Alistair, have you managed to make your patches working on top of this series? > > Alistair > > > > > > -# define __clock_settime64 __clock_settime > > > -#else > > > extern int __clock_settime64 (clockid_t clock_id, > > > const struct __timespec64 *tp); > > > libc_hidden_proto (__clock_settime64) > > > -#endif > > > > > > /* Compute the `struct tm' representation of T, > > > offset OFFSET seconds east of UTC, > > > diff --git a/sysdeps/unix/sysv/linux/clock_settime.c > > > b/sysdeps/unix/sysv/linux/clock_settime.c > > > index f5e084238a5..ab033c56ea9 100644 > > > --- a/sysdeps/unix/sysv/linux/clock_settime.c > > > +++ b/sysdeps/unix/sysv/linux/clock_settime.c > > > @@ -54,7 +54,6 @@ __clock_settime64 (clockid_t clock_id, const > > > struct __timespec64 *tp) > > > #endif > > > } > > > > > > -#if __TIMESIZE != 64 > > > > When I look on RISC-V patchset: > > https://patchwork.ozlabs.org/patch/1155391/ > > > > for the __clock_nanosleep for example you follow the same > > convention. And you don't need to remove #if __TIMESIZE != 64 for > > example. > > > > One difference is that the clock_settime64 code will be Y2038 on > > e.g. 32 bit ARM only when I apply on top of it following patch: > > https://github.com/lmajewski/y2038_glibc/commit/c6740c05ea9b224a552847d10402f98da8376994 > > > > (it enables redirect for clock_settime and support for > > -D_TIME_BITS=64 compilation flag). > > > > > > > > > int > > > __clock_settime (clockid_t clock_id, const struct timespec *tp) > > > { > > > @@ -63,7 +62,6 @@ __clock_settime (clockid_t clock_id, const > > > struct timespec *tp) > > > valid_timespec_to_timespec64 (tp, &ts64); > > > return __clock_settime64 (clock_id, &ts64); > > > } > > > -#endif > > > > > > libc_hidden_def (__clock_settime) > > > > > > Alistair > > > > > > > > > > > > > > > Lukasz Majewski (3): > > > > y2038: Introduce internal for glibc struct __timespec64 > > > > y2038: Provide conversion helpers for struct __timespec64 > > > > y2038: linux: Provide __clock_settime64 implementation > > > > > > > > include/time.h | 116 > > > > ++++++++++++++++++++++++ > > > > sysdeps/unix/sysv/linux/clock_settime.c | 38 +++++++- 2 files > > > > changed, 150 insertions(+), 4 deletions(-) > > > > > > > > -- > > > > 2.20.1 > > > > > > > > > > > > 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 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
On Fri, Sep 13, 2019 at 7:36 AM Lukasz Majewski <lukma@denx.de> wrote: > > Hi Alistair, > > > On Fri, Sep 6, 2019 at 2:55 PM Lukasz Majewski <lukma@denx.de> wrote: > > > > > > Hi Alistair, > > > > > > > On Fri, Sep 6, 2019 at 7:59 AM Lukasz Majewski <lukma@denx.de> > > > > wrote: > > > > > > > > > > This patch set introduces the conversion of clock_settime to > > > > > explicit 64 bit struct __timespec64 arguments. As a result this > > > > > function is now Y2038 safe. > > > > > > > > > > This work is (loosely) based on a previous development/patches: > > > > > https://libc-alpha.sourceware.narkive.com/zniMOWui/rfc-patch-00-52-make-glibc-y2038-proof#post68 > > > > > > > > > > Github branch (including the y2038 conversion example): > > > > > https://github.com/lmajewski/y2038_glibc/commits/glibc__clock_settime-conversion-v7 > > > > > > > > > > Those patches have been applied on top of master branch: > > > > > SHA1: a26918cfda4bc4b9dad8aae1496e3ef7cbb63d96 > > > > > > > > > > Shall be used with provided meta-y2038 for development and > > > > > testing: https://github.com/lmajewski/meta-y2038 > > > > > > > > > > I've used guidelines from: > > > > > https://www.gnu.org/software/libc/manual/html_mono/libc.html > > > > > "D.2.1 64-bit time symbol handling in the GNU C Library" > > > > > to convert *clock_settime*. > > > > > > > > > > and most notably from: > > > > > https://sourceware.org/glibc/wiki/Y2038ProofnessDesign#clock_gettime.28.29 > > > > > > > > > > > > > Thanks for the patches Lukassz! > > > > > > > > With my RV32 port I see this failure when compiling: > > > > > > > > > > But I did not change the core code between v5 (to which you refer > > > here): https://patchwork.ozlabs.org/cover/1155397/ > > > and then: > > > https://sourceware.org/ml/libc-alpha/2019-05/msg00661.html > > > > > > and v7: > > > https://patchwork.ozlabs.org/patch/1159056/ > > > > > > (The only change in v6 was to use 32 bit call to clock_settime > > > syscall as requested by Arnd to facilitate validation on existing > > > systems. However, Joseph was against this change). > > > > > > The notable change (but not in this patch set) was moving > > > clock_settime symbol solely to glibc (and it has been removed from > > > librt). > > > > Yes, I have had this issue for awhile. > > > > > > In file included from ../sysdeps/generic/hp-timing.h:23, > > > > from ../nptl/descr.h:27, > > > > from ../sysdeps/riscv/nptl/tls.h:41, > > > > from ../include/errno.h:25, > > > > from > > > > ../sysdeps/unix/sysv/linux/clock_settime.c:18: > > > > ../include/time.h:129:28: error: conflicting types for > > > > '__clock_settime' # define __clock_settime64 __clock_settime > > > > ^~~~~~~~~~~~~~~ ../sysdeps/unix/sysv/linux/clock_settime.c:25:1: > > > > note: in expansion of macro '__clock_settime64' > > > > __clock_settime64 (clockid_t clock_id, const struct __timespec64 > > > > *tp) ^~~~~~~~~~~~~~~~~ > > > > In file included from <command-line>: > > > > ../include/time.h:25:20: note: previous declaration of > > > > '__clock_settime' was here > > > > libc_hidden_proto (__clock_settime) > > > > ^~~~~~~~~~~~~~~ > > > > ./../include/libc-symbols.h:598:33: note: in definition of macro > > > > '__hidden_proto' > > > > extern thread __typeof (name) name __hidden_proto_hiddenattr > > > > (attrs); ^~~~ > > > > ./../include/libc-symbols.h:617:44: note: in expansion of macro > > > > 'hidden_proto' # define libc_hidden_proto(name, attrs...) > > > > hidden_proto (name, ##attrs) ^~~~~~~~~~~~ > > > > ../include/time.h:25:1: note: in expansion of macro > > > > 'libc_hidden_proto' libc_hidden_proto (__clock_settime) > > > > ^~~~~~~~~~~~~~~~~ > > > > ../sysdeps/unix/sysv/linux/clock_settime.c:70:42: error: > > > > conflicting types for 'clock_settime' > > > > versioned_symbol (libc, __clock_settime, clock_settime, > > > > GLIBC_2_17); ^~~~~~~~~~~~~ > > > > ./../include/libc-symbols.h:152:26: note: in definition of macro > > > > '_weak_alias' extern __typeof (name) aliasname __attribute__ > > > > ((weak, alias (#name))) \ ^~~~~~~~~ > > > > ../include/shlib-compat.h:74:3: note: in expansion of macro > > > > 'weak_alias' weak_alias (local, symbol) > > > > ^~~~~~~~~~ > > > > ../sysdeps/unix/sysv/linux/clock_settime.c:70:1: note: in > > > > expansion of macro 'versioned_symbol' > > > > versioned_symbol (libc, __clock_settime, clock_settime, > > > > GLIBC_2_17); ^~~~~~~~~~~~~~~~ > > > > In file included from ../include/time.h:2, > > > > from ../sysdeps/generic/hp-timing.h:23, > > > > from ../nptl/descr.h:27, > > > > from ../sysdeps/riscv/nptl/tls.h:41, > > > > from ../include/errno.h:25, > > > > from > > > > ../sysdeps/unix/sysv/linux/clock_settime.c:18: > > > > ../time/time.h:222:12: note: previous declaration of > > > > 'clock_settime' was here extern int clock_settime (clockid_t > > > > __clock_id, const struct timespec *__tp) > > > > > > > > Which I can fix with this diff: > > > > > > > > diff --git a/include/time.h b/include/time.h > > > > index 7ed3aa61d1d..28e2722de21 100644 > > > > --- a/include/time.h > > > > +++ b/include/time.h > > > > @@ -125,13 +125,9 @@ extern __time64_t __timegm64 (struct tm > > > > *__tp) __THROW; libc_hidden_proto (__timegm64) > > > > #endif > > > > > > > > -#if __TIMESIZE == 64 > > > > > > What is the value of __TIMESIZE on your port? Is it 32 or 64 ? > > > > It's 64. > > > > > > > > Do you also #define __USE_TIME_BITS64 1 ? > > > > I don't. I can try with that, but I don't see if defined in the > > source. > > Alistair, have you managed to make your patches working on top of this > series? Hey Lukasz, I have been travelling recently and haven't had a chance to look at fixing this (besides the diff I provided already). I should be able to look at it this week. Alistair > > > > > > Alistair > > > > > > > > > -# define __clock_settime64 __clock_settime > > > > -#else > > > > extern int __clock_settime64 (clockid_t clock_id, > > > > const struct __timespec64 *tp); > > > > libc_hidden_proto (__clock_settime64) > > > > -#endif > > > > > > > > /* Compute the `struct tm' representation of T, > > > > offset OFFSET seconds east of UTC, > > > > diff --git a/sysdeps/unix/sysv/linux/clock_settime.c > > > > b/sysdeps/unix/sysv/linux/clock_settime.c > > > > index f5e084238a5..ab033c56ea9 100644 > > > > --- a/sysdeps/unix/sysv/linux/clock_settime.c > > > > +++ b/sysdeps/unix/sysv/linux/clock_settime.c > > > > @@ -54,7 +54,6 @@ __clock_settime64 (clockid_t clock_id, const > > > > struct __timespec64 *tp) > > > > #endif > > > > } > > > > > > > > -#if __TIMESIZE != 64 > > > > > > When I look on RISC-V patchset: > > > https://patchwork.ozlabs.org/patch/1155391/ > > > > > > for the __clock_nanosleep for example you follow the same > > > convention. And you don't need to remove #if __TIMESIZE != 64 for > > > example. > > > > > > One difference is that the clock_settime64 code will be Y2038 on > > > e.g. 32 bit ARM only when I apply on top of it following patch: > > > https://github.com/lmajewski/y2038_glibc/commit/c6740c05ea9b224a552847d10402f98da8376994 > > > > > > (it enables redirect for clock_settime and support for > > > -D_TIME_BITS=64 compilation flag). > > > > > > > > > > > > > int > > > > __clock_settime (clockid_t clock_id, const struct timespec *tp) > > > > { > > > > @@ -63,7 +62,6 @@ __clock_settime (clockid_t clock_id, const > > > > struct timespec *tp) > > > > valid_timespec_to_timespec64 (tp, &ts64); > > > > return __clock_settime64 (clock_id, &ts64); > > > > } > > > > -#endif > > > > > > > > libc_hidden_def (__clock_settime) > > > > > > > > Alistair > > > > > > > > > > > > > > > > > > > Lukasz Majewski (3): > > > > > y2038: Introduce internal for glibc struct __timespec64 > > > > > y2038: Provide conversion helpers for struct __timespec64 > > > > > y2038: linux: Provide __clock_settime64 implementation > > > > > > > > > > include/time.h | 116 > > > > > ++++++++++++++++++++++++ > > > > > sysdeps/unix/sysv/linux/clock_settime.c | 38 +++++++- 2 files > > > > > changed, 150 insertions(+), 4 deletions(-) > > > > > > > > > > -- > > > > > 2.20.1 > > > > > > > > > > > > > > > > > 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 > > > > 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
diff --git a/include/time.h b/include/time.h index 7ed3aa61d1d..28e2722de21 100644 --- a/include/time.h +++ b/include/time.h @@ -125,13 +125,9 @@ extern __time64_t __timegm64 (struct tm *__tp) __THROW; libc_hidden_proto (__timegm64) #endif -#if __TIMESIZE == 64 -# define __clock_settime64 __clock_settime -#else extern int __clock_settime64 (clockid_t clock_id, const struct __timespec64 *tp); libc_hidden_proto (__clock_settime64) -#endif /* Compute the `struct tm' representation of T, offset OFFSET seconds east of UTC, diff --git a/sysdeps/unix/sysv/linux/clock_settime.c b/sysdeps/unix/sysv/linux/clock_settime.c index f5e084238a5..ab033c56ea9 100644 --- a/sysdeps/unix/sysv/linux/clock_settime.c +++ b/sysdeps/unix/sysv/linux/clock_settime.c @@ -54,7 +54,6 @@ __clock_settime64 (clockid_t clock_id, const struct __timespec64 *tp) #endif } -#if __TIMESIZE != 64 int __clock_settime (clockid_t clock_id, const struct timespec *tp)