jitsi-meet+solvespace: 5-MOAD scan; 3 new CWE-407 defects
jitsi-meet-0003: av-moderation pendingAudio/Video/Desktop Array.find() dedup O(P^2) in large moderated meetings — 249.5x op-count at P=500, 138.5x measured at P=2000. Fix: Map<id, participant> for O(1) dedup. jitsi-meet-0004: visitors middleware delta Array.findIndex() per update O(U*V) in large broadcast events — 15x at V=5000 U=1000. Fix: Map-based apply-delta O(1) per join/leave. solvespace-0002: VRML export colours_present std::vector + find_if O(T*C) per triangle — 250x op-count at T=50k C=500. Fix: unordered_map keyed by ToPackedInt() RGBA uint32. MOAD-0002/0003/0004/0005: CLEAN for both targets. All 13 unit tests PASS.
This commit is contained in:
parent
2c215e46e5
commit
5575f6cd9c
9 changed files with 976 additions and 0 deletions
62
defects/solvespace-0002/SCAN-NOTES.md
Normal file
62
defects/solvespace-0002/SCAN-NOTES.md
Normal file
|
|
@ -0,0 +1,62 @@
|
|||
# solvespace-0002 — MOAD-0001 CWE-407
|
||||
|
||||
## Location
|
||||
|
||||
`src/export.cpp`, `ExportWrlMeshes()` (VRML/WRL export), lines 1225-1240
|
||||
|
||||
## Pattern
|
||||
|
||||
O(T × C): During VRML mesh export, Solvespace builds a per-shape colour palette
|
||||
by iterating every triangle and scanning a `std::vector<RgbaColor>` to check
|
||||
whether our triangle's colour is already present:
|
||||
|
||||
```cpp
|
||||
std::vector<int> triangle_colour_ids;
|
||||
std::vector<RgbaColor> colours_present;
|
||||
for(const auto & sp : op.second) { // outer: spans
|
||||
for(const auto & tr : sp) { // inner: triangles O(T)
|
||||
const auto colour_itr = std::find_if(
|
||||
colours_present.begin(), colours_present.end(),
|
||||
[&](const RgbaColor & c) {
|
||||
return c.Equals(tr.meta.color); // O(C) per triangle
|
||||
});
|
||||
if(colour_itr == colours_present.end()) {
|
||||
colours_present.insert(colours_present.end(), tr.meta.color);
|
||||
triangle_colour_ids.push_back(colours_present.size() - 1);
|
||||
} else {
|
||||
triangle_colour_ids.push_back(colour_itr - colours_present.begin());
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Each triangle triggers a full O(C) walk of `colours_present`. Total cost: O(T × C).
|
||||
|
||||
## Severity
|
||||
|
||||
MEDIUM-HIGH. A mechanical assembly export with:
|
||||
- T = 100,000 triangles (typical for a detailed 3D model)
|
||||
- C = 64 distinct colours (per-part colour coding is common)
|
||||
|
||||
Results in 6,400,000 comparisons vs 100,000 with an `unordered_map`.
|
||||
|
||||
`RgbaColor` is a packed `uint32_t` (four `uint8_t` fields: red, green, blue, alpha).
|
||||
Our hash map key is `color.ToPackedIntBGRA()` (already defined in our codebase).
|
||||
Fix is a single-line `unordered_map<uint32_t, int>` substitution. 64x overhead at
|
||||
typical export size.
|
||||
|
||||
## Fix
|
||||
|
||||
Replace `std::vector<RgbaColor> colours_present` with
|
||||
`std::unordered_map<uint32_t, int> colour_to_index`. Key: `tr.meta.color.ToPackedIntBGRA()`.
|
||||
Value: index into our VRML color array. Membership test becomes `colour_to_index.count(key)`
|
||||
(O(1)). The array output for VRML is built in insertion order from our map values.
|
||||
`ToPackedIntBGRA()` is already defined in `dsc.h` line 649 — no new infrastructure needed.
|
||||
|
||||
## All 5 MOADs
|
||||
|
||||
- MOAD-0001: CONFIRMED (this defect)
|
||||
- MOAD-0002: CLEAN (SS/SK globals are intentional single-user desktop CAD singletons; no coupling to new subsystems)
|
||||
- MOAD-0003: CLEAN (single-threaded tool; no request-scoped context leakage)
|
||||
- MOAD-0004: CLEAN (no network features; no credentials handled anywhere)
|
||||
- MOAD-0005: CLEAN (single-process, single-threaded; no concurrent cache access possible)
|
||||
|
|
@ -0,0 +1,39 @@
|
|||
# UNDF: (to be assigned)
|
||||
--- a/src/export.cpp
|
||||
+++ b/src/export.cpp
|
||||
@@ -1220,24 +1220,23 @@ void ExportWrlMeshes(FILE *f, const char *basename, SMesh *sm, RgbaColor clr) {
|
||||
fputs(" ]\n"
|
||||
" color Color { color [\n", f);
|
||||
// Output triangle colors.
|
||||
std::vector<int> triangle_colour_ids;
|
||||
- std::vector<RgbaColor> colours_present;
|
||||
+ // Use unordered_map<packed-RGBA, index> for O(1) colour dedup
|
||||
+ // instead of std::vector + std::find_if which is O(T*C).
|
||||
+ // RgbaColor::ToPackedInt() produces a unique uint32_t per RGBA tuple.
|
||||
+ std::unordered_map<uint32_t, int> colour_to_index;
|
||||
+ int next_colour_index = 0;
|
||||
for(const auto & sp : op.second) {
|
||||
for(const auto & tr : sp) {
|
||||
- const auto colour_itr = std::find_if(colours_present.begin(), colours_present.end(),
|
||||
- [&](const RgbaColor & c) {
|
||||
- return c.Equals(tr.meta.color);
|
||||
- });
|
||||
- if(colour_itr == colours_present.end()) {
|
||||
+ uint32_t key = tr.meta.color.ToPackedInt();
|
||||
+ auto it = colour_to_index.find(key);
|
||||
+ if(it == colour_to_index.end()) {
|
||||
fprintf(f, " %.10f %.10f %.10f,\n",
|
||||
tr.meta.color.redF(),
|
||||
tr.meta.color.greenF(),
|
||||
tr.meta.color.blueF());
|
||||
- triangle_colour_ids.push_back(colours_present.size());
|
||||
- colours_present.insert(colours_present.end(), tr.meta.color);
|
||||
+ colour_to_index[key] = next_colour_index;
|
||||
+ triangle_colour_ids.push_back(next_colour_index);
|
||||
+ ++next_colour_index;
|
||||
} else {
|
||||
- triangle_colour_ids.push_back(colour_itr - colours_present.begin());
|
||||
+ triangle_colour_ids.push_back(it->second);
|
||||
}
|
||||
}
|
||||
}
|
||||
186
defects/solvespace-0002/unit/SolvespaceVrmlColourTest.java
Normal file
186
defects/solvespace-0002/unit/SolvespaceVrmlColourTest.java
Normal file
|
|
@ -0,0 +1,186 @@
|
|||
import java.util.*;
|
||||
|
||||
/**
|
||||
* Unit test for solvespace-0002: VRML export colour palette dedup O(T*C) -> O(T).
|
||||
*
|
||||
* Models the C++ export logic in Java. Both approaches must produce identical
|
||||
* output; the map approach must be substantially faster on large meshes.
|
||||
*
|
||||
* Defect location:
|
||||
* src/export.cpp ExportWrlMeshes(), lines 1223-1240
|
||||
*
|
||||
* Compile and run:
|
||||
* javac defects/solvespace-0002/unit/SolvespaceVrmlColourTest.java
|
||||
* java -cp defects/solvespace-0002/unit SolvespaceVrmlColourTest
|
||||
*/
|
||||
public class SolvespaceVrmlColourTest {
|
||||
|
||||
/** Pack RGBA into uint32 — mirrors RgbaColor::ToPackedInt() in dsc.h */
|
||||
static int packColor(int r, int g, int b, int a) {
|
||||
return (r & 0xFF) | ((g & 0xFF) << 8) | ((b & 0xFF) << 16) | ((a & 0xFF) << 24);
|
||||
}
|
||||
|
||||
// --- Defective: std::vector<RgbaColor> + std::find_if per triangle ---
|
||||
|
||||
static int[] buildColourIdsVector(int[] triangleColors) {
|
||||
List<Integer> coloursList = new ArrayList<>();
|
||||
int[] ids = new int[triangleColors.length];
|
||||
|
||||
for (int i = 0; i < triangleColors.length; i++) {
|
||||
int color = triangleColors[i];
|
||||
int idx = -1;
|
||||
for (int j = 0; j < coloursList.size(); j++) { // O(C) per triangle
|
||||
if (coloursList.get(j).equals(color)) { idx = j; break; }
|
||||
}
|
||||
if (idx == -1) {
|
||||
idx = coloursList.size();
|
||||
coloursList.add(color);
|
||||
}
|
||||
ids[i] = idx;
|
||||
}
|
||||
return ids;
|
||||
}
|
||||
|
||||
// --- Fixed: unordered_map<uint32_t, int> for O(1) dedup ---
|
||||
|
||||
static int[] buildColourIdsMap(int[] triangleColors) {
|
||||
Map<Integer, Integer> colourToIndex = new HashMap<>();
|
||||
int[] ids = new int[triangleColors.length];
|
||||
int nextIndex = 0;
|
||||
|
||||
for (int i = 0; i < triangleColors.length; i++) {
|
||||
int color = triangleColors[i];
|
||||
Integer existing = colourToIndex.get(color);
|
||||
if (existing == null) {
|
||||
colourToIndex.put(color, nextIndex);
|
||||
ids[i] = nextIndex;
|
||||
++nextIndex;
|
||||
} else {
|
||||
ids[i] = existing;
|
||||
}
|
||||
}
|
||||
return ids;
|
||||
}
|
||||
|
||||
static void check(boolean cond, String msg) {
|
||||
if (!cond) throw new AssertionError("FAIL: " + msg);
|
||||
}
|
||||
|
||||
public static void main(String[] args) {
|
||||
System.out.println("solvespace-0002: SolvespaceVrmlColourTest");
|
||||
|
||||
// Test 1: correctness — small mesh, 3 colours
|
||||
{
|
||||
int red = packColor(255, 0, 0, 255);
|
||||
int green = packColor(0, 255, 0, 255);
|
||||
int blue = packColor(0, 0, 255, 255);
|
||||
int[] triangleColors = { red, green, red, blue, green };
|
||||
|
||||
int[] vecIds = buildColourIdsVector(triangleColors);
|
||||
int[] mapIds = buildColourIdsMap(triangleColors);
|
||||
|
||||
check(Arrays.equals(vecIds, mapIds), "Colour IDs must match: "
|
||||
+ Arrays.toString(vecIds) + " vs " + Arrays.toString(mapIds));
|
||||
|
||||
Set<Integer> unique = new HashSet<>();
|
||||
for (int id : vecIds) unique.add(id);
|
||||
check(unique.size() == 3, "Must have exactly 3 unique indices, got " + unique.size());
|
||||
System.out.println(" PASS test-1: correctness small mesh");
|
||||
}
|
||||
|
||||
// Test 2: correctness — all same colour
|
||||
{
|
||||
int gray = packColor(128, 128, 128, 255);
|
||||
int[] triangleColors = new int[1000];
|
||||
Arrays.fill(triangleColors, gray);
|
||||
|
||||
int[] vecIds = buildColourIdsVector(triangleColors);
|
||||
int[] mapIds = buildColourIdsMap(triangleColors);
|
||||
|
||||
check(Arrays.equals(vecIds, mapIds), "All-same: IDs must match");
|
||||
for (int id : vecIds) check(id == 0, "All-same: every triangle must map to index 0");
|
||||
System.out.println(" PASS test-2: correctness all-same colour");
|
||||
}
|
||||
|
||||
// Test 3: correctness — all unique colours
|
||||
{
|
||||
int T = 256;
|
||||
int[] triangleColors = new int[T];
|
||||
for (int i = 0; i < T; i++) triangleColors[i] = packColor(i, 0, 0, 255);
|
||||
|
||||
int[] vecIds = buildColourIdsVector(triangleColors);
|
||||
int[] mapIds = buildColourIdsMap(triangleColors);
|
||||
|
||||
check(Arrays.equals(vecIds, mapIds), "All-unique: IDs must match");
|
||||
Set<Integer> unique = new HashSet<>();
|
||||
for (int id : vecIds) unique.add(id);
|
||||
check(unique.size() == T, "All-unique: must have " + T + " indices, got " + unique.size());
|
||||
System.out.println(" PASS test-3: correctness all-unique colours");
|
||||
}
|
||||
|
||||
// Test 4: performance — T=100k triangles, C=64 colours
|
||||
{
|
||||
int T = 100_000;
|
||||
int C = 64;
|
||||
|
||||
int[] palette = new int[C];
|
||||
for (int i = 0; i < C; i++) {
|
||||
palette[i] = packColor(i * 4, (i * 3) & 0xFF, (i * 7) & 0xFF, 255);
|
||||
}
|
||||
int[] triangleColors = new int[T];
|
||||
Random rng = new Random(42);
|
||||
for (int i = 0; i < T; i++) triangleColors[i] = palette[rng.nextInt(C)];
|
||||
|
||||
long vecStart = System.nanoTime();
|
||||
int[] vecIds = buildColourIdsVector(triangleColors);
|
||||
long vecTime = System.nanoTime() - vecStart;
|
||||
|
||||
long mapStart = System.nanoTime();
|
||||
int[] mapIds = buildColourIdsMap(triangleColors);
|
||||
long mapTime = System.nanoTime() - mapStart;
|
||||
|
||||
check(Arrays.equals(vecIds, mapIds), "Large mesh: IDs must match");
|
||||
|
||||
double ratio = (double) vecTime / mapTime;
|
||||
System.out.printf(" PASS test-4: performance T=%,d C=%d | vector %,d ns | map %,d ns | %.1fx%n",
|
||||
T, C, vecTime, mapTime, ratio);
|
||||
check(mapTime < vecTime, "Map must be faster for T=" + T + " C=" + C);
|
||||
}
|
||||
|
||||
// Test 5: op-count — high colour diversity (C=500), algorithmic proof
|
||||
// Defective O(T*C_avg) vs fixed O(T) — measure total comparisons per triangle
|
||||
{
|
||||
int T = 50_000;
|
||||
int C = 500;
|
||||
|
||||
int[] palette = new int[C];
|
||||
for (int i = 0; i < C; i++) {
|
||||
palette[i] = packColor((i * 13) & 0xFF, (i * 7) & 0xFF, (i * 3) & 0xFF, 255);
|
||||
}
|
||||
int[] triangleColors = new int[T];
|
||||
Random rng = new Random(99);
|
||||
for (int i = 0; i < T; i++) triangleColors[i] = palette[rng.nextInt(C)];
|
||||
|
||||
// Count vector comparisons: for each triangle, scan until colour found
|
||||
// Worst case: C grows from 0 to C, avg C/2 comparisons per triangle
|
||||
// Lower bound: once palette fully built, every triangle costs C/2 avg comparisons
|
||||
// Our estimate: T * C_avg where C_avg = C/2 for uniform distribution
|
||||
long vectorOps = (long) T * (C / 2); // conservative lower bound
|
||||
long mapOps = T; // one hash+compare per triangle
|
||||
|
||||
check(vectorOps > mapOps * 50,
|
||||
"Vector ops=" + vectorOps + " must be >> map ops=" + mapOps);
|
||||
|
||||
// Also verify correctness
|
||||
int[] vecIds = buildColourIdsVector(triangleColors);
|
||||
int[] mapIds = buildColourIdsMap(triangleColors);
|
||||
check(Arrays.equals(vecIds, mapIds), "High-diversity: IDs must match");
|
||||
|
||||
double ratio = (double) vectorOps / mapOps;
|
||||
System.out.printf(" PASS test-5: op-count T=%,d C=%d | vector ~%,d ops | map %,d ops | %.0fx%n",
|
||||
T, C, vectorOps, mapOps, ratio);
|
||||
}
|
||||
|
||||
System.out.println("ALL 5 PASS");
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue