subagent transcript

session 18e25685 · workflow wf_23df5b1c
← 18e25685 · archive
delegated task
You are an adversarial verifier for a FreeCAD-wasm code review. A reviewer claims: SEVERITY: low CATEGORY: state-leak-gating FILE: src/Gui/WasmGLFixed
▸ Magik
You are an adversarial verifier for a FreeCAD-wasm code review. A reviewer claims: SEVERITY: low CATEGORY: state-leak-gating FILE: src/Gui/WasmGLFixedFunc.cpp SUMMARY: Fixed-function gating keys solely on arrays.vertex.on; if Coin ever leaves GL_VERTEX_ARRAY enabled, later draws routed through the C glDrawArrays symbol could be hijacked into the FF shader. DETAIL/EVIDENCE: ffFixedFuncActive (lines 388-390) returns true whenever arrays.vertex.on. glEnable/glDisableClientState toggle it and Coin normally disables it after each draw, but there is no scoping or save/restore. glPushAttrib/glPopAttrib and glPushClientAttrib/glPopClientAttrib are no-ops (lines 471-474), so a node relying on attrib-stack restore of client-array enables would leak state. In practice this is largely moot because Coin's main draws bypass our symbol (see critical finding) and Qt resolves GL through its own proc-address pointers, but it remains fragile. FIX: snapshot/restore the FF-active flag around draws and implement glPushClientAttrib/glPopClientAttrib. 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 need to look into this more carefully to understand what's being claimed.
Fable
⚙ Read {"file_path": "/home/magik6k/lcad-wasm/freecad-port/FreeCAD/src/Gui/WasmGLFixedFunc.cpp", "offset": 360, "limit": 140}
tool result
360	void glEnableClientState(GLenum a){ ensure(); ffClientState(a,1); }
361	void glDisableClientState(GLenum a){ ensure(); ffClientState(a,0); }
362	void glVertexPointer(GLint s, GLenum t, GLsizei st, const void* p){ ensure(); ffPointer(0,s,t,st,(GLintptr)p); }
363	void glNormalPointer(GLenum t, GLsizei st, const void* p){ ensure(); ffPointer(1,3,t,st,(GLintptr)p); }
364	void glColorPointer(GLint s, GLenum t, GLsizei st, const void* p){ ensure(); ffPointer(2,s,t,st,(GLintptr)p); }
365	void glTex  …[truncated]
Fable
thinking
I'm examining the ffClientState and how the arrays.vertex.on toggle works.
Fable
⚙ Bash {"command": "grep -n \"arrays.vertex\\|ffClientState\\|GL_VERTEX_ARRAY\\|vertex:{on\\|vertex.on\\|__ff\" src/Gui/WasmGLFixedFunc.cpp | head -50"}
tool result
33:/* One-time JS state + helpers. Attached to globalThis.__ff. */
35:  if (globalThis.__ff) return;
55:  globalThis.__ff = {
65:      vertex:{on:false,size:3,type:0x1406,stride:0,ptr:0,vbo:0},
118:  const F = globalThis.__ff; const g = F.gl(); if (!g) return;
169:  const vertClient = F.arrays.vertex.on && !F.arrays.vertex.glbuf;
179:      F._maxIndex = F.arrays.vertex._count ? F.arrays.vertex._count-1 : count-1;
185:  bind(F.arrays.vertex, 0, 3);
231:EM_JS(void, ffMatrixMode, (GLenum m), { glob  …[truncated]
Fable
thinking
The code behavior checks out—ffFixedFuncActive only keys on arrays.vertex.on, and those attribute push/pop functions are indeed no-ops. But this is a low-severity fragility issue since the reviewer themselves acknowledges it's largely irrelevant given how Coin's main features sidestep this code path anyway. The actual failure scenario requires Coin to leave GL state enabled in an abnormal way, which the reviewer acknowledges isn't typical behavior. This is more of a defensive robustness observation than a confirmed bug, so low severity fits—the evidence supports it as a legitimate fragility point even if it's unlikely to occur in practice.
Fable
The code evidence matches the claim exactly. Let me confirm the ffClientState toggle and the no-op attrib functions.
Fable
⚙ Read {"file_path": "/home/magik6k/lcad-wasm/freecad-port/FreeCAD/src/Gui/WasmGLFixedFunc.cpp", "offset": 346, "limit": 6}
tool result
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), {
Fable
thinking
I'm verifying the evidence across the codebase: the fixed function pipeline activation depends on the vertex array state, the draw calls are gated by this activation check, and the client state toggles directly modify the vertex array without any attribute preservation. The stack operations for attribute and client state appear to be incomplete in the trace.

The finding holds up—it's a legitimate robustness concern, though acknowledged as having limited practical impact. Low severity is the right call.
Fable
All evidence in the finding is factually accurate and verified directly against the file: - Lines 388-390: `ffFixedFuncActive` returns true solely based on `arrays.vertex.on`. - Lines 397-406: `glDrawArrays`/`glDrawElements` route to `ff_setup_and_draw` whenever `ffFixedFuncActive()` is true, else pass through. - Lines 346-350: `ffClientState` toggles `arrays.vertex.on` on enable/disable with no scoping or save/restore. - Lines 471-474: `glPushAttrib`, `glPopAttrib`, `glPushClientAttrib`, `glPopClientAttrib` are all empty no-ops, so any node relying on attrib-stack restore of client-array enables would leak the enabled state. The described mechanism is genuinely present: gating keys only on the sticky `arrays.vertex.on` flag with no push/pop restore, so a stale-enabled `GL_VERTEX_ARRAY` would hijack a later `glDrawArrays` routed through this symbol into the FF shader path. This is a real robustness/fragility issue rather than an observed active bug — the finding itself honestly notes it is "largely moot" because Coin disables the array after each draw, Coin's main draws bypass this symbol (per the referenced critical finding), and Qt resolves GL through its own proc-address pointers. Severity of low is appropriate for a defensive/fragility observation with no demonstrated concrete failure path in current usage.
Fable
⚙ StructuredOutput {"verdict": "CONFIRMED", "reason": "All cited code is accurate: ffFixedFuncActive (lines 388-390) gates draws solely on arrays.vertex.on, ffClientState (346-350) toggles that flag with no scoping/save-restore, and glPushAttrib/glPopAttrib/g…
tool result
Structured output provided successfully