On Fri, May 15, 2020 at 7:34 PM Peter Maydell <peter.mayd...@linaro.org> wrote:
>
> On Thu, 7 May 2020 at 21:25, Amanieu d'Antras <aman...@gmail.com> wrote:
> >
> > This fixes signal handlers running with the wrong endianness if the
> > interrupted code used SETEND to dynamically switch endianness.
> >
> > Signed-off-by: Amanieu d'Antras <aman...@gmail.com>
> > ---
> > linux-user/arm/signal.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/linux-user/arm/signal.c b/linux-user/arm/signal.c
> > index d96fc27ce1..8aca5f61b7 100644
> > --- a/linux-user/arm/signal.c
> > +++ b/linux-user/arm/signal.c
> > @@ -244,6 +244,12 @@ setup_return(CPUARMState *env, struct target_sigaction
> > *ka,
> > } else {
> > cpsr &= ~CPSR_T;
> > }
> > + cpsr &= ~CPSR_E;
> > +#ifdef TARGET_WORDS_BIGENDIAN
> > + if (env->cp15.sctlr_el[1] & SCTLR_E0E) {
> > + cpsr |= CPSR_E;
> > + }
> > +#endif
> >
> > if (ka->sa_flags & TARGET_SA_RESTORER) {
> > if (is_fdpic) {
> > @@ -287,7 +293,8 @@ setup_return(CPUARMState *env, struct target_sigaction
> > *ka,
> > env->regs[13] = frame_addr;
> > env->regs[14] = retcode;
> > env->regs[15] = handler & (thumb ? ~1 : ~3);
> > - cpsr_write(env, cpsr, CPSR_IT | CPSR_T, CPSRWriteByInstr);
> > + cpsr_write(env, cpsr, CPSR_IT | CPSR_T | CPSR_E, CPSRWriteByInstr);
> > + arm_rebuild_hflags(env);
>
> I was just looking at the signal code's handling of CPSR for a different
> reason, and I noticed that at the moment we don't allow CPSR.E to be
> updated from the signal frame when the signal handler returns
> (because CPSR_USER doesn't contain CPSR_E and that's what we
> use in restore_sigcontext() to define what bits from the frame we
> allow updating). Don't you find that when the interrupted code
> returns from the signal handler that it ends up running with the
> wrong endianness (ie the endianness the handler used) ?
I actually found this while trying to test the SETEND instruction
under risu. The signal handler was crashing because it loaded a
pointer with the wrong endianness, which was pretty obvious. However I
missed the fact that code was now running with the wrong endianness
after returning from the signal handler since I had both the master
and the apprentice running under qemu-arm.
> I'm going to fix this by putting CPSR_E in CPSR_USER, anyway.
You also need to call arm_rebuild_hflags() after modifying CPSR_E
otherwise the change doesn't take effect.
-- Amanieu
