[Qemu-devel] [PATCH 0/2] linux-user: Drop direct use of openat etc syscalls

[Qemu-devel] [PATCH 0/2] linux-user: Drop direct use of openat etc syscalls

[Peter Maydell]

The linux-user syscall emulation layer currently supports the openat
family of syscalls via two mechanisms: simply calling the corresponding
libc functions, and making direct syscalls. Since glibc has supported
these functions since at least glibc 2.5, there's no real need to
retain the (essentially untested) direct syscall fallback code, so
this patchset simply deletes it.

This allows us to remove some ifdeffery that was attempting to disable
provision of some of the syscalls if the host didn't seem to support
them, which in some cases was actually wrong. For example where there
are several flavours of the syscall, we only need one of them, not
necessarily the exact one the guest has, as with the fstatat* calls.
And if the guest needs the futimesat() syscall we can provide it
via glibc, even if that syscall is deprecated or not provided in the
host (because the host implements utimensat instead). AArch64 in
particular hits the last of these, which resulted in a compile
failure due to an unused function, because the syscall implementation's
ifdef was inconsistent with the ifdef used to define the sys_futimesat()
function.

Basically, removing the ugly direct syscall access seemed nicer
than trying to fix up and render consistent the broken ifdefs :-)

[RHEL5 has glibc2.5 and provides these functions. RHEL4 did not
but we don't build on RHEL4 anyhow because its glib is too old.
uClibc provides these functions.]

Peter Maydell (2):
  linux-user: Drop direct use of openat etc syscalls
  configure: Drop CONFIG_ATFILE test

 configure            |   26 ------
 linux-user/syscall.c |  218 ++++++--------------------------------------------
 2 files changed, 24 insertions(+), 220 deletions(-)

-- 
1.7.9.5

[Qemu-devel] [PATCH 1/2] linux-user: Drop direct use of openat etc syscalls

[Peter Maydell]

The linux-user syscall emulation layer currently supports the
openat family of syscalls via two mechanisms: simply calling
the corresponding libc functions, and making direct syscalls.
Since glibc has supported these functions since at least glibc
2.5, there's no real need to retain the (essentially untested)
direct syscall fallback code, so simply delete it. This allows
us to remove some ifdeffery that was attempting to disable
provision of some of the syscalls if the host didn't seem to
support them, which in some cases was actually wrong (eg where
there are several flavours of the syscall and we only need
one of them, not necessarily the exact one the guest has,
as with the fstatat* calls).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c |  218 ++++++--------------------------------------------
 1 file changed, 24 insertions(+), 194 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 121218b..053fa14 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -181,29 +181,14 @@ static type name (type1 arg1,type2 arg2,type3 arg3,type4 arg4,type5 arg5,	\
 
 
 #define __NR_sys_uname __NR_uname
-#define __NR_sys_faccessat __NR_faccessat
-#define __NR_sys_fchmodat __NR_fchmodat
-#define __NR_sys_fchownat __NR_fchownat
-#define __NR_sys_fstatat64 __NR_fstatat64
-#define __NR_sys_futimesat __NR_futimesat
 #define __NR_sys_getcwd1 __NR_getcwd
 #define __NR_sys_getdents __NR_getdents
 #define __NR_sys_getdents64 __NR_getdents64
 #define __NR_sys_getpriority __NR_getpriority
-#define __NR_sys_linkat __NR_linkat
-#define __NR_sys_mkdirat __NR_mkdirat
-#define __NR_sys_mknodat __NR_mknodat
-#define __NR_sys_newfstatat __NR_newfstatat
-#define __NR_sys_openat __NR_openat
-#define __NR_sys_readlinkat __NR_readlinkat
-#define __NR_sys_renameat __NR_renameat
 #define __NR_sys_rt_sigqueueinfo __NR_rt_sigqueueinfo
-#define __NR_sys_symlinkat __NR_symlinkat
 #define __NR_sys_syslog __NR_syslog
 #define __NR_sys_tgkill __NR_tgkill
 #define __NR_sys_tkill __NR_tkill
-#define __NR_sys_unlinkat __NR_unlinkat
-#define __NR_sys_utimensat __NR_utimensat
 #define __NR_sys_futex __NR_futex
 #define __NR_sys_inotify_init __NR_inotify_init
 #define __NR_sys_inotify_add_watch __NR_inotify_add_watch
@@ -336,72 +321,6 @@ static int sys_getcwd1(char *buf, size_t size)
   return strlen(buf)+1;
 }
 
-#ifdef CONFIG_ATFILE
-/*
- * Host system seems to have atfile syscall stubs available.  We
- * now enable them one by one as specified by target syscall_nr.h.
- */
-
-#ifdef TARGET_NR_faccessat
-static int sys_faccessat(int dirfd, const char *pathname, int mode)
-{
-  return (faccessat(dirfd, pathname, mode, 0));
-}
-#endif
-#ifdef TARGET_NR_fchmodat
-static int sys_fchmodat(int dirfd, const char *pathname, mode_t mode)
-{
-  return (fchmodat(dirfd, pathname, mode, 0));
-}
-#endif
-#if defined(TARGET_NR_fchownat)
-static int sys_fchownat(int dirfd, const char *pathname, uid_t owner,
-    gid_t group, int flags)
-{
-  return (fchownat(dirfd, pathname, owner, group, flags));
-}
-#endif
-#ifdef __NR_fstatat64
-static int sys_fstatat64(int dirfd, const char *pathname, struct stat *buf,
-    int flags)
-{
-  return (fstatat(dirfd, pathname, buf, flags));
-}
-#endif
-#ifdef __NR_newfstatat
-static int sys_newfstatat(int dirfd, const char *pathname, struct stat *buf,
-    int flags)
-{
-  return (fstatat(dirfd, pathname, buf, flags));
-}
-#endif
-#ifdef TARGET_NR_futimesat
-static int sys_futimesat(int dirfd, const char *pathname,
-    const struct timeval times[2])
-{
-  return (futimesat(dirfd, pathname, times));
-}
-#endif
-#ifdef TARGET_NR_linkat
-static int sys_linkat(int olddirfd, const char *oldpath,
-    int newdirfd, const char *newpath, int flags)
-{
-  return (linkat(olddirfd, oldpath, newdirfd, newpath, flags));
-}
-#endif
-#ifdef TARGET_NR_mkdirat
-static int sys_mkdirat(int dirfd, const char *pathname, mode_t mode)
-{
-  return (mkdirat(dirfd, pathname, mode));
-}
-#endif
-#ifdef TARGET_NR_mknodat
-static int sys_mknodat(int dirfd, const char *pathname, mode_t mode,
-    dev_t dev)
-{
-  return (mknodat(dirfd, pathname, mode, dev));
-}
-#endif
 #ifdef TARGET_NR_openat
 static int sys_openat(int dirfd, const char *pathname, int flags, mode_t mode)
 {
@@ -415,91 +334,6 @@ static int sys_openat(int dirfd, const char *pathname, int flags, mode_t mode)
   return (openat(dirfd, pathname, flags));
 }
 #endif
-#ifdef TARGET_NR_readlinkat
-static int sys_readlinkat(int dirfd, const char *pathname, char *buf, size_t bufsiz)
-{
-  return (readlinkat(dirfd, pathname, buf, bufsiz));
-}
-#endif
-#ifdef TARGET_NR_renameat
-static int sys_renameat(int olddirfd, const char *oldpath,
-    int newdirfd, const char *newpath)
-{
-  return (renameat(olddirfd, oldpath, newdirfd, newpath));
-}
-#endif
-#ifdef TARGET_NR_symlinkat
-static int sys_symlinkat(const char *oldpath, int newdirfd, const char *newpath)
-{
-  return (symlinkat(oldpath, newdirfd, newpath));
-}
-#endif
-#ifdef TARGET_NR_unlinkat
-static int sys_unlinkat(int dirfd, const char *pathname, int flags)
-{
-  return (unlinkat(dirfd, pathname, flags));
-}
-#endif
-#else /* !CONFIG_ATFILE */
-
-/*
- * Try direct syscalls instead
- */
-#if defined(TARGET_NR_faccessat) && defined(__NR_faccessat)
-_syscall3(int,sys_faccessat,int,dirfd,const char *,pathname,int,mode)
-#endif
-#if defined(TARGET_NR_fchmodat) && defined(__NR_fchmodat)
-_syscall3(int,sys_fchmodat,int,dirfd,const char *,pathname, mode_t,mode)
-#endif
-#if defined(TARGET_NR_fchownat) && defined(__NR_fchownat)
-_syscall5(int,sys_fchownat,int,dirfd,const char *,pathname,
-          uid_t,owner,gid_t,group,int,flags)
-#endif
-#if (defined(TARGET_NR_fstatat64) || defined(TARGET_NR_newfstatat)) && \
-        defined(__NR_fstatat64)
-_syscall4(int,sys_fstatat64,int,dirfd,const char *,pathname,
-          struct stat *,buf,int,flags)
-#endif
-#if defined(TARGET_NR_futimesat) && defined(__NR_futimesat)
-_syscall3(int,sys_futimesat,int,dirfd,const char *,pathname,
-         const struct timeval *,times)
-#endif
-#if (defined(TARGET_NR_newfstatat) || defined(TARGET_NR_fstatat64) ) && \
-        defined(__NR_newfstatat)
-_syscall4(int,sys_newfstatat,int,dirfd,const char *,pathname,
-          struct stat *,buf,int,flags)
-#endif
-#if defined(TARGET_NR_linkat) && defined(__NR_linkat)
-_syscall5(int,sys_linkat,int,olddirfd,const char *,oldpath,
-      int,newdirfd,const char *,newpath,int,flags)
-#endif
-#if defined(TARGET_NR_mkdirat) && defined(__NR_mkdirat)
-_syscall3(int,sys_mkdirat,int,dirfd,const char *,pathname,mode_t,mode)
-#endif
-#if defined(TARGET_NR_mknodat) && defined(__NR_mknodat)
-_syscall4(int,sys_mknodat,int,dirfd,const char *,pathname,
-          mode_t,mode,dev_t,dev)
-#endif
-#if defined(TARGET_NR_openat) && defined(__NR_openat)
-_syscall4(int,sys_openat,int,dirfd,const char *,pathname,int,flags,mode_t,mode)
-#endif
-#if defined(TARGET_NR_readlinkat) && defined(__NR_readlinkat)
-_syscall4(int,sys_readlinkat,int,dirfd,const char *,pathname,
-          char *,buf,size_t,bufsize)
-#endif
-#if defined(TARGET_NR_renameat) && defined(__NR_renameat)
-_syscall4(int,sys_renameat,int,olddirfd,const char *,oldpath,
-          int,newdirfd,const char *,newpath)
-#endif
-#if defined(TARGET_NR_symlinkat) && defined(__NR_symlinkat)
-_syscall3(int,sys_symlinkat,const char *,oldpath,
-          int,newdirfd,const char *,newpath)
-#endif
-#if defined(TARGET_NR_unlinkat) && defined(__NR_unlinkat)
-_syscall3(int,sys_unlinkat,int,dirfd,const char *,pathname,int,flags)
-#endif
-
-#endif /* CONFIG_ATFILE */
 
 #ifdef CONFIG_UTIMENSAT
 static int sys_utimensat(int dirfd, const char *pathname,
@@ -5348,7 +5182,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             unlock_user(p, arg1, 0);
         }
         break;
-#if defined(TARGET_NR_linkat) && defined(__NR_linkat)
+#if defined(TARGET_NR_linkat)
     case TARGET_NR_linkat:
         {
             void * p2 = NULL;
@@ -5359,7 +5193,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             if (!p || !p2)
                 ret = -TARGET_EFAULT;
             else
-                ret = get_errno(sys_linkat(arg1, p, arg3, p2, arg5));
+                ret = get_errno(linkat(arg1, p, arg3, p2, arg5));
             unlock_user(p, arg2, 0);
             unlock_user(p2, arg4, 0);
         }
@@ -5371,11 +5205,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         ret = get_errno(unlink(p));
         unlock_user(p, arg1, 0);
         break;
-#if defined(TARGET_NR_unlinkat) && defined(__NR_unlinkat)
+#if defined(TARGET_NR_unlinkat)
     case TARGET_NR_unlinkat:
         if (!(p = lock_user_string(arg2)))
             goto efault;
-        ret = get_errno(sys_unlinkat(arg1, p, arg3));
+        ret = get_errno(unlinkat(arg1, p, arg3));
         unlock_user(p, arg2, 0);
         break;
 #endif
@@ -5493,11 +5327,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         ret = get_errno(mknod(p, arg2, arg3));
         unlock_user(p, arg1, 0);
         break;
-#if defined(TARGET_NR_mknodat) && defined(__NR_mknodat)
+#if defined(TARGET_NR_mknodat)
     case TARGET_NR_mknodat:
         if (!(p = lock_user_string(arg2)))
             goto efault;
-        ret = get_errno(sys_mknodat(arg1, p, arg3, arg4));
+        ret = get_errno(mknodat(arg1, p, arg3, arg4));
         unlock_user(p, arg2, 0);
         break;
 #endif
@@ -5628,7 +5462,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             unlock_user(p, arg1, 0);
         }
         break;
-#if defined(TARGET_NR_futimesat) && defined(__NR_futimesat)
+#if defined(TARGET_NR_futimesat)
     case TARGET_NR_futimesat:
         {
             struct timeval *tvp, tv[2];
@@ -5643,7 +5477,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             }
             if (!(p = lock_user_string(arg2)))
                 goto efault;
-            ret = get_errno(sys_futimesat(arg1, path(p), tvp));
+            ret = get_errno(futimesat(arg1, path(p), tvp));
             unlock_user(p, arg2, 0);
         }
         break;
@@ -5666,7 +5500,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
     case TARGET_NR_faccessat:
         if (!(p = lock_user_string(arg2)))
             goto efault;
-        ret = get_errno(sys_faccessat(arg1, p, arg3));
+        ret = get_errno(faccessat(arg1, p, arg3, 0));
         unlock_user(p, arg2, 0);
         break;
 #endif
@@ -5699,7 +5533,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             unlock_user(p, arg1, 0);
         }
         break;
-#if defined(TARGET_NR_renameat) && defined(__NR_renameat)
+#if defined(TARGET_NR_renameat)
     case TARGET_NR_renameat:
         {
             void *p2;
@@ -5708,7 +5542,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             if (!p || !p2)
                 ret = -TARGET_EFAULT;
             else
-                ret = get_errno(sys_renameat(arg1, p, arg3, p2));
+                ret = get_errno(renameat(arg1, p, arg3, p2));
             unlock_user(p2, arg4, 0);
             unlock_user(p, arg2, 0);
         }
@@ -5720,11 +5554,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         ret = get_errno(mkdir(p, arg2));
         unlock_user(p, arg1, 0);
         break;
-#if defined(TARGET_NR_mkdirat) && defined(__NR_mkdirat)
+#if defined(TARGET_NR_mkdirat)
     case TARGET_NR_mkdirat:
         if (!(p = lock_user_string(arg2)))
             goto efault;
-        ret = get_errno(sys_mkdirat(arg1, p, arg3));
+        ret = get_errno(mkdirat(arg1, p, arg3));
         unlock_user(p, arg2, 0);
         break;
 #endif
@@ -6410,7 +6244,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             unlock_user(p, arg1, 0);
         }
         break;
-#if defined(TARGET_NR_symlinkat) && defined(__NR_symlinkat)
+#if defined(TARGET_NR_symlinkat)
     case TARGET_NR_symlinkat:
         {
             void *p2;
@@ -6419,7 +6253,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             if (!p || !p2)
                 ret = -TARGET_EFAULT;
             else
-                ret = get_errno(sys_symlinkat(p, arg2, p2));
+                ret = get_errno(symlinkat(p, arg2, p2));
             unlock_user(p2, arg3, 0);
             unlock_user(p, arg1, 0);
         }
@@ -6450,7 +6284,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             unlock_user(p, arg1, 0);
         }
         break;
-#if defined(TARGET_NR_readlinkat) && defined(__NR_readlinkat)
+#if defined(TARGET_NR_readlinkat)
     case TARGET_NR_readlinkat:
         {
             void *p2;
@@ -6459,7 +6293,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             if (!p || !p2)
         	ret = -TARGET_EFAULT;
             else
-                ret = get_errno(sys_readlinkat(arg1, path(p), p2, arg4));
+                ret = get_errno(readlinkat(arg1, path(p), p2, arg4));
             unlock_user(p2, arg3, ret);
             unlock_user(p, arg2, 0);
         }
@@ -6594,11 +6428,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
     case TARGET_NR_fchmod:
         ret = get_errno(fchmod(arg1, arg2));
         break;
-#if defined(TARGET_NR_fchmodat) && defined(__NR_fchmodat)
+#if defined(TARGET_NR_fchmodat)
     case TARGET_NR_fchmodat:
         if (!(p = lock_user_string(arg2)))
             goto efault;
-        ret = get_errno(sys_fchmodat(arg1, p, arg3));
+        ret = get_errno(fchmodat(arg1, p, arg3, 0));
         unlock_user(p, arg2, 0);
         break;
 #endif
@@ -7686,8 +7520,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             ret = host_to_target_stat64(cpu_env, arg2, &st);
         break;
 #endif
-#if (defined(TARGET_NR_fstatat64) || defined(TARGET_NR_newfstatat)) && \
-        (defined(__NR_fstatat64) || defined(__NR_newfstatat))
+#if (defined(TARGET_NR_fstatat64) || defined(TARGET_NR_newfstatat))
 #ifdef TARGET_NR_fstatat64
     case TARGET_NR_fstatat64:
 #endif
@@ -7696,11 +7529,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
         if (!(p = lock_user_string(arg2)))
             goto efault;
-#ifdef __NR_fstatat64
-        ret = get_errno(sys_fstatat64(arg1, path(p), &st, arg4));
-#else
-        ret = get_errno(sys_newfstatat(arg1, path(p), &st, arg4));
-#endif
+        ret = get_errno(fstatat(arg1, path(p), &st, arg4));
         if (!is_error(ret))
             ret = host_to_target_stat64(cpu_env, arg3, &st);
         break;
@@ -7782,11 +7611,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
     case TARGET_NR_fchown:
         ret = get_errno(fchown(arg1, low2highuid(arg2), low2highgid(arg3)));
         break;
-#if defined(TARGET_NR_fchownat) && defined(__NR_fchownat)
+#if defined(TARGET_NR_fchownat)
     case TARGET_NR_fchownat:
         if (!(p = lock_user_string(arg2))) 
             goto efault;
-        ret = get_errno(sys_fchownat(arg1, p, low2highuid(arg3), low2highgid(arg4), arg5));
+        ret = get_errno(fchownat(arg1, p, low2highuid(arg3),
+                                 low2highgid(arg4), arg5));
         unlock_user(p, arg2, 0);
         break;
 #endif
-- 
1.7.9.5

[Qemu-devel] [PATCH 2/2] configure: Drop CONFIG_ATFILE test

[Peter Maydell]

Nobody uses the CONFIG_ATFILE test now, so just drop it.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure |   26 --------------------------
 1 file changed, 26 deletions(-)

