Moved the allocation header to SafePointer and made it reusable for other allocators.

Source/DFPSR/base/SafePointer.cpp

@@ -24,55 +24,117 @@
 #include "SafePointer.h"
 #include "../api/stringAPI.h"
 
+#ifdef SAFE_POINTER_CHECKS
+	#include <thread>
+	#include <mutex>
+#endif
+
 using namespace dsr;
 
-void dsr::assertNonNegativeSize(intptr_t size) {
-	if (size < 0) {
-		throwError(U"Negative size of SafePointer!\n");
+// Thread hash of memory without any specific owner.
+static uint64_t ANY_THREAD_HASH = 0xF986BA1496E872A5;
+
+#ifdef SAFE_POINTER_CHECKS
+	// Hashed thread identity.
+	std::hash<std::thread::id> hasher;
+	thread_local uint64_t threadHash = hasher(std::this_thread::get_id());
+
+	// Globally unique identifiers for memory allocations.
+	// Different allocations can have the same address at different times when allocations are recycled,
+	//   so a globally unique identifier is needed to make sure that we access the same allocation.
+	static std::mutex idLock;
+	static uint64_t idCounter = 0xD13A98271E08BF57;
+	static uint64_t createIdentity() {
+		uint64_t result;
+		idLock.lock();
+			result = idCounter;
+			idCounter++;
+		idLock.unlock();
+		return result;
 	}
-}
 
-void dsr::assertInsideSafePointer(const char* method, const char* name, const uint8_t* pointer, const uint8_t* data, const uint8_t* regionStart, const uint8_t* regionEnd, intptr_t claimedSize, intptr_t elementSize) {
-	if (regionStart == nullptr || pointer < regionStart || pointer + claimedSize > regionEnd) {
-		String message;
+	AllocationHeader::AllocationHeader()
+	: totalSize(0), threadHash(0), allocationIdentity(0) {}
+
+	AllocationHeader::AllocationHeader(uintptr_t totalSize, bool threadLocal)
+	: totalSize(totalSize), threadHash(threadLocal ? threadHash : ANY_THREAD_HASH), allocationIdentity(createIdentity()) {
+	}
+#else
+	AllocationHeader::AllocationHeader()
+	: totalSize(0) {}
+
+	AllocationHeader::AllocationHeader(uintptr_t totalSize, bool threadLocal)
+	: totalSize(totalSize) {}
+#endif
+
+#ifdef SAFE_POINTER_CHECKS
+	void dsr::assertNonNegativeSize(intptr_t size) {
+		if (size < 0) {
+			throwError(U"Negative size of SafePointer!\n");
+		}
+	}
+
+	void dsr::assertInsideSafePointer(const char* method, const char* name, const uint8_t* pointer, const uint8_t* data, const uint8_t* regionStart, const uint8_t* regionEnd, const AllocationHeader *header, uint64_t allocationIdentity, intptr_t claimedSize, intptr_t elementSize) {
+		// If the pointer has an allocation header, check that the identity matches the one stored in the pointer.
+		if (header != nullptr) {
+			// TODO: Print more useful information.
+			try {
+				// Both allocation identity and thread hash may match by mistake, but in most of the cases this will give more information about why it happened.
+				uint64_t headerIdentity = header->allocationIdentity;
+				uint64_t headerHash = header->threadHash;
+				if (headerIdentity != allocationIdentity) {
+					throwError(U"SafePointer exception! Accessing freed memory or currupted allocation header!\n");
+					return;
+				} else if (headerHash != ANY_THREAD_HASH && headerHash != threadHash) {
+					throwError(U"SafePointer exception! Accessing another thread's private memory!\n");
+					return;
+				}
+			} catch(...) {
+				throwError(U"SafePointer exception! Tried to access memory not available to the application!\n");
+				return;
+			}
+		}
 		if (regionStart == nullptr) {
-			string_append(message, U"\n _____________________ SafePointer null exception! _____________________\n");
-		} else {
-			string_append(message, U"\n _________________ SafePointer out of bound exception! _________________\n");
+			throwError(U"SafePointer exception! Tried to use a null pointer!\n");
+			return;
 		}
-		string_append(message, U"/\n");
-		string_append(message, U"|  Name: ", name, U"\n");
-		string_append(message, U"|  Method: ", method, U"\n");
-		string_append(message, U"|  Region: ", (uintptr_t)regionStart, U" to ", (uintptr_t)regionEnd, U"\n");
-		string_append(message, U"|  Region size: ", (intptr_t)(regionEnd - regionStart), U" bytes\n");
-		string_append(message, U"|  Base pointer: ", (uintptr_t)data, U"\n");
-		string_append(message, U"|  Requested pointer: ", (uintptr_t)pointer, U"\n");
-		string_append(message, U"|  Requested size: ", claimedSize, U" bytes\n");
+		if (pointer < regionStart || pointer + claimedSize > regionEnd) {
+			String message;
+			string_append(message, U"\n _________________ SafePointer out of bound exception! _________________\n");
+			string_append(message, U"/\n");
+			string_append(message, U"|  Name: ", name, U"\n");
+			string_append(message, U"|  Method: ", method, U"\n");
+			string_append(message, U"|  Region: ", (uintptr_t)regionStart, U" to ", (uintptr_t)regionEnd, U"\n");
+			string_append(message, U"|  Region size: ", (intptr_t)(regionEnd - regionStart), U" bytes\n");
+			string_append(message, U"|  Base pointer: ", (uintptr_t)data, U"\n");
+			string_append(message, U"|  Requested pointer: ", (uintptr_t)pointer, U"\n");
+			string_append(message, U"|  Requested size: ", claimedSize, U" bytes\n");
 
-		intptr_t startOffset = (intptr_t)pointer - (intptr_t)regionStart;
-		intptr_t baseOffset = (intptr_t)pointer - (intptr_t)data;
+			intptr_t startOffset = (intptr_t)pointer - (intptr_t)regionStart;
+			intptr_t baseOffset = (intptr_t)pointer - (intptr_t)data;
 
-		// Index relative to allocation start
-		//   regionStart is the start of the accessible memory region
-		if (startOffset != baseOffset) {
-			string_append(message, U"|  Start offset: ", startOffset, U" bytes\n");
-			if (startOffset % elementSize == 0) {
-				intptr_t index = startOffset / elementSize;
-				intptr_t elementCount = ((intptr_t)regionEnd - (intptr_t)regionStart) / elementSize;
-				string_append(message, U"|    Start index: ", index, U" [0..", (elementCount - 1), U"]\n");
+			// Index relative to allocation start
+			//   regionStart is the start of the accessible memory region
+			if (startOffset != baseOffset) {
+				string_append(message, U"|  Start offset: ", startOffset, U" bytes\n");
+				if (startOffset % elementSize == 0) {
+					intptr_t index = startOffset / elementSize;
+					intptr_t elementCount = ((intptr_t)regionEnd - (intptr_t)regionStart) / elementSize;
+					string_append(message, U"|    Start index: ", index, U" [0..", (elementCount - 1), U"]\n");
+				}
 			}
-		}
 
-		// Base index relative to the stored pointer within the region
-		//   data is the base of the allocation at index zero
-		string_append(message, U"|  Base offset: ", baseOffset, U" bytes\n");
-		if (baseOffset % elementSize == 0) {
-			intptr_t index = baseOffset / elementSize;
-			intptr_t elementCount = ((intptr_t)regionEnd - (intptr_t)data) / elementSize;
-			string_append(message, U"|    Base index: ", index, U" [0..", (elementCount - 1), U"]\n");
+			// Base index relative to the stored pointer within the region
+			//   data is the base of the allocation at index zero
+			string_append(message, U"|  Base offset: ", baseOffset, U" bytes\n");
+			if (baseOffset % elementSize == 0) {
+				intptr_t index = baseOffset / elementSize;
+				intptr_t elementCount = ((intptr_t)regionEnd - (intptr_t)data) / elementSize;
+				string_append(message, U"|    Base index: ", index, U" [0..", (elementCount - 1), U"]\n");
+			}
+			string_append(message, U"\\_______________________________________________________________________\n\n");
+			throwError(message);
+			return;
 		}
-		string_append(message, U"\\_______________________________________________________________________\n\n");
-		throwError(message);
 	}
-}
-
+#endif

Source/DFPSR/base/SafePointer.h

@@ -52,9 +52,23 @@
 
 namespace dsr {
 
-// Generic implementaions
-void assertInsideSafePointer(const char* method, const char* name, const uint8_t* pointer, const uint8_t* data, const uint8_t* regionStart, const uint8_t* regionEnd, intptr_t claimedSize, intptr_t elementSize);
-void assertNonNegativeSize(intptr_t size);
+struct AllocationHeader {
+	uintptr_t totalSize; // Size of both header and payload.
+	#ifdef SAFE_POINTER_CHECKS
+		uint64_t threadHash; // Hash of the owning thread identity for thread local memory, 0 for shared memory.
+		uint64_t allocationIdentity; // Rotating identity of the allocation, to know if the memory has been freed and reused within a memory allocator.
+	#endif
+	// Header for freed memory.
+	AllocationHeader();
+	// Header for allocated memory.
+	// threadLocal should be true iff the memory may not be accessed from other threads, such as virtual stack memory.
+	AllocationHeader(uintptr_t totalSize, bool threadLocal);
+};
+
+#ifdef SAFE_POINTER_CHECKS
+	void assertInsideSafePointer(const char* method, const char* name, const uint8_t* pointer, const uint8_t* data, const uint8_t* regionStart, const uint8_t* regionEnd, const AllocationHeader *header, uint64_t allocationIdentity, intptr_t claimedSize, intptr_t elementSize);
+	void assertNonNegativeSize(intptr_t size);
+#endif
 
 template<typename T>
 class SafePointer {
@@ -63,30 +77,43 @@ private:
 	//   Mutable because only the data being pointed to is write protected in a const SafePointer
 	mutable T *data;
 	#ifdef SAFE_POINTER_CHECKS
+		// Optional pointer to an allocation header to know if it still exists and which threads are allowed to access it.
+		mutable AllocationHeader *header = nullptr;
+		// The identity that should match the allocation header's identity.
+		mutable uint64_t allocationIdentity = 0;
+		// Points to the first accessible byte, which should have the same alignment as the data pointer.
 		mutable T *regionStart;
+		// Marks the end of the allowed region, pointing to the first byte that is not accessible.
 		mutable T *regionEnd;
-		mutable const char * name;
+		// Pointer to an ascii literal containing the name for improving error messages for crashes in debug mode.
+		mutable const char *name;
 	#endif
 public:
 	#ifdef SAFE_POINTER_CHECKS
 	SafePointer() : data(nullptr), regionStart(nullptr), regionEnd(nullptr), name("Unnamed null pointer") {}
 	explicit SafePointer(const char* name) : data(nullptr), regionStart(nullptr), regionEnd(nullptr), name(name) {}
-	SafePointer(const char* name, T* regionStart, intptr_t regionByteSize = sizeof(T)) : data(regionStart), regionStart(regionStart), regionEnd((T*)(((uint8_t*)regionStart) + (intptr_t)regionByteSize)), name(name) {
+	SafePointer(const char* name, T* regionStart, intptr_t regionByteSize = sizeof(T), AllocationHeader *header = nullptr)
+	: data(regionStart), regionStart(regionStart), regionEnd((T*)(((uint8_t*)regionStart) + (intptr_t)regionByteSize)), name(name), header(header) {
 		assertNonNegativeSize(regionByteSize);
+		// If the pointer has a header, then store the allocation's identity in the pointer.
+		if (header != nullptr) {
+			this->allocationIdentity = header->allocationIdentity;
+		}
 	}
-	SafePointer(const char* name, T* regionStart, intptr_t regionByteSize, T* data) : data(data), regionStart(regionStart), regionEnd((T*)(((uint8_t*)regionStart) + (intptr_t)regionByteSize)), name(name) {
+	SafePointer(const char* name, T* regionStart, intptr_t regionByteSize, T* data, AllocationHeader *header = nullptr)
+	: data(data), regionStart(regionStart), regionEnd((T*)(((uint8_t*)regionStart) + (intptr_t)regionByteSize)), name(name), header(header) {
 		assertNonNegativeSize(regionByteSize);
 	}
 	#else
 	SafePointer() : data(nullptr) {}
 	explicit SafePointer(const char* name) : data(nullptr) {}
-	SafePointer(const char* name, T* regionStart, intptr_t regionByteSize = sizeof(T)) : data(regionStart) {}
-	SafePointer(const char* name, T* regionStart, intptr_t regionByteSize, T* data) : data(data) {}
+	SafePointer(const char* name, T* regionStart, intptr_t regionByteSize = sizeof(T), AllocationHeader *header = nullptr) : data(regionStart) {}
+	SafePointer(const char* name, T* regionStart, intptr_t regionByteSize, T* data, AllocationHeader *header = nullptr) : data(data) {}
 	#endif
 public:
 	#ifdef SAFE_POINTER_CHECKS
 	inline void assertInside(const char* method, const T* pointer, intptr_t size = (intptr_t)sizeof(T)) const {
-		assertInsideSafePointer(method, this->name, (const uint8_t*)pointer, (const uint8_t*)this->data, (const uint8_t*)this->regionStart, (const uint8_t*)this->regionEnd, size, sizeof(T));
+		assertInsideSafePointer(method, this->name, (const uint8_t*)pointer, (const uint8_t*)this->data, (const uint8_t*)this->regionStart, (const uint8_t*)this->regionEnd, this->header, this->allocationIdentity, size, sizeof(T));
 	}
 	inline void assertInside(const char* method) const {
 		this->assertInside(method, this->data);
@@ -233,6 +260,8 @@ public:
 	inline const SafePointer<T>& operator=(const SafePointer<T>& source) const {
 		this->data = source.data;
 		#ifdef SAFE_POINTER_CHECKS
+			this->header = source.header;
+			this->allocationIdentity = source.allocationIdentity;
 			this->regionStart = source.regionStart;
 			this->regionEnd = source.regionEnd;
 			this->name = source.name;

Source/DFPSR/base/virtualStack.cpp

@@ -26,43 +26,20 @@
 #include "../api/stringAPI.h"
 
 namespace dsr {
-	// A structure that is placed in front of each stack allocation while allocating backwards along decreasing addresses to allow aligning memory quickly using bit masking.
-	struct StackAllocationHeader {
-		uint32_t totalSize; // Size of both header and payload.
-		#ifdef SAFE_POINTER_CHECKS
-		uint32_t identity; // A unique identifier that can be used to reduce the risk of using a block of memory after it has been freed.
-		#endif
-		StackAllocationHeader(uint32_t totalSize);
-	};
-
 	// How many bytes that are allocated directly in thread local memory.
 	static const uint64_t VIRTUAL_STACK_SIZE = 262144;
 	static const int MAX_EXTRA_STACKS = 63;
 
-	static const uintptr_t stackHeaderPaddedSize = memory_getPaddedSize<StackAllocationHeader>();
-	static const uintptr_t stackHeaderAlignmentAndMask = memory_createAlignmentAndMask((uintptr_t)alignof(StackAllocationHeader));
-
-	#ifdef SAFE_POINTER_CHECKS
-		// In debug mode, each new thread creates a hash from its own identity to catch most of the memory errors in debug mode.
-		std::hash<std::thread::id> hasher;
-		thread_local uint32_t threadIdentity = hasher(std::this_thread::get_id());
-		//   To check the allocation identity, subtract the padded size of the header from the base pointer, cast to the head type and compare to the pointer's identity.
-		thread_local uint32_t nextIdentity = threadIdentity;
-	#endif
-	StackAllocationHeader::StackAllocationHeader(uint32_t totalSize) : totalSize(totalSize) {
-		#ifdef SAFE_POINTER_CHECKS
-			// No identity may be zero, because identity zero is no identity.
-			if (nextIdentity == 0) nextIdentity++;
-			this->identity = nextIdentity;
-			nextIdentity++;
-		#endif
-	}
+	static const uintptr_t stackHeaderPaddedSize = memory_getPaddedSize<AllocationHeader>();
+	static const uintptr_t stackHeaderAlignmentAndMask = memory_createAlignmentAndMask((uintptr_t)alignof(AllocationHeader));
 
 	struct StackMemory {
 		uint8_t *top = nullptr; // The stack pointer is here when completely full.
 		uint8_t *stackPointer = nullptr; // The virtual stack pointer.
 		uint8_t *bottom = nullptr; // The stack pointer is here when empty.
 	};
+
+	// The first block of stack memory in stread local memory.
 	struct FixedStackMemory : public StackMemory {
 		uint8_t data[VIRTUAL_STACK_SIZE];
 		FixedStackMemory() {
@@ -71,9 +48,9 @@ namespace dsr {
 			this->bottom = this->data + VIRTUAL_STACK_SIZE;
 		}
 	};
+
+	// Additional stacks in heap memory.
 	struct DynamicStackMemory : public StackMemory {
-		// Allocate the data dynamically in top when needed.
-		// Clean up when the thread terminates.
 		~DynamicStackMemory() {
 			if (this->top != nullptr) {
 				free(this->top);
@@ -110,7 +87,7 @@ namespace dsr {
 		} else {
 			stack.stackPointer = newStackPointer;
 			// Write the header to memory.
-			*((StackAllocationHeader*)stack.stackPointer) = StackAllocationHeader(payloadTotalSize + headerTotalSize);
+			*((AllocationHeader*)stack.stackPointer) = AllocationHeader(payloadTotalSize + headerTotalSize, true);
 			// Clear the new allocation for determinism.
 			std::memset((void*)result, 0, payloadTotalSize);
 			// Return a pointer to the payload.
@@ -181,9 +158,10 @@ namespace dsr {
 			// If the allocated memory does not fit a header, then it is empty.
 			return false;
 		} else {
-			// TODO: Prevent deallocating past the top.
 			// Read the header.
-			StackAllocationHeader header = *((StackAllocationHeader*)stack.stackPointer);
+			AllocationHeader header = *((AllocationHeader*)stack.stackPointer);
+			// Overwrite the header.
+			*((AllocationHeader*)stack.stackPointer) = AllocationHeader();
 			// Deallocate both header and payload using the stored total size.
 			decreaseStackPointer(stack.stackPointer, header.totalSize);
 			return true;