Skip to content

fix(npcppwebapi): declare DataStatus iterator output storage - #15850

Closed
JJenkx wants to merge 2 commits into
boykopovar:mainfrom
JJenkx:fix/npcwa-data-status-iterator-abi-20261104
Closed

JJenkx wants to merge 2 commits into
boykopovar:mainfrom
JJenkx:fix/npcwa-data-status-iterator-abi-20261104

Conversation

@JJenkx

@JJenkx JJenkx commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Current-main integration — 2026-11-05. Exact head 69ffe68b60e3e51ca98b2fbe07a8fbf5f31ac328 retains previous public 8fd753b1eaca8151bc79dc8648c6e199af8b4d40 and tested main 6d173c1a19788ff69d43b3d0ca62833e312ef939 as ancestors. The DataStatus output-argument declarations and two refusing bodies are preserved; both exports still throw. Their ABI/export topic patch has the same stable patch-id as the prior head, while current main’s neighboring exports and CommonConstIterator additions remain. No new throwing-stub test or online behavior is claimed.

Linux x86-64, GCC16.2.1, Ninja Release: actual production modules and focused fixture owners rebuilt; 13/13 runtime tests pass, zero skips: guest_np_cpp_web_api_intrusive_ptr, guest_np_cpp_web_api_friends, guest_np_cpp_web_api_signed_out, guest_np_cpp_web_api_offline, guest_np_cpp_web_api_matches, guest_np_cpp_web_api_matchmaking, guest_np_cpp_web_api_string, guest_np_cpp_web_api_title_cloud_storage, guest_np_cpp_web_api_alias_append, guest_np_cpp_web_api_alias_string_vector, guest_np_cpp_web_api_alias_intrusive_vector, guest_np_cpp_web_api_binary, guest_np_cpp_web_api_activities. Matching pinned main passes all25 selected baseline groups. Production NID patching, conventions/whitespace, and MinGW syntax for Export.cpp pass.

Public-header audit: failed on both pinned main and this head with the same five missing WKSecurityOrigin* declarations (Create, CopyToString, CopyHost, CopyProtocol, GetPort), with no additional declaration errors. The failure is retained in the logs; no all-green header/full-suite claim is made. The concrete shared-main repair is #16010, exact head 545e24b2ee2e549001421f7fc9f87ff801f3423c. Its independent qualification reports a green audit of 406 libraries/30,572 exports and two standalone units compiled by each of GCC and MinGW. A maintainer still needs to merge that repair and rerun the affected checks; this unmodified qualification tree is not relabelled header-green.

The older-main Windows DepthSurface::createSampleShadow allocation compile error is already corrected by merged #15855. This head contains that fix, and its DepthSurface.cpp/ImageMemory.hpp blobs match the fixing source. The fix’s hosted Windows build passed; that is attributed upstream evidence, not a Windows build/runtime result for this head. No duplicate AGC change was added. A final merge preview against newer main 7ad9749ed0311cc91d69bc7c690d09c92b9d42d0 is clean; qualification remains pinned to 6d173c1a19788ff69d43b3d0ca62833e312ef939.

Source/parent/provider/gitlink proofs, commands, raw logs, JUnit, hashes and known baseline failure. Historical platform, module-reference and performance reports below retain their original pins; no new Windows/macOS, SDK/physical-console, title/GPU or performance run.

Earlier pinned reports and the contribution template follow.

What

Correct Common::Vector<IntrusivePtr<TitleCloudStorage::V1::DataStatus>>::begin/end to take (out, self): the first pointer is hidden iterator-return storage. Both inspected bundled modules read the vector from RSI and return the original RDI destination. The current one-argument declarations misidentify that destination as self.

Both exports continue to throw NotImplemented. This adds no implemented functions or game unblock. The two bundled modules use different endpoint layouts (+0x18/+0x28 versus +0x00/+0x10) and different helper sequences, so the debt entry records why a single-offset implementation would be unsupported. Helper semantics, iterator storage/lifetime and safe layout selection remain unresolved. No title-specific code is added.

Tested

Source 8fd753b1eaca8151bc79dc8648c6e199af8b4d40, base 9ab6a281ea61c9e62a7eb07eb4cef8f19ede1c5c. Linux x86-64, GCC 16.2.1, Ninja Release: actual NpCppWebApi CMake owner and root-registered fixture owners, 11/11 tests on main and 11/11 on the change, no failures or skips. Public-header audits, conventions and whitespace checks pass. The built library SHA-256 is identical on both arms: existing refusing runtime bodies are preserved. The scoped recipe uses a matching-main frozen libc provider; it is not a full-project build. No new tests duplicate a throwing stub.

The complete exported bodies in the modules bundled with PPSA21564 and PPSA05347 were read statically; neither module was executed. No Windows, physical-console, title/GPU, online response or FPS behavior is claimed.

Source patch/pins, static-reference hashes, raw owner-build/test logs/JUnit, provider hash and overlap review, SHA-256 8a9044b9f4db3eef263730db0e9cf58025015fa9dd50685ad14baae1331e2e00 (download verified).

The overlap scan covered 1,840 open heads against main 0263899e996f; the relevant module/fixtures/debt are unchanged since the qualified base. #14122/#14136/#14273 have adjacent header changes but retain these one-argument declarations: combining should preserve both corrections. The fail-closed #13486/#14212 merge trees were reviewed by reversing only this topic; pre-existing conflicts remain unchanged. No runtime dependency or combined-runtime claim.

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: none
  • GPUs tested: n/a
  • AI-assisted: yes (Codex)

@JJenkx

JJenkx commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed to 69ffe68b60e3e51ca98b2fbe07a8fbf5f31ac328 on tested main 6d173c1a19788ff69d43b3d0ca62833e312ef939, retaining the prior public head. The DataStatus output-argument declarations and two refusing bodies are preserved; both exports still throw. Their ABI/export topic patch has the same stable patch-id as the prior head, while current main’s neighboring exports and CommonConstIterator additions remain. No new throwing-stub test or online behavior is claimed.

Fresh Linux CPU-only qualification: 13/13 runtime tests pass, zero skips; actual module/fixture builds, production NID patching, conventions/whitespace and MinGW syntax pass. Public-header auditing remains red on main and this head for exactly the same five missing WKSecurityOrigin declarations; this is recorded, with no header-green claim.

The inherited older-main Windows allocation compile failure is fixed by already merged #15855; the fixing source is preserved here. Its successful Windows build is upstream evidence, not this branch’s Windows qualification. Final merge preview against main 7ad9749ed0311cc91d69bc7c690d09c92b9d42d0 is clean. Historical Windows/native-module reports stay at their original pins.

Immutable source/log/JUnit/control evidence, SHA-256 8cd459887f36477c0d43bb1ef4a3601ede91abb1a54dbc6c54ab43a4ed085c0d. No new Windows/macOS runtime, SDK-module/physical-console, title/GPU or speed claim.

AI-assisted: yes (Codex).

@akkadbakkad89

Copy link
Copy Markdown
Contributor

Confirmed the (out, self) sret ABI for these two exports against the Sony module bundled with GTA Vice City (PPSA03530) — begin writes the element base, end one element past the last, both returning the iterator. #16056 implements both bodies in the canonical Vector<String>::begin/end form (CommonIterator*, const IntrusivePtr*), so the declarations here are subsumed if that lands first.

@TechLevelZero

Copy link
Copy Markdown
Contributor

✅ AMD GPU test: passed

PR head 69ffe68 merged onto main e772fb2 · tested 2026-11-05 13:56 UTC · 16 min