diff --git a/configure b/configure
index 918dc36..034e3c3 100755
--- a/configure
+++ b/configure
@@ -2595,29 +2595,6 @@ EOF
   fi
 fi
 
-#
-# Check for xxxat() functions when we are building linux-user
-# emulator.  This is done because older glibc versions don't
-# have syscall stubs for these implemented.
-#
-atfile=no
-cat > $TMPC << EOF
-#define _ATFILE_SOURCE
-#include <sys/types.h>
-#include <fcntl.h>
-#include <unistd.h>
-
-int
-main(void)
-{
-	/* try to unlink nonexisting file */
-	return (unlinkat(AT_FDCWD, "nonexistent_file", 0));
-}
-EOF
-if compile_prog "" "" ; then
-  atfile=yes
-fi
-
 # Check for inotify functions when we are building linux-user
 # emulator.  This is done because older glibc versions don't
 # have syscall stubs for these implemented.  In that case we
@@ -3758,9 +3735,6 @@ fi
 if test "$curses" = "yes" ; then
   echo "CONFIG_CURSES=y" >> $config_host_mak
 fi
-if test "$atfile" = "yes" ; then
-  echo "CONFIG_ATFILE=y" >> $config_host_mak
-fi
 if test "$utimens" = "yes" ; then
   echo "CONFIG_UTIMENSAT=y" >> $config_host_mak
 fi
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 0/2] linux-user: Drop direct use of openat etc syscalls

[Richard Henderson]

On 06/01/2013 03:35 PM, Peter Maydell wrote:
> Basically, removing the ugly direct syscall access seemed nicer
> than trying to fix up and render consistent the broken ifdefs :-)
> 
> [RHEL5 has glibc2.5 and provides these functions. RHEL4 did not
> but we don't build on RHEL4 anyhow because its glib is too old.
> uClibc provides these functions.]
> 
> Peter Maydell (2):
>   linux-user: Drop direct use of openat etc syscalls
>   configure: Drop CONFIG_ATFILE test

Reviewed-by: Richard Henderson <rth@twiddle.net>

r~

Re: [Qemu-devel] [PATCH 0/2] linux-user: Drop direct use of openat etc syscalls

[Claudio Fontana]

On 02.06.2013 00:35, Peter Maydell wrote:
> The linux-user syscall emulation layer currently supports the openat
> family of syscalls via two mechanisms: simply calling the corresponding
> libc functions, and making direct syscalls. Since glibc has supported
> these functions since at least glibc 2.5, there's no real need to
> retain the (essentially untested) direct syscall fallback code, so
> this patchset simply deletes it.
> 
> This allows us to remove some ifdeffery that was attempting to disable
> provision of some of the syscalls if the host didn't seem to support
> them, which in some cases was actually wrong. For example where there
> are several flavours of the syscall, we only need one of them, not
> necessarily the exact one the guest has, as with the fstatat* calls.
> And if the guest needs the futimesat() syscall we can provide it
> via glibc, even if that syscall is deprecated or not provided in the
> host (because the host implements utimensat instead). AArch64 in
> particular hits the last of these, which resulted in a compile
> failure due to an unused function, because the syscall implementation's
> ifdef was inconsistent with the ifdef used to define the sys_futimesat()
> function.
> 
> Basically, removing the ugly direct syscall access seemed nicer
> than trying to fix up and render consistent the broken ifdefs :-)
> 
> [RHEL5 has glibc2.5 and provides these functions. RHEL4 did not
> but we don't build on RHEL4 anyhow because its glib is too old.
> uClibc provides these functions.]
> 
> Peter Maydell (2):
>   linux-user: Drop direct use of openat etc syscalls
>   configure: Drop CONFIG_ATFILE test
> 
>  configure            |   26 ------
>  linux-user/syscall.c |  218 ++++++--------------------------------------------
>  2 files changed, 24 insertions(+), 220 deletions(-)
> 

Tested on aarch64 with Foundation v8.

Tested-by: Claudio Fontana <claudio.fontana@huawei.com>