summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Evan Brown <ezb@google.com>2023-10-17 12:30:58 -0700
committerGravatar Copybara-Service <copybara-worker@google.com>2023-10-17 12:31:55 -0700
commit7676c565eedd2581b66839c239f3825d8e381119 (patch)
tree8d64dc6f9771724736e1144c1af6a3255e441f52
parentb87875aa079571a3aa8829d61538672d88657566 (diff)
Rollback: Add sanitizer mode checks that element constructors/destructors don't make reentrant calls to raw_hash_set member functions.
PiperOrigin-RevId: 574232718 Change-Id: I8ef25fec00b76ee5fb9424e7614ca55edd6ba81b
-rw-r--r--absl/container/BUILD.bazel2
-rw-r--r--absl/container/CMakeLists.txt2
-rw-r--r--absl/container/internal/container_memory.h8
-rw-r--r--absl/container/internal/raw_hash_set.h39
-rw-r--r--absl/container/internal/raw_hash_set_test.cc84
5 files changed, 22 insertions, 113 deletions
diff --git a/absl/container/BUILD.bazel b/absl/container/BUILD.bazel
index 4aa67d35..5b69ae6b 100644
--- a/absl/container/BUILD.bazel
+++ b/absl/container/BUILD.bazel
@@ -686,10 +686,8 @@ cc_test(
"//absl/base:config",
"//absl/base:core_headers",
"//absl/base:prefetch",
- "//absl/functional:function_ref",
"//absl/log",
"//absl/strings",
- "//absl/types:optional",
"@com_google_googletest//:gtest",
"@com_google_googletest//:gtest_main",
],
diff --git a/absl/container/CMakeLists.txt b/absl/container/CMakeLists.txt
index 8362c440..96cdf59c 100644
--- a/absl/container/CMakeLists.txt
+++ b/absl/container/CMakeLists.txt
@@ -743,12 +743,10 @@ absl_cc_test(
absl::core_headers
absl::flat_hash_map
absl::flat_hash_set
- absl::function_ref
absl::hash_function_defaults
absl::hash_policy_testing
absl::hashtable_debug
absl::log
- absl::optional
absl::prefetch
absl::raw_hash_set
absl::strings
diff --git a/absl/container/internal/container_memory.h b/absl/container/internal/container_memory.h
index a735ca34..f59ca4ee 100644
--- a/absl/container/internal/container_memory.h
+++ b/absl/container/internal/container_memory.h
@@ -249,14 +249,6 @@ inline void SanitizerUnpoisonObject(const T* object) {
SanitizerUnpoisonMemoryRegion(object, sizeof(T));
}
-template <typename Container, typename Alloc, typename F>
-void RunWithReentrancyGuard(Container& c, Alloc& a, F f) {
- SanitizerPoisonObject(&c);
- if (!std::is_empty<Alloc>()) SanitizerUnpoisonObject(&a);
- f();
- SanitizerUnpoisonObject(&c);
-}
-
namespace memory_internal {
// If Pair is a standard-layout type, OffsetOf<Pair>::kFirst and
diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h
index 67964f54..a8fd80ca 100644
--- a/absl/container/internal/raw_hash_set.h
+++ b/absl/container/internal/raw_hash_set.h
@@ -2143,7 +2143,7 @@ class raw_hash_set {
alignas(slot_type) unsigned char raw[sizeof(slot_type)];
slot_type* slot = reinterpret_cast<slot_type*>(&raw);
- construct(slot, std::forward<Args>(args)...);
+ PolicyTraits::construct(&alloc_ref(), slot, std::forward<Args>(args)...);
const auto& elem = PolicyTraits::element(slot);
return PolicyTraits::apply(InsertSlot<true>{*this, std::move(*slot)}, elem);
}
@@ -2248,7 +2248,7 @@ class raw_hash_set {
// a better match if non-const iterator is passed as an argument.
void erase(iterator it) {
AssertIsFull(it.ctrl_, it.generation(), it.generation_ptr(), "erase()");
- destroy(it.slot_);
+ PolicyTraits::destroy(&alloc_ref(), it.slot_);
erase_meta_only(it);
}
@@ -2541,9 +2541,10 @@ class raw_hash_set {
std::pair<iterator, bool> operator()(const K& key, Args&&...) && {
auto res = s.find_or_prepare_insert(key);
if (res.second) {
- s.transfer(s.slot_array() + res.first, &slot);
+ PolicyTraits::transfer(&s.alloc_ref(), s.slot_array() + res.first,
+ &slot);
} else if (do_destroy) {
- s.destroy(&slot);
+ PolicyTraits::destroy(&s.alloc_ref(), &slot);
}
return {s.iterator_at(res.first), res.second};
}
@@ -2552,31 +2553,13 @@ class raw_hash_set {
slot_type&& slot;
};
- // Helpers to enable sanitizer mode validation to protect against reentrant
- // calls during element constructor/destructor.
- template <typename... Args>
- inline void construct(slot_type* slot, Args&&... args) {
- RunWithReentrancyGuard(*this, alloc_ref(), [&] {
- PolicyTraits::construct(&alloc_ref(), slot, std::forward<Args>(args)...);
- });
- }
- inline void destroy(slot_type* slot) {
- RunWithReentrancyGuard(*this, alloc_ref(),
- [&] { PolicyTraits::destroy(&alloc_ref(), slot); });
- }
- inline void transfer(slot_type* to, slot_type* from) {
- RunWithReentrancyGuard(*this, alloc_ref(), [&] {
- PolicyTraits::transfer(&alloc_ref(), to, from);
- });
- }
-
inline void destroy_slots() {
const size_t cap = capacity();
const ctrl_t* ctrl = control();
slot_type* slot = slot_array();
for (size_t i = 0; i != cap; ++i) {
if (IsFull(ctrl[i])) {
- destroy(slot + i);
+ PolicyTraits::destroy(&alloc_ref(), slot + i);
}
}
}
@@ -2639,7 +2622,7 @@ class raw_hash_set {
size_t new_i = target.offset;
total_probe_length += target.probe_length;
SetCtrl(common(), new_i, H2(hash), sizeof(slot_type));
- transfer(new_slots + new_i, old_slots + i);
+ PolicyTraits::transfer(&alloc_ref(), new_slots + new_i, old_slots + i);
}
}
if (old_capacity) {
@@ -2742,7 +2725,7 @@ class raw_hash_set {
reserve(size);
for (iterator it = that.begin(); it != that.end(); ++it) {
insert(std::move(PolicyTraits::element(it.slot_)));
- that.destroy(it.slot_);
+ PolicyTraits::destroy(&that.alloc_ref(), it.slot_);
}
that.dealloc();
that.common() = CommonFields{};
@@ -2833,7 +2816,8 @@ class raw_hash_set {
// POSTCONDITION: *m.iterator_at(i) == value_type(forward<Args>(args)...).
template <class... Args>
void emplace_at(size_t i, Args&&... args) {
- construct(slot_array() + i, std::forward<Args>(args)...);
+ PolicyTraits::construct(&alloc_ref(), slot_array() + i,
+ std::forward<Args>(args)...);
assert(PolicyTraits::apply(FindElement{*this}, *iterator_at(i)) ==
iterator_at(i) &&
@@ -2899,7 +2883,8 @@ class raw_hash_set {
}
static void transfer_slot_fn(void* set, void* dst, void* src) {
auto* h = static_cast<raw_hash_set*>(set);
- h->transfer(static_cast<slot_type*>(dst), static_cast<slot_type*>(src));
+ PolicyTraits::transfer(&h->alloc_ref(), static_cast<slot_type*>(dst),
+ static_cast<slot_type*>(src));
}
// Note: dealloc_fn will only be used if we have a non-standard allocator.
static void dealloc_fn(CommonFields& common, const PolicyFunctions&) {
diff --git a/absl/container/internal/raw_hash_set_test.cc b/absl/container/internal/raw_hash_set_test.cc
index 4e67b79e..d194ca1b 100644
--- a/absl/container/internal/raw_hash_set_test.cc
+++ b/absl/container/internal/raw_hash_set_test.cc
@@ -49,10 +49,8 @@
#include "absl/container/internal/hash_policy_testing.h"
#include "absl/container/internal/hashtable_debug.h"
#include "absl/container/internal/test_allocator.h"
-#include "absl/functional/function_ref.h"
#include "absl/log/log.h"
#include "absl/strings/string_view.h"
-#include "absl/types/optional.h"
namespace absl {
ABSL_NAMESPACE_BEGIN
@@ -411,15 +409,19 @@ struct StringTable
using Base::Base;
};
-template <typename T>
-struct ValueTable : raw_hash_set<ValuePolicy<T>, hash_default_hash<T>,
- std::equal_to<T>, std::allocator<T>> {
- using Base = typename ValueTable::raw_hash_set;
+struct IntTable
+ : raw_hash_set<IntPolicy, hash_default_hash<int64_t>,
+ std::equal_to<int64_t>, std::allocator<int64_t>> {
+ using Base = typename IntTable::raw_hash_set;
using Base::Base;
};
-using IntTable = ValueTable<int64_t>;
-using Uint8Table = ValueTable<uint8_t>;
+struct Uint8Table
+ : raw_hash_set<Uint8Policy, hash_default_hash<uint8_t>,
+ std::equal_to<uint8_t>, std::allocator<uint8_t>> {
+ using Base = typename Uint8Table::raw_hash_set;
+ using Base::Base;
+};
template <typename T>
struct CustomAlloc : std::allocator<T> {
@@ -2487,72 +2489,6 @@ using RawHashSetAlloc = raw_hash_set<IntPolicy, hash_default_hash<int64_t>,
TEST(Table, AllocatorPropagation) { TestAllocPropagation<RawHashSetAlloc>(); }
-struct ConstructCaller {
- explicit ConstructCaller(int v) : val(v) {}
- ConstructCaller(int v, absl::FunctionRef<void()> func) : val(v) { func(); }
- template <typename H>
- friend H AbslHashValue(H h, const ConstructCaller& d) {
- return H::combine(std::move(h), d.val);
- }
- bool operator==(const ConstructCaller& c) const { return val == c.val; }
-
- int val;
-};
-
-struct DestroyCaller {
- explicit DestroyCaller(int v) : val(v) {}
- DestroyCaller(int v, absl::FunctionRef<void()> func)
- : val(v), destroy_func(func) {}
- DestroyCaller(DestroyCaller&& that)
- : val(that.val), destroy_func(std::move(that.destroy_func)) {
- that.Deactivate();
- }
- ~DestroyCaller() {
- if (destroy_func) (*destroy_func)();
- }
- void Deactivate() { destroy_func = absl::nullopt; }
-
- template <typename H>
- friend H AbslHashValue(H h, const DestroyCaller& d) {
- return H::combine(std::move(h), d.val);
- }
- bool operator==(const DestroyCaller& d) const { return val == d.val; }
-
- int val;
- absl::optional<absl::FunctionRef<void()>> destroy_func;
-};
-
-#if defined(ABSL_HAVE_ADDRESS_SANITIZER) || defined(ABSL_HAVE_MEMORY_SANITIZER)
-TEST(Table, ReentrantCallsFail) {
- constexpr const char* kDeathMessage =
- "use-after-poison|use-of-uninitialized-value";
- {
- ValueTable<ConstructCaller> t;
- t.insert(ConstructCaller{0});
- auto erase_begin = [&] { t.erase(t.begin()); };
- EXPECT_DEATH_IF_SUPPORTED(t.emplace(1, erase_begin), kDeathMessage);
- }
- {
- ValueTable<DestroyCaller> t;
- t.insert(DestroyCaller{0});
- auto find_0 = [&] { t.find(DestroyCaller{0}); };
- t.insert(DestroyCaller{1, find_0});
- for (int i = 10; i < 20; ++i) t.insert(DestroyCaller{i});
- EXPECT_DEATH_IF_SUPPORTED(t.clear(), kDeathMessage);
- for (auto& elem : t) elem.Deactivate();
- }
- {
- ValueTable<DestroyCaller> t;
- t.insert(DestroyCaller{0});
- auto insert_1 = [&] { t.insert(DestroyCaller{1}); };
- t.insert(DestroyCaller{1, insert_1});
- for (int i = 10; i < 20; ++i) t.insert(DestroyCaller{i});
- EXPECT_DEATH_IF_SUPPORTED(t.clear(), kDeathMessage);
- for (auto& elem : t) elem.Deactivate();
- }
-}
-#endif
-
} // namespace
} // namespace container_internal
ABSL_NAMESPACE_END