GPU AMD Radeon RX 6300 (RADV NAVI24), driver Mesa 25.2.8-0ubuntu0.24.04.4, Vulkan 1.4.318
Host Ubuntu 26.04.1 LTS, kernel 7.0.0-42-generic, container ubuntu:24.04
Logs build and test logs for this run
Build ✅ passed (Release, -DANYPS5_ENABLE_SPIRV_TOOLS=ON)
Tests ✅ 1,328 passed · 7 failed (0 new, 5 also fail on main, 2 unstable on main) · 7 skipped (ANYPS5_REQUIRE_VULKAN=1)
Test setup CPU-only tests without a GPU; GPU tests (and any CPU-only test that failed or skipped without one) once on the GPU with VK_LAYER_KHRONOS_validation and ANYPS5_NO_SHADER_CACHE=1; main ran the same way
Test selection 1,342 of 4,074 tests run: the ones this change can reach, chosen by tools/select_tests.py from main's build graph
Vulkan validation ✅ no new validation IDs vs main (642 tests run on the GPU)
Slow tests (>30 s, GPU tests timed with validation layers) ⚠️ agc_driver_pipeline_journal 120.5 s, ⚠️ public_headers 83.8 s, ⚠️ agc_driver_image_sample_unnormalized 41.7 s, ⚠️ agc_driver_sample_multisample_target 35.5 s, ⚠️ agc_driver_lazy_resolved_abi 34.6 s
Unstable on main (2, these also failed in recent main runs on this machine; not counted against this PR)
agc_driver_pipeline_journal: failed in 9 of the last 10 main runs
agc_driver_two_sample_alpha_to_coverage_sample_mask_export: failed in 5 of the last 10 main runs
Failing on main too (5, not counted against this PR)
agc_driver_alpha_to_coverage_sample_mask_export
agc_driver_image_table
agc_driver_native_depth_bda_clear
agc_driver_native_multisample_depth
agc_driver_submit_progress
Validation IDs already present on main (4 IDs in 554 tests)
VUID-VkDeviceCreateInfo-pNext-pNext (554 tests): agc_driver_address_draw_space_reuse, agc_driver_address_passes, agc_driver_alias_copy_label, agc_driver_alpha_to_coverage, agc_driver_alpha_to_coverage_sample_mask_export, agc_driver_array_layer_target, agc_driver_bc_block_copy, agc_driver_bda_anchor_reads, and 546 more
VUID-vkCmdCopyImage-srcImage-01548 (4 tests): agc_driver_depth_clear_hle, agc_driver_depth_storage_dispatch, agc_driver_depth_surface_from_storage, agc_driver_dispatch_move_depth
VUID-vkCmdCopyImage-srcImage-01551 (4 tests): agc_driver_depth_clear_hle, agc_driver_depth_storage_dispatch, agc_driver_depth_surface_from_storage, agc_driver_dispatch_move_depth
VUID-VkPipelineColorBlendStateCreateInfo-pAttachments-00605 (1 test): agc_driver_prepared_exports
Skipped (7)
agc_driver_copied_writer_label_order: skipped, the device reports a minStorageBufferOffsetAlignment of 4, so the dispatch binds its record at +0x1004 in place and nothing is written back; the test needs an alignment above 4
agc_driver_float16_unused_nan_payloads: skipped, the device has no native f16 conversions with RTE rounding and kept f16 denormals
agc_driver_gcn_display_present: skipped, no display: Vulkan support is either not configured in SDL or not available in current SDL video driver (offscreen) or platform
agc_driver_linear_target_direct_write_completion: skipped, the device does not keep host imports write-watched, so no completion stamp over the target is tracked
agc_driver_ngg_vertex_wave_reduction: skipped, s3 describes a 32-lane wave only on 32-lane subgroups, and the device has 64 lanes
agc_driver_recorded_draw_staging: skipped, the device does not keep host imports write-watched, so the target is never resident
agc_driver_vertex_elected_atomic_max: skipped, the device runs vertex programs in subgroups, so they keep their cross-lane reductions
Reproduce
git fetch origin 69ffe68b60e3e51ca98b2fbe07a8fbf5f31ac328 e772fb25cc72ee5faf77847f06bbc69533016e82
git checkout e772fb25cc72ee5faf77847f06bbc69533016e82 && git merge --no-ff 69ffe68b60e3e51ca98b2fbe07a8fbf5f31ac328
git submodule update --init --recursive
cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTING=ON -DANYPS5_ENABLE_SPIRV_TOOLS=ON
cmake --build build && cmake --build build --target libs
VK_DRIVER_FILES=/nonexistent/icd.json ctest --test-dir build -E '^(agc|shader|recompiler|videoout|video_out|guest_video_out|vulkan)|gpu' --timeout 120 --parallel 12 --output-on-failure
ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1 VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation \
  ctest --test-dir build -R '^(agc|shader|recompiler|videoout|video_out|guest_video_out|vulkan)|gpu' --timeout 120 --parallel 16 --output-on-failure
# failures are then re-run alone: --timeout 360 --parallel 2; new validation IDs: the test alone, twice, --parallel 1

Automated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Nov 5, 2026
@JJenkx

JJenkx commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by merged #16056 (da44e6b73d20292f1cc226962f48ac186a3fb3b8). I verified current main 151a2cb6e274f594a09c8a2ca42f44af2e5c40d8: both DataStatus vector iterator exports now declare the explicit output destination followed by the vector object, using CommonIterator* and const IntrusivePtr*, and both bodies are implemented. This covers the ABI correction proposed here; rebasing the old refusing bodies would regress those implementations. Thank you @akkadbakkad89 for the module check and implementation.

The historical SDK-layout observations in this PR remain evidence at their original pins; this closure does not claim all SDK versions share one layout or add console verification. Closing as superseded, rather than parking for a slot. AI-assisted: yes.

@JJenkx JJenkx closed this Nov 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs needs rebase Conflicts with main, rebase needed prx

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants