Re: [Qemu-arm] [PATCH 2/2] target-arm: Fix an exception return on AArch32 instruction ADDS

[Peter Maydell]

On 19 April 2016 at 19:07, Sergey Sorokin <address@hidden> wrote:
> In AArch32 instruction ADDS r15, ... is used for exception return.
>
> Signed-off-by: Sergey Sorokin <address@hidden>
> ---
>  target-arm/translate.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 68671b7..3e64ba9 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -8512,12 +8512,21 @@ static void disas_arm_insn(DisasContext *s, unsigned 
> int insn)
>              store_reg_bx(s, rd, tmp);
>              break;
>          case 0x04:
> -            if (set_cc) {
> +            if (set_cc && rd == 15) {
> +                /* ADDS r15, ... is used for exception return. */
> +                if (IS_USER(s)) {
> +                    goto illegal_op;
> +                }
>                  gen_add_CC(tmp, tmp, tmp2);
> +                gen_exception_return(s, tmp);
>              } else {
> -                tcg_gen_add_i32(tmp, tmp, tmp2);
> +                if (set_cc) {
> +                    gen_add_CC(tmp, tmp, tmp2);
> +                } else {
> +                    tcg_gen_add_i32(tmp, tmp, tmp2);
> +                }
> +                store_reg_bx(s, rd, tmp);
>              }
> -            store_reg_bx(s, rd, tmp);
>              break;
>          case 0x05:
>              if (set_cc) {

>From my reading of the ARM ARM, this "if set_cc and rd == 15 then exception
return" behaviour should apply to more of these ops, not just 0x02 (SUB),
0x04 (ADD) and 0x0d (MOV). RSB, ADC, SBC, RSC, AND, BIC, EOR all have
exception return behaviour too. That suggests that perhaps we should be
handling the gen_exception_return() at a level further out than this
case statement. That would also allow us to do the IS_USER() check
before we emit any TCG ops, which avoids spurious complaints about TCG
temporary leaks.

thanks
-- PMM