| Message ID | 572C6DA9.9080807@suse.cz |
|---|---|
| State | New |
| Headers | show |
On Fri, May 6, 2016 at 12:10 PM, Martin Liška <mliska@suse.cz> wrote: > Hi. > > I've spotted couple of occurrences of following memory leak seen by valgrind: > > malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) > operator new(unsigned long) (new_op.cc:50) > remap_dependence_clique(copy_body_data*, unsigned short) (tree-inline.c:845) > remap_gimple_op_r(tree_node**, int*, void*) (tree-inline.c:954) > walk_tree_1(tree_node**, tree_node* (*)(tree_node**, int*, void*), void*, hash_set<tree_node*, default_hash_traits<tree_node*> >*, tree_node* (*)(tree_node**, int*, tree_node* (*)(tree_node**, int*, void*), void*, hash_set<tree_node*, default_hash_traits<tree_node*> >*)) (tree.c:11498) > walk_tree_1(tree_node**, tree_node* (*)(tree_node**, int*, void*), void*, hash_set<tree_node*, default_hash_traits<tree_node*> >*, tree_node* (*)(tree_node**, int*, tree_node* (*)(tree_node**, int*, void*), void*, hash_set<tree_node*, default_hash_traits<tree_node*> >*)) (tree.c:11815) > walk_tree_1(tree_node**, tree_node* (*)(tree_node**, int*, void*), void*, hash_set<tree_node*, default_hash_traits<tree_node*> >*, tree_node* (*)(tree_node**, int*, tree_node* (*)(tree_node**, int*, void*), void*, hash_set<tree_node*, default_hash_traits<tree_node*> >*)) (tree.c:11815) > walk_tree_1(tree_node**, tree_node* (*)(tree_node**, int*, void*), void*, hash_set<tree_node*, default_hash_traits<tree_node*> >*, tree_node* (*)(tree_node**, int*, tree_node* (*)(tree_node**, int*, void*), void*, hash_set<tree_node*, default_hash_traits<tree_node*> >*)) (tree.c:11815) > copy_debug_stmt (tree-inline.c:2869) > copy_debug_stmts (tree-inline.c:2927) > copy_body(copy_body_data*, long, int, basic_block_def*, basic_block_def*, basic_block_def*) (tree-inline.c:2961) > tree_function_versioning(tree_node*, tree_node*, vec<ipa_replace_map*, va_gc, vl_embed>*, bool, bitmap_head*, bool, bitmap_head*, basic_block_def*) (tree-inline.c:5907) > save_inline_function_body (ipa-inline-transform.c:485) > inline_transform(cgraph_node*) (ipa-inline-transform.c:541) > > Problem is that the id->dependence_map is released before copy_debug_stmts is called. > > Patch can bootstrap and survives regression tests on x86_64-linux-gnu. > Ready for trunk? Hmmm. But this means debug stmt remapping calls remap_dependence_clique which may end up bumping cfun->last_clique and thus may change code generation. So what debug stmts contain MEM_REFs? If you put an assert processing_debug_stmt == 0 in remap_dependence_clique I'd like to see a testcase that triggers it. Richard. > Martin
On 05/06/2016 12:56 PM, Richard Biener wrote: > Hmmm. But this means debug stmt remapping calls > remap_dependence_clique which may end up bumping > cfun->last_clique and thus may change code generation. > > So what debug stmts contain MEM_REFs? If you put an assert > processing_debug_stmt == 0 in > remap_dependence_clique I'd like to see a testcase that triggers it. > > Richard. Ok, I've placed the suggested assert which is triggered for following debug statement: (gdb) p debug_gimple_stmt(stmt) # DEBUG D#21 => a_1(D)->dim[0].ubound (gdb) p debug_tree(*tp) <mem_ref 0x7ffff66c1b40 type <record_type 0x7ffff6a642a0 array1_unknown type_1 BLK size <integer_cst 0x7ffff6a46180 constant 384> unit size <integer_cst 0x7ffff6a23c90 constant 48> align 64 symtab -160828560 alias set -1 canonical type 0x7ffff6a4f000 fields <field_decl 0x7ffff6a47980 data type <pointer_type 0x7ffff68a7348> unsigned DI file /home/marxin/Programming/gcc/gcc/testsuite/gfortran.dg/actual_array_constructor_1.f90 line 21 col 0 size <integer_cst 0x7ffff6886bd0 constant 64> unit size <integer_cst 0x7ffff6886be8 constant 8> align 64 offset_align 128 offset <integer_cst 0x7ffff6886c00 constant 0> bit offset <integer_cst 0x7ffff6886c48 constant 0> context <record_type 0x7ffff6a4f498 array_descriptor1> chain <field_decl 0x7ffff6a47a18 offset>> pointer_to_this <pointer_type 0x7ffff6a64540> reference_to_this <reference_type 0x7ffff6a645e8> chain <type_decl 0x7ffff6a511c8 D.3431>> arg 0 <ssa_name 0x7ffff66b41f8 type <reference_type 0x7ffff6a64690 type <record_type 0x7ffff6a642a0 array1_unknown> public unsigned restrict DI size <integer_cst 0x7ffff6886bd0 64> unit size <integer_cst 0x7ffff6886be8 8> align 64 symtab 0 alias set -1 canonical type 0x7ffff6a53150> var <parm_decl 0x7ffff6696800 a>def_stmt GIMPLE_NOP version 1> arg 1 <integer_cst 0x7ffff6a7a438 type <reference_type 0x7ffff6a64690> constant 0>> for the following test-case: gfortran /home/marxin/Programming/gcc/gcc/testsuite/gfortran.dg/actual_array_constructor_1.f90 -O3 -g Martin
From c67fe154a24b939edad28a823c2ecb16f6e056c1 Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Wed, 6 Jan 2016 11:41:31 +0100 Subject: [PATCH] Properly release memory in copy_body gcc/ChangeLog: 2016-01-06 Martin Liska <mliska@suse.cz> * tree-inline.c (copy_cfg_body): Remove eh_map deletion. (copy_body): Release it here. --- gcc/tree-inline.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c index 19f202e..92169e6 100644 --- a/gcc/tree-inline.c +++ b/gcc/tree-inline.c @@ -2807,17 +2807,6 @@ copy_cfg_body (copy_body_data * id, gcov_type count, int frequency_scale, entry_block_map->aux = NULL; exit_block_map->aux = NULL; - if (id->eh_map) - { - delete id->eh_map; - id->eh_map = NULL; - } - if (id->dependence_map) - { - delete id->dependence_map; - id->dependence_map = NULL; - } - return new_fndecl; } @@ -2963,6 +2952,17 @@ copy_body (copy_body_data *id, gcov_type count, int frequency_scale, new_entry); copy_debug_stmts (id); + if (id->eh_map) + { + delete id->eh_map; + id->eh_map = NULL; + } + if (id->dependence_map) + { + delete id->dependence_map; + id->dependence_map = NULL; + } + return body; } -- 2.8.1