Re: [Qemu-devel] [PATCH] target/m68k: add a mechanism to automatically free TCGv

[Laurent Vivier]

Le 19/03/2018 à 09:39, Richard Henderson a écrit :
> On 03/19/2018 12:12 AM, Laurent Vivier wrote:
>> SRC_EA() and gen_extend() can return either a temporary
>> TCGv or a memory allocated one. Mark them when they are
>> allocated, and free them automatically at end of the
>> instruction translation.
>>
>> We want to free locally allocated TCGv to avoid
>> overflow in sequence like:
>>
>>   0xc00ae406:  movel %fp@(-132),%fp@(-268)
>>   0xc00ae40c:  movel %fp@(-128),%fp@(-264)
>>   0xc00ae412:  movel %fp@(-20),%fp@(-212)
>>   0xc00ae418:  movel %fp@(-16),%fp@(-208)
>>   0xc00ae41e:  movel %fp@(-60),%fp@(-220)
>>   0xc00ae424:  movel %fp@(-56),%fp@(-216)
>>   0xc00ae42a:  movel %fp@(-124),%fp@(-252)
>>   0xc00ae430:  movel %fp@(-120),%fp@(-248)
>>   0xc00ae436:  movel %fp@(-12),%fp@(-260)
>>   0xc00ae43c:  movel %fp@(-8),%fp@(-256)
>>   0xc00ae442:  movel %fp@(-52),%fp@(-276)
>>   0xc00ae448:  movel %fp@(-48),%fp@(-272)
>>   ...
>>
>> That can fill a lot of TCGv entries in a sequence,
>> especially since 15fa08f845 ("tcg: Dynamically allocate TCGOps")
>> we have no limit to fill the TCGOps cache and we can fill
>> the entire TCG variables array and overflow it.
>>
>> Suggested-by: Richard Henderson <address@hidden>
>> Signed-off-by: Laurent Vivier <address@hidden>
>> ---
> 
> This is a good start.  It's hard to see all of where else might have been
> missed; at least the call to gen_load in gen_lea_indexed.
> 
> For next development cycle it would be good to convert the translator loop and
> enable TCGv leak detection.

I agree.

> That said, this looks good so far.
> Reviewed-by: Richard Henderson <address@hidden>

I'm going to update the patch by splitting it in two as Philippe asked
and mark the missing gen_load() in gen_lea_indexed().

Thanks,
Laurent