Skip to content

fix(libc): keep the caller errno when localtime cannot represent a time - #4015

Merged
adriadam10 merged 2 commits into
boykopovar:mainfrom
sherySJ:agent/libc-time-eoverflow
Oct 13, 2026
Merged

adriadam10 merged 2 commits into
boykopovar:mainfrom
sherySJ:agent/libc-time-eoverflow

Conversation

@sherySJ

@sherySJ sherySJ commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

What

When a time is outside the range struct tm can hold, localtime and localtime_s return NULL and leave the host conversion's errno in the guest's errno. On Linux that is glibc's EOVERFLOW, which is 75, and 75 is EPROGUNAVAIL in the guest's FreeBSD numbering. On Windows it is the CRT's EINVAL.

The console's FreeBSD 11 tzcode (contrib/tzcode/stdtime/localtime.c on stable/11) returns NULL from timesub on overflow without touching errno, as gmtime and gmtime_s already do here. localtime_s now saves the caller's errno before the host conversion and restores it when the conversion fails. GuestTimeConversion.cpp sets errno to a sentinel and checks that gmtime, gmtime_s, localtime and localtime_s keep it at INT64_MIN and INT64_MAX. The existing ctime check, which expects EINVAL for an unrepresentable time, is unchanged and still passes.

Tested

  • Linux (Ubuntu 24.04, GCC, Release) on main 43e3a70: full build passed. ctest -j2 passed 994 of 994; the GPU and display tests skip on this headless machine.
  • Negative control: with this PR's test against main's Time.cpp, guest_time_conversion aborts with "localtime_s of an unrepresentable time changed errno". With the change it passes.
  • Windows: not tested on this head. The Windows change is the same restore, in the localtime_s failure branch.
  • python3 tools/check_conventions.py --base upstream/main and git diff --check passed.

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 in TechnicalDebt
  • 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
  • AI-assisted: yes (Claude Code)

@github-actions github-actions Bot added the libc label Oct 11, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

What is the 84 based on for the console? FreeBSD's timesub sets EOVERFLOW only in the current tzcode; in stable/11 (contrib/tzcode/stdtime/localtime.c) it returns NULL and leaves errno alone, which is what gmtime does on main. The host errno leaking out of localtime and localtime_s is a real bug; keeping the guest's errno unchanged there fixes it without a new value.

@xXPBSXx

xXPBSXx commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Windows test of head 480382d66 merged onto current main (d592d54e1, clean merge; built together with two other open PRs, so the run is for the combination): Windows 10 (19045), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release, full build 0 errors, -j2.

guest_time_conversion (the test this PR changes) passes. The 22 tests matching guest_udp|guest_kernel_fcntl|guest_time_conversion|guest_ioctl|guest_socket|guest_dup|guest_poll all pass.

Not checked: that the test fails without the change, Linux, a title run.

AI-assisted (Claude Code).

@sherySJ
sherySJ force-pushed the agent/libc-time-eoverflow branch from 480382d to 23d5da2 Compare October 12, 2026 23:45
@sherySJ sherySJ changed the title fix(libc): set EOVERFLOW when gmtime or localtime cannot represent a time fix(libc): keep the caller errno when localtime cannot represent a time Oct 12, 2026
@sherySJ

sherySJ commented Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

You're right, thanks. I checked stable/11's contrib/tzcode/stdtime/localtime.c: timesub returns NULL on overflow and never touches errno, and the 84 came from current tzcode. I've force-pushed the change you suggested:

  • localtime and localtime_s now save the caller's errno before the host conversion and restore it when the host conversion fails, so neither Linux's 75 nor the Windows CRT's EINVAL reaches the guest.
  • gmtime and gmtime_s are unchanged.
  • The test now sets errno to a sentinel and checks that all four exports keep it for INT64_MIN and INT64_MAX. Against main, it fails at localtime_s.

I've updated the PR title and description to match. @xXPBSXx, thanks for the Windows run. That run was of the previous head, so it doesn't cover this one.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

The new head removes the four CheckAsctime rows from GuestTimeConversion.cpp (lines 277-283 on main); please put them back. The errno change itself is right now.

@sherySJ
sherySJ force-pushed the agent/libc-time-eoverflow branch from 23d5da2 to f656da3 Compare October 13, 2026 04:36
@sherySJ

sherySJ commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

Restored the four CheckAsctime rows in GuestTimeConversion.cpp (they were dropped by mistake when I reworked the branch); force-pushed as f656da3. The diff to that file is now only the errno checks. Tested on Ubuntu 24.04 x86-64: cmake --build build --target guest_time_conversion_tests, ctest --test-dir build -R guest_time_conversion (passed), python3 tools/check_conventions.py --base upstream/main (passed).

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 13, 2026
@sherySJ

sherySJ commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (the merged Windows pre-1970/post-3000 localtime fix in Time.cpp conflicted); force-pushed as 4aede94. The failing localtime_s and localtime_r paths still restore the caller's errno, and the new EINVAL on tm_year overflow is left as main has it. Tested on Ubuntu 24.04 x86-64: cmake --build build --target guest_time_conversion_tests, ctest --test-dir build -R guest_time_conversion (passed), python3 tools/check_conventions.py --base upstream/main (passed). Not built or run on Windows.

@sherySJ
sherySJ force-pushed the agent/libc-time-eoverflow branch from f656da3 to 4aede94 Compare October 13, 2026 07:37
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 13, 2026
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the diff, guest_time_conversion and the 90 libs/libc tests on Linux; the new errno checks fail without the Time.cpp change. localtime goes through localtime_s, so both keep the caller's errno on failure. Merging once the full Build is green.

@adriadam10

Copy link
Copy Markdown
Collaborator

The full Windows run failed in guest_time_conversion: "localtime_s of an unrepresentable time changed errno". On Windows INT64_MIN/INT64_MAX are reduced by calendar cycles, so the host localtime_s succeeds and the tm_year overflow branch in localtime_s_nid_postfix set errno = EINVAL, which the new errno checks forbid.

Pushed a small commit that restores the caller's errno in that branch too (ctime sets its own EINVAL, so its check is unaffected). Linux build and the 92 libs/libc tests pass; the Windows result comes from the full run, merging once it is green.

@adriadam10
adriadam10 force-pushed the agent/libc-time-eoverflow branch from 4aede94 to a37ee5f Compare October 13, 2026 13:00
@adriadam10
adriadam10 merged commit c98fda8 into boykopovar:main Oct 13, 2026
12 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.

4 participants