diff --git a/CMakeLists.txt b/CMakeLists.txt index 2bfabfd29..a6906d957 100644 --- a/CMakeLists.txt +++ b/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 diff --git a/src/base/system.cpp b/src/base/system.cpp index 84e59fb6d..877daebc0 100644 --- a/src/base/system.cpp +++ b/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); diff --git a/src/base/system.h b/src/base/system.h index bb98d36e0..9fa444342 100644 --- a/src/base/system.h +++ b/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 +inline void mem_zero(T *block, size_t size) +{ + static_assert((std::is_trivially_constructible::value && std::is_trivially_destructible::value) || std::is_fundamental::value); + memset(block, 0, size); +} /** * Compares two blocks of memory @@ -2830,7 +2835,7 @@ bool shell_unregister_application(const char *executable, bool *updated); * Notifies the system that a protocol or file extension has been changed and the shell needs to be updated. * * @ingroup Shell - * + * * @remark This is a potentially expensive operation, so it should only be called when necessary. */ void shell_update(); diff --git a/src/base/vmath.h b/src/base/vmath.h index 6797e5db1..b83a1ae02 100644 --- a/src/base/vmath.h +++ b/src/base/vmath.h @@ -23,11 +23,7 @@ public: T y, v; }; - constexpr vector2_base() : - x(T()), y(T()) - { - } - + constexpr vector2_base() = default; constexpr vector2_base(T nx, T ny) : x(nx), y(ny) { @@ -198,11 +194,7 @@ public: T z, b, l, w; }; - constexpr vector3_base() : - x(T()), y(T()), z(T()) - { - } - + constexpr vector3_base() = default; 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() : - x(T()), y(T()), z(T()), w(T()) - { - } - + constexpr vector4_base() = default; constexpr vector4_base(T nx, T ny, T nz, T nw) : x(nx), y(ny), z(nz), w(nw) { diff --git a/src/engine/client/client.cpp b/src/engine/client/client.cpp index 708b39e3d..2e313fabc 100644 --- a/src/engine/client/client.cpp +++ b/src/engine/client/client.cpp @@ -4195,9 +4195,7 @@ void CClient::RegisterCommands() static CClient *CreateClient() { - CClient *pClient = static_cast(malloc(sizeof(*pClient))); - mem_zero(pClient, sizeof(CClient)); - return new(pClient) CClient; + return new 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(); - free(pClient); + delete pClient; }); const std::thread::id MainThreadId = std::this_thread::get_id(); diff --git a/src/engine/client/graphics_threaded.cpp b/src/engine/client/graphics_threaded.cpp index f3576253f..a43038c4c 100644 --- a/src/engine/client/graphics_threaded.cpp +++ b/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; diff --git a/src/engine/shared/netban.h b/src/engine/shared/netban.h index f21060f6f..fd7dbb8a8 100644 --- a/src/engine/shared/netban.h +++ b/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); diff --git a/src/engine/shared/network_client.cpp b/src/engine/shared/network_client.cpp index 7b52665cb..a97815475 100644 --- a/src/engine/shared/network_client.cpp +++ b/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; diff --git a/src/engine/shared/network_console.cpp b/src/engine/shared/network_console.cpp index 38874bc15..27c2fd2d1 100644 --- a/src/engine/shared/network_console.cpp +++ b/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 diff --git a/src/engine/shared/network_server.cpp b/src/engine/shared/network_server.cpp index 14c402984..0dd81393c 100644 --- a/src/engine/shared/network_server.cpp +++ b/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); diff --git a/src/game/client/prediction/entities/character.cpp b/src/game/client/prediction/entities/character.cpp index 701e5b387..d73e99403 100644 --- a/src/game/client/prediction/entities/character.cpp +++ b/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) { diff --git a/src/game/server/entities/character.cpp b/src/game/server/entities/character.cpp index fe25504d9..6506570fe 100644 --- a/src/game/server/entities/character.cpp +++ b/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; diff --git a/src/game/server/gamecontroller.cpp b/src/game/server/gamecontroller.cpp index 6b0a3de5d..c6aa0ce79 100644 --- a/src/game/server/gamecontroller.cpp +++ b/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; diff --git a/src/test/memory.cpp b/src/test/memory.cpp new file mode 100644 index 000000000..58e661c77 --- /dev/null +++ b/src/test/memory.cpp @@ -0,0 +1,187 @@ +#include + +#include + +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))); +}