Merge pull request #3 from SergeyMakeev/fix-memory-stomp

Refactor: prevent potential memory stomps in BlobBuilder and fix array safety
SergeyMakeev · Jan 8, 2025 · b30bee6 · b30bee6
2 parents 2502cfa + e35c3b1
commit b30bee6
Show file tree

Hide file tree

Showing 14 changed files with 338 additions and 118 deletions.
diff --git a/.gitignore b/.gitignore
@@ -1,4 +1,5 @@
 build2019
+build2022
 build
 *.log
 *.zm

diff --git a/Zmeya.h b/Zmeya.h
diff --git a/ZmeyaTest01.cpp b/ZmeyaTest01.cpp
@@ -1,6 +1,64 @@
+#include "gtest/gtest.h"
+
+/*
+namespace Memory
+{
+
+size_t mallocCount = 0;
+size_t freeCount = 0;
+
+struct Header
+{
+    void* p;
+    size_t size;
+    size_t magic;
+};
+
+const size_t kMinValidAlignment = 4;
+
+void* malloc(size_t bytesCount, size_t alignment)
+{
+    mallocCount++;
+    if (alignment < kMinValidAlignment)
+    {
+        alignment = kMinValidAlignment;
+    }
+    void* p;
+    void** p2;
+    size_t offset = alignment - 1 + sizeof(Header);
+    if ((p = (void*)std::malloc(bytesCount + offset)) == NULL)
+    {
+        return NULL;
+    }
+    p2 = (void**)(((size_t)(p) + offset) & ~(alignment - 1));
+
+    Header* h = reinterpret_cast<Header*>(reinterpret_cast<char*>(p2) - sizeof(Header));
+    h->p = p;
+    h->size = bytesCount;
+    h->magic = size_t(0x13061979);
+    return p2;
+}
+
+void mfree(void* p)
+{
+    freeCount++;
+    if (!p)
+    {
+        return;
+    }
+    Header* h = reinterpret_cast<Header*>(reinterpret_cast<char*>(p) - sizeof(Header));
+    ASSERT_EQ(h->magic, size_t(0x13061979));
+    std::free(h->p);
+}
+
+} // namespace Memory
+
+#define ZMEYA_ALLOC(sizeInBytes, alignment) Memory::malloc(sizeInBytes, alignment)
+#define ZMEYA_FREE(ptr) Memory::mfree(ptr)
+*/
+
 #include "TestHelper.h"
 #include "Zmeya.h"
-#include "gtest/gtest.h"
 
 struct SimpleTestRoot
 {
@@ -11,8 +69,6 @@ struct SimpleTestRoot
     uint32_t arr[32];
 };
 
-
-
 static void validate(const SimpleTestRoot* root)
 {
     EXPECT_FLOAT_EQ(root->a, 13.0f);
@@ -30,7 +86,7 @@ TEST(ZmeyaTestSuite, SimpleTest)
     std::vector<char> bytesCopy;
     {
         // create blob
-        std::shared_ptr<zm::BlobBuilder> blobBuilder = zm::BlobBuilder::create();
+        std::shared_ptr<zm::BlobBuilder> blobBuilder = zm::BlobBuilder::create(1);
 
         // allocate structure
         zm::BlobPtr<SimpleTestRoot> root = blobBuilder->allocate<SimpleTestRoot>();
@@ -61,3 +117,74 @@ TEST(ZmeyaTestSuite, SimpleTest)
     const SimpleTestRoot* rootCopy = (const SimpleTestRoot*)(bytesCopy.data());
     validate(rootCopy);
 }
+
+struct Desc
+{
+    zm::String name;
+    float v1;
+    uint32_t v2;
+};
+
+struct TestRoot
+{
+    zm::Array<Desc> arr;
+};
+
+TEST(ZmeyaTestSuite, SimpleTest2)
+{
+    const std::vector<std::string> names = {"apple",   "banana",  "orange",  "castle",  "dragon",  "flower",  "guitar",
+                                            "hockey",  "island",  "jungle",  "kingdom", "library", "monster", "notable",
+                                            "oceanic", "painter", "quarter", "rescue",  "seventh", "trivial", "umbrella",
+                                            "village", "warrior", "xenial",  "yonder",  "zephyr"};
+
+/*
+    Memory::mallocCount = 0;
+    Memory::freeCount = 0;
+    EXPECT_EQ(Memory::mallocCount, size_t(0));
+    EXPECT_EQ(Memory::freeCount, size_t(0));
+*/
+
+    std::vector<char> blob;
+    {
+        std::shared_ptr<zm::BlobBuilder> blobBuilder = zm::BlobBuilder::create(1);
+        zm::BlobPtr<TestRoot> root = blobBuilder->allocate<TestRoot>();
+
+        blobBuilder->resizeArray(root->arr, names.size());
+        EXPECT_EQ(root->arr.size(), names.size());
+
+        for (size_t i = 0; i < names.size(); i++)
+        {
+            zm::BlobPtr<Desc> desc = blobBuilder->getArrayElement(root->arr, i);
+            blobBuilder->copyTo(desc->name, names[i].c_str());
+            const char* s1 = root->arr[i].name.c_str();
+            const char* s2 = names[i].c_str();
+            EXPECT_STREQ(s1, s2);
+            desc->v1 = (float)(i);
+            desc->v2 = (uint32_t)(i);
+        }
+        zm::Span<char> bytes = blobBuilder->finalize();
+        blob = std::vector<char>(bytes.data, bytes.data + bytes.size);
+    }
+
+/*
+    EXPECT_GT(Memory::mallocCount, size_t(0));
+    EXPECT_GT(Memory::freeCount, size_t(0));
+    EXPECT_EQ(Memory::mallocCount, Memory::freeCount);
+*/
+
+    // validate
+    const TestRoot* rootCopy = (const TestRoot*)(blob.data());
+    EXPECT_EQ(rootCopy->arr.size(), names.size());
+
+    for (size_t i = 0; i < names.size(); i++)
+    {
+        const Desc& desc = rootCopy->arr[i];
+        EXPECT_STREQ(desc.name.c_str(), names[i].c_str());
+        EXPECT_FLOAT_EQ(desc.v1, (float)(i));
+        EXPECT_EQ(desc.v2, (uint32_t)(i));
+    }
+
+/*
+    EXPECT_EQ(Memory::mallocCount, Memory::freeCount);
+*/
+}
diff --git a/ZmeyaTest02.cpp b/ZmeyaTest02.cpp
@@ -35,7 +35,7 @@ TEST(ZmeyaTestSuite, PointerTest)
 {
     std::vector<char> bytesCopy;
     {
-        std::shared_ptr<zm::BlobBuilder> blobBuilder = zm::BlobBuilder::create();
+        std::shared_ptr<zm::BlobBuilder> blobBuilder = zm::BlobBuilder::create(1);
 
         zm::BlobPtr<PointerTestRoot> root = blobBuilder->allocate<PointerTestRoot>();
         zm::BlobPtr<PointerTestNode> nodeLeft = blobBuilder->allocate<PointerTestNode>();

diff --git a/ZmeyaTest03.cpp b/ZmeyaTest03.cpp
@@ -99,7 +99,7 @@ TEST(ZmeyaTestSuite, ArrayTest)
 {
     std::vector<char> bytesCopy;
     {
-        std::shared_ptr<zm::BlobBuilder> blobBuilder = zm::BlobBuilder::create();
+        std::shared_ptr<zm::BlobBuilder> blobBuilder = zm::BlobBuilder::create(1);
         zm::BlobPtr<ArrayTestRoot> root = blobBuilder->allocate<ArrayTestRoot>();
 
         // assign from std::vector
@@ -116,17 +116,18 @@ TEST(ZmeyaTestSuite, ArrayTest)
         // resize array
         blobBuilder->resizeArray(root->arr4, 4);
         // assign array elements (sub-arrays)
-        blobBuilder->copyTo(root->arr4[0], {1.2f, 2.3f});
-        blobBuilder->copyTo(root->arr4[1], {7.1f, 8.8f, 3.2f});
-        blobBuilder->copyTo(root->arr4[2], {16.0f, 12.0f, 99.5f, -143.0f});
-        blobBuilder->copyTo(root->arr4[3], {-1.0f});
+
+        blobBuilder->copyTo(blobBuilder->getArrayElement(root->arr4, 0), {1.2f, 2.3f});
+        blobBuilder->copyTo(blobBuilder->getArrayElement(root->arr4, 1), {7.1f, 8.8f, 3.2f});
+        blobBuilder->copyTo(blobBuilder->getArrayElement(root->arr4, 2), {16.0f, 12.0f, 99.5f, -143.0f});
+        blobBuilder->copyTo(blobBuilder->getArrayElement(root->arr4, 3), {-1.0f});
 
         // resize array
         blobBuilder->resizeArray(root->arr5, 793);
         for (size_t i = 0; i < root->arr5.size(); i++)
         {
             zm::BlobPtr<Payload> payload = blobBuilder->allocate<Payload>(1.3f + float(i) * 0.4f, uint32_t(i) + 3);
-            root->arr5[i] = payload;
+            *blobBuilder->getArrayElement(root->arr5, i) = payload;
         }
 
         std::vector<std::vector<uint32_t>> vec2 = {{1, 2}, {2, 7, 11, 9, 141}, {15, 9, 33, 7}};

diff --git a/ZmeyaTest05.cpp b/ZmeyaTest05.cpp
@@ -58,7 +58,7 @@ TEST(ZmeyaTestSuite, StringTest)
 {
     std::vector<char> bytesCopy;
     {
-        std::shared_ptr<zm::BlobBuilder> blobBuilder = zm::BlobBuilder::create();
+        std::shared_ptr<zm::BlobBuilder> blobBuilder = zm::BlobBuilder::create(1);
         zm::BlobPtr<StringTestRoot> root = blobBuilder->allocate<StringTestRoot>();
 
         // assign from null-terminated c string (74 bytes long)
@@ -89,7 +89,7 @@ TEST(ZmeyaTestSuite, StringTest)
         blobBuilder->resizeArray(root->strArr4, numStrings);
         for (size_t i = 0; i < root->strArr4.size(); i++)
         {
-            blobBuilder->referTo(root->strArr4[i], root->str1);
+            blobBuilder->referTo(blobBuilder->getArrayElement(root->strArr4, i), root->str1);
         }
 
         validate(root.get());

diff --git a/ZmeyaTest06.cpp b/ZmeyaTest06.cpp
@@ -70,7 +70,7 @@ TEST(ZmeyaTestSuite, HashSetTest)
 {
     std::vector<char> bytesCopy;
     {
-        std::shared_ptr<zm::BlobBuilder> blobBuilder = zm::BlobBuilder::create();
+        std::shared_ptr<zm::BlobBuilder> blobBuilder = zm::BlobBuilder::create(1);
         zm::BlobPtr<HashSetTestRoot> root = blobBuilder->allocate<HashSetTestRoot>();
 
         // assign from std::unordered_set

diff --git a/ZmeyaTest07.cpp b/ZmeyaTest07.cpp
@@ -87,7 +87,7 @@ TEST(ZmeyaTestSuite, HashMapTest)
 {
     std::vector<char> bytesCopy;
     {
-        std::shared_ptr<zm::BlobBuilder> blobBuilder = zm::BlobBuilder::create();
+        std::shared_ptr<zm::BlobBuilder> blobBuilder = zm::BlobBuilder::create(1);
         zm::BlobPtr<HashMapTestRoot> root = blobBuilder->allocate<HashMapTestRoot>();
 
         // assign from std::unordered_map

diff --git a/ZmeyaTest08.cpp b/ZmeyaTest08.cpp
@@ -55,7 +55,7 @@ TEST(ZmeyaTestSuite, IteratorsTest)
 {
     std::vector<char> bytesCopy;
     {
-        std::shared_ptr<zm::BlobBuilder> blobBuilder = zm::BlobBuilder::create();
+        std::shared_ptr<zm::BlobBuilder> blobBuilder = zm::BlobBuilder::create(1);
         zm::BlobPtr<IteratorsTestRoot> root = blobBuilder->allocate<IteratorsTestRoot>();
 
         blobBuilder->copyTo(root->arr, {10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0});

diff --git a/ZmeyaTest09.cpp b/ZmeyaTest09.cpp
@@ -79,20 +79,20 @@ static void generateTestFile(const char* fileName)
     {
         std::vector<std::string> objectNames = {"root", "test1", "floor", "window", "arrow", "door"};
 
-        std::shared_ptr<zm::BlobBuilder> blobBuilder = zm::BlobBuilder::create();
+        std::shared_ptr<zm::BlobBuilder> blobBuilder = zm::BlobBuilder::create(1);
         zm::BlobPtr<SimpleFileTestRoot> root = blobBuilder->allocate<SimpleFileTestRoot>();
         root->magic = 0x59454D5A;
         blobBuilder->resizeArray(root->objects, 6);
         for (size_t i = 0; i < root->objects.size(); i++)
         {
-            Object& object = root->objects[i];
-            blobBuilder->copyTo(object.name, objectNames[i]);
-            object.position = Vec2(float(i), float(i + 4));
+            zm::BlobPtr<Object> object = blobBuilder->getArrayElement(root->objects, i);
+            blobBuilder->copyTo(object->name, objectNames[i]);
+            object->position = Vec2(float(i), float(i + 4));
 
             if (i > 0)
             {
                 const Object& parentObject = root->objects[i - 1];
-                blobBuilder->assignTo(object.parent, parentObject);
+                blobBuilder->assignTo(object->parent, parentObject);
             }
         }
 

diff --git a/ZmeyaTest10.cpp b/ZmeyaTest10.cpp
@@ -141,7 +141,8 @@ void createChildren(zm::BlobBuilder* blobBuilder, const zm::BlobPtr<MMapTestNode
         blobBuilder->copyTo(node->name, "leaf_" + std::to_string(startIndex + i));
         node->payload = uint32_t(count + startIndex * 13);
         node->parent = parent;
-        parent->children[i] = node;
+        zm::BlobPtr<zm::Pointer<MMapTestNode>> ch = blobBuilder->getArrayElement(parent->children, i);
+        *ch = node;
     }
 }
 
@@ -173,7 +174,7 @@ zm::BlobPtr<MMapTestNode> allocateNode2(zm::BlobBuilder* blobBuilder, const zm::
 
 static void generateTestFile(const char* fileName)
 {
-    std::shared_ptr<zm::BlobBuilder> blobBuilder = zm::BlobBuilder::create();
+    std::shared_ptr<zm::BlobBuilder> blobBuilder = zm::BlobBuilder::create(1);
     zm::BlobPtr<MMapTestRoot> root = blobBuilder->allocate<MMapTestRoot>();
     root->magic = 0x59454D5A;
     blobBuilder->copyTo(root->desc, "Zmyea test file. This is supposed to be a long enough string. I think it is long enough now.");
@@ -191,7 +192,8 @@ static void generateTestFile(const char* fileName)
         {
             rootNode = allocateNode2(blobBuilder.get(), root, i);
         }
-        root->roots[i] = rootNode;
+        zm::BlobPtr<zm::Pointer<MMapTestNode>> rt = blobBuilder->getArrayElement(root->roots, i);
+        *rt = rootNode;
     }
 
     validate(root.get());

diff --git a/ZmeyaTest11.cpp b/ZmeyaTest11.cpp
@@ -22,7 +22,7 @@ struct ReferToTestRoot
 
 template <typename T> static void validateNode(const T* node)
 {
-    EXPECT_EQ(node->str, "This is supposed to be a long enough string. I think it is long enough now.");
+    EXPECT_STREQ(node->str.c_str(), "This is supposed to be a long enough string. I think it is long enough now.");
     EXPECT_EQ(node->arr.size(), std::size_t(10));
     EXPECT_EQ(node->arr[0], 1);
     EXPECT_EQ(node->arr[1], 2);
@@ -45,10 +45,10 @@ template <typename T> static void validateNode(const T* node)
     EXPECT_FALSE(node->hashSet.contains(32));
 
     EXPECT_EQ(node->hashMap.size(), std::size_t(4));
-    EXPECT_FLOAT_EQ(node->hashMap.find("one", 0.0f), 1.0f);
-    EXPECT_FLOAT_EQ(node->hashMap.find("two", 0.0f), 2.0f);
-    EXPECT_FLOAT_EQ(node->hashMap.find("three", 0.0f), 3.0f);
-    EXPECT_FLOAT_EQ(node->hashMap.find("four", 0.0f), 4.0f);
+    EXPECT_FLOAT_EQ(node->hashMap.find("one", -1.0f), 1.0f);
+    EXPECT_FLOAT_EQ(node->hashMap.find("two", -1.0f), 2.0f);
+    EXPECT_FLOAT_EQ(node->hashMap.find("three", -1.0f), 3.0f);
+    EXPECT_FLOAT_EQ(node->hashMap.find("four", -1.0f), 4.0f);
 }
 
 static void validate(const ReferToTestRoot* root)
@@ -66,7 +66,7 @@ TEST(ZmeyaTestSuite, ReferToTest)
     std::vector<char> bytesCopy;
     {
         // create blob
-        std::shared_ptr<zm::BlobBuilder> blobBuilder = zm::BlobBuilder::create();
+        std::shared_ptr<zm::BlobBuilder> blobBuilder = zm::BlobBuilder::create(1);
 
         // allocate structure
         zm::BlobPtr<ReferToTestRoot> root = blobBuilder->allocate<ReferToTestRoot>();
@@ -75,15 +75,28 @@ TEST(ZmeyaTestSuite, ReferToTest)
         blobBuilder->copyTo(root->hashSet, {1, 5, 15, 23, 38, 31});
         blobBuilder->copyTo(root->hashMap, {{"one", 1.0f}, {"two", 2.0f}, {"three", 3.0f}, {"four", 4.0f}});
 
+        float f1 = root->hashMap.find("one", -1.0f);
+        EXPECT_FLOAT_EQ(f1, 1.0f);
+
+        float f2 = root->hashMap.find("two", -1.0f);
+        EXPECT_FLOAT_EQ(f2, 2.0f);
+
+        float f3 = root->hashMap.find("three", -1.0f);
+        EXPECT_FLOAT_EQ(f3, 3.0f);
+
+        float f4 = root->hashMap.find("four", -1.0f);
+        EXPECT_FLOAT_EQ(f4, 4.0f);
+
+
         size_t nodesCount = 10000;
         blobBuilder->resizeArray(root->nodes, nodesCount);
         for (size_t i = 0; i < nodesCount; i++)
         {
-            ReferToTestNode& node = root->nodes[i];
-            blobBuilder->referTo(node.str, root->str);
-            blobBuilder->referTo(node.arr, root->arr);
-            blobBuilder->referTo(node.hashSet, root->hashSet);
-            blobBuilder->referTo(node.hashMap, root->hashMap);
+            zm::BlobPtr<ReferToTestNode> node = blobBuilder->getArrayElement(root->nodes, i);
+            blobBuilder->referTo(node->str, root->str);
+            blobBuilder->referTo(node->arr, root->arr);
+            blobBuilder->referTo(node->hashSet, root->hashSet);
+            blobBuilder->referTo(node->hashMap, root->hashMap);
         }
 
         validate(root.get());

diff --git a/gen.cmd → gen19.cmd b/gen.cmd → gen19.cmd
diff --git a/gen22.cmd b/gen22.cmd
@@ -0,0 +1,10 @@
+@echo off
+set builddir=build2022
+if not exist %builddir% goto GENERATE
+del %builddir% /S /Q
+:GENERATE
+mkdir %builddir%
+cd %builddir%
+cmake -G "Visual Studio 17 2022" ../
+cd ..
+