public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r12-3625] Fix 'hash_table::expand' to destruct stale Value objects
@ 2021-09-17 15:40 Thomas Schwinge
  0 siblings, 0 replies; only message in thread
From: Thomas Schwinge @ 2021-09-17 15:40 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:89be17a1b231ade643f28fbe616d53377e069da8

commit r12-3625-g89be17a1b231ade643f28fbe616d53377e069da8
Author: Thomas Schwinge <thomas@codesourcery.com>
Date:   Fri Aug 13 18:03:38 2021 +0200

    Fix 'hash_table::expand' to destruct stale Value objects
    
    Thus plugging potentional memory leaks if these have non-trivial
    constructor/destructor.
    
    See
    <https://stackoverflow.com/questions/6730403/how-to-delete-object-constructed-via-placement-new-operator>
    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<class_decl_loc_t::class_key_loc_t>(vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_embed>*&, unsigned int, bool) (vec.h:290)
        -   by 0xA8B373: vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_ptr>::reserve(unsigned int, bool) (vec.h:1858)
        -   by 0xA8B277: vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_ptr>::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<class_decl_loc_t::class_key_loc_t>(vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_embed>*&, unsigned int, bool) (vec.h:290)
        -   by 0xA8B373: vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_ptr>::reserve(unsigned int, bool) (vec.h:1858)
        -   by 0xA8B277: vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_ptr>::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<deferred_access_check, va_gc, vl_embed>*, 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<class_decl_loc_t::class_key_loc_t>(vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_embed>*&, unsigned int, bool) (vec.h:290)
        -   by 0xA8B373: vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_ptr>::reserve(unsigned int, bool) (vec.h:1858)
        -   by 0xA8B277: vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_ptr>::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<class_decl_loc_t::class_key_loc_t>(vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_embed>*&, unsigned int, bool) (vec.h:290)
        -   by 0xA8B373: vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_ptr>::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<class_decl_loc_t::class_key_loc_t>(vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_embed>*&, unsigned int, bool) (vec.h:290)
        -   by 0xA901ED: bool vec_safe_reserve<class_decl_loc_t::class_key_loc_t, va_heap>(vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_embed>*&, unsigned int, bool) (vec.h:697)
        -   by 0xA8F161: void vec_alloc<class_decl_loc_t::class_key_loc_t, va_heap>(vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_embed>*&, unsigned int) (vec.h:718)
        -   by 0xA8D18D: vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_embed>::copy() const (vec.h:979)
        -   by 0xA8B0C3: vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_ptr>::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<class_decl_loc_t::class_key_loc_t>(vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_embed>*&, unsigned int, bool) (vec.h:290)
        -   by 0xA901ED: bool vec_safe_reserve<class_decl_loc_t::class_key_loc_t, va_heap>(vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_embed>*&, unsigned int, bool) (vec.h:697)
        -   by 0xA8F161: void vec_alloc<class_decl_loc_t::class_key_loc_t, va_heap>(vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_embed>*&, unsigned int) (vec.h:718)
        -   by 0xA8D18D: vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_embed>::copy() const (vec.h:979)
        -   by 0xA8B0C3: vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_ptr>::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<class_decl_loc_t::class_key_loc_t>(vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_embed>*&, unsigned int, bool) (vec.h:290)
        -   by 0xA8B373: vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_ptr>::reserve(unsigned int, bool) (vec.h:1858)
        -   by 0xA8B277: vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_ptr>::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<class_decl_loc_t::class_key_loc_t>(vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_embed>*&, unsigned int, bool) (vec.h:290)
        -   by 0xA901ED: bool vec_safe_reserve<class_decl_loc_t::class_key_loc_t, va_heap>(vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_embed>*&, unsigned int, bool) (vec.h:697)
        -   by 0xA8F161: void vec_alloc<class_decl_loc_t::class_key_loc_t, va_heap>(vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_embed>*&, unsigned int) (vec.h:718)
        -   by 0xA8D18D: vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_embed>::copy() const (vec.h:979)
        -   by 0xA8B0C3: vec<class_decl_loc_t::class_key_loc_t, va_heap, vl_ptr>::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<hash_map<tree_decl_hash, class_decl_loc_t, simple_hashmap_traits<default_hash_traits<tree_decl_hash>, class_decl_loc_t> >::hash_entry, false, xcallocator>::expand() (hash-table.h:839)
        -   by 0xA8D4B3: hash_table<hash_map<tree_decl_hash, class_decl_loc_t, simple_hashmap_traits<default_hash_traits<tree_decl_hash>, 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<tree_decl_hash, class_decl_loc_t, simple_hashmap_traits<default_hash_traits<tree_decl_hash>, 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<Descriptor, Lazy, Allocator>::expand):
            Destruct stale Value objects.
            * hash-map-tests.c (test_map_of_type_with_ctor_and_dtor_expand):
            Update.

Diff:
---
 gcc/hash-map-tests.c | 10 ++++++----
 gcc/hash-table.h     |  3 +++
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/gcc/hash-map-tests.c b/gcc/hash-map-tests.c
index 6acc0d4337e..511d4342087 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 a6e0ac8eea9..ff415c7250b 100644
--- a/gcc/hash-table.h
+++ b/gcc/hash-table.h
@@ -820,6 +820,9 @@ hash_table<Descriptor, Lazy, Allocator>::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++;


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2021-09-17 15:40 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-17 15:40 [gcc r12-3625] Fix 'hash_table::expand' to destruct stale Value objects Thomas Schwinge

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).