Skip to content

feat(agc): push and set debug markers from a text span - #8924

Merged
adriadam10 merged 3 commits into
boykopovar:mainfrom
hypermoist:feat/agc-marker-span
Oct 24, 2026
Merged

adriadam10 merged 3 commits into
boykopovar:mainfrom
hypermoist:feat/agc-marker-span

Conversation

@hypermoist

Copy link
Copy Markdown
Contributor

What

Four libSceAgc exports were NotImplemented stubs in Unimplemented.cpp. In the decrypted PS5 libSceAgc.sprx they take (buffer, text, length, color). Each one builds the same driver user-data marker as its NUL-terminated counterpart, but with the length supplied by the caller instead of strlen. sceAgcAcbPushMarker (0x1B50) and sceAgcAcbPushMarkerSpan (0x1A90) both call sceAgcDriverUserDataWritePushMarker with the same arguments; only where the length comes from differs.

Export (NID, address) Here
sceAgcAcbPushMarkerSpan (8Kly1JrJUlw, 0x1A90) the existing push-marker packet, with the text cut at length and terminated
sceAgcAcbSetMarkerSpan (pxx-GoOSdw4, 0x1900) push then pop, as this project's sceAgcAcbSetMarker does
sceAgcDcbPushMarkerSpan (N7M3+N-sBCw, 0x7930) as the ACB one
sceAgcDcbSetMarkerSpan (BYcSvEsINWU, 0x77A0) as the ACB one

Agc::Marker::Push now measures the string and calls a new Agc::Marker::PushSpan, so the packet format is defined in one place. A null text pointer is accepted only with a zero length. The colour is ignored, as in the existing marker calls.

The bodies reuse this project's marker packets; nothing from the module is copied.

Tested

Linux (CachyOS, GCC, Release): full build. ctest -R "guest_|kernel|ampr|agc": 1381/1381 pass on the second run, with agc_driver_stencil_ref_export skipped as usual. The first run reported one failure that did not reproduce, and I could not tell which test it was.

The new testMarkerSpans in agc_commands checks:

  • that a span packet stops at its length ("framebuffer", 5, gives "frame");
  • that it equals pushing "frame" whole;
  • that the set variants return the push packet and append a pop;
  • that ACB and DCB produce identical words;
  • that a null buffer, and a null text with a non-zero length, are rejected.

With sceAgcAcbPushMarkerSpan ignoring its length, the test fails.

tools/check_conventions.py is clean. No open PR implements these names or NIDs. Merge note: #1060 (sceAgcAcquireMemSetEngine) and my #8318 (sceAgcSetSemaphoreMemory) delete the stub blocks between these four in Unimplemented.cpp. No line is changed by both, but git reports the neighbouring deletions as a conflict. Whichever merges second needs those stub deletions combined, nothing else.

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
  • Unblocks: PPSA08427 (Killing Floor III) imports all four through kitt_support; its libSceJobManager also imports the two ACB ones, as does PPSA29340's
  • GPUs tested: n/a, no shader or VideoOut change; the packets are checked in agc_commands
  • AI-assisted: yes

🤖 Generated with Claude Code

https://claude.ai/code/session_01RbLgECLQ2bpDM7oe2urGZK

@github-actions github-actions Bot added the unblocks title Names a title ID on the Unblocks line label Oct 21, 2026
@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Progress report (5d251e6 → 21eb489)

📈 System libraries: 49.02% (+0.04%, +4 implemented functions)

✅ 4 implemented
Library Name
libSceAgc sceAgcAcbPushMarkerSpan
libSceAgc sceAgcAcbSetMarkerSpan
libSceAgc sceAgcDcbPushMarkerSpan
libSceAgc sceAgcDcbSetMarkerSpan

Changed libraries (implemented exports)

Library Before After Change
libSceAgc 226/240 (94.17%) 230/240 (95.83%) +1.67 pp

Agc::Command::Require(buf != nullptr, function, "null command buffer");
const char* text = str ? str : "";
const std::size_t bytes = std::strlen(text) + 1;
const std::size_t length = std::strlen(text);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This refactor removes the previous Require(buf != nullptr) before strlen. If a caller supplies a null command buffer and an invalid non-null guest string pointer, the legacy marker APIs now read that pointer before reporting the null-buffer error; this can fault instead of taking the checked error path. Please retain the buffer check before measuring str.

@MarcZX10

Copy link
Copy Markdown
Contributor

I prepared follow-up commit b213bd5e4e on top of this PR head. It restores the null-command-buffer check before the legacy marker path calls strlen, preserving the previous validation order. The upstream-based conventions check and git diff --check pass; I could not run agc_commands because there is no configured full build in this workspace. GitHub denied pushing to your fork (403), so the commit is available on mine if you want to cherry-pick it.

@hypermoist

Copy link
Copy Markdown
Contributor Author

Thanks, you're right: moving the check into PushSpan let Push call strlen before the null-buffer check. I've taken your b213bd5 onto the branch as-is and added fd5e00e, a regression test: sceAgcAcbPushMarker(nullptr, (const char*)0x10, 0) must take the checked error path. Without your line, agc_commands segfaults on it; with it, all four agc_command tests pass.

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The author incorporated the null-buffer validation fix and added the regression case with an invalid text pointer and null command buffer. The reported agc_commands cases now pass with the check in place. The updated fix preserves validation order before strlen; approved.

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

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 1060 core/libs/prx/libSceAgc/Unimplemented.cpp
core/libs/prx/libSceAgc/tests/Commands.cpp

@github-actions github-actions Bot added agc libSceAgc* needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 21, 2026
@adriadam10 adriadam10 self-assigned this Oct 23, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 24, 2026
hypermist and others added 3 commits October 24, 2026 14:46
sceAgcAcbPushMarkerSpan, sceAgcAcbSetMarkerSpan, sceAgcDcbPushMarkerSpan and
sceAgcDcbSetMarkerSpan take (buffer, text, length, color) and, on the console,
build the same user-data marker packet as their NUL-terminated counterparts
with the length given by the caller. They now share the existing marker
packets: the text is cut at the given length and terminated, and the set
variants push and pop as sceAgc*SetMarker does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RbLgECLQ2bpDM7oe2urGZK
A null command buffer with an unreadable text pointer must take the
checked error path in sceAgcAcbPushMarker; without the buffer check
before measuring the text, the test faults.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RbLgECLQ2bpDM7oe2urGZK
@adriadam10
adriadam10 force-pushed the feat/agc-marker-span branch from 806949b to 21eb489 Compare October 24, 2026 12:47
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 24, 2026
@adriadam10
adriadam10 merged commit d4adeae into boykopovar:main Oct 24, 2026
10 checks passed
@adriadam10

Copy link
Copy Markdown
Collaborator

Rebased onto current main: the only conflict was the neighbouring stub blocks in Unimplemented.cpp (the four *MarkerSpan stubs removed, the signatures main added for sceAgcAcquireMemSetEngine and sceAgcCbMemsetExclusive kept). Rebuilt libSceAgc and its 25 tests pass. Checked the span packet against the existing Push format (length cap unchanged, null buffer rejected before the text is read, null text only with length 0), the push+pop pairing for the Set variants, and that testMarkerSpans pins the cut at length, equality with a whole push and ACB/DCB parity; merged.

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

Labels

agc libSceAgc* unblocks title Names a title ID on the Unblocks line

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants