Skip to content

fix(relinker): reject --frame-red-zone without --windows - #10912

Merged
oneandonlydean merged 1 commit into
boykopovar:mainfrom
ahmadmuj:fix/anyps5-frame-red-zone-requires-windows
Oct 26, 2026
Merged

oneandonlydean merged 1 commit into
boykopovar:mainfrom
ahmadmuj:fix/anyps5-frame-red-zone-requires-windows

Conversation

@ahmadmuj

Copy link
Copy Markdown
Contributor

What

--frame-red-zone only works for Windows output, but ParseArgs let it through without --windows. The only check was in Conversion::Convert, so the relinker exited 2 (conversion failed) instead of 1 (invalid arguments), and usually with an unrelated error first, since sce_module discovery runs before that check:

$ relinker --frame-red-zone in.elf out.elf
FAIL: sce_module/*.prx was not found in: ...     # exit 2

--windows-diagnostics, --windows-gui and --windows-fixed-base already fail in ParseArgs with "requires --windows". This adds the same check for --frame-red-zone:

FAIL: --frame-red-zone requires --windows        # exit 1

The Conversion.cpp guard stays for library callers. Same flag as #10682, different gap.

Tested

Ubuntu 24.04 on WSL2, GCC 13.3, relinker-only build. New frameRedZoneRequiresWindows in cli_args fails on main with expected an exception for [--frame-red-zone input.elf output.elf] and passes with the change. All 265 relinker-only tests pass. check_conventions.py --base origin/main passes. Ran the CLI before and after as shown above; --windows --frame-red-zone still gets to the framing pass.

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

@thepoppfish thepoppfish 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.

Reviewed exact head 1c3bb466a89e745441023a618e7144abd6440bcb. No findings in this focused change; this adds native Windows coverage to the author's Linux report.

Windows 11 build 26200, MSYS2 UCRT64 GCC 16.2.0-6, CMake 4.3.1, Ninja 1.13.2, Python 3.14.7:

  • Configured the repository's actual relinker-only Release build and built relinker and cli_args_tests (93 Ninja steps), without compiler warnings in the build log. No custom replacement for parser, path handling or conversion behavior.
  • The complete registered cli_args test passes three consecutive runs (0.04/0.01/0.01 s). A scratch control removing only the two-line guard, retaining the full submitted CliArgsTests.cpp and actual NativePath.cpp, exits 1 with expected an exception for [--frame-red-zone input.elf output.elf].
  • Seven registered tests pass: cli_help, input_magic, input_preservation, output_collisions, secondary_output_collisions, input_file, input_nonascii (37.71 s total, slowest individual test 14.39 s).
  • Direct invocations of the actual built relinker with nonexistent input and an existing output sentinel confirm four rejections: default target, --macos before/after the flag, and --to-rosetta. Each exits 1 with exactly FAIL: --frame-red-zone requires --windows, before input access; the sentinel survives. Both orders of --windows and --frame-red-zone parse successfully and reach the missing-input conversion failure (exit 2) with the sentinel intact. These accepted-argument checks are not a claim that a title was successfully framed.
  • Conventions and whitespace pass against local main b2c7a388406614fb3bc6723a6717a37196d617b8; merge-tree against that same main is clean.

The check is alongside the existing Windows-only option checks, after parsing all flags, so either accepted option order works. The Conversion::Convert guard remains available for library callers. The added unit test covers default/Linux and explicit macOS rejection plus Windows acceptance, without changing existing assertions.

Scope: the actual Windows CLI and focused tests above. I did not run all 265 registered tests, a full system-library/product build, Linux/macOS execution, sanitizer, title/GPU tests or cross-PR composition. AI-assisted: yes, OpenAI Codex.

Replay commands and standalone control

Put MinGW GCC, CMake and Ninja on PATH. On the exact checkout:

cmake -S . -B ../build-relinker-10912 -G Ninja -DCMAKE_BUILD_TYPE=Release -DANYPS5_RELINKER_ONLY=ON -DBUILD_TESTING=ON '-DCMAKE_CXX_COMPILER=g++.exe' '-DPython3_EXECUTABLE=C:/path/to/python.exe'
cmake --build ../build-relinker-10912 --target relinker cli_args_tests -j 3
ctest --test-dir ../build-relinker-10912 -R '^cli_args$' --output-on-failure --repeat until-fail:3
ctest --test-dir ../build-relinker-10912 -R '^(cli_help|input_magic|input_file|input_preservation|output_collisions|secondary_output_collisions|input_nonascii)$' --output-on-failure

For the standalone negative control, place the following files in ../review-10912-controls with the exact checkout at ../review-10912 relative to that directory (or adjust REPO and the script paths). Run python make_control.py, configure/build its CMake project, and run build16/cli_args_old.exe. It should fail at the new guard assertion above. smoke_cli.py runs the six direct invocations against the real built relinker; its temporary input/output directory cleans itself up.

CMakeLists.txt

cmake_minimum_required(VERSION 3.20)
project(AnyPS5CliArgsControl LANGUAGES CXX)
set(REPO "${CMAKE_CURRENT_LIST_DIR}/../review-10912" CACHE PATH "Exact PR checkout")
add_executable(cli_args_old
    "${REPO}/core/relinker/cli/tests/CliArgsTests.cpp"
    CliArgs-old.cpp
    "${REPO}/core/relinker/io/src/NativePath.cpp")
target_compile_features(cli_args_old PRIVATE cxx_std_20)
target_include_directories(cli_args_old PRIVATE
    "${REPO}/core/relinker/cli/include"
    "${REPO}/core/relinker/io/include"
    "${REPO}/core/relinker/domain/include")
target_link_options(cli_args_old PRIVATE -static -static-libgcc -static-libstdc++)

make_control.py

from pathlib import Path

root = Path(__file__).resolve().parent
source = (root.parent / 'review-10912/core/relinker/cli/src/CliArgs.cpp').read_text()
guard = '    if (args.frameRedZone && !args.toWindows)\n        throw std::runtime_error("--frame-red-zone requires --windows");\n'
assert source.count(guard) == 1
(root / 'CliArgs-old.cpp').write_text(source.replace(guard, ''))

smoke_cli.py

from pathlib import Path
import subprocess
import tempfile

root = Path(__file__).resolve().parent
tool = root.parent / 'build-relinker-10912/core/relinker/relinker.exe'
with tempfile.TemporaryDirectory() as directory:
    source = Path(directory) / 'missing.elf'
    output = Path(directory) / 'output.exe'
    original = b'existing output must survive'
    output.write_bytes(original)
    rejected = (
        ['--frame-red-zone'],
        ['--macos', '--frame-red-zone'],
        ['--frame-red-zone', '--macos'],
        ['--to-rosetta', '--frame-red-zone']
    )
    accepted = (
        ['--windows', '--frame-red-zone', '--skip-sce-module'],
        ['--frame-red-zone', '--windows', '--skip-sce-module']
    )
    for flags in rejected:
        result = subprocess.run([str(tool), *flags, str(source), str(output)],
                                capture_output=True, text=True, timeout=30)
        assert result.returncode == 1, (flags, result.returncode, result.stdout, result.stderr)
        assert result.stderr.strip() == 'FAIL: --frame-red-zone requires --windows', (flags, result.stderr)
        assert output.read_bytes() == original and not source.exists()
        print('argument rejection:', ' '.join(flags), 'exit 1, exact diagnostic, output preserved')
    for flags in accepted:
        result = subprocess.run([str(tool), *flags, str(source), str(output)],
                                capture_output=True, text=True, timeout=30)
        assert result.returncode == 2, (flags, result.returncode, result.stdout, result.stderr)
        assert '--frame-red-zone requires --windows' not in result.stderr
        assert output.read_bytes() == original and not source.exists()
        print('accepted arguments:', ' '.join(flags), 'reaches missing-input failure, exit 2, output preserved')

@ahmadmuj

Copy link
Copy Markdown
Contributor Author

Checked #9408 by hand since the bot couldn't compare it. In CliArgs.cpp it only removes the --trace-imports / --macos check in ParseArgs (lines 202-203 on current main). This PR only adds the --frame-red-zone / --windows check after --windows-diagnostics (lines 196-197). Applied both hunks to main acbfd7c in either order and ran git merge-file: clean, no conflicts, same result. No shared lines, so no Depends on either way.

@oneandonlydean oneandonlydean self-assigned this Oct 26, 2026
@oneandonlydean
oneandonlydean merged commit 37caf45 into boykopovar:main Oct 26, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants