Re: [Qemu-devel] [PATCH] target/arm: Use v7m_stack_read() for reading the frame signature

[Philippe Mathieu-Daudé]

On 04/19/2018 11:21 AM, Peter Maydell wrote:
> In commit 95695effe8caa552b8f2 we changed the v7M/v8M stack
> pop code to use a new v7m_stack_read() function that checks
> whether the read should fail due to an MPU or bus abort.
> We missed one call though, the one which reads the signature
> word for the callee-saved register part of the frame.

Good catch.

> Correct the omission.

I checked, this was the unique omission.

> Signed-off-by: Peter Maydell <address@hidden>

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

> ---
>  target/arm/helper.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index b14fdab140..2ebd086ef2 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -6913,7 +6913,6 @@ static bool v7m_push_stack(ARMCPU *cpu)
>  static void do_v7m_exception_exit(ARMCPU *cpu)
>  {
>      CPUARMState *env = &cpu->env;
> -    CPUState *cs = CPU(cpu);
>      uint32_t excret;
>      uint32_t xpsr;
>      bool ufault = false;
> @@ -7112,9 +7111,11 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
>              ((excret & R_V7M_EXCRET_ES_MASK) == 0 ||
>               (excret & R_V7M_EXCRET_DCRS_MASK) == 0)) {
>              uint32_t expected_sig = 0xfefa125b;
> -            uint32_t actual_sig = ldl_phys(cs->as, frameptr);
> +            uint32_t actual_sig;
>  
> -            if (expected_sig != actual_sig) {
> +            pop_ok = v7m_stack_read(cpu, &actual_sig, frameptr, mmu_idx);
> +
> +            if (pop_ok && expected_sig != actual_sig) {
>                  /* Take a SecureFault on the current stack */
>                  env->v7m.sfsr |= R_V7M_SFSR_INVIS_MASK;
>                  armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SECURE, 
> false);
> @@ -7125,7 +7126,7 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
>                  return;
>              }
>  
> -            pop_ok =
> +            pop_ok = pop_ok &&
>                  v7m_stack_read(cpu, &env->regs[4], frameptr + 0x8, mmu_idx) 
> &&
>                  v7m_stack_read(cpu, &env->regs[4], frameptr + 0x8, mmu_idx) 
> &&
>                  v7m_stack_read(cpu, &env->regs[5], frameptr + 0xc, mmu_idx) 
> &&
>