Re: [PATCH 02/23] target/i386: fix gen_prepare_size_nz condition

[Richard Henderson <richard.henderson@linaro.org>]

On 6/28/24 05:42, Alex Bennée wrote:
> Incorrect brace positions causes an unintended overflow on 32 bit
> builds and shenanigans result.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2413
> Suggested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   target/i386/tcg/translate.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> index ad1819815a..94f13541c3 100644
> --- a/target/i386/tcg/translate.c
> +++ b/target/i386/tcg/translate.c
> @@ -877,7 +877,7 @@ static CCPrepare gen_prepare_sign_nz(TCGv src, MemOp size)
>           return (CCPrepare) { .cond = TCG_COND_LT, .reg = src };
>       } else {
>           return (CCPrepare) { .cond = TCG_COND_TSTNE, .reg = src,
> -                             .imm = 1ull << ((8 << size) - 1) };
> +                             .imm = (1ull << (8 << size)) - 1 };

This is incorrect -- we want only to test the sign bit.
Perhaps MAKE_64BIT_MASK((8 << size) - 1, 1) would make this more explicit?

I'll have a quick look at the issue and see if I can reproduce.


r~