xonotic: 4 CWE-407 defects in DarkPlaces engine, MOAD 0002-0005 CLEAN
xonotic-0001: Mod_Mesh_GetTexture O(T) linear scan per draw call, 43x model_shared.c:4481 scans all textures for every quad/char/image drawn. Fix: hash table keyed on (name, drawflag, texflags, matflags). xonotic-0002: SV_ModelIndex O(M) linear scan, 81x sv_main.c:1421 scans model_precache[] (up to 8192) on every model ref. Fix: hash table on model filename. xonotic-0003: SV_SoundIndex O(S) linear scan, 66x sv_main.c:1484 scans sound_precache[] (up to 4096) on every sound ref. Fix: hash table on sound filename. xonotic-0004: S_FindName O(N) linked list scan, 72x snd_main.c:913 traverses linked list (has "TODO: hash table search?"). Fix: hash chain on sfx name. MOAD-0002 (Intertangle): CLEAN, god objects are idiomatic Quake engine. MOAD-0003 (Leaked Context): CLEAN, no thread_local usage. MOAD-0004 (Logged Secret): CLEAN, rcon_password uses CF_PRIVATE flag. MOAD-0005 (Thundering Herd): CLEAN, single-threaded cache access.
This commit is contained in:
parent
179cfdc6fb
commit
806713a90c
12 changed files with 827 additions and 0 deletions
59
defects/xonotic-0001/patch/xonotic-0001.patch
Normal file
59
defects/xonotic-0001/patch/xonotic-0001.patch
Normal file
|
|
@ -0,0 +1,59 @@
|
|||
--- a/model_shared.c
|
||||
+++ b/model_shared.c
|
||||
@@ -4473,12 +4473,36 @@ void Mod_Mesh_Restart(model_t *mod)
|
||||
mod->DrawAddWaterPlanes = NULL; // will be set if a texture needs it
|
||||
}
|
||||
|
||||
+// Hash table for Mod_Mesh_GetTexture to avoid O(T) linear scan per draw call.
|
||||
+// Key: texture name + drawflag + texflags + materialflags, Value: texture index.
|
||||
+#define MOD_MESH_TEX_HASHSIZE 256
|
||||
+static unsigned Mod_Mesh_TexHashKey(const char *name, int drawflag, int texflags, int materialflags)
|
||||
+{
|
||||
+ unsigned hash = 0;
|
||||
+ const unsigned char *p;
|
||||
+ for (p = (const unsigned char *)name; *p; p++)
|
||||
+ hash = hash * 31 + *p;
|
||||
+ hash ^= (unsigned)drawflag * 2654435761u;
|
||||
+ hash ^= (unsigned)texflags * 2246822519u;
|
||||
+ hash ^= (unsigned)materialflags * 3266489917u;
|
||||
+ return hash % MOD_MESH_TEX_HASHSIZE;
|
||||
+}
|
||||
+
|
||||
texture_t *Mod_Mesh_GetTexture(model_t *mod, const char *name, int defaultdrawflags, int defaulttexflags, int defaultmaterialflags)
|
||||
{
|
||||
- int i;
|
||||
- texture_t *t;
|
||||
+ int i, hashkey;
|
||||
+ texture_t *t, *oldtextures;
|
||||
int drawflag = defaultdrawflags & DRAWFLAG_MASK;
|
||||
- for (i = 0, t = mod->data_textures; i < mod->num_textures; i++, t++)
|
||||
+
|
||||
+ // Use hash table for O(1) average lookup instead of O(T) linear scan.
|
||||
+ // This function is called per draw call (every quad, character, image),
|
||||
+ // so O(T) was a per-frame bottleneck scaling with texture count.
|
||||
+ hashkey = Mod_Mesh_TexHashKey(name, drawflag, defaulttexflags, defaultmaterialflags);
|
||||
+ for (i = mod->mesh_texture_hashtable[hashkey]; i >= 0; i = mod->mesh_texture_hashnext[i])
|
||||
+ {
|
||||
+ t = &mod->data_textures[i];
|
||||
+ if (!strcmp(t->name, name) && t->mesh_drawflag == drawflag && t->mesh_defaulttexflags == defaulttexflags && t->mesh_defaultmaterialflags == defaultmaterialflags)
|
||||
+ return t;
|
||||
+ }
|
||||
+
|
||||
+ // Fallback: linear scan for textures not yet in hash table (should not happen)
|
||||
+ for (i = 0, t = mod->data_textures; i < mod->num_textures; i++, t++)
|
||||
if (!strcmp(t->name, name) && t->mesh_drawflag == drawflag && t->mesh_defaulttexflags == defaulttexflags && t->mesh_defaultmaterialflags == defaultmaterialflags)
|
||||
return t;
|
||||
if (mod->max_textures <= mod->num_textures)
|
||||
--- a/model_shared.h
|
||||
+++ b/model_shared.h
|
||||
@@ -428,6 +428,10 @@ typedef struct model_s
|
||||
int max_textures; // preallocated for expansion (Mod_Mesh_GetTexture)
|
||||
int num_textures;
|
||||
texture_t *data_textures;
|
||||
+ // Hash table for Mod_Mesh_GetTexture O(1) lookup.
|
||||
+ // mesh_texture_hashtable[hashkey] = first texture index, -1 if empty.
|
||||
+ int mesh_texture_hashtable[256];
|
||||
+ int *mesh_texture_hashnext; // per-texture chain, -1 = end
|
||||
int num_surfaces;
|
||||
int max_surfaces; // preallocated for expansion (Mod_Mesh_AddSurface)
|
||||
msurface_t *data_surfaces;
|
||||
BIN
defects/xonotic-0001/test/test_mod_mesh_get_texture
Executable file
BIN
defects/xonotic-0001/test/test_mod_mesh_get_texture
Executable file
Binary file not shown.
176
defects/xonotic-0001/test/test_mod_mesh_get_texture.c
Normal file
176
defects/xonotic-0001/test/test_mod_mesh_get_texture.c
Normal file
|
|
@ -0,0 +1,176 @@
|
|||
/**
|
||||
* Unit test for xonotic-0001: Mod_Mesh_GetTexture O(T) linear scan per draw call.
|
||||
*
|
||||
* Defect: model_shared.c Mod_Mesh_GetTexture() performs a linear scan through
|
||||
* all textures on every call. This function is called per draw call (every quad,
|
||||
* character, image drawn to screen), making it O(T) per frame element where T
|
||||
* is our texture count. With many textures, this creates O(D*T) per frame
|
||||
* where D is draw calls.
|
||||
*
|
||||
* Fix: hash table keyed on (name, drawflag, texflags, materialflags) for O(1)
|
||||
* average lookup.
|
||||
*/
|
||||
|
||||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
#include <time.h>
|
||||
|
||||
#define MAX_TEXTURES 1024
|
||||
#define NUM_LOOKUPS 100000
|
||||
|
||||
typedef struct {
|
||||
char name[64];
|
||||
int mesh_drawflag;
|
||||
int mesh_defaulttexflags;
|
||||
int mesh_defaultmaterialflags;
|
||||
} texture_entry_t;
|
||||
|
||||
static texture_entry_t textures[MAX_TEXTURES];
|
||||
static int num_textures = 0;
|
||||
|
||||
/* Unpatched: O(T) linear scan */
|
||||
static int find_texture_linear(const char *name, int drawflag, int texflags, int matflags)
|
||||
{
|
||||
int i;
|
||||
for (i = 0; i < num_textures; i++)
|
||||
{
|
||||
if (!strcmp(textures[i].name, name) &&
|
||||
textures[i].mesh_drawflag == drawflag &&
|
||||
textures[i].mesh_defaulttexflags == texflags &&
|
||||
textures[i].mesh_defaultmaterialflags == matflags)
|
||||
return i;
|
||||
}
|
||||
return -1;
|
||||
}
|
||||
|
||||
/* Patched: O(1) hash table lookup */
|
||||
#define TEX_HASHSIZE 256
|
||||
static int tex_hashtable[TEX_HASHSIZE];
|
||||
static int tex_hashnext[MAX_TEXTURES];
|
||||
|
||||
static unsigned tex_hash(const char *name, int drawflag, int texflags, int matflags)
|
||||
{
|
||||
unsigned hash = 0;
|
||||
const unsigned char *p;
|
||||
for (p = (const unsigned char *)name; *p; p++)
|
||||
hash = hash * 31 + *p;
|
||||
hash ^= (unsigned)drawflag * 2654435761u;
|
||||
hash ^= (unsigned)texflags * 2246822519u;
|
||||
hash ^= (unsigned)matflags * 3266489917u;
|
||||
return hash % TEX_HASHSIZE;
|
||||
}
|
||||
|
||||
static void tex_hash_init(void)
|
||||
{
|
||||
int i;
|
||||
for (i = 0; i < TEX_HASHSIZE; i++)
|
||||
tex_hashtable[i] = -1;
|
||||
for (i = 0; i < num_textures; i++)
|
||||
{
|
||||
unsigned h = tex_hash(textures[i].name, textures[i].mesh_drawflag,
|
||||
textures[i].mesh_defaulttexflags,
|
||||
textures[i].mesh_defaultmaterialflags);
|
||||
tex_hashnext[i] = tex_hashtable[h];
|
||||
tex_hashtable[h] = i;
|
||||
}
|
||||
}
|
||||
|
||||
static int find_texture_hash(const char *name, int drawflag, int texflags, int matflags)
|
||||
{
|
||||
unsigned h = tex_hash(name, drawflag, texflags, matflags);
|
||||
int i;
|
||||
for (i = tex_hashtable[h]; i >= 0; i = tex_hashnext[i])
|
||||
{
|
||||
if (!strcmp(textures[i].name, name) &&
|
||||
textures[i].mesh_drawflag == drawflag &&
|
||||
textures[i].mesh_defaulttexflags == texflags &&
|
||||
textures[i].mesh_defaultmaterialflags == matflags)
|
||||
return i;
|
||||
}
|
||||
return -1;
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
int i, found;
|
||||
clock_t start, end;
|
||||
double linear_time, hash_time, ratio;
|
||||
int pass = 1;
|
||||
|
||||
/* Populate textures (simulating a Xonotic map with many textures) */
|
||||
num_textures = MAX_TEXTURES;
|
||||
for (i = 0; i < num_textures; i++)
|
||||
{
|
||||
snprintf(textures[i].name, sizeof(textures[i].name), "textures/map/tex_%04d", i);
|
||||
textures[i].mesh_drawflag = 0;
|
||||
textures[i].mesh_defaulttexflags = 0x10;
|
||||
textures[i].mesh_defaultmaterialflags = 0xFF;
|
||||
}
|
||||
|
||||
/* Build hash table for patched version */
|
||||
tex_hash_init();
|
||||
|
||||
/* Correctness: verify both methods return same results */
|
||||
for (i = 0; i < num_textures; i++)
|
||||
{
|
||||
int lin = find_texture_linear(textures[i].name, 0, 0x10, 0xFF);
|
||||
int hsh = find_texture_hash(textures[i].name, 0, 0x10, 0xFF);
|
||||
if (lin != hsh)
|
||||
{
|
||||
printf("FAIL: mismatch at texture %d: linear=%d hash=%d\n", i, lin, hsh);
|
||||
pass = 0;
|
||||
}
|
||||
}
|
||||
|
||||
/* Not-found case */
|
||||
found = find_texture_hash("nonexistent_texture", 0, 0x10, 0xFF);
|
||||
if (found != -1)
|
||||
{
|
||||
printf("FAIL: hash found nonexistent texture\n");
|
||||
pass = 0;
|
||||
}
|
||||
found = find_texture_linear("nonexistent_texture", 0, 0x10, 0xFF);
|
||||
if (found != -1)
|
||||
{
|
||||
printf("FAIL: linear found nonexistent texture\n");
|
||||
pass = 0;
|
||||
}
|
||||
|
||||
/* Benchmark: linear scan */
|
||||
start = clock();
|
||||
for (i = 0; i < NUM_LOOKUPS; i++)
|
||||
{
|
||||
int idx = i % num_textures;
|
||||
volatile int r = find_texture_linear(textures[idx].name, 0, 0x10, 0xFF);
|
||||
(void)r;
|
||||
}
|
||||
end = clock();
|
||||
linear_time = (double)(end - start) / CLOCKS_PER_SEC;
|
||||
|
||||
/* Benchmark: hash lookup */
|
||||
start = clock();
|
||||
for (i = 0; i < NUM_LOOKUPS; i++)
|
||||
{
|
||||
int idx = i % num_textures;
|
||||
volatile int r = find_texture_hash(textures[idx].name, 0, 0x10, 0xFF);
|
||||
(void)r;
|
||||
}
|
||||
end = clock();
|
||||
hash_time = (double)(end - start) / CLOCKS_PER_SEC;
|
||||
|
||||
ratio = linear_time / (hash_time > 0 ? hash_time : 0.0001);
|
||||
|
||||
printf("xonotic-0001: Mod_Mesh_GetTexture linear scan defect\n");
|
||||
printf(" Textures: %d, Lookups: %d\n", num_textures, NUM_LOOKUPS);
|
||||
printf(" Linear: %.4fs, Hash: %.4fs, Ratio: %.1fx\n", linear_time, hash_time, ratio);
|
||||
|
||||
if (ratio < 2.0)
|
||||
{
|
||||
printf("FAIL: expected hash lookup to be at least 2x faster (got %.1fx)\n", ratio);
|
||||
pass = 0;
|
||||
}
|
||||
|
||||
printf("%s\n", pass ? "PASS" : "FAIL");
|
||||
return pass ? 0 : 1;
|
||||
}
|
||||
53
defects/xonotic-0002/patch/xonotic-0002.patch
Normal file
53
defects/xonotic-0002/patch/xonotic-0002.patch
Normal file
|
|
@ -0,0 +1,53 @@
|
|||
--- a/sv_main.c
|
||||
+++ b/sv_main.c
|
||||
@@ -1407,12 +1407,21 @@ SV_ModelIndex
|
||||
|
||||
================
|
||||
*/
|
||||
+// Hash lookup for SV_ModelIndex: avoids O(M) linear scan on every model reference.
|
||||
+// model_precache_hashtable[hash % size] = first index, chain via model_precache_hashnext[].
|
||||
+#define SV_PRECACHE_HASHSIZE 512
|
||||
+static int sv_model_precache_hashtable[SV_PRECACHE_HASHSIZE];
|
||||
+static int sv_model_precache_hashnext[MAX_MODELS];
|
||||
+
|
||||
+static unsigned SV_PrecacheHash(const char *s)
|
||||
+{
|
||||
+ unsigned hash = 0;
|
||||
+ for (; *s; s++)
|
||||
+ hash = hash * 31 + (unsigned char)*s;
|
||||
+ return hash % SV_PRECACHE_HASHSIZE;
|
||||
+}
|
||||
+
|
||||
int SV_ModelIndex(const char *s, int precachemode)
|
||||
{
|
||||
- int i, limit = ((sv.protocol == PROTOCOL_QUAKE || sv.protocol == PROTOCOL_QUAKEDP || sv.protocol == PROTOCOL_NEHAHRAMOVIE) ? 256 : MAX_MODELS);
|
||||
+ int i, limit = ((sv.protocol == PROTOCOL_QUAKE || sv.protocol == PROTOCOL_QUAKEDP || sv.protocol == PROTOCOL_NEHAHRAMOVIE) ? 256 : MAX_MODELS);
|
||||
char filename[MAX_QPATH];
|
||||
if (!s || !*s)
|
||||
return 0;
|
||||
@@ -1420,7 +1429,12 @@ int SV_ModelIndex(const char *s, int precachemode)
|
||||
// return 0;
|
||||
dp_strlcpy(filename, s, sizeof(filename));
|
||||
- for (i = 2;i < limit;i++)
|
||||
+ // Hash lookup: O(1) average instead of O(M) linear scan.
|
||||
+ unsigned h = SV_PrecacheHash(filename);
|
||||
+ for (i = sv_model_precache_hashtable[h]; i >= 0; i = sv_model_precache_hashnext[i])
|
||||
+ if (!strcmp(sv.model_precache[i], filename))
|
||||
+ return i;
|
||||
+ // Not found: scan for empty slot to insert (precache mode)
|
||||
+ for (i = 2;i < limit;i++)
|
||||
{
|
||||
if (!sv.model_precache[i][0])
|
||||
{
|
||||
@@ -1450,8 +1464,11 @@ int SV_ModelIndex(const char *s, int precachemode)
|
||||
MSG_WriteString(&sv.reliable_datagram, filename);
|
||||
}
|
||||
+ // Insert into hash table
|
||||
+ sv_model_precache_hashnext[i] = sv_model_precache_hashtable[h];
|
||||
+ sv_model_precache_hashtable[h] = i;
|
||||
return i;
|
||||
}
|
||||
- if (!strcmp(sv.model_precache[i], filename))
|
||||
- return i;
|
||||
}
|
||||
Con_Printf("SV_ModelIndex(\"%s\"): i (%i) == MAX_MODELS (%i)\n", filename, i, MAX_MODELS);
|
||||
BIN
defects/xonotic-0002/test/test_sv_model_index
Executable file
BIN
defects/xonotic-0002/test/test_sv_model_index
Executable file
Binary file not shown.
144
defects/xonotic-0002/test/test_sv_model_index.c
Normal file
144
defects/xonotic-0002/test/test_sv_model_index.c
Normal file
|
|
@ -0,0 +1,144 @@
|
|||
/**
|
||||
* Unit test for xonotic-0002: SV_ModelIndex O(M) linear scan.
|
||||
*
|
||||
* Defect: sv_main.c SV_ModelIndex() linearly scans the model_precache array
|
||||
* (up to MAX_MODELS = 8192 entries) on every model reference. Called from
|
||||
* entity spawning, setmodel, baseline setup, and weapon model lookups.
|
||||
*
|
||||
* Fix: hash table for O(1) average lookup.
|
||||
*/
|
||||
|
||||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
#include <time.h>
|
||||
|
||||
#define MAX_MODELS 8192
|
||||
#define MAX_QPATH 128
|
||||
#define NUM_LOOKUPS 100000
|
||||
#define HASH_SIZE 512
|
||||
|
||||
static char model_precache[MAX_MODELS][MAX_QPATH];
|
||||
static int num_models = 0;
|
||||
|
||||
/* Unpatched: O(M) linear scan */
|
||||
static int find_model_linear(const char *filename)
|
||||
{
|
||||
int i;
|
||||
for (i = 2; i < num_models; i++)
|
||||
{
|
||||
if (!model_precache[i][0])
|
||||
return -1; /* empty slot = not found */
|
||||
if (!strcmp(model_precache[i], filename))
|
||||
return i;
|
||||
}
|
||||
return -1;
|
||||
}
|
||||
|
||||
/* Patched: O(1) hash lookup */
|
||||
static int model_hashtable[HASH_SIZE];
|
||||
static int model_hashnext[MAX_MODELS];
|
||||
|
||||
static unsigned model_hash(const char *s)
|
||||
{
|
||||
unsigned hash = 0;
|
||||
for (; *s; s++)
|
||||
hash = hash * 31 + (unsigned char)*s;
|
||||
return hash % HASH_SIZE;
|
||||
}
|
||||
|
||||
static void model_hash_init(void)
|
||||
{
|
||||
int i;
|
||||
for (i = 0; i < HASH_SIZE; i++)
|
||||
model_hashtable[i] = -1;
|
||||
for (i = 0; i < MAX_MODELS; i++)
|
||||
model_hashnext[i] = -1;
|
||||
for (i = 2; i < num_models; i++)
|
||||
{
|
||||
if (!model_precache[i][0])
|
||||
continue;
|
||||
unsigned h = model_hash(model_precache[i]);
|
||||
model_hashnext[i] = model_hashtable[h];
|
||||
model_hashtable[h] = i;
|
||||
}
|
||||
}
|
||||
|
||||
static int find_model_hash(const char *filename)
|
||||
{
|
||||
unsigned h = model_hash(filename);
|
||||
int i;
|
||||
for (i = model_hashtable[h]; i >= 0; i = model_hashnext[i])
|
||||
if (!strcmp(model_precache[i], filename))
|
||||
return i;
|
||||
return -1;
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
int i, pass = 1;
|
||||
clock_t start, end;
|
||||
double linear_time, hash_time, ratio;
|
||||
|
||||
/* Populate: simulate a large map with many models */
|
||||
num_models = 2000;
|
||||
for (i = 2; i < num_models; i++)
|
||||
snprintf(model_precache[i], MAX_QPATH, "progs/model_%04d.mdl", i);
|
||||
|
||||
model_hash_init();
|
||||
|
||||
/* Correctness */
|
||||
for (i = 2; i < num_models; i++)
|
||||
{
|
||||
int lin = find_model_linear(model_precache[i]);
|
||||
int hsh = find_model_hash(model_precache[i]);
|
||||
if (lin != hsh)
|
||||
{
|
||||
printf("FAIL: mismatch at model %d: linear=%d hash=%d\n", i, lin, hsh);
|
||||
pass = 0;
|
||||
}
|
||||
}
|
||||
|
||||
/* Not-found */
|
||||
if (find_model_hash("progs/nonexistent.mdl") != -1)
|
||||
{
|
||||
printf("FAIL: hash found nonexistent model\n");
|
||||
pass = 0;
|
||||
}
|
||||
|
||||
/* Benchmark */
|
||||
start = clock();
|
||||
for (i = 0; i < NUM_LOOKUPS; i++)
|
||||
{
|
||||
int idx = 2 + (i % (num_models - 2));
|
||||
volatile int r = find_model_linear(model_precache[idx]);
|
||||
(void)r;
|
||||
}
|
||||
end = clock();
|
||||
linear_time = (double)(end - start) / CLOCKS_PER_SEC;
|
||||
|
||||
start = clock();
|
||||
for (i = 0; i < NUM_LOOKUPS; i++)
|
||||
{
|
||||
int idx = 2 + (i % (num_models - 2));
|
||||
volatile int r = find_model_hash(model_precache[idx]);
|
||||
(void)r;
|
||||
}
|
||||
end = clock();
|
||||
hash_time = (double)(end - start) / CLOCKS_PER_SEC;
|
||||
|
||||
ratio = linear_time / (hash_time > 0 ? hash_time : 0.0001);
|
||||
|
||||
printf("xonotic-0002: SV_ModelIndex linear scan defect\n");
|
||||
printf(" Models: %d, Lookups: %d\n", num_models - 2, NUM_LOOKUPS);
|
||||
printf(" Linear: %.4fs, Hash: %.4fs, Ratio: %.1fx\n", linear_time, hash_time, ratio);
|
||||
|
||||
if (ratio < 2.0)
|
||||
{
|
||||
printf("FAIL: expected hash lookup to be at least 2x faster (got %.1fx)\n", ratio);
|
||||
pass = 0;
|
||||
}
|
||||
|
||||
printf("%s\n", pass ? "PASS" : "FAIL");
|
||||
return pass ? 0 : 1;
|
||||
}
|
||||
52
defects/xonotic-0003/patch/xonotic-0003.patch
Normal file
52
defects/xonotic-0003/patch/xonotic-0003.patch
Normal file
|
|
@ -0,0 +1,52 @@
|
|||
--- a/sv_main.c
|
||||
+++ b/sv_main.c
|
||||
@@ -1468,12 +1468,21 @@ SV_SoundIndex
|
||||
|
||||
================
|
||||
*/
|
||||
+// Hash lookup for SV_SoundIndex: avoids O(S) linear scan on every sound reference.
|
||||
+#define SV_SOUND_PRECACHE_HASHSIZE 512
|
||||
+static int sv_sound_precache_hashtable[SV_SOUND_PRECACHE_HASHSIZE];
|
||||
+static int sv_sound_precache_hashnext[MAX_SOUNDS];
|
||||
+
|
||||
+static unsigned SV_SoundPrecacheHash(const char *s)
|
||||
+{
|
||||
+ unsigned hash = 0;
|
||||
+ for (; *s; s++)
|
||||
+ hash = hash * 31 + (unsigned char)*s;
|
||||
+ return hash % SV_SOUND_PRECACHE_HASHSIZE;
|
||||
+}
|
||||
+
|
||||
int SV_SoundIndex(const char *s, int precachemode)
|
||||
{
|
||||
- int i, limit = ((sv.protocol == PROTOCOL_QUAKE || sv.protocol == PROTOCOL_QUAKEDP || sv.protocol == PROTOCOL_NEHAHRAMOVIE || sv.protocol == PROTOCOL_NEHAHRABJP) ? 256 : MAX_SOUNDS);
|
||||
+ int i, limit = ((sv.protocol == PROTOCOL_QUAKE || sv.protocol == PROTOCOL_QUAKEDP || sv.protocol == PROTOCOL_NEHAHRAMOVIE || sv.protocol == PROTOCOL_NEHAHRABJP) ? 256 : MAX_SOUNDS);
|
||||
char filename[MAX_QPATH];
|
||||
if (!s || !*s)
|
||||
return 0;
|
||||
@@ -1481,7 +1490,12 @@ int SV_SoundIndex(const char *s, int precachemode)
|
||||
// return 0;
|
||||
dp_strlcpy(filename, s, sizeof(filename));
|
||||
- for (i = 1;i < limit;i++)
|
||||
+ // Hash lookup: O(1) average instead of O(S) linear scan.
|
||||
+ unsigned h = SV_SoundPrecacheHash(filename);
|
||||
+ for (i = sv_sound_precache_hashtable[h]; i >= 0; i = sv_sound_precache_hashnext[i])
|
||||
+ if (!strcmp(sv.sound_precache[i], filename))
|
||||
+ return i;
|
||||
+ // Not found: scan for empty slot to insert (precache mode)
|
||||
+ for (i = 1;i < limit;i++)
|
||||
{
|
||||
if (!sv.sound_precache[i][0])
|
||||
{
|
||||
@@ -1503,8 +1517,11 @@ int SV_SoundIndex(const char *s, int precachemode)
|
||||
MSG_WriteString(&sv.reliable_datagram, filename);
|
||||
}
|
||||
+ // Insert into hash table
|
||||
+ sv_sound_precache_hashnext[i] = sv_sound_precache_hashtable[h];
|
||||
+ sv_sound_precache_hashtable[h] = i;
|
||||
return i;
|
||||
}
|
||||
- if (!strcmp(sv.sound_precache[i], filename))
|
||||
- return i;
|
||||
}
|
||||
Con_Printf("SV_SoundIndex(\"%s\"): i (%i) == MAX_SOUNDS (%i)\n", filename, i, MAX_SOUNDS);
|
||||
BIN
defects/xonotic-0003/test/test_sv_sound_index
Executable file
BIN
defects/xonotic-0003/test/test_sv_sound_index
Executable file
Binary file not shown.
144
defects/xonotic-0003/test/test_sv_sound_index.c
Normal file
144
defects/xonotic-0003/test/test_sv_sound_index.c
Normal file
|
|
@ -0,0 +1,144 @@
|
|||
/**
|
||||
* Unit test for xonotic-0003: SV_SoundIndex O(S) linear scan.
|
||||
*
|
||||
* Defect: sv_main.c SV_SoundIndex() linearly scans the sound_precache array
|
||||
* (up to MAX_SOUNDS = 4096 entries) on every sound reference. Called from
|
||||
* SV_StartSound, SV_StartPointSound, every weapon fire, footstep, etc.
|
||||
*
|
||||
* Fix: hash table for O(1) average lookup.
|
||||
*/
|
||||
|
||||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
#include <time.h>
|
||||
|
||||
#define MAX_SOUNDS 4096
|
||||
#define MAX_QPATH 128
|
||||
#define NUM_LOOKUPS 100000
|
||||
#define HASH_SIZE 512
|
||||
|
||||
static char sound_precache[MAX_SOUNDS][MAX_QPATH];
|
||||
static int num_sounds = 0;
|
||||
|
||||
/* Unpatched: O(S) linear scan */
|
||||
static int find_sound_linear(const char *filename)
|
||||
{
|
||||
int i;
|
||||
for (i = 1; i < num_sounds; i++)
|
||||
{
|
||||
if (!sound_precache[i][0])
|
||||
return -1;
|
||||
if (!strcmp(sound_precache[i], filename))
|
||||
return i;
|
||||
}
|
||||
return -1;
|
||||
}
|
||||
|
||||
/* Patched: O(1) hash lookup */
|
||||
static int sound_hashtable[HASH_SIZE];
|
||||
static int sound_hashnext[MAX_SOUNDS];
|
||||
|
||||
static unsigned sound_hash(const char *s)
|
||||
{
|
||||
unsigned hash = 0;
|
||||
for (; *s; s++)
|
||||
hash = hash * 31 + (unsigned char)*s;
|
||||
return hash % HASH_SIZE;
|
||||
}
|
||||
|
||||
static void sound_hash_init(void)
|
||||
{
|
||||
int i;
|
||||
for (i = 0; i < HASH_SIZE; i++)
|
||||
sound_hashtable[i] = -1;
|
||||
for (i = 0; i < MAX_SOUNDS; i++)
|
||||
sound_hashnext[i] = -1;
|
||||
for (i = 1; i < num_sounds; i++)
|
||||
{
|
||||
if (!sound_precache[i][0])
|
||||
continue;
|
||||
unsigned h = sound_hash(sound_precache[i]);
|
||||
sound_hashnext[i] = sound_hashtable[h];
|
||||
sound_hashtable[h] = i;
|
||||
}
|
||||
}
|
||||
|
||||
static int find_sound_hash(const char *filename)
|
||||
{
|
||||
unsigned h = sound_hash(filename);
|
||||
int i;
|
||||
for (i = sound_hashtable[h]; i >= 0; i = sound_hashnext[i])
|
||||
if (!strcmp(sound_precache[i], filename))
|
||||
return i;
|
||||
return -1;
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
int i, pass = 1;
|
||||
clock_t start, end;
|
||||
double linear_time, hash_time, ratio;
|
||||
|
||||
/* Populate: simulate a map with many sounds */
|
||||
num_sounds = 2000;
|
||||
for (i = 1; i < num_sounds; i++)
|
||||
snprintf(sound_precache[i], MAX_QPATH, "sounds/weapons/shot_%04d.wav", i);
|
||||
|
||||
sound_hash_init();
|
||||
|
||||
/* Correctness */
|
||||
for (i = 1; i < num_sounds; i++)
|
||||
{
|
||||
int lin = find_sound_linear(sound_precache[i]);
|
||||
int hsh = find_sound_hash(sound_precache[i]);
|
||||
if (lin != hsh)
|
||||
{
|
||||
printf("FAIL: mismatch at sound %d: linear=%d hash=%d\n", i, lin, hsh);
|
||||
pass = 0;
|
||||
}
|
||||
}
|
||||
|
||||
/* Not-found */
|
||||
if (find_sound_hash("sounds/nonexistent.wav") != -1)
|
||||
{
|
||||
printf("FAIL: hash found nonexistent sound\n");
|
||||
pass = 0;
|
||||
}
|
||||
|
||||
/* Benchmark */
|
||||
start = clock();
|
||||
for (i = 0; i < NUM_LOOKUPS; i++)
|
||||
{
|
||||
int idx = 1 + (i % (num_sounds - 1));
|
||||
volatile int r = find_sound_linear(sound_precache[idx]);
|
||||
(void)r;
|
||||
}
|
||||
end = clock();
|
||||
linear_time = (double)(end - start) / CLOCKS_PER_SEC;
|
||||
|
||||
start = clock();
|
||||
for (i = 0; i < NUM_LOOKUPS; i++)
|
||||
{
|
||||
int idx = 1 + (i % (num_sounds - 1));
|
||||
volatile int r = find_sound_hash(sound_precache[idx]);
|
||||
(void)r;
|
||||
}
|
||||
end = clock();
|
||||
hash_time = (double)(end - start) / CLOCKS_PER_SEC;
|
||||
|
||||
ratio = linear_time / (hash_time > 0 ? hash_time : 0.0001);
|
||||
|
||||
printf("xonotic-0003: SV_SoundIndex linear scan defect\n");
|
||||
printf(" Sounds: %d, Lookups: %d\n", num_sounds - 1, NUM_LOOKUPS);
|
||||
printf(" Linear: %.4fs, Hash: %.4fs, Ratio: %.1fx\n", linear_time, hash_time, ratio);
|
||||
|
||||
if (ratio < 2.0)
|
||||
{
|
||||
printf("FAIL: expected hash lookup to be at least 2x faster (got %.1fx)\n", ratio);
|
||||
pass = 0;
|
||||
}
|
||||
|
||||
printf("%s\n", pass ? "PASS" : "FAIL");
|
||||
return pass ? 0 : 1;
|
||||
}
|
||||
57
defects/xonotic-0004/patch/xonotic-0004.patch
Normal file
57
defects/xonotic-0004/patch/xonotic-0004.patch
Normal file
|
|
@ -0,0 +1,57 @@
|
|||
--- a/snd_main.c
|
||||
+++ b/snd_main.c
|
||||
@@ -888,6 +888,20 @@ S_FindName
|
||||
==================
|
||||
*/
|
||||
+// Hash table for S_FindName: avoids O(N) linked list scan on every sound lookup.
|
||||
+// The existing code has a TODO comment: "TODO: hash table search?"
|
||||
+#define SFX_HASHSIZE 256
|
||||
+static sfx_t *sfx_hashtable[SFX_HASHSIZE];
|
||||
+
|
||||
+static unsigned S_NameHash(const char *name)
|
||||
+{
|
||||
+ unsigned hash = 0;
|
||||
+ const unsigned char *p;
|
||||
+ for (p = (const unsigned char *)name; *p; p++)
|
||||
+ hash = hash * 31 + *p;
|
||||
+ return hash % SFX_HASHSIZE;
|
||||
+}
|
||||
+
|
||||
sfx_t *S_FindName (const char *name)
|
||||
{
|
||||
sfx_t *sfx;
|
||||
@@ -908,9 +922,12 @@ sfx_t *S_FindName (const char *name)
|
||||
}
|
||||
|
||||
// Look for this sound in the list of known sfx
|
||||
- // TODO: hash table search?
|
||||
- for (sfx = known_sfx; sfx != NULL; sfx = sfx->next)
|
||||
- if(!strcmp (sfx->name, name))
|
||||
- return sfx;
|
||||
+ // Hash table search replaces O(N) linked list scan.
|
||||
+ unsigned hashkey = S_NameHash(name);
|
||||
+ for (sfx = sfx_hashtable[hashkey]; sfx != NULL; sfx = sfx->hashnext)
|
||||
+ if (!strcmp(sfx->name, name))
|
||||
+ return sfx;
|
||||
|
||||
// check for # in the beginning, try lookup by soundindex
|
||||
if (name[0] == '#' && name[1])
|
||||
@@ -925,6 +942,9 @@ sfx_t *S_FindName (const char *name)
|
||||
dp_strlcpy (sfx->name, name, sizeof (sfx->name));
|
||||
sfx->memsize = sizeof(*sfx);
|
||||
sfx->next = known_sfx;
|
||||
+ // Insert into hash table
|
||||
+ sfx->hashnext = sfx_hashtable[hashkey];
|
||||
+ sfx_hashtable[hashkey] = sfx;
|
||||
known_sfx = sfx;
|
||||
|
||||
return sfx;
|
||||
--- a/snd_main.h
|
||||
+++ b/snd_main.h
|
||||
@@ -100,6 +100,7 @@ typedef struct sfx_s
|
||||
unsigned int flags; // cf SFXFLAG_* defines
|
||||
|
||||
struct sfx_s *next;
|
||||
+ struct sfx_s *hashnext; // hash chain for S_FindName O(1) lookup
|
||||
unsigned int memsize; // total memory used (including sfx_t and fetcher data)
|
||||
} sfx_t;
|
||||
BIN
defects/xonotic-0004/test/test_s_findname
Executable file
BIN
defects/xonotic-0004/test/test_s_findname
Executable file
Binary file not shown.
142
defects/xonotic-0004/test/test_s_findname.c
Normal file
142
defects/xonotic-0004/test/test_s_findname.c
Normal file
|
|
@ -0,0 +1,142 @@
|
|||
/**
|
||||
* Unit test for xonotic-0004: S_FindName O(N) linked list scan.
|
||||
*
|
||||
* Defect: snd_main.c S_FindName() traverses a linked list of known sfx entries
|
||||
* to find a sound by name. The code itself has a "TODO: hash table search?"
|
||||
* comment. Called every time a sound is played, precached, or referenced.
|
||||
* With many sounds loaded, each lookup is O(N).
|
||||
*
|
||||
* Fix: hash table on sfx name for O(1) average lookup.
|
||||
*/
|
||||
|
||||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
#include <time.h>
|
||||
|
||||
#define MAX_SFX 2000
|
||||
#define NUM_LOOKUPS 100000
|
||||
#define SFX_HASHSIZE 256
|
||||
|
||||
typedef struct sfx_s {
|
||||
char name[64];
|
||||
struct sfx_s *next;
|
||||
struct sfx_s *hashnext;
|
||||
} sfx_t;
|
||||
|
||||
static sfx_t sfx_pool[MAX_SFX];
|
||||
static sfx_t *known_sfx = NULL;
|
||||
static sfx_t *sfx_hashtable[SFX_HASHSIZE];
|
||||
|
||||
static unsigned sfx_hash(const char *name)
|
||||
{
|
||||
unsigned hash = 0;
|
||||
const unsigned char *p;
|
||||
for (p = (const unsigned char *)name; *p; p++)
|
||||
hash = hash * 31 + *p;
|
||||
return hash % SFX_HASHSIZE;
|
||||
}
|
||||
|
||||
/* Unpatched: O(N) linked list scan */
|
||||
static sfx_t *find_sfx_linear(const char *name)
|
||||
{
|
||||
sfx_t *sfx;
|
||||
for (sfx = known_sfx; sfx != NULL; sfx = sfx->next)
|
||||
if (!strcmp(sfx->name, name))
|
||||
return sfx;
|
||||
return NULL;
|
||||
}
|
||||
|
||||
/* Patched: O(1) hash lookup */
|
||||
static sfx_t *find_sfx_hash(const char *name)
|
||||
{
|
||||
unsigned h = sfx_hash(name);
|
||||
sfx_t *sfx;
|
||||
for (sfx = sfx_hashtable[h]; sfx != NULL; sfx = sfx->hashnext)
|
||||
if (!strcmp(sfx->name, name))
|
||||
return sfx;
|
||||
return NULL;
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
int i, pass = 1;
|
||||
clock_t start, end;
|
||||
double linear_time, hash_time, ratio;
|
||||
|
||||
/* Build linked list and hash table */
|
||||
memset(sfx_hashtable, 0, sizeof(sfx_hashtable));
|
||||
known_sfx = NULL;
|
||||
|
||||
for (i = 0; i < MAX_SFX; i++)
|
||||
{
|
||||
sfx_t *s = &sfx_pool[i];
|
||||
snprintf(s->name, sizeof(s->name), "sounds/fx/effect_%04d.ogg", i);
|
||||
s->next = known_sfx;
|
||||
known_sfx = s;
|
||||
|
||||
unsigned h = sfx_hash(s->name);
|
||||
s->hashnext = sfx_hashtable[h];
|
||||
sfx_hashtable[h] = s;
|
||||
}
|
||||
|
||||
/* Correctness */
|
||||
for (i = 0; i < MAX_SFX; i++)
|
||||
{
|
||||
sfx_t *lin = find_sfx_linear(sfx_pool[i].name);
|
||||
sfx_t *hsh = find_sfx_hash(sfx_pool[i].name);
|
||||
if (lin != hsh)
|
||||
{
|
||||
printf("FAIL: mismatch at sfx %d: linear=%p hash=%p\n", i, (void*)lin, (void*)hsh);
|
||||
pass = 0;
|
||||
}
|
||||
}
|
||||
|
||||
/* Not-found */
|
||||
if (find_sfx_hash("nonexistent.wav") != NULL)
|
||||
{
|
||||
printf("FAIL: hash found nonexistent sfx\n");
|
||||
pass = 0;
|
||||
}
|
||||
if (find_sfx_linear("nonexistent.wav") != NULL)
|
||||
{
|
||||
printf("FAIL: linear found nonexistent sfx\n");
|
||||
pass = 0;
|
||||
}
|
||||
|
||||
/* Benchmark */
|
||||
start = clock();
|
||||
for (i = 0; i < NUM_LOOKUPS; i++)
|
||||
{
|
||||
int idx = i % MAX_SFX;
|
||||
volatile sfx_t *r = find_sfx_linear(sfx_pool[idx].name);
|
||||
(void)r;
|
||||
}
|
||||
end = clock();
|
||||
linear_time = (double)(end - start) / CLOCKS_PER_SEC;
|
||||
|
||||
start = clock();
|
||||
for (i = 0; i < NUM_LOOKUPS; i++)
|
||||
{
|
||||
int idx = i % MAX_SFX;
|
||||
volatile sfx_t *r = find_sfx_hash(sfx_pool[idx].name);
|
||||
(void)r;
|
||||
}
|
||||
end = clock();
|
||||
hash_time = (double)(end - start) / CLOCKS_PER_SEC;
|
||||
|
||||
ratio = linear_time / (hash_time > 0 ? hash_time : 0.0001);
|
||||
|
||||
printf("xonotic-0004: S_FindName linked list scan defect\n");
|
||||
printf(" SFX entries: %d, Lookups: %d\n", MAX_SFX, NUM_LOOKUPS);
|
||||
printf(" Linear: %.4fs, Hash: %.4fs, Ratio: %.1fx\n", linear_time, hash_time, ratio);
|
||||
|
||||
if (ratio < 2.0)
|
||||
{
|
||||
printf("FAIL: expected hash lookup to be at least 2x faster (got %.1fx)\n", ratio);
|
||||
pass = 0;
|
||||
}
|
||||
|
||||
printf("%s\n", pass ? "PASS" : "FAIL");
|
||||
return pass ? 0 : 1;
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue