[libc++] Fix exception safety of __hash_table::__copy_construct (avoid memory leak) (#201452)

Slightly easier to digest repro: https://godbolt.org/z/ejjs5br5f
This commit is contained in:
Nikita Taranov 2026-06-10 09:05:56 +02:00 committed by GitHub
parent fe298c3006
commit 66a0203502
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 134 additions and 0 deletions

View file

@ -41,6 +41,7 @@
#include <__type_traits/is_swappable.h>
#include <__type_traits/remove_const.h>
#include <__type_traits/remove_cvref.h>
#include <__utility/exception_guard.h>
#include <__utility/exchange.h>
#include <__utility/forward.h>
#include <__utility/move.h>
@ -697,6 +698,17 @@ private:
_LIBCPP_HIDE_FROM_ABI void __copy_construct(__next_pointer __other_iter) {
__next_pointer __own_iter = __first_node_.__ptr();
// If copying a node throws, the nodes that have already been constructed and linked into the
// list have to be deallocated. When this is called from the copy constructor the enclosing
// __hash_table is not fully constructed yet, so its destructor would not run to release them.
// This is also reached from operator= when assigning into an empty table; there the table
// stays alive, so we must leave it in a valid empty state: besides releasing the nodes we
// clear the bucket pointers, which were made to point into the now-deallocated node list.
auto __guard = std::__make_exception_guard([&] {
__deallocate_node_list(__first_node_.__next_);
__first_node_.__next_ = nullptr;
std::fill_n(__bucket_list_.get(), bucket_count(), nullptr);
});
{
__node_holder __new_node = __construct_node_hash(__other_iter->__hash(), __other_iter->__upcast()->__get_value());
__own_iter->__next_ = static_cast<__next_pointer>(__new_node.release());
@ -707,6 +719,7 @@ private:
__other_iter = __other_iter->__next_;
__own_iter = __own_iter->__next_;
__copy_construct(__other_iter, __own_iter, __current_chash);
__guard.__complete();
}
public:

View file

@ -0,0 +1,121 @@
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
// UNSUPPORTED: no-exceptions
// <unordered_map>
// Check that the unordered_map copy constructor does not leak nodes when a node allocation
// throws partway through copying the elements. The already-copied nodes must be released even
// though the container itself never finishes constructing.
//
// Also check the copy-assignment-into-empty path, which shares the same node-copying helper.
// There the assigned-to container stays alive, so after a throw it must be left in a valid
// (usable, leak-free) state with no dangling bucket pointers.
#include <cassert>
#include <cstddef>
#include <functional>
#include <memory>
#include <new>
#include <unordered_map>
#include <utility>
#include "count_new.h"
#include "test_macros.h"
// An allocator that throws std::bad_alloc once a shared countdown reaches zero. It forwards
// to std::allocator (operator new) so that count_new.h can observe leaked nodes.
template <class T>
struct CountedThrowingAllocator {
typedef T value_type;
int* countdown_;
explicit CountedThrowingAllocator(int& countdown) : countdown_(&countdown) {}
template <class U>
CountedThrowingAllocator(const CountedThrowingAllocator<U>& other) : countdown_(other.countdown_) {}
T* allocate(std::size_t n) {
if (*countdown_ == 0)
throw std::bad_alloc();
--*countdown_;
return std::allocator<T>().allocate(n);
}
void deallocate(T* p, std::size_t n) { std::allocator<T>().deallocate(p, n); }
template <class U>
friend bool operator==(const CountedThrowingAllocator&, const CountedThrowingAllocator<U>&) {
return true;
}
template <class U>
friend bool operator!=(const CountedThrowingAllocator&, const CountedThrowingAllocator<U>&) {
return false;
}
};
int main(int, char**) {
typedef CountedThrowingAllocator<std::pair<const int, int> > Alloc;
typedef std::unordered_map<int, int, std::hash<int>, std::equal_to<int>, Alloc> Map;
const int never_throw = 1 << 30;
int countdown = never_throw;
// Build a source map large enough that copying it performs several node allocations. The
// allocator is propagated by select_on_container_copy_construction, so the copy shares this
// same countdown.
Map src((Alloc(countdown)));
for (int i = 0; i < 16; ++i)
src.insert(std::make_pair(i, i));
// Make a node allocation fail at every position partway through the copy and verify that no
// nodes are leaked. `outstanding_before` captures the allocations held by the source so the
// check is unaffected by it.
for (int fail_at = 0; fail_at < 32; ++fail_at) {
const int outstanding_before = globalMemCounter.outstanding_new;
countdown = fail_at;
try {
Map copy(src);
(void)copy;
} catch (const std::bad_alloc&) {
}
countdown = never_throw;
assert(globalMemCounter.outstanding_new == outstanding_before);
}
// Same, but through copy-assignment into an empty (default-constructed) map. The destination
// survives the exception, so it must be left valid: we exercise it with insert()/find() (which
// read the bucket array and would dereference a stale entry under a sanitizer) and confirm no
// nodes are leaked once it goes out of scope.
for (int fail_at = 0; fail_at < 32; ++fail_at) {
const int outstanding_before = globalMemCounter.outstanding_new;
{
Map dst((Alloc(countdown)));
countdown = fail_at;
try {
dst = src;
} catch (const std::bad_alloc&) {
}
countdown = never_throw;
// The container must be in a valid state whether or not the assignment threw.
for (int i = 0; i < 16; ++i)
dst.insert(std::make_pair(i, i));
for (int i = 0; i < 16; ++i)
assert(dst.find(i) != dst.end());
}
assert(globalMemCounter.outstanding_new == outstanding_before);
}
return 0;
}