Repository navigation
Conversation
rooklz
left a comment
rooklz
left a comment
There was a problem hiding this comment.
I ran this on macOS (the body has no macOS run). Head is a131f0f6a6, base 319621ba2e.
Setup. macOS 26.2, Apple clang, arm64 and x86_64 (Rosetta). Built with -O1 -fsanitize=address,undefined -fno-sanitize-recover=undefined and the target's -include HostStandardStreams.hpp. Standalone build of Time.cpp, Environment.cpp and Multibyte.cpp with GuestMktime.cpp, plus a one-line __error_nid_postfix → &errno shim.
- Head passes
guest_mktime3/3 on x86_64 and on arm64, with no sanitizer reports. The macOS-onlyLord_Howe/Moscowcases are included. - Head's tests with
main'sTime.cppfail (unavailable daylight hint changed the timestamp), as the body says for Linux and Windows. - Cost: a call with
tm_isdst=1takes about 4.2 µs instead of 3.4 µs onmain(-O2, 20k calls,UTC0/EST5EDT/America/New_York).tm_isdst<=0is unchanged.
Regression: tm_isdst>0 at the representable year edges now throws. The new block only checks utcCalendarFields(local). SonyHostDaylightAmount_nid_no_patch then asks localtime_r about times up to 11 × 2629746 s on each side, plus the zone offset. Close to the int tm_year limits those times are outside what localtime_r can represent, so SonyHostZoneAt_nid_no_patch throws runtime_error and the title aborts. main returns a value for all of these. I swept tm_year ∈ {INT_MIN, INT_MIN+1, INT_MAX}, every month, tm_isdst ∈ {1,0,-1}, in UTC0 and EST5EDT. Exactly 12 rows changed, all tm_isdst=1, and all of them went from a value to a throw:
tm_year=INT_MIN tm_mon=0..10 tm_isdst=1 main -67768040608487925 ... head THROW
tm_year=INT_MAX tm_mon=11 tm_isdst=1 main 67768036190247600 head THROW
(no host local time for 67768036192880946)
The debt line does say that a lookup that can't represent its candidates propagates. But before this PR, these inputs didn't do a lookup at all. A guard that only enters the block when the whole search window converts keeps main's result at the edges:
- GuestTm normalized{};
- if (utcCalendarFields(local, normalized)) {
+ GuestTm early{};
+ GuestTm late{};
+ constexpr std::int64_t searchSeconds = 12 * 2629746 + SECONDS_PER_DAY;
+ if (utcCalendarFields(local - searchSeconds, early) && utcCalendarFields(local + searchSeconds, late)) {With this change, head's guest_mktime stays green on both arches, and the 108-row edge sweep is byte-identical to main in both zones. This test kills the throw (head RED with the uncaught runtime_error, guarded version green):
@@ -96,6 +96,22 @@
#endif
+static void CheckRepresentableYearEdges(Converter convert) {
+ const struct { int year; int month; } cases[]{
+ {std::numeric_limits<int>::min(), 0},
+ {std::numeric_limits<int>::max(), 11}
+ };
+ for (const auto& item : cases) {
+ GuestTm input{};
+ input.tm_year = item.year;
+ input.tm_mon = item.month;
+ input.tm_mday = 15;
+ input.tm_hour = 12;
+ input.tm_isdst = 1;
+ Require(convert(&input) != -1, "daylight hint at a representable year edge failed");
+ }
+}
+
static void CheckFailure(Converter convert, int hint = -1) {
GuestTm input{};
input.tm_year = std::numeric_limits<int>::max();
@@ -123,6 +139,7 @@
}
CheckFailure(convert);
CheckFailure(convert, 1);
+ CheckRepresentableYearEdges(convert);
for (const int year : {40, 70, 124}) CheckUnavailableDaylightHint(convert, year);
}
}(I only ran the test on macOS. Please check it on your Linux and Windows hosts before taking it. Whether the console applies the hint at those years is unmeasured. The diff only restores the value main returned and doesn't claim it's correct.)
Mutants (arm64 ASan+UBSan, head tests):
- Killed: always clearing the hint (
available daylight hint was ignored), and entering only fortm_isdst == 1(the2/INT_MAXhints catch it). - Survived: dropping the errno restore. That's equivalent on macOS, since
localtime_rdoesn't set errno here. On Windows it may not be equivalent; I couldn't check. Usingguessinstead ofzone, or plainlocal, as the search origin also survived, as did a forward-only search and a 6-month search. None of the test zones puts the result within a month of a rule boundary. These are only test-strength notes and don't block, because the body's two-library comparison covers the rule behaviour.
Conventions and git merge-tree against 319621ba2e are clean.
|
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.
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
Thanks @adriadam10 for the contributed guard in Both hosts also completed the 216-row edge sweep. All 144 rows at the outer years The body now names your contributed commit and links the both-host raw evidence. The earlier transition-policy limitations and pending #10409 landing-order coordination remain explicit. |
|
Windows run of this head
AI-assisted: yes (Claude Code) |
|
A Windows run with GCC 16: Windows 11 Pro 10.0.22631 (host zone
AI-assisted: yes (Claude Code) |
adriadam10
left a comment
adriadam10
left a comment
There was a problem hiding this comment.
The full Build of this head merged onto main (run 41057229445) fails on windows: guest_mktime aborts (0xc0000409) with available daylight hint was ignored, the check at core/libs/tests/GuestMktime.cpp:83 in CheckAvailableDaylightHints (TZ=EST5EDT, hints 1/0/1 across January and July). linux is green and main does not fail this test.
The Windows runs reported in the thread used hosts in Eastern/Pacific zones; the CI runner's host zone is UTC. The new SonyHostZoneAt_nid_no_patch (SonyCrt.hpp) reads _get_timezone/_get_dstbias and localtime_s, and with a UTC system zone and TZ set from the guest those can give a different daylight amount than on your hosts, so the available-hint case is treated as unavailable (or the reverse).
Please reproduce on Windows with the host zone set to UTC, find which of the six conversions differs, and fix the helper so the seasonal cases of CheckAvailableDaylightHints pass there too. The failing case should be named in the test message so CI shows it.
|
Addressed with a normal fast-forward to Windows: prior production/new tests passes3/4 suites and fails the injected mktime matrix; fixed production and the final named-fixture rerun pass4/4 with zero skips. Linux:4/4 per arm, libc binary unchanged. Updated the description with exact heads, all six named conversions, logs/source diff/reproducer and the earlier diagnostic-label correction. The physical Windows host remains Eastern; this is a controlled zero-bias reproduction, not an actual UTC-host run. Upstream UTC CI is still needed to verify the original host condition. |
|
The new Windows job is green, but its log explicitly reports “No guest core/libs test sources changed”; it did not execute I traced the selection gap to MinGW’s actual |
|
The selector follow-up #13661 at Please rerun this head I also prepared a manual UTC qualification workflow on the fork, with prior production |
|
Windows run with the host's system zone actually set to UTC, as asked in the review. Windows 11 Enterprise 10.0.22621, Ryzen 9 5950X, WinLibs GCC 15.2.0 UCRT r7, Ninja, Release,
Results under the UTC system zone:
So on a real UTC-zone Windows host the AI-assisted: yes (Claude Code). |
|
@adriadam10 The requested real UTC-system-zone Windows reproduction is now recorded in the follow-up. The unchanged public head is Raw Windows receipts, source pins and logs, SHA-256 |
|
@adriadam10 Could you re-review? I'm now the primary owner of this PR. Where it stands at the unchanged head
A maintainer rerun of full CI on this head would replace that stale result and confirm the UTC runner too. 🤖 Generated with Claude Code |
What
When
tm_isdstis positive in a zone/year with no nearby daylight adjustment, the host CRT subtracts an hour that the two measured title-bundled libraries do not subtract. For example, 2024-01-15 12:00 inUTC0withtm_isdst=1should remain 12:00. Both mktime entrypoints now use automatic resolution when the shared host-zone query finds no available adjustment. Seasonal hints remain effective: an EST5EDT winter daylight hint still shifts an hour, and a summer standard hint still selects standard time.Share libkernel's existing zone lookup and nearby-daylight calculation through SonyCrt.hpp rather than creating a libc-to-libkernel link cycle or duplicating the rules. The kernel wrappers retain their calculation. Preserve the Buenos Aires historical-DST round trip, errno, normalization and failure behavior. Record remaining transition and host-range limitations in libc debt.
Fulfilled landing order: #12549 has merged, supplying its separate Windows nonordinary-calendar normalization correction. This branch contains this daylight fix and its Windows review follow-up on the original main baseline
319621ba2e578bd053b0baba2877fe70520e0737; it does not copy that PR's implementation. The private combined tree is tested below. No title unblock or FPS improvement is claimed.Tested
Windows review follow-up at
59d2b51c999b8e228bd51eefeffbfb55c4b3093c: a standalone native UCRT probe shows that a zero process_dstbiassurvives switching to an explicitEST5EDTzone. All six available-hint conversions then differ in timestamp or normalized hour. The shared Windows refresh now applies the explicit DST zone's one-hour adjustment before conversion; libc's outer zone scope uses it too, so standard-hint and local-time calls do not depend on an earlier positive-hint call. An unset hostTZretains the OS-derived adjustment. This follows the CRT's documented explicit-TZ US daylight rules.Eastern Standard Timeunchanged: prior PR production plus the new tests (1d527de967) passes 3/4 focused suites and failsguest_mktimeat all six injected-zero-bias rows. Fixed production9fd017708fpasses 4/4, zero skips. Final59d2b51c99corrects only diagnostic labels; rebuilding the fixture and rerunning all four suites passes, with production module hashes unchanged.1d527de967and9fd017708f: 4/4 suites per arm, zero skips; the resulting libc binaries are identical. Groups:guest_mktime,guest_time_conversion,guest_time_extras,guest_strftime.libkernel; it is actually libc'smktime_nid_postfixforwarding export. Final labels arelibc_mktimeandmktime; the archive explicitly maps the earlier negative log labels.Follow-up source diff, Windows negative/positive logs and JUnit, final-label rerun, Linux paired logs, and process-local CRT reproducer, SHA-256
46ad518ad715f2baadbcc56b0f95a2974b0f82c33b3844c0c26ffea3c7fc32fb(download verified). This is focused module/suite qualification, not a whole-project/game/GPU run. Actual UTC-OS-host validation remains pending: the local negative control injects the zero process bias while leaving the physical Windows timezone unchanged. The fork's hosted diagnostic could not run because Actions is disabled there. Upstream UTC CI is still needed; these local results are not described as a UTC-machine run.Year-edge follow-up at
7e8465539372e0c6615956deac04adfe5886b30e(contributed by @adriadam10): guard the complete optional daylight-search window before entering it. Linux and actual Windows each pass all six focused groups three times (18 executions per host). Priora131production with the new tests throws on Linux; the contributed guard removes that throw. Both-host 216-row boundary probes includeINT_MIN,INT_MIN+1,INT_MAX, all months and hints 1/0/-1 in UTC0/EST5EDT. Every tested outer-year row (INT_MIN/INT_MAX, 144 per host) matches main's conversion, without a throw. The full sweep is intentionally not described as identical: the interiorINT_MIN+1positive-hint cases retain the availability correction (12 changed rows per Linux zone; 12 in Windows UTC0 and zero in Windows EST5EDT). These are host-preservation checks, not a console edge oracle. All 21,965 Linux normal-range rows remain unchanged from the prior head.Guard qualification, negative controls, both-host boundary rows, normal-range preservation and JUnit, SHA-256
1d707eb7d61a0a713e0bc1b9632c0e4ed61430572e8c40e0d9aaf51c2f3f46e5.Initial measurement head
a131f0f6a6f57bd5f7293deddff1099f0fab535f, Release, independent CPU-only source/builds:guest_mktime,guest_time_conversion,guest_time_extras,guest_strftime,guest_system_service_time_zone,guest_utime. Command:ctest --test-dir <build> -R '^(guest_mktime|guest_time_conversion|guest_time_extras|guest_strftime|guest_system_service_time_zone|guest_utime)$' --repeat until-fail:3 --output-on-failure.guest_mktimefailsunavailable daylight hint changed the timestamp. Restore candidate source/build and rerun successfully. Full logs, restoration records and JUnit files attached.5d02c43cc567d638269e1ae818fd7030cc1a32a7passes all 18 focused executions and leaves only the same 1899 mismatch across 20,015 rows. Windows source/build restored afterward. Guards check the canonical GuestTm storage; equality compares timestamp plus nine normalized fields, with errno recorded separately.Known limits: 52 directed gap/fold/negative-daylight cases remain different; no local macOS execution or console transition-policy qualification. The separately attributed macOS standalone review is not a full local macOS build; console year-edge behavior remains unmeasured. Host lookup failures propagate. This is a bounded correctness improvement, not complete mktime equivalence.
Raw logs, guarded numeric reference rows, probes, patches, manifest and both-host evidence. SHA-256:
c9c44603bc56a044ac0c960056f6e117a7519c75138c181f39a0ee9aea8cecd9. No SDK/game binaries are included.Independent Windows validation, reported by bunbeans2-arch for topic
7e8465539372e0c6615956deac04adfe5886b30eagainst mainf7f03daf52: Windows 11 Pro 22631, i9-9900K, MSYS2 UCRT64 GCC 16.1, Release.guest_mktimepasses 5/5 with host zoneEastern Standard Time; retaining the new test while restoring the three production files to main fails 5/5 at the unavailable-daylight-hint timestamp assertion; plain main passes 3/3. This is the contributor's run, not a new local Windows execution.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)