Merge branch 'fix_use_after_move' into 'master'

Fix use after move in InfoOrder::insertInfo

See merge request OpenMW/openmw!3296

apps/openmw_test_suite/CMakeLists.txt

@@ -89,6 +89,7 @@ file(GLOB UNITTEST_SRC_FILES
     esm3/readerscache.cpp
     esm3/testsaveload.cpp
     esm3/testesmwriter.cpp
+    esm3/testinfoorder.cpp
 
     nifosg/testnifloader.cpp
 )

apps/openmw_test_suite/esm3/testinfoorder.cpp

@@ -0,0 +1,27 @@
+#include <components/esm3/infoorder.hpp>
+
+#include <gtest/gtest.h>
+
+namespace ESM
+{
+    namespace
+    {
+        struct Value
+        {
+            RefId mId;
+            RefId mPrev;
+
+            Value() = default;
+            Value(const Value&) = delete;
+            Value(Value&&) = default;
+            Value& operator=(const Value&) = delete;
+            Value& operator=(Value&&) = default;
+        };
+
+        TEST(Esm3InfoOrderTest, insertInfoShouldNotCopyValue)
+        {
+            InfoOrder<Value> order;
+            order.insertInfo(Value{}, false);
+        }
+    }
+}

components/esm3/infoorder.hpp

@@ -24,19 +24,12 @@ namespace ESM
 
             auto it = mInfoPositions.find(value.mId);
 
-            if (it != mInfoPositions.end())
+            if (it != mInfoPositions.end() && it->second.mPosition->mPrev == value.mPrev)
             {
-                bool samePrev = it->second.mPosition->mPrev == value.mPrev;
                 *it->second.mPosition = std::forward<V>(value);
                 it->second.mDeleted = deleted;
-                if (samePrev)
-                    return;
+                return;
             }
-            else
-                it = mInfoPositions.emplace(value.mId, Item{ .mPosition = mOrderedInfo.end(), .mDeleted = deleted })
-                         .first;
-
-            Item& item = it->second;
 
             auto before = mOrderedInfo.begin();
             if (!value.mPrev.empty())
@@ -47,10 +40,22 @@ namespace ESM
                 else
                     before = mOrderedInfo.end();
             }
-            if (item.mPosition == mOrderedInfo.end())
-                item.mPosition = mOrderedInfo.insert(before, std::forward<V>(value));
+
+            if (it == mInfoPositions.end())
+            {
+                const RefId id = value.mId;
+                mInfoPositions.emplace(id,
+                    Item{
+                        .mPosition = mOrderedInfo.insert(before, std::forward<V>(value)),
+                        .mDeleted = deleted,
+                    });
+            }
             else
-                mOrderedInfo.splice(before, mOrderedInfo, item.mPosition);
+            {
+                *it->second.mPosition = std::forward<V>(value);
+                it->second.mDeleted = deleted;
+                mOrderedInfo.splice(before, mOrderedInfo, it->second.mPosition);
+            }
         }
 
         void removeInfo(const RefId& infoRefId)