* [Qemu-devel] [PATCH v2 05/13] signal/all: remove return value from restore_sigcontext
@ 2014-06-06 9:46 riku.voipio
2014-06-07 21:30 ` Peter Maydell
0 siblings, 1 reply; 3+ messages in thread
From: riku.voipio @ 2014-06-06 9:46 UTC (permalink / raw)
To: qemu-devel; +Cc: Riku Voipio
From: Riku Voipio <riku.voipio@linaro.org>
make most implementations of restore_sigcontext void and
remove checking it's return value from functions calling
restore_sigcontext.
The exception is the X86 version of the function that is
too different from others to deal in this way.
Signed-off-by: Riku Voipio <riku.voipio@linaro.org>
---
linux-user/signal.c | 65 +++++++++++++----------------------------------------
1 file changed, 16 insertions(+), 49 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 421bd48..7b828bf 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -1546,12 +1546,6 @@ static const abi_ulong retcodes[4] = {
SWI_SYS_RT_SIGRETURN, SWI_THUMB_RT_SIGRETURN
};
-
-static inline int valid_user_regs(CPUARMState *regs)
-{
- return 1;
-}
-
static void
setup_sigcontext(struct target_sigcontext *sc, /*struct _fpstate *fpstate,*/
CPUARMState *env, abi_ulong mask)
@@ -1842,10 +1836,9 @@ static void setup_rt_frame(int usig, struct target_sigaction *ka,
}
}
-static int
+static void
restore_sigcontext(CPUARMState *env, struct target_sigcontext *sc)
{
- int err = 0;
uint32_t cpsr;
__get_user(env->regs[0], &sc->arm_r0);
@@ -1868,10 +1861,6 @@ restore_sigcontext(CPUARMState *env, struct target_sigcontext *sc)
__get_user(cpsr, &sc->arm_cpsr);
cpsr_write(env, cpsr, CPSR_USER | CPSR_EXEC);
#endif
-
- err |= !valid_user_regs(env);
-
- return err;
}
static long do_sigreturn_v1(CPUARMState *env)
@@ -1905,8 +1894,7 @@ static long do_sigreturn_v1(CPUARMState *env)
target_to_host_sigset_internal(&host_set, &set);
do_sigprocmask(SIG_SETMASK, &host_set, NULL);
- if (restore_sigcontext(env, &frame->sc))
- goto badframe;
+ restore_sigcontext(env, &frame->sc);
#if 0
/* Send SIGTRAP if we're single-stepping */
@@ -1986,8 +1974,7 @@ static int do_sigframe_return_v2(CPUARMState *env, target_ulong frame_addr,
target_to_host_sigset(&host_set, &uc->tuc_sigmask);
do_sigprocmask(SIG_SETMASK, &host_set, NULL);
- if (restore_sigcontext(env, &uc->tuc_mcontext))
- return 1;
+ restore_sigcontext(env, &uc->tuc_mcontext);
/* Restore coprocessor signal frame */
regspace = uc->tuc_regspace;
@@ -2077,8 +2064,7 @@ static long do_rt_sigreturn_v1(CPUARMState *env)
target_to_host_sigset(&host_set, &frame->uc.tuc_sigmask);
do_sigprocmask(SIG_SETMASK, &host_set, NULL);
- if (restore_sigcontext(env, &frame->uc.tuc_mcontext))
- goto badframe;
+ restore_sigcontext(env, &frame->uc.tuc_mcontext);
if (do_sigaltstack(frame_addr + offsetof(struct rt_sigframe_v1, uc.tuc_stack), 0, get_sp_from_cpustate(env)) == -EFAULT)
goto badframe;
@@ -2889,10 +2875,9 @@ static inline void setup_sigcontext(CPUMIPSState *regs,
}
}
-static inline int
+static inline void
restore_sigcontext(CPUMIPSState *regs, struct target_sigcontext *sc)
{
- int err = 0;
int i;
__get_user(regs->CP0_EPC, &sc->sc_pc);
@@ -2919,8 +2904,6 @@ restore_sigcontext(CPUMIPSState *regs, struct target_sigcontext *sc)
for (i = 0; i < 32; ++i) {
__get_user(regs->active_fpu.fpr[i].d, &sc->sc_fpregs[i]);
}
-
- return err;
}
/*
@@ -3031,8 +3014,7 @@ long do_sigreturn(CPUMIPSState *regs)
target_to_host_sigset_internal(&blocked, &target_set);
do_sigprocmask(SIG_SETMASK, &blocked, NULL);
- if (restore_sigcontext(regs, &frame->sf_sc))
- goto badframe;
+ restore_sigcontext(regs, &frame->sf_sc);
#if 0
/*
@@ -3135,8 +3117,7 @@ long do_rt_sigreturn(CPUMIPSState *env)
target_to_host_sigset(&blocked, &frame->rs_uc.tuc_sigmask);
do_sigprocmask(SIG_SETMASK, &blocked, NULL);
- if (restore_sigcontext(env, &frame->rs_uc.tuc_mcontext))
- goto badframe;
+ restore_sigcontext(env, &frame->rs_uc.tuc_mcontext);
if (do_sigaltstack(frame_addr +
offsetof(struct target_rt_sigframe, rs_uc.tuc_stack),
@@ -3249,10 +3230,9 @@ static void setup_sigcontext(struct target_sigcontext *sc,
__put_user(mask, &sc->oldmask);
}
-static int restore_sigcontext(CPUSH4State *regs, struct target_sigcontext *sc,
+static void restore_sigcontext(CPUSH4State *regs, struct target_sigcontext *sc,
target_ulong *r0_p)
{
- unsigned int err = 0;
int i;
#define COPY(x) __get_user(regs->x, &sc->sc_##x)
@@ -3277,7 +3257,6 @@ static int restore_sigcontext(CPUSH4State *regs, struct target_sigcontext *sc,
regs->tra = -1; /* disable syscall checks */
__get_user(*r0_p, &sc->sc_gregs[0]);
- return err;
}
static void setup_frame(int sig, struct target_sigaction *ka,
@@ -3422,8 +3401,7 @@ long do_sigreturn(CPUSH4State *regs)
target_to_host_sigset_internal(&blocked, &target_set);
do_sigprocmask(SIG_SETMASK, &blocked, NULL);
- if (restore_sigcontext(regs, &frame->sc, &r0))
- goto badframe;
+ restore_sigcontext(regs, &frame->sc, &r0);
unlock_user_struct(frame, frame_addr, 0);
return r0;
@@ -3451,8 +3429,7 @@ long do_rt_sigreturn(CPUSH4State *regs)
target_to_host_sigset(&blocked, &frame->uc.tuc_sigmask);
do_sigprocmask(SIG_SETMASK, &blocked, NULL);
- if (restore_sigcontext(regs, &frame->uc.tuc_mcontext, &r0))
- goto badframe;
+ restore_sigcontext(regs, &frame->uc.tuc_mcontext, &r0);
if (do_sigaltstack(frame_addr +
offsetof(struct target_rt_sigframe, uc.tuc_stack),
@@ -5086,10 +5063,9 @@ static void setup_sigcontext(struct target_sigcontext *sc, CPUM68KState *env,
__put_user(env->pc, &sc->sc_pc);
}
-static int
+static void
restore_sigcontext(CPUM68KState *env, struct target_sigcontext *sc, int *pd0)
{
- int err = 0;
int temp;
__get_user(env->aregs[7], &sc->sc_usp);
@@ -5101,8 +5077,6 @@ restore_sigcontext(CPUM68KState *env, struct target_sigcontext *sc, int *pd0)
env->sr = (env->sr & 0xff00) | (temp & 0xff);
*pd0 = tswapl(sc->sc_d0);
-
- return err;
}
/*
@@ -5343,8 +5317,7 @@ long do_sigreturn(CPUM68KState *env)
/* restore registers */
- if (restore_sigcontext(env, &frame->sc, &d0))
- goto badframe;
+ restore_sigcontext(env, &frame->sc, &d0);
unlock_user_struct(frame, frame_addr, 0);
return d0;
@@ -5462,11 +5435,11 @@ static void setup_sigcontext(struct target_sigcontext *sc, CPUAlphaState *env,
__put_user(0, &sc->sc_traparg_a2); /* FIXME */
}
-static int restore_sigcontext(CPUAlphaState *env,
+static void restore_sigcontext(CPUAlphaState *env,
struct target_sigcontext *sc)
{
uint64_t fpcr;
- int i, err = 0;
+ int i;
__get_user(env->pc, &sc->sc_pc);
@@ -5479,8 +5452,6 @@ static int restore_sigcontext(CPUAlphaState *env,
__get_user(fpcr, &sc->sc_fpcr);
cpu_alpha_store_fpcr(env, fpcr);
-
- return err;
}
static inline abi_ulong get_sigframe(struct target_sigaction *sa,
@@ -5614,9 +5585,7 @@ long do_sigreturn(CPUAlphaState *env)
target_to_host_sigset_internal(&set, &target_set);
do_sigprocmask(SIG_SETMASK, &set, NULL);
- if (restore_sigcontext(env, sc)) {
- goto badframe;
- }
+ restore_sigcontext(env, sc);
unlock_user_struct(sc, sc_addr, 0);
return env->ir[IR_V0];
@@ -5637,9 +5606,7 @@ long do_rt_sigreturn(CPUAlphaState *env)
target_to_host_sigset(&set, &frame->uc.tuc_sigmask);
do_sigprocmask(SIG_SETMASK, &set, NULL);
- if (restore_sigcontext(env, &frame->uc.tuc_mcontext)) {
- goto badframe;
- }
+ restore_sigcontext(env, &frame->uc.tuc_mcontext);
if (do_sigaltstack(frame_addr + offsetof(struct target_rt_sigframe,
uc.tuc_stack),
0, env->ir[IR_SP]) == -EFAULT) {
--
2.0.0.rc2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v2 05/13] signal/all: remove return value from restore_sigcontext
2014-06-06 9:46 [Qemu-devel] [PATCH v2 05/13] signal/all: remove return value from restore_sigcontext riku.voipio
@ 2014-06-07 21:30 ` Peter Maydell
2014-06-09 12:13 ` [Qemu-devel] [PATCH v3] " riku.voipio
0 siblings, 1 reply; 3+ messages in thread
From: Peter Maydell @ 2014-06-07 21:30 UTC (permalink / raw)
To: Riku Voipio; +Cc: QEMU Developers
On 6 June 2014 10:46, <riku.voipio@linaro.org> wrote:
> From: Riku Voipio <riku.voipio@linaro.org>
>
> make most implementations of restore_sigcontext void and
> remove checking it's return value from functions calling
> restore_sigcontext.
>
> The exception is the X86 version of the function that is
> too different from others to deal in this way.
>
> Signed-off-by: Riku Voipio <riku.voipio@linaro.org>
> ---
> linux-user/signal.c | 65 +++++++++++++----------------------------------------
> 1 file changed, 16 insertions(+), 49 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 421bd48..7b828bf 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -1546,12 +1546,6 @@ static const abi_ulong retcodes[4] = {
> SWI_SYS_RT_SIGRETURN, SWI_THUMB_RT_SIGRETURN
> };
>
> -
> -static inline int valid_user_regs(CPUARMState *regs)
> -{
> - return 1;
> -}
I don't think we should remove this function -- there are a number
of checks we should be making here for correct behaviour to
ensure that the guest hasn't passed us a bogus CPSR. Compare
the kernel's version of this function:
http://lxr.linux.no/#linux+v3.14.5/arch/arm/include/asm/ptrace.h#L46
We basically want all those checks (except we can drop the
ones related to 26-bit CPUs and M-profile). We don't need to
implement them in this series but we shouldn't remove the function
and code path which are the right places to add them later.
thanks
-- PMM
^ permalink raw reply [flat|nested] 3+ messages in thread
* [Qemu-devel] [PATCH v3] signal/all: remove return value from restore_sigcontext
2014-06-07 21:30 ` Peter Maydell
@ 2014-06-09 12:13 ` riku.voipio
0 siblings, 0 replies; 3+ messages in thread
From: riku.voipio @ 2014-06-09 12:13 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Riku Voipio
From: Riku Voipio <riku.voipio@linaro.org>
make most implementations of restore_sigcontext void and
remove checking it's return value from functions calling
restore_sigcontext.
The exception is the X86 version of the function that is
too different from others to deal in this way, and arm
version, to keep possibility of erroring out from failed
valid_user_regs.
v3: keep arm valid_user_regs for filling in near future.
Signed-off-by: Riku Voipio <riku.voipio@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
linux-user/signal.c | 43 ++++++++++++-------------------------------
1 file changed, 12 insertions(+), 31 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 421bd48..b2bc729 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -2889,10 +2889,9 @@ static inline void setup_sigcontext(CPUMIPSState *regs,
}
}
-static inline int
+static inline void
restore_sigcontext(CPUMIPSState *regs, struct target_sigcontext *sc)
{
- int err = 0;
int i;
__get_user(regs->CP0_EPC, &sc->sc_pc);
@@ -2919,8 +2918,6 @@ restore_sigcontext(CPUMIPSState *regs, struct target_sigcontext *sc)
for (i = 0; i < 32; ++i) {
__get_user(regs->active_fpu.fpr[i].d, &sc->sc_fpregs[i]);
}
-
- return err;
}
/*
@@ -3031,8 +3028,7 @@ long do_sigreturn(CPUMIPSState *regs)
target_to_host_sigset_internal(&blocked, &target_set);
do_sigprocmask(SIG_SETMASK, &blocked, NULL);
- if (restore_sigcontext(regs, &frame->sf_sc))
- goto badframe;
+ restore_sigcontext(regs, &frame->sf_sc);
#if 0
/*
@@ -3135,8 +3131,7 @@ long do_rt_sigreturn(CPUMIPSState *env)
target_to_host_sigset(&blocked, &frame->rs_uc.tuc_sigmask);
do_sigprocmask(SIG_SETMASK, &blocked, NULL);
- if (restore_sigcontext(env, &frame->rs_uc.tuc_mcontext))
- goto badframe;
+ restore_sigcontext(env, &frame->rs_uc.tuc_mcontext);
if (do_sigaltstack(frame_addr +
offsetof(struct target_rt_sigframe, rs_uc.tuc_stack),
@@ -3249,10 +3244,9 @@ static void setup_sigcontext(struct target_sigcontext *sc,
__put_user(mask, &sc->oldmask);
}
-static int restore_sigcontext(CPUSH4State *regs, struct target_sigcontext *sc,
+static void restore_sigcontext(CPUSH4State *regs, struct target_sigcontext *sc,
target_ulong *r0_p)
{
- unsigned int err = 0;
int i;
#define COPY(x) __get_user(regs->x, &sc->sc_##x)
@@ -3277,7 +3271,6 @@ static int restore_sigcontext(CPUSH4State *regs, struct target_sigcontext *sc,
regs->tra = -1; /* disable syscall checks */
__get_user(*r0_p, &sc->sc_gregs[0]);
- return err;
}
static void setup_frame(int sig, struct target_sigaction *ka,
@@ -3422,8 +3415,7 @@ long do_sigreturn(CPUSH4State *regs)
target_to_host_sigset_internal(&blocked, &target_set);
do_sigprocmask(SIG_SETMASK, &blocked, NULL);
- if (restore_sigcontext(regs, &frame->sc, &r0))
- goto badframe;
+ restore_sigcontext(regs, &frame->sc, &r0);
unlock_user_struct(frame, frame_addr, 0);
return r0;
@@ -3451,8 +3443,7 @@ long do_rt_sigreturn(CPUSH4State *regs)
target_to_host_sigset(&blocked, &frame->uc.tuc_sigmask);
do_sigprocmask(SIG_SETMASK, &blocked, NULL);
- if (restore_sigcontext(regs, &frame->uc.tuc_mcontext, &r0))
- goto badframe;
+ restore_sigcontext(regs, &frame->uc.tuc_mcontext, &r0);
if (do_sigaltstack(frame_addr +
offsetof(struct target_rt_sigframe, uc.tuc_stack),
@@ -5086,10 +5077,9 @@ static void setup_sigcontext(struct target_sigcontext *sc, CPUM68KState *env,
__put_user(env->pc, &sc->sc_pc);
}
-static int
+static void
restore_sigcontext(CPUM68KState *env, struct target_sigcontext *sc, int *pd0)
{
- int err = 0;
int temp;
__get_user(env->aregs[7], &sc->sc_usp);
@@ -5101,8 +5091,6 @@ restore_sigcontext(CPUM68KState *env, struct target_sigcontext *sc, int *pd0)
env->sr = (env->sr & 0xff00) | (temp & 0xff);
*pd0 = tswapl(sc->sc_d0);
-
- return err;
}
/*
@@ -5343,8 +5331,7 @@ long do_sigreturn(CPUM68KState *env)
/* restore registers */
- if (restore_sigcontext(env, &frame->sc, &d0))
- goto badframe;
+ restore_sigcontext(env, &frame->sc, &d0);
unlock_user_struct(frame, frame_addr, 0);
return d0;
@@ -5462,11 +5449,11 @@ static void setup_sigcontext(struct target_sigcontext *sc, CPUAlphaState *env,
__put_user(0, &sc->sc_traparg_a2); /* FIXME */
}
-static int restore_sigcontext(CPUAlphaState *env,
+static void restore_sigcontext(CPUAlphaState *env,
struct target_sigcontext *sc)
{
uint64_t fpcr;
- int i, err = 0;
+ int i;
__get_user(env->pc, &sc->sc_pc);
@@ -5479,8 +5466,6 @@ static int restore_sigcontext(CPUAlphaState *env,
__get_user(fpcr, &sc->sc_fpcr);
cpu_alpha_store_fpcr(env, fpcr);
-
- return err;
}
static inline abi_ulong get_sigframe(struct target_sigaction *sa,
@@ -5614,9 +5599,7 @@ long do_sigreturn(CPUAlphaState *env)
target_to_host_sigset_internal(&set, &target_set);
do_sigprocmask(SIG_SETMASK, &set, NULL);
- if (restore_sigcontext(env, sc)) {
- goto badframe;
- }
+ restore_sigcontext(env, sc);
unlock_user_struct(sc, sc_addr, 0);
return env->ir[IR_V0];
@@ -5637,9 +5620,7 @@ long do_rt_sigreturn(CPUAlphaState *env)
target_to_host_sigset(&set, &frame->uc.tuc_sigmask);
do_sigprocmask(SIG_SETMASK, &set, NULL);
- if (restore_sigcontext(env, &frame->uc.tuc_mcontext)) {
- goto badframe;
- }
+ restore_sigcontext(env, &frame->uc.tuc_mcontext);
if (do_sigaltstack(frame_addr + offsetof(struct target_rt_sigframe,
uc.tuc_stack),
0, env->ir[IR_SP]) == -EFAULT) {
--
2.0.0.rc2
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-06-09 12:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-06 9:46 [Qemu-devel] [PATCH v2 05/13] signal/all: remove return value from restore_sigcontext riku.voipio
2014-06-07 21:30 ` Peter Maydell
2014-06-09 12:13 ` [Qemu-devel] [PATCH v3] " riku.voipio
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).