We need values 0-3 for TCG_TYPE_I128 on 32-bit hosts.

Cc: qemu-stable@nongnu.org
Fixes: 43eef72f4109 ("tcg: Add temp allocation for TCGv_i128")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2159
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---

I feel certain that I made this change back when I introduced TCGv_i128.
I imagine that something went wrong with a rebase and it got lost.
Worse, we don't use temp_subindex often, and we usually handle i128
this value correctly.  It took a quirk of register allocation ordering
to make an invalid value in temp_subindex lead to a crash.

---
 include/tcg/tcg.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index daf2a5bf9e..451f3fec41 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -412,7 +412,7 @@ typedef struct TCGTemp {
     unsigned int mem_coherent:1;
     unsigned int mem_allocated:1;
     unsigned int temp_allocated:1;
-    unsigned int temp_subindex:1;
+    unsigned int temp_subindex:2;
 
     int64_t val;
     struct TCGTemp *mem_base;
-- 
2.34.1

13.02.2024 05:21, Richard Henderson:
> We need values 0-3 for TCG_TYPE_I128 on 32-bit hosts.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: 43eef72f4109 ("tcg: Add temp allocation for TCGv_i128")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2159
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> 
> I feel certain that I made this change back when I introduced TCGv_i128.
> I imagine that something went wrong with a rebase and it got lost.
> Worse, we don't use temp_subindex often, and we usually handle i128
> this value correctly.  It took a quirk of register allocation ordering
> to make an invalid value in temp_subindex lead to a crash.

Somehow it feels deja vue too, maybe we increased some other width like
this already, in stable series too, but I can't find it now.

"This" in "this value" should be omitted. With this fixed,

Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>

Thank you for the quick fix Richard!

/mjt

> ---
>   include/tcg/tcg.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
> index daf2a5bf9e..451f3fec41 100644
> --- a/include/tcg/tcg.h
> +++ b/include/tcg/tcg.h
> @@ -412,7 +412,7 @@ typedef struct TCGTemp {
>       unsigned int mem_coherent:1;
>       unsigned int mem_allocated:1;
>       unsigned int temp_allocated:1;
> -    unsigned int temp_subindex:1;
> +    unsigned int temp_subindex:2;
>   
>       int64_t val;
>       struct TCGTemp *mem_base;

13.02.2024 08:44, Michael Tokarev wrote:
> 13.02.2024 05:21, Richard Henderson:
>> We need values 0-3 for TCG_TYPE_I128 on 32-bit hosts.
>>
>> Cc: qemu-stable@nongnu.org
>> Fixes: 43eef72f4109 ("tcg: Add temp allocation for TCGv_i128")
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2159
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>
>> I feel certain that I made this change back when I introduced TCGv_i128.
>> I imagine that something went wrong with a rebase and it got lost.
>> Worse, we don't use temp_subindex often, and we usually handle i128
>> this value correctly.  It took a quirk of register allocation ordering
>> to make an invalid value in temp_subindex lead to a crash.
> 
> Somehow it feels deja vue too, maybe we increased some other width like
> this already, in stable series too, but I can't find it now.
> 
> "This" in "this value" should be omitted. With this fixed,

This is that "ENOCOFFEE" time once again ;) - I haven't noticed this
is a comment *after* the patch description.

> Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>

Tested-by: Michael Tokarev <mjt@tls.msk.ru>

I run several different guests here which did show the same crash, -
none of them crashes after this patch.

/mjt

On 13/2/24 03:21, Richard Henderson wrote:
> We need values 0-3 for TCG_TYPE_I128 on 32-bit hosts.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: 43eef72f4109 ("tcg: Add temp allocation for TCGv_i128")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2159
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> 
> I feel certain that I made this change back when I introduced TCGv_i128.
> I imagine that something went wrong with a rebase and it got lost.
> Worse, we don't use temp_subindex often, and we usually handle i128
> this value correctly.  It took a quirk of register allocation ordering
> to make an invalid value in temp_subindex lead to a crash.
> 
> ---
>   include/tcg/tcg.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>