I appreciate the work that Jocelyn did to correct the types used
throughout linux-user/syscall.c. Along those same lines I am working on
several patches to eliminate some incorrect constructs that have crept
into syscall.c - some of which I have ignorantly propagated in previous
patches that I have submitted.
I have noticed that many functions in syscall.c return a *host* errno
when a *target* errno should be return. At the same time, there are
several places in syscall.c:do_syscall() that immediately return an
errno rather than setting the return value and exiting through the
syscall return value reporting at the end of do_syscall().
This patch addresses both of those problems at once rather than touching
the exact same errno return lines twice in do_syscall(). It also
touches a few functions in linux-user/signal.c that are called from
do_syscall().
Please send comments - I have several more patches that will build on
this one as well as a few more patches that will fix other incorrect
constructs with target/host address handling.
Thanks.
Index: qemu/linux-user/syscall.c
===================================================================
--- qemu.orig/linux-user/syscall.c 2007-10-10 17:00:00.000000000 -0600
+++ qemu/linux-user/syscall.c 2007-10-10 17:09:34.000000000 -0600
@@ -166,6 +166,8 @@
#ifdef __NR_gettid
_syscall0(int, gettid)
#else
+/* This is a replacement for the host gettid() and must return a host
+ * errno. */
static int gettid(void) {
return -ENOSYS;
}
@@ -387,6 +389,7 @@
target_original_brk = target_brk = HOST_PAGE_ALIGN(new_brk);
}
+/* do_brk() must return target values and target errnos. */
target_long do_brk(target_ulong new_brk)
{
target_ulong brk_page;
@@ -396,7 +399,7 @@
if (!new_brk)
return target_brk;
if (new_brk < target_original_brk)
- return -ENOMEM;
+ return -TARGET_ENOMEM;
brk_page = HOST_PAGE_ALIGN(target_brk);
@@ -530,6 +533,7 @@
}
+/* do_select() must return target values and target errnos. */
static target_long do_select(int n,
target_ulong rfd_p, target_ulong wfd_p,
target_ulong efd_p, target_ulong target_tv)
@@ -704,6 +708,7 @@
msgh->msg_controllen = tswapl(space);
}
+/* do_setsockopt() Must return target values and target errnos. */
static target_long do_setsockopt(int sockfd, int level, int optname,
target_ulong optval, socklen_t optlen)
{
@@ -714,7 +719,7 @@
case SOL_TCP:
/* TCP options all take an 'int' value. */
if (optlen < sizeof(uint32_t))
- return -EINVAL;
+ return -TARGET_EINVAL;
val = tget32(optval);
ret = get_errno(setsockopt(sockfd, level, optname, &val, sizeof(val)));
@@ -812,7 +817,7 @@
goto unimplemented;
}
if (optlen < sizeof(uint32_t))
- return -EINVAL;
+ return -TARGET_EINVAL;
val = tget32(optval);
ret = get_errno(setsockopt(sockfd, SOL_SOCKET, optname, &val, sizeof(val)));
@@ -820,11 +825,12 @@
default:
unimplemented:
gemu_log("Unsupported setsockopt level=%d optname=%d \n", level, optname);
- ret = -ENOSYS;
+ ret = -TARGET_ENOSYS;
}
return ret;
}
+/* do_getsockopt() Must return target values and target errnos. */
static target_long do_getsockopt(int sockfd, int level, int optname,
target_ulong optval, target_ulong optlen)
{
@@ -851,7 +857,7 @@
int_case:
len = tget32(optlen);
if (len < 0)
- return -EINVAL;
+ return -TARGET_EINVAL;
lv = sizeof(int);
ret = get_errno(getsockopt(sockfd, level, optname, &val, &lv));
if (ret < 0)
@@ -884,7 +890,7 @@
case IP_MULTICAST_LOOP:
len = tget32(optlen);
if (len < 0)
- return -EINVAL;
+ return -TARGET_EINVAL;
lv = sizeof(int);
ret = get_errno(getsockopt(sockfd, level, optname, &val, &lv));
if (ret < 0)
@@ -908,7 +914,7 @@
unimplemented:
gemu_log("getsockopt level=%d optname=%d not yet supported\n",
level, optname);
- ret = -ENOSYS;
+ ret = -TARGET_ENOSYS;
break;
}
return ret;
@@ -945,6 +951,7 @@
unlock_user (target_vec, target_addr, 0);
}
+/* do_socket() Must return target values and target errnos. */
static target_long do_socket(int domain, int type, int protocol)
{
#if defined(TARGET_MIPS)
@@ -972,6 +979,7 @@
return get_errno(socket(domain, type, protocol));
}
+/* do_bind() Must return target values and target errnos. */
static target_long do_bind(int sockfd, target_ulong target_addr,
socklen_t addrlen)
{
@@ -981,6 +989,7 @@
return get_errno(bind(sockfd, addr, addrlen));
}
+/* do_connect() Must return target values and target errnos. */
static target_long do_connect(int sockfd, target_ulong target_addr,
socklen_t addrlen)
{
@@ -990,6 +999,7 @@
return get_errno(connect(sockfd, addr, addrlen));
}
+/* do_sendrecvmsg() Must return target values and target errnos. */
static target_long do_sendrecvmsg(int fd, target_ulong target_msg,
int flags, int send)
{
@@ -1033,6 +1043,7 @@
return ret;
}
+/* do_accept() Must return target values and target errnos. */
static target_long do_accept(int fd, target_ulong target_addr,
target_ulong target_addrlen)
{
@@ -1048,6 +1059,7 @@
return ret;
}
+/* do_getpeername() Must return target values and target errnos. */
static target_long do_getpeername(int fd, target_ulong target_addr,
target_ulong target_addrlen)
{
@@ -1063,6 +1075,7 @@
return ret;
}
+/* do_getsockname() Must return target values and target errnos. */
static target_long do_getsockname(int fd, target_ulong target_addr,
target_ulong target_addrlen)
{
@@ -1078,6 +1091,7 @@
return ret;
}
+/* do_socketpair() Must return target values and target errnos. */
static target_long do_socketpair(int domain, int type, int protocol,
target_ulong target_tab)
{
@@ -1092,6 +1106,7 @@
return ret;
}
+/* do_sendto() Must return target values and target errnos. */
static target_long do_sendto(int fd, target_ulong msg, size_t len, int flags,
target_ulong target_addr, socklen_t addrlen)
{
@@ -1111,6 +1126,7 @@
return ret;
}
+/* do_recvfrom() Must return target values and target errnos. */
static target_long do_recvfrom(int fd, target_ulong msg, size_t len, int flags,
target_ulong target_addr,
target_ulong target_addrlen)
@@ -1142,6 +1158,7 @@
}
#ifdef TARGET_NR_socketcall
+/* do_socketcall() Must return target values and target errnos. */
static target_long do_socketcall(int num, target_ulong vptr)
{
target_long ret;
@@ -1299,7 +1316,7 @@
break;
default:
gemu_log("Unsupported socketcall: %d\n", num);
- ret = -ENOSYS;
+ ret = -TARGET_ENOSYS;
break;
}
return ret;
@@ -1637,6 +1654,7 @@
}
/* ??? This only works with linear mappings. */
+/* do_ipc() must return target values and target errnos. */
static target_long do_ipc(unsigned int call, int first,
int second, int third,
target_long ptr, target_long fifth)
@@ -1665,7 +1683,7 @@
case IPCOP_semtimedop:
gemu_log("Unsupported ipc call: %d (version %d)\n", call, version);
- ret = -ENOSYS;
+ ret = -TARGET_ENOSYS;
break;
case IPCOP_msgget:
@@ -1721,7 +1739,7 @@
}
}
if (put_user(raddr, (target_ulong *)third))
- return -EFAULT;
+ return -TARGET_EFAULT;
ret = 0;
break;
case IPCOP_shmdt:
@@ -1755,7 +1773,7 @@
default:
unimplemented:
gemu_log("Unsupported ipc call: %d (version %d)\n", call, version);
- ret = -ENOSYS;
+ ret = -TARGET_ENOSYS;
break;
}
return ret;
@@ -1801,6 +1819,7 @@
};
/* ??? Implement proper locking for ioctls. */
+/* do_ioctl() Must return target values and target errnos. */
static target_long do_ioctl(int fd, target_long cmd, target_long arg)
{
const IOCTLEntry *ie;
@@ -1814,7 +1833,7 @@
for(;;) {
if (ie->target_cmd == 0) {
gemu_log("Unsupported ioctl: cmd=0x%04lx\n", (long)cmd);
- return -ENOSYS;
+ return -TARGET_ENOSYS;
}
if (ie->target_cmd == cmd)
break;
@@ -1869,7 +1888,7 @@
default:
gemu_log("Unsupported ioctl type: cmd=0x%04lx type=%d\n",
(long)cmd, arg_type[0]);
- ret = -ENOSYS;
+ ret = -TARGET_ENOSYS;
break;
}
return ret;
@@ -2104,6 +2123,7 @@
}
/* XXX: add locking support */
+/* write_ldt() returns host errnos */
static int write_ldt(CPUX86State *env,
target_ulong ptr, unsigned long bytecount, int oldmode)
{
@@ -2186,6 +2206,8 @@
}
/* specific and weird i386 syscalls */
+/* do_modify_ldt() returns host errnos (it is inconsistent with the
+ * other do_*() functions which return target errnos). */
int do_modify_ldt(CPUX86State *env, int func, target_ulong ptr, unsigned long bytecount)
{
int ret = -ENOSYS;
@@ -2218,6 +2240,9 @@
return 0;
}
+/* do_fork() Must return host values and target errnos (unlike most
+ * do_*() functions).
+ */
int do_fork(CPUState *env, unsigned int flags, target_ulong newsp)
{
int ret;
@@ -2541,6 +2566,15 @@
unlock_user_struct(target_ts, target_addr, 1);
}
+/* return_err() is a do_syscall() helper for setting the return code
+ * and jumping to the end of do_syscall().
+ */
+#define return_err(err) do { ret = - err; goto fail; } while(0)
+
+/* do_syscall() should always have a single exit point at the end so
+ * that actions, such as logging of syscall results, can be performed.
+ * All errnos that do_syscall() returns must be -TARGET_<errcode>
+ */
target_long do_syscall(void *cpu_env, int num, target_long arg1,
target_long arg2, target_long arg3, target_long arg4,
target_long arg5, target_long arg6)
@@ -2583,13 +2617,12 @@
break;
#if defined(TARGET_NR_openat) && defined(__NR_openat)
case TARGET_NR_openat:
- if (!arg2) {
- ret = -EFAULT;
- goto fail;
- }
+ if (!arg2)
+ return_err(TARGET_EFAULT);
p = lock_user_string(arg2);
if (!access_ok(VERIFY_READ, p, 1))
- ret = -EFAULT;
+ /* Don't fail immediately so that cleanup can happen. */
+ ret = -TARGET_EFAULT;
else
ret = get_errno(sys_openat(arg1,
path(p),
@@ -2637,17 +2670,16 @@
break;
#if defined(TARGET_NR_linkat) && defined(__NR_linkat)
case TARGET_NR_linkat:
- if (!arg2 || !arg4) {
- ret = -EFAULT;
- goto fail;
- }
+ if (!arg2 || !arg4)
+ return_err(TARGET_EFAULT);
{
void * p2 = NULL;
p = lock_user_string(arg2);
p2 = lock_user_string(arg4);
if (!access_ok(VERIFY_READ, p, 1)
|| !access_ok(VERIFY_READ, p2, 1))
- ret = -EFAULT;
+ /* Don't fail immediately so that cleanup can happen. */
+ ret = -TARGET_EFAULT;
else
ret = get_errno(sys_linkat(arg1, p, arg3, p2, arg5));
if (p2)
@@ -2664,13 +2696,12 @@
break;
#if defined(TARGET_NR_unlinkat) && defined(__NR_unlinkat)
case TARGET_NR_unlinkat:
- if (!arg2) {
- ret = -EFAULT;
- goto fail;
- }
+ if (!arg2)
+ return_err(TARGET_EFAULT);
p = lock_user_string(arg2);
if (!access_ok(VERIFY_READ, p, 1))
- ret = -EFAULT;
+ /* Don't fail immediately so that cleanup can happen. */
+ ret = -TARGET_EFAULT;
else
ret = get_errno(sys_unlinkat(arg1, p, arg3));
if (p)
@@ -2755,13 +2786,12 @@
break;
#if defined(TARGET_NR_mknodat) && defined(__NR_mknodat)
case TARGET_NR_mknodat:
- if (!arg2) {
- ret = -EFAULT;
- goto fail;
- }
+ if (!arg2)
+ return_err(TARGET_EFAULT);
p = lock_user_string(arg2);
if (!access_ok(VERIFY_READ, p, 1))
- ret = -EFAULT;
+ /* Don't fail immediately so that cleanup can happen. */
+ ret = -TARGET_EFAULT;
else
ret = get_errno(sys_mknodat(arg1, p, arg3, arg4));
if (p)
@@ -2887,13 +2917,12 @@
break;
#if defined(TARGET_NR_faccessat) && defined(__NR_faccessat)
case TARGET_NR_faccessat:
- if (!arg2) {
- ret = -EFAULT;
- goto fail;
- }
+ if (!arg2)
+ return_err(TARGET_EFAULT);
p = lock_user_string(arg2);
if (!access_ok(VERIFY_READ, p, 1))
- ret = -EFAULT;
+ /* Don't fail immediately so that cleanup can happen. */
+ ret = -TARGET_EFAULT;
else
ret = get_errno(sys_faccessat(arg1, p, arg3, arg4));
if (p)
@@ -2928,17 +2957,18 @@
break;
#if defined(TARGET_NR_renameat) && defined(__NR_renameat)
case TARGET_NR_renameat:
- if (!arg2 || !arg4) {
- ret = -EFAULT;
- goto fail;
- }
{
void *p2 = NULL;
+
+ if (!arg2 || !arg4)
+ return_err(TARGET_EFAULT);
+
p = lock_user_string(arg2);
p2 = lock_user_string(arg4);
if (!access_ok(VERIFY_READ, p, 1)
|| !access_ok(VERIFY_READ, p2, 1))
- ret = -EFAULT;
+ /* Don't fail immediately so that cleanup can happen. */
+ ret = -TARGET_EFAULT;
else
ret = get_errno(sys_renameat(arg1, p, arg3, p2));
if (p2)
@@ -2955,13 +2985,12 @@
break;
#if defined(TARGET_NR_mkdirat) && defined(__NR_mkdirat)
case TARGET_NR_mkdirat:
- if (!arg2) {
- ret = -EFAULT;
- goto fail;
- }
+ if (!arg2)
+ return_err(TARGET_EFAULT);
p = lock_user_string(arg2);
if (!access_ok(VERIFY_READ, p, 1))
- ret = -EFAULT;
+ /* Don't fail immediately so that cleanup can happen. */
+ ret = -TARGET_EFAULT;
else
ret = get_errno(sys_mkdirat(arg1, p, arg3));
if (p)
@@ -3196,8 +3225,7 @@
how = SIG_SETMASK;
break;
default:
- ret = -EINVAL;
- goto fail;
+ return_err(TARGET_EINVAL);
}
p = lock_user(arg2, sizeof(target_sigset_t), 1);
target_to_host_old_sigset(&set, p);
@@ -3233,8 +3261,7 @@
how = SIG_SETMASK;
break;
default:
- ret = -EINVAL;
- goto fail;
+ return_err(TARGET_EINVAL);
}
p = lock_user(arg2, sizeof(target_sigset_t), 1);
target_to_host_sigset(&set, p);
@@ -3427,17 +3454,18 @@
break;
#if defined(TARGET_NR_symlinkat) && defined(__NR_symlinkat)
case TARGET_NR_symlinkat:
- if (!arg1 || !arg3) {
- ret = -EFAULT;
- goto fail;
- }
{
void *p2 = NULL;
+
+ if (!arg1 || !arg3)
+ return_err(TARGET_EFAULT);
+
p = lock_user_string(arg1);
p2 = lock_user_string(arg3);
if (!access_ok(VERIFY_READ, p, 1)
|| !access_ok(VERIFY_READ, p2, 1))
- ret = -EFAULT;
+ /* Don't fail immediately so that cleanup can happen. */
+ return_err(TARGET_EFAULT);
else
ret = get_errno(sys_symlinkat(p, arg2, p2));
if (p2)
@@ -3463,17 +3491,18 @@
break;
#if defined(TARGET_NR_readlinkat) && defined(__NR_readlinkat)
case TARGET_NR_readlinkat:
- if (!arg2 || !arg3) {
- ret = -EFAULT;
- goto fail;
- }
{
void *p2 = NULL;
+
+ if (!arg2 || !arg3)
+ return_err(TARGET_EFAULT);
+
p = lock_user_string(arg2);
p2 = lock_user(arg3, arg4, 0);
if (!access_ok(VERIFY_READ, p, 1)
|| !access_ok(VERIFY_READ, p2, 1))
- ret = -EFAULT;
+ /* Don't fail immediately so that cleanup can happen. */
+ ret = -TARGET_EFAULT;
else
ret = get_errno(sys_readlinkat(arg1, path(p), p2, arg4));
if (p2)
@@ -3589,13 +3618,12 @@
break;
#if defined(TARGET_NR_fchmodat) && defined(__NR_fchmodat)
case TARGET_NR_fchmodat:
- if (!arg2) {
- ret = -EFAULT;
- goto fail;
- }
+ if (!arg2)
+ return_err(TARGET_EFAULT);
p = lock_user_string(arg2);
if (!access_ok(VERIFY_READ, p, 1))
- ret = -EFAULT;
+ /* Don't fail immediately so that cleanup can happen. */
+ ret = -TARGET_EFAULT;
else
ret = get_errno(sys_fchmodat(arg1, p, arg3, arg4));
if (p)
@@ -4047,7 +4075,7 @@
dirp = malloc(count);
if (!dirp)
- return -ENOMEM;
+ return_err(TARGET_ENOMEM);
ret = get_errno(sys_getdents(arg1, dirp, count));
if (!is_error(ret)) {
@@ -4204,9 +4232,9 @@
break;
#endif
case TARGET_NR__sysctl:
- /* We don't implement this, but ENODIR is always a safe
+ /* We don't implement this, but ENOTDIR is always a safe
return value. */
- return -ENOTDIR;
+ return_err(TARGET_ENOTDIR);
case TARGET_NR_sched_setparam:
{
struct sched_param *target_schp;
@@ -4320,9 +4348,9 @@
case TARGET_NR_sigaltstack:
#if defined(TARGET_I386) || defined(TARGET_ARM) || defined(TARGET_MIPS) || \
defined(TARGET_SPARC) || defined(TARGET_PPC) || defined(TARGET_ALPHA)
- ret = do_sigaltstack((struct target_sigaltstack *)arg1,
- (struct target_sigaltstack *)arg2,
- get_sp_from_cpustate((CPUState *)cpu_env));
+ ret = get_errno(do_sigaltstack((struct target_sigaltstack *)arg1,
+ (struct target_sigaltstack *)arg2,
+ get_sp_from_cpustate((CPUState *)cpu_env)));
break;
#else
goto unimplemented;
@@ -4504,13 +4532,12 @@
break;
#if defined(TARGET_NR_fchownat) && defined(__NR_fchownat)
case TARGET_NR_fchownat:
- if (!arg2) {
- ret = -EFAULT;
- goto fail;
- }
+ if (!arg2)
+ return_err(TARGET_EFAULT);
p = lock_user_string(arg2);
if (!access_ok(VERIFY_READ, p, 1))
- ret = -EFAULT;
+ /* Don't fail immediately so that cleanup can happen. */
+ ret = -TARGET_EFAULT;
else
ret = get_errno(sys_fchownat(arg1, p, low2highuid(arg3), low2highgid(arg4), arg5));
if (p)
@@ -4949,7 +4976,8 @@
else {
p = lock_user_string(arg2);
if (!access_ok(VERIFY_READ, p, 1))
- ret = -EFAULT;
+ /* Don't fail immediately so that cleanup can happen. */
+ ret = -TARGET_EFAULT;
else
ret = get_errno(sys_utimensat(arg1, path(p), ts, arg4));
if (p)
@@ -4965,7 +4993,7 @@
#if defined(TARGET_NR_setxattr) || defined(TARGET_NR_get_thread_area) || defined(TARGET_NR_getdomainname) || defined(TARGET_NR_set_robust_list)
unimplemented_nowarn:
#endif
- ret = -ENOSYS;
+ ret = -TARGET_ENOSYS;
break;
}
fail:
Index: qemu/linux-user/signal.c
===================================================================
--- qemu.orig/linux-user/signal.c 2007-10-10 17:00:00.000000000 -0600
+++ qemu/linux-user/signal.c 2007-10-10 17:05:01.000000000 -0600
@@ -438,6 +438,7 @@
}
}
+/* do_sigaltstack() returns host values and errnos. */
int do_sigaltstack(const struct target_sigaltstack *uss,
struct target_sigaltstack *uoss,
target_ulong sp)
@@ -499,12 +500,14 @@
return ret;
}
+/* do_sigaction() return host values and errnos */
int do_sigaction(int sig, const struct target_sigaction *act,
struct target_sigaction *oact)
{
struct emulated_sigaction *k;
struct sigaction act1;
int host_sig;
+ int ret = 0;
if (sig < 1 || sig > TARGET_NSIG || sig == SIGKILL || sig == SIGSTOP)
return -EINVAL;
@@ -546,10 +549,10 @@
} else {
act1.sa_sigaction = host_signal_handler;
}
- sigaction(host_sig, &act1, NULL);
+ ret = sigaction(host_sig, &act1, NULL);
}
}
- return 0;
+ return ret;
}
#ifndef offsetof
