From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id 7D5E83858CD1; Wed, 20 Mar 2024 08:35:17 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7D5E83858CD1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1710923717; bh=tlfq/ipzYvOovdAGW2zwgAbtz253ZgOn7Zb7154QEbQ=; h=From:To:Subject:Date:From; b=ncmwul1DexfkFZMpeBrjQsQnsqu2EXtAqwIuPESbuFUGZje4K4k7QTnYrh4V6YTre PDy2K98BIUhrSKdJUK6dNVYx4yhuMJDgc8/fvcFjT0rD5he6F8o/BMP2nLaLLxQoiJ 2Qhwvx2KSEBkb+Bdg+sD+X5yTwYE/qm2VMlQSz/Q= From: "jblair149 at gmail dot com" 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 X-Bugzilla-Reason: CC X-Bugzilla-Type: new X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: libstdc++ X-Bugzilla-Version: 14.0 X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: jblair149 at gmail dot com X-Bugzilla-Status: UNCONFIRMED X-Bugzilla-Resolution: X-Bugzilla-Priority: P3 X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: bug_id short_desc product version bug_status bug_severity priority component assigned_to reporter target_milestone attachments.created Message-ID: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 List-Id: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D114401 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=3D57742&action=3Dedit Source code for test program demonstrating the issue with allocator destruc= tion on node handle insertion. Summary: Implementation in the following functions that reinsert nodes into `_Rb_tre= e` 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 __hin= t, 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 destruct= or, 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 t= he 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 lea= k. One potential solution is to: 1) Add a new method to `_Node_handle_common` that will be called after a no= de is reinserted rather than simply setting `_M_ptr` to `nullptr`. That method would invoke `_M_alloc.release()` like `_M_reset()` does, and set the point= er `_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 =3D 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 discussio= n. I believe the current master branch of GCC exhibits the issue; I've confirm= ed it is present at least as of commit 994d8f922b9d88f45775f57a490409ab1c3baf5= 9. 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 nod= e, 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 =3D 2 allocator copied; active count =3D 3 allocator copied; active count =3D 4 allocator destructed; active count =3D 3 allocator destructed; active count =3D 2 allocator copied; active count =3D 3 End scope for allocator and set. allocator destructed; active count =3D 2 allocator destructed; active count =3D 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 =3D 2 allocator copied; active count =3D 3 allocator copied; active count =3D 4 allocator destructed; active count =3D 3 allocator destructed; active count =3D 2 allocator copied; active count =3D 3 allocator copied; active count =3D 4 allocator destructed; active count =3D 3 allocator destructed; active count =3D 2 End scope for allocator and set. allocator destructed; active count =3D 1 allocator destructed; active count =3D 0 deallocate memory pools fully destructed Allocator should be completely desctructed.=