Skip to content

feat(libkernel): implement dup and dup2 - #2478

Merged
oneandonlydean merged 3 commits into
boykopovar:mainfrom
itsbo5:feat/libkernel-dup
Oct 10, 2026
Merged

oneandonlydean merged 3 commits into
boykopovar:mainfrom
itsbo5:feat/libkernel-dup

Conversation

@itsbo5

@itsbo5 itsbo5 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

What

Implement dup and dup2 in libkernel.

Tested

Full build with BUILD_TESTING=ON:

  • GuestDup tests pass: pipe duplicate outlives original, dup2 target replacement, invalid descriptors, and socket duplication.
  • tools/check_conventions.py --base upstream/main passed.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR
  • No comments in code except technical debt
  • Generic behaviour, not specific to one title
  • Depends on: none
  • AI-assisted: yes

@adriadam10

Copy link
Copy Markdown
Collaborator

Checked: rebased on current main, full build and ctest green (guest_dup passes, and fails to link without the change). Error returns match FreeBSD kern_dup; unmodelled combinations throw.

Added as our own commit on your branch:

  • TechnicalDebt line (Functional) for the cases dup/dup2 throw on, the shared guest-socket object and the descriptor numbering.

Not verified here: Windows runtime behaviour of _dup/_dup2 (code review only).

Merging once CI is green.

@github-actions github-actions Bot added the intake paused Closed while PR intake is limited to authors with at least 10 PRs merged into main label Oct 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

Thank you for your interest in the project! We are sorry, but we cannot accept more PRs while we work through the existing backlog. This PR has been automatically closed and labeled "intake paused". This is not a judgment of the quality of your changes.

@github-actions github-actions Bot closed this Oct 9, 2026
@oneandonlydean oneandonlydean reopened this Oct 9, 2026
@oneandonlydean oneandonlydean removed the intake paused Closed while PR intake is limited to authors with at least 10 PRs merged into main label Oct 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Progress report (197ffac → df131b9)

📈 System libraries: 84.56% (+0.01%, +2 implemented functions)

✅ 2 implemented
Library Name
libkernel dup2_nid_postfix
libkernel dup_nid_postfix

@babyhuey

Copy link
Copy Markdown
Contributor

Windows runtime check, since the review above left it at code review: this PR (5234b19) merged locally onto main c9da0f9 (clean), Windows 11, MinGW-w64 GCC from WinLibs, Release.

  • guest_dup passes 5/5 (--repeat until-fail:5). This runs the _dup, _dup2 and _get_osfhandle paths; no CRT assertion dialog or abort.
  • Prospero Win (PPSA99995) gets past dup2. On main it stops at FAIL: unresolved ELF import wdUufa9g-D8 (dup2) from libkernel.prx; with this PR it stops at FAIL: unresolved ELF import D8cuU4d72xM from libSceSysmodule.prx (sceSysmoduleGetModuleHandleInternal).
  • XPSemu (PPSA97358) stops at qlWiRfOJx1A (feat(libs): implement C runtime re-exports for libSceAmpr #545) on this main before it reaches dup, with or without this PR.

Linux (Ubuntu 24.04, GCC 13) on the same merge gives the same: guest_dup 5/5, Prospero Win undefined symbol: D8cuU4d72xM, XPSemu undefined symbol: qlWiRfOJx1A.

@TheAggie TheAggie mentioned this pull request Oct 10, 2026
9 tasks done
@popcoder565

Copy link
Copy Markdown
Contributor

Native Windows run, since no Windows results were posted. Setup: Windows 10 (19045), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release. Head 5234b196, alone on main afb8cd2.

No title was run.

🤖 Addressed by Claude Code

@sandexzx

sandexzx commented Oct 10, 2026 •

Copy link
Copy Markdown

Review: dup/dup2 do not maintain the random-device registration invariant

/dev/random and /dev/urandom are emulated: OpenRandomDevice() opens the host /dev/null and records the fd in g_descriptors (core/libs/prx/libkernel/File/src/RandomDevice.cpp:30), and reads are intercepted by fd number (RandomDevice.cpp:49). The invariant is: every host fd backing an emulated random device is present in g_descriptors, and is removed on close/replace.

dup_nid_postfix and dup2_nid_postfix create fd aliases but do not update that set, so both break the invariant. Reproduced locally on Linux (lavapipe, GCC, Release) by exercising the exported functions of the built libkernel.prx.

Bug 1 — dup of a random-device fd returns EOF

core/libs/prx/libkernel/File/src/Stdio.cpp:311-316: after a successful ::dup(d), the duplicate is never added to g_descriptors. It points at the host /dev/null, so ReadRandomDevice does not intercept reads and they return EOF.

random = open_nid_postfix("/dev/urandom", 0, 0);   // random=6
duplicate = dup_nid_postfix(random);               // duplicate=7
read_nid_postfix(random,    buf, 16);              // 16  (ok)
read_nid_postfix(duplicate, buf, 16);              // 0   (expected 16)

Bug 2 — dup2 leaves a stale registration on the replaced target

core/libs/prx/libkernel/File/src/Stdio.cpp:338-339: ::dup2(from, to) closes the old to, but ForgetRandomDevice(to) is not called. If to was a random-device fd, IsRandomDevice(to) stays true and subsequent reads on to return random bytes instead of the new object's content — a silent data corruption. The reverse case (from is a random device) is the mirror of Bug 1: the new target is never registered.

pipe_nid_postfix(pipe);
write_nid_postfix(pipe[1], "hello", 5);
random = open_nid_postfix("/dev/urandom", 0, 0);
dup2_nid_postfix(pipe[0], random);
read_nid_postfix(random, buf, 5);   // 5 random bytes (observed 0x03aeaeacaa), not "hello"
                                    // "hello" is never drained from the pipe

Suggested fix (scoped to this PR)

  1. Add void RememberRandomDevice(int fd) (register an existing fd) next to OpenRandomDevice in RandomDevice.hpp/.cpp.
  2. In dup_nid_postfix, after the success/range checks: if (File::IsRandomDevice(d)) File::RememberRandomDevice(duplicate);.
  3. In dup2_nid_postfix, snapshot const bool sourceIsRandom = File::IsRandomDevice(from); before the native call, and only on success: File::ForgetRandomDevice(to); if (sourceIsRandom) File::RememberRandomDevice(to);. Keep the target untouched on failure, and keep the from == to no-op from losing the registration.
  4. Windows branch has the same defect and needs the same fix (there is no random-device bookkeeping at all in the _dup/_dup2 paths), plus File::ForgetFileLock(to) next to the existing ForgetDirectoryDescriptor(to) (Stdio.cpp:335) — the current code leaves a stale entry in g_locks, and File::Flock returns success for it without actually locking the new file (File/src/FileLock.cpp:53-54).
  5. core/libs/tests/GuestDup.cpp does not cover random devices at all — please add scenarios for dup(random), dup2(pipe_read, random_fd), dup2(random, host_fd), dup2(random, random), and the error/from == to cases.

Out of scope (separate issues, not blockers for this PR)

  • fdopen/fclose bypass ForgetRandomDevice: libc/src/FileAndHeap.cpp:47-66 wraps the fd with native fdopen without duplicating, and FileStream::Close() calls std::fclose directly (libc/include/FileStream.hpp:102-104); freopen likewise (FileStream.hpp:69-73). Reproduced: after fclose, a reused fd number still reads as random. Pre-existing.
  • ReadRandomIovecs (File/src/Stdio.cpp:618) ignores the false return of ReadRandomDevice and still adds the length.
  • There is no shared synchronization of the fd lifecycle (native fd + its metadata): Remember/Forget only protect the std::set, so a concurrent reader can still observe the target as random between the native dup2 and the bookkeeping. The fix above does not address this; flagging it as a known limitation.

@github-actions

github-actions Bot commented Oct 10, 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
#2649 dup2_nid_postfix
dup_nid_postfix
core/libs/tests/GuestDup.cpp

@itsbo5
itsbo5 force-pushed the feat/libkernel-dup branch from 5234b19 to df131b9 Compare October 10, 2026 11:54
@itsbo5

itsbo5 commented Oct 10, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the review feedback in a new commit and rebased onto latest main:

  • Added File::RememberRandomDevice(fd) in RandomDevice.hpp / RandomDevice.cpp.
  • In dup_nid_postfix, registered duplicate fds backing random devices via File::RememberRandomDevice(duplicate).
  • In dup2_nid_postfix, snapshotted whether source is random prior to calling native dup2; on success cleaned up stale registrations via File::ForgetRandomDevice(to) and File::ForgetFileLock(to) (Windows), and re-registered the target when the source was random.
  • Handled the from == to case without dropping registrations.
  • Added comprehensive coverage in core/libs/tests/GuestDup.cpp exercising dup(random), dup2(random, random), dup2(pipe, random), dup2(random, host_fd), and error handling without mutating the target.

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 10, 2026
@babyhuey

Copy link
Copy Markdown
Contributor

Rechecked the new head (df131b9, with the random device bookkeeping change, based on main 197ffac), Linux (Ubuntu 24.04, GCC 13) and Windows 11 (MinGW-w64 GCC 15.2 from WinLibs), Release.

Title main with this PR
XPSemu (PPSA97358) iiQjzvfWDq0 (dup) G-MYv5erXaU (sceKernelGetAppInfo)
OpenRCT2 Native Port (PPSA99728) wdUufa9g-D8 (dup2) 0NJDcQ2B8vE (execvp)
Prospero Win (PPSA99995) wdUufa9g-D8 (dup2) D8cuU4d72xM (sceSysmoduleGetModuleHandleInternal)

Example lines for OpenRCT2: main Windows FAIL: unresolved ELF import wdUufa9g-D8 (dup2) from libkernel.prx; with this PR, Linux app.elf: symbol lookup error: app.elf: undefined symbol: 0NJDcQ2B8vE, Windows FAIL: unresolved ELF import 0NJDcQ2B8vE (execvp) from libkernel.prx.

@TheAggie

Copy link
Copy Markdown
Contributor

Two more titles from a triage of the whole homebrew catalog on main cf8b129 (Windows 11, MinGW-w64 GCC 15.2.0; each release ZIP checked against its catalog sha256, relinked with relinker --windows --registry, run for 12 s). Both stop at an import this PR adds, and both move on with it:

Title main with this PR
XPSemu (PPSA97358) dup G-MYv5erXaU (sceKernelGetAppInfo)
OpenRCT2 Native Port (PPSA99728) wdUufa9g-D8 (dup2) 0NJDcQ2B8vE (execvp)

The "with this PR" column comes from one build of main with this PR plus #1872, #2765 and #2024, which add unrelated imports (pthread_set_name_np, alarm, utime).

AI-assisted (Claude Code).

Sanjaysrinivas added a commit to Sanjaysrinivas/AnyPS5 that referenced this pull request Oct 10, 2026
…ion point

boykopovar#2478 adds its own entry after "Thread cancellation", the same anchor this
entry used. Two pull requests inserting different lines at one position
conflict when either reaches main first, which the overlap report on this
pull request flagged.

The entry moves to the end of the same "### Functional" section, where it is
145 lines from boykopovar#2478's insertion, so the two no longer share a hunk and will
merge in either order. The text is unchanged, and this also keeps the entry
away from the other entries in the section, as the review asked.
@Amoenus

Amoenus commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

Independent Windows validation on Sonic Unleashed Recompiled (PPSA99902),
content version 01.003.000.

I replayed the exact delta from head df131b9
onto clean main b5f5362. Local validation
commit: 7d5f45351e995fb8a0b3c4b228e84841a104ee54.
Windows 11 Enterprise build 26200; WinLibs MinGW-w64 GCC 15.2.0 r7;
Release, BUILD_TESTING=ON.

  • Built libs and guest_dup_tests, plus the nearby filesystem/socket-pair tests.
  • guest_dup, guest_filesystem and guest_socket_pair each passed 5/5
    (ctest --repeat until-fail:5).
  • check_conventions.py passed against the recorded baseline.
  • With the baseline relinker and title's unwrapped bundled libc, main stops at:
    FAIL: unresolved ELF import iiQjzvfWDq0 from libkernel.prx (dup).
  • With this patch, startup gets past that import and stops at:
    FAIL: unresolved ELF import cjt-4ySmYTs from libkernel_sys.prx (fork).
    Both default conversion and unused-filter=2 produce the same respective stop.
    All runs exit 0xC0000139 during import resolution, before guest entry.

Important control: the bundled libc contains a zero-return export shim. With
#2705's shim-rejection relinker, the first stop is hGljHZEfF0U (flockfile), both
with and without this dup/dup2 patch. The dup-only result therefore validates a
loader dependency; it is not evidence that guest code executed dup or that the
whole title starts.

No full ctest suite, Linux run, console behavior check or gameplay is claimed.
AI-assisted investigation; results reproduced locally. Logs available for review.

This was referenced Oct 10, 2026
@oneandonlydean
oneandonlydean merged commit 9c5d67f into boykopovar:main Oct 10, 2026
14 checks passed
Sanjaysrinivas added a commit to Sanjaysrinivas/AnyPS5 that referenced this pull request Oct 10, 2026
…ion point

entry used. Two pull requests inserting different lines at one position
conflict when either reaches main first, which the overlap report on this
pull request flagged.

The entry moves to the end of the same "### Functional" section, where it is
145 lines from boykopovar#2478's insertion, so the two no longer share a hunk and will
merge in either order. The text is unchanged, and this also keeps the entry
away from the other entries in the section, as the review asked.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants