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: high CATEGORY: gl-function-resolution FILE: freecad-port/
▸ Magik
You are an adversarial verifier for a FreeCAD-wasm code review. A reviewer claims: SEVERITY: high CATEGORY: gl-function-resolution FILE: freecad-port/FreeCAD/src/3rdParty/coin/src/glue/gl.cpp SUMMARY: After JSPI, invert and complete the getprocaddress path so Coin resolves EVERY modern GL entry point through our signature-matched `fcWasmResolveGL` C symbols instead of emscripten_GetProcAddress — this removes Blocker B (indirect-call signature traps) deterministically. DETAIL/EVIDENCE: src/3rdParty/coin/src/glue/gl.cpp:609-651 `cc_glglue_getprocaddress`: the `#ifdef __EMSCRIPTEN__` branch currently tries `emscripten_GetProcAddress(symname)` FIRST (line 620) and only falls back to `fcWasmResolveGL` (line 627) when that returns null. That is backwards per R9 §3.1: emscripten's returned pointers are the ones called *indirectly* by Coin, where wasm enforces exact type signatures and mismatches trap ('function signature mismatch' / 'table index out of bounds'). Coin resolves ~157 modern entry points this way — 138 via `PROC(w, ...)` (macro at gl.cpp:881) plus 19 direct `getprocaddress(w, "gl...EXT")` (glGenFramebuffersEXT, glBindRenderbufferEXT, glGenerateMipmap, glRenderbufferStorageEXT, ...). Fix: (1) on wasm, resolve through `fcWasmResolveGL` FIRST and only use emscripten as a last resort for functions we don't wrap; (2) extend the `fcWasmResolveGL` table in src/Gui/WasmGLFixedFunc.cpp:508-525 (currently only ~15 legacy symbols) to cover the whole PROC() set — buffers (glGenBuffers/BindBuffer/BufferData/DeleteBuffers/MapBuffer*), textures (glGenTextures/BindTexture/TexImage2D/TexParameteri/glActiveTexture), FBO/renderbuffer family, glDrawRangeElements, blend/stencil/glPolygonOffset — each as a thin `EM_JS`->`GLctx.<call>` wrapper (real WebGL2) or an emulation entry, with a signature exactly matching Coin's `PFNGL...PROC` typedef. Coin then only ever calls our statically-linked C symbols, eliminating the indirect-call type check entirely (R9 §3.1, Phase C1). RECOMMENDATION: Reorder the __EMSCRIPTEN__ branch to prefer fcWasmResolveGL; enumerate all PROC()/getprocaddress symbols (grep in gl.cpp) and add a signature-matched wrapper for each in WasmGLFixedFunc.cpp. Keep emscripten_GetProcAddress only as a fallback for symbols with no wrapper. 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 actual files to verify what's being claimed.
Fable
⚙ Read {"file_path": "/home/magik6k/lcad-wasm/freecad-port/FreeCAD/src/3rdParty/coin/src/glue/gl.cpp", "offset": 590, "limit": 80}
tool result
590	int
591	coin_glglue_stencil_bits_hack(void)
592	{
593	  const char * env = coin_getenv("COIN_OFFSCREEN_STENCIL_BITS");
594	  if (!env) { return -1; }
595	  return atoi(env);
596	}
597	
598	cc_libhandle
599	coin_glglue_dl_handle(const cc_glglue * glue)
600	{
601	  if (!glue->dl_handle) {
602	    const_cast <cc_glglue *> (glue)->dl_handle = cc_dl_handle_with_gl_symbols();
603	  }
604	  return glue->dl_handle;
605	}
606	
607	/* doc in header file */
608	void *
609	cc_glglue_getprocaddress(const  …[truncated]
Fable
⚙ Bash {"command": "cd /home/magik6k/lcad-wasm/freecad-port/FreeCAD && git log --oneline -5 2>/dev/null; echo \"---\"; grep -n \"fcWasmResolveGL\\|emscripten_GetProcAddress\\|__EMSCRIPTEN__\" src/3rdParty/coin/src/glue/gl.cpp | head -40", "descrip…
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
123e7c2888 wasm: FreeCAD GUI fully boots in the browser (Stage 3 gate PASSED)
9e3f90a6e2 wasm: asyncify Qt — GUI renders in browser, modal dialogs work
---
613:#ifdef __EMSCRIPTEN__
619:    extern void * emscripten_GetProcAddress(const char *);
620:    ptr = e  …[truncated]
Fable
thinking
The reviewer's evidence checks out—emscripten_GetProcAddress is called first, followed by fcWasmResolveGL. Now I need to verify the PROC macro claims, the direct getprocaddress calls, and what's in the WasmGLFixedFunc.cpp table.
Fable
⚙ Read {"file_path": "/home/magik6k/lcad-wasm/freecad-port/FreeCAD/src/Gui/WasmGLFixedFunc.cpp", "offset": 490, "limit": 60}
tool result
490	void glLoadName(GLuint){ }
491	/* display lists: unsupported (Coin has a VBO fallback) */
492	GLuint glGenLists(GLsizei){ return 0; }
493	void glNewList(GLuint, GLenum){ }
494	void glEndList(void){ }
495	void glCallList(GLuint){ }
496	void glCallLists(GLsizei, GLenum, const void*){ }
497	void glDeleteLists(GLuint, GLsizei){ }
498	void glListBase(GLuint){ }
499	
500	/* Resolver so Coin's cc_glglue_getprocaddress can find the legacy GL entry
501	 * points we emulate (glActiveTexture is real We  …[truncated]
Fable
⚙ Bash {"command": "cd /home/magik6k/lcad-wasm/freecad-port/FreeCAD && echo \"=== PROC( count ===\"; grep -c \"PROC(w,\" src/3rdParty/coin/src/glue/gl.cpp; echo \"=== PROC macro def ===\"; grep -n \"define PROC\" src/3rdParty/coin/src/glue/gl.cpp;…
tool result
=== PROC( count ===
136
=== PROC macro def ===
881:#define PROC(_glue_, _func_) cc_glglue_getprocaddress(_glue_, SO__QUOTE(_func_))
924:#define PROC(_glue_, _func_) (&_func_)
=== direct getprocaddress EXT count ===
19
=== sample direct ===
1969:      cc_glglue_getprocaddress(w, "glGenerateMipmap");
1972:        cc_glglue_getprocaddress(w, "glGenerateMipmapARB");
1978:        cc_glglue_getprocaddress(w, "glGenerateMipmapEXT");
1983:    w->glIsRenderbuffer = (COIN_PFNGLISRENDERBUFFERPROC) cc_glglu  …[truncated]
Fable
thinking
I'm verifying the key findings against the evidence: the backwards ordering of the fallback mechanism is confirmed, the macro count matches closely at 136 versus the expected 138, and I've verified exactly 19 direct getprocaddress calls.

Now I need to examine the core technical claim itself. The reviewer argues that emscripten_GetProcAddress returns pointers that cause type signature mismatches when called indirectly through the wasm table, triggering traps. But this premise seems flawed — emscripten's GL function wrappers are generated to match standard GL prototypes, so the signatures should align correctly. Even more fundamentally, the claim that using statically-linked C symbols through  …
Fable
⚙ Bash {"command": "echo \"=== find R9 / Blocker B docs ===\"; grep -rniEl \"blocker b|signature mismatch|indirect.?call|R9\" --include=*.md --include=*.txt . 2>/dev/null | head; echo \"=== search fcWasmResolveGL usages ===\"; grep -rn \"fcWasmRes…
tool result
=== find R9 / Blocker B docs ===
LibreCAD/externals/Catch2/docs/test-cases-and-sections.md
LibreCAD/libraries/libdxfrw/DWG_ROADMAP.md
freecad-port/STATUS.md
freecad-port/FreeCAD/src/3rdParty/coin/docs/BUGS.txt
freecad-port/research/R8-jspi-migration-plan.md
freecad-port/research/R9-coin-webgl-strategy.md
freecad-port/research/R10-eh-jspi-asyncify-findings.md
=== search fcWasmResolveGL usages ===
freecad-port/FreeCAD/src/3rdParty/coin/src/glue/gl.cpp:626:      extern void * fcWasmResolveGL(const   …[truncated]
Fable
thinking
I need to check what section 3.1 of the R9 document actually specifies to see if the reviewer's interpretation aligns with the original design.
Fable
⚙ Bash {"command": "cd /home/magik6k/lcad-wasm/freecad-port/research; echo \"=== R9 section 3.1 ===\"; grep -n \"3.1\\|signature\\|indirect\\|Blocker\\|getprocaddress\\|fcWasmResolveGL\\|emscripten_GetProcAddress\" R9-coin-webgl-strategy.md | head…
tool result
=== R9 section 3.1 ===
15:via a `getprocaddress`** and calls them through function pointers.
20:- **Function resolution**: on wasm, `getprocaddress` had no backend (WGL/EGL/
21:  GLX/dlsym all null), and even wired to `emscripten_GetProcAddress` the
22:  returned pointers are called *indirectly*, where wasm enforces exact type
23:  signatures — mismatches trap ("function signature mismatch" / "table index
45:### 3.1 The key architectural refinement (fixes "Blocker B")
48:(signature/table-index f  …[truncated]
Fable
thinking
I need to review the full section of R9 3.1 to see if this concern is already addressed in the documentation.
Fable
⚙ Read {"file_path": "/home/magik6k/lcad-wasm/freecad-port/research/R9-coin-webgl-strategy.md", "offset": 10, "limit": 90}
tool result
10	Coin renders with **OpenGL 1.x fixed-function**: matrix stacks, `glMaterial`/
11	`glLight`, `glBegin/glEnd` immediate mode, `glVertexPointer`+`glDrawElements`
12	client arrays, `glActiveTexture`/`glMultiTexCoord`. WebGL2/GLES3 has *none* of
13	that (shaders mandatory, no matrix stack, no immediate mode, no client-side
14	`glVertexPointer`). Coin also **resolves 1.2+/extension entry points at run time
15	via a `getprocaddress`** and calls them through function pointers.
16	
17	Two independent   …[truncated]
Fable
thinking
The reviewer's finding is essentially restating what's documented in R9 §3.1's Phase C1 plan, and it accurately reflects the design. Looking at the evidence, the code does have the ordering backwards—emscripten comes first, then fcWasmResolveGL—but the documented architecture explicitly says everything should resolve through fcWasmResolveGL, not emscripten, so there's a clear contradiction between the current implementation and the spec.

The concrete details all check out: the PROC macro counts are roughly accurate, the fcWasmResolveGL table does have around 14-15 legacy symbols that need completing, and the specific line numbers the reviewer cited are correct. The underlying technical prem  …
Fable
I've verified the finding against the actual source and the repo's own design docs. Here's what I found. **Evidence checks (all confirmed):** - gl.cpp:609-651 `cc_glglue_getprocaddress`: the `#ifdef __EMSCRIPTEN__` branch does try `emscripten_GetProcAddress(symname)` FIRST (line 620) with `fcWasmResolveGL` only as fallback (line 627). Ordering is exactly as described — backwards relative to the intended architecture. - PROC macro at gl.cpp:881 (`cc_glglue_getprocaddress` form) — confirmed. Count is 136 `PROC(w,...)` sites (reviewer said 138) + exactly 19 direct `getprocaddress(w,"gl...EXT")` calls (FBO/renderbuffer family, glGenerateMipmap, etc.) — reviewer's "~157" is 155; immaterial. - `fcWasmResolveGL` in WasmGLFixedFunc.cpp:508-525 — confirmed, ~14 legacy symbols only (glClientActiveTexture, glMultiTexCoord*, matrix/client-array set). **Alignment with repo intent:** research/R9-coin-webgl-strategy.md §3.1 ("The key architectural refinement (fixes Blocker B)") explicitly states: do NOT resolve to emscripten's implementations; resolve *everything* through `fcWasmResolveGL` with signatures matching Coin's `PFNGL…PROC` typedefs. R9 lines 20-25 document "function signature mismatch"/"table index out of bounds" traps as an *observed* failure class. So the finding faithfully restates the project's own plan, and the current ordering does contradict it. **Where I moderate severity from high to medium:** 1. Not a demonstrated live crash. The current ASYNCIFY build "fully boots" and Stage 4 renders (git log + R9 §3 "proven to work"). The inversion is an interim state, not a regression breaking present rendering. 2. R9 §3.1 explicitly sequences this AFTER JSPI ("Do the JSPI migration first... then do the wrapper work"), and the git log shows JSPI is not yet in ("keep ASYNCIFY link config"). The finding's own summary concedes "After JSPI." So it's tracked, gated future work, not an urgent defect. 3. The recommendation understates effort/risk: for genuine WebGL2 functions (glGenBuffers/glBindBuffer/glTexImage2D/glGenTextures...), emscripten's GL layer performs non-trivial GL-integer-name ↔ WebGLObject handle translation. Routing these to "thin EM_JS→GLctx wrappers" would bypass that handle table and break unless reimplemented — so it is not thin, and blindly wrapping functions emscripten already handles correctly risks regressions. The phrase "eliminating the indirect-call type check entirely" is also imprecise: calling our own C symbols via function pointers still uses `call_indirect` with a signature check; the benefit is that our signatures match by construction, not that the check disappears. The factual evidence is accurate and doc-backed, so the finding stands, but as medium-severity planned/architectural work rather than a live high-severity bug.
Fable
⚙ StructuredOutput {"verdict": "CONFIRMED", "reason": "All concrete evidence verified in source: gl.cpp:620 calls emscripten_GetProcAddress FIRST with fcWasmResolveGL only as fallback at line 627 (ordering is backwards vs. intended architecture); PROC macro a…
tool result
Structured output provided successfully