From 4186ea490f4c6892b22e454f935f6264240f6a6e Mon Sep 17 00:00:00 2001 From: Viktor Vaczi Date: Sat, 13 Jun 2026 17:39:34 +0200 Subject: [PATCH] test(wasm-dom): repros + fixes for the text-ctrl reentry and tooltip UAF bugs Bump wxwidgets (8814ddb) for the two DOM-port fixes and add their reproductions: - tests/apps/standalone/{textctrl-reentry,tooltip-lifetime}: standalone wx repro apps + Makefile.wasm targets (textctrl links -fexceptions to throw from a wxEVT_TEXT handler), driven by tests/e2e/dom-port-bugs.spec.ts. Each app is deterministic and self-contained (no UB, ASAN, or timing dependence). - docs/features/wx-dom-port/branch-review.md: branch review with findings #2/#3 marked fixed and a "Bug reproductions and fixes" section, including the asyncify + legacy-EH gotcha (catch/destructor landing pads are unreliable while unwinding through asyncify frames). Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/features/wx-dom-port/branch-review.md | 235 ++++++++++++++++++ tests/apps/Makefile.wasm | 25 +- .../textctrl-reentry_test.cpp | 104 ++++++++ .../tooltip-lifetime_test.cpp | 105 ++++++++ tests/e2e/dom-port-bugs.spec.ts | 73 ++++++ wxwidgets | 2 +- 6 files changed, 542 insertions(+), 2 deletions(-) create mode 100644 docs/features/wx-dom-port/branch-review.md create mode 100644 tests/apps/standalone/textctrl-reentry/textctrl-reentry_test.cpp create mode 100644 tests/apps/standalone/tooltip-lifetime/tooltip-lifetime_test.cpp create mode 100644 tests/e2e/dom-port-bugs.spec.ts diff --git a/docs/features/wx-dom-port/branch-review.md b/docs/features/wx-dom-port/branch-review.md new file mode 100644 index 0000000..984daa9 --- /dev/null +++ b/docs/features/wx-dom-port/branch-review.md @@ -0,0 +1,235 @@ +# `feature/wx-dom-port` — branch review (every change vs main) + +**Date:** 2026-06-13 +**Scope:** the complete diff of `feature/wx-dom-port` against each repo's mainline +(root `main`, kicad/wxwidgets `wasm-port`, pcbjam-shared `main`). +**Method:** the wx core touches, the async core, the JS layer, the build system, +and all root scripts/specs/docs were read line-by-line; the ~30 additive +`src/wasm/*` control files were read in full by four parallel readers and +cross-checked (especially for the menubar use-after-free class). Two baseline +screenshots were opened to confirm the renders are real. + +## Verdict + +Clean, careful, well-commented work that **reduces** the wxWidgets fork's +divergence from upstream rather than growing it, and leaves KiCad untouched. +The DOM-event architecture is sound (fresh-entry dispatch, freed windows +unregistered before destruction). There are a few real loose ends — one of +which (CI) is a functional breakage — plus two known feature gaps and two +latent lifetime/async hazards worth tracking. None of them block promoting the +branch. + +## Fast-forward status + +The branch is zero-divergence against every mainline (`behind: 0` everywhere), +so each promotion is a true fast-forward — no merge commit, no force. + +| Repo | Target → feature | FF? | Commits | Notes | +|---|---|---|---|---| +| root | `main` → `feature/wx-dom-port` | yes | 23 | local `main` == `origin/main` (`14ca16c`) | +| wxwidgets | `wasm-port` → feature | yes | 20 | `origin/wasm-port` is an ancestor; HEAD `d33769ab56` | +| kicad | `wasm-port` → feature | no-op | 0 | feature HEAD `693c513293` **==** `origin/wasm-port` | +| pcbjam-shared | `main` → … | no-op | 0 | detached HEAD `92b2634` **==** `origin/main` | + +Two mechanics to remember: + +1. The feature branch is **unpushed** (no `origin/feature/wx-dom-port`), and the + submodules have **no local `wasm-port` branch** — only `origin/wasm-port`. So + "fast-forward `wasm-port`" is a fast-forward *push*, not a local merge: + ``` + git -C wxwidgets push origin feature/wx-dom-port:wasm-port # 20 commits + # kicad: nothing to push (already == origin/wasm-port) + git push origin feature/wx-dom-port:main # root, 23 commits + ``` + Push **wxwidgets before root** so the published `main` references an + already-published submodule commit. +2. pcbjam-shared's *local* `main` branch is stale (`5f27571`) vs `origin/main` + (`92b2634`); irrelevant to the recorded pointer, just don't FF that local + branch without fetching. + +## What changed, by repo + +| Repo | Net | Nature | +|---|---|---| +| **kicad** | 0 commits | Fork untouched — divergence not increased. | +| **pcbjam-shared** | 0 | Unchanged. | +| **wxwidgets** | +9,632 / −54,725, 161 files | The substance (below). The −54.7k is dominated by a junk `configure~` backup (−50,075) and the deleted canvas theme `src/univ/themes/wasm.cpp` (−2,960). | +| **root** | 36 text files + 339 baseline PNGs | Build-script renames, DOM test wiring, docs. | + +## wxwidgets — full audit + +### 1. Cleanup / divergence reduction +- Deleted `configure~` (a 50,075-line committed editor backup) and + `src/univ/themes/wasm.cpp` (the 2,960-line canvas renderer the DOM port + replaces). +- The `src/generic/*`, `src/aui/*`, `src/common/prntbase.cpp`, + `src/propgrid/propgrid.cpp`, `src/stc/stc.cpp` touches are **all + `__EMSCRIPTEN__`-gated** and are *removals/refactors* of the old canvas-port + element-tracking hooks into a clean `wx/wasm/elementtracker.h` API + (`WasmRegisterRenderedElement(...)` → `wxWasmTrackElement(...)`, and + screen-coords → parent-relative for DOM positioning). Native builds are + byte-identical; the WASM-side divergence shrinks. + Representative: `framemanager.cpp`, `grid.cpp`, `tabg.cpp` (pure removal), + `ctrlrend.cpp`. `prntbase.cpp` adds only `|| defined(__WXWASM__)` to one + preprocessor condition (PostScript DC — no native printer in WASM). + +### 2. Additive DOM port (~30 new `src/wasm/*.cpp` + `include/wx/wasm/*.h`) +Each wx control maps onto a real DOM element. Read in full and found clean +apart from the findings table below. Intentional stubs are clearly TODO-tagged. + +- **Buttons / static:** `control`, `anybutton`, `button`, `bmpbuttn`, `tglbtn`, + `checkbox`, `radiobut`, `stattext`, `statline`, `statbox`, `statbmp`, `gauge`. + `bmpbuttn` intentionally pushes its bitmap twice (documented: `wxButton::Create`'s + `SetLabel` clears the ``). +- **Text:** `textentry` (mixin) + `textctrl` (``/`