[PATCH 15/21] PR jit/63854: lra.c: Fix leak of point_freq_vec's buffer when calling lra_inheritance
Valgrind showed a leak of 1640 bytes per iteration of one of the jit testcases (I think this is per-function in a compile): 8,200 bytes in 5 blocks are definitely lost in loss record 214 of 223 at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x5D75D1F: xrealloc (xmalloc.c:177) by 0x4E0C94E: void va_heap::reserveint(vecint, va_heap, vl_embed*, unsigned int, bool) (vec.h:310) by 0x4E0C7A3: vecint, va_heap, vl_ptr::reserve(unsigned int, bool) (vec.h:1428) by 0x4E0C55A: vecint, va_heap, vl_ptr::reserve_exact(unsigned int) (vec.h:1448) by 0x4E0C2C6: vecint, va_heap, vl_ptr::create(unsigned int) (vec.h:1463) by 0x519B316: lra_create_live_ranges(bool) (lra-lives.c:1248) by 0x5179BD7: lra(_IO_FILE*) (lra.c:2297) by 0x5126C4E: do_reload() (ira.c:5380) by 0x5127098: (anonymous namespace)::pass_reload::execute(function*) (ira.c:5550) by 0x52235BD: execute_one_pass(opt_pass*) (passes.c:2306) by 0x5223834: execute_pass_list_1(opt_pass*) (passes.c:2358) which is this line: point_freq_vec.create (get_max_uid () * 2); point_freq_vec's buffer is released in lra_clear_live_ranges. Am unfamiliar with LRA, but my reading of the lra code is that the live_p boolean tracks whether or not we need to call lra_clear_live_ranges. Debugging calls to the above line and calls to lra_clear_live_ranges showed a mismatch; the call to lra_create_live_ranges before lra_inheritance wasn't setting live_p. Setting it after that callsite fixes the leak (though perhaps the code could be cleaned up by having live_p be set/cleared by the allocation/clear functions themselves, rather than by their callers? Vladimir?) gcc/ChangeLog: PR jit/63854 * lra.c (lra): After creating live ranges in preparation for call to lra_inheritance, set live_p to true. --- gcc/lra.c | 1 + 1 file changed, 1 insertion(+) diff --git a/gcc/lra.c b/gcc/lra.c index 9309d5e..ec122c7 100644 --- a/gcc/lra.c +++ b/gcc/lra.c @@ -2297,6 +2297,7 @@ lra (FILE *f) actual_call_used_reg_set, which is needed during lra_inheritance. */ lra_create_live_ranges (true, true); + live_p = true; } lra_inheritance (); } -- 1.8.5.3
Re: [PATCH 15/21] PR jit/63854: lra.c: Fix leak of point_freq_vec's buffer when calling lra_inheritance
On 11/19/14 03:46, David Malcolm wrote: Valgrind showed a leak of 1640 bytes per iteration of one of the jit testcases (I think this is per-function in a compile): 8,200 bytes in 5 blocks are definitely lost in loss record 214 of 223 at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x5D75D1F: xrealloc (xmalloc.c:177) by 0x4E0C94E: void va_heap::reserveint(vecint, va_heap, vl_embed*, unsigned int, bool) (vec.h:310) by 0x4E0C7A3: vecint, va_heap, vl_ptr::reserve(unsigned int, bool) (vec.h:1428) by 0x4E0C55A: vecint, va_heap, vl_ptr::reserve_exact(unsigned int) (vec.h:1448) by 0x4E0C2C6: vecint, va_heap, vl_ptr::create(unsigned int) (vec.h:1463) by 0x519B316: lra_create_live_ranges(bool) (lra-lives.c:1248) by 0x5179BD7: lra(_IO_FILE*) (lra.c:2297) by 0x5126C4E: do_reload() (ira.c:5380) by 0x5127098: (anonymous namespace)::pass_reload::execute(function*) (ira.c:5550) by 0x52235BD: execute_one_pass(opt_pass*) (passes.c:2306) by 0x5223834: execute_pass_list_1(opt_pass*) (passes.c:2358) which is this line: point_freq_vec.create (get_max_uid () * 2); point_freq_vec's buffer is released in lra_clear_live_ranges. Am unfamiliar with LRA, but my reading of the lra code is that the live_p boolean tracks whether or not we need to call lra_clear_live_ranges. Debugging calls to the above line and calls to lra_clear_live_ranges showed a mismatch; the call to lra_create_live_ranges before lra_inheritance wasn't setting live_p. Setting it after that callsite fixes the leak (though perhaps the code could be cleaned up by having live_p be set/cleared by the allocation/clear functions themselves, rather than by their callers? Vladimir?) gcc/ChangeLog: PR jit/63854 * lra.c (lra): After creating live ranges in preparation for call to lra_inheritance, set live_p to true. OK for the trunk. And yes, a followup where live_p's handling moves into lra_create_live_ranges or something similar would be good. Jeff
