Skip to content

feat(agc): implement deferred faults for reads through null SRT pointers - #759

Merged
oneandonlydean merged 1 commit into
boykopovar:mainfrom
Celegans12:feat/agc-guarded-null-pointers
Oct 9, 2026
Merged

oneandonlydean merged 1 commit into
boykopovar:mainfrom
Celegans12:feat/agc-guarded-null-pointers

Conversation

@Celegans12

@Celegans12 Celegans12 commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

What

Capturing a shader throws AGC driver: null or misaligned address when the flat SRT walk reads through a pointer loaded from memory that is null or unmapped, even if the program skips that s_load; the console faults only when a wave runs it. Following boykopovar's guidance, this is a pure runtime check: shaders still compile at registration and nothing per capture enters a key.

Tested

  • Windows 11, MinGW-w64 GCC 15.2.0, based on main 1be6f9f: configure -DBUILD_TESTING=ON, full build, --target libs, ctest:
  • 486/486 tests pass (ctest --output-on-failure)
  • Same build on lavapipe (Mesa 26.2.4 llvmpipe, 8-wide subgroups, VK_DRIVER_FILES): 485/485 pass; agc_driver_flat_store is left out there because it takes several minutes on Windows llvmpipe
  • New agc_driver_guarded_null_pointer (GPU): skipped null reads (SCC branch, s_cbranch_execz) don't fault; executed ones fault at their pc and address, even with one lane or an empty EXEC; all runs of a program share one pipeline variant.
  • agc_shader_memory: null, unmapped and chained records, one variant, specialization and module for poisoned and clean captures, replay, and the remaining throws. Neither test builds on main; both pass with -DANYPS5_ENABLE_SPIRV_TOOLS=ON.
  • PPSA26344 v1.008, before perf(agc): baseline shader recompilation on registration #639: 0x800027c200 dispatched with no fault. Not run again.
  • Not tested: graphics-stage execution on a GPU, console behaviour.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • AI-assisted: yes

@github-actions github-actions Bot added build shader core/shader docs agc libSceAgc* labels Oct 5, 2026
@Celegans12
Celegans12 force-pushed the feat/agc-guarded-null-pointers branch from 55640cf to 4303b6f Compare October 6, 2026 00:19
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 6, 2026
@Celegans12
Celegans12 force-pushed the feat/agc-guarded-null-pointers branch from 4303b6f to ccec334 Compare October 6, 2026 02:50
@Celegans12
Celegans12 force-pushed the feat/agc-guarded-null-pointers branch from ccec334 to 06645dd Compare October 6, 2026 04:43
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 6, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 6, 2026
@Celegans12
Celegans12 force-pushed the feat/agc-guarded-null-pointers branch from 06645dd to f40fed6 Compare October 6, 2026 09:48
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 6, 2026
@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
#218 core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp L1527, L1549-1550
#285 core/libs/prx/libSceAgcDriver/Graphics/src/ShaderValidation.cpp L456
#494 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawCapture.cpp L90-108
#920 core/libs/prx/libSceAgcDriver/tests/execution/ImageTable.cpp
core/libs/prx/libSceAgcDriver/tests/execution/ImageTableCache.cpp
core/shader/recompiler/ImageTableAbi.hpp
#940 core/shader/recompiler/SpirvBackend/src/SpirvImageEmitter.cpp L627-629, L968-971
#1074 core/libs/prx/libSceAgcDriver/Execution/src/VulkanDevice.cpp L197, L2458
core/libs/prx/libSceAgcDriver/Graphics/include/Context.hpp L137
core/libs/prx/libSceAgcDriver/Graphics/include/Pipeline.hpp L92
core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp L521
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderValidation.cpp L126, L495, L511
#1077 core/libs/prx/libSceAgcDriver/Execution/src/VulkanDevice.cpp L197, L2458
core/libs/prx/libSceAgcDriver/Graphics/include/Context.hpp L137
core/libs/prx/libSceAgcDriver/Graphics/include/Pipeline.hpp L92
core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp L521
core/libs/prx/libSceAgcDriver/Graphics/src/ShaderValidation.cpp L126, L256, L495, L511

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 8, 2026
@Celegans12

Copy link
Copy Markdown
Contributor Author

Thanks for the RADV run, it's good to see 0x80003aa700 get through on AMD too. The conflict with main came from today's cache format bump. 6fb7f68 is rebased onto it and merges cleanly now. For the #759 + #822 combination, I put a merged branch on my fork and described it on #822: https://github.com/Celegans12/AnyPS5/tree/test/yotei-759-822

Capturing a shader threw "null or misaligned address" when the flat
SRT walk read through a pointer loaded from memory that is null or
unmapped, even when the program skips that s_load. Such a read now
yields zero and a record with its pc and address, as does a slot whose
address needs it. The registered artifact checks one flag word per
guarded slot at each ReadConst; the capture appends the flags and the
records to the flattened SRT, and a set flag raises an Unmapped BDA
fault through the fault buffer and stops the invocation as a BDA read
does. Nothing per capture enters a key: poisoned and clean captures
share one variant and specialization. Descriptors, branch conditions,
uniform fills, table bases and user-data pointers that need such a
value still throw, and poisoned results stay out of the dispatch and
draw caches. The shader disk cache format becomes 21.
@Celegans12
Celegans12 force-pushed the feat/agc-guarded-null-pointers branch from 6fb7f68 to 56f8a62 Compare October 8, 2026 22:35
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 8, 2026
@Celegans12

Copy link
Copy Markdown
Contributor Author

I've rebuilt this as a pure runtime check, following boykopovar's guidance. It now sits directly on main instead of on #758 and #757, so it doesn't depend on either of them any more. This is the redo after #639 that adriadam10 asked about.

At registration, every flat SRT slot that can depend on a read through a pointer loaded from memory gets a guard: its ReadConst loads one flag word from the flattened SRT and branches on it. When a capture finds such a read null or unmapped, it skips the read, the slot reads 0, and the flag points at a {pc, address} record. If a wave actually executes the read, the shader raises an Unmapped BDA fault at that pc and stops the invocation. Nothing per capture goes into any key, so poisoned and clean captures share one variant, specialization and pipeline.

oneandonlydean mentioned this is the next fix Astro Bot needs. I haven't run Astro Bot on the new head myself, so a run from someone with the title would be very welcome. Otherwise it's ready for review.

Details

@EdouardSence

Copy link
Copy Markdown
Contributor

Astro Bot (PPSA21567, retail) run on the new head 56f8a62. No regression from this PR, but the run doesn't reach its code yet.

Setup: RX 9070 XT (RDNA4), RADV / Mesa 26.2.4, Linux 7.2.9. Scripted input, headless gamescope, from boot. All stacks are on main a8b032b and merged cleanly.

Stack Where the run stops
this PR + #1836 + #1837 (needed to get past registration since #639) about 2 min 20 s: shader image heap capacity exceeded (#1789)
+ CouchRiv's #1789 branch (bd5f22c) about 3 min: prepared shader artifact is missing for the requested static ABI ... stage=5 wave=64 (the null pixel program, #1901)
same stack without this PR the same stop at the same point
+ #1902 about 2 min 50 s: prepared rectangle artifacts are missing (details on #1902)
  • With this PR applied, no error is new compared with the same stack without it. No Unmapped fault came from the guarded reads.
  • No frame was presented in any of these runs, so the scene that needed this PR on the old astrobot branch is never reached. I'll run it again once the walls above are fixed.

Tested and written by Claude (AI assistant) on a contributor's machine.

@EdouardSence

Copy link
Copy Markdown
Contributor

Astro Bot (PPSA21567) now reaches this PR's code, and it gets past the stop. Tested on head 56f8a62, Linux, RX 9070 XT (RADV), scripted and headless.

Base: main 43972e6 + #1836, #1837, #2105, #1893, #1902, #1928 and CouchRiv's #1789 branch (bd5f22c).

No frame is presented yet in these runs.

Tested and written by Claude (AI assistant) on a contributor's machine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* build docs shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants