| Message ID | 20250401144332.41615-1-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [PATCH-for-10.0] tcg: Allocate TEMP_VAL_MEM frame in temp_load() | expand |
On 4/1/25 09:43, Philippe Mathieu-Daudé wrote: > Be sure to allocate the temp frame if it wasn't. > > Fixes: c896fe29d6c ("TCG code generator") > Reported-by: Michael Tokarev <mjt@tls.msk.ru> > Reported-by: Helge Konetzka <hk@zapateado.de> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2891 > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2899 > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > tcg/tcg.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/tcg/tcg.c b/tcg/tcg.c > index e8950df2ad3..dfd48b82642 100644 > --- a/tcg/tcg.c > +++ b/tcg/tcg.c > @@ -4671,6 +4671,9 @@ static void temp_load(TCGContext *s, TCGTemp *ts, TCGRegSet desired_regs, > ts->mem_coherent = 0; > break; > case TEMP_VAL_MEM: > + if (!ts->mem_allocated) { > + temp_allocate_frame(s, ts); > + } > reg = tcg_reg_alloc(s, desired_regs, allocated_regs, > preferred_regs, ts->indirect_base); > tcg_out_ld(s, ts->type, reg, ts->mem_base->reg, ts->mem_offset); I suspect this is a read from a temporary that is uninitialized. Ordinarily the stack slot would have been allocated by the store. I guess I should have a look at the testcase... r~
On 4/1/25 10:02, Richard Henderson wrote: > On 4/1/25 09:43, Philippe Mathieu-Daudé wrote: >> Be sure to allocate the temp frame if it wasn't. >> >> Fixes: c896fe29d6c ("TCG code generator") >> Reported-by: Michael Tokarev <mjt@tls.msk.ru> >> Reported-by: Helge Konetzka <hk@zapateado.de> >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2891 >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2899 >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> tcg/tcg.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/tcg/tcg.c b/tcg/tcg.c >> index e8950df2ad3..dfd48b82642 100644 >> --- a/tcg/tcg.c >> +++ b/tcg/tcg.c >> @@ -4671,6 +4671,9 @@ static void temp_load(TCGContext *s, TCGTemp *ts, TCGRegSet >> desired_regs, >> ts->mem_coherent = 0; >> break; >> case TEMP_VAL_MEM: >> + if (!ts->mem_allocated) { >> + temp_allocate_frame(s, ts); >> + } >> reg = tcg_reg_alloc(s, desired_regs, allocated_regs, >> preferred_regs, ts->indirect_base); >> tcg_out_ld(s, ts->type, reg, ts->mem_base->reg, ts->mem_offset); > > I suspect this is a read from a temporary that is uninitialized. Ordinarily the stack > slot would have been allocated by the store. > > I guess I should have a look at the testcase... Interesting. This is a case of incomplete dead code elimination: the store was eliminated and the load *should* have been eliminated. In any case, the uninitialized load isn't actually reachable, so all we need to do is not crash. For 10.0, Reviewed-by: Richard Henderson <richard.henderson@linaro.org> For 10.1, we should probably fix the dead code elimination issue. r~
diff --git a/tcg/tcg.c b/tcg/tcg.c index e8950df2ad3..dfd48b82642 100644 --- a/tcg/tcg.c +++ b/tcg/tcg.c @@ -4671,6 +4671,9 @@ static void temp_load(TCGContext *s, TCGTemp *ts, TCGRegSet desired_regs, ts->mem_coherent = 0; break; case TEMP_VAL_MEM: + if (!ts->mem_allocated) { + temp_allocate_frame(s, ts); + } reg = tcg_reg_alloc(s, desired_regs, allocated_regs, preferred_regs, ts->indirect_base); tcg_out_ld(s, ts->type, reg, ts->mem_base->reg, ts->mem_offset);
Be sure to allocate the temp frame if it wasn't. Fixes: c896fe29d6c ("TCG code generator") Reported-by: Michael Tokarev <mjt@tls.msk.ru> Reported-by: Helge Konetzka <hk@zapateado.de> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2891 Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2899 Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- tcg/tcg.c | 3 +++ 1 file changed, 3 insertions(+)