define non-sized deallocator and memory counting debugging

 * it turned out to be possible to provoke memory_usage
   overflows or underruns, resulting in unrecoverable states
 * a possible reason can be that at least with G++ 5.4.0,
   the compiler would sometimes call the (default) non-sized
   delete followed by our custom sized delete/deallocator.
 * This was true even after compiling Scintilla with -fsized-deallocation.
 * therefore we provide an empty non-sized delete now.
 * memory_usage counting can now be debugged by uncommenting
   DEBUG_MAGIC in memory.cpp. This uses a magic value to detect
   instrumented allocations being mixed with non-instrumented
   allocations.
 * simplified the global sized-deallocation functions
   (they are identical to the Object-class allocators).

2 files changed, 39 insertions, 13 deletions

diff --git a/src/memory.cpp b/src/memory.cpp
index 77980b5..b592d9e 100644
--- a/src/memory.cpp
+++ b/src/memory.cpp
@@ -45,6 +45,15 @@
 
 namespace SciTECO {
 
+/*
+ * Define this to prefix each heap object allocated
+ * by the custom allocators with a magic value.
+ * This helps to detect non-matching calls to the
+ * overridden new/delete operators which can cause
+ * underruns of the memory counter.
+ */
+//#define DEBUG_MAGIC ((guintptr)0xDEAD15DE5E1BEAF0)
+
 MemoryLimit memlimit;
 
 /*
@@ -235,6 +244,11 @@ Object::operator new(size_t size) noexcept
 	memory_usage += size;
 #endif
 
+#ifdef DEBUG_MAGIC
+	guintptr *ptr = (guintptr *)g_malloc(sizeof(guintptr) + size);
+	*ptr = DEBUG_MAGIC;
+	return ptr + 1;
+#else
 	/*
 	 * Since we've got the sized-delete operator
 	 * below, we could allocate via g_slice.
@@ -254,11 +268,19 @@ Object::operator new(size_t size) noexcept
 	 * from memory limit exhaustions.
 	 */
 	return g_malloc(size);
+#endif
 }
 
 void
 Object::operator delete(void *ptr, size_t size) noexcept
 {
+#ifdef DEBUG_MAGIC
+	if (ptr) {
+		ptr = (guintptr *)ptr - 1;
+		g_assert(*(guintptr *)ptr == DEBUG_MAGIC);
+	}
+#endif
+
 	g_free(ptr);
 
 #ifdef MEMORY_USAGE_FALLBACK
@@ -284,23 +306,25 @@ Object::operator delete(void *ptr, size_t size) noexcept
 void *
 operator new(size_t size)
 {
-#ifdef MEMORY_USAGE_FALLBACK
-	SciTECO::memory_usage += size;
-#endif
-
-	return g_malloc(size);
+	return SciTECO::Object::operator new(size);
 }
 
 void
 operator delete(void *ptr, size_t size) noexcept
 {
-	g_free(ptr);
-
-#ifdef MEMORY_USAGE_FALLBACK
-	SciTECO::memory_usage -= size;
-#endif
+	SciTECO::Object::operator delete(ptr, size);
 }
 
+/*
+ * FIXME: I'm a bit puzzled of why this is necessary.
+ * Apparently, G++ sometimes calls the non-sized,
+ * FOLLOWED BY the sized deallocator even when
+ * building Scintilla with -fsized-deallocation.
+ * It is still uncertain whether the compiler may
+ * call the non-sized WITHOUT the sized variant.
+ */
+void operator delete(void *ptr) noexcept {}
+
 #else
 /*
  * In strict C++11, we can still use global non-sized
diff --git a/src/memory.h b/src/memory.h
index 903fd16..693a208 100644
--- a/src/memory.h
+++ b/src/memory.h
@@ -40,9 +40,11 @@ namespace SciTECO {
  * scalars (e.g. new char[5]).
  *
  * C++14 (supported by GCC >= 5) has global sized delete
- * replacements which would be effective in the entire application
- * but we're still using the base-class approach since
- * we must support the older compilers anyway.
+ * replacements which would be effective in the entire application.
+ * We're using them too if support is detected and there is
+ * also a fallback using malloc_usable_size().
+ * Another fallback with a size field would be possible
+ * but is probably not worth the trouble.
  */
 class Object {
 public: