[PATCH-for-10.0] tcg: Allocate TEMP_VAL_MEM frame in temp_load()

[PATCH-for-10.0] tcg: Allocate TEMP_VAL_MEM frame in temp_load()

[Philippe Mathieu-Daudé]

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);
-- 
2.47.1

Re: [PATCH-for-10.0] tcg: Allocate TEMP_VAL_MEM frame in temp_load()

[Richard Henderson]

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~

Re: [PATCH-for-10.0] tcg: Allocate TEMP_VAL_MEM frame in temp_load()

[Richard Henderson]

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~

Re: [PATCH-for-10.0] tcg: Allocate TEMP_VAL_MEM frame in temp_load()

[Richard Henderson]

On 4/1/25 07: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);

Queued for 10.0.


r~