Fixed the new memory safety.

Source/DFPSR/base/SafePointer.cpp

@@ -36,8 +36,9 @@ static uint64_t ANY_THREAD_HASH = 0xF986BA1496E872A5;
 
 #ifdef SAFE_POINTER_CHECKS
 	// Hashed thread identity.
+	// TODO: Create a function for geterating a better thread hash with better entropy.
 	std::hash<std::thread::id> hasher;
-	thread_local uint64_t threadHash = hasher(std::this_thread::get_id());
+	thread_local const uint64_t currentThreadHash = 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,
@@ -57,8 +58,7 @@ static uint64_t ANY_THREAD_HASH = 0xF986BA1496E872A5;
 	: totalSize(0), threadHash(0), allocationIdentity(0) {}
 
 	AllocationHeader::AllocationHeader(uintptr_t totalSize, bool threadLocal)
-	: totalSize(totalSize), threadHash(threadLocal ? threadHash : ANY_THREAD_HASH), allocationIdentity(createIdentity()) {
-	}
+	: totalSize(totalSize), threadHash(threadLocal ? currentThreadHash : ANY_THREAD_HASH), allocationIdentity(createIdentity()) {}
 #else
 	AllocationHeader::AllocationHeader()
 	: totalSize(0) {}
@@ -75,28 +75,28 @@ static uint64_t ANY_THREAD_HASH = 0xF986BA1496E872A5;
 	}
 
 	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 (regionStart == nullptr) {
+			throwError(U"SafePointer exception! Tried to use a null pointer!\n");
+			return;
+		}
 		// 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.
+			uint64_t headerIdentity, headerHash;
 			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;
-				}
+				headerIdentity = header->allocationIdentity;
+				headerHash = header->threadHash;
 			} catch(...) {
 				throwError(U"SafePointer exception! Tried to access memory not available to the application!\n");
 				return;
 			}
-		}
-		if (regionStart == nullptr) {
-			throwError(U"SafePointer exception! Tried to use a null pointer!\n");
-			return;
+			if (headerIdentity != allocationIdentity) {
+				throwError(U"SafePointer exception! Accessing freed memory or corrupted allocation header!\n  headerIdentity = ", headerIdentity, U"\n  allocationIdentity = ", allocationIdentity, U"");
+				return;
+			} else if (headerHash != ANY_THREAD_HASH && headerHash != currentThreadHash) {
+				throwError(U"SafePointer exception! Accessing another thread's private memory!\n  headerHash = ", headerHash, U"\n  currentThreadHash = ", currentThreadHash, U"\n");
+				return;
+			}
 		}
 		if (pointer < regionStart || pointer + claimedSize > regionEnd) {
 			String message;

Source/DFPSR/base/SafePointer.h

@@ -77,16 +77,16 @@ 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;
 		// Pointer to an ascii literal containing the name for improving error messages for crashes in debug mode.
 		mutable const char *name;
+		// 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;
 	#endif
 public:
 	#ifdef SAFE_POINTER_CHECKS

Source/DFPSR/base/virtualStack.cpp

@@ -72,26 +72,27 @@ namespace dsr {
 		pointer += totalSize;
 	}
 
-	static uint8_t *stackAllocate(StackMemory& stack, uint64_t paddedSize, uintptr_t alignmentAndMask) {
+	static UnsafeAllocation stackAllocate(StackMemory& stack, uint64_t paddedSize, uintptr_t alignmentAndMask) {
 		uint8_t *newStackPointer = stack.stackPointer;
 		// Allocate memory for payload.
 		uint64_t payloadTotalSize = increaseStackPointer(newStackPointer, paddedSize, alignmentAndMask);
 		// Get a pointer to the payload.
-		uint8_t *result = newStackPointer;
+		uint8_t *data = newStackPointer;
 		// Allocate memory for header.
 		uint64_t headerTotalSize = increaseStackPointer(newStackPointer, stackHeaderPaddedSize, stackHeaderAlignmentAndMask);
 		// Check that we did not run out of memory.
 		if (newStackPointer < stack.top) {
 			// Not enough space.
-			return nullptr;
+			return UnsafeAllocation(nullptr, nullptr);
 		} else {
 			stack.stackPointer = newStackPointer;
 			// Write the header to memory.
-			*((AllocationHeader*)stack.stackPointer) = AllocationHeader(payloadTotalSize + headerTotalSize, true);
+			AllocationHeader *header = (AllocationHeader*)(stack.stackPointer);
+			*header = AllocationHeader(payloadTotalSize + headerTotalSize, true);
 			// Clear the new allocation for determinism.
-			std::memset((void*)result, 0, payloadTotalSize);
+			std::memset((void*)data, 0, payloadTotalSize);
 			// Return a pointer to the payload.
-			return result;
+			return UnsafeAllocation(data, header);
 		}
 	}
 
@@ -99,11 +100,11 @@ namespace dsr {
 	thread_local DynamicStackMemory dynamicMemory[MAX_EXTRA_STACKS]; // Index 0..MAX_EXTRA_STACKS-1
 	thread_local int32_t stackIndex = -1;
 
-	uint8_t *virtualStack_push(uint64_t paddedSize, uintptr_t alignmentAndMask) {
+	UnsafeAllocation virtualStack_push(uint64_t paddedSize, uintptr_t alignmentAndMask) {
 		if (stackIndex < 0) {
-			uint8_t *result = stackAllocate(fixedMemory, paddedSize, alignmentAndMask);
+			UnsafeAllocation result = stackAllocate(fixedMemory, paddedSize, alignmentAndMask);
 			// Check that we did not run out of memory.
-			if (result == nullptr) {
+			if (result.data == nullptr) {
 				// Not enough space in thread local memory. Moving to the first dynamic stack.
 				stackIndex = 0;
 				goto allocateDynamic;
@@ -126,7 +127,7 @@ namespace dsr {
 			uint8_t *newMemory = (uint8_t*)malloc(regionSize);
 			if (newMemory == nullptr) {
 				throwError(U"Failed to allocate ", regionSize, U" bytes of heap memory for expanding the virtual stack when trying to allocate ", paddedSize, " bytes!\n");
-				return nullptr;
+				return UnsafeAllocation(nullptr, nullptr);
 			} else {
 				// Keep the new allocation.
 				dynamicMemory[stackIndex].top = newMemory;
@@ -137,11 +138,11 @@ namespace dsr {
 		}
 		assert(dynamicMemory[stackIndex].stackPointer != nullptr);
 		// Allocate memory.
-		uint8_t *result = stackAllocate(dynamicMemory[stackIndex], paddedSize, alignmentAndMask);
-		if (result == nullptr) {
+		UnsafeAllocation result = stackAllocate(dynamicMemory[stackIndex], paddedSize, alignmentAndMask);
+		if (result.data == nullptr) {
 			if (stackIndex >= MAX_EXTRA_STACKS - 1) {
 				throwError(U"Exceeded MAX_EXTRA_STACKS to allocate more heap memory for a thread local virtual stack!\n");
-				return nullptr;
+				return UnsafeAllocation(nullptr, nullptr);
 			} else {
 				stackIndex++;
 				goto allocateDynamic;

Source/DFPSR/base/virtualStack.h

@@ -49,10 +49,16 @@ namespace dsr {
 		return ~(alignment - 1);
 	}
 
+	struct UnsafeAllocation {
+		uint8_t *data;
+		AllocationHeader *header;
+		UnsafeAllocation(uint8_t *data, AllocationHeader *header)
+		: data(data), header(header) {}
+	};
 	// Allocate memory in the virtual stack owned by the current thread.
 	//   paddedSize is the number of bytes to allocate including all elements and internal padding.
 	//   alignmentMask should only contain zeroes at the bits to round away for alignment.
-	uint8_t *virtualStack_push(uint64_t paddedSize, uintptr_t alignmentAndMask);
+	UnsafeAllocation virtualStack_push(uint64_t paddedSize, uintptr_t alignmentAndMask);
 
 	// A simpler way to get the correct alignment is to allocate a number of elements with a specific type.
 	// TODO: Create another function for manual alignment exceeding the type's alignment using another template argument.
@@ -63,9 +69,9 @@ namespace dsr {
 		// Calculate element size and multiply by element count to get the total size.
 		uint64_t paddedSize = memory_getPaddedSize<T>() * elementCount;
 		// Allocate the data with the amount of alignment requested by the element type T.
-		uint8_t *data = virtualStack_push(paddedSize, memory_createAlignmentAndMask((uintptr_t)alignof(T)));
+		UnsafeAllocation result = virtualStack_push(paddedSize, memory_createAlignmentAndMask((uintptr_t)alignof(T)));
 		// Return a safe pointer to the allocated data.
-		return SafePointer<T>(name, (T*)data, (intptr_t)paddedSize);
+		return SafePointer<T>(name, (T*)(result.data), (intptr_t)paddedSize, result.header);
 	}
 
 	// Free the last allocation from the virtual stack.

Source/test/tests/VirtualStackTest.cpp

@@ -17,8 +17,10 @@ START_TEST(VirtualStack)
 			x[6] = 67;
 			x[7] = 78;
 			x[8] = 89;
-			VirtualStackAllocation<int32_t> y(3);
+			SafePointer<int32_t> pointerY;
 			{
+				VirtualStackAllocation<int32_t> y(3);
+				pointerY = y;
 				ASSERT_EQUAL((uintptr_t)(y.getUnsafe()) % alignof(int32_t), 0);
 				y[0] =  2147483000;
 				y[1] = -2147483000;
@@ -33,9 +35,9 @@ START_TEST(VirtualStack)
 					ASSERT_CRASH(y[3]);
 				#endif
 			}
-			//#ifdef SAFE_POINTER_CHECKS
-			// TODO: Implement and test memory safety against deallocated data by remembering the allocation's identity in the pointer and erasing it when freeing.
-			//#endif
+			#ifdef SAFE_POINTER_CHECKS
+				ASSERT_CRASH(pointerY[0]);
+			#endif
 		}
 		#ifdef SAFE_POINTER_CHECKS
 			ASSERT_CRASH(x[-1]);