From 8122ca265d75933bc32608d764e2aaa1c8adb2c4 Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Mon, 9 Oct 2023 19:59:21 -0700 Subject: [PATCH] Debugging another memory bug. --- Arena.cc | 43 ++++++++++++++++- Arena.h | 8 ++++ Buffer.cc | 116 ++++++++++++++++++++++++++++++++-------------- Buffer.h | 3 ++ CMakeDocs.txt | 10 ++++ CMakeLists.txt | 18 +++++-- Test.cc | 42 +++++++++++++++++ Test.h | 31 +++++++++++++ bufferTest.cc | 20 ++++++-- dictionaryTest.cc | 2 +- tlvTest.cc | 3 ++ 11 files changed, 253 insertions(+), 43 deletions(-) create mode 100644 CMakeDocs.txt create mode 100644 Test.cc create mode 100644 Test.h diff --git a/Arena.cc b/Arena.cc index ae1fe72..5f18269 100644 --- a/Arena.cc +++ b/Arena.cc @@ -13,6 +13,7 @@ #define PROT_RW (PROT_WRITE|PROT_READ) #elif defined(__WIN64__) || defined(__WIN32__) +#include #include #endif @@ -144,7 +145,47 @@ Arena::Create(const char *path, size_t fileSize, mode_t mode) int Arena::Open(const char *path) { + HANDLE fHandle; + size_t fSize; + size_t fRead = 0; + size_t fRemaining; + auto cursor = this->store; + OVERLAPPED overlap; + fHandle = CreateFileA( + (LPSTR)path, + GENERIC_READ, + (FILE_SHARE_DELETE|FILE_SHARE_READ|FILE_SHARE_WRITE), + NULL, + OPEN_ALWAYS, + FILE_ATTRIBUTE_NORMAL, + NULL); + if (fHandle == INVALID_HANDLE_VALUE) { + return -1; + } + + if (!GetFileSizeEx(fHandle, reinterpret_cast(&fSize))) { + CloseHandle(fHandle); + return -1; + } + + this->SetAlloc(fSize); + + fRemaining = fSize; + while (fRemaining != 0) { + overlap.Offset = (fSize - fRemaining); + if (!ReadFile(fHandle, cursor, fSize, reinterpret_cast(&fRead), &overlap)) { + CloseHandle(fHandle); + this->Destroy(); + return -1; + } + + cursor += fRead; + fRemaining -= fRead; + } + + CloseHandle(fHandle); + return 0; } #endif @@ -227,7 +268,7 @@ operator<<(std::ostream &os, Arena &arena) { auto cursor = arena.Store(); char cursorString[33] = {0}; - snprintf(cursorString, 32, "%#016lx", cursor); + snprintf(cursorString, 32, "%#016llx", cursor); os << "Arena<"; switch (arena.Type()) { diff --git a/Arena.h b/Arena.h index e02e237..c682ac9 100644 --- a/Arena.h +++ b/Arena.h @@ -1,3 +1,9 @@ +/// @file Arena.h +/// @author K. Isom +/// @brief Memory management using an arena. +/// @section DESCRIPTION +/// Arena defines a memory management backend for pre-allocating memory. + #ifndef KIMODEM_ARENA_H #define KIMODEM_ARENA_H @@ -41,6 +47,8 @@ public: int MemoryMap(int memFileDes, size_t memSize); // Arena will own fd. int Create(const char *path, size_t fileSize, mode_t mode); int Open(const char *path); +#elif defined(__WIN64__) || defined(__WIN32__) + int Open(const char *path); #endif uint8_t *NewCursor() const { return this->store; } diff --git a/Buffer.cc b/Buffer.cc index 478872e..df40e00 100644 --- a/Buffer.cc +++ b/Buffer.cc @@ -4,14 +4,17 @@ #include #include -#include "Buffer.h" +#include #include +#include + +#include "Buffer.h" namespace klib { -constexpr size_t defaultCapacity = 32; -constexpr size_t maxReasonableLine = 8192; +constexpr size_t defaultCapacity = 32; +constexpr size_t maxReasonableLine = 8192; static size_t @@ -21,7 +24,7 @@ nearestPower(size_t x) return 0; } - std::cout << "x -> "; + std::cout << "x -> "; x--; @@ -32,48 +35,48 @@ nearestPower(size_t x) x |= x >> 16; x |= x >> 32; - std::cout << x + 1 << std::endl; + std::cout << x + 1 << std::endl; - return x+1; + return x + 1; } Buffer::Buffer() : contents(nullptr), length(0), capacity(0) { - this->Resize(defaultCapacity); + this->Resize(defaultCapacity); } Buffer::Buffer(size_t initialCapacity) : contents(nullptr), length(0), capacity(0) { - this->Resize(initialCapacity); + this->Resize(initialCapacity); } Buffer::Buffer(const char *data) : contents(nullptr), length(0), capacity(0) { - size_t datalen = strnlen(data, maxReasonableLine); + size_t datalen = strnlen(data, maxReasonableLine); - this->Append((uint8_t *)data, datalen); + this->Append((uint8_t *) data, datalen); } bool Buffer::Append(const char *s) { - size_t slen = strnlen(s, maxReasonableLine); + size_t slen = strnlen(s, maxReasonableLine); - return this->Append((uint8_t *)s, slen); + return this->Append((uint8_t *) s, slen); } bool Buffer::Append(uint8_t *data, size_t datalen) { - auto resized = false; - auto newCap = this->mustGrow(datalen); + auto resized = false; + auto newCap = this->mustGrow(datalen); if (newCap > 0) { this->Resize(newCap); @@ -96,16 +99,16 @@ Buffer::Append(uint8_t c) bool Buffer::Insert(size_t index, const char *s) { - size_t slen = strnlen(s, maxReasonableLine); + size_t slen = strnlen(s, maxReasonableLine); - return this->Insert(index, (uint8_t *)(s), slen); + return this->Insert(index, (uint8_t *) (s), slen); } bool Buffer::Insert(size_t index, uint8_t *data, size_t datalen) { - auto resized = this->shiftRight(index, datalen); + auto resized = this->shiftRight(index, datalen); memcpy(this->contents + index, data, datalen); this->length += datalen; @@ -140,7 +143,7 @@ void Buffer::Resize(size_t newCapacity) { if (newCapacity < this->length) { - return; + newCapacity = nearestPower(this->length + newCapacity); } auto newContents = new uint8_t[newCapacity]; @@ -151,6 +154,7 @@ Buffer::Resize(size_t newCapacity) } delete this->contents; + this->contents = nullptr; this->contents = newContents; this->capacity = newCapacity; } @@ -158,7 +162,7 @@ Buffer::Resize(size_t newCapacity) size_t Buffer::Trim() { - size_t projectedCapacity = nearestPower(this->length); + size_t projectedCapacity = nearestPower(this->length); assert(projectedCapacity >= length); @@ -173,9 +177,9 @@ Buffer::Trim() void Buffer::Clear() { - if (this->length == 0) { - return; - } + if (this->length == 0) { + return; + } memset(this->contents, 0, this->length); this->length = 0; } @@ -183,15 +187,23 @@ Buffer::Clear() void Buffer::Reclaim() { + std::cout << "clear" << std::endl; this->Clear(); - if (this->contents == nullptr) { - assert(this->length == 0); - assert(this->capacity == 0); - return; - } + std::cout << "nullptr check" << std::endl; + if (this->contents == nullptr) { + std::cout << "assert checks" << std::endl; + assert(this->length == 0); + assert(this->capacity == 0); + return; + } + + std::cout << "delete " << this->Capacity() << "B" << std::endl; + this->HexDump(std::cout); delete this->contents; - this->contents = nullptr; + std::cout << "reset contents" << std::endl; + this->contents = nullptr; + std::cout << "reset capacity" << std::endl; this->capacity = 0; } @@ -207,20 +219,56 @@ Buffer::mustGrow(size_t delta) const } +void +Buffer::HexDump(std::ostream &os) +{ +#ifndef NDEBUG + size_t index = 0; + + os << std::hex; + os << std::setfill('0'); + + for (index = 0; index < this->length; index++) { + bool eol = (index % 16) == 0; + if (eol && (index > 0)) { + os << std::endl; + } + + if (eol) { + os << std::setw(8); + os << index << " "; + os << std::setw(2); + } + + os << (unsigned short)this->contents[index]; + + if ((index % 15) != 0 || (index == 0)) { + os << " "; + } + } + + if ((index % 16) != 0) { + os << std::endl; + } + + os << std::setw(0) << std::dec; +#endif +} + bool Buffer::shiftRight(size_t offset, size_t delta) { - auto resized = false; - auto newCap = this->mustGrow(delta); + auto resized = false; + auto newCap = this->mustGrow(delta); if (newCap > 0) { this->Resize(newCap); resized = true; } - if (this->length == 0) return 0; + if (this->length == 0) return 0; - memmove(this->contents+(offset+delta), this->contents+offset, this->length); + memmove(this->contents + (offset + delta), this->contents + offset, this->length); return resized; } @@ -232,13 +280,13 @@ Buffer::shiftLeft(size_t offset, size_t delta) // this->contents[i] = this->contents[i+delta]; // } - memmove(this->contents+offset, this->contents+(offset+delta), this->length); + memmove(this->contents + offset, this->contents + (offset + delta), this->length); return this->Trim() != 0; } -uint8_t& +uint8_t & Buffer::operator[](size_t index) { return this->contents[index]; diff --git a/Buffer.h b/Buffer.h index b059e89..397226a 100644 --- a/Buffer.h +++ b/Buffer.h @@ -5,6 +5,7 @@ #ifndef KGE_BUFFER_H #define KGE_BUFFER_H +#include #include @@ -39,6 +40,8 @@ public: void Clear(); void Reclaim(); + void HexDump(std::ostream &os); + uint8_t &operator[](size_t index); private: size_t mustGrow(size_t delta) const; diff --git a/CMakeDocs.txt b/CMakeDocs.txt new file mode 100644 index 0000000..a20bd27 --- /dev/null +++ b/CMakeDocs.txt @@ -0,0 +1,10 @@ +# Doxygen support for klib. +find_package(Doxygen) + +if (${DOXYGEN_FOUND}) + +doxygen_add_docs(klib_docs + ${HEADER_FILES} ${SOURCE_FILES} + USE_STAMP_FILE) + +endif () \ No newline at end of file diff --git a/CMakeLists.txt b/CMakeLists.txt index bb281d5..65a5821 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -19,13 +19,22 @@ set(HEADER_FILES Arena.h Buffer.h Dictionary.h + Test.h TLV.h) +set(SOURCE_FILES + Arena.cc + Buffer.cc + Dictionary.cc + Test.cc + TLV.cc) + add_library(klib STATIC Arena.cc Buffer.cc Dictionary.cc - TLV.cc) + TLV.cc +) install(TARGETS klib LIBRARY DESTINATION ${PREFIX}/lib) install(FILES ${HEADER_FILES} DESTINATION include/klib) install(FILES klibConfig.cmake DESTINATION share/klib/cmake) @@ -45,11 +54,12 @@ add_executable(buffer_test bufferTest.cc) target_link_libraries(buffer_test klib) add_test(bufferTest buffer_test) -include(CMakePack.txt) - include(CMakePackageConfigHelpers) write_basic_package_version_file( klibConfig.cmake VERSION ${PACKAGE_VERSION} COMPATIBILITY AnyNewerVersion -) \ No newline at end of file +) + +include(CMakePack.txt) +include(CMakeDocs.txt) \ No newline at end of file diff --git a/Test.cc b/Test.cc new file mode 100644 index 0000000..ba442ba --- /dev/null +++ b/Test.cc @@ -0,0 +1,42 @@ +// +// Created by kyle on 2023-10-09. +// + +#include "Test.h" + + +#include +#include + + +namespace klib { + + +void +TestAssert(bool condition, std::string message = "Assertion failed.") +{ +#if defined(NDEBUG) + if (!condition) { + throw AssertionFailed(message); + } +#else + if (!condition) { + std::cerr << message << std::endl; + } + assert(condition); +#endif +} + + +AssertionFailed::AssertionFailed(std::string message) : msg(message) +{ +} + +char * +AssertionFailed::what() const noexcept +{ + return const_cast(this->msg.c_str()); +} + + +} // namespace klib \ No newline at end of file diff --git a/Test.h b/Test.h new file mode 100644 index 0000000..cec9ee5 --- /dev/null +++ b/Test.h @@ -0,0 +1,31 @@ +// +// Created by kyle on 2023-10-09. +// + +#include + +#ifndef KLIB_TEST_H +#define KLIB_TEST_H + + +namespace klib { + + +void +TestAssert(bool condition, std::string message); + + +class AssertionFailed : public std::exception { +public: + explicit AssertionFailed(std::string message); + + char *what() const noexcept override; + +public: + std::string msg; +}; + + +} // namespace klib + +#endif //KLIB_TEST_H diff --git a/bufferTest.cc b/bufferTest.cc index 9048159..1efc418 100644 --- a/bufferTest.cc +++ b/bufferTest.cc @@ -18,12 +18,26 @@ main(int argc, char *argv[]) std::cout << buffer.Contents() << std::endl; + std::cout << "remove end" << std::endl; buffer.Remove(buffer.Length() - 1); - buffer.Remove(0, 5); - buffer.Insert(0, 'g'); - buffer.Insert(1, (uint8_t *) "oodbye", 6); + std::cout << "remove start" << std::endl; + buffer.Remove(0, 5); + + std::cout << "insert char" << std::endl; + buffer.Insert(0, 'g'); + + std::cout << "insert chunk" << std::endl; + buffer.Insert(1, (uint8_t *) "oodbye", 6); + std::cout << "cruel" << std::endl; + buffer.Insert(9, (uint8_t *)"cruel ", 6); + std::cout << buffer.Contents() << std::endl; + buffer.HexDump(std::cout); + + std::cout << "reclaim" << std::endl; buffer.Reclaim(); + + std::cout << "append" << std::endl; buffer.Append("and now for something completely different..."); std::cout << buffer.Contents() << std::endl; diff --git a/dictionaryTest.cc b/dictionaryTest.cc index b7ef42c..daba3df 100644 --- a/dictionaryTest.cc +++ b/dictionaryTest.cc @@ -48,7 +48,7 @@ main(int argc, const char *argv[]) abort(); } #else - if (AllocNewArena(arena, ARENA_SIZE) == -1) { + if (arena.SetAlloc(ARENA_SIZE) == -1) { abort(); } #endif diff --git a/tlvTest.cc b/tlvTest.cc index f1a40da..04f378c 100644 --- a/tlvTest.cc +++ b/tlvTest.cc @@ -4,10 +4,13 @@ #include #include "Arena.h" +#include "Test.h" #include "TLV.h" #include "testFixtures.h" +using namespace klib; + static uint8_t arenaBuffer[ARENA_SIZE];