| Message ID | 20240213022132.116383-1-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tcg: Increase width of temp_subindex | expand |
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>
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
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;
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(-)