Repository navigation
fix(libSceFiber): pin fiber contexts writable under write tracking on Windows - #283
Conversation
|
Thanks. Which retail title did you check this with, and does it still terminate on current main without this PR? The branch is from 10-01, and CI hasn't run on it. A rebase onto main, plus the title ID and the point where it terminates, would show that both fixes (the TIB bounds order and pinning the fiber contexts) are still needed. |
|
@oneandonlydean PPSA21564 v01.007.000, on Windows 11, Ryzen 7 7800X3D, RTX 5070 Ti. What I have so far is from your I'll rebase onto current Written with AI assistance (Claude Code). |
… Windows Write tracking protects shared guest pages read-only. A push on a fiber stack in such a page faults, and Windows cannot dispatch it because the exception frame lands on the same protected stack, so the process was terminated without a report. Pin fiber contexts writable for their lifetime; pinned pages are always collected as written. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2443a93 to
57588cf
Compare
|
@oneandonlydean results. PPSA21564 v01.007.000, Windows 11, Ryzen 7 7800X3D, RTX 5070 Ti, MinGW-w64 GCC 15.2.0, Current Your
So only the pinning is needed. That makes sense: the bounds order only matters if a push on the fiber stack faults, and with the contexts pinned writable those pages are never armed, so the push doesn't fault. I've dropped the bounds change: the branch is now one commit on current Separately: without Written with AI assistance (Claude Code). |
adriadam10
left a comment
There was a problem hiding this comment.
Thanks for isolating the pinning from the bounds change, the 4-way table makes the cause clear. Code reading checks out (the only arm is behind clear && !pinned, and Pin/Unpin invalidate the clean ranges). Two things before merge:
- No test. The CI jobs that are green don't exercise any of this, and the evidence is a retail title run we (and Linux CI) can't reproduce.
core/libs/tests/GuestMemory.cppalready has_WIN32cases usingGuestArena, so a case there would run on the Windows job: map a direct memory range,GuestArenaPinWritable_nid_postfixpart of it,GuestArenaCollectWrites_nid_postfix(..., clear=true)twice, then check that the pinned page is stillPAGE_READWRITE(VirtualQuery) and is reported on both collects, while an unpinned page in the same range is armed and only reported once; afterUnpin+ collect it gets armed. It should fail without the change. - Relation to #637 (DotDebian, same header, same root cause as #361). With #637, direct memory mapped at a single address lives in
MEM_WRITE_WATCHmemory and is never write-protected, so the fiber-stack fault only remains for pages mapped at two or more addresses. The two PRs conflict textually inWindowsMappings.hpp; whichever lands second needs a rebase. Please say in the description whether pinning is still needed on top of #637 (i.e. whether the title's fiber contexts are in aliased pages), so it's clear what this PR covers after both.
Minor: sceFiberInitialize on an object that is already initialised pins again without unpinning, so those pages stay pinned (reported as written on every collect) for good. Unpinning the previous context when magic == FIBER_MAGIC would avoid it, if a title re-initialises without finalising.
…itten Also unpin the previous context when a fiber object is initialised again without being finalised. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@adriadam10 thanks, all three are in
Written with AI assistance (Claude Code). |
adriadam10
left a comment
There was a problem hiding this comment.
Thanks, all three points are addressed. Checked: the PR merged onto current main builds and passes ctest on Linux (400/400). In code review, a pinned page is never armed (clear && !pinned), Pin/Unpin invalidate the clean ranges, and the expected counts in CheckPinnedSharedPages match the 16 KiB view / 4 KiB report granularity. The Windows CI job runs the new case. Merging once Windows and Linux CI are green. #637 will need a rebase on WindowsMappings.hpp after this.
What
Fiber contexts live in guest memory. When write tracking had armed such a page read-only, a push on the fiber stack faulted and Windows could not dispatch the exception, because the exception frame is written to the same protected stack, so the process was terminated without any report. Fiber contexts are now pinned writable (
GuestArenaPinWritable_nid_postfix) for their lifetime; pinned pages are never armed and are always collected as written. Initialising a fiber object again without finalising it unpins the previous context first.Relation to #637
#637 keeps direct memory mapped at a single address in
MEM_WRITE_WATCHmemory, which is never write-protected. After #637 this PR only matters for fiber contexts in pages mapped at two or more addresses. In the title I tested, all 13 pins (9 distinct contexts, 1 MiB each) are in pages with a single mapping, so with #637 that title would not need the pinning. Whichever PR lands second gets rebased (both touchWindowsMappings.hpp). If #637 lands first, I'm fine with closing this one, or keeping it for aliased contexts.Tested
Windows 11, MinGW-w64 GCC 15.2.0 (winlibs, ucrt posix seh). Full build;
ctestpasses. New caseCheckPinnedSharedPagesincore/libs/tests/GuestMemory.cpp(Windows only). It maps direct memory and pins its middle page. The pinned page must stayPAGE_READWRITEand be reported on every collect. Its unpinned neighbour must be armed and reported once per write. AfterUnpinand a collect, the page must be armed again. The case fails if pinning is removed, or if collects arm pinned pages. Checked with a retail title: without this change it terminates silently (0xC0000005, no report) 39–57 s after start in 4 of 4 runs, with it it runs on (2 of 2, 120 s). Details in the comments.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository🤖 Generated with Claude Code