From 89be17a1b231ade643f28fbe616d53377e069da8 Mon Sep 17 00:00:00 2001 From: Thomas Schwinge Date: Fri, 13 Aug 2021 18:03:38 +0200 Subject: Fix 'hash_table::expand' to destruct stale Value objects Thus plugging potentional memory leaks if these have non-trivial constructor/destructor. See and others. As one example, compilation of 'g++.dg/warn/Wmismatched-tags.C' per 'valgrind --leak-check=full' improves as follows: [...] - -104 bytes in 1 blocks are definitely lost in loss record 399 of 519 - at 0x483DFAF: realloc (vg_replace_malloc.c:836) - by 0x223B62C: xrealloc (xmalloc.c:179) - by 0xA8D848: void va_heap::reserve(vec*&, unsigned int, bool) (vec.h:290) - by 0xA8B373: vec::reserve(unsigned int, bool) (vec.h:1858) - by 0xA8B277: vec::safe_push(class_decl_loc_t::class_key_loc_t const&) (vec.h:1967) - by 0xA57481: class_decl_loc_t::add_or_diag_mismatched_tag(tree_node*, tag_types, bool, bool) (parser.c:32967) - by 0xA573E1: class_decl_loc_t::add(cp_parser*, unsigned int, tag_types, tree_node*, bool, bool) (parser.c:32941) - by 0xA56C52: cp_parser_check_class_key(cp_parser*, unsigned int, tag_types, tree_node*, bool, bool) (parser.c:32819) - by 0xA3AD12: cp_parser_elaborated_type_specifier(cp_parser*, bool, bool) (parser.c:20227) - by 0xA37EF2: cp_parser_type_specifier(cp_parser*, int, cp_decl_specifier_seq*, bool, int*, bool*) (parser.c:18942) - by 0xA31CDD: cp_parser_decl_specifier_seq(cp_parser*, int, cp_decl_specifier_seq*, int*) (parser.c:15517) - by 0xA43C71: cp_parser_parameter_declaration(cp_parser*, int, bool, bool*) (parser.c:24242) - -168 bytes in 3 blocks are definitely lost in loss record 422 of 519 - at 0x483DFAF: realloc (vg_replace_malloc.c:836) - by 0x223B62C: xrealloc (xmalloc.c:179) - by 0xA8D848: void va_heap::reserve(vec*&, unsigned int, bool) (vec.h:290) - by 0xA8B373: vec::reserve(unsigned int, bool) (vec.h:1858) - by 0xA8B277: vec::safe_push(class_decl_loc_t::class_key_loc_t const&) (vec.h:1967) - by 0xA57481: class_decl_loc_t::add_or_diag_mismatched_tag(tree_node*, tag_types, bool, bool) (parser.c:32967) - by 0xA573E1: class_decl_loc_t::add(cp_parser*, unsigned int, tag_types, tree_node*, bool, bool) (parser.c:32941) - by 0xA56C52: cp_parser_check_class_key(cp_parser*, unsigned int, tag_types, tree_node*, bool, bool) (parser.c:32819) - by 0xA3AD12: cp_parser_elaborated_type_specifier(cp_parser*, bool, bool) (parser.c:20227) - by 0xA37EF2: cp_parser_type_specifier(cp_parser*, int, cp_decl_specifier_seq*, bool, int*, bool*) (parser.c:18942) - by 0xA31CDD: cp_parser_decl_specifier_seq(cp_parser*, int, cp_decl_specifier_seq*, int*) (parser.c:15517) - by 0xA53385: cp_parser_single_declaration(cp_parser*, vec*, bool, bool, bool*) (parser.c:31072) - -488 bytes in 7 blocks are definitely lost in loss record 449 of 519 - at 0x483DFAF: realloc (vg_replace_malloc.c:836) - by 0x223B62C: xrealloc (xmalloc.c:179) - by 0xA8D848: void va_heap::reserve(vec*&, unsigned int, bool) (vec.h:290) - by 0xA8B373: vec::reserve(unsigned int, bool) (vec.h:1858) - by 0xA8B277: vec::safe_push(class_decl_loc_t::class_key_loc_t const&) (vec.h:1967) - by 0xA57481: class_decl_loc_t::add_or_diag_mismatched_tag(tree_node*, tag_types, bool, bool) (parser.c:32967) - by 0xA573E1: class_decl_loc_t::add(cp_parser*, unsigned int, tag_types, tree_node*, bool, bool) (parser.c:32941) - by 0xA56C52: cp_parser_check_class_key(cp_parser*, unsigned int, tag_types, tree_node*, bool, bool) (parser.c:32819) - by 0xA3AD12: cp_parser_elaborated_type_specifier(cp_parser*, bool, bool) (parser.c:20227) - by 0xA37EF2: cp_parser_type_specifier(cp_parser*, int, cp_decl_specifier_seq*, bool, int*, bool*) (parser.c:18942) - by 0xA31CDD: cp_parser_decl_specifier_seq(cp_parser*, int, cp_decl_specifier_seq*, int*) (parser.c:15517) - by 0xA49508: cp_parser_member_declaration(cp_parser*) (parser.c:26440) - -728 bytes in 7 blocks are definitely lost in loss record 455 of 519 - at 0x483B7F3: malloc (vg_replace_malloc.c:309) - by 0x223B63F: xrealloc (xmalloc.c:177) - by 0xA8D848: void va_heap::reserve(vec*&, unsigned int, bool) (vec.h:290) - by 0xA8B373: vec::reserve(unsigned int, bool) (vec.h:1858) - by 0xA57508: class_decl_loc_t::add_or_diag_mismatched_tag(tree_node*, tag_types, bool, bool) (parser.c:32980) - by 0xA573E1: class_decl_loc_t::add(cp_parser*, unsigned int, tag_types, tree_node*, bool, bool) (parser.c:32941) - by 0xA56C52: cp_parser_check_class_key(cp_parser*, unsigned int, tag_types, tree_node*, bool, bool) (parser.c:32819) - by 0xA48BC6: cp_parser_class_head(cp_parser*, bool*) (parser.c:26090) - by 0xA4674B: cp_parser_class_specifier_1(cp_parser*) (parser.c:25302) - by 0xA47D76: cp_parser_class_specifier(cp_parser*) (parser.c:25680) - by 0xA37E27: cp_parser_type_specifier(cp_parser*, int, cp_decl_specifier_seq*, bool, int*, bool*) (parser.c:18912) - by 0xA31CDD: cp_parser_decl_specifier_seq(cp_parser*, int, cp_decl_specifier_seq*, int*) (parser.c:15517) - -832 bytes in 8 blocks are definitely lost in loss record 458 of 519 - at 0x483B7F3: malloc (vg_replace_malloc.c:309) - by 0x223B63F: xrealloc (xmalloc.c:177) - by 0xA8D848: void va_heap::reserve(vec*&, unsigned int, bool) (vec.h:290) - by 0xA901ED: bool vec_safe_reserve(vec*&, unsigned int, bool) (vec.h:697) - by 0xA8F161: void vec_alloc(vec*&, unsigned int) (vec.h:718) - by 0xA8D18D: vec::copy() const (vec.h:979) - by 0xA8B0C3: vec::copy() const (vec.h:1824) - by 0xA896B1: class_decl_loc_t::operator=(class_decl_loc_t const&) (parser.c:32697) - by 0xA571FD: class_decl_loc_t::add(cp_parser*, unsigned int, tag_types, tree_node*, bool, bool) (parser.c:32899) - by 0xA56C52: cp_parser_check_class_key(cp_parser*, unsigned int, tag_types, tree_node*, bool, bool) (parser.c:32819) - by 0xA3AD12: cp_parser_elaborated_type_specifier(cp_parser*, bool, bool) (parser.c:20227) - by 0xA37EF2: cp_parser_type_specifier(cp_parser*, int, cp_decl_specifier_seq*, bool, int*, bool*) (parser.c:18942) - -1,144 bytes in 11 blocks are definitely lost in loss record 466 of 519 - at 0x483B7F3: malloc (vg_replace_malloc.c:309) - by 0x223B63F: xrealloc (xmalloc.c:177) - by 0xA8D848: void va_heap::reserve(vec*&, unsigned int, bool) (vec.h:290) - by 0xA901ED: bool vec_safe_reserve(vec*&, unsigned int, bool) (vec.h:697) - by 0xA8F161: void vec_alloc(vec*&, unsigned int) (vec.h:718) - by 0xA8D18D: vec::copy() const (vec.h:979) - by 0xA8B0C3: vec::copy() const (vec.h:1824) - by 0xA896B1: class_decl_loc_t::operator=(class_decl_loc_t const&) (parser.c:32697) - by 0xA571FD: class_decl_loc_t::add(cp_parser*, unsigned int, tag_types, tree_node*, bool, bool) (parser.c:32899) - by 0xA56C52: cp_parser_check_class_key(cp_parser*, unsigned int, tag_types, tree_node*, bool, bool) (parser.c:32819) - by 0xA48BC6: cp_parser_class_head(cp_parser*, bool*) (parser.c:26090) - by 0xA4674B: cp_parser_class_specifier_1(cp_parser*) (parser.c:25302) - -1,376 bytes in 10 blocks are definitely lost in loss record 467 of 519 - at 0x483DFAF: realloc (vg_replace_malloc.c:836) - by 0x223B62C: xrealloc (xmalloc.c:179) - by 0xA8D848: void va_heap::reserve(vec*&, unsigned int, bool) (vec.h:290) - by 0xA8B373: vec::reserve(unsigned int, bool) (vec.h:1858) - by 0xA8B277: vec::safe_push(class_decl_loc_t::class_key_loc_t const&) (vec.h:1967) - by 0xA57481: class_decl_loc_t::add_or_diag_mismatched_tag(tree_node*, tag_types, bool, bool) (parser.c:32967) - by 0xA573E1: class_decl_loc_t::add(cp_parser*, unsigned int, tag_types, tree_node*, bool, bool) (parser.c:32941) - by 0xA56C52: cp_parser_check_class_key(cp_parser*, unsigned int, tag_types, tree_node*, bool, bool) (parser.c:32819) - by 0xA3AD12: cp_parser_elaborated_type_specifier(cp_parser*, bool, bool) (parser.c:20227) - by 0xA37EF2: cp_parser_type_specifier(cp_parser*, int, cp_decl_specifier_seq*, bool, int*, bool*) (parser.c:18942) - by 0xA31CDD: cp_parser_decl_specifier_seq(cp_parser*, int, cp_decl_specifier_seq*, int*) (parser.c:15517) - by 0xA301E0: cp_parser_simple_declaration(cp_parser*, bool, tree_node**) (parser.c:14772) - -3,552 bytes in 33 blocks are definitely lost in loss record 483 of 519 - at 0x483B7F3: malloc (vg_replace_malloc.c:309) - by 0x223B63F: xrealloc (xmalloc.c:177) - by 0xA8D848: void va_heap::reserve(vec*&, unsigned int, bool) (vec.h:290) - by 0xA901ED: bool vec_safe_reserve(vec*&, unsigned int, bool) (vec.h:697) - by 0xA8F161: void vec_alloc(vec*&, unsigned int) (vec.h:718) - by 0xA8D18D: vec::copy() const (vec.h:979) - by 0xA8B0C3: vec::copy() const (vec.h:1824) - by 0xA8964A: class_decl_loc_t::class_decl_loc_t(class_decl_loc_t const&) (parser.c:32689) - by 0xA8F515: hash_table, class_decl_loc_t> >::hash_entry, false, xcallocator>::expand() (hash-table.h:839) - by 0xA8D4B3: hash_table, class_decl_loc_t> >::hash_entry, false, xcallocator>::find_slot_with_hash(tree_node* const&, unsigned int, insert_option) (hash-table.h:1008) - by 0xA8B1DC: hash_map, class_decl_loc_t> >::get_or_insert(tree_node* const&, bool*) (hash-map.h:200) - by 0xA57128: class_decl_loc_t::add(cp_parser*, unsigned int, tag_types, tree_node*, bool, bool) (parser.c:32888) [...] LEAK SUMMARY: - definitely lost: 8,440 bytes in 81 blocks + definitely lost: 48 bytes in 1 blocks indirectly lost: 12,529 bytes in 329 blocks possibly lost: 0 bytes in 0 blocks still reachable: 1,644,376 bytes in 768 blocks gcc/ * hash-table.h (hash_table::expand): Destruct stale Value objects. * hash-map-tests.c (test_map_of_type_with_ctor_and_dtor_expand): Update. --- gcc/hash-map-tests.c | 10 ++++++---- gcc/hash-table.h | 3 +++ 2 files changed, 9 insertions(+), 4 deletions(-) (limited to 'gcc') diff --git a/gcc/hash-map-tests.c b/gcc/hash-map-tests.c index 6acc0d4..511d434 100644 --- a/gcc/hash-map-tests.c +++ b/gcc/hash-map-tests.c @@ -317,7 +317,8 @@ test_map_of_type_with_ctor_and_dtor () } } -/* Verify aspects of 'hash_table::expand'. */ +/* Verify aspects of 'hash_table::expand', in particular that it doesn't leak + Value objects. */ static void test_map_of_type_with_ctor_and_dtor_expand (bool remove_some_inline) @@ -400,9 +401,9 @@ test_map_of_type_with_ctor_and_dtor_expand (bool remove_some_inline) ASSERT_EQ (val_t::ncopy, n_expand_moved); ASSERT_EQ (val_t::nassign, 0); if (remove_some_inline) - ASSERT_EQ (val_t::ndtor, (i + 2) / 3); + ASSERT_EQ (val_t::ndtor, n_expand_moved + (i + 2) / 3); else - ASSERT_EQ (val_t::ndtor, 0); + ASSERT_EQ (val_t::ndtor, n_expand_moved); /* Remove some inline. This never triggers an 'expand' here, but via 'm_n_deleted' does influence any following one. */ @@ -416,7 +417,7 @@ test_map_of_type_with_ctor_and_dtor_expand (bool remove_some_inline) ASSERT_EQ (val_t::ndefault, 1 + i); ASSERT_EQ (val_t::ncopy, n_expand_moved); ASSERT_EQ (val_t::nassign, 0); - ASSERT_EQ (val_t::ndtor, 1 + (i + 2) / 3); + ASSERT_EQ (val_t::ndtor, n_expand_moved + 1 + (i + 2) / 3); } } ASSERT_EQ (expand_c, expand_c_expected); @@ -439,6 +440,7 @@ test_map_of_type_with_ctor_and_dtor_expand (bool remove_some_inline) ASSERT_EQ (val_t::nassign, nassign); ASSERT_EQ (val_t::ndtor, ndtor); } + ASSERT_EQ (val_t::ndefault + val_t::ncopy, val_t::ndtor); } /* Test calling empty on a hash_map that has a key type with non-zero diff --git a/gcc/hash-table.h b/gcc/hash-table.h index a6e0ac8..ff415c7 100644 --- a/gcc/hash-table.h +++ b/gcc/hash-table.h @@ -820,6 +820,9 @@ hash_table::expand () { value_type *q = find_empty_slot_for_expand (Descriptor::hash (x)); new ((void*) q) value_type (std::move (x)); + /* After the resources of 'x' have been moved to a new object at 'q', + we now have to destroy the 'x' object, to end its lifetime. */ + x.~value_type (); } p++; -- cgit v1.1