From e5fa67850bc530c0a80d719189bd6f17ca2ceb9c Mon Sep 17 00:00:00 2001 From: Michele Caini Date: Wed, 3 Feb 2021 16:18:25 +0100 Subject: [PATCH] sparse_set/registry: avoids UB with sparse_set::clear and component-less registry::clear --- src/entt/entity/registry.hpp | 8 +------- src/entt/entity/sparse_set.hpp | 4 +++- test/entt/entity/sparse_set.cpp | 16 +++++++++++++++- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/entt/entity/registry.hpp b/src/entt/entity/registry.hpp index 8044afca4..9f0a1d3ad 100644 --- a/src/entt/entity/registry.hpp +++ b/src/entt/entity/registry.hpp @@ -889,15 +889,9 @@ public: template void clear() { if constexpr(sizeof...(Component) == 0) { - for(auto pos = pools.size(); pos; --pos) { - if(auto &pdata = pools[pos-1]; pdata.pool) { - pdata.poly->remove(*this, pdata.pool->rbegin(), pdata.pool->rend()); - } - } - for(auto pos = entities.size(); pos; --pos) { if(const auto entt = entities[pos - 1]; (to_integral(entt) & traits_type::entity_mask) == (pos - 1)) { - release_entity(entt, version(entt) + 1u); + destroy(entt); } } } else { diff --git a/src/entt/entity/sparse_set.hpp b/src/entt/entity/sparse_set.hpp index 6743e3348..3d843b2e9 100644 --- a/src/entt/entity/sparse_set.hpp +++ b/src/entt/entity/sparse_set.hpp @@ -427,6 +427,8 @@ public: const auto other = packed.back(); sparse[page(other)][offset(other)] = ref; + // If it looks weird, imagine what the subtle bugs it prevents are + ENTT_ASSERT((packed.back() = entt, true)); packed[pos] = other; ref = null; @@ -571,7 +573,7 @@ public: /*! @brief Clears a sparse set. */ void clear() ENTT_NOEXCEPT { - remove(rbegin(), rend()); + remove(begin(), end()); } private: diff --git a/test/entt/entity/sparse_set.cpp b/test/entt/entity/sparse_set.cpp index 40d744541..ee32e5c5e 100644 --- a/test/entt/entity/sparse_set.cpp +++ b/test/entt/entity/sparse_set.cpp @@ -152,7 +152,7 @@ TEST(SparseSet, Remove) { ASSERT_TRUE(set.empty()); set.insert(std::begin(entities), std::end(entities)); - set.remove(set.rbegin(), set.rend()); + set.remove(set.begin(), set.end()); ASSERT_TRUE(set.empty()); @@ -171,6 +171,20 @@ TEST(SparseSet, Remove) { ASSERT_EQ(*set.begin(), entt::entity{42}); } +TEST(SparseSet, Clear) { + entt::sparse_set set; + + set.emplace(entt::entity{3}); + set.emplace(entt::entity{42}); + set.emplace(entt::entity{9}); + + ASSERT_FALSE(set.empty()); + + set.clear(); + + ASSERT_TRUE(set.empty()); +} + TEST(SparseSet, Iterator) { using iterator = typename entt::sparse_set::iterator;