pcbjam/docs/features/async/10-resolution-menubar-uaf.md
Viktor Vaczi fa408c0c06 fix(wasm): bump wxwidgets for the wxMenuBar Detach() UAF fix
Submodule pointer bump for the wxMenuBar dangling-child fix that resolved the 18 DOM-port kicad e2e failures. Also adds the async-debug dossier (docs/features/async/08-10) root-causing the 'indirect call to null' / 'signature mismatch' family to the menu-bar use-after-free, and folds the asyncify harness into 'npm test' (new test:wx for wx-only); tests/README + WHATWORKS updated to match.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-13 14:10:05 +02:00

5.7 KiB
Raw Blame History

10 — Resolution: the DOM-port regression was a wxMenuBar use-after-free

Status: root cause found and fixed; confirmed on eeschema (the 3 deterministic eeschema-ui red cases pass with zero corruption signatures). Full-suite validation across all apps + cleanup + commit pending.

TL;DR

The 18 "asyncify-corruption" kicad failures were not an Asyncify bug at all. They were a single use-after-free of a wxMenuBar that the DOM port leaves dangling in a frame's child list:

  • wxMenuBar::Attach(frame) (src/wasm/menu.cpp) lazily calls Create(frame)wxWindowWasm::Createparent->AddChild(this), so in the DOM port the menu bar is a real child window of the frame (it renders as a DOM node and the frame's paint walk reaches it). Native ports do NOT do this — their menu bar is a separate native object.
  • Replacing the menu bar (wxFrame::SetMenuBar(newBar)DetachMenuBar()wxMenuBar::Detach()) ends in wxMenuBarBase::Detach(), which does m_menuBarFrame = NULL; SetParent(NULL); — it clears m_parent but never removes the bar from the frame's GetChildren() list.
  • So the old bar sits in the live frame's child list with m_parent == NULL. When it is later destroyed (KiCad's ReCreateMenuBar() deletes the old bar), both ~wxWindowWasm and ~wxWindowBase gate their RemoveChild() on a non-NULL parent — so neither unlinks it. A freed wxMenuBar is now a dangling pointer in a live wxFrame's child list.
  • Every ~16 ms the parked main loop runs ProcessEvents → wxApp::Paint → DoPaint → PaintChildren, which calls child->IsFrozen()/IsShown()a virtual call on the freed barcall_indirect through a stale/zeroed vtable → RuntimeError: indirect call to null (zeroed) or … signature mismatch (reused). The idle walk (SendIdleEvents) is a second detonation site for the same dangling pointer.

The fix (wasm layer only)

src/wasm/menu.cpp, wxMenuBar::Detach() — unlink the bar from its parent while it is still alive, before delegating to the base:

void wxMenuBar::Detach()
{
    if ( wxWindow* parent = GetParent() )
        parent->RemoveChild(this);   // removes from m_children AND SetParent(NULL)
    wxMenuBarBase::Detach();
}

One-file change. KiCad and wx-core stay untouched. (RemoveChild already does DeleteObject + SetParent(NULL), so the subsequent base SetParent(NULL) is a no-op and ~wxWindowBase won't double-unlink.)

Why this looked like an Asyncify bug for so long

The crash signature (indirect call to null / index out of bounds / signature mismatch) is identical to the genuine Asyncify family in docs 0107, and it appears in the same place (the parked main loop). But:

  • the wasm table is intact (a slot-flip monitor saw zero non-null→null changes) → the null index comes from a dead C++ object's vtable, not table corruption;
  • unwind/rewind bookkeeping, currData, SPs and exportCallStack are all clean, and the fault fires at Asyncify.state == Normal → not a transition race;
  • noExitRuntime = true → the park throw runs no destructors → not de-park.

The Asyncify machinery is only the harness that runs the paint/idle walk; the corruption is plain C++ object lifetime.

Why it's DOM-port-only (main/canvas is green)

Only the DOM port makes wxMenuBar a real child of the frame (for DOM rendering). On the canvas/native side the menu bar is not in GetChildren(), so wxMenuBarBase::Detach()'s SetParent(NULL)-without-RemoveChild leaves nothing dangling. Same wx/kicad commits; the DOM child-ness is the delta.

Relationship to docs 08 / 09

  • 08 correctly excluded the entire Asyncify framing (table intact, clean bookkeeping, Normal-state fault) and localized the crash to a virtual call on a bad wxWindow in the parked-loop tree walk.
  • 09 had the shape right — a freed window still reachable through a parent's GetChildren(), detonated by the idle/paint walk — but proposed the wrong free-path (destruction-time DOM re-entry) and fix (~wxWindowWasm reorder + wx_dom_event IsBeingDeleted guard). That reorder could not help here: the menu bar's parent is already NULL at destruction (cleared by Detach), so any destructor-time RemoveChild is skipped. The real fix is to not create the inconsistency in the first place — unlink on Detach.
  • The 09 reorder + wx_dom_event guard are reverted (they did not fix this bug and add unrelated divergence). They remain reasonable optional defensive hygiene if a future, distinct destruction-time re-entry is ever proven.

How it was pinned (diagnostic path, for the next hard UAF)

  1. SHIM_DIAGNOSTICS=1 table-integrity monitor → ruled out table corruption.
  2. A C++ live-window set (wxWindowBase ctor insert / dtor erase) checked at the tree-walk sites; membership needs no deref, so it flags a freed window in any vtable state. The check had to be placed in PaintChildren before the first virtual call on each child (DoPaint entry was one virtual call too late — the parent's pre-checks crash first).
  3. An occurrence count of the bad child in the parent's list (==1, not a duplicate) + the recovered class (wxMenuBar) named the bug exactly.

All of these were temporary instrumentation and have been removed; the technique is recorded here so it can be re-created for the next hard UAF. Two build aids were used and also removed: a -g (keep-names) pass so wasm stack traces are symbolized, and an override to empty the asyncify removelist. The key signature to remember: with the wasm table intact, an "indirect call to null" / "signature mismatch" in the parked-loop paint/idle walk is a dead C++ object's vtable, i.e. an object-lifetime bug — not Asyncify.