public inbox for gcc-bugs@sourceware.org help / color / mirror / Atom feed
From: "jblair149 at gmail dot com" <gcc-bugzilla@gcc.gnu.org> To: gcc-bugs@gcc.gnu.org Subject: [Bug libstdc++/114401] New: libstdc++ allocator destructor omitted when reinserting node_handle into tree- and hashtable-based containers Date: Wed, 20 Mar 2024 08:35:13 +0000 [thread overview] Message-ID: <bug-114401-4@http.gcc.gnu.org/bugzilla/> (raw) https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114401 Bug ID: 114401 Summary: libstdc++ allocator destructor omitted when reinserting node_handle into tree- and hashtable-based containers Product: gcc Version: 14.0 Status: UNCONFIRMED Severity: normal Priority: P3 Component: libstdc++ Assignee: unassigned at gcc dot gnu.org Reporter: jblair149 at gmail dot com Target Milestone: --- Created attachment 57742 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57742&action=edit Source code for test program demonstrating the issue with allocator destruction on node handle insertion. Summary: Implementation in the following functions that reinsert nodes into `_Rb_tree` and `_Hashtable` may set the node handle data pointer to `nullptr`. This prevents stateful allocators from being properly destructed when extracting and reinserting nodes. Functions in stl_tree.h impacted: `insert_return_type _Rb_tree_impl::_M_reinsert_node_unique(node_type&& __nh)` `iterator _Rb_tree_impl::_M_reinsert_node_equal(node_type&& __nh)` `iterator _Rb_tree_impl::_M_reinsert_node_hint_unique(const_iterator __hint, node_type&& __nh)` `iterator _Rb_tree_impl::_M_reinsert_node_hint_equal(const_iterator __hint, node_type&& __nh)` Functions in hashtable.h impacted: `insert_return_type _Hashtable::_M_reinsert_node_unique(node_type&& __nh)` `insert_return_type _Hashtable::_M_reinsert_node_multi(const_iterator __hint, node_type&& __nh)` Details: The allocator of the node is asserted to be equal to the allocator of the underlying tree or hashtable in each function, but otherwise the allocator is unused, and on successful insertion `__nh._M_ptr` is set to `nullptr`. Following that, in the destructor of `_Node_handle_common`, `_M_reset()` is never called since the node is detected as `empty()`. Further, the union `_Optional_alloc` defined within `_Node_handle_common` has a no-op destructor, so only a manual invocation of `_Optional_alloc::release()` can trigger the destructor of the allocator contained, which may only be invoked if active. As a result, the allocator instance within the node is never destructed once the node has been reinserted into the tree or hashtable. This doesn't matter for stateless allocators, but for stateful ones this can result in a memory leak. One potential solution is to: 1) Add a new method to `_Node_handle_common` that will be called after a node is reinserted rather than simply setting `_M_ptr` to `nullptr`. That method would invoke `_M_alloc.release()` like `_M_reset()` does, and set the pointer `_M_ptr` to `nullptr`, but omit the actions to destroy the object and deallocate the memory since the node has been returned to a compatible container. 2) Within each of the functions mentioned within the files above, replace `__nh._M_ptr = nullptr;` with an invocation of the function in (1). I do not know for certain this is the maximal extent of the issue when reinserting extracted nodes, but I've expanded from my initial issue using `std::set` to the associative containers I'm aware of which expose `node_handle`. I also am not familiar with the naming conventions within the library. To that end, I'm omitting a patch since this may require discussion. I believe the current master branch of GCC exhibits the issue; I've confirmed it is present at least as of commit 994d8f922b9d88f45775f57a490409ab1c3baf59. This issue is likely present in past releases as well, but I don't know how that should be handled. Within the repository, the files I know are impacted are: gcc.git/libstdc++-v3/include/bits/node_handle.h gcc.git/libstdc++-v3/include/bits/stl_tree.h gcc.git/libstdc++-v3/include/bits/hashtable.h I'm attaching a relatively minimal program which demonstrates the issue. The program uses a logging, stateful allocator within a `std::set`, inserts elements, extracts a node, modifies the value of the element within the node, reinserts the node, and triggers the destruction of the set by the end of scope. Compilation of the program is as follows: g++ test_node_reinsert.cpp -o test_node_reinsert The test program may be run without arguments. The output of this program prior to the fix: Begin scope for allocator and set. initial construction allocate memory pools allocator copied; active count = 2 allocator copied; active count = 3 allocator copied; active count = 4 allocator destructed; active count = 3 allocator destructed; active count = 2 allocator copied; active count = 3 End scope for allocator and set. allocator destructed; active count = 2 allocator destructed; active count = 1 Allocator should be completely desctructed. The output of this program after my (hacky?) fix, following the instructions above: Begin scope for allocator and set. initial construction allocate memory pools allocator copied; active count = 2 allocator copied; active count = 3 allocator copied; active count = 4 allocator destructed; active count = 3 allocator destructed; active count = 2 allocator copied; active count = 3 allocator copied; active count = 4 allocator destructed; active count = 3 allocator destructed; active count = 2 End scope for allocator and set. allocator destructed; active count = 1 allocator destructed; active count = 0 deallocate memory pools fully destructed Allocator should be completely desctructed.
next reply other threads:[~2024-03-20 8:35 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2024-03-20 8:35 jblair149 at gmail dot com [this message] 2024-03-20 11:23 ` [Bug libstdc++/114401] " redi at gcc dot gnu.org 2024-03-20 14:46 ` redi at gcc dot gnu.org 2024-03-20 14:49 ` redi at gcc dot gnu.org 2024-03-21 12:06 ` redi at gcc dot gnu.org 2024-03-22 22:51 ` cvs-commit at gcc dot gnu.org 2024-03-22 22:55 ` redi at gcc dot gnu.org 2024-04-03 10:46 ` cvs-commit at gcc dot gnu.org
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=bug-114401-4@http.gcc.gnu.org/bugzilla/ \ --to=gcc-bugzilla@gcc.gnu.org \ --cc=gcc-bugs@gcc.gnu.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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).