Message ID | 20210622172713.3504003-1-adhemerval.zanella@linaro.org |
---|---|
State | New |
Headers | show |
Series | [v2] Use LFS and 64 bit time for installed programs (BZ #15333) | expand |
Note: I built this patch and wrote a perl script to extract out the relevent symbols from the symlists of all binaries, and verified (as best I could ;) that they're doing the right thing. Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org> writes: > diff --git a/Makeconfig b/Makeconfig > +# Use 64 bit time_t support for installed programs > +installed-modules = nonlib nscd lddlibc4 libresolv ldconfig locale_programs \ > + iconvprogs libnss_files libnss_compat libnss_db libnss_hesiod \ > + libutil libpcprofile libSegFault > ++extra-time-flags = $(if $(filter $(installed-modules),\ > + $(in-module)),-D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64) > + If "in-module" is in the list of "installed-modules", we add two more -D's. Ok. Whitelist good :-) > - testsuite > + testsuite testsuite-internal Ok. > override CFLAGS = -std=gnu11 -fgnu89-inline $(config-extra-cflags) \ > $(filter-out %frame-pointer,$(+cflags)) $(+gccwarn-c) \ > - $(+extra-math-flags) \ > + $(+extra-math-flags) $(+extra-time-flags) \ Globally use the new flags set above, if. Ok. > diff --git a/Makerules b/Makerules > -all-nonlib := $(strip $(tests-internal) $(test-internal-extras) \ > - $(others) $(others-extras)) > +all-nonlib := $(strip $(others) $(others-extras)) Removing internal tests from the "nonlib" module. > ifneq (,$(all-nonlib)) > cpp-srcs-left = $(all-nonlib) > lib := nonlib > include $(patsubst %,$(..)libof-iterator.mk,$(cpp-srcs-left)) > endif > > +# All internal tests use testsuite-internal module since for 64 bit time > +# support is set as default for MODULE_NAME=nonlib (which include some > +# installed programs). > +all-testsuite-internal := $(strip $(tests-internal) $(test-internal-extras)) > +ifneq (,$(all-testsuite-internal)) > +cpp-srcs-left = $(all-testsuite-internal) > +lib := testsuite-internal > +include $(patsubst %,$(..)libof-iterator.mk,$(cpp-srcs-left)) > +endif Hook up all the tests that the above (nonlib) case doesn't find, since they're internal/exceptions. Ok. > diff --git a/elf/sotruss-lib.c b/elf/sotruss-lib.c > - out_fd = open (fullname, O_RDWR | O_CREAT | O_TRUNC, 0666); > + out_fd = open64 (fullname, O_RDWR | O_CREAT | O_TRUNC, 0666); Ok. > - out_fd = fcntl (STDERR_FILENO, F_DUPFD, 1000); > + out_fd = fcntl64 (STDERR_FILENO, F_DUPFD, 1000); Ok. LGTM Reviewed-by: DJ Delorie <dj@redhat.com>
* Adhemerval Zanella via Libc-alpha: > The installed programs are built with a combination of different > values for MODULE_NAME, as below. To enable both Long File Support > and 64 bt time, -D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64 is added for > nonlibi, nscd, lddlibc4, libresolv, ldconfig, locale_programs, > iconvprogs, libnss_files, libnss_compat, libnss_db, libnss_hesiod, > libutil, libpcprofile, and libSegFault. -D_TIME_BITS=64 does not work for building glibc because the internal aliases such as __fstat64 do not follow _TIME_BITS and are hard-wired to 32-bit time. A symptom is a broken ldconfig, e.g.: $ elf/ldconfig -p elf/ldconfig: mmap of cache file failed. : Invalid argument Thanks, Florian
On 28/12/2021 17:37, Florian Weimer wrote: > * Adhemerval Zanella via Libc-alpha: > >> The installed programs are built with a combination of different >> values for MODULE_NAME, as below. To enable both Long File Support >> and 64 bt time, -D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64 is added for >> nonlibi, nscd, lddlibc4, libresolv, ldconfig, locale_programs, >> iconvprogs, libnss_files, libnss_compat, libnss_db, libnss_hesiod, >> libutil, libpcprofile, and libSegFault. > > -D_TIME_BITS=64 does not work for building glibc because the internal > aliases such as __fstat64 do not follow _TIME_BITS and are hard-wired to > 32-bit time. A symptom is a broken ldconfig, e.g.: > > $ elf/ldconfig -p > elf/ldconfig: mmap of cache file failed. > : Invalid argument I think the real problem is the usage of internal symbol in installed binaries, it works for LFS stat because they are either redirected to compat xstat ones or __stat (which I think it is also error prone). I am fixing it by using the default stat names, installed programs should not use proper LFS and time64.
* Adhemerval Zanella: > On 28/12/2021 17:37, Florian Weimer wrote: >> * Adhemerval Zanella via Libc-alpha: >> >>> The installed programs are built with a combination of different >>> values for MODULE_NAME, as below. To enable both Long File Support >>> and 64 bt time, -D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64 is added for >>> nonlibi, nscd, lddlibc4, libresolv, ldconfig, locale_programs, >>> iconvprogs, libnss_files, libnss_compat, libnss_db, libnss_hesiod, >>> libutil, libpcprofile, and libSegFault. >> >> -D_TIME_BITS=64 does not work for building glibc because the internal >> aliases such as __fstat64 do not follow _TIME_BITS and are hard-wired to >> 32-bit time. A symptom is a broken ldconfig, e.g.: >> >> $ elf/ldconfig -p >> elf/ldconfig: mmap of cache file failed. >> : Invalid argument > > I think the real problem is the usage of internal symbol in installed > binaries, it works for LFS stat because they are either redirected to > compat xstat ones or __stat (which I think it is also error prone). > > I am fixing it by using the default stat names, installed programs > should not use proper LFS and time64. They need to be able to deal with 64-bit inode numbers, though. Thanks, Florian
On 29/12/2021 09:35, Florian Weimer wrote: > * Adhemerval Zanella: > >> On 28/12/2021 17:37, Florian Weimer wrote: >>> * Adhemerval Zanella via Libc-alpha: >>> >>>> The installed programs are built with a combination of different >>>> values for MODULE_NAME, as below. To enable both Long File Support >>>> and 64 bt time, -D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64 is added for >>>> nonlibi, nscd, lddlibc4, libresolv, ldconfig, locale_programs, >>>> iconvprogs, libnss_files, libnss_compat, libnss_db, libnss_hesiod, >>>> libutil, libpcprofile, and libSegFault. >>> >>> -D_TIME_BITS=64 does not work for building glibc because the internal >>> aliases such as __fstat64 do not follow _TIME_BITS and are hard-wired to >>> 32-bit time. A symptom is a broken ldconfig, e.g.: >>> >>> $ elf/ldconfig -p >>> elf/ldconfig: mmap of cache file failed. >>> : Invalid argument >> >> I think the real problem is the usage of internal symbol in installed >> binaries, it works for LFS stat because they are either redirected to >> compat xstat ones or __stat (which I think it is also error prone). >> >> I am fixing it by using the default stat names, installed programs >> should not use proper LFS and time64. > > They need to be able to deal with 64-bit inode numbers, though. Oops, I meant that installed programs *should* use proper LFS and time64 (using default stat names accomplish it for static linking).
On 14/12/2021 20:35, DJ Delorie via Libc-alpha wrote: ... > > LGTM > Reviewed-by: DJ Delorie <dj@redhat.com> > Starting with this commit a6d2f948b71adcb5ea395cb04833bc645eab45e6, I get a test fail on s390: FAIL: resolv/tst-p_secstodate Test 0: 0 -> 19700101000000 Test 1: 12345 -> 19700101000000 test 1 failedTest 2: 999999999 -> 19700101000000 test 2 failedTest 3: 2147483647 -> 19700101000000 test 3 failedTest 4: 2147483648 -> <overflow> Test 5: 4294967295 -> <overflow> It turns out that there is a type-mismatch of time_t on caller and callee side. in libresolv.so: resolv/res_debug.c: __p_secstodate(u_long secs) time_t clock = secs; struct tm *time = __gmtime_r(&clock, &timebuf); (gdb) p &clock $1 = (time_t *) 0x7ffff1c8 (gdb) ptype time_t type = long long (gdb) p sizeof(time_t) $2 = 8 (gdb) x/2xw 0x7ffff1c8 0x7ffff1c8: 0x00000000 0x7fffffff The secs are stored as 8byte long long on stack and the pointer is passed to __gmtime_r in libc.so: time/gmtime.c: /* Provide a 32-bit variant if needed. */ #if __TIMESIZE != 64 struct tm * __gmtime_r (const time_t *t, struct tm *tp) { __time64_t t64 = *t; return __gmtime64_r (&t64, tp); } #endif (gdb) ptype time_t type = long (gdb) p sizeof(time_t) $3 = 4 (gdb) p *t $4 = 0 (gdb) ptype __time64_t type = long long On libc.so side, time_t is defined as 4byte long, thus the value of t is read as 0x0 instead of 0x7fffffff. resolv/res_debug.c is built with -D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64 and time/gmtime.c is built without those defines. @Adhemerval: Can you please have a look? Thanks Stefan
On 12/01/2022 11:13, Stefan Liebler wrote: > On 14/12/2021 20:35, DJ Delorie via Libc-alpha wrote: > ... >> >> LGTM >> Reviewed-by: DJ Delorie <dj@redhat.com> >> > > Starting with this commit a6d2f948b71adcb5ea395cb04833bc645eab45e6, I > get a test fail on s390: > FAIL: resolv/tst-p_secstodate > > Test 0: 0 -> 19700101000000 > Test 1: 12345 -> 19700101000000 > test 1 failedTest 2: 999999999 -> 19700101000000 > test 2 failedTest 3: 2147483647 -> 19700101000000 > test 3 failedTest 4: 2147483648 -> <overflow> > Test 5: 4294967295 -> <overflow> > > It turns out that there is a type-mismatch of time_t on caller and > callee side. > > in libresolv.so: resolv/res_debug.c: __p_secstodate(u_long secs) > time_t clock = secs; > struct tm *time = __gmtime_r(&clock, &timebuf); > > (gdb) p &clock > $1 = (time_t *) 0x7ffff1c8 > (gdb) ptype time_t > type = long long > (gdb) p sizeof(time_t) > $2 = 8 > (gdb) x/2xw 0x7ffff1c8 > 0x7ffff1c8: 0x00000000 0x7fffffff > > The secs are stored as 8byte long long on stack and the pointer is > passed to __gmtime_r in libc.so: > time/gmtime.c: > /* Provide a 32-bit variant if needed. */ > #if __TIMESIZE != 64 > struct tm * > __gmtime_r (const time_t *t, struct tm *tp) > { > __time64_t t64 = *t; > return __gmtime64_r (&t64, tp); > } > #endif > > (gdb) ptype time_t > type = long > (gdb) p sizeof(time_t) > $3 = 4 > (gdb) p *t > $4 = 0 > (gdb) ptype __time64_t > type = long long > > On libc.so side, time_t is defined as 4byte long, thus the value of t is > read as 0x0 instead of 0x7fffffff. > > resolv/res_debug.c is built with -D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64 > and time/gmtime.c is built without those defines. > > > @Adhemerval: > Can you please have a look? libresolv should not be built with the time64 flags, the above patch should fix it. diff --git a/Makeconfig b/Makeconfig index 06f1cca320..9b6fc6b08f 100644 --- a/Makeconfig +++ b/Makeconfig @@ -867,7 +867,7 @@ endif +extra-math-flags = $(if $(filter libm,$(in-module)),-fno-math-errno,-fmath-errno) # Use 64 bit time_t support for installed programs -installed-modules = nonlib nscd lddlibc4 libresolv ldconfig locale_programs \ +installed-modules = nonlib nscd lddlibc4 ldconfig locale_programs \ iconvprogs libnss_files libnss_compat libnss_db libnss_hesiod \ libutil libpcprofile libSegFault +extra-time-flags = $(if $(filter $(installed-modules),\ I have this issue on my backlog since I saw it on powerpc32. I will check on i686 and powerpc and install it.
On 12/01/2022 15:44, Adhemerval Zanella wrote: > > > On 12/01/2022 11:13, Stefan Liebler wrote: >> On 14/12/2021 20:35, DJ Delorie via Libc-alpha wrote: >> ... >>> >>> LGTM >>> Reviewed-by: DJ Delorie <dj@redhat.com> >>> >> >> Starting with this commit a6d2f948b71adcb5ea395cb04833bc645eab45e6, I >> get a test fail on s390: >> FAIL: resolv/tst-p_secstodate >> >> Test 0: 0 -> 19700101000000 >> Test 1: 12345 -> 19700101000000 >> test 1 failedTest 2: 999999999 -> 19700101000000 >> test 2 failedTest 3: 2147483647 -> 19700101000000 >> test 3 failedTest 4: 2147483648 -> <overflow> >> Test 5: 4294967295 -> <overflow> >> >> It turns out that there is a type-mismatch of time_t on caller and >> callee side. >> >> in libresolv.so: resolv/res_debug.c: __p_secstodate(u_long secs) >> time_t clock = secs; >> struct tm *time = __gmtime_r(&clock, &timebuf); >> >> (gdb) p &clock >> $1 = (time_t *) 0x7ffff1c8 >> (gdb) ptype time_t >> type = long long >> (gdb) p sizeof(time_t) >> $2 = 8 >> (gdb) x/2xw 0x7ffff1c8 >> 0x7ffff1c8: 0x00000000 0x7fffffff >> >> The secs are stored as 8byte long long on stack and the pointer is >> passed to __gmtime_r in libc.so: >> time/gmtime.c: >> /* Provide a 32-bit variant if needed. */ >> #if __TIMESIZE != 64 >> struct tm * >> __gmtime_r (const time_t *t, struct tm *tp) >> { >> __time64_t t64 = *t; >> return __gmtime64_r (&t64, tp); >> } >> #endif >> >> (gdb) ptype time_t >> type = long >> (gdb) p sizeof(time_t) >> $3 = 4 >> (gdb) p *t >> $4 = 0 >> (gdb) ptype __time64_t >> type = long long >> >> On libc.so side, time_t is defined as 4byte long, thus the value of t is >> read as 0x0 instead of 0x7fffffff. >> >> resolv/res_debug.c is built with -D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64 >> and time/gmtime.c is built without those defines. >> >> >> @Adhemerval: >> Can you please have a look? > > > libresolv should not be built with the time64 flags, the above patch should fix > it. > > diff --git a/Makeconfig b/Makeconfig > index 06f1cca320..9b6fc6b08f 100644 > --- a/Makeconfig > +++ b/Makeconfig > @@ -867,7 +867,7 @@ endif > +extra-math-flags = $(if $(filter libm,$(in-module)),-fno-math-errno,-fmath-errno) > > # Use 64 bit time_t support for installed programs > -installed-modules = nonlib nscd lddlibc4 libresolv ldconfig locale_programs \ > +installed-modules = nonlib nscd lddlibc4 ldconfig locale_programs \ > iconvprogs libnss_files libnss_compat libnss_db libnss_hesiod \ > libutil libpcprofile libSegFault > +extra-time-flags = $(if $(filter $(installed-modules),\ > > I have this issue on my backlog since I saw it on powerpc32. I will check on > i686 and powerpc and install it. Thanks for the commit. This fixes the failing test.
diff --git a/Makeconfig b/Makeconfig index 407df9e6a1..37ce4ef365 100644 --- a/Makeconfig +++ b/Makeconfig @@ -851,6 +851,13 @@ endif # -fno-math-errno. +extra-math-flags = $(if $(filter libm,$(in-module)),-fno-math-errno,-fmath-errno) +# Use 64 bit time_t support for installed programs +installed-modules = nonlib nscd lddlibc4 libresolv ldconfig locale_programs \ + iconvprogs libnss_files libnss_compat libnss_db libnss_hesiod \ + libutil libpcprofile libSegFault ++extra-time-flags = $(if $(filter $(installed-modules),\ + $(in-module)),-D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64) + # We might want to compile with some stack-protection flag. ifneq ($(stack-protector),) +stack-protector=$(stack-protector) @@ -951,7 +958,7 @@ libio-include = -I$(..)libio built-modules = iconvprogs iconvdata ldconfig lddlibc4 libmemusage \ libSegFault libpcprofile librpcsvc locale-programs \ memusagestat nonlib nscd extramodules libnldbl libsupport \ - testsuite + testsuite testsuite-internal in-module = $(subst -,_,$(firstword $(libof-$(basename $(@F))) \ $(libof-$(<F)) \ @@ -991,7 +998,7 @@ endif override CFLAGS = -std=gnu11 -fgnu89-inline $(config-extra-cflags) \ $(filter-out %frame-pointer,$(+cflags)) $(+gccwarn-c) \ - $(+extra-math-flags) \ + $(+extra-math-flags) $(+extra-time-flags) \ $(sysdep-CFLAGS) $(CFLAGS-$(suffix $@)) $(CFLAGS-$(<F)) \ $(CFLAGS-$(@F)) $(tls-model) \ $(foreach lib,$(libof-$(basename $(@F))) \ diff --git a/Makerules b/Makerules index 12f1a5cb50..84345afc21 100644 --- a/Makerules +++ b/Makerules @@ -1313,14 +1313,22 @@ lib := testsuite include $(patsubst %,$(..)libof-iterator.mk,$(cpp-srcs-left)) endif -all-nonlib := $(strip $(tests-internal) $(test-internal-extras) \ - $(others) $(others-extras)) +all-nonlib := $(strip $(others) $(others-extras)) ifneq (,$(all-nonlib)) cpp-srcs-left = $(all-nonlib) lib := nonlib include $(patsubst %,$(..)libof-iterator.mk,$(cpp-srcs-left)) endif +# All internal tests use testsuite-internal module since for 64 bit time +# support is set as default for MODULE_NAME=nonlib (which include some +# installed programs). +all-testsuite-internal := $(strip $(tests-internal) $(test-internal-extras)) +ifneq (,$(all-testsuite-internal)) +cpp-srcs-left = $(all-testsuite-internal) +lib := testsuite-internal +include $(patsubst %,$(..)libof-iterator.mk,$(cpp-srcs-left)) +endif ifeq ($(build-shared),yes) # Generate normalized lists of symbols, versions, and data sizes. diff --git a/elf/sotruss-lib.c b/elf/sotruss-lib.c index b711f7b0c8..b51458992a 100644 --- a/elf/sotruss-lib.c +++ b/elf/sotruss-lib.c @@ -90,7 +90,7 @@ init (void) if (which_process == NULL || which_process[0] == '\0') snprintf (endp, 13, ".%ld", (long int) pid); - out_fd = open (fullname, O_RDWR | O_CREAT | O_TRUNC, 0666); + out_fd = open64 (fullname, O_RDWR | O_CREAT | O_TRUNC, 0666); if (out_fd != -1) print_pid = 0; } @@ -103,7 +103,7 @@ init (void) program. */ if (out_fd == -1) { - out_fd = fcntl (STDERR_FILENO, F_DUPFD, 1000); + out_fd = fcntl64 (STDERR_FILENO, F_DUPFD, 1000); if (out_fd == -1) out_fd = dup (STDERR_FILENO); }