Repository navigation
Conversation
|
Thanks for this, and for the TLOU coordination evidence on #3965. Review at 992ab94: one blocker before this can go further.
Exit and cancel are common: Ask:
Non-blocking notes:
Merge order with #6540, #7135 and #3965 is fine; the text merges cleanly and there's no semantic overlap. |
|
One addition to the exit/cancel ask above, from reviewing #3965 alongside this. With #3965 the main thread also runs on a guest arena stack. If the main thread calls Since this PR owns Planned order: #6540 -> #7135 -> #7149 (with this fix) -> #3965. |
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
992ab94 to
9c1ec7f
Compare
|
Fixed the blocker: Windows scePthreadExit now restores host TEB StackBase/Limit/DeallocationStack before _endthreadex and retires the guest mapping instead of freeing inline. Host bounds live in thread-local GuestThreadState (FLS), not PthreadPrivate, so they survive ReleaseThread(self) deleting self. Retire duplicates the thread HANDLE and unmaps once signalled, mirroring Linux Retire/Reap; cancellation goes through the same path. Adopted-main is covered by one check: if the frame is in a GuestStacks registered range, restore MainHostBounds (main restores only, no retire, process-lifetime; #3965 EnterGuestMainStack should call GuestStackNoteMainHostBounds before switching). Test: GuestPthreadStack on Windows runs 4 scePthreadExit + 4 cancelled (64 KiB) + 1 adopted exit; after join VirtualQuery asserts each old guest low is no longer MEM_COMMIT. Suites (Release Ninja, winlibs gcc15): fiber, guest_memory, guest_pthread_start (30s), guest_pthread_stack, guest_pthread_cancel, guest_once, guest_pthread_destructor_lifetime — 7/7 pass. Start first segfaulted in the trampoline (Detach retired while the worker had not read guestStack); Detach no longer retires. SEH left out: the rsp-switch cannot be expressed, wrong directives would mislead more. Commit-upfront noted in debt. Merge order ack #6540 -> #7135 -> #7149 -> #3965; Depends on stays none (clean rebase onto f1af6c5). |
|
Thanks, that resolves the blocker. scePthreadExit and cancellation now retire the guest stack, restore the host TEB bounds and then call _endthreadex, and the stack is unmapped once the thread handle is signalled. The new exit and cancel checks in GuestPthreadStack cover exactly the path that leaked before. We're running full CI on 9c1ec7f now. The adopted-main check can wait for #3965, since the hook does nothing until #3965 calls GuestStackNoteMainHostBounds. |
|
Windows run of Setup: Windows 11, MinGW-w64 GCC 15.2.0 UCRT, full configure with submodules, Release,
The rest of the non-GPU suite passes (744 of 746; the two failures are the ones above).
Not checked: Linux and macOS (the change is Windows-only), a title, and the combination with #6540, #7135 and #3965 that the plan above mentions. If the two tests only fail without those, that would explain it. Written with AI assistance (Claude Code). I ran the builds and tests above on my machine. |
|
Thanks @Bonjourcestmatt for the Windows run. Full CI on 9c1ec7f shows the same thing: Windows 1. The guard check: the arena commits in 16 KiB pieces.
Please don't just drop the 2. The pages below the reported stack can now be changed by the guest. This is the real regression, and the next asserts in the same test check it. With (1) fixed, I expect the two 3. Not verified here: these are from reading the code (our bench is Linux, so I can't run the Windows path). The arena granule and the missing Windows overlap check are both visible in the current head. When you push, please run |
… margin guard Commit each Windows guest thread stack with granule = whole stack so a 64 KiB guard mprotect spans one VirtualQuery region, record stacks in guestStackMappings and enable GuestThreadStackHostRangeOverlaps on Windows so mprotect of the host-only margin fails, cache freed stacks for same-size reuse, release one host allocation per unmap, and shrink the native reservation to 256 KiB.
9c1ec7f to
2456bc0
Compare
|
Addressed all three findings (rebased on current main, force-pushed). (1) Single-allocation commit: DoMapAnon down through CommitArenaRange takes a commit granule (0 keeps the old 16 KiB behavior for other callers); AllocateGuestStack passes bytes, so each stack is one MEM_REPLACE_PLACEHOLDER and the 64 KiB guard mprotect spans one region — the RegionSize check is untouched and passes. Required follow-up: UnmapRegistered now releases one host allocation at a time (registry pieces split by the guard protection are accumulated per allocation); per-piece reset truncated the single allocation ( (2) Windows stacks now use the Linux mechanism exactly: RegisterGuestStackMapping/UnregisterGuestStackMapping plus guestStackMappings compiled on Windows, Register in StartNativeThread, Unregister in FreeGuestStack/ReapRetiredWinStacks, GuestThreadStackHostRangeOverlaps enabled — mprotect below the reported start fails with EACCES. (3) Reuse plus shrink: FreeGuestStack caches up to 4 committed stacks (exit-retired stacks still unmap on signal, keeping the RequireUncommitted asserts); Allocate reuses same-size entries (verified engaged: 6765 reuses / 10100 creates). Native _beginthreadex reserve cut from stackSize+1 MiB to 256 KiB (entry/exit and SEH only; no test reads the host reserve). guest_pthread_start: 14.5s before, 1.5-1.8s after (3 runs), vs main's 1.6-2.3s. Linux unverified here: Windows Release Ninja (winlibs gcc15) only — guest_kernel_stack_protect, guest_pthread_start, guest_pthread_stack, guest_pthread_cancel, guest_once, guest_pthread_destructor_lifetime, fiber, guest_memory all green; git diff --check and check_conventions.py clean. |
|
Thanks, the single allocation, the Windows overlap check and the reuse cache all read right. Two things before CI:
Minor: when |
|
I checked the current head
These findings are on the current head; the PR’s latest Linux/Windows app jobs are skipped, so they were not caught by hosted checks. |
|
@Pouare514 a finding for your rebase, from @frankea's Windows runs on #7700 (comment). RTX 4090, Windows 11, on current For context, #7700 (NHL 26's Heartbeat walking an ended thread's stack) now has its Windows fix planned on top of this PR. frankea has a local follow-up that moves #7838's spare-stack list out of the AI-assisted: yes (Claude Code) |
|
A separate Windows regression surfaced in the RTX 4090 run documented in #7700: |
|
Rebased branch, Corrects Linux mmap/platform guards, split arena unmaps, partial host-allocation release and fixed-map rejection across host-only stack margins. Baseline and candidate pass all 8 focused Windows cases (Windows 11 26200, GCC 15.2.0-r7). Earlier independent unmap controls fail when the defects are restored. Original-head Linux syntax fails; integrated syntax passes with current headers under WSL GCC 15.2. #4563 and #9272 address separate stack-lifetime concerns. No full Linux build or title run. AI-assisted: yes. |
|
I checked the exact open heads for #7149 and dependent follow-up #9582 with |
This branch has not been deployed
6 participants
What
scePthreadCreatethreads now run their guest entry on aGuestArenastack:AllocateGuestStackviaDoMapAnon+Mprotectguard,FreeGuestStackon failure/exit, andCallOnGuestStackasm trampoline that swaps TEBStackBase/StackLimit/0x1478to the guest range for the entry (catch-and-rethrow) then restores host bounds.libSceFiber: small contexts (<64 KiB) now allocate fromGuestArenainstead of hostVirtualAlloc, sorsp-masking idioms resolve to mapped guest memory; large contexts unchanged.GuestPthreadStackasserts 9-digit (>= 0x100000000) guest address (GuestArena starts0x200000000, host <4 GiB).scePthreadSelf) and main threads still run on host stacks — main entry is covered by fix(windows): enter guest startup on an arena-backed main stack #3965.Tested
guest_pthread_stackfail-without (stash-abort: worker RSP0x2b0fbc0host) / pass-with (4/4 RunThread entries in guest range); neighbors pass:fiber,guest_memory,guest_pthread_start(4/4 on rebased tip, Release Ninja).0x1a54d81fault), 13NdJobWorkerThreadson guest stacks (0x1404xxbc80), Vulkan ready + swapchain 1472x828. Next wall is an unrelatedVideoOutbad_alloc.git diff --checkclean;python tools/check_conventions.py --base origin/main --head HEADpasses.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)