subagent transcript

session 18e25685 · workflow wf_23df5b1c
delegated task
You are an adversarial verifier for a FreeCAD-wasm code review. A reviewer claims: SEVERITY: low CATEGORY: correctness-edge FILE: src/Gui/WasmGLFixedF
▸ Magik
You are an adversarial verifier for a FreeCAD-wasm code review. A reviewer claims: SEVERITY: low CATEGORY: correctness-edge FILE: src/Gui/WasmGLFixedFunc.cpp SUMMARY: Client-array indexed-draw sizing references F.arrays.vertex._count which is never assigned, so the VBO-indices-with-client-vertices fallback can under-size the gather and corrupt/miss geometry. DETAIL/EVIDENCE: At line 179, when indices live in a bound ELEMENT_ARRAY_BUFFER but vertices are client memory, maxIndex is set from F.arrays.vertex._count (never set anywhere), falling back to count-1. If the true max index exceeds count-1 the scratch Float32Array (line 150) is too small and the gather reads past it (NaN) or omits vertices. Niche combination (Coin usually keeps indices and vertices together) but a real latent bug. FIX: read back the element buffer (getBufferSubData) for the true max index, or record the vertex count in ffPointer and store it as _count. RECOMMENDATION: (none) Independently verify by reading the actual file(s) and running git/grep as needed (repo at /home/magik6k/lcad-wasm/freecad-port/FreeCAD, toolchain at /opt/toolchains, deploy at /home/magik6k/lcad-wasm/freecad-port/deploy-gui). Default to REFUTED if you cannot confirm the evidence. Adjust severity to what the evidence actually supports.
Fable
thinking
I should check the file to confirm that.
Fable
⚙ Read {"file_path": "/home/magik6k/lcad-wasm/freecad-port/FreeCAD/src/Gui/WasmGLFixedFunc.cpp", "offset": 120, "limit": 120}
tool result
120	  g.useProgram(F.prog);
121	
122	  const mvp = F.mul(F.pr[F.pr.length-1], F.mv[F.mv.length-1]);
123	  g.uniformMatrix4fv(F.loc.mvp, false, new Float32Array(mvp));
124	  g.uniformMatrix3fv(F.loc.nm, false, new Float32Array(F.normalMat3(F.mv[F.mv.length-1])));
125	  g.uniform4fv(F.loc.color, new Float32Array(F.color));
126	  g.uniform1i(F.loc.useCol, F.arrays.color.on ? 1 : 0);
127	  g.uniform1i(F.loc.lit, F.lighting ? 1 : 0);
128	  g.uniform3fv(F.loc.ldir, new Float32Array(F.lightDir));
129	   …[truncated]
Fable
thinking
I need to check if `_count` is actually assigned anywhere in the code, since line 179 references it but it might never be initialized.
Fable
⚙ Bash {"command": "grep -n \"_count\" /home/magik6k/lcad-wasm/freecad-port/FreeCAD/src/Gui/WasmGLFixedFunc.cpp", "description": "grep for _count usages"}
tool result
179:      F._maxIndex = F.arrays.vertex._count ? F.arrays.vertex._count-1 : count-1;
Fable
thinking
I'm checking where `_count` gets referenced and whether it's ever actually assigned anywhere in the `F.arrays.vertex` definition.
Fable
⚙ Bash {"command": "grep -n \"arrays\\|\\.vertex\\|count\\b\\|glbuf\\|\\.on\\b\\|ptr\\b\\|stride\\|size\\b\" /home/magik6k/lcad-wasm/freecad-port/FreeCAD/src/Gui/WasmGLFixedFunc.cpp | head -60", "description": "grep for arrays and vertex spec fiel…
tool result
31:typedef ptrdiff_t GLintptr;
64:    arrays: { // enabled + pointer specs
65:      vertex:{on:false,size:3,type:0x1406,stride:0,ptr:0,vbo:0},
66:      normal:{on:false,size:3,type:0x1406,stride:0,ptr:0,vbo:0},
67:      color: {on:false,size:4,type:0x1406,stride:0,ptr:0,vbo:0},
106:        psize:g.getUniformLocation(p,'uPointSize') };
117:EM_JS(void, ff_setup_and_draw, (GLenum prim, GLsizei count, GLenum idxType, GLintptr idxPtr, GLint first, int isElements), {
126:  g.uniform1i(F.loc.useCol, F.  …[truncated]
Fable
thinking
I'm checking the ffPointer function to verify that `_count` isn't being initialized anywhere in that section.
Fable
⚙ Read {"file_path": "/home/magik6k/lcad-wasm/freecad-port/FreeCAD/src/Gui/WasmGLFixedFunc.cpp", "offset": 345, "limit": 20}
tool result
345	/* ---- client vertex arrays ---- */
346	EM_JS(void, ffClientState, (GLenum arr, int on), {
347	  const A=globalThis.__ff.arrays;
348	  if(arr===0x8074)A.vertex.on=!!on; else if(arr===0x8075)A.normal.on=!!on;
349	  else if(arr===0x8076)A.color.on=!!on;
350	})
351	EM_JS(void, ffPointer, (int which, GLint size, GLenum type, GLsizei stride, GLintptr ptr), {
352	  const F=globalThis.__ff; const A=F.arrays; const g=F.gl();
353	  const spec = which===0?A.vertex:which===1?A.normal:A.color;
354	  sp  …[truncated]
Fable
thinking
So `_count` is never actually defined on the vertex spec object—it only has properties like `on`, `size`, `type`, `stride`, `ptr`, `vbo`, and `glbuf`. That means `F.arrays.vertex._count` is always undefined, causing the ternary to default to `count-1`. Now I'm tracing through the actual bug scenario where we have vertices in client memory and indices in a buffer.

When `isElements && vertClient` is true, `F._maxIndex` gets set to `count-1` since `_count` is undefined. Then in the bind phase, `maxV` becomes `count`, and the scratch array is sized to `count*sz`. But here's the problem: for indexed drawing, `count` is the number of indices being drawn, not the number of vertices. The actual max  …
Fable
I've verified all the load-bearing claims. Let me confirm my findings. The evidence checks out: 1. **Line 179** reads `F.arrays.vertex._count ? F.arrays.vertex._count-1 : count-1` — confirmed. 2. **`_count` is never assigned anywhere.** `grep -n "_count"` returns only line 179 (the read). The vertex spec object is defined at line 65 as `{on,size,type,stride,ptr,vbo}` and populated in `ffPointer` (lines 351-357), which sets only `size,type,stride,ptr,glbuf`. No `_count` property is ever written, so the ternary always evaluates the falsy branch and falls back to `count-1`. 3. **The under-sizing path is real.** When `isElements && vertClient && elemBuf` (indices in a bound ELEMENT_ARRAY_BUFFER, vertices in client memory), `F._maxIndex = count-1`, so `maxV = count` (line 149) and the scratch `Float32Array` is `count*sz` (line 150). For indexed draws, `count` is the number of *indices*, not vertices; the true maximum vertex index can exceed `count-1` (e.g., 3 indices {5,100,200}). The gather then produces only `count` vertices, and `drawElements` at line 193 reads the VBO-resident indices which reference beyond the gathered range → garbage/NaN or missing geometry. 4. The developer explicitly acknowledged this as an assumption in the comment at lines 177-178. Minor imprecision in the write-up: the gather loop itself stays within its own `maxV` bound; the corruption actually happens at the subsequent `drawElements` reading past the under-sized gathered VBO. Net effect is the same (corrupt/missing geometry), so this doesn't change the verdict. Severity: low is appropriate — the triggering combination (VBO indices + client vertices) is a niche mix, and in the common indexed case (vertices reused, indices > vertex count) `count-1` happens to over-size safely. It's a genuine latent bug but rarely hit.
Fable
⚙ StructuredOutput {"verdict": "CONFIRMED", "reason": "Verified against src/Gui/WasmGLFixedFunc.cpp. Line 179 references F.arrays.vertex._count, and grep confirms _count is never assigned anywhere in the file; the vertex spec (line 65) and its only writer ffP…
tool result
Structured output provided successfully