Repository navigation
Conversation
GuestImageReader rejects standard dynamic tags 15–19 and 22; add an invalid-case harness entry for tag 15 using the existing error text.
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed — looks good
The added case checks DT_RPATH rejection for both plain ELF and SCE fixtures; the Linux relinker suite passed on the reviewed head.
What I ran
Reviewed head bcf68c12ccab9185e50a9d058f95fdb0375831e0, against base e2df06e661f94d6e63b8f6622c8c58507b0245c1; the current-main merge preview was clean. Local environment: Debian GNU/Linux x86_64, Python 3.11.2, Git 2.39.5. No local compiler or build was run.
git diff --check e2df06e661f94d6e63b8f6622c8c58507b0245c1...bcf68c12ccab9185e50a9d058f95fdb0375831e0 -- core/relinker/relinker/tests/test_guest_none_relocations.py
python3 -m py_compile core/relinker/relinker/tests/test_guest_none_relocations.py
python3 tools/check_test_registration.py --scripts
python3 tools/pr_stack_check.py 13727 --repo boykopovar/AnyPS5 --output-dir /tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-13727-bcf68c12ccab/stack
gh pr checks 13727 --repo boykopovar/AnyPS5The repository precheck was also attempted; its isolated copy lacked the workspace-only test-quality helper, so I ran that heuristic directly against the changed test file. Hosted CI is distinguished below from local checks.
Results
| Check | Result |
|---|---|
| Diff whitespace | Passed |
| Python syntax | Passed |
| Test registration | Passed |
| Test-quality heuristic | No findings (heuristic only) |
| Current-main merge preview | Clean |
| Hosted relinker tests | Ubuntu, Windows, and ASAN passed; two macOS jobs still pending |
| Hosted conventions, intake, eligibility, labels | Passed |
| Full product Linux/Windows and specialized jobs | Skipped by workflow filters |
| Repository precheck wrapper | Incomplete because its isolated copy lacked the local helper; relevant checks were run individually |
Raw command output
# git diff --check ...
[no output; exit status 0]
# python3 -m py_compile ...
[no output; exit status 0]
# python3 tools/check_test_registration.py --scripts
230 test script(s), each run by an add_test or a workflow
# python3 tools/pr_stack_check.py ...
/tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-13727-bcf68c12ccab/stack/stack-report.md
/tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-13727-bcf68c12ccab/stack/stack-report.json
# test-quality heuristic JSON
{"findings": [], "negative_control_candidates": [], "limits": ["Heuristics can miss weak assertions and can flag valid tests; inspect every finding.", "A test that passes against base is not by itself proof that the PR test is inadequate.", "Negative controls are only candidates until the test actually runs against base and fails by assertion rather than import/setup error."]}
# Hosted CI snapshot for head bcf68c12ccab9185e50a9d058f95fdb0375831e0
relinker (ubuntu-latest, g++) pass
relinker (windows-latest, g++) pass
relinker-asan pass
conventions pass
intake pass
eligibility pass
labels pass
relinker (macos-latest, clang++) pending
relinker (macos-latest, clang++, x86_64) pending
linux skipped; windows skipped; register-context skipped; write-watch skipped
# Repository precheck helper error
/usr/bin/python3: can't open file '.../tools/pr_test_quality.py': [Errno 2] No such file or directory
Sources
The new invalid case is at test_guest_none_relocations.py:153, and the harness checks the rejection code, diagnostic, and absence of output artifacts at lines 218–221.
Not verified
The two hosted macOS relinker jobs were still pending at final refresh. I did not run the relinker executable locally or perform a negative-control mutation. Full product and specialized CI jobs were skipped.
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed — changes requested
The DT_RPATH regression case is appropriately narrow, but its duplicated fixture helper and shared test-file changes need an agreed landing order.
What I ran
Reviewed head bcf68c12ccab9185e50a9d058f95fdb0375831e0 against PR base e2df06e661f94d6e63b8f6622c8c58507b0245c1; the merge preview onto current main 6475267b456de6a0a23b230aa751f0b917d12688 was clean. Environment: Debian Linux x86_64, Python 3.11.2, GCC 12.2.0. I did not build or run CTest locally.
python3 tools/pr_precheck.py --repo boykopovar/AnyPS5 --pr 13727
python3 tools/pr_test_quality.py --root /tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-13727-bcf68c12ccab/head-bcf68c12ccab-f4170a17 --json core/relinker/relinker/tests/test_guest_none_relocations.py
gh pr checks 13727 --repo boykopovar/AnyPS5Results
| Check | Result |
|---|---|
| Diff check | Passed. |
| Conventions and PR template | Passed. |
| Python syntax | Passed. |
| Python test registration | Passed; repository reports all 230 test scripts are registered. |
| Test-quality heuristic | Isolated precheck could not find this workspace-only helper; rerun on the exact-head checkout produced no findings. |
| Merge preview | Clean on current main at 6475267b456de6a0a23b230aa751f0b917d12688. |
Hosted conventions, eligibility, intake, and labels |
Passed. |
| Hosted relinker Ubuntu, Windows, and ASAN | Passed. |
| Hosted relinker macOS ARM64 and x86_64 | Pending. |
| Hosted Linux, Windows product, register-context, and write-watch jobs | Skipped for this PR. |
| Author-reported CTest | Description reports the focused case and 526/526 tests passing on main 0cc02f7cde; I did not independently run them, and the exact tested PR head is not stated. |
| Local build and CTest | Not run. |
Unedited output excerpts
Conventions check:
All conventions checks passed.
Python test registration:
230 test script(s), each run by an add_test or a workflow
Test-quality helper output on the exact-head checkout:
{
"findings": [],
"negative_control_candidates": [],
"limits": [
"Heuristics can miss weak assertions and can flag valid tests; inspect every finding.",
"A test that passes against base is not by itself proof that the PR test is inadequate.",
"Negative controls are only candidates until the test actually runs against base and fails by assertion rather than import/setup error."
]
}Latest relevant gh pr checks output:
linux skipping 0 https://github.com/boykopovar/AnyPS5/actions/runs/41161917882/job/124421226104
register-context skipping 0 https://github.com/boykopovar/AnyPS5/actions/runs/41161917882/job/124421226682
intake pass 5s https://github.com/boykopovar/AnyPS5/actions/runs/41161917806/job/124421116037
labels pass 9s https://github.com/boykopovar/AnyPS5/actions/runs/41161917794/job/124421115614
relinker (ubuntu-latest, g++) pass 2m36s https://github.com/boykopovar/AnyPS5/actions/runs/41161917882/job/124421225126
relinker (windows-latest, g++) pass 5m29s https://github.com/boykopovar/AnyPS5/actions/runs/41161917882/job/124421225332
relinker-asan pass 2m43s https://github.com/boykopovar/AnyPS5/actions/runs/41161917882/job/124421225107
windows skipping 0 https://github.com/boykopovar/AnyPS5/actions/runs/41161917882/job/124421226380
relinker (macos-latest, clang++) pending 0 https://github.com/boykopovar/AnyPS5/actions/runs/41161917882/job/124421225123
relinker (macos-latest, clang++, x86_64) pending 0 https://github.com/boykopovar/AnyPS5/actions/runs/41161917882/job/124421225117
write-watch skipping 0 https://github.com/boykopovar/AnyPS5/actions/runs/41161917882/job/124421225858
conventions pass 14s https://github.com/boykopovar/AnyPS5/actions/runs/41161917894/job/124421115472
eligibility pass 5s https://github.com/boykopovar/AnyPS5/actions/runs/41161917882/job/124421114826
Negative control
| Assertion | Change to code under test | Did the test fail? |
|---|---|---|
DT_RPATH must produce the unsupported-tag error for ELF and SCE fixtures |
No negative-control mutation or exact-head relinker binary was available without building. | Not exercised locally. |
Findings
Blocking
The PR description identifies merge-order overlap with open #13711, #13714, #13717, #13721, and #13723 while the checklist says Depends on: none. Diff inspection also found that #13723 and this PR add identical insert_dynamic_tag helpers; open #13729 adds the same helper as well. The repository requires agreement on the order of overlapping changes and recording it with Depends on. Please coordinate with the affected authors, keep one shared helper, and update the checklist to show the agreed order.
Non-blocking
The description does not state the exact PR head SHA exercised by its focused and full CTest runs. Please add that SHA so those results can be tied to this revision.
Sources
- Overlap-order rule on current
main - PR #13727 test case at the reviewed head
- PR #13723 shared fixture helper
- PR #13729 shared fixture helper
- PR #13711 adjacent invalid-case change
- PR #13714 adjacent invalid-case change
- PR #13717 fixture and invalid-case changes
- PR #13721 adjacent invalid-case change
Not verified
No relinker build, focused CTest, full CTest, or negative-control mutation was run locally. The exact head tested by the author is unspecified. Hosted macOS relinker checks were pending; platform product checks were skipped.
To get this to a good state
Please coordinate and record the merge order, retain a single shared fixture helper, and add the tested head SHA. I will rerun the relevant checks on the updated head.
|
Consolidated into #14079, which adds all of these guest_none_relocations invalid cases in one change so the test file has a single open pull request. Closing this per-case pull request in favour of that one. |
What
GuestImageReaderrejects several legacy standard dynamic tags (15–19 and 22) withUnsupported dynamic tag <n>. PR #13723 pinsDT_FLAGS/ tag 30; this adds one invalid-case harness entry for tag 15 (DT_RPATH) for both plain ELF and SCE-tagged fixtures, using the existing reader message.Tested
macOS arm64, upstream/main
0cc02f7cde, Python 3.13, relinker-only Release-DANYPS5_RELINKER_ONLY=ON -DBUILD_TESTING=ON:ctest -R guest_none_relocations: pass (newunsupported-dynamic-tag-rpathforsce=False/True).ctest -j 8 -E vulkan_validation_gate --timeout 30: 526/526.python3 tools/check_conventions.py --base upstream/main: passed.python3 tools/pr_overlap.py --branch test/unsupported-guest-dynamic-tag-15: no overlap with open PRs (merge-order with test(relinker): pin invalid guest lifecycle function relocation #13711/test(relinker): pin unaligned guest lifecycle array size rejection #13714/test(relinker): pin guest preinit array without module initializer #13717/test(relinker): pin invalid guest symbol relocation rejection #13721/test(relinker): pin unsupported guest dynamic flags rejection #13723 on the same test file).Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)