Skip to content

fix(agc): accept unaligned shader headers in ABI resolution - #4809

Merged
oneandonlydean merged 2 commits into
boykopovar:mainfrom
MarcZX10:codex/unaligned-shader-abi-4782
Oct 13, 2026
Merged

oneandonlydean merged 2 commits into
boykopovar:mainfrom
MarcZX10:codex/unaligned-shader-abi-4782

Conversation

@MarcZX10

@MarcZX10 MarcZX10 commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

What

Carries the shader-header alignment fix from the second patch attached to #4782. ABI resolution and registered-header lookup now copy the fixed Shader fields into an aligned local object before reading them. This lets those paths handle guest headers whose addresses are not naturally aligned, while preserving the existing guest-memory range check.

Extends PreparedShaders with a header address four bytes off its natural alignment and exercises ABI resolution through it. The unrelated exports/library patch from #4782 is not included.

#4043 and #2589 also touch ShaderRegistry.cpp and/or PreparedShaders.cpp, but address relocated snapshots and background ABI compilation. This change has no dependency on either PR; the file overlap is disclosed for rebasing coordination.

Tested

  • Debian 12, GCC 12.2, Release, Ninja, BUILD_TESTING=ON: built agc_driver_prepared_shaders_tests, including the changed ShaderRegistry.cpp and PreparedShaders.cpp.
  • With Mesa lavapipe forced (mesa-vulkan-drivers 22.3.6): agc_driver_prepared_shaders passed (1/1).
  • python3 tools/check_conventions.py --base upstream/main and git diff --check upstream/main...HEAD: passed.
  • No console run; PPSA01317's unaligned header was observed by the reporter.

Checklist

  • Based on current main; no open PR implements the same alignment behavior
  • 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); no new silent stubs
  • No notes, investigation .md files or images added to the repository
  • No new third-party code
  • Generic behavior, not title-specific
  • Depends on: none
  • Unblocks: none; the title still needs its unresolved imports addressed
  • GPUs tested: Mesa lavapipe software Vulkan renderer, Debian 12, mesa-vulkan-drivers 22.3.6
  • AI-assisted: yes

dawid-swiecicki and others added 2 commits October 12, 2026 22:24
Gran Turismo 7 (PPSA01317) passes shader headers at 4-byte aligned addresses to
sceAgcUnknownCreateInterpolantMapping. RegisterShader already reads them through a
copy (boykopovar#902); ResolveShaderAbi, ResolveGraphicsStagesAbi, ResolveGraphicsAbi and
RegisteredHeader still required 8-byte alignment and threw "null or misaligned
address". Read the fixed fields through GuestShaderFields at those sites.
@GoodTimeclubs

Copy link
Copy Markdown
Contributor

I had prepared the same fix on my own branch (not pushed), so I won't open a duplicate. While testing it I found two things that might be worth adding here:

  1. The shader registers bound with a header are still checked with alignof(ShaderRegister) and read through the pointer. This happens in ResolveGraphicsStagesAbi when fields.type != snapshot.type (CheckRange(fields.sh_registers, ..., alignof(ShaderRegister)), then fields.sh_registers[index]), and in FusedHullHeader (CheckRange(bound, bytes, alignof(ShaderRegister)), then bound[index]). If a header sits at an address that is not 4-byte aligned, the registers that follow it are misaligned too. In that case these two paths still throw null or misaligned address. In practice it only affects a fused hull header (type 3 against a registered type 7) at such an address, so it's narrow. Reading them with CheckRange(..., 1) plus memcpy, as for the fixed fields, closes the gap:
ShaderRegister GuestShaderRegister(const ShaderRegister* registers, std::size_t index) {
    ShaderRegister reg;
    std::memcpy(&reg, static_cast<const void*>(registers + index), sizeof(reg));
    return reg;
}
  1. Coverage at offset 1 and on the compute path. These are two small additions I used:
// tests/Submit.cpp, testShaderHeaderAlignment, after the registration loop
for (const std::size_t offset : {4, 1}) AgcDriverResolveShaderAbi_nid_postfix(at(offset, shader), {}, {});

// tests/execution/PreparedShaders.cpp, SharedCodeHeaders, after resolving `copy`
alignas(8) std::array<std::byte, sizeof(Shader) + 4> unaligned{};
for (const std::size_t offset : {4, 1}) {
    std::memcpy(unaligned.data() + offset, &copy, sizeof(copy));
    AgcDriverResolveShaderAbi_nid_postfix(reinterpret_cast<const Shader*>(unaligned.data() + offset), {}, primitive);
}

On main both fail with AGC driver: null or misaligned address. On this branch both pass. I tested on Ubuntu 24.04 with GCC 14.2 and lavapipe.

AI-assisted (Claude Code).

@github-actions

github-actions Bot commented Oct 12, 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
#4043 core/libs/prx/libSceAgcDriver/tests/execution/PreparedShaders.cpp L479-481

@kr4y4

kr4y4 commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

A run on GPU hardware, since the description and the comment above used lavapipe.

Setup: Windows 11 (26200), WinLibs GCC 15.2.0 r7, Release, CI configure flags, an RTX 4070 Laptop GPU and an Intel Raptor Lake-S iGPU. The head 1988607 merged into main 284a1c3, which passes 1020 of 1020 on both GPUs.

  • agc_driver_prepared_shaders passes on both GPUs with --repeat until-fail:3. The full suite (ctest --timeout 120 --parallel 4) passes 1020 of 1020 on both.
  • With ShaderRegistry.cpp as on main, agc_driver_prepared_shaders fails on both GPUs with AGC driver: null or misaligned address.

Written with AI assistance.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants