On Wed, Feb 10, 2021 at 6:16 PM Martin Sebor <mse...@gmail.com> wrote: > > The attached patch replaces calls to print_generic_expr_to_str() with > a helper function that returns a std::string and releases the caller > from the responsibility to explicitly free memory.
I don't like this. What's the reason to use generic_expr_as_string anyway? We have %E pretty-printers after all. Couldn't you have fixed the leak by doing if (newbnd) free (newbndstr); etc.? > With the patch installed, Valgrind shows more leaks in this code that > I'm not sure what to do about: > > 1) A tree built by build_type_attribute_qual_variant() called from > attr_access::array_as_string() to build a temporary type only > for the purposes of formatting it. > > 2) A tree (an attribute list) built by tree_cons() called from > build_attr_access_from_parms() that's used only for the duration > of the caller. > > Do these temporary trees need to be released somehow or are the leaks > expected? You should configure GCC with --enable-valgrind-annotations to make it aware of our GC. > Martin > > Leak 1: > ==7112== 56 bytes in 1 blocks are still reachable in loss record 73 of 691 > ==7112== at 0x483AB1A: calloc (vg_replace_malloc.c:762) > ==7112== by 0x294ED71: xcalloc (xmalloc.c:162) > ==7112== by 0xB8B21D: alloc_page(unsigned int) (ggc-page.c:918) > ==7112== by 0xB8BAD1: ggc_internal_alloc(unsigned long, void > (*)(void*), unsigned long, unsigned long) (ggc-page.c:1294) > ==7112== by 0x9E1F33: ggc_internal_alloc(unsigned long) (ggc.h:130) > ==7112== by 0x198AB1A: ggc_alloc_tree_node_stat(unsigned long) > (ggc.h:309) > ==7112== by 0x193BD18: copy_node(tree_node*) (tree.c:1223) > ==7112== by 0x1953BDD: build_distinct_type_copy(tree_node*) (tree.c:6730) > ==7112== by 0x9A1379: build_type_attribute_qual_variant(tree_node*, > tree_node*, int) (attribs.c:1161) > ==7112== by 0x9A483D: > attr_access::array_as_string[abi:cxx11](tree_node*) const (attribs.c:2332) > ==7112== by 0xB7C5A7: warn_parm_array_mismatch(unsigned int, > tree_node*, tree_node*) (c-warn.c:3449) > ==7112== by 0xA3EB57: c_parser_declaration_or_fndef(c_parser*, bool, > bool, bool, bool, bool, tree_node**, vec<c_token, va_heap, vl_ptr>, > bool, tree_node*, oacc_routine_data*, bool*) (c-parser.c:2342) > > > Leak 2: > ==7112== 64 bytes in 1 blocks are still reachable in loss record 150 of 691 > ==7112== at 0x483AB1A: calloc (vg_replace_malloc.c:762) > ==7112== by 0x294ED71: xcalloc (xmalloc.c:162) > ==7112== by 0xB8B21D: alloc_page(unsigned int) (ggc-page.c:918) > ==7112== by 0xB8BAD1: ggc_internal_alloc(unsigned long, void > (*)(void*), unsigned long, unsigned long) (ggc-page.c:1294) > ==7112== by 0x9E1F33: ggc_internal_alloc(unsigned long) (ggc.h:130) > ==7112== by 0x198AB1A: ggc_alloc_tree_node_stat(unsigned long) > (ggc.h:309) > ==7112== by 0x19433E6: tree_cons(tree_node*, tree_node*, tree_node*) > (tree.c:3331) > ==7112== by 0xB67188: build_attr_access_from_parms(tree_node*, bool) > (c-attribs.c:5060) > ==7112== by 0xB7C008: warn_parm_array_mismatch(unsigned int, > tree_node*, tree_node*) (c-warn.c:3364) > ==7112== by 0xA3EB57: c_parser_declaration_or_fndef(c_parser*, bool, > bool, bool, bool, bool, tree_node**, vec<c_token, va_heap, vl_ptr>, > bool, tree_node*, oacc_routine_data*, bool*) (c-parser.c:2342) > ==7112== by 0xA3D5A9: c_parser_external_declaration(c_parser*) > (c-parser.c:1777) > ==7112== by 0xA3D06A: c_parser_translation_unit(c_parser*) > (c-parser.c:1650)
