Skip to content

fix(shader): reject temporary owners for borrowed memory regions - #8495

Merged
adriadam10 merged 3 commits into
boykopovar:mainfrom
yabanana:fix/borrowed-memory-regions
Oct 23, 2026
Merged

adriadam10 merged 3 commits into
boykopovar:mainfrom
yabanana:fix/borrowed-memory-regions

Conversation

@yabanana

@yabanana yabanana commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

What

request.context.memory = memory.Regions() currently compiles, but the returned vector dies at the semicolon and leaves the request's span dangling. #7759, now merged, fixes the 21 existing call sites. This change makes direct assignment from a temporary owning range a compile error, so that pattern cannot be reintroduced silently.

MemoryRegionView keeps the existing const-span interface and deletes conversion from non-borrowed rvalue contiguous ranges. Named mutable/const vectors and arrays, existing spans, pointer/count pairs, empty views and request copies remain supported. The conversion from a span stays constexpr/noexcept; tests pin size, alignment and trivial copyability. The view adds no ownership or allocation.

The existing request serializer test now checks these compile-time acceptance/rejection rules, pointer identity, observing changes to the live owner, copying a request and serializing/replaying its memory. Removing only the deleted constructor makes the negative assertions fail.

The view does not make arbitrary lifetime errors impossible. A named owner can still die too early, and a preconstructed span or raw pointer can already refer to dead storage. The region array and all byte storage must outlive their consumers; the debt entry records that limit. Shader instruction semantics and the request wire format are unchanged.

#7759 is merged upstream and is retained with its authorship. There is no remaining landing dependency. The API guard changes Recompiler.hpp, the existing request serializer test and the shader debt entry. No ARM64 AOT branch or feature is part of this change.

Overlap check: #6970 has an independent change at the same include-list location in Recompiler.hpp; both includes must be retained if it lands first. Several stale branches share files and could not be compared automatically. No other temporary-owner API guard was found.

Tested

Head 13b1892fddb844e229c7903df7cff2679fed3661 on main 513ca7e5585edd6e84b47e61328729588a7afe9d.

  • Native macOS Clang: complete request_serializer CTest passes with ASan and UBSan. The earlier use-after-free and deleted-constructor negative controls remain historical evidence.
  • Main and candidate each pass 11 common CTests plus one exact RGBA byte-readback test. The common group covers serializer, shader memory/bindless/disk cache, image-table cache/reuse, bindless images, flip/flip-compute, address-space teardown and D32 readback. Two additional color32/color64 optimized-readback cases are skipped on both revisions because llvmpipe exposes 8 subgroup lanes and they require 32; skips are not counted as passes.

Matched main/candidate runs use Ubuntu 24.04 x86-64, GCC 13.3 cross compiler hosted on ARM64, QEMU 10.2.3, SPIRV-Tools, Mesa 25.2.8 llvmpipe / LLVM 20.1.2. Vulkan is required, shader cache is disabled and VK_LAYER_KHRONOS_validation is requested; loader diagnostics confirm the layer is inserted. All runs have the same baseline VUID-VkDeviceCreateInfo-pNext-pNext for structure type 1000574000 from layer 1.3.275: no new VUIDs, not zero validation errors. Write watching is unavailable and the existing byte-comparison fallback is used.

Whitespace and conventions checks pass. Published commits and maintainer changes are preserved; integration was checked conflict-free against main de4be245b8f0bb941dd8b10cb4c700f9d564ecb9. Full upstream Linux/Windows CI requires maintainer dispatch. No console, title-unblock or native Windows runtime claim. No physical-GPU or title run of this head.

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 under Silent stubs in their library's technical debt file
  • 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
  • Fixes: none (related to AGC tests: request memory spans outlive temporary Regions() vectors #7754; fix(agc): retain shader memory region vectors #7759 owns the call-site fix)
  • Unblocks: none
  • GPUs tested: Mesa llvmpipe software renderer, Mesa 25.2.8 / LLVM 20.1.2, Ubuntu 24.04.5 x86-64 under QEMU 10.2.3; Khronos validation 1.3.275 loaded, identical baseline VUID as noted above; no physical GPU
  • AI-assisted: yes

@github-actions

github-actions Bot commented Oct 20, 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
PR 9989 core/shader/recompiler/Recompiler.hpp L15

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.

Pull request Shared files not compared
PR 5371 docs/dev/debt/shader.md
PR 5609 core/shader/recompiler/Recompiler.hpp
PR 5925 core/shader/recompiler/Recompiler.hpp
PR 6188 core/shader/recompiler/Recompiler.hpp
PR 6235 core/shader/recompiler/Recompiler.hpp
core/shader/recompiler/tests/RequestSerializerTests.cpp
docs/dev/debt/shader.md
PR 6280 docs/dev/debt/shader.md
PR 6412 core/shader/recompiler/Recompiler.hpp
docs/dev/debt/shader.md
PR 6488 docs/dev/debt/shader.md
PR 6539 core/shader/recompiler/Recompiler.hpp
PR 6612 core/shader/recompiler/Recompiler.hpp
docs/dev/debt/shader.md
PR 6924 docs/dev/debt/shader.md
PR 6959 core/shader/recompiler/Recompiler.hpp
docs/dev/debt/shader.md
PR 6970 core/shader/recompiler/Recompiler.hpp
PR 7201 core/shader/recompiler/Recompiler.hpp
PR 7410 core/shader/recompiler/Recompiler.hpp
core/shader/recompiler/tests/RequestSerializerTests.cpp
docs/dev/debt/shader.md
PR 7484 core/shader/recompiler/Recompiler.hpp
docs/dev/debt/shader.md
PR 7642 core/shader/recompiler/Recompiler.hpp
PR 7684 docs/dev/debt/shader.md
PR 7689 core/shader/recompiler/Recompiler.hpp
PR 7922 docs/dev/debt/shader.md
PR 8004 core/shader/recompiler/Recompiler.hpp
core/shader/recompiler/tests/RequestSerializerTests.cpp
docs/dev/debt/shader.md
PR 8077 docs/dev/debt/shader.md
PR 8277 docs/dev/debt/shader.md
PR 8281 docs/dev/debt/shader.md
PR 8406 docs/dev/debt/shader.md
PR 8410 docs/dev/debt/shader.md
PR 8489 core/shader/recompiler/Recompiler.hpp
docs/dev/debt/shader.md
PR 8564 core/shader/recompiler/Recompiler.hpp
core/shader/recompiler/tests/RequestSerializerTests.cpp
docs/dev/debt/shader.md
PR 8658 core/shader/recompiler/Recompiler.hpp
PR 8792 docs/dev/debt/shader.md
PR 9020 core/shader/recompiler/Recompiler.hpp
core/shader/recompiler/tests/RequestSerializerTests.cpp
docs/dev/debt/shader.md
PR 9183 docs/dev/debt/shader.md
PR 9193 core/shader/recompiler/Recompiler.hpp
core/shader/recompiler/tests/RequestSerializerTests.cpp
docs/dev/debt/shader.md
PR 9335 core/shader/recompiler/Recompiler.hpp
core/shader/recompiler/tests/RequestSerializerTests.cpp
docs/dev/debt/shader.md
PR 9356 core/shader/recompiler/Recompiler.hpp
core/shader/recompiler/tests/RequestSerializerTests.cpp
PR 9387 core/shader/recompiler/Recompiler.hpp
core/shader/recompiler/tests/RequestSerializerTests.cpp
PR 9508 core/shader/recompiler/Recompiler.hpp
PR 9748 core/shader/recompiler/Recompiler.hpp
core/shader/recompiler/tests/RequestSerializerTests.cpp
docs/dev/debt/shader.md
PR 9889 docs/dev/debt/shader.md
PR 9897 core/shader/recompiler/Recompiler.hpp
PR 10000 core/shader/recompiler/Recompiler.hpp
docs/dev/debt/shader.md
PR 10049 core/shader/recompiler/Recompiler.hpp
PR 10193 core/shader/recompiler/Recompiler.hpp
PR 10348 core/shader/recompiler/Recompiler.hpp
core/shader/recompiler/tests/RequestSerializerTests.cpp

@github-actions github-actions Bot added shader core/shader docs agc libSceAgc* labels Oct 20, 2026
@yabanana
yabanana marked this pull request as ready for review October 23, 2026 11:57
@yabanana

Copy link
Copy Markdown
Contributor Author

Updated at 13b1892fddb844e229c7903df7cff2679fed3661. #7759 has merged upstream; its changes/authorship are retained and there is no remaining landing dependency.

Native macOS request-serializer tests pass with ASan and UBSan. Matched main/candidate software-GPU runs each pass 11 common CTests plus exact RGBA byte readback, including flip and teardown. Both skip the same two 32-lane-only color cases; skips are excluded from the pass count. Khronos validation is loaded and the existing pNext VUID is identical to main, with no new VUIDs.

No wire-format, shader semantics or AOT changes. Marking ready for review; full Linux/Windows CI still needs maintainer dispatch.

@adriadam10 adriadam10 self-assigned this Oct 23, 2026
@adriadam10
adriadam10 merged commit 1e1ab77 into boykopovar:main Oct 23, 2026
11 checks passed
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the deleted rvalue-range constructor against copy/move and the named-owner, array, span, pointer/count and empty-assignment paths; no remaining temporary-owner assignments in the tree; the compile-time assertions in RequestSerializerTests.cpp fail without the = delete. Shader semantics and wire format are unchanged; merged.

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

Labels

agc libSceAgc* docs shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants