* [Fwd: [Qemu-devel] RFC: linux user problems]
@ 2007-09-18 23:28 J. Mayer
2007-09-19 9:07 ` Thiemo Seufer
0 siblings, 1 reply; 3+ messages in thread
From: J. Mayer @ 2007-09-18 23:28 UTC (permalink / raw)
To: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 4086 bytes --]
Following my previous message, I did a patch that makes syscalls take
target_long/target_ulong argument and return target_long value instead
of long/unsigned long.
I also included the #ifdef protection for do_socketcall and do_ipc to
avoid compilation warnings.
And I also converted the fd given to do_ioctl to be an int.
In addition to my previous remarks, I noticed some other things while
reading the code:
- the do_msgctl function seems very strange to me. It looks like half of
the code is missing in the switch...
- do_ipc directly uses pointers from the emulated environment without
using lock_user related functions like it seems to be done everywhere
else.
- there are at least two problems in IPCOP_shmat:
* the returned address could not fit in the target address space when
emulating a 32 bits
target on a 64 bits host
* the returned address is always casted into a 32 bits value. I changed
this to be target_ulong.
- I also noticed some suspicious warnings (cast between pointer and
integer of different size) that may hide other problems:
* target_to_host_cmsg:567
* host_to_target_cmsg:612
* do_ipc:1609
* do_ipc: 1621
* do_ipc: 1645
* do_ipc: 1655
* do_ipc: 1677 (multiple times)
* do_ipc: 1687
* do_ipc: 1711
* do_syscall:2686
* do_syscall: 3903
* do_syscall: 4671
May someone take a look at my patch and say if it seems reasonable to
include this in the repository ?
I did not notice any regression, launching 32 bits programs on a 64 bits
host, but I did not do any unitary tests to trigger specific bugs, just
launched command lines programs like bash, make, ... and used them. Then
other looks to the patch would be welcome !
-------- Forwarded Message --------
> From: J. Mayer <l_indien@magic.fr>
> Reply-To: qemu-devel@nongnu.org
> To: qemu-devel@nongnu.org
> Subject: [Qemu-devel] RFC: linux user problems
> Date: Mon, 17 Sep 2007 23:04:00 +0200
>
> It seems to me that there are many problems in linux-user/syscall.c
> - minor fixes, just to avoid compilation warnings:
> do_socketcall should be inside a #ifdef TARGET_NR_socketcall block
> do_ipc should be inside a #ifdef TARGET_NR_ipc block
> - problems for 64 bits targets:
> it seems that do_syscall and child functions should take target_long /
> target_ulong arguments instead of long / unsigned long. This would make
> a chance for 64 bits targets to be ran on 32 bits hosts (even if, yes,
> there would also be other problems to fix elsewhere...).
> - ipc specific problems:
> some structure used for IPC definitions have been merged. They used to
> be target specific and now are generic. But it seems to me that many
> mistakes have been done here, while comparing with the PowerPC 64 target
> definition, which has not been merged:
> struct target_ipc_perm {
> int __key;
> unsigned short uid;
> unsigned short gid;
> unsigned short cuid;
> unsigned short cgid;
> unsigned short mode;
> unsigned short seq;
> };
> in PowerPC 64 becomes:
> struct target_ipc_perm
> {
> target_long __key;
> target_ulong uid;
> target_ulong gid;
> target_ulong cuid;
> target_ulong cgid;
> unsigned short int mode;
> unsigned short int __pad1;
> unsigned short int __seq;
> unsigned short int __pad2;
> target_ulong __unused1;
> target_ulong __unused2;
> };
> in generic code.
> Problems are, imho:
> int is not the same size than target_long on 64 bits targets.
> unsigned short is never the same size than target_ulong (am I wrong ?)
> there should be a target_short definition: are we sure short on the host
> is always the same size than target_short ?
> I also don't understand the padding logic here (does the original
> target_ipc_perm structure relies on alignments generated by the
> compiler ?).
> I found the same kind of problems for the target_msqid_ds and
> target_msgbuf structure.
> I may be wrong, but it seems to me that those problems are not PowerPC
> 64 specific and that there are some serious bugs to be fixed here. Can
> someone confirm this or tell me what I missed ?
>
> Regards.
>
--
J. Mayer <l_indien@magic.fr>
Never organized
[-- Attachment #2: syscall.diff --]
[-- Type: text/x-patch, Size: 17794 bytes --]
Index: linux-user/qemu.h
===================================================================
RCS file: /sources/qemu/qemu/linux-user/qemu.h,v
retrieving revision 1.33
diff -u -d -d -p -r1.33 qemu.h
--- linux-user/qemu.h 16 Sep 2007 21:07:58 -0000 1.33
+++ linux-user/qemu.h 18 Sep 2007 22:46:51 -0000
@@ -126,10 +127,11 @@ int load_flt_binary(struct linux_binprm
void memcpy_to_target(target_ulong dest, const void *src,
unsigned long len);
void target_set_brk(target_ulong new_brk);
-long do_brk(target_ulong new_brk);
+target_long do_brk(target_ulong new_brk);
void syscall_init(void);
-long do_syscall(void *cpu_env, int num, long arg1, long arg2, long arg3,
- long arg4, long arg5, long arg6);
+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);
void gemu_log(const char *fmt, ...) __attribute__((format(printf,1,2)));
extern CPUState *global_env;
void cpu_loop(CPUState *env);
Index: linux-user/syscall.c
===================================================================
RCS file: /sources/qemu/qemu/linux-user/syscall.c,v
retrieving revision 1.121
diff -u -d -d -p -r1.121 syscall.c
--- linux-user/syscall.c 17 Sep 2007 08:09:50 -0000 1.121
+++ linux-user/syscall.c 18 Sep 2007 22:46:52 -0000
@@ -310,7 +310,7 @@ static inline int host_to_target_errno(i
return err;
}
-static inline long get_errno(long ret)
+static inline target_long get_errno(target_long ret)
{
if (ret == -1)
return -host_to_target_errno(errno);
@@ -318,9 +318,9 @@ static inline long get_errno(long ret)
return ret;
}
-static inline int is_error(long ret)
+static inline int is_error(target_long ret)
{
- return (unsigned long)ret >= (unsigned long)(-4096);
+ return (target_ulong)ret >= (target_ulong)(-4096);
}
static target_ulong target_brk;
@@ -331,10 +331,10 @@ void target_set_brk(target_ulong new_brk
target_original_brk = target_brk = HOST_PAGE_ALIGN(new_brk);
}
-long do_brk(target_ulong new_brk)
+target_long do_brk(target_ulong new_brk)
{
target_ulong brk_page;
- long mapped_addr;
+ target_long mapped_addr;
int new_alloc_size;
if (!new_brk)
@@ -415,7 +415,7 @@ static inline void host_to_target_fds(ta
#define HOST_HZ 100
#endif
-static inline long host_to_target_clock_t(long ticks)
+static inline target_long host_to_target_clock_t(long ticks)
{
#if HOST_HZ == TARGET_HZ
return ticks;
@@ -474,15 +474,15 @@ static inline void host_to_target_timeva
}
-static long do_select(long n,
- target_ulong rfd_p, target_ulong wfd_p,
- target_ulong efd_p, target_ulong target_tv)
+static target_long do_select(target_long n,
+ target_ulong rfd_p, target_ulong wfd_p,
+ target_ulong efd_p, target_ulong target_tv)
{
fd_set rfds, wfds, efds;
fd_set *rfds_ptr, *wfds_ptr, *efds_ptr;
target_long *target_rfds, *target_wfds, *target_efds;
struct timeval tv, *tv_ptr;
- long ret;
+ target_long ret;
int ok;
if (rfd_p) {
@@ -648,10 +648,11 @@ static inline void host_to_target_cmsg(s
msgh->msg_controllen = tswapl(space);
}
-static long do_setsockopt(int sockfd, int level, int optname,
- target_ulong optval, socklen_t optlen)
+static target_long do_setsockopt(int sockfd, int level, int optname,
+ target_ulong optval, socklen_t optlen)
{
- int val, ret;
+ target_long ret;
+ int val;
switch(level) {
case SOL_TCP:
@@ -768,10 +769,11 @@ static long do_setsockopt(int sockfd, in
return ret;
}
-static long do_getsockopt(int sockfd, int level, int optname,
- target_ulong optval, target_ulong optlen)
+static target_long do_getsockopt(int sockfd, int level, int optname,
+ target_ulong optval, target_ulong optlen)
{
- int len, lv, val, ret;
+ target_long ret;
+ int len, lv, val;
switch(level) {
case TARGET_SOL_SOCKET:
@@ -887,7 +889,7 @@ static void unlock_iovec(struct iovec *v
unlock_user (target_vec, target_addr, 0);
}
-static long do_socket(int domain, int type, int protocol)
+static target_long do_socket(int domain, int type, int protocol)
{
#if defined(TARGET_MIPS)
switch(type) {
@@ -914,8 +916,8 @@ static long do_socket(int domain, int ty
return get_errno(socket(domain, type, protocol));
}
-static long do_bind(int sockfd, target_ulong target_addr,
- socklen_t addrlen)
+static target_long do_bind(int sockfd, target_ulong target_addr,
+ socklen_t addrlen)
{
void *addr = alloca(addrlen);
@@ -923,8 +925,8 @@ static long do_bind(int sockfd, target_u
return get_errno(bind(sockfd, addr, addrlen));
}
-static long do_connect(int sockfd, target_ulong target_addr,
- socklen_t addrlen)
+static target_long do_connect(int sockfd, target_ulong target_addr,
+ socklen_t addrlen)
{
void *addr = alloca(addrlen);
@@ -932,10 +934,10 @@ static long do_connect(int sockfd, targe
return get_errno(connect(sockfd, addr, addrlen));
}
-static long do_sendrecvmsg(int fd, target_ulong target_msg,
- int flags, int send)
+static target_long do_sendrecvmsg(int fd, target_ulong target_msg,
+ int flags, int send)
{
- long ret;
+ target_long ret;
struct target_msghdr *msgp;
struct msghdr msg;
int count;
@@ -975,12 +977,12 @@ static long do_sendrecvmsg(int fd, targe
return ret;
}
-static long do_accept(int fd, target_ulong target_addr,
- target_ulong target_addrlen)
+static target_long do_accept(int fd, target_ulong target_addr,
+ target_ulong target_addrlen)
{
socklen_t addrlen = tget32(target_addrlen);
void *addr = alloca(addrlen);
- long ret;
+ target_long ret;
ret = get_errno(accept(fd, addr, &addrlen));
if (!is_error(ret)) {
@@ -990,12 +992,12 @@ static long do_accept(int fd, target_ulo
return ret;
}
-static long do_getpeername(int fd, target_ulong target_addr,
- target_ulong target_addrlen)
+static target_long do_getpeername(int fd, target_ulong target_addr,
+ target_ulong target_addrlen)
{
socklen_t addrlen = tget32(target_addrlen);
void *addr = alloca(addrlen);
- long ret;
+ target_long ret;
ret = get_errno(getpeername(fd, addr, &addrlen));
if (!is_error(ret)) {
@@ -1005,12 +1007,12 @@ static long do_getpeername(int fd, targe
return ret;
}
-static long do_getsockname(int fd, target_ulong target_addr,
- target_ulong target_addrlen)
+static target_long do_getsockname(int fd, target_ulong target_addr,
+ target_ulong target_addrlen)
{
socklen_t addrlen = tget32(target_addrlen);
void *addr = alloca(addrlen);
- long ret;
+ target_long ret;
ret = get_errno(getsockname(fd, addr, &addrlen));
if (!is_error(ret)) {
@@ -1020,11 +1022,11 @@ static long do_getsockname(int fd, targe
return ret;
}
-static long do_socketpair(int domain, int type, int protocol,
- target_ulong target_tab)
+static target_long do_socketpair(int domain, int type, int protocol,
+ target_ulong target_tab)
{
int tab[2];
- long ret;
+ target_long ret;
ret = get_errno(socketpair(domain, type, protocol, tab));
if (!is_error(ret)) {
@@ -1034,12 +1036,12 @@ static long do_socketpair(int domain, in
return ret;
}
-static long do_sendto(int fd, target_ulong msg, size_t len, int flags,
- target_ulong target_addr, socklen_t addrlen)
+static target_long do_sendto(int fd, target_ulong msg, size_t len, int flags,
+ target_ulong target_addr, socklen_t addrlen)
{
void *addr;
void *host_msg;
- long ret;
+ target_long ret;
host_msg = lock_user(msg, len, 1);
if (target_addr) {
@@ -1053,13 +1055,14 @@ static long do_sendto(int fd, target_ulo
return ret;
}
-static long do_recvfrom(int fd, target_ulong msg, size_t len, int flags,
- target_ulong target_addr, target_ulong target_addrlen)
+static target_long do_recvfrom(int fd, target_ulong msg, size_t len, int flags,
+ target_ulong target_addr,
+ target_ulong target_addrlen)
{
socklen_t addrlen;
void *addr;
void *host_msg;
- long ret;
+ target_long ret;
host_msg = lock_user(msg, len, 0);
if (target_addr) {
@@ -1082,9 +1085,10 @@ static long do_recvfrom(int fd, target_u
return ret;
}
-static long do_socketcall(int num, target_ulong vptr)
+#ifdef TARGET_NR_socketcall
+static target_long do_socketcall(int num, target_ulong vptr)
{
- long ret;
+ target_long ret;
const int n = sizeof(target_ulong);
switch(num) {
@@ -1244,6 +1248,7 @@ static long do_socketcall(int num, targe
}
return ret;
}
+#endif
#define N_SHM_REGIONS 32
@@ -1414,12 +1422,13 @@ static inline void host_to_target_semun(
}
}
-static inline long do_semctl(long first, long second, long third, long ptr)
+static inline target_long do_semctl(target_long first, target_long second,
+ target_long third, target_long ptr)
{
union semun arg;
struct semid_ds dsarg;
int cmd = third&0xff;
- long ret = 0;
+ target_long ret = 0;
switch( cmd ) {
case GETVAL:
@@ -1513,11 +1525,12 @@ static inline void host_to_target_msqid_
unlock_user_struct(target_md, target_addr, 1);
}
-static inline long do_msgctl(long first, long second, long ptr)
+static inline target_long do_msgctl(target_long first, target_long second,
+ target_long ptr)
{
struct msqid_ds dsarg;
int cmd = second&0xff;
- long ret = 0;
+ target_long ret = 0;
switch( cmd ) {
case IPC_STAT:
case IPC_SET:
@@ -1530,11 +1543,12 @@ static inline long do_msgctl(long first,
char mtext[1];
};
-static inline long do_msgsnd(long msqid, long msgp, long msgsz, long msgflg)
+static inline target_long do_msgsnd(target_long msqid, target_long msgp,
+ target_long msgsz, target_long msgflg)
{
struct target_msgbuf *target_mb;
struct msgbuf *host_mb;
- long ret = 0;
+ target_long ret = 0;
lock_user_struct(target_mb,msgp,0);
host_mb = malloc(msgsz+sizeof(long));
@@ -1552,11 +1569,13 @@ static inline long do_msgsnd(long msqid,
return ret;
}
-static inline long do_msgrcv(long msqid, long msgp, long msgsz, long msgtype, long msgflg)
+static inline target_long do_msgrcv(target_long msqid, target_long msgp,
+ target_long msgsz, target_long msgtype,
+ target_long msgflg)
{
struct target_msgbuf *target_mb;
struct msgbuf *host_mb;
- long ret = 0;
+ target_long ret = 0;
lock_user_struct(target_mb, msgp, 0);
host_mb = malloc(msgsz+sizeof(long));
@@ -1571,11 +1590,13 @@ static inline long do_msgrcv(long msqid,
}
/* ??? This only works with linear mappings. */
-static long do_ipc(long call, long first, long second, long third,
- long ptr, long fifth)
+#ifdef TARGET_NR_ipc
+static target_long do_ipc(target_long call, target_long first,
+ target_long second, target_long third,
+ target_long ptr, target_long fifth)
{
int version;
- long ret = 0;
+ target_long ret = 0;
unsigned long raddr;
struct shmid_ds shm_info;
int i;
@@ -1653,7 +1674,7 @@ static long do_ipc(long call, long first
break;
}
}
- if (put_user(raddr, (uint32_t *)third))
+ if (put_user(raddr, (target_ulong *)third))
return -EFAULT;
ret = 0;
break;
@@ -1687,12 +1708,14 @@ static long do_ipc(long call, long first
break;
default:
unimplemented:
- gemu_log("Unsupported ipc call: %ld (version %d)\n", call, version);
+ gemu_log("Unsupported ipc call: %ld (version %d)\n",
+ (long)call, version);
ret = -ENOSYS;
break;
}
return ret;
}
+#endif
/* kernel structure types definitions */
#define IFNAMSIZ 16
@@ -1733,11 +1756,11 @@ IOCTLEntry ioctl_entries[] = {
};
/* ??? Implement proper locking for ioctls. */
-static long do_ioctl(long fd, long cmd, long arg)
+static target_long do_ioctl(int fd, target_long cmd, target_long arg)
{
const IOCTLEntry *ie;
const argtype *arg_type;
- long ret;
+ target_long ret;
uint8_t buf_temp[MAX_STRUCT_SIZE];
int target_size;
void *argptr;
@@ -1745,7 +1768,7 @@ static long do_ioctl(long fd, long cmd,
ie = ioctl_entries;
for(;;) {
if (ie->target_cmd == 0) {
- gemu_log("Unsupported ioctl: cmd=0x%04lx\n", cmd);
+ gemu_log("Unsupported ioctl: cmd=0x%04lx\n", (long)cmd);
return -ENOSYS;
}
if (ie->target_cmd == cmd)
@@ -1754,7 +1777,7 @@ static long do_ioctl(long fd, long cmd,
}
arg_type = ie->arg_type;
#if defined(DEBUG)
- gemu_log("ioctl: cmd=0x%04lx (%s)\n", cmd, ie->name);
+ gemu_log("ioctl: cmd=0x%04lx (%s)\n", (long)cmd, ie->name);
#endif
switch(arg_type[0]) {
case TYPE_NULL:
@@ -1799,7 +1822,8 @@ static long do_ioctl(long fd, long cmd,
}
break;
default:
- gemu_log("Unsupported ioctl type: cmd=0x%04lx type=%d\n", cmd, arg_type[0]);
+ gemu_log("Unsupported ioctl type: cmd=0x%04lx type=%d\n",
+ (long)cmd, arg_type[0]);
ret = -ENOSYS;
break;
}
@@ -2149,7 +2173,7 @@ static int clone_func(void *arg)
return 0;
}
-int do_fork(CPUState *env, unsigned int flags, unsigned long newsp)
+int do_fork(CPUState *env, unsigned int flags, target_ulong newsp)
{
int ret;
TaskState *ts;
@@ -2235,13 +2259,13 @@ int do_fork(CPUState *env, unsigned int
return ret;
}
-static long do_fcntl(int fd, int cmd, target_ulong arg)
+static target_long do_fcntl(int fd, int cmd, target_ulong arg)
{
struct flock fl;
struct target_flock *target_fl;
struct flock64 fl64;
struct target_flock64 *target_fl64;
- long ret;
+ target_long ret;
switch(cmd) {
case TARGET_F_GETLK:
@@ -2400,6 +2424,7 @@ void syscall_init(void)
}
}
+#if TARGET_LONG_BITS == 32
static inline uint64_t target_offset64(uint32_t word0, uint32_t word1)
{
#ifdef TARGET_WORDS_BIG_ENDIAN
@@ -2408,10 +2433,18 @@ static inline uint64_t target_offset64(u
return ((uint64_t)word1 << 32) | word0;
#endif
}
+#else /* TARGET_LONG_BITS == 32 */
+static inline uint64_t target_offset64(uint64_t word0, uint64_t word1)
+{
+ return word0;
+}
+#endif /* TARGET_LONG_BITS != 32 */
#ifdef TARGET_NR_truncate64
-static inline long target_truncate64(void *cpu_env, const char *arg1,
- long arg2, long arg3, long arg4)
+static inline target_long target_truncate64(void *cpu_env, const char *arg1,
+ target_long arg2,
+ target_long arg3,
+ target_long arg4)
{
#ifdef TARGET_ARM
if (((CPUARMState *)cpu_env)->eabi)
@@ -2425,8 +2458,10 @@ static inline long target_truncate64(voi
#endif
#ifdef TARGET_NR_ftruncate64
-static inline long target_ftruncate64(void *cpu_env, long arg1, long arg2,
- long arg3, long arg4)
+static inline target_long target_ftruncate64(void *cpu_env, target_long arg1,
+ target_long arg2,
+ target_long arg3,
+ target_long arg4)
{
#ifdef TARGET_ARM
if (((CPUARMState *)cpu_env)->eabi)
@@ -2461,10 +2496,11 @@ static inline void host_to_target_timesp
unlock_user_struct(target_ts, target_addr, 1);
}
-long do_syscall(void *cpu_env, int num, long arg1, long arg2, long arg3,
- long arg4, long arg5, long arg6)
+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)
{
- long ret;
+ target_long ret;
struct stat st;
struct statfs stfs;
void *p;
@@ -3781,7 +3817,7 @@ long do_syscall(void *cpu_env, int num,
{
struct target_dirent *target_dirp;
struct dirent *dirp;
- long count = arg3;
+ target_long count = arg3;
dirp = malloc(count);
if (!dirp)
@@ -3823,7 +3859,7 @@ long do_syscall(void *cpu_env, int num,
#else
{
struct dirent *dirp;
- long count = arg3;
+ target_long count = arg3;
dirp = lock_user(arg2, count, 0);
ret = get_errno(sys_getdents(arg1, dirp, count));
@@ -3851,7 +3887,7 @@ long do_syscall(void *cpu_env, int num,
case TARGET_NR_getdents64:
{
struct dirent64 *dirp;
- long count = arg3;
+ target_long count = arg3;
dirp = lock_user(arg2, count, 0);
ret = get_errno(sys_getdents64(arg1, dirp, count));
if (!is_error(ret)) {
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Fwd: [Qemu-devel] RFC: linux user problems]
2007-09-18 23:28 [Fwd: [Qemu-devel] RFC: linux user problems] J. Mayer
@ 2007-09-19 9:07 ` Thiemo Seufer
2007-09-19 9:23 ` Jocelyn Mayer
0 siblings, 1 reply; 3+ messages in thread
From: Thiemo Seufer @ 2007-09-19 9:07 UTC (permalink / raw)
To: J. Mayer; +Cc: qemu-devel
J. Mayer wrote:
> Following my previous message, I did a patch that makes syscalls take
> target_long/target_ulong argument and return target_long value instead
> of long/unsigned long.
> I also included the #ifdef protection for do_socketcall and do_ipc to
> avoid compilation warnings.
> And I also converted the fd given to do_ioctl to be an int.
>
> In addition to my previous remarks, I noticed some other things while
> reading the code:
> - the do_msgctl function seems very strange to me. It looks like half of
> the code is missing in the switch...
> - do_ipc directly uses pointers from the emulated environment without
> using lock_user related functions like it seems to be done everywhere
> else.
> - there are at least two problems in IPCOP_shmat:
> * the returned address could not fit in the target address space when
> emulating a 32 bits
> target on a 64 bits host
> * the returned address is always casted into a 32 bits value. I changed
> this to be target_ulong.
> - I also noticed some suspicious warnings (cast between pointer and
> integer of different size) that may hide other problems:
> * target_to_host_cmsg:567
> * host_to_target_cmsg:612
> * do_ipc:1609
> * do_ipc: 1621
> * do_ipc: 1645
> * do_ipc: 1655
> * do_ipc: 1677 (multiple times)
> * do_ipc: 1687
> * do_ipc: 1711
> * do_syscall:2686
> * do_syscall: 3903
> * do_syscall: 4671
>
> May someone take a look at my patch and say if it seems reasonable to
> include this in the repository ?
Looks reasonable, but introduces new compiler warnings
(on a ppc32/Linux host):
Most (all?) targets show:
/home/ths/qemu/qemu-work/linux-user/syscall.c: In function do_ipc':
/home/ths/qemu/qemu-work/linux-user/syscall.c:1612: warning: long int format, target_long arg (arg 2)
There's also one instance of:
/home/ths/qemu/qemu-work/linux-user/syscall.c: At top level:
/home/ths/qemu/qemu-work/linux-user/syscall.c:1258: warning: 'shm_regions' defined but not used
which looks like a missing #ifdef TARGET_NR_ipc.
Thiemo
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Fwd: [Qemu-devel] RFC: linux user problems]
2007-09-19 9:07 ` Thiemo Seufer
@ 2007-09-19 9:23 ` Jocelyn Mayer
0 siblings, 0 replies; 3+ messages in thread
From: Jocelyn Mayer @ 2007-09-19 9:23 UTC (permalink / raw)
To: qemu-devel
On Wed, 2007-09-19 at 10:07 +0100, Thiemo Seufer wrote:
> J. Mayer wrote:
> > Following my previous message, I did a patch that makes syscalls take
> > target_long/target_ulong argument and return target_long value instead
> > of long/unsigned long.
> > I also included the #ifdef protection for do_socketcall and do_ipc to
> > avoid compilation warnings.
> > And I also converted the fd given to do_ioctl to be an int.
> >
> > In addition to my previous remarks, I noticed some other things while
> > reading the code:
> > - the do_msgctl function seems very strange to me. It looks like half of
> > the code is missing in the switch...
> > - do_ipc directly uses pointers from the emulated environment without
> > using lock_user related functions like it seems to be done everywhere
> > else.
> > - there are at least two problems in IPCOP_shmat:
> > * the returned address could not fit in the target address space when
> > emulating a 32 bits
> > target on a 64 bits host
> > * the returned address is always casted into a 32 bits value. I changed
> > this to be target_ulong.
> > - I also noticed some suspicious warnings (cast between pointer and
> > integer of different size) that may hide other problems:
> > * target_to_host_cmsg:567
> > * host_to_target_cmsg:612
> > * do_ipc:1609
> > * do_ipc: 1621
> > * do_ipc: 1645
> > * do_ipc: 1655
> > * do_ipc: 1677 (multiple times)
> > * do_ipc: 1687
> > * do_ipc: 1711
> > * do_syscall:2686
> > * do_syscall: 3903
> > * do_syscall: 4671
> >
> > May someone take a look at my patch and say if it seems reasonable to
> > include this in the repository ?
>
> Looks reasonable, but introduces new compiler warnings
> (on a ppc32/Linux host):
>
> Most (all?) targets show:
>
> /home/ths/qemu/qemu-work/linux-user/syscall.c: In function do_ipc':
> /home/ths/qemu/qemu-work/linux-user/syscall.c:1612: warning: long int format, target_long arg (arg 2)
My fault, I did only compile on x86_64, forgot to check in 32 bits mode,
and did not see this warning (I may have missed it...), sorry.
>
> There's also one instance of:
>
> /home/ths/qemu/qemu-work/linux-user/syscall.c: At top level:
> /home/ths/qemu/qemu-work/linux-user/syscall.c:1258: warning: 'shm_regions' defined but not used
>
> which looks like a missing #ifdef TARGET_NR_ipc.
I will check more closely as there are also a lot of inline functions
(then not generating compilation warnings) that are used only from
do_ipc. Putting them in the #ifdef TARGET_NR_ipc may show other unused
variables or functions.
Thanks for the report.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2007-09-19 9:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-09-18 23:28 [Fwd: [Qemu-devel] RFC: linux user problems] J. Mayer
2007-09-19 9:07 ` Thiemo Seufer
2007-09-19 9:23 ` Jocelyn Mayer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).