Re: [PATCH v2] Properly sign extend BBIT branch offset during calculation

[Richard Henderson]

On 10/13/22 15:08, Christopher Wrogg wrote:
> The Octeon specific BBIT instruction incorrectly computes
> the branch offset. The 16 bit value is not sign extended.
>
> Signed-off-by: Christopher Wrogg <cwrogg@umich.edu <mailto:cwrogg@umich.edu>>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1251 
> <https://gitlab.com/qemu-project/qemu/-/issues/1251>
> ---
>   target/mips/tcg/octeon_translate.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/target/mips/tcg/octeon_translate.c 
> b/target/mips/tcg/octeon_translate.c
> index 6a207d2e7e..90f7b105cb 100644
> --- a/target/mips/tcg/octeon_translate.c
> +++ b/target/mips/tcg/octeon_translate.c
> @@ -38,7 +38,10 @@ static bool trans_BBIT(DisasContext *ctx, arg_BBIT *a)
>       }
>
>       ctx->hflags |= MIPS_HFLAG_BC;
> -    ctx->btarget = ctx->base.pc_next + 4 + a->offset * 4;
> +    a->offset *= 4;
> +    a->offset = (target_long)(int16_t)a->offset;
> +    ctx->btarget = ctx->base.pc_next + 4 + a->offset;

This looks wrong, because you're sign-extending after scaling, which gives you only 14 
bits of offset instead of 16.

The correct fix should be earlier in decode:

- BBIT         11 set:1 . 10 rs:5 ..... offset:16 p=%bbit_p
+ BBIT         11 set:1 . 10 rs:5 ..... offset:s16 p=%bbit_p

to indicate a extract a signed field from the instruction.


r~