Expand win32k GDI/user syscall emulation

[AdvDebug]

Adds broad win32k coverage with many new NtGdi/NtUser/Nt* syscall handlers, including bitmap blits, DIB transfer, region ops, timer APIs, deferred window positioning, cursor/icon metadata, window placement/state APIs, and additional process/misc NT calls. It also refactors message-queue behavior to be thread-aware, synthesizes WM_PAINT/WM_TIMER correctly, and tracks paint/update state more accurately.

Rendering and text support were extended across hosts: a new Blit GDI primitive is wired through Win32/Linux/Android backends, font family enumeration is exposed through the window manager stack, and Android adds a new SwitcherView control. Supporting updates include safer Unicorn hook callback failure handling, drive-map reporting for ProcessDeviceMap, command-line/process launch fixes, and Android build script updates for .NET 10.

[AdvDebug]

@claude review

[claude]

Claude finished @AdvDebug's task in 1m 7s —— View job

---

Review complete

Todo list

• Gather context (repo guidelines, diff overview)
• Review Win32k core changes (Win32kHelper.cs, message queue/paint/timer logic)
• Review new NtGdi syscall handlers
• Review new NtUser syscall handlers
• Review WindowManager backend changes (Windows/Linux/Android) + misc/process/Unicorn changes
• Post final review feedback

This is a large PR (86 files, +4288/-124), so I reviewed it in four parallel passes: win32k core state/message-queue logic, new NtGdi* handlers, new NtUser* handlers, and the window-manager backends/Unicorn/misc syscalls. Overall the handle-validation discipline in the new NtUser* handlers is solid (proper dictionary-based lookups, correct guest-memory size checks), but I found two real bugs I verified directly against the code, plus several lower-severity issues worth addressing.

High-severity findings (verified)

1. Region-handle type confusion → cross-object memory corruption
Brovan/Core/Emulation/OS/Windows/Win32k/Win32kHelper.cs:1281,1298 (TryReadRegionRect/TryWriteRegionRect) resolve a handle via GetGdiKernelObject (WinSyscallsHelper.cs:4165), which only calls ValidateGdiHandle — that checks the handle's index/uniqueness tag but never checks the handle's type bits. Contrast with GetDcAttributeAddress (WinSyscallsHelper.cs:4184), which does check ((Hdc >> 16) & GdiHandleTypeMask) != 1, showing the codebase already knows this check is necessary elsewhere.

Concrete failure: a guest opens a DC via NtGdiOpenDCW and then passes that DC handle to NtGdiOffsetRgn.cs or NtGdiCombineRgn.cs where a region handle is expected. GetGdiKernelObject decrypts the DC's attribute-block pointer and TryReadRegionRect/TryWriteRegionRect blindly reinterpret it as a 0x30-byte region-rect struct, writing 20 guest-controlled bytes into the live DC attribute block instead of returning ERROR_INVALID_HANDLE. This is a confused-deputy write: any valid GDI handle of the wrong type can be used to corrupt an unrelated live object's backing state.
Fix: validate the handle's type bits equal the region type before dereferencing in GetGdiKernelObject (or add a type check to TryReadRegionRect/TryWriteRegionRect directly), matching the pattern in GetDcAttributeAddress.

2. Deleting the shared display-surface bitmap leaves it permanently dangling
Brovan/Core/Emulation/OS/Windows/Win32k/Win32kHelper.cs:504 now marks every window DC as Display = Display || Hwnd != 0, so GetDcSelectedBitmap/EnsureDisplaySurfaceBitmap (lines 1442-1456) route window DCs to a single shared, lazily-created State.DisplaySurfaceBitmap. IsStockObject (line 1001) only special-cases State.StockBitmap — it has no equivalent protection for DisplaySurfaceBitmap.

Concrete failure: guest calls NtGdiGetDCObject/GetCurrentObject(hdc, OBJ_BITMAP) on any window DC to obtain this handle, then calls NtGdiDeleteObjectApp on it. NtGdiDeleteObjectApp.cs:11 only skips deletion for stock objects, so it proceeds to RemoveBitmap, which deletes the bitmap and unmaps its backing guest-memory region — but State.DisplaySurfaceBitmap is never reset to 0. Every subsequent window-DC bitmap query returns/reuses this now-invalid handle pointing at unmapped memory, breaking window-DC bitmap operations for the rest of the process.
Fix: shield DisplaySurfaceBitmap from deletion the same way StockBitmap is shielded (extend IsStockObject, or otherwise refuse DeleteObject on it).

Medium/low-severity findings

3. Unbounded guest-controlled blit dimensions (DoS) — NtGdiBitBlt.cs:21-29, NtGdiSetDIBitsToDeviceInternal.cs, NtGdiStretchDIBitsInternal.cs, NtUserUpdateLayeredWindow.cs:58-59 only check Width/Height <= 0, with no upper bound. Large values (e.g. Width=90000,Height=90000) overflow the int Width*Height multiplication or drive multi-GB ArrayPool<uint>.Shared.Rent() allocations, which can throw unhandled or exhaust host memory. Worth clamping to a sane maximum (e.g. bitmap/screen bounds) before allocating.

4. Unicorn hook exceptions now silently stop emulation and report success — Brovan/Core/Emulation/Backends/Unicorn/UnicornBackend.cs hook thunks (~lines 342-495) catch the broadest Exception type and call uc_emu_stop, only logging via Utils.LogError. Since uc_emu_stop makes uc_emu_start return success, callers of Emulate() can no longer distinguish "ran to completion" from "crashed mid-hook with unknown partial state," which risks the emulator continuing on top of corrupted state. Consider surfacing the failure back to the caller instead of only logging.

5. PostThreadMessage not covered by thread-aware filtering — Win32kHelper.cs OwnedByThread/MatchesFilter (~lines 3702-3726) treats any message with Hwnd == 0 as matching every thread's filter, but NtUserPostThreadMessage queues thread messages with Hwnd = 0 and no other thread-id carried on Win32kMessage. A different GUI thread calling GetMessage/PeekMessage first can steal a message intended for another thread — undermining the PR's stated thread-aware queue goal for this message class.

6. KillTimer doesn't check timer ownership — unlike SetTimer/FindDueTimer, Win32kHelper.cs:~1932 KillTimer doesn't verify Timer.ThreadId against the caller, so any thread can kill another thread's timer.

7. Minor leak in NtUserCreateEmptyCursorObject.cs — the allocated handle is never inserted into Win32kState.CursorIcons, so DestroyCursor called before SetCursorIconDataEx populates it fails the removal check and leaks the handle.

8. NtGdiDoPalette.cs:11-16 never reads/validates the palette handle argument — any value (including 0) is accepted, functionally incorrect though not a memory-safety issue.

Not an issue

SaveDC/RestoreDC level math, DeferWindowPos bookkeeping, region-combine math, cursor/icon struct marshalling, process-launch argument quoting, and the Win32/Linux/Android Blit bounds-checking were all reviewed and look correct.

I did not run a build/test suite here — this environment doesn't have a verified build step configured; let me know if you'd like me to attempt one.

[AdvDebug]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 76634ccda5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Route display-DC blits through the display surface

When a guest draws to an HDC returned by NtGdiOpenDCW/CreateDC("DISPLAY"), this flag only changes GetDcSelectedBitmap. BlitBlockToDc instead probes TryGetDcBitmap, which still sees the 1-bit stock bitmap; it cannot blit there and, since this DC has Hwnd == 0, returns false. Consequently BitBlt, SetDIBitsToDevice, and StretchDIBits targeting a display DC fail rather than draw to the advertised display surface.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Restrict timer deadline wakeups to eligible message waits

This assigns every newly created timer's deadline to every waiting GetMessage/WaitMessage thread without checking the timer's owner, HWND filter, or message range. For example, a thread waiting with GetMessage(hwndA, ...) receives the deadline for a timer on hwndB; when it expires, that timer cannot be dequeued by the filter, so the wait remains armed with an expired deadline and is repeatedly checked by the scheduler. Reuse the ownership/filter logic from GetNextTimerDue before updating each waiting thread.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Report empty mapped drives as available

An explicitly mapped drive is usable even when its host root is empty: SetDriveMapping creates and registers that directory, and path resolution uses it directly. This code sets the drive-map bit only after finding an entry, so an empty mapped D: is omitted from ProcessDeviceMap even though guest file operations can use it. Mark an existing mapped root as present rather than requiring it to contain a file.

Useful? React with 👍 / 👎.

[AdvDebug]

@claude review

[claude]

Claude finished @AdvDebug's task in 2m 43s —— View job

---

Review complete

Todo list

• Gather context (repo guidelines, diff overview)
• Review Win32k core changes (Win32kHelper.cs, message queue/paint/timer logic)
• Review new NtGdi syscall handlers
• Review new NtUser syscall handlers
• Review WindowManager backend changes (Windows/Linux/Android) + misc/process/Unicorn changes
• Post final review feedback

This PR has already been through two review rounds (my earlier pass and Codex's), and commits 76634cc ("Fix claude pointings") and 1e9d1a4 ("Fix DC/timer wakeups and drive detection") addressed essentially everything both of us flagged. I verified each fix against the current diff rather than re-deriving them from scratch:

Confirmed fixed

• Region-handle type confusion — GetGdiKernelObject (WinSyscallsHelper.cs:4165) now takes an ExpectedType and checks the handle's type bits before dereferencing; TryReadRegionRect/TryWriteRegionRect pass RegionHandleType. A DC handle can no longer be reinterpreted as a region object.
• Dangling shared display-surface bitmap — IsStockObject (Win32kHelper.cs:1010) now also protects State.DisplaySurfaceBitmap, so NtGdiDeleteObjectApp can't delete it out from under every window DC.
• Unbounded blit dimensions (DoS) — IsBlitExtentValid (Win32kHelper.cs:1003, capped at MaxBitmapBytes/4 pixels) is now used by NtGdiBitBlt, NtGdiSetDIBitsToDeviceInternal, NtGdiStretchDIBitsInternal, and NtUserUpdateLayeredWindow.
• Unicorn hook exceptions silently reporting success — UnicornBackend.cs now tracks [ThreadStatic] HookFailure and makes Emulate() return false when a hook threw, instead of relying solely on uc_emu_stop's "success" return. BinaryEmulator.StartEmulation (BinaryEmulator.cs:3803) already treats a false result as a real failure, so this now propagates correctly.
• KillTimer missing ownership check — now routed through the new shared TimerOwnedByThread helper, matching SetTimer/GetNextTimerDue.
• Timer deadlines handed to unrelated waiters — WakeMessageWaiters (Win32kHelper.cs:2013) now recomputes each waiting thread's deadline via GetNextTimerDue with that thread's own filter/ownership instead of blindly propagating the newest timer's Due to every parked thread.
• Display DC blits failing — TryGetDcBitmap (Win32kHelper.cs:1518) now resolves through EnsureDisplaySurfaceBitmap for screen DCs, so BitBlt/SetDIBitsToDevice/StretchDIBits against a DISPLAY DC no longer silently fail.
• Empty mapped drives omitted from ProcessDeviceMap — GetWindowsDriveMap (GeneralHelper.cs) now marks a drive present as soon as it's an explicitly configured mapping, without requiring a file to already exist under it.
• NtUserCreateEmptyCursorObject handle leak — now calls SetCursorIconData with an empty struct at creation time so DestroyCursor can find and remove it even before SetCursorIconDataEx populates it.

New finding (not covered by either previous round)

NtGdiGetDIBitsInternal.cs:52 — stride computed from unclamped guest-controlled Header.Width, unlike the sibling blit handlers

int Width = Math.Min(Header.Width, Bitmap.Width);   // clamped, used for the copy
...
int Stride = ((Header.Width * Header.BitsPerPixel + 31) / 32) * 4;  // uses raw Header.Width

TryReadDibHeader reads Header.Width straight from guest memory with no range validation. Width (used to bound the actual pixel copy) is correctly clamped to Bitmap.Width, but Stride — used to size LineBuffer (ArrayPool<byte>.Shared.Rent(Stride), line 62) and to compute the per-row WriteMemory offset — is derived from the raw Header.Width. A guest can pass a BITMAPINFOHEADER with a large biWidth (e.g. 0x08000000 at 32bpp) so Header.Width * Header.BitsPerPixel overflows int and wraps to a small/garbage Stride. The inner loop then indexes Line[Column * BytesPerPixel + 2] up to (Width-1)*BytesPerPixel (bounded by the real, clamped Width), which can exceed the undersized Stride-length span and throw IndexOutOfRangeException, or Rent(Stride) can throw outright if Stride wraps negative.

Because Spans are bounds-checked this isn't a memory-corruption bug, and the Unicorn hook-failure handling added in this same PR (UnicornBackend.cs FailHook) will catch the exception and cleanly fail the Emulate() call rather than crashing the host process — but it still aborts the entire emulation slice on a malformed header instead of returning ERROR_INVALID_PARAMETER to the guest like the other DIB paths do. Worth validating Header.Width/computing Stride the same defensive way IsBlitExtentValid does elsewhere (e.g. reject before use, or derive Stride from the already-clamped Width plus a sanity bound on Header.Width).

Fix this →

Still open from the earlier rounds (unchanged, lower priority)

• NtUserPostThreadMessage not covered by thread-aware filtering (Win32kHelper.cs:3742 OwnedByThread, NtUserPostThreadMessage.cs:22) — the handler still comments that "a thread message is queued with no window," and OwnedByThread still returns true unconditionally whenever Hwnd == 0. Any GUI thread calling GetMessage/PeekMessage can still consume a message another thread's PostThreadMessage call was meant for. Not touched by the two fix-up commits.
• NtGdiDoPalette.cs:11-16 still never reads/validates the palette handle argument — functionally incorrect (any value including 0 is accepted) but not a memory-safety issue, unchanged from before.

Not an issue

Everything else from the original 87-file diff (region-combine math, SaveDC/RestoreDC level math, DeferWindowPos bookkeeping, cursor/icon struct marshalling, process-launch argument quoting, and the new NtGdiEnumFonts/window-placement/window-manager backend code) still looks correct on this pass — font-family enumeration counts are host-derived, not guest-controlled, so no overflow risk there.
· Branch: win32k-big-coverage

[AdvDebug]

@codex review

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.