Merge pull request #6256 from heinrich5991/pr_ddnet_typesafe_mem_zero

Typesafe `mem_zero` function

CMakeLists.txt

@@ -325,6 +325,8 @@ if(NOT MSVC AND NOT HAIKU)
   add_cxx_compiler_flag_if_supported(OUR_FLAGS_OWN -Wthread-safety)
   add_cxx_compiler_flag_if_supported(OUR_FLAGS_OWN -Wthread-safety-negative)
   add_cxx_compiler_flag_if_supported(OUR_FLAGS_OWN -Wsuggest-override)
+  add_cxx_compiler_flag_if_supported(OUR_FLAGS_OWN -Wdynamic-class-memaccess) # clang
+  add_cxx_compiler_flag_if_supported(OUR_FLAGS_OWN -Wclass-memaccess) # gcc
   add_linker_flag_if_supported(OUR_FLAGS_LINK -Wno-alloc-size-larger-than) # save.cpp with LTO
   # add_cxx_compiler_flag_if_supported(OUR_FLAGS_OWN -Wdouble-promotion) # Many occurrences
   # add_cxx_compiler_flag_if_supported(OUR_FLAGS_OWN -Wnull-dereference) # Many occurrences
@@ -2750,6 +2752,7 @@ if(GTEST_FOUND OR DOWNLOAD_GTEST)
     linereader.cpp
     mapbugs.cpp
     math.cpp
+    memory.cpp
     name_ban.cpp
     net.cpp
     netaddr.cpp

src/base/system.cpp

@@ -236,11 +236,6 @@ void mem_move(void *dest, const void *source, size_t size)
 	memmove(dest, source, size);
 }
 
-void mem_zero(void *block, size_t size)
-{
-	memset(block, 0, size);
-}
-
 int mem_comp(const void *a, const void *b, size_t size)
 {
 	return memcmp(a, b, size);

src/base/system.h

@@ -171,7 +171,12 @@ void mem_move(void *dest, const void *source, size_t size);
  * @param block Pointer to the block to zero out.
  * @param size Size of the block.
  */
-void mem_zero(void *block, size_t size);
+template<typename T>
+inline void mem_zero(T *block, size_t size)
+{
+	static_assert((std::is_trivially_constructible<T>::value && std::is_trivially_destructible<T>::value) || std::is_fundamental<T>::value);
+	memset(block, 0, size);
+}
 
 /**
  * Compares two blocks of memory

src/base/vmath.h

@@ -23,11 +23,7 @@ public:
 		T y, v;
 	};
 
-	constexpr vector2_base() :
+	constexpr vector2_base() = default;
-		x(T()), y(T())
-	{
-	}
-
 	constexpr vector2_base(T nx, T ny) :
 		x(nx), y(ny)
 	{
@@ -198,11 +194,7 @@ public:
 		T z, b, l, w;
 	};
 
-	constexpr vector3_base() :
+	constexpr vector3_base() = default;
-		x(T()), y(T()), z(T())
-	{
-	}
-
 	constexpr vector3_base(T nx, T ny, T nz) :
 		x(nx), y(ny), z(nz)
 	{
@@ -326,11 +318,7 @@ public:
 		T w, a;
 	};
 
-	constexpr vector4_base() :
+	constexpr vector4_base() = default;
-		x(T()), y(T()), z(T()), w(T())
-	{
-	}
-
 	constexpr vector4_base(T nx, T ny, T nz, T nw) :
 		x(nx), y(ny), z(nz), w(nw)
 	{

src/engine/client/client.cpp

@@ -4195,9 +4195,7 @@ void CClient::RegisterCommands()
 
 static CClient *CreateClient()
 {
-	CClient *pClient = static_cast<CClient *>(malloc(sizeof(*pClient)));
+	return new CClient;
-	mem_zero(pClient, sizeof(CClient));
-	return new(pClient) CClient;
 }
 
 void CClient::HandleConnectAddress(const NETADDR *pAddr)
@@ -4369,8 +4367,7 @@ int main(int argc, const char **argv)
 	CleanerFunctions.emplace([pKernel, pClient]() {
 		pKernel->Shutdown();
 		delete pKernel;
-		pClient->~CClient();
+		delete pClient;
-		free(pClient);
 	});
 
 	const std::thread::id MainThreadId = std::this_thread::get_id();

src/engine/client/graphics_threaded.cpp

@@ -787,7 +787,6 @@ bool CGraphics_Threaded::ScreenshotDirect()
 {
 	// add swap command
 	CImageInfo Image;
-	mem_zero(&Image, sizeof(Image));
 
 	bool DidSwap = false;
 

src/engine/shared/netban.h

@@ -66,7 +66,7 @@ protected:
 		int m_Hash;
 		int m_HashIndex; // matching parts for ranges, 0 for addr
 
-		CNetHash() {}
+		CNetHash() = default;
 		CNetHash(const NETADDR *pAddr);
 		CNetHash(const CNetRange *pRange);
 

src/engine/shared/network_client.cpp

@@ -12,7 +12,7 @@ bool CNetClient::Open(NETADDR BindAddr)
 		return false;
 
 	// clean it
-	mem_zero(this, sizeof(*this));
+	*this = CNetClient{};
 
 	// init
 	m_Socket = Socket;

src/engine/shared/network_console.cpp

@@ -8,7 +8,7 @@
 bool CNetConsole::Open(NETADDR BindAddr, CNetBan *pNetBan)
 {
 	// zero out the whole structure
-	mem_zero(this, sizeof(*this));
+	*this = CNetConsole{};
 	m_pNetBan = pNetBan;
 
 	// open socket

src/engine/shared/network_server.cpp

@@ -44,7 +44,8 @@ static SECURITY_TOKEN ToSecurityToken(const unsigned char *pData)
 bool CNetServer::Open(NETADDR BindAddr, CNetBan *pNetBan, int MaxClients, int MaxClientsPerIP)
 {
 	// zero out the whole structure
-	mem_zero(this, sizeof(*this));
+	this->~CNetServer();
+	new(this) CNetServer{};
 
 	// open socket
 	m_Socket = net_udp_create(BindAddr);

src/game/client/prediction/entities/character.cpp

@@ -457,7 +457,7 @@ void CCharacter::FireWeapon()
 	break;
 	}
 
-	m_AttackTick = GameWorld()->GameTick();
+	m_AttackTick = GameWorld()->GameTick(); // NOLINT(clang-analyzer-unix.Malloc)
 
 	if(!m_ReloadTimer)
 	{

src/game/server/entities/character.cpp

@@ -526,7 +526,7 @@ void CCharacter::FireWeapon()
 				MouseTarget //InitDir
 			);
 
-			GameServer()->CreateSound(m_Pos, SOUND_GUN_FIRE, TeamMask());
+			GameServer()->CreateSound(m_Pos, SOUND_GUN_FIRE, TeamMask()); // NOLINT(clang-analyzer-unix.Malloc)
 		}
 	}
 	break;
@@ -540,7 +540,7 @@ void CCharacter::FireWeapon()
 			LaserReach = TuningList()[m_TuneZone].m_LaserReach;
 
 		new CLaser(&GameServer()->m_World, m_Pos, Direction, LaserReach, m_pPlayer->GetCID(), WEAPON_SHOTGUN);
-		GameServer()->CreateSound(m_Pos, SOUND_SHOTGUN_FIRE, TeamMask());
+		GameServer()->CreateSound(m_Pos, SOUND_SHOTGUN_FIRE, TeamMask()); // NOLINT(clang-analyzer-unix.Malloc)
 	}
 	break;
 
@@ -578,7 +578,7 @@ void CCharacter::FireWeapon()
 			LaserReach = TuningList()[m_TuneZone].m_LaserReach;
 
 		new CLaser(GameWorld(), m_Pos, Direction, LaserReach, m_pPlayer->GetCID(), WEAPON_LASER);
-		GameServer()->CreateSound(m_Pos, SOUND_LASER_FIRE, TeamMask());
+		GameServer()->CreateSound(m_Pos, SOUND_LASER_FIRE, TeamMask()); // NOLINT(clang-analyzer-unix.Malloc)
 	}
 	break;
 

src/game/server/gamecontroller.cpp

@@ -384,11 +384,11 @@ bool IGameController::OnEntity(int Index, int x, int y, int Layer, int Flags, bo
 		new CGun(&GameServer()->m_World, Pos, false, false, Layer, Number);
 	}
 
-	if(Type != -1)
+	if(Type != -1) // NOLINT(clang-analyzer-unix.Malloc)
 	{
 		CPickup *pPickup = new CPickup(&GameServer()->m_World, Type, SubType, Layer, Number);
 		pPickup->m_Pos = Pos;
-		return true;
+		return true; // NOLINT(clang-analyzer-unix.Malloc)
 	}
 
 	return false;

src/test/memory.cpp

@@ -0,0 +1,187 @@
+#include <gtest/gtest.h>
+
+#include <base/system.h>
+
+static bool mem_is_null(const void *block, size_t size)
+{
+	const unsigned char *bytes = (const unsigned char *)block;
+	size_t i;
+	for(i = 0; i < size; i++)
+	{
+		if(bytes[i] != 0)
+		{
+			return false;
+		}
+	}
+	return true;
+}
+
+TEST(Memory, BaseTypes)
+{
+	void *aVoid[123];
+	secure_random_fill(aVoid, sizeof(aVoid));
+	EXPECT_FALSE(mem_is_null(aVoid, sizeof(aVoid)));
+	mem_zero(&aVoid, sizeof(aVoid));
+	EXPECT_TRUE(mem_is_null(aVoid, sizeof(aVoid)));
+	secure_random_fill(aVoid, sizeof(aVoid));
+	EXPECT_FALSE(mem_is_null(aVoid, sizeof(aVoid)));
+	mem_zero(&aVoid[0], 123 * sizeof(void *));
+	EXPECT_TRUE(mem_is_null(aVoid, sizeof(aVoid)));
+
+	secure_random_fill(aVoid, sizeof(aVoid));
+	EXPECT_FALSE(mem_is_null(aVoid, sizeof(aVoid)));
+	mem_zero(aVoid, sizeof(aVoid));
+	EXPECT_TRUE(mem_is_null(aVoid, sizeof(aVoid)));
+
+	int aInt[512];
+	secure_random_fill(aInt, sizeof(aInt));
+	EXPECT_FALSE(mem_is_null(aInt, sizeof(aInt)));
+	mem_zero(&aInt, sizeof(aInt));
+	EXPECT_TRUE(mem_is_null(aInt, sizeof(aInt)));
+	secure_random_fill(aInt, sizeof(aInt));
+	EXPECT_FALSE(mem_is_null(aInt, sizeof(aInt)));
+	mem_zero(&aInt[0], sizeof(aInt));
+	EXPECT_TRUE(mem_is_null(aInt, sizeof(aInt)));
+
+	secure_random_fill(aInt, sizeof(aInt));
+	EXPECT_FALSE(mem_is_null(aInt, sizeof(aInt)));
+	mem_zero(aInt, sizeof(aInt));
+	EXPECT_TRUE(mem_is_null(aInt, sizeof(aInt)));
+
+	int *apInt[512];
+	secure_random_fill(apInt, sizeof(apInt));
+	EXPECT_FALSE(mem_is_null(apInt, sizeof(apInt)));
+	mem_zero(&apInt, sizeof(apInt));
+	EXPECT_TRUE(mem_is_null(apInt, sizeof(apInt)));
+
+	secure_random_fill(apInt, sizeof(apInt));
+	EXPECT_FALSE(mem_is_null(apInt, sizeof(apInt)));
+	mem_zero(apInt, sizeof(apInt));
+	EXPECT_TRUE(mem_is_null(apInt, sizeof(apInt)));
+
+	int aaInt[10][20];
+	secure_random_fill(aaInt, sizeof(aaInt));
+	EXPECT_FALSE(mem_is_null(aaInt, sizeof(aaInt)));
+	mem_zero(&aaInt, sizeof(aaInt));
+	EXPECT_TRUE(mem_is_null(aaInt, sizeof(aaInt)));
+	secure_random_fill(aaInt, sizeof(aaInt));
+	EXPECT_FALSE(mem_is_null(aaInt, sizeof(aaInt)));
+	mem_zero(&aaInt[0], sizeof(aaInt));
+	EXPECT_TRUE(mem_is_null(aaInt, sizeof(aaInt)));
+	secure_random_fill(aaInt, sizeof(aaInt));
+	EXPECT_FALSE(mem_is_null(aaInt, sizeof(aaInt)));
+	mem_zero(&aaInt[0][0], sizeof(aaInt));
+	EXPECT_TRUE(mem_is_null(aaInt, sizeof(aaInt)));
+
+	secure_random_fill(aaInt, sizeof(aaInt));
+	EXPECT_FALSE(mem_is_null(aaInt, sizeof(aaInt)));
+	mem_zero(aaInt, sizeof(aaInt));
+	EXPECT_TRUE(mem_is_null(aaInt, sizeof(aaInt)));
+	secure_random_fill(aaInt, sizeof(aaInt));
+	EXPECT_FALSE(mem_is_null(aaInt, sizeof(aaInt)));
+	mem_zero(aaInt[0], sizeof(aaInt));
+	EXPECT_TRUE(mem_is_null(aaInt, sizeof(aaInt)));
+
+	int *aapInt[10][20];
+	secure_random_fill(aapInt, sizeof(aapInt));
+	EXPECT_FALSE(mem_is_null(aapInt, sizeof(aapInt)));
+	mem_zero(&aapInt, sizeof(aapInt));
+	EXPECT_TRUE(mem_is_null(aapInt, sizeof(aapInt)));
+	secure_random_fill(aapInt, sizeof(aapInt));
+	EXPECT_FALSE(mem_is_null(aapInt, sizeof(aapInt)));
+	mem_zero(&aapInt[0], sizeof(aapInt));
+	EXPECT_TRUE(mem_is_null(aapInt, sizeof(aapInt)));
+
+	secure_random_fill(aapInt, sizeof(aapInt));
+	EXPECT_FALSE(mem_is_null(aapInt, sizeof(aapInt)));
+	mem_zero(aapInt, sizeof(aapInt));
+	EXPECT_TRUE(mem_is_null(aapInt, sizeof(aapInt)));
+	secure_random_fill(aapInt, sizeof(aapInt));
+	EXPECT_FALSE(mem_is_null(aapInt, sizeof(aapInt)));
+	mem_zero(aapInt[0], sizeof(aapInt));
+	EXPECT_TRUE(mem_is_null(aapInt, sizeof(aapInt)));
+}
+
+TEST(Memory, PodTypes)
+{
+	NETADDR aAddr[123];
+	secure_random_fill(aAddr, sizeof(aAddr));
+	EXPECT_FALSE(mem_is_null(aAddr, sizeof(aAddr)));
+	mem_zero(&aAddr, sizeof(aAddr));
+	EXPECT_TRUE(mem_is_null(aAddr, sizeof(aAddr)));
+	secure_random_fill(aAddr, sizeof(aAddr));
+	EXPECT_FALSE(mem_is_null(aAddr, sizeof(aAddr)));
+	mem_zero(&aAddr[0], sizeof(aAddr));
+	EXPECT_TRUE(mem_is_null(aAddr, sizeof(aAddr)));
+
+	secure_random_fill(aAddr, sizeof(aAddr));
+	EXPECT_FALSE(mem_is_null(aAddr, sizeof(aAddr)));
+	mem_zero(aAddr, sizeof(aAddr));
+	EXPECT_TRUE(mem_is_null(aAddr, sizeof(aAddr)));
+
+	NETADDR *apAddr[123];
+	secure_random_fill(apAddr, sizeof(apAddr));
+	EXPECT_FALSE(mem_is_null(apAddr, sizeof(apAddr)));
+	mem_zero((NETADDR **)apAddr, sizeof(apAddr));
+	EXPECT_TRUE(mem_is_null(apAddr, sizeof(apAddr)));
+	secure_random_fill(apAddr, sizeof(apAddr));
+	EXPECT_FALSE(mem_is_null(apAddr, sizeof(apAddr)));
+	mem_zero(&apAddr, sizeof(apAddr));
+	EXPECT_TRUE(mem_is_null(apAddr, sizeof(apAddr)));
+	secure_random_fill(apAddr, sizeof(apAddr));
+	EXPECT_FALSE(mem_is_null(apAddr, sizeof(apAddr)));
+	mem_zero(&apAddr[0], sizeof(apAddr));
+	EXPECT_TRUE(mem_is_null(apAddr, sizeof(apAddr)));
+	secure_random_fill(apAddr, sizeof(apAddr));
+	EXPECT_FALSE(mem_is_null(apAddr, sizeof(apAddr)));
+	mem_zero(apAddr, sizeof(apAddr));
+	EXPECT_TRUE(mem_is_null(apAddr, sizeof(apAddr)));
+
+	// 2D arrays
+	NETADDR aaAddr[10][20];
+	secure_random_fill(aaAddr, sizeof(aaAddr));
+	EXPECT_FALSE(mem_is_null((NETADDR *)aaAddr, sizeof(aaAddr)));
+	mem_zero(&aaAddr, sizeof(aaAddr));
+	EXPECT_TRUE(mem_is_null((NETADDR *)aaAddr, sizeof(aaAddr)));
+	secure_random_fill(aaAddr, sizeof(aaAddr));
+	EXPECT_FALSE(mem_is_null((NETADDR *)aaAddr, sizeof(aaAddr)));
+	mem_zero(&aaAddr[0], sizeof(aaAddr));
+	EXPECT_TRUE(mem_is_null((NETADDR *)aaAddr, sizeof(aaAddr)));
+	secure_random_fill(aaAddr, sizeof(aaAddr));
+	EXPECT_FALSE(mem_is_null((NETADDR *)aaAddr, sizeof(aaAddr)));
+	mem_zero(&aaAddr[0][0], sizeof(aaAddr));
+	EXPECT_TRUE(mem_is_null((NETADDR *)aaAddr, sizeof(aaAddr)));
+
+	secure_random_fill(aaAddr, sizeof(aaAddr));
+	EXPECT_FALSE(mem_is_null((NETADDR *)aaAddr, sizeof(aaAddr)));
+	mem_zero(aaAddr, sizeof(aaAddr));
+	EXPECT_TRUE(mem_is_null((NETADDR *)aaAddr, sizeof(aaAddr)));
+	secure_random_fill(aaAddr, sizeof(aaAddr));
+	EXPECT_FALSE(mem_is_null((NETADDR *)aaAddr, sizeof(aaAddr)));
+	mem_zero(aaAddr[0], sizeof(aaAddr));
+	EXPECT_TRUE(mem_is_null((NETADDR *)aaAddr, sizeof(aaAddr)));
+
+	// 2D pointer arrays
+	NETADDR *aapAddr[10][20];
+	secure_random_fill(aapAddr, sizeof(aapAddr));
+	EXPECT_FALSE(mem_is_null(aapAddr, sizeof(aapAddr)));
+	mem_zero(&aapAddr, sizeof(aapAddr));
+	EXPECT_TRUE(mem_is_null(aapAddr, sizeof(aapAddr)));
+	secure_random_fill(aapAddr, sizeof(aapAddr));
+	EXPECT_FALSE(mem_is_null(aapAddr, sizeof(aapAddr)));
+	mem_zero(&aapAddr[0], sizeof(aapAddr));
+	EXPECT_TRUE(mem_is_null(aapAddr, sizeof(aapAddr)));
+	secure_random_fill(aapAddr, sizeof(aapAddr));
+	EXPECT_FALSE(mem_is_null(aapAddr, sizeof(aapAddr)));
+	mem_zero(&aapAddr[0][0], sizeof(aapAddr));
+	EXPECT_TRUE(mem_is_null(aapAddr, sizeof(aapAddr)));
+
+	secure_random_fill(aapAddr, sizeof(aapAddr));
+	EXPECT_FALSE(mem_is_null(aapAddr, sizeof(aapAddr)));
+	mem_zero(aapAddr, sizeof(aapAddr));
+	EXPECT_TRUE(mem_is_null(aapAddr, sizeof(aapAddr)));
+	secure_random_fill(aapAddr, sizeof(aapAddr));
+	EXPECT_FALSE(mem_is_null(aapAddr, sizeof(aapAddr)));
+	mem_zero(aapAddr[0], sizeof(aapAddr));
+	EXPECT_TRUE(mem_is_null(aapAddr, sizeof(aapAddr)));
+}