| Message ID | 20190327085210.22019-1-lukma@denx.de |
|---|---|
| Headers | show |
| Series | y2038: clock_settime rework to be Y2038 safe on 32bit systems | expand |
On Wed, 27 Mar 2019, Lukasz Majewski wrote: > -- The Y2038 support is enabled only when _ALL_ syscalls/functions are Y2038 ready Yes, that's correct. All the header changes to support _TIME_BITS=64, new symbol versions and corresponding changes to ABI baselines should be made at once. > - I've read that clock_* functions were moved from librt to glibc - what > was the motivation behind this move? librt depends on libpthread. Linking against libpthread causes some multithreaded code paths to be used, so slowing down programs even if they are single-threaded. The clock_* functions are commonly usable without threads, so it makes sense to have them in libc. (Whether we should somehow avoid the slowdown from pthreads functions being present, and have everything in libc, is a separate question.) > Why to we have compatibility code in the rt/* for > clock_settime() if the _real_ support is in time/* ? We have to stay compatible with existing programs built with older glibc versions that expect to get functions such as clock_settime from librt.so. When making changes in this area, running the glibc testsuite for a range of architectures is helpful - the ABI tests and linknamespace tests will catch some possible bugs. But you should also manually inspect the clock_* symbols exported from librt.so to make sure they look the same before and after the change (as opposed to any GLIBC_PRIVATE __clock_* exports, where changes are OK as long as the symbols exported match the ones used in other libraries). For example, the ABI tests do not verify whether a given symbol is a compat symbol or not. If you look at the output of objdump --dynamic-syms on librt.so you'll see e.g. 0000000000004dc0 g iD .text 0000000000000008 (GLIBC_2.2.5) clock_gettime where the parentheses around the symbol version indicate it's a compat symbol - so you should verify that remains the same before and after the patch. The 'i' in 'iD' indicates an IFUNC because that's how the redirection from librt to libc versions works, so make sure it remains an IFUNC after the changes (on architectures supporting IFUNCs; on others, it should remain a wrapper). And do this verification for both 32-bit and 64-bit architectures.
Hi Joseph, > On Wed, 27 Mar 2019, Lukasz Majewski wrote: > > > -- The Y2038 support is enabled only when _ALL_ > > syscalls/functions are Y2038 ready > > Yes, that's correct. All the header changes to support > _TIME_BITS=64, new symbol versions and corresponding changes to ABI > baselines should be made at once. Ok. So the work plan is as follows: 1. Rewrite relevant syscalls (for sysdeps/unix/sysv/linux - Linux) to use __clock_settime64 () { } #if TIMESIZE != 64 clock_settime () { ... __clock_settime64(); ... } #endif Those shouldn't introduce make xcheck/check regressions. And can be added syscall by syscall - i.e. clock_{settime|gettime|nanosleep|getres} In this step the internal glibc explicit 64bit types shall be introduced (struct __timespec64, __time64_t, struct __timeval64, etc). 2. When _TIME_BITS=64 patch is introduced: This shall be done in a single patch (!), which would include: redirection of functions, manual entry, __USE_TIME_BITS64 definition etc. To reduce the overwhelming patch size from point 2 - tests shall be added in point 1 (so I only then add -D_TIME_BITS=64 switch to be compiled in). > > > - I've read that clock_* functions were moved from librt to glibc - > > what was the motivation behind this move? > > librt depends on libpthread. Linking against libpthread causes some > multithreaded code paths to be used, so slowing down programs even if > they are single-threaded. The clock_* functions are commonly usable > without threads, so it makes sense to have them in libc. > > (Whether we should somehow avoid the slowdown from pthreads functions > being present, and have everything in libc, is a separate question.) > > > Why to we have compatibility code in the rt/* for > > clock_settime() if the _real_ support is in time/* ? > > We have to stay compatible with existing programs built with older > glibc versions that expect to get functions such as clock_settime > from librt.so. > > When making changes in this area, running the glibc testsuite for a > range of architectures is helpful - the ABI tests and linknamespace > tests will catch some possible bugs. But you should also manually > inspect the clock_* symbols exported from librt.so to make sure they > look the same before and after the change (as opposed to any > GLIBC_PRIVATE __clock_* exports, where changes are OK as long as the > symbols exported match the ones used in other libraries). > > For example, the ABI tests do not verify whether a given symbol is a > compat symbol or not. If you look at the output of objdump > --dynamic-syms on librt.so you'll see e.g. > > 0000000000004dc0 g iD .text 0000000000000008 (GLIBC_2.2.5) > clock_gettime > > where the parentheses around the symbol version indicate it's a > compat symbol - so you should verify that remains the same before and > after the patch. The 'i' in 'iD' indicates an IFUNC because that's > how the redirection from librt to libc versions works, so make sure > it remains an IFUNC after the changes (on architectures supporting > IFUNCs; on others, it should remain a wrapper). And do this > verification for both 32-bit and 64-bit architectures. > And this explains why this RFC causes following regression: FAIL: rt/check-abi-librt ^^^ - I do not export clock_settime anymore for librt FAIL: time/check-wrapper-headers ^^^ What are the "wrapper-headers"? Is this a different name for "installed" headers in glibc terms ? Summary of test results: 9 FAIL 6048 PASS 21 UNSUPPORTED 17 XFAIL 2 XPASS If I may ask: What is the workflow/setup for build-many-glibcs.py ? Is it ./src/glibc/scripts/build-many-glibcs.py -j8 . checkout and then ./src/glibc/scripts/build-many-glibcs.py -j8 . bot 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 Thu, 28 Mar 2019, Lukasz Majewski wrote: > FAIL: time/check-wrapper-headers > ^^^ What are the "wrapper-headers"? Is this a different name > for "installed" headers in glibc terms ? Wrapper headers are the headers in include/ that are *not* installed but wrap round the headers in other directories that *are* installed, so that the glibc build process can find the right header from the source tree rather than one that might have been installed by an older glibc version and so is found by the compiler by default. For example, the installed <time.h> comes from time/time.h in the glibc source tree; compilations in subdirectories other than time/ don't search time/ for headers, so they can only find time/time.h, rather than an older header like /usr/include/time.h, because of the include/time.h wrapper that includes <time/time.h> explicitly (and then adds some internal declarations). Recent discussions and commits (which it is advisable to follow when maintaining any large patch series over an extended period of time, to keep that series up to date with such global changes) will show how we agreed that all installed headers should have a wrapper, even if they are not actually included from any source file outside the directory containing the header, and how a test was added to verify this requirement (after missing wrappers had been added). If you see a recently added test failing with (or indeed without) your patches, the commit adding that test, and surrounding discussions on libc-alpha, are the first place to look to understand what is being tested. > If I may ask: > > What is the workflow/setup for build-many-glibcs.py ? The commit message for the commit that added the script includes instructions, which are still current. If you only wish to test one configuration (e.g. i686-gnu), naming that on the "compilers" and "glibcs" commands will save a lot of time. > and then > ./src/glibc/scripts/build-many-glibcs.py -j8 . bot You are unlikely to want to use it with "bot" unless you actually intend to run your own bot that continuously builds and sends test logs to a mailing list.
This patch set is a RFC to present the conversion and support of clock_settime to be Y2038 safe. This work is based on a previous development/patches: https://libc-alpha.sourceware.narkive.com/zniMOWui/rfc-patch-00-52-make-glibc-y2038-proof#post68 And shall be applied on top of: https://github.com/lmajewski/y2038_glibc/commits/mktime_v3_fixes Github repository: https://github.com/lmajewski/y2038_glibc/commits/Y2038-2.29-glibc-__clock_settime64_v1-27-03-2019 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*. - The clock_settime() has been choosen as a relatively easy syscall for conversion (i.e. no VDO) - The idea for proceeding: convert a single syscall (clock_settime) or related group of syscalls (clock_gettime, clock_nanosleep,clock_getres, clock_settime) - to keep the number of changes manageable - The explicit 64 bit time related syscalls (Y2038 safe) have been already accepted to mainline Linux kernel (5.1-rc2): https://elixir.bootlin.com/linux/v5.1-rc2/source/arch/arm/tools/syscall.tbl#L420 - Test(s) for Y2038 (with using clock_settime) would be added to glibc after the pair of clock_settime/clock_gettime is accepted/reviewed - Last two patches of this series shows the way how the Y2038 support is going to be added to glibc. - The important question about enabling Y2038 support in glibc: -- If a well tested group of functions/syscalls (like clock_*) is accepted with the Y2038 support enabled (_TIME_BITS passed to user program enables 64 bit time) or -- The Y2038 support is enabled only when _ALL_ syscalls/functions are Y2038 ready - I've read that clock_* functions were moved from librt to glibc - what was the motivation behind this move? Why to we have compatibility code in the rt/* for clock_settime() if the _real_ support is in time/* ? Comments are more than welcome. Lukasz Majewski (7): y2038: Introduce struct __timespec64 y2038: Provide conversion helpers for struct __timespec64 y2038: clock_settime: Provide __clock_settime64 implementation for linux y2038: rt: clock_settime: Convert __clock_settime to __clock_settime64 y2038: clock_settime: implementation for Unix generic y2038: Support for Y2038 safe time on 32 bit systems (generic code) y2038: Introduce support for clock_settime() being Y2038 safe include/features.h | 19 +++++++ include/time.h | 97 ++++++++++++++++++++++++++++++++- manual/creature.texi | 28 ++++++++++ rt/Versions | 2 +- rt/clock-compat.c | 3 - rt/clock_settime.c | 4 +- sysdeps/unix/clock_settime.c | 23 ++++++-- sysdeps/unix/sysv/linux/clock_settime.c | 51 +++++++++++++---- time/Makefile | 2 +- time/bits/types/struct___timespec64.h | 39 +++++++++++++ time/bits/types/struct_timespec.h | 8 +++ time/bits/types/time_t.h | 4 ++ time/time.h | 9 +++ 13 files changed, 265 insertions(+), 24 deletions(-) create mode 100644 time/bits/types/struct___timespec64.h