Re: [Qemu-devel] [PATCH v2][RESEND] target-arm: cleanup internal resource leaks

[Juha.Riihimaki]

On Oct 26, 2009, at 12:46, ext Laurent Desnogues wrote:

>> @@ -5511,8 +5539,9 @@ static int disas_neon_data_insn(CPUState *  
>> env, DisasContext *s, uint32_t insn)
>>                    tcg_gen_movi_i32(tmp, 0);
>>                }
>>                tmp2 = neon_load_reg(rm, 0);
>> -                gen_helper_neon_tbl(tmp2, tmp2, tmp, tcg_const_i32 
>> (rn),
>> -                                    tcg_const_i32(n));
>> +                tmp4 = tcg_const_i32(rn);
>> +                tmp5 = tcg_const_i32(n);
>> +                gen_helper_neon_tbl(tmp2, tmp2, tmp, tmp4, tmp5);
>>                dead_tmp(tmp);
>>                if (insn & (1 << 6)) {
>>                    tmp = neon_load_reg(rd, 1);
>> @@ -5521,8 +5550,9 @@ static int disas_neon_data_insn(CPUState *  
>> env, DisasContext *s, uint32_t insn)
>>                    tcg_gen_movi_i32(tmp, 0);
>>                }
>>                tmp3 = neon_load_reg(rm, 1);
>> -                gen_helper_neon_tbl(tmp3, tmp3, tmp, tcg_const_i32 
>> (rn),
>> -                                    tcg_const_i32(n));
>> +                gen_helper_neon_tbl(tmp3, tmp3, tmp, tmp4, tmp5);
>> +                dead_tmp(tmp5);
>> +                dead_tmp(tmp4);
>
> I missed this mistake when I acked the patch:  you're using
> dead_tmp instead of tcg_temp_free...  One more reason
> to drop dead_tmp :-)

Indeed. I'll fix it, didn't notice it either because I'm using other  
new_xxx/dead_xxx helpers myself to catch resource leaks. I should work  
around that to get more similar code and eliminate this kind of issues.

The bug will cause false resource leak reports, the actual freeing of  
the temporary is done correctly. Since this patch seems to have  
already been applied, I'll just provide a patch that fixes this  
specific error in the mainline instead of reposting a fixed version of  
the original patch.


Regards,
Juha