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: robustness FILE: /home/magik6k/lcad-wasm/fr
▸ Magik
You are an adversarial verifier for a FreeCAD-wasm code review. A reviewer claims: SEVERITY: low CATEGORY: robustness FILE: /home/magik6k/lcad-wasm/freecad-port/FreeCAD/src/3rdParty/coin/src/glue/gl.cpp SUMMARY: Null-guard added only to cc_glglue_glActiveTexture; sibling multitexture wrappers still assert on a null pointer. DETAIL/EVIDENCE: gl.cpp:2992-2999 guards cc_glglue_glActiveTexture against a null w->glActiveTexture (probed-before-context case), but cc_glglue_glClientActiveTexture (gl.cpp:3002+) and the cc_glglue_glMultiTexCoord* wrappers retain the bare assert()+call and would fault (or, in NDEBUG, call a null pointer) in the same probed-before-context scenario. In the normal flow all these resolve non-null (emscripten for glActiveTexture, fcWasmResolveGL for the rest), so it is not hit in practice, but the guard is asymmetric. RECOMMENDATION: Either add the same __EMSCRIPTEN__ null-tolerance to the sibling wrappers, or rely on cc_glglue_has_multitexture() gating and drop the special case for consistency. 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 relevant file sections to verify this claim.
Fable
⚙ Read {"file_path": "/home/magik6k/lcad-wasm/freecad-port/FreeCAD/src/3rdParty/coin/src/glue/gl.cpp", "offset": 2975, "limit": 90}
tool result
2975	{
2976	  assert(w->glCopyTexSubImage3D);
2977	  w->glCopyTexSubImage3D(target,
2978	                         level,
2979	                         xoffset,
2980	                         yoffset,
2981	                         zoffset,
2982	                         x,
2983	                         y,
2984	                         width,
2985	                         height);
2986	}
2987	
2988	void
2989	cc_glglue_glActiveTexture(const cc_glglue * w,
2990	                          GLenum texture  …[truncated]
Fable
thinking
Looking at the actual guard conditions across these wrapper functions, I see that cc_glglue_glActiveTexture has its guard, but cc_glglue_glClientActiveTexture uses a different guard that only returns early for GL_TEXTURE0, meaning it still hits the assert for other textures. The MultiTexCoord wrappers don't have any guard at all—they just assert directly. So the claim about cc_glglue_glClientActiveTexture retaining a bare assert needs clarification since it actually has a conditional check, even if it's not the same guard pattern.

The core observation holds though: the guards are asymmetric across these sibling functions, and in normal execution this doesn't matter since everything resolves  …
Fable
⚙ Bash {"command": "cd /home/magik6k/lcad-wasm/freecad-port/FreeCAD && git log --oneline -3 -L 2988,3000:src/3rdParty/coin/src/glue/gl.cpp 2>/dev/null | head -40", "description": "git blame for the guard region"}
tool result
(Bash completed with no output)
Fable
⚙ Bash {"command": "cd /home/magik6k/lcad-wasm/freecad-port/FreeCAD && git diff HEAD -- src/3rdParty/coin/src/glue/gl.cpp 2>/dev/null | grep -n \"EMSCRIPTEN\\|glActiveTexture\\|glClientActiveTexture\\|MultiTexCoord\" | head -30; echo \"---status--…
tool result
---status---
Fable
thinking
I need to check if there's an actual git repository and understand the current state of the file, then look at the git history to see what changes were made.
Fable
⚙ Bash {"command": "cd /home/magik6k/lcad-wasm/freecad-port/FreeCAD && git log --oneline -5 -- src/3rdParty/coin/src/glue/gl.cpp 2>/dev/null; echo \"---\"; git log --oneline -3 2>/dev/null", "description": "git log for gl.cpp"}
tool result
---
439ed31cb5 wasm: keep ASYNCIFY link config (JSPI documented as forward path)
6ed7f660f2 wasm: update vendored Coin (WebGL2 glue port) + Quarter context sync
f87496364c wasm: Stage 4 WebGL viewport — fixed-function emulator + Coin glue port
Fable
thinking
The file shows the guard for glActiveTexture is confirmed at lines 2992-2999, and the bare asserts in the MultiTexCoord wrappers are at lines 3017, 3026, 3035, and 3044. I'm also noting that glClientActiveTexture has a partial guard in place at lines 3006-3008.

The claim's severity assessment of "low" is accurate—this is a consistency issue that doesn't manifest in normal usage, and the asymmetry between the guarded glActiveTexture and the unguarded MultiTexCoord wrappers is a real but minor robustness concern.
Fable
Evidence confirmed by reading the file: - Lines 2992-2999: `cc_glglue_glActiveTexture` has the `#ifdef __EMSCRIPTEN__` null-guard (`if (!w->glActiveTexture) { return; }`) before the `assert`. - Lines 3017, 3026, 3035, 3044: the `cc_glglue_glMultiTexCoord2f/2fv/3fv/4fv` wrappers all have bare `assert(...)` + call with no null-tolerance. - Lines 3006-3008: `cc_glglue_glClientActiveTexture` has a partial guard (`if (!w->glClientActiveTexture && texture == GL_TEXTURE0) return;`) then asserts — so for non-TEXTURE0 targets it still asserts on null. The claim's characterization of it as a "bare assert()+call" is slightly inaccurate (it does have an upstream Coin guard for the common TEXTURE0 case), but the underlying point (asymmetric, still faults for the non-TEXTURE0 null case) holds. The asymmetry is real: only `glActiveTexture` got the wasm null-tolerance. The claim honestly acknowledges the normal flow resolves all these non-null so it is not hit in practice, making this a consistency/robustness nit. Low severity is accurate. The one minor imprecision (glClientActiveTexture is not strictly a bare assert) doesn't undermine the finding, since the MultiTexCoord* wrappers are genuinely bare and the overall asymmetry claim is verified.
Fable
⚙ StructuredOutput {"verdict": "CONFIRMED", "reason": "Confirmed by reading gl.cpp: cc_glglue_glActiveTexture (2992-2999) has the __EMSCRIPTEN__ null-guard, while cc_glglue_glMultiTexCoord2f/2fv/3fv/4fv (3017, 3026, 3035, 3044) retain bare assert()+call and w…
tool result
Structured output provided successfully