Must check validity before setting new size and alignment.

5 files changed, 18 insertions, 9 deletions

diff --git a/vespamalloc/src/tests/overwrite/CMakeLists.txt b/vespamalloc/src/tests/overwrite/CMakeLists.txt
index 29b6ac46eb4..9f8274ea2ce 100644
--- a/vespamalloc/src/tests/overwrite/CMakeLists.txt
+++ b/vespamalloc/src/tests/overwrite/CMakeLists.txt
@@ -13,3 +13,11 @@ vespa_add_executable(vespamalloc_expectsignal-overwrite_app
 vespa_add_test(NAME vespamalloc_overwrite_test_app NO_VALGRIND COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/overwrite_test.sh
                DEPENDS vespamalloc_overwrite_test_app vespamalloc_expectsignal-overwrite_app
                        vespamalloc vespamallocd)
+
+vespa_add_executable(vespamalloc_overwrite_test_with_vespamallocd_app TEST
+    SOURCES
+    overwrite.cpp
+    DEPENDS
+    vespamallocd
+)
+vespa_add_test(NAME vespamalloc_overwrite_test_with_vespamallocd_app NO_VALGRIND COMMAND vespamalloc_overwrite_test_with_vespamallocd_app testmemoryfill)
diff --git a/vespamalloc/src/tests/overwrite/overwrite.cpp b/vespamalloc/src/tests/overwrite/overwrite.cpp
index 84f96fbbb3e..1919a75ab00 100644
--- a/vespamalloc/src/tests/overwrite/overwrite.cpp
+++ b/vespamalloc/src/tests/overwrite/overwrite.cpp
@@ -29,9 +29,7 @@ private:
     void verifyWriteAfterFreeDetection(); // Should abort
 };
 
-Test::~Test()
-{
-}
+Test::~Test() = default;
 
 void Test::testFillValue(char *a)
 {
diff --git a/vespamalloc/src/vespamalloc/malloc/malloc.h b/vespamalloc/src/vespamalloc/malloc/malloc.h
index 27a195ebc5e..df40197bbbd 100644
--- a/vespamalloc/src/vespamalloc/malloc/malloc.h
+++ b/vespamalloc/src/vespamalloc/malloc/malloc.h
@@ -161,11 +161,11 @@ void * MemoryManager<MemBlockPtrT, ThreadListT>::malloc(size_t sz)
     MemBlockPtrT mem;
     ThreadPool & tp = _threadList.getCurrent();
     tp.malloc(mem.adjustSize(sz), mem);
-    mem.setExact(sz);
     if (!mem.validFree()) {
         fprintf(stderr, "Memory %p(%ld) has been tampered with after free.\n", mem.ptr(), mem.size());
         crash();
     }
+    mem.setExact(sz);
     mem.alloc(_prAllocLimit<=mem.adjustSize(sz));
     return mem.ptr();
 }
@@ -176,11 +176,11 @@ void * MemoryManager<MemBlockPtrT, ThreadListT>::malloc(size_t sz, std::align_va
     MemBlockPtrT mem;
     ThreadPool & tp = _threadList.getCurrent();
     tp.malloc(mem.adjustSize(sz, alignment), mem);
-    mem.setExact(sz, alignment);
     if (!mem.validFree()) {
         fprintf(stderr, "Memory %p(%ld) has been tampered with after free.\n", mem.ptr(), mem.size());
         crash();
     }
+    mem.setExact(sz, alignment);
     mem.alloc(_prAllocLimit<=mem.adjustSize(sz, alignment));
     return mem.ptr();
 }
diff --git a/vespamalloc/src/vespamalloc/malloc/memblockboundscheck.cpp b/vespamalloc/src/vespamalloc/malloc/memblockboundscheck.cpp
index 0c608fed5d5..d147bd5ba41 100644
--- a/vespamalloc/src/vespamalloc/malloc/memblockboundscheck.cpp
+++ b/vespamalloc/src/vespamalloc/malloc/memblockboundscheck.cpp
@@ -15,7 +15,8 @@ void MemBlockBoundsCheckBaseTBase::verifyFill() const
     const uint8_t *c(static_cast<const uint8_t *>(ptr())), *e(c+size());
     for(;(c < e) && (*c == _fillValue); c++) { }
     if (c != e) {
-        fprintf(_logFile, "Incorrect fillvalue (%2x) instead of (%2x) at position %ld of %ld\n", *c, _fillValue, c - static_cast<const uint8_t *>(ptr()), size());
+        fprintf(_logFile, "Incorrect fillvalue (%2x) instead of (%2x) at position %ld(%p) of %ld(%p - %p)\n",
+                *c, _fillValue, c - static_cast<const uint8_t *>(ptr()), c, size(), ptr(), e);
         abort();
     }
 }
diff --git a/vespamalloc/src/vespamalloc/malloc/memblockboundscheck.h b/vespamalloc/src/vespamalloc/malloc/memblockboundscheck.h
index d64eb592b3b..1860f2f36d3 100644
--- a/vespamalloc/src/vespamalloc/malloc/memblockboundscheck.h
+++ b/vespamalloc/src/vespamalloc/malloc/memblockboundscheck.h
@@ -25,8 +25,8 @@ public:
     size_t size()                   const { return static_cast<const uint32_t *>(_ptr)[0]; }
     size_t alignment()              const { return static_cast<const uint32_t *>(_ptr)[1]; }
     int threadId()                  const { return static_cast<int*>(_ptr)[2]; }
-    Stack * callStack()                   { return reinterpret_cast<Stack *>((char *)_ptr + size() + 4*sizeof(unsigned)); }
-    const Stack * callStack()       const { return reinterpret_cast<const Stack *>((const char *)_ptr + size() + 4*sizeof(unsigned)); }
+    Stack * callStack()                   { return reinterpret_cast<Stack *>((char *)_ptr + size() + alignment()); }
+    const Stack * callStack()       const { return reinterpret_cast<const Stack *>((const char *)_ptr + size() + alignment()); }
     void fillMemory(size_t sz) {
         if (_fillValue != NO_FILL) {
             memset(ptr(), _fillValue, sz);
@@ -167,7 +167,9 @@ protected:
     static constexpr size_t overhead(std::align_val_t alignment) {
         return preambleOverhead(alignment) + postambleOverhead();
     }
-    void setTailMagic() { *(reinterpret_cast<unsigned *> ((char*)_ptr + size() + alignment() + StackTraceLen*sizeof(void *))) = TAIL_MAGIC; }
+    void setTailMagic() {
+        *(reinterpret_cast<unsigned *> ((char*)_ptr + size() + alignment() + StackTraceLen*sizeof(void *))) = TAIL_MAGIC;
+    }
     void init(size_t sz, size_t alignment) {
         if (_ptr) {
             setSize(sz);