[v2] zic, various tests: use 64-bit stat/ftw/readdir functions

Message ID 87k23zhvas.fsf@esperi.org.uk
State New, archived
Headers

Commit Message

Nix June 26, 2017, 10:56 a.m. UTC
  From: Nick Alcock <nick.alcock@oracle.com>

This fixes this compilation failure when testing a 32-bit glibc on a
system using a 64-bit-inode XFS filesystem, and a variety of related
test failures of dirent/list, io/bug-ftw2, and posix/tst-regex:

env GCONV_PATH=/usr/src/glibc/i686-loom/iconvdata LOCPATH=/usr/src/glibc/i686-loom/localedata LC_ALL=C  /usr/src/glibc/i686-loom/elf/ld-linux.so.2 --library-path /usr/src/glibc/i686-loom:/usr/src/glibc/i686-loom/math:/usr/src/glibc/i686-loom/elf:/usr/src/glibc/i686-loom/dlfcn:/usr/src/glibc/i686-loom/nss:/usr/src/glibc/i686-loom/nis:/usr/src/glibc/i686-loom/rt:/usr/src/glibc/i686-loom/resolv:/usr/src/glibc/i686-loom/crypt:/usr/src/glibc/i686-loom/mathvec:/usr/src/glibc/i686-loom/support:/usr/src/glibc/i686-loom/nptl /usr/src/glibc/i686-loom/timezone/zic -d /usr/src/glibc/i686-loom/timezone/testdata -y ./yearistype australasia; ../scripts/evaluate-test.sh timezone/testdata/Australia/Melbourne $? false false > /usr/src/glibc/i686-loom/timezone/testdata/Australia/Melbourne.test-result
warning: "etcetera", line 13: /usr/src/glibc/i686-loom/timezone/zic: Can't create directory /usr/src: File exists
/bin/sh: /usr/src/glibc/i686-loom/timezone/testdata/Etc/UTC.test-result: No such file or directory
make[2]: *** [Makefile:98: /usr/src/glibc/i686-loom/timezone/testdata/Etc/UTC] Error 1

This happens because itsdir() in timezone/zic.c does a stat() of each
element of the path in turn, and this returns -EOVERFLOW because on this
system /usr has an inode number of 7516193792 and we did not compile zic
with -D_FILE_OFFSET_BITS=64 or use stat64().  Do the former in
tz-cflags, to avoid unnecessarily skewing zic from its upstream.

We can fix the other test failures via use of the *64() functions
directly.

This at least allows a 32-bit libc with the source tree on a
64-bit-inode fs to pass make check for me. It may well not be the
minimum required for successful operations, though nothing else in
glibc's tools is obviously troublesome that I can see: among other
things, my *objdir* is still 32-bit... (There is one test that calls ftw
("/") which is unlikely to have an inode number > 2^32 on any current
filesystem, and one that calls it on a directory under /tmp which I
cannot easily test a fix for because my /tmp is a tmpfs with 32-bit
inodes. I can fix these two as well if you like, though, and make this
more of a maximal fix rather than a minimal one.)

This probably does not fix all of #15333, because of the cases above,
but it gets us a bit closer.

	[BZ #15333]
	* timezone/Makefile (tz-cflags): Set FILE_OFFSET_BITS.
	* posix/tst-regex.c (do_test): Use fstat64.
	* dirent/list.c (list): Use dirent64/readdir64.
	* io/bug-ftw.c (main): Use ftw64.
  

Comments

Joseph Myers June 26, 2017, 5:24 p.m. UTC | #1
On Mon, 26 Jun 2017, Nick Alcock wrote:

> Index: glibc/io/bug-ftw2.c
> ===================================================================
> --- glibc.orig/io/bug-ftw2.c	2017-06-26 11:32:54.460712810 +0100
> +++ glibc/io/bug-ftw2.c	2017-06-26 11:33:38.730559302 +0100
> @@ -69,7 +69,7 @@
>  {
>    mtrace ();
>  
> -  ftw (".", callback, 10);
> +  ftw64 (".", callback, 10);
>  
>    if (! sawcur)
>      {

A test of ftw should arguably be using ftw exactly, not ftw64, and 
probably be paired with a corresponding tests (using a shared test 
skeleton) that tests the same things for ftw64.  I.e., adding appropriate 
checks so this test doesn't fail for 64-bit inode numbers might be better, 
to avoid changing what function this test tests.
  
Nix June 26, 2017, 5:47 p.m. UTC | #2
On 26 Jun 2017, Joseph Myers said:

> On Mon, 26 Jun 2017, Nick Alcock wrote:
>
>> Index: glibc/io/bug-ftw2.c
>> ===================================================================
>> --- glibc.orig/io/bug-ftw2.c	2017-06-26 11:32:54.460712810 +0100
>> +++ glibc/io/bug-ftw2.c	2017-06-26 11:33:38.730559302 +0100
>> @@ -69,7 +69,7 @@
>>  {
>>    mtrace ();
>>  
>> -  ftw (".", callback, 10);
>> +  ftw64 (".", callback, 10);
>>  
>>    if (! sawcur)
>>      {
>
> A test of ftw should arguably be using ftw exactly, not ftw64, and 
> probably be paired with a corresponding tests (using a shared test 
> skeleton) that tests the same things for ftw64.  I.e., adding appropriate 
> checks so this test doesn't fail for 64-bit inode numbers might be better, 
> to avoid changing what function this test tests.

i.e. UNSUPPORTED-out this test iff it EOVERFLOWs, and have another
bug-ftw64-2.c that uses ftw64 instead and is run unconditionally?

(And similarly for the other non-64 ftw tests, probably all using the
usual macro-expansion-and-#include trick to keep code duplication down.)

Makes considerable sense: I'll look at that shortly.
  
Joseph Myers June 26, 2017, 5:51 p.m. UTC | #3
On Mon, 26 Jun 2017, Nick Alcock wrote:

> > A test of ftw should arguably be using ftw exactly, not ftw64, and 
> > probably be paired with a corresponding tests (using a shared test 
> > skeleton) that tests the same things for ftw64.  I.e., adding appropriate 
> > checks so this test doesn't fail for 64-bit inode numbers might be better, 
> > to avoid changing what function this test tests.
> 
> i.e. UNSUPPORTED-out this test iff it EOVERFLOWs, and have another
> bug-ftw64-2.c that uses ftw64 instead and is run unconditionally?

Yes, exactly.  tst-regex.c is different because that's not testing the I/O 
functions, just using them, so unconditionally using fstat64 makes sense 
there.
  

Patch

Index: glibc/dirent/list.c
===================================================================
--- glibc.orig/dirent/list.c	2017-06-26 11:32:54.460712810 +0100
+++ glibc/dirent/list.c	2017-06-26 11:33:38.730559302 +0100
@@ -26,7 +26,7 @@ 
 test (const char *name)
 {
   DIR *dirp;
-  struct dirent *entp;
+  struct dirent64 *entp;
   int retval = 0;
 
   puts (name);
@@ -39,7 +39,7 @@ 
     }
 
   errno = 0;
-  while ((entp = readdir (dirp)) != NULL)
+  while ((entp = readdir64 (dirp)) != NULL)
     printf ("%s\tfile number %lu\n",
 	    entp->d_name, (unsigned long int) entp->d_fileno);
 
Index: glibc/io/bug-ftw2.c
===================================================================
--- glibc.orig/io/bug-ftw2.c	2017-06-26 11:32:54.460712810 +0100
+++ glibc/io/bug-ftw2.c	2017-06-26 11:33:38.730559302 +0100
@@ -69,7 +69,7 @@ 
 {
   mtrace ();
 
-  ftw (".", callback, 10);
+  ftw64 (".", callback, 10);
 
   if (! sawcur)
     {
Index: glibc/posix/tst-regex.c
===================================================================
--- glibc.orig/posix/tst-regex.c	2017-06-26 11:32:59.470695445 +0100
+++ glibc/posix/tst-regex.c	2017-06-26 11:33:38.730559302 +0100
@@ -57,7 +57,7 @@ 
 {
   const char *file;
   int fd;
-  struct stat st;
+  struct stat64 st;
   int result;
   char *inmem;
   char *outmem;
@@ -72,7 +72,7 @@ 
   if (fd == -1)
     error (EXIT_FAILURE, errno, "cannot open %s", basename (file));
 
-  if (fstat (fd, &st) != 0)
+  if (fstat64 (fd, &st) != 0)
     error (EXIT_FAILURE, errno, "cannot stat %s", basename (file));
   memlen = st.st_size;
 
Index: glibc/timezone/Makefile
===================================================================
--- glibc.orig/timezone/Makefile	2017-06-26 11:33:00.120693193 +0100
+++ glibc/timezone/Makefile	2017-06-26 11:34:16.380428632 +0100
@@ -60,7 +60,8 @@ 
 	    -DTZDEFRULES='"$(posixrules-file)"' \
 	    -DTM_GMTOFF=tm_gmtoff -DTM_ZONE=tm_zone \
 	    -DHAVE_GETTEXT -DUSE_LTZ=0 -D_ISOMAC -DTZ_DOMAIN='"libc"' \
-	    -include $(common-objpfx)config.h -Wno-maybe-uninitialized
+	    -include $(common-objpfx)config.h -Wno-maybe-uninitialized \
+	    -D_FILE_OFFSET_BITS=64
 
 # The -Wno-unused-variable flag is used to prevent GCC 6
 # from warning about time_t_min and time_t_max which are