warzone2100-0001: PROJECTILE::psDamaged std::find O(G*D) per tick, 9.5x
Defect: projectile.cpp line 872, std::find on std::vector<BASE_OBJECT*> psDamaged inside grid neighbor iteration loop. Every projectile tick, for each nearby object, does O(D) linear scan to check if already damaged. Penetrating weapons inherit and grow psDamaged across hits. Fix: replace std::vector with std::unordered_set for O(1) lookup. push_back becomes insert, std::find becomes count, remove_if becomes iterator-based erase loop. Severity: MEDIUM. Hot path (per projectile per tick), scales with battle density. D=200 damaged, G=100 grid neighbors: 9.5x speedup. MOAD 0002-0005 CLEAN: - 0002: global state is architectural (Eidos-era C game), not coupling defect - 0003: no thread_local usage found - 0004: no secrets logged (public keys and IPs only, standard for server logs) - 0005: no unsynchronized cache patterns (game logic is single-threaded)
This commit is contained in:
parent
1326aeefec
commit
1a7022ea83
3 changed files with 204 additions and 0 deletions
85
defects/warzone2100-0001/patch/warzone2100-0001.patch
Normal file
85
defects/warzone2100-0001/patch/warzone2100-0001.patch
Normal file
|
|
@ -0,0 +1,85 @@
|
|||
--- a/src/projectiledef.h
|
||||
+++ b/src/projectiledef.h
|
||||
@@ -22,7 +22,7 @@
|
||||
#ifndef __INCLUDED_PROJECTILEDEF_H__
|
||||
#define __INCLUDED_PROJECTILEDEF_H__
|
||||
|
||||
-#include "basedef.h"
|
||||
+#include "basedef.h" // SIMPLE_OBJECT, BASE_OBJECT, OBJ_PROJECTILE
|
||||
#include "lib/gamelib/gtime.h"
|
||||
|
||||
-#include <vector>
|
||||
+#include <unordered_set>
|
||||
|
||||
|
||||
enum PROJ_STATE
|
||||
@@ -49,7 +49,7 @@
|
||||
WEAPON_STATS *psWStats; ///< firing weapon stats
|
||||
BASE_OBJECT *psSource; ///< what fired the projectile
|
||||
BASE_OBJECT *psDest; ///< target of this projectile
|
||||
- std::vector<BASE_OBJECT *> psDamaged; ///< the targets that have already been dealt damage to (don't damage the same target twice)
|
||||
+ std::unordered_set<BASE_OBJECT *> psDamaged; ///< the targets that have already been dealt damage to (O(1) lookup instead of O(N) linear scan)
|
||||
|
||||
Vector3i src = Vector3i(0, 0, 0); ///< Where projectile started
|
||||
Vector3i dst = Vector3i(0, 0, 0); ///< The target coordinates
|
||||
--- a/src/projectile.cpp
|
||||
+++ b/src/projectile.cpp
|
||||
@@ -376,7 +376,7 @@
|
||||
psProj->rot.direction, psProj->rot.pitch, psProj->rot.roll,
|
||||
psProj->state,
|
||||
(int)psProj->expectedDamageCaused,
|
||||
- (int)psProj->psDamaged.size(),
|
||||
+ (int)psProj->psDamaged.size(), // unchanged: .size() works on unordered_set
|
||||
};
|
||||
_syncDebugIntList(function, "%c projectile = p%d;pos(%d,%d,%d),rot(%d,%d,%d),state%d,expectedDamageCaused%d,numberDamaged%u", list, ARRAY_SIZE(list));
|
||||
}
|
||||
@@ -869,7 +869,7 @@
|
||||
BASE_OBJECT *psTempObj = *gi;
|
||||
CHECK_OBJECT(psTempObj);
|
||||
|
||||
- if (std::find(psProj->psDamaged.begin(), psProj->psDamaged.end(), psTempObj) != psProj->psDamaged.end())
|
||||
+ if (psProj->psDamaged.count(psTempObj) != 0)
|
||||
{
|
||||
// Dont damage one target twice
|
||||
continue;
|
||||
@@ -950,7 +950,7 @@
|
||||
asWeap.nStat = psStats - asWeaponStats.data();
|
||||
|
||||
// Assume we damaged the chosen target
|
||||
- psProj->psDamaged.push_back(closestCollisionObject);
|
||||
+ psProj->psDamaged.insert(closestCollisionObject);
|
||||
|
||||
spawnedProjectile = proj_SendProjectileInternal(&asWeap, psProj, psProj->player, psProj->dst, nullptr, true, -1);
|
||||
}
|
||||
@@ -1290,7 +1290,7 @@
|
||||
|
||||
if (relativeDamage >= 0) // So long as the target wasn't killed
|
||||
{
|
||||
- psObj->psDamaged.push_back(psObj->psDest);
|
||||
+ psObj->psDamaged.insert(psObj->psDest);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1396,7 +1396,10 @@
|
||||
setProjectileDestination(psObj, nullptr);
|
||||
}
|
||||
// Remove dead objects from psDamaged.
|
||||
- psDamaged.erase(std::remove_if(psDamaged.begin(), psDamaged.end(), [](const BASE_OBJECT *psObj) { return ::isDead(psObj); }), psDamaged.end());
|
||||
+ for (auto it = psDamaged.begin(); it != psDamaged.end(); )
|
||||
+ {
|
||||
+ it = ::isDead(*it) ? psDamaged.erase(it) : std::next(it);
|
||||
+ }
|
||||
|
||||
// This extra check fixes a crash in cam2, mission1
|
||||
if (worldOnMap(psObj->pos.x, psObj->pos.y) == false)
|
||||
@@ -1954,9 +1957,9 @@
|
||||
checkObject(psProjectile->psSource, location_description, function, recurse - 1);
|
||||
}
|
||||
|
||||
- for (unsigned n = 0; n != psProjectile->psDamaged.size(); ++n)
|
||||
+ for (const auto *psObj : psProjectile->psDamaged)
|
||||
{
|
||||
- checkObject(psProjectile->psDamaged[n], location_description, function, recurse - 1);
|
||||
+ checkObject(psObj, location_description, function, recurse - 1);
|
||||
}
|
||||
}
|
||||
BIN
defects/warzone2100-0001/test/test_psDamaged_lookup
Executable file
BIN
defects/warzone2100-0001/test/test_psDamaged_lookup
Executable file
Binary file not shown.
119
defects/warzone2100-0001/test/test_psDamaged_lookup.cpp
Normal file
119
defects/warzone2100-0001/test/test_psDamaged_lookup.cpp
Normal file
|
|
@ -0,0 +1,119 @@
|
|||
// Unit test for warzone2100-0001: PROJECTILE::psDamaged linear scan O(G*D)
|
||||
// Defect: std::find on std::vector<BASE_OBJECT*> inside grid iteration loop
|
||||
// Fix: std::unordered_set<BASE_OBJECT*> gives O(1) lookup instead of O(N)
|
||||
//
|
||||
// CWE-407: Algorithmic Complexity — list membership check in hot loop
|
||||
|
||||
#include <vector>
|
||||
#include <unordered_set>
|
||||
#include <algorithm>
|
||||
#include <chrono>
|
||||
#include <cstdio>
|
||||
#include <cassert>
|
||||
#include <cstdint>
|
||||
|
||||
// Simulate BASE_OBJECT as an opaque pointer (just need distinct addresses)
|
||||
struct FakeObject {
|
||||
uint32_t id;
|
||||
};
|
||||
|
||||
// ---- BEFORE (vector + std::find) ----
|
||||
static int projectile_collision_check_before(
|
||||
const std::vector<FakeObject*>& psDamaged,
|
||||
const std::vector<FakeObject*>& gridNeighbors)
|
||||
{
|
||||
int skipped = 0;
|
||||
for (FakeObject* psTempObj : gridNeighbors)
|
||||
{
|
||||
if (std::find(psDamaged.begin(), psDamaged.end(), psTempObj) != psDamaged.end())
|
||||
{
|
||||
skipped++;
|
||||
continue;
|
||||
}
|
||||
// ... collision detection would happen here
|
||||
}
|
||||
return skipped;
|
||||
}
|
||||
|
||||
// ---- AFTER (unordered_set + count) ----
|
||||
static int projectile_collision_check_after(
|
||||
const std::unordered_set<FakeObject*>& psDamaged,
|
||||
const std::vector<FakeObject*>& gridNeighbors)
|
||||
{
|
||||
int skipped = 0;
|
||||
for (FakeObject* psTempObj : gridNeighbors)
|
||||
{
|
||||
if (psDamaged.count(psTempObj) != 0)
|
||||
{
|
||||
skipped++;
|
||||
continue;
|
||||
}
|
||||
// ... collision detection would happen here
|
||||
}
|
||||
return skipped;
|
||||
}
|
||||
|
||||
int main()
|
||||
{
|
||||
// Simulate a penetrating projectile that has passed through many objects.
|
||||
// D = damaged count (objects already hit by this projectile)
|
||||
// G = grid neighbor count (objects near projectile to check each tick)
|
||||
const int D = 200; // penetrating area-effect weapon in a dense battle
|
||||
const int G = 100; // grid neighbors in PROJ_NEIGHBOUR_RANGE
|
||||
|
||||
// Create fake objects
|
||||
std::vector<FakeObject> allObjects(D + G);
|
||||
for (int i = 0; i < D + G; i++) {
|
||||
allObjects[i].id = i;
|
||||
}
|
||||
|
||||
// Build psDamaged list (first D objects already damaged)
|
||||
std::vector<FakeObject*> psDamagedVec;
|
||||
std::unordered_set<FakeObject*> psDamagedSet;
|
||||
for (int i = 0; i < D; i++) {
|
||||
psDamagedVec.push_back(&allObjects[i]);
|
||||
psDamagedSet.insert(&allObjects[i]);
|
||||
}
|
||||
|
||||
// Build grid neighbors: half already damaged, half new
|
||||
std::vector<FakeObject*> gridNeighbors;
|
||||
for (int i = D/2; i < D/2 + G; i++) {
|
||||
gridNeighbors.push_back(&allObjects[i]);
|
||||
}
|
||||
|
||||
// Correctness check
|
||||
int skipBefore = projectile_collision_check_before(psDamagedVec, gridNeighbors);
|
||||
int skipAfter = projectile_collision_check_after(psDamagedSet, gridNeighbors);
|
||||
assert(skipBefore == skipAfter);
|
||||
printf("PASS correctness: both skip %d objects\n", skipBefore);
|
||||
|
||||
// Benchmark: simulate many projectile ticks
|
||||
const int TICKS = 5000;
|
||||
|
||||
auto t0 = std::chrono::high_resolution_clock::now();
|
||||
volatile int sinkBefore = 0;
|
||||
for (int t = 0; t < TICKS; t++) {
|
||||
sinkBefore += projectile_collision_check_before(psDamagedVec, gridNeighbors);
|
||||
}
|
||||
auto t1 = std::chrono::high_resolution_clock::now();
|
||||
volatile int sinkAfter = 0;
|
||||
for (int t = 0; t < TICKS; t++) {
|
||||
sinkAfter += projectile_collision_check_after(psDamagedSet, gridNeighbors);
|
||||
}
|
||||
auto t2 = std::chrono::high_resolution_clock::now();
|
||||
|
||||
double msBefore = std::chrono::duration<double, std::milli>(t1 - t0).count();
|
||||
double msAfter = std::chrono::duration<double, std::milli>(t2 - t1).count();
|
||||
double ratio = msBefore / msAfter;
|
||||
|
||||
printf("BEFORE (vector std::find): %.2f ms (%d ticks)\n", msBefore, TICKS);
|
||||
printf("AFTER (unordered_set): %.2f ms (%d ticks)\n", msAfter, TICKS);
|
||||
printf("Speedup ratio: %.1fx\n", ratio);
|
||||
|
||||
// Patched version must be faster
|
||||
assert(ratio > 2.0 && "Expected at least 2x speedup from set lookup");
|
||||
printf("PASS performance: %.1fx speedup (D=%d, G=%d)\n", ratio, D, G);
|
||||
|
||||
printf("\nAll tests PASS\n");
|
||||
return 0;
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue