This is the mail archive of the libc-ports@sources.redhat.com mailing list for the libc-ports project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Alternative fix for MIPS fstatat() problems


Hi Richard

I applied the patch and tried to build mips64/n32 abi and this failed because the build picked
default fxstatat.c from libc/sysdeps/unix/sysv/linux/


mips32/o32 was ok

Thanks

Khem

Richard Sandiford said the following on 09/19/2006 11:51 AM:
As Khem said here:

http://sources.redhat.com/ml/libc-ports/2006-08/msg00036.html

it isn't possible to build MIPS with --enable-kernel>=2.6.17.
Khem added:

  Index: sysdeps/unix/sysv/linux/mips/fxstatat.c
  ===================================================================
  --- /dev/null
  +++ sysdeps/unix/sysv/linux/mips/fxstatat.c
  @@ -0,0 +1 @@
  +#include <sysdeps/unix/sysv/linux/i386/fxstatat.c>

but although this compiles, I don't think it's right.  i386/fxstatat.c
expects the syscall to be called __NR_fstatat64, but it isn't for any of
the three MIPS ABIs.  fxstatat() therefore becomes an empty function,
without even a return statement.  (gcc does warn.)  The same problem
already applies to fxstatat64.c.

As I said on the linux-mips list, MIPS is unique in calling the
fstatat() syscall "__NR_fstatat".  I think this is misleading for o32,
as the syscall provides a stat64, and all the other stat64 syscalls have
a "64" suffix.  Other 64-bit targets call the syscall "__NR_newfstatat"
instead of "__NR_fstatat".

So I think we should:

(1) Rename the o32 syscall to "__NR_fstatat64".

(2) Rename the n32 and n64 syscalls to "__NR_newfstatat".

  (3) Add Khem's forwarder to mips/mip32 and thus use i386/fstatat.c
      for o32.  The file expects a stat64 syscall called "__NR_fstatat64"
      and uses __xstat32_conv to convert it to a 32-bit stat.

  (4) Use the generic fstatat64.c for o32.  The file expects a stat64
      syscall called "__NR_fstatat64" and does no conversion on the
      result.  This is OK because the kernel and user stat64 fields
      have the same offsets and sizes on o32.  The only difference
      is that the user stat64 has more trailing padding.

  (5) Use the generic fstatat.c for n32 and n64.  The file expects
      a plain stat syscall called "__NR_newfstatat".  It uses
      __xconv_stat to convert the result.

  (6) Use a new fstatat64.c for n32 and n64.  Call __NR_newfstatat
      and use __xstat_conv to convert the result.  (The kernel stat
      fields are not the same as the user stat64 fields; in particular,
      st_dev is 32-bit for the former and 64-bit for the latter.)

(7) Define __xstat32_conv for o32, as required by (3).

I've submitted a patch for (1) and (2) to the linux-mips list.
The patch below does (3)-(7).  I also noticed a couple of other
things in xstatconv.c:

  - __xstat_conv doesn't check for overflow
  - we don't clear all the st_pad5 padding

I thought I'd fix those problems at the same time because the code
in question is basically the same as the new __xstat32_conv code.
I thought all three xstat functions should remain consistent.

I've tested the patch with and without --enable-kernel=2.6.18.
In the "without" case, I tested with a stock 2.6.18 kernel and
then with one in which the syscalls were disabled.  All three
combinations seemed to work for all three ABIs.

I've attached the test I used.  I compiled it twice for each
combination, once with and once without -D_FILE_OFFSET_BITS=64.

Richard


* sysdeps/unix/sysv/linux/mips/xstatconv.c: Remove STAT_IS_KERNEL_STAT code. (__xstat_conv): Use memset to clear padding arrays. Check for overflow. (__xstat64_conv): Use memset to clear padding arrays. (__xstat32_conv): New function. * sysdeps/unix/sysv/linux/mips/mips32/fxstatat.c: New file. * sysdeps/unix/sysv/linux/mips/mips64/fxstatat64.c: Likewise.

------------------------------------------------------------------------

Index: sysdeps/unix/sysv/linux/mips/xstatconv.c
===================================================================
RCS file: /cvs/glibc/ports/sysdeps/unix/sysv/linux/mips/xstatconv.c,v
retrieving revision 1.6
diff -u -p -r1.6 xstatconv.c
--- sysdeps/unix/sysv/linux/mips/xstatconv.c 2 Jun 2006 15:29:03 -0000 1.6
+++ sysdeps/unix/sysv/linux/mips/xstatconv.c 19 Sep 2006 18:14:26 -0000
@@ -21,16 +21,8 @@
#include <sys/stat.h>
#include <kernel_stat.h>
-#ifdef STAT_IS_KERNEL_STAT
-
-/* Dummy. */
-struct kernel_stat;
-
-#else
-
#include <string.h>
-
int
__xstat_conv (int vers, struct kernel_stat *kbuf, void *ubuf)
{
@@ -49,30 +41,43 @@ __xstat_conv (int vers, struct kernel_st
/* Convert to current kernel version of `struct stat'. */
buf->st_dev = kbuf->st_dev;
- buf->st_pad1[0] = 0; buf->st_pad1[1] = 0; buf->st_pad1[2] = 0;
+ memset (&buf->st_pad1, 0, sizeof (buf->st_pad1));
buf->st_ino = kbuf->st_ino;
+ /* Check for overflow. */
+ if (buf->st_ino != kbuf->st_ino)
+ {
+ __set_errno (EOVERFLOW);
+ return -1;
+ }
buf->st_mode = kbuf->st_mode;
buf->st_nlink = kbuf->st_nlink;
buf->st_uid = kbuf->st_uid;
buf->st_gid = kbuf->st_gid;
buf->st_rdev = kbuf->st_rdev;
- buf->st_pad2[0] = 0; buf->st_pad2[1] = 0;
- buf->st_pad3 = 0;
+ memset (&buf->st_pad2, 0, sizeof (buf->st_pad2));
buf->st_size = kbuf->st_size;
- buf->st_blksize = kbuf->st_blksize;
- buf->st_blocks = kbuf->st_blocks;
-
+ /* Check for overflow. */
+ if (buf->st_size != kbuf->st_size)
+ {
+ __set_errno (EOVERFLOW);
+ return -1;
+ }
+ buf->st_pad3 = 0;
buf->st_atim.tv_sec = kbuf->st_atime_sec;
buf->st_atim.tv_nsec = kbuf->st_atime_nsec;
buf->st_mtim.tv_sec = kbuf->st_mtime_sec;
buf->st_mtim.tv_nsec = kbuf->st_mtime_nsec;
buf->st_ctim.tv_sec = kbuf->st_ctime_sec;
buf->st_ctim.tv_nsec = kbuf->st_ctime_nsec;
-
- buf->st_pad5[0] = 0; buf->st_pad5[1] = 0;
- buf->st_pad5[2] = 0; buf->st_pad5[3] = 0;
- buf->st_pad5[4] = 0; buf->st_pad5[5] = 0;
- buf->st_pad5[6] = 0; buf->st_pad5[7] = 0;
+ buf->st_blksize = kbuf->st_blksize;
+ buf->st_blocks = kbuf->st_blocks;
+ /* Check for overflow. */
+ if (buf->st_blocks != kbuf->st_blocks)
+ {
+ __set_errno (EOVERFLOW);
+ return -1;
+ }
+ memset (&buf->st_pad5, 0, sizeof (buf->st_pad5));
}
break;
@@ -97,14 +102,14 @@ __xstat64_conv (int vers, struct kernel_
struct stat64 *buf = ubuf;
buf->st_dev = kbuf->st_dev;
- buf->st_pad1[0] = 0; buf->st_pad1[1] = 0; buf->st_pad1[2] = 0;
+ memset (&buf->st_pad1, 0, sizeof (buf->st_pad1));
buf->st_ino = kbuf->st_ino;
buf->st_mode = kbuf->st_mode;
buf->st_nlink = kbuf->st_nlink;
buf->st_uid = kbuf->st_uid;
buf->st_gid = kbuf->st_gid;
buf->st_rdev = kbuf->st_rdev;
- buf->st_pad2[0] = 0; buf->st_pad2[1] = 0; buf->st_pad2[2] = 0;
+ memset (&buf->st_pad2, 0, sizeof (buf->st_pad2));
buf->st_pad3 = 0;
buf->st_size = kbuf->st_size;
buf->st_blksize = kbuf->st_blksize;
@@ -117,10 +122,7 @@ __xstat64_conv (int vers, struct kernel_
buf->st_ctim.tv_sec = kbuf->st_ctime_sec;
buf->st_ctim.tv_nsec = kbuf->st_ctime_nsec;
- buf->st_pad4[0] = 0; buf->st_pad4[1] = 0;
- buf->st_pad4[2] = 0; buf->st_pad4[3] = 0;
- buf->st_pad4[4] = 0; buf->st_pad4[5] = 0;
- buf->st_pad4[6] = 0; buf->st_pad4[7] = 0;
+ memset (&buf->st_pad4, 0, sizeof (buf->st_pad4));
}
break;
@@ -136,4 +138,65 @@ __xstat64_conv (int vers, struct kernel_
#endif
}
-#endif /* ! STAT_IS_KERNEL_STAT */
+#if _MIPS_SIM == _ABIO32
+int
+__xstat32_conv (int vers, struct stat64 *kbuf, struct stat *buf)
+{
+ switch (vers)
+ {
+ case _STAT_VER_LINUX:
+ /* Convert current kernel version of `struct stat64' to
+ `struct stat'. The layout of the fields in the kernel's
+ stat64 is the same as that in the user stat64; the only
+ difference is that the latter has more trailing padding. */
+ buf->st_dev = kbuf->st_dev;
+ memset (&buf->st_pad1, 0, sizeof (buf->st_pad1));
+ buf->st_ino = kbuf->st_ino;
+ /* Check for overflow. */
+ if (buf->st_ino != kbuf->st_ino)
+ {
+ __set_errno (EOVERFLOW);
+ return -1;
+ }
+ buf->st_mode = kbuf->st_mode;
+ buf->st_nlink = kbuf->st_nlink;
+ buf->st_uid = kbuf->st_uid;
+ buf->st_gid = kbuf->st_gid;
+ buf->st_rdev = kbuf->st_rdev;
+ memset (&buf->st_pad2, 0, sizeof (buf->st_pad2));
+ buf->st_size = kbuf->st_size;
+ /* Check for overflow. */
+ if (buf->st_size != kbuf->st_size)
+ {
+ __set_errno (EOVERFLOW);
+ return -1;
+ }
+ buf->st_pad3 = 0;
+ buf->st_atim.tv_sec = kbuf->st_atim.tv_sec;
+ buf->st_atim.tv_nsec = kbuf->st_atim.tv_nsec;
+ buf->st_mtim.tv_sec = kbuf->st_mtim.tv_sec;
+ buf->st_mtim.tv_nsec = kbuf->st_mtim.tv_nsec;
+ buf->st_ctim.tv_sec = kbuf->st_ctim.tv_sec;
+ buf->st_ctim.tv_nsec = kbuf->st_ctim.tv_nsec;
+ buf->st_blksize = kbuf->st_blksize;
+ buf->st_blocks = kbuf->st_blocks;
+ /* Check for overflow. */
+ if (buf->st_blocks != kbuf->st_blocks)
+ {
+ __set_errno (EOVERFLOW);
+ return -1;
+ }
+ memset (&buf->st_pad5, 0, sizeof (buf->st_pad5));
+ break;
+
+ /* If struct stat64 is different from struct stat then
+ _STAT_VER_KERNEL does not make sense. */
+ case _STAT_VER_KERNEL:
+ default:
+ __set_errno (EINVAL);
+ return -1;
+ }
+
+ return 0;
+}
+#endif /* _MIPS_SIM == _ABIO32 */
Index: sysdeps/unix/sysv/linux/mips/mips32/fxstatat.c
===================================================================
RCS file: sysdeps/unix/sysv/linux/mips/mips32/fxstatat.c
diff -N sysdeps/unix/sysv/linux/mips/mips32/fxstatat.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ sysdeps/unix/sysv/linux/mips/mips32/fxstatat.c 19 Sep 2006 18:14:26 -0000
@@ -0,0 +1 @@
+#include <sysdeps/unix/sysv/linux/i386/fxstatat.c>
Index: sysdeps/unix/sysv/linux/mips/mips64/fxstatat64.c
===================================================================
RCS file: sysdeps/unix/sysv/linux/mips/mips64/fxstatat64.c
diff -N sysdeps/unix/sysv/linux/mips/mips64/fxstatat64.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ sysdeps/unix/sysv/linux/mips/mips64/fxstatat64.c 19 Sep 2006 18:14:26 -0000
@@ -0,0 +1,113 @@
+/* Copyright (C) 2005,2006 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, write to the Free
+ Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+ 02111-1307 USA. */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <kernel_stat.h>
+
+#include <sysdep.h>
+#include <sys/syscall.h>
+#include <bp-checks.h>
+
+#include <kernel-features.h>
+
+#include <xstatconv.h>
+
+/* Get information about the file NAME in BUF. */
+
+int
+__fxstatat64 (int vers, int fd, const char *file, struct stat64 *st, int flag)
+{
+ if (__builtin_expect (vers != _STAT_VER_LINUX, 0))
+ {
+ __set_errno (EINVAL);
+ return -1;
+ }
+
+ int result;
+ INTERNAL_SYSCALL_DECL (err);
+ struct kernel_stat kst;
+
+#ifdef __NR_newfstatat
+# ifndef __ASSUME_ATFCTS
+ if (__have_atfcts >= 0)
+# endif
+ {
+ result = INTERNAL_SYSCALL (newfstatat, err, 4, fd, file, &kst, flag);
+# ifndef __ASSUME_ATFCTS
+ if (__builtin_expect (INTERNAL_SYSCALL_ERROR_P (result, err), 1)
+ && INTERNAL_SYSCALL_ERRNO (result, err) == ENOSYS)
+ __have_atfcts = -1;
+ else
+# endif
+ if (!__builtin_expect (INTERNAL_SYSCALL_ERROR_P (result, err), 1))
+ return __xstat64_conv (vers, &kst, st);
+ else
+ {
+ __set_errno (INTERNAL_SYSCALL_ERRNO (result, err));
+ return -1;
+ }
+ }
+#endif
+
+#ifndef __ASSUME_ATFCTS
+ if (flag & ~AT_SYMLINK_NOFOLLOW)
+ {
+ __set_errno (EINVAL);
+ return -1;
+ }
+
+ char *buf = NULL;
+
+ if (fd != AT_FDCWD && file[0] != '/')
+ {
+ size_t filelen = strlen (file);
+ static const char procfd[] = "/proc/self/fd/%d/%s";
+ /* Buffer for the path name we are going to use. It consists of
+ - the string /proc/self/fd/
+ - the file descriptor number
+ - the file name provided.
+ The final NUL is included in the sizeof. A bit of overhead
+ due to the format elements compensates for possible negative
+ numbers. */
+ size_t buflen = sizeof (procfd) + sizeof (int) * 3 + filelen;
+ buf = alloca (buflen);
+
+ __snprintf (buf, buflen, procfd, fd, file);
+ file = buf;
+ }
+
+ if (flag & AT_SYMLINK_NOFOLLOW)
+ result = INTERNAL_SYSCALL (lstat, err, 2, CHECK_STRING (file),
+ __ptrvalue (&kst));
+ else
+ result = INTERNAL_SYSCALL (stat, err, 2, CHECK_STRING (file),
+ __ptrvalue (&kst));
+
+ if (__builtin_expect (!INTERNAL_SYSCALL_ERROR_P (result, err), 1))
+ return __xstat64_conv (vers, &kst, st);
+
+ __atfct_seterrno (INTERNAL_SYSCALL_ERRNO (result, err), fd, buf);
+ return -1;
+#endif
+}
+libc_hidden_def (__fxstatat64)
------------------------------------------------------------------------


#define _ATFILE_SOURCE

#include <sys/types.h>
#include <sys/stat.h>
#include <dirent.h>
#include <alloca.h>
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <fcntl.h>

static int fails;

static void
do_directory (const char *dirname)
{
  char *pathname, *filename;
  DIR *dir;
  struct dirent *entry;
  size_t dirname_len;
  struct stat stat1, stat2;
  int fd;

  dir = opendir (dirname);
  if (dir == NULL)
    {
      perror (dirname);
      exit (1);
    }

  /* This test is not designed for deep hierarchies. ;)  */
  dirname_len = strlen (dirname);
  pathname = alloca (dirname_len + 1 + NAME_MAX + 1);
  memcpy (pathname, dirname, dirname_len);
  pathname[dirname_len] = '/';
  filename = pathname + dirname_len + 1;

  fd = dirfd (dir);
  fd = open (".", O_RDONLY);
  while ((entry = readdir (dir)))
    if (strcmp (entry->d_name, ".") != 0
	&& strcmp (entry->d_name, "..") != 0)
      {
	strcpy (filename, entry->d_name);
	if (stat (pathname, &stat1) != 0
	    || fstatat (AT_FDCWD, pathname, &stat2, 0) != 0)
	  {
	    perror (pathname);
	    exit (1);
	  }
	if (stat1.st_dev != stat2.st_dev
	    || stat1.st_ino != stat2.st_ino
	    || stat1.st_mode != stat2.st_mode
	    || stat1.st_nlink != stat2.st_nlink
	    || stat1.st_uid != stat2.st_uid
	    || stat1.st_gid != stat2.st_gid
	    || stat1.st_rdev != stat2.st_rdev
	    || stat1.st_size != stat2.st_size
	    || stat1.st_atime != stat2.st_atime
	    || stat1.st_mtime != stat2.st_mtime
	    || stat1.st_ctime != stat2.st_ctime
	    || stat1.st_blksize != stat2.st_blksize
	    || stat1.st_blocks != stat2.st_blocks)
	  {
	    fprintf (stderr, "Mismatch for %s\n", pathname);
	    fails++;
	  }
	if (S_ISDIR (stat1.st_mode))
	  do_directory (pathname);
      }
  close (fd);
  closedir (dir);
}

int
main (void)
{
do_directory (".");
return fails != 0;
}


--
Khem Raj
MontaVista Software Inc.
kraj@mvista.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]