Skip to content

fix(agc): publish relocated shader snapshot on ABI resolution - #4043

Closed
NRAPM wants to merge 1 commit into
boykopovar:mainfrom
NRAPM:fix/agc-shader-abi-relocated-header
Closed

NRAPM wants to merge 1 commit into
boykopovar:mainfrom
NRAPM:fix/agc-shader-abi-relocated-header

Conversation

@NRAPM

@NRAPM NRAPM commented Oct 11, 2026

Copy link
Copy Markdown

Fixes #2914.

What

In titles such as Astro's Playroom (PPSA01325), shader ABI resolution calls (ResolveGraphicsStagesAbi, ResolveShaderAbi, ResolveGraphicsAbi) can pass a Shader* whose code pointer matches a registered shader, but whose header is allocated or copied at a different guest address.

When SameHeader matches the relocated header, the snapshot was not published for the new address. When decoders resolve stage info against the registered snapshot, pointer offsets are computed against the original base address instead of the relocated copy, leading to out-of-bounds header offsets.

This change:

  • Clones and publishes a new snapshot for the relocated header via RelocateSnapshotHeader and PublishRegisteredShader under mutex at the three resolve sites (ResolveGraphicsStagesAbi, ResolveShaderAbi, ResolveGraphicsAbi).
  • Keeps ShaderSnapshot strictly immutable without mutable members.
  • Expands PreparedShaders.cpp tests with relocated header copies.

Tested

  • Ran python tools/check_conventions.py --base origin/main (passed).
  • Ran python -m unittest discover tools/tests (60 passed).
  • Verified git diff --check (clean).

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
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • AI-assisted: yes

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Which line does PPSA01325 stop at on current main? Please post the log from an unpatched build. Two things I can't match with the description:

  • On main RegisteredHeader returns the registered snapshot with its own headerAddress and header bytes, and nothing rewrites them, so _readHeaderPod reads a consistent header. The stale-header case in Astro's Playroom (PPSA01325): shader ABI resolution passes a different Shader* header pointer for the same code #2914 comes from 9ad1b82a, which isn't on main.
  • RelocateSnapshotHeader copies header_size bytes from the pointer it is given. A caller that copies only the Shader struct has nothing behind it (Shader copy = header.shader in PreparedShaders.cpp, Registration), and every new copy address clones the registry and adds a snapshot.

Do the two added resolves fail without the change? The Tested section lists no build or ctest run.

@blawesom

Copy link
Copy Markdown
Contributor

Pre-review found a deterministic test regression on Linux — flagging early since full CI doesn't run automatically on PRs.

Independent pre-review (CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release, BUILD_TESTING=ON, AMD RX 9070 XT / RADV):

  • The PR builds and tools/check_conventions.py passes.
  • 4 tests fail deterministically with the same assertion:
agc_driver_prepared_shaders
agc_driver_prepared_shaders_indirect
agc_driver_prepared_shaders_uncached
agc_driver_prepared_shaders_debug

Each fails fast with AGC graphics: missing AGC user-data header.

Negative control: the same 7-test family passes 100% at this branch's own merge base (ad3369236) on the same machine — so the regression is introduced by the branch's changes, not the environment or the base.

Separately: this branch is ~1364 commits behind main and will need a rebase before review regardless. Reproduce with:

ctest -R agc_driver_prepared_shaders --output-on-failure

Happy to provide the full output if useful.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 13, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Closing: with this branch agc_driver_prepared_shaders, _indirect, _uncached and _debug fail with missing AGC user-data header (blawesom's run above), and the questions from 10-11 are still open. Please reopen with a build and ctest run and the line PPSA01325 stops at on unpatched main.

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

Labels

agc libSceAgc* needs rebase Conflicts with main, rebase needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Astro's Playroom (PPSA01325): shader ABI resolution passes a different Shader* header pointer for the same code

3 participants