[Qemu-devel] [PATCH 0/8] linux-user: Fix gcc 4.6 compile warnings

[Qemu-devel] [PATCH 0/8] linux-user: Fix gcc 4.6 compile warnings

[Peter Maydell]

This is a collection of patches which fix the gcc 4.6 compile warnings
in the linux-user code. Mostly they're from Juan's earlier patchset,
in some cases with some minor tweaks by me as per review comments on
that patchset. The last three I fixed in a different way so those
are new patches.

Sorry for producing yet another gcc 4.6 related patchset, but
splitting out the linux-user patches to go through Riku's queue
seems like a good way of moving these forward.


(For those keeping score on the gcc 4.6 warning front the other
fixes needed are:
 * two target-alpha fixes both acked by Richard Henderson:
   http://patchwork.ozlabs.org/patch/100463/ 
   http://patchwork.ozlabs.org/patch/100456/
 * lsi53c895a.c fix: in the trivial-patches pullreq
 * target-i386/kvm.c: being fixed by Jan Kiszka
 * exec.c patch http://patchwork.ozlabs.org/patch/98384/  )

Juan Quintela (5):
  linuxload: id_change was a write only variable
  syscall: really return ret code
  linux-user: syscall should use sanitized arg1
  flatload: end_code was only used in a debug message
  flatload: memp was a write-only variable

Peter Maydell (3):
  linux-user: Bump do_syscall() up to 8 syscall arguments
  linux-user/signal.c: Remove only-ever-set variable fpu_save_addr
  linux-user/signal.c: Remove unused fenab

 linux-user/flatload.c  |    8 ++------
 linux-user/linuxload.c |   25 +------------------------
 linux-user/main.c      |   37 ++++++++++++++++++++++++-------------
 linux-user/qemu.h      |    3 ++-
 linux-user/signal.c    |   17 ++++++++++-------
 linux-user/syscall.c   |   20 +++++++++++---------
 6 files changed, 50 insertions(+), 60 deletions(-)

[Qemu-devel] [PATCH 1/8] linuxload: id_change was a write only variable

[Peter Maydell]

From: Juan Quintela <quintela@redhat.com>

Signed-off-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/linuxload.c |   25 +------------------------
 1 files changed, 1 insertions(+), 24 deletions(-)

diff --git a/linux-user/linuxload.c b/linux-user/linuxload.c
index ac8c486..62ebc7e 100644
--- a/linux-user/linuxload.c
+++ b/linux-user/linuxload.c
@@ -26,22 +26,6 @@ abi_long memcpy_to_target(abi_ulong dest, const void *src,
     return 0;
 }
 
-static int in_group_p(gid_t g)
-{
-    /* return TRUE if we're in the specified group, FALSE otherwise */
-    int		ngroup;
-    int		i;
-    gid_t	grouplist[NGROUPS];
-
-    ngroup = getgroups(NGROUPS, grouplist);
-    for(i = 0; i < ngroup; i++) {
-	if(grouplist[i] == g) {
-	    return 1;
-	}
-    }
-    return 0;
-}
-
 static int count(char ** vec)
 {
     int		i;
@@ -57,7 +41,7 @@ static int prepare_binprm(struct linux_binprm *bprm)
 {
     struct stat		st;
     int mode;
-    int retval, id_change;
+    int retval;
 
     if(fstat(bprm->fd, &st) < 0) {
 	return(-errno);
@@ -73,14 +57,10 @@ static int prepare_binprm(struct linux_binprm *bprm)
 
     bprm->e_uid = geteuid();
     bprm->e_gid = getegid();
-    id_change = 0;
 
     /* Set-uid? */
     if(mode & S_ISUID) {
     	bprm->e_uid = st.st_uid;
-	if(bprm->e_uid != geteuid()) {
-	    id_change = 1;
-	}
     }
 
     /* Set-gid? */
@@ -91,9 +71,6 @@ static int prepare_binprm(struct linux_binprm *bprm)
      */
     if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) {
 	bprm->e_gid = st.st_gid;
-	if (!in_group_p(bprm->e_gid)) {
-		id_change = 1;
-	}
     }
 
     retval = read(bprm->fd, bprm->buf, BPRM_BUF_SIZE);
-- 
1.7.1

[Qemu-devel] [PATCH 2/8] syscall: really return ret code

[Peter Maydell]

From: Juan Quintela <quintela@redhat.com>

We assign ret with the error code, but then return 0 unconditionally.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 5cb27c7..f3d03b0 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3751,10 +3751,10 @@ static abi_long do_get_thread_area(CPUX86State *env, abi_ulong ptr)
 #ifndef TARGET_ABI32
 static abi_long do_arch_prctl(CPUX86State *env, int code, abi_ulong addr)
 {
-    abi_long ret;
+    abi_long ret = 0;
     abi_ulong val;
     int idx;
-    
+
     switch(code) {
     case TARGET_ARCH_SET_GS:
     case TARGET_ARCH_SET_FS:
@@ -3773,13 +3773,13 @@ static abi_long do_arch_prctl(CPUX86State *env, int code, abi_ulong addr)
             idx = R_FS;
         val = env->segs[idx].base;
         if (put_user(val, addr, abi_ulong))
-            return -TARGET_EFAULT;
+            ret = -TARGET_EFAULT;
         break;
     default:
         ret = -TARGET_EINVAL;
         break;
     }
-    return 0;
+    return ret;
 }
 #endif
 
-- 
1.7.1

[Qemu-devel] [PATCH 3/8] linux-user: syscall should use sanitized arg1

[Peter Maydell]

From: Juan Quintela <quintela@redhat.com>

Looking at the other architectures, we should be using "how" not "arg1".

Signed-off-by: Juan Quintela <quintela@redhat.com>
[peter.maydell@linaro.org: remove unnecessary initialisation of how]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index f3d03b0..2635b75 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7058,7 +7058,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
     case TARGET_NR_osf_sigprocmask:
         {
             abi_ulong mask;
-            int how = arg1;
+            int how;
             sigset_t set, oldset;
 
             switch(arg1) {
@@ -7077,7 +7077,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             }
             mask = arg2;
             target_to_host_old_sigset(&set, &mask);
-            sigprocmask(arg1, &set, &oldset);
+            sigprocmask(how, &set, &oldset);
             host_to_target_old_sigset(&mask, &oldset);
             ret = mask;
         }
-- 
1.7.1

[Qemu-devel] [PATCH 4/8] flatload: end_code was only used in a debug message

[Peter Maydell]

From: Juan Quintela <quintela@redhat.com>

Just unfold its definition in only use.

Signed-off-by: Juan Quintela <quintela@redhat.com>
[peter.maydell@linaro.org: fixed typo in the debug code,
added parentheses to fix precedence issue]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/flatload.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/linux-user/flatload.c b/linux-user/flatload.c
index cd7af7c..6fb78f5 100644
--- a/linux-user/flatload.c
+++ b/linux-user/flatload.c
@@ -384,7 +384,7 @@ static int load_flat_file(struct linux_binprm * bprm,
     abi_ulong reloc = 0, rp;
     int i, rev, relocs = 0;
     abi_ulong fpos;
-    abi_ulong start_code, end_code;
+    abi_ulong start_code;
     abi_ulong indx_len;
 
     hdr = ((struct flat_hdr *) bprm->buf);		/* exec-header */
@@ -552,11 +552,10 @@ static int load_flat_file(struct linux_binprm * bprm,
 
     /* The main program needs a little extra setup in the task structure */
     start_code = textpos + sizeof (struct flat_hdr);
-    end_code = textpos + text_len;
 
     DBG_FLT("%s %s: TEXT=%x-%x DATA=%x-%x BSS=%x-%x\n",
             id ? "Lib" : "Load", bprm->filename,
-            (int) start_code, (int) end_code,
+            (int) start_code, (int) (textpos + text_len),
             (int) datapos,
             (int) (datapos + data_len),
             (int) (datapos + data_len),
-- 
1.7.1

[Qemu-devel] [PATCH 5/8] flatload: memp was a write-only variable

[Peter Maydell]

From: Juan Quintela <quintela@redhat.com>

Signed-off-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/flatload.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/linux-user/flatload.c b/linux-user/flatload.c
index 6fb78f5..1062da3 100644
--- a/linux-user/flatload.c
+++ b/linux-user/flatload.c
@@ -379,7 +379,6 @@ static int load_flat_file(struct linux_binprm * bprm,
     abi_long result;
     abi_ulong realdatastart = 0;
     abi_ulong text_len, data_len, bss_len, stack_len, flags;
-    abi_ulong memp = 0; /* for finding the brk area */
     abi_ulong extra;
     abi_ulong reloc = 0, rp;
     int i, rev, relocs = 0;
@@ -491,7 +490,6 @@ static int load_flat_file(struct linux_binprm * bprm,
         }
 
         reloc = datapos + (ntohl(hdr->reloc_start) - text_len);
-        memp = realdatastart;
 
     } else {
 
@@ -506,7 +504,6 @@ static int load_flat_file(struct linux_binprm * bprm,
         realdatastart = textpos + ntohl(hdr->data_start);
         datapos = realdatastart + indx_len;
         reloc = (textpos + ntohl(hdr->reloc_start) + indx_len);
-        memp = textpos;
 
 #ifdef CONFIG_BINFMT_ZFLAT
 #error code needs checking
-- 
1.7.1

[Qemu-devel] [PATCH 6/8] linux-user: Bump do_syscall() up to 8 syscall arguments

[Peter Maydell]

On 32 bit MIPS a few syscalls have 7 arguments, and so to call
them via NR_syscall the guest needs to be able to pass 8 arguments
to do_syscall(). Raise the number of arguments do_syscall() takes
accordingly.

This fixes some gcc 4.6 compiler warnings about arg7 and arg8
variables being set and never used.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/main.c    |   37 ++++++++++++++++++++++++-------------
 linux-user/qemu.h    |    3 ++-
 linux-user/syscall.c |    8 +++++---
 3 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 71dd253..1293450 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -319,7 +319,8 @@ void cpu_loop(CPUX86State *env)
                                           env->regs[R_EDX],
                                           env->regs[R_ESI],
                                           env->regs[R_EDI],
-                                          env->regs[R_EBP]);
+                                          env->regs[R_EBP],
+                                          0, 0);
             break;
 #ifndef TARGET_ABI32
         case EXCP_SYSCALL:
@@ -331,7 +332,8 @@ void cpu_loop(CPUX86State *env)
                                           env->regs[R_EDX],
                                           env->regs[10],
                                           env->regs[8],
-                                          env->regs[9]);
+                                          env->regs[9],
+                                          0, 0);
             env->eip = env->exception_next_eip;
             break;
 #endif
@@ -735,7 +737,8 @@ void cpu_loop(CPUARMState *env)
                                                   env->regs[2],
                                                   env->regs[3],
                                                   env->regs[4],
-                                                  env->regs[5]);
+                                                  env->regs[5],
+                                                  0, 0);
                     }
                 } else {
                     goto error;
@@ -831,7 +834,8 @@ void cpu_loop(CPUState *env)
                                                   env->regs[2],
                                                   env->regs[3],
                                                   env->regs[4],
-                                                  env->regs[5]);
+                                                  env->regs[5],
+                                                  0, 0);
                     }
                 } else {
                     goto error;
@@ -1018,7 +1022,8 @@ void cpu_loop (CPUSPARCState *env)
             ret = do_syscall (env, env->gregs[1],
                               env->regwptr[0], env->regwptr[1],
                               env->regwptr[2], env->regwptr[3],
-                              env->regwptr[4], env->regwptr[5]);
+                              env->regwptr[4], env->regwptr[5],
+                              0, 0);
             if ((abi_ulong)ret >= (abi_ulong)(-515)) {
 #if defined(TARGET_SPARC64) && !defined(TARGET_ABI32)
                 env->xcc |= PSR_CARRY;
@@ -1611,7 +1616,7 @@ void cpu_loop(CPUPPCState *env)
             env->crf[0] &= ~0x1;
             ret = do_syscall(env, env->gpr[0], env->gpr[3], env->gpr[4],
                              env->gpr[5], env->gpr[6], env->gpr[7],
-                             env->gpr[8]);
+                             env->gpr[8], 0, 0);
             if (ret == (uint32_t)(-TARGET_QEMU_ESIGRETURN)) {
                 /* Returning from a successful sigreturn syscall.
                    Avoid corrupting register state.  */
@@ -2072,7 +2077,7 @@ void cpu_loop(CPUMIPSState *env)
                                  env->active_tc.gpr[5],
                                  env->active_tc.gpr[6],
                                  env->active_tc.gpr[7],
-                                 arg5, arg6/*, arg7, arg8*/);
+                                 arg5, arg6, arg7, arg8);
             }
             if (ret == -TARGET_QEMU_ESIGRETURN) {
                 /* Returning from a successful sigreturn syscall.
@@ -2160,7 +2165,8 @@ void cpu_loop (CPUState *env)
                              env->gregs[6],
                              env->gregs[7],
                              env->gregs[0],
-                             env->gregs[1]);
+                             env->gregs[1],
+                             0, 0);
             env->gregs[0] = ret;
             break;
         case EXCP_INTERRUPT:
@@ -2229,7 +2235,8 @@ void cpu_loop (CPUState *env)
                              env->regs[12], 
                              env->regs[13], 
                              env->pregs[7], 
-                             env->pregs[11]);
+                             env->pregs[11],
+                             0, 0);
             env->regs[10] = ret;
             break;
         case EXCP_DEBUG:
@@ -2288,7 +2295,8 @@ void cpu_loop (CPUState *env)
                              env->regs[7], 
                              env->regs[8], 
                              env->regs[9], 
-                             env->regs[10]);
+                             env->regs[10],
+                             0, 0);
             env->regs[3] = ret;
             env->sregs[SR_PC] = env->regs[14];
             break;
@@ -2398,7 +2406,8 @@ void cpu_loop(CPUM68KState *env)
                                           env->dregs[3],
                                           env->dregs[4],
                                           env->dregs[5],
-                                          env->aregs[0]);
+                                          env->aregs[0],
+                                          0, 0);
             }
             break;
         case EXCP_INTERRUPT:
@@ -2576,7 +2585,8 @@ void cpu_loop (CPUState *env)
                 sysret = do_syscall(env, trapnr,
                                     env->ir[IR_A0], env->ir[IR_A1],
                                     env->ir[IR_A2], env->ir[IR_A3],
-                                    env->ir[IR_A4], env->ir[IR_A5]);
+                                    env->ir[IR_A4], env->ir[IR_A5],
+                                    0, 0);
                 if (trapnr == TARGET_NR_sigreturn
                     || trapnr == TARGET_NR_rt_sigreturn) {
                     break;
@@ -2707,7 +2717,8 @@ void cpu_loop(CPUS390XState *env)
                            env->regs[4],
                            env->regs[5],
                            env->regs[6],
-                           env->regs[7]);
+                           env->regs[7],
+                           0, 0);
             }
             break;
         case EXCP_ADDR:
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 237386c..627c8b3 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -192,7 +192,8 @@ abi_long do_brk(abi_ulong new_brk);
 void syscall_init(void);
 abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
                     abi_long arg2, abi_long arg3, abi_long arg4,
-                    abi_long arg5, abi_long arg6);
+                    abi_long arg5, abi_long arg6, abi_long arg7,
+                    abi_long arg8);
 void gemu_log(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 extern THREAD CPUState *thread_env;
 void cpu_loop(CPUState *env);
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 2635b75..a95025f 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -4484,7 +4484,8 @@ int get_osversion(void)
    All errnos that do_syscall() returns must be -TARGET_<errcode>. */
 abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
                     abi_long arg2, abi_long arg3, abi_long arg4,
-                    abi_long arg5, abi_long arg6)
+                    abi_long arg5, abi_long arg6, abi_long arg7,
+                    abi_long arg8)
 {
     abi_long ret;
     struct stat st;
@@ -6029,8 +6030,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #ifdef TARGET_NR_syscall
     case TARGET_NR_syscall:
-    	ret = do_syscall(cpu_env,arg1 & 0xffff,arg2,arg3,arg4,arg5,arg6,0);
-    	break;
+        ret = do_syscall(cpu_env, arg1 & 0xffff, arg2, arg3, arg4, arg5,
+                         arg6, arg7, arg8, 0);
+        break;
 #endif
     case TARGET_NR_wait4:
         {
-- 
1.7.1

[Qemu-devel] [PATCH 7/8] linux-user/signal.c: Remove only-ever-set variable fpu_save_addr

[Peter Maydell]

Move the access of fpu_save into the commented out skeleton code for
restoring FPU registers on SPARC sigreturn, thus silencing a gcc
4.6 "variable set but never used" warning.
(This doesn't affect the calculation of 'err' because in fact
__get_user() can never fail.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/signal.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 11b25be..2e34ffb 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -2080,7 +2080,6 @@ long do_sigreturn(CPUState *env)
         uint32_t up_psr, pc, npc;
         target_sigset_t set;
         sigset_t host_set;
-        abi_ulong fpu_save_addr;
         int err, i;
 
         sf_addr = env->regwptr[UREG_FP];
@@ -2120,10 +2119,11 @@ long do_sigreturn(CPUState *env)
 		err |= __get_user(env->regwptr[i + UREG_I0], &sf->info.si_regs.u_regs[i+8]);
 	}
 
-        err |= __get_user(fpu_save_addr, &sf->fpu_save);
-
-        //if (fpu_save)
-        //        err |= restore_fpu_state(env, fpu_save);
+        /* FIXME: implement FPU save/restore:
+         * __get_user(fpu_save, &sf->fpu_save);
+         * if (fpu_save)
+         *        err |= restore_fpu_state(env, fpu_save);
+         */
 
         /* This is pretty much atomic, no amount locking would prevent
          * the races which exist anyways.
-- 
1.7.1

[Qemu-devel] [PATCH 8/8] linux-user/signal.c: Remove unused fenab

[Peter Maydell]

Remove fenab as it is only written, never used. Add a FIXME
comment about the discrepancy between our behaviour and that
of the Linux kernel for this routine.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/signal.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 2e34ffb..73670ac 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -2228,7 +2228,6 @@ void sparc64_set_context(CPUSPARCState *env)
     target_mc_gregset_t *grp;
     abi_ulong pc, npc, tstate;
     abi_ulong fp, i7, w_addr;
-    unsigned char fenab;
     int err;
     unsigned int i;
 
@@ -2293,7 +2292,11 @@ void sparc64_set_context(CPUSPARCState *env)
     if (put_user(i7, w_addr + offsetof(struct target_reg_window, ins[7]), 
                  abi_ulong) != 0)
         goto do_sigsegv;
-    err |= __get_user(fenab, &(ucp->tuc_mcontext.mc_fpregs.mcfpu_enab));
+    /* FIXME this does not match how the kernel handles the FPU in
+     * its sparc64_set_context implementation. In particular the FPU
+     * is only restored if fenab is non-zero in:
+     *   __get_user(fenab, &(ucp->tuc_mcontext.mc_fpregs.mcfpu_enab));
+     */
     err |= __get_user(env->fprs, &(ucp->tuc_mcontext.mc_fpregs.mcfpu_fprs));
     {
         uint32_t *src, *dst;
-- 
1.7.1