Преглед изворни кода

sparse_set/registry: avoids UB with sparse_set::clear and component-less registry::clear

Michele Caini пре 5 година
родитељ
комит
e5fa67850b
3 измењених фајлова са 19 додато и 9 уклоњено
  1. 1 7
      src/entt/entity/registry.hpp
  2. 3 1
      src/entt/entity/sparse_set.hpp
  3. 15 1
      test/entt/entity/sparse_set.cpp

+ 1 - 7
src/entt/entity/registry.hpp

@@ -889,15 +889,9 @@ public:
     template<typename... Component>
     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 {

+ 3 - 1
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:

+ 15 - 1
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;