If a signal is delivered during the execution of a delay slot,
or a gUSA region, clear those bits from the environment so that
the signal handler does not start in that same state.

Cleaning the bits on signal return is paranoid good sense.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 linux-user/signal.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index a537778..8c0b851 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -3544,6 +3544,7 @@ static void restore_sigcontext(CPUSH4State *regs, struct target_sigcontext *sc)
     __get_user(regs->fpul, &sc->sc_fpul);
 
     regs->tra = -1;         /* disable syscall checks */
+    regs->flags &= ~(DELAY_SLOT_MASK | GUSA_MASK);
 }
 
 static void setup_frame(int sig, struct target_sigaction *ka,
@@ -3587,6 +3588,7 @@ static void setup_frame(int sig, struct target_sigaction *ka,
     regs->gregs[5] = 0;
     regs->gregs[6] = frame_addr += offsetof(typeof(*frame), sc);
     regs->pc = (unsigned long) ka->_sa_handler;
+    regs->flags &= ~(DELAY_SLOT_MASK | GUSA_MASK);
 
     unlock_user_struct(frame, frame_addr, 1);
     return;
@@ -3649,6 +3651,7 @@ static void setup_rt_frame(int sig, struct target_sigaction *ka,
     regs->gregs[5] = frame_addr + offsetof(typeof(*frame), info);
     regs->gregs[6] = frame_addr + offsetof(typeof(*frame), uc);
     regs->pc = (unsigned long) ka->_sa_handler;
+    regs->flags &= ~(DELAY_SLOT_MASK | GUSA_MASK);
 
     unlock_user_struct(frame, frame_addr, 1);
     return;
-- 
2.9.4

On 2017-07-06 16:20, Richard Henderson wrote:
> If a signal is delivered during the execution of a delay slot,
> or a gUSA region, clear those bits from the environment so that
> the signal handler does not start in that same state.

How are signals delivered in linux-user? At least in system mode we
forbid interrupts in the delay slot (see commit 5c6f3eb7db), as the
manual clearly declare them as indivisible. Maybe the same should be
done for linux-user?

> 
> Cleaning the bits on signal return is paranoid good sense.
> 
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  linux-user/signal.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index a537778..8c0b851 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -3544,6 +3544,7 @@ static void restore_sigcontext(CPUSH4State *regs, struct target_sigcontext *sc)
>      __get_user(regs->fpul, &sc->sc_fpul);
>  
>      regs->tra = -1;         /* disable syscall checks */
> +    regs->flags &= ~(DELAY_SLOT_MASK | GUSA_MASK);
>  }
>  
>  static void setup_frame(int sig, struct target_sigaction *ka,

Why not using TB_FLAG_ENVFLAGS_MASK introduced earlier in this patch
series?

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

On 07/15/2017 12:59 PM, Aurelien Jarno wrote:
> On 2017-07-06 16:20, Richard Henderson wrote:
>> If a signal is delivered during the execution of a delay slot,
>> or a gUSA region, clear those bits from the environment so that
>> the signal handler does not start in that same state.
> 
> How are signals delivered in linux-user? At least in system mode we
> forbid interrupts in the delay slot (see commit 5c6f3eb7db), as the
> manual clearly declare them as indivisible. Maybe the same should be
> done for linux-user?

Signals get queued, and delivered eventually.  I don't believe that we do 
anything to check that "signals can't be delivered yet" like we do in system mode.

>> +    regs->flags &= ~(DELAY_SLOT_MASK | GUSA_MASK);
>>   }
>>   
>>   static void setup_frame(int sig, struct target_sigaction *ka,
> 
> Why not using TB_FLAG_ENVFLAGS_MASK introduced earlier in this patch
> series?

I really want to clear these two sets.  I didn't want to assume that 
ENVFLAGS_MASK would never contain anything else.


r~

On 2017-07-15 16:33, Richard Henderson wrote:
> On 07/15/2017 12:59 PM, Aurelien Jarno wrote:
> > On 2017-07-06 16:20, Richard Henderson wrote:
> > > If a signal is delivered during the execution of a delay slot,
> > > or a gUSA region, clear those bits from the environment so that
> > > the signal handler does not start in that same state.
> > 
> > How are signals delivered in linux-user? At least in system mode we
> > forbid interrupts in the delay slot (see commit 5c6f3eb7db), as the
> > manual clearly declare them as indivisible. Maybe the same should be
> > done for linux-user?
> 
> Signals get queued, and delivered eventually.  I don't believe that we do
> anything to check that "signals can't be delivered yet" like we do in system
> mode.

Ok. I think it might be a good idea to implement that. I am not always
sure that running the signal handler between an instruction and the
corresponding delay slot will lead to correct behaviour.

> > > +    regs->flags &= ~(DELAY_SLOT_MASK | GUSA_MASK);
> > >   }
> > >   static void setup_frame(int sig, struct target_sigaction *ka,
> > 
> > Why not using TB_FLAG_ENVFLAGS_MASK introduced earlier in this patch
> > series?
> 
> I really want to clear these two sets.  I didn't want to assume that
> ENVFLAGS_MASK would never contain anything else.
> 

Fair enough. Therefore:

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